Re: Code Review Request for translatability bugs

[Weijun Wang]

Looks fine.

Thanks
Max


On 03/29/2011 02:46 AM, Sean Mullan wrote:

I have addressed all of your comments and posted another webrev at:

http://cr.openjdk.java.net/~mullan/webrevs/7019937_et_al/webrev.01/

Max, can you check that I integrated your patch ok?

Thanks,
Sean

On 3/25/11 12:15 AM, Weijun Wang wrote:

Another thing, maybe you can also combine the JarSigner
TSA-unavailable warnings
like you did for KeyTool integrity-not-checked ones?

I've attached a diff and you can directly apply it at jdk level.

Thanks
Max


On 03/25/2011 09:35 AM, Weijun Wang wrote:

AuthResources.java:
===

1.

{"expected.", "expected "},
- {".read.end.of.file", ", read end of file"},
+ {"expected.expect.read.end.of.file.",
+ "expected {0}, read end of file"},

The "expected." is now useless. At least I grep thru all jdk/src files
and see none of it.

2.

- {"provided.null.name", "provided null name"}

This one is still used in:

./com/sun/security/auth/SolarisPrincipal.java:79
./com/sun/security/auth/SolarisNumericGroupPrincipal.java:92
./com/sun/security/auth/SolarisNumericUserPrincipal.java:83
./com/sun/security/auth/X500Principal.java:90

There is an entry with the same name in Resources.java, but
unfortunately our codes use both.


Thanks
Max


On 03/25/2011 04:15 AM, Brad Wetmore wrote:


On 3/24/2011 6:49 AM, Sean Mullan wrote:

Max, Freda,

Could you please review this webrev for a batch of translatability
bugs:

http://cr.openjdk.java.net/~mullan/webrevs/7019937_et_al/webrev.00/


KeyTool.java:
=
Looks good. Only minor comments about line length, which is kind of
hard
in this environment.

Brad

hg: jdk7/tl/jdk: 7031166: (pack200) tools/pack200/CommandLineTests.java fail with testsdk on RO filesystem

[kumar . x . srinivasan]

Changeset: a42760dae179
Author:ksrini
Date:  2011-03-28 13:50 -0700
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/a42760dae179

7031166: (pack200) tools/pack200/CommandLineTests.java fail with testsdk on RO 
filesystem
Reviewed-by: alanb

! test/tools/pack200/CommandLineTests.java
! test/tools/pack200/TimeStamp.java
! test/tools/pack200/Utils.java

Re: Code Review Request for translatability bugs

[Sean Mullan]

I have addressed all of your comments and posted another webrev at:

http://cr.openjdk.java.net/~mullan/webrevs/7019937_et_al/webrev.01/

Max, can you check that I integrated your patch ok?

Thanks,
Sean

On 3/25/11 12:15 AM, Weijun Wang wrote:

Another thing, maybe you can also combine the JarSigner TSA-unavailable warnings
like you did for KeyTool integrity-not-checked ones?

I've attached a diff and you can directly apply it at jdk level.

Thanks
Max


On 03/25/2011 09:35 AM, Weijun Wang wrote:

AuthResources.java:
===

1.

{"expected.", "expected "},
- {".read.end.of.file", ", read end of file"},
+ {"expected.expect.read.end.of.file.",
+ "expected {0}, read end of file"},

The "expected." is now useless. At least I grep thru all jdk/src files
and see none of it.

2.

- {"provided.null.name", "provided null name"}

This one is still used in:

./com/sun/security/auth/SolarisPrincipal.java:79
./com/sun/security/auth/SolarisNumericGroupPrincipal.java:92
./com/sun/security/auth/SolarisNumericUserPrincipal.java:83
./com/sun/security/auth/X500Principal.java:90

There is an entry with the same name in Resources.java, but
unfortunately our codes use both.


Thanks
Max


On 03/25/2011 04:15 AM, Brad Wetmore wrote:


On 3/24/2011 6:49 AM, Sean Mullan wrote:

Max, Freda,

Could you please review this webrev for a batch of translatability bugs:

http://cr.openjdk.java.net/~mullan/webrevs/7019937_et_al/webrev.00/


KeyTool.java:
=
Looks good. Only minor comments about line length, which is kind of hard
in this environment.

Brad

Re: Exception while processing 'no-addresses' flag in KrbApReq.java

[Weijun Wang]

Sorry for the late reply.

I suppose your client side program is not in Java? Because in JDK a 
service ticker's addresses field is always null.


Thanks
Max




On 03/25/2011 07:53 PM, Szabolcs Pota wrote:

[+ adding back security-dev]

Hi Henry,

Thank you for your reply. My answers are below.

On Fri, Mar 25, 2011 at 1:26 AM, Henry B. Hotz mailto:hbh...@dslextreme.com>> wrote:

No-list reply since I'm subscribed with an alias which my ISP won't
let me send with.

On Mar 23, 2011, at 5:16 AM, Szabolcs Pota wrote:

 > Our global krb5.conf files have 'noaddresses=false' for both client
 > and server hence we get this exception. Please correct me if someone
 > thinks that setting this flag to false on the server side would be
 > incorrect.

There are two issues here.  The one you're not looking at is that
that config option is different for every one of the major C
implementations of Kerberos.

[appdefaults]
no-addresses = true # Heimdal
no_addresses = true # Sun

[lidefaults]
noaddresses = true  # MIT

I have no idea which of these is understood by Java, though I would
guess the Sun one, and hope that all of them are.  Also the default
value varies with the version.  AFAIK all now default to disable
address checking.


As I've seen in sun.security.krb5.Config#useAddresses() it reads either
the 'noaddresses' or the 'no-addresses' flag and indeed the default is
no-addresses=true. We are using 'noaddresses' that is parsed without
problems by the current code.

--

As for what you are actually asking about:  almost all of us have
stopped worrying about addresses because the address check does not
work in the real world with ubiquitous NAT and multiple private IP
spaces.  (Are you sure you're not running into one of those?)  I
personally would not care if Java simply stopped supporting address
checking.

That may not be an appropriate thing for the universal JGSS
implementation to do though.

What's *supposed* to happen (without reading the RFC) is the
endpoint gets the IP from the socket for the other end, and compares
it with the appropriate field in the ticket.  If they don't match,
then the ticket *may* have been copied and is being injected from
someplace it shouldn't be.

Since (almost) nobody is using the feature anymore I would actually
be surprised if it works on IPv6 networks.  As I said it is
guaranteed to fail if there is a NAT involved.

-

To answer the specific question in the above paragraph, I would say
checking addresses on a server is actually wrong if *any* of the
clients are connecting via VPN, or through your typical home router
box.  It can only be guaranteed correct if all clients are on the
same corporate network as the server.


I agree with you that checking the client address is error prone and
even the RFC says so. It could be done only on a best effort basis. At
the moment I think that the server should do one of the followings:

   1. If EncTicketPart.caddr is set then try to get the client IP and
  check if it is in the list. If it is not then it *may* throw and
  exception.
   2. Skip the whole no-addresses processing because of the unreliable
  client IP check.

My problem is that the current logic in KrbApReq java does non of these
but throws an Exception. This prevents us using OpenJDK with
'noaddresses=false' in Kerberos configuration.

Regards,

Szabolcs

--
The opinions expressed in this message are mine,
not those of Caltech, JPL, NASA, or the US Government.
henry.b.h...@jpl.nasa.gov , or
hbh...@oxy.edu 






--
Szabolcs Pota
Morgan Stanley | MSJava, EAI (MSSM)
Lechner Odon fasor 8 | Floor 07
Budapest, 1095
Phone: +36 1 881-3979
szabolcs.p...@morganstanley.com 

hg: jdk7/tl/jdk: 2 new changesets

[weijun . wang]

Changeset: 86ace035d04d
Author:weijun
Date:  2011-03-28 18:04 +0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/86ace035d04d

7019384: Realm.getRealmsList returns realms list in wrong (reverse) order
Reviewed-by: xuelei

! src/share/classes/sun/security/krb5/Realm.java
! test/sun/security/krb5/ParseCAPaths.java
! test/sun/security/krb5/krb5-capaths.conf

Changeset: 79cd9368b555
Author:weijun
Date:  2011-03-28 18:04 +0800
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/79cd9368b555

7031536: test/sun/security/krb5/auto/HttpNegotiateServer.java should not use 
static ports
Reviewed-by: xuelei

! test/sun/security/jgss/GssNPE.java
! test/sun/security/jgss/spnego/NoSpnegoAsDefMech.java
! test/sun/security/krb5/ConfPlusProp.java
! test/sun/security/krb5/ConfigWithQuotations.java
! test/sun/security/krb5/DnsFallback.java
! test/sun/security/krb5/IPv6.java
! test/sun/security/krb5/ParseConfig.java
! test/sun/security/krb5/RFC396xTest.java
! test/sun/security/krb5/TimeInCCache.java
! test/sun/security/krb5/auto/HttpNegotiateServer.java
! test/sun/security/krb5/etype/ETypeOrder.java
! test/sun/security/krb5/ktab/HighestKvno.java

Re: code review request: 7031536: test/sun/security/krb5/auto/HttpNegotiateServer.java should not use static ports

[Xuelei Fan]

On 3/28/2011 4:23 PM, Weijun Wang wrote:

Hi Xuelei

This webrev includes 2 kinds of code changes:

   http://cr.openjdk.java.net/~weijun/7031536/webrev.00/

1. HttpNegotiateServer: now servers open on port 0.

Looks fine.


2. Others: I add run/othervm for all tests in jgss and krb5 that call 
System.setProperty


As it only impact the test cases, I did not look into the code very 
carefully. There is no bad impact to add the "run/othervm" for all test. 
So generally, those update looks good to me, too.



Xuelei


Thanks
Max


 Original Message 
*Change Request ID*: 7031536

*Synopsis*: test/sun/security/krb5/auto/HttpNegotiateServer.java 
should not use static ports



=== *Description* 

test/sun/security/krb5/auto/HttpNegotiateServer.java uses static port 
numbers when starting web and proxy servers. If another process uses 
the same ports, test would fail.

code review request: 7031536: test/sun/security/krb5/auto/HttpNegotiateServer.java should not use static ports

[Weijun Wang]

Hi Xuelei

This webrev includes 2 kinds of code changes:

   http://cr.openjdk.java.net/~weijun/7031536/webrev.00/

1. HttpNegotiateServer: now servers open on port 0.

2. Others: I add run/othervm for all tests in jgss and krb5 that call 
System.setProperty


Thanks
Max


 Original Message 
*Change Request ID*: 7031536

*Synopsis*: test/sun/security/krb5/auto/HttpNegotiateServer.java should 
not use static ports



=== *Description* 

test/sun/security/krb5/auto/HttpNegotiateServer.java uses static port 
numbers when starting web and proxy servers. If another process uses the 
same ports, test would fail.