Re: JDK 8 Code Review Request: 6500133/6931888: CertificateParsingException for CDP

2012-08-15 Thread Jason Uh

Thanks, Sean.

New webrev updated with your suggestions:
http://cr.openjdk.java.net/~juh/6500133/webrev.01/

Jason

On 08/15/2012 10:38 AM, Sean Mullan wrote:

This looks good to me. Couple of comments:

111: Can you add a comment, something like "Try parsing the URI again
after encoding/escaping any illegal characters".

113-4: When this code was written there probably wasn't yet an
IOException(String, Throwable) ctor. Now there is, so you can change
this to:

throw new IOException("invalid URI name:" + name, use2);

There are also a couple other places in URIName where you can replace
the same code using initCause with the IOExc ctor above. That's a
low-risk refactoring you can include in this change.

--Sean

On 08/14/2012 11:51 PM, Jason Uh wrote:

Hi all,

This change fixes --
6500133: CertificateParsingException for CRL Distribution Point with
blank; and
6931888: Inconsistent behavior for invalid URI name in cert file

CRs:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6500133
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6931888

They are effectively duplicates, both regarding an exception thrown when
parsing CRL Distribution Point URIs with invalid characters, like a
space or backslash. This change uses
sun.net.www.ParseUtil.encodePath(String) to re-encode bad URIs.

Webrev: http://cr.openjdk.java.net/~juh/6500133/webrev.00/

Thanks,
Jason




Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Brad Wetmore



On 8/15/2012 3:26 AM, Xuelei Fan wrote:

More comments about whether we are able to override the default value.

On 8/15/2012 10:45 AM, Xuelei Fan wrote:

Thought more about the design, I would have to say that we cannot return
the default value in sslParameters.getServerNames().  Otherwise, the
following two block of codes look very weird to me:
  // case one:
1   SSLparameters sslParameters = sslSocket.getSSLParameters();
2   sslParameters.clearServerName("host_name");
3   Map names = sslParameters.getServerNames();
4   sslSocket.setSSLParameters(sslParameters);
5   sslParameters = sslSocket.getSSLParameters();
6   names = sslParameters.getServerNames();

In line 3, the returned map does not contain "host_name" entry. But in
line 6, it may be expected that no "host_name" in the returned map. But
if we want to return default values, line 6 do need to return a map
containing "host_name".  The behavior is pretty confusing. We may want
to try avoid the confusion.


I'm not following your confusion, it seemed pretty straightforward to
me, it works much like CipherSuites.  We have a set of ciphersuites
which are enabled by default.  We can turn some off by using
SSLParameters.

Compatibility is my concerns here.

When the SSLParameters class was was introduced in JDK 6,
set/getCipherSuites() and set/getProtocols were new, so there was no
compatibility issue for these two pairs methods at that time, as they
were not used in old applications.

In JDK 7, we introduced two new methods, set/getAlgorithmConstraints().
We were luck that we cannot allow the override of default constraints
because of security consideration,


I didn't quite follow this sentence.  We do allow the override, right? 
Looking at SSLSocketImpl.setSSLParameters():


algorithmConstraints = params.getAlgorithmConstraints();


and the concept of algorithm
constraints was new in JDK 7. So we needed not to consider the
compatibility issue too much between JDK 6 and 7 for this pair of methods.

However, things get changed for the Server Name Inidication (SNI),
because in JDK 7 we have already support default SNI extension implicit.
We have to consider the compatibility issues more between JDK 7 and JDK
8, we need to make sure the behaviors are consistent whenever we call
the set/getServerName(). If we allow override of default value, the
application may run into corner trap as the description in my previous mail.


Not sure this is a problem, possibly I didn't explain my thought fully.


That's the background of my thoughts. Hope it helps you understand the
current design.


Here's what I was trying to propose:

SSLParameters:

// Local storage like the other variables
private Map sniNames = null;

...deleted...

/**
 * Set the SNI Names.
 *
 * A null parameter means don't set anything.  Nothing is set.
 *
 * disallow invalid String->null entries.
 *
 * valid String->String entries will be added to SNI extension
 * if there is a recognized Key type.
 */
public void setServerNames(Map map)

/**
 * Returns a copy of the array of servernames or null if
 * none have been set.
 */
public Map getServerNames()

SSLSocketImpl:

synchronized public void setSSLParameters(SSLParameters params) {
super.setSSLParameters();
...deleted...
Map sniNames = params.getServerNames();
if (sniNames != null) {
sniNameStorage = sniNames.clone();
}

synchronized public SSLParameters getSSLParameters() {
SSLParameters params = super.getSSLParameters();
...deleted...
// sniNameStorage currently has one entry:
// "host_name", "www.example.com"
params.setServerNames = sniNameStorage.clone();

So looking at the two use cases you pointed out:

1.   SSLParameters sslp = new SSLParameters()

getServernames() would return null.  When SSLSocketImpl.setSSLParameters 
is called, the null indicates there is nothing to set, and the default 
value remains.  If app wants to set something here, it can, which would 
override the existing default.


2.  SSLParameters sslp = SSLSocket.getSSLParameters()

The SSLParameters will be populated with the SNI map value stored in 
SSLSocketImpl/SSLEngineImpl, and apps can do whatever they choose with 
the Map.  When setSSLParameters is called, the potentially new values 
are just pushed back to the SNI.  In this case, the application has full 
access to the default Map and can see what will be sent by default.


In both cases, the default remains unless the application takes steps to 
change it.  For applications, this is the same call model as 
setCipherSuites()/setProtocols(), but the magic happens at different levels.


Am I missing something?

> because in JDK 7 we have already support default SNI extension
> implicit.

We are just providing API access to that functionality.

Brad




Thanks,
Xuelei


  Expanding a bit on your example here, I'll describe what

hg: jdk8/tl/jdk: 6931128: (spec) File attribute tests fail when run as root.

2012-08-15 Thread rob . mckenna
Changeset: da14e2c90bcb
Author:robm
Date:  2012-08-15 22:46 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/da14e2c90bcb

6931128: (spec) File attribute tests fail when run as root.
Reviewed-by: alanb

! src/share/classes/java/io/File.java
! test/java/io/File/Basic.java
! test/java/io/File/SetAccess.java
! test/java/io/File/SetReadOnly.java
! test/java/io/File/SymLinks.java
+ test/java/io/File/Util.java



hg: jdk8/tl/langtools: 7191449: update copyright year to match last edit in jdk8 langtools repository

2012-08-15 Thread james . holmlund
Changeset: 9d47f4850714
Author:jjh
Date:  2012-08-15 13:48 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/9d47f4850714

7191449: update copyright year to match last edit in jdk8 langtools repository
Reviewed-by: jjh
Contributed-by: steve.si...@oracle.com

! make/jprt.properties
! make/tools/anttasks/CompilePropertiesTask.java
! make/tools/anttasks/GenStubsTask.java
! make/tools/anttasks/SelectToolTask.java
! make/tools/compileproperties/CompileProperties.java
! make/tools/genstubs/GenStubs.java
! src/share/classes/com/sun/tools/javac/code/Source.java
! src/share/classes/com/sun/tools/javac/code/Type.java
! src/share/classes/com/sun/tools/javac/comp/AttrContext.java
! src/share/classes/com/sun/tools/javac/comp/TransTypes.java
! src/share/classes/com/sun/tools/javac/file/ZipFileIndex.java
! src/share/classes/com/sun/tools/javac/main/Main.java
! src/share/classes/com/sun/tools/javac/util/RichDiagnosticFormatter.java
! test/tools/javac/ProtectedInnerClass/ProtectedInnerClass.sh
! test/tools/javac/api/7086261/T7086261.java
! test/tools/javac/api/T6397104.java
! test/tools/javac/diags/CheckExamples.java
! test/tools/javac/diags/MessageInfo.java
! test/tools/javac/diags/RunExamples.java
! test/tools/javac/diags/examples/ApplicableMethodFound1.java
! test/tools/javac/diags/examples/IllegalDot.java
! test/tools/javac/diags/examples/InconvertibleTypes.java
! test/tools/javac/diags/examples/KindnameConstructor.java
! test/tools/javac/diags/examples/NotApplicableMethodFound.java
! test/tools/javac/diags/examples/PossibleLossPrecision.java
! test/tools/javac/diags/examples/ResourceNotApplicableToType.java
! test/tools/javac/diags/examples/VarargsArgumentMismatch.java
! test/tools/javac/diags/examples/VerboseResolveMulti1.java
! test/tools/javac/diags/examples/WhereCaptured.java
! test/tools/javac/diags/examples/WhereCaptured1.java
! test/tools/javac/diags/examples/WhereIntersection.java
! test/tools/javac/diags/examples/WhereTypeVar.java
! test/tools/javac/generics/typevars/T7148242.java
! test/tools/javac/newlines/Newlines.sh
! test/tools/javac/parser/T4881269.java
! test/tools/javac/processing/TestWarnErrorCount.java



Re: JDK 8 Code Review Request: 6500133/6931888: CertificateParsingException for CDP

2012-08-15 Thread Sean Mullan

This looks good to me. Couple of comments:

111: Can you add a comment, something like "Try parsing the URI again 
after encoding/escaping any illegal characters".


113-4: When this code was written there probably wasn't yet an 
IOException(String, Throwable) ctor. Now there is, so you can change 
this to:


throw new IOException("invalid URI name:" + name, use2);

There are also a couple other places in URIName where you can replace 
the same code using initCause with the IOExc ctor above. That's a 
low-risk refactoring you can include in this change.


--Sean

On 08/14/2012 11:51 PM, Jason Uh wrote:

Hi all,

This change fixes --
6500133: CertificateParsingException for CRL Distribution Point with
blank; and
6931888: Inconsistent behavior for invalid URI name in cert file

CRs:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6500133
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6931888

They are effectively duplicates, both regarding an exception thrown when
parsing CRL Distribution Point URIs with invalid characters, like a
space or backslash. This change uses
sun.net.www.ParseUtil.encodePath(String) to re-encode bad URIs.

Webrev: http://cr.openjdk.java.net/~juh/6500133/webrev.00/

Thanks,
Jason




Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Xuelei Fan
On Aug 15, 2012, at 11:05 PM, Weijun Wang  wrote:

>>> 
 4. I just noticed that the type can be also "sni-". What if someone
 call setServerName("sni-0", "...")? Is it the same as calling
 setServerName(SNI_HOST_NAME, "...")?
 
>>> Unspecified behavior. Maybe not, because of the value encoding method.
>>> We may throw exception in Oracle provider. Other providers may want to
>>> support "sni-32" or "principal" for its private server name type.
>> 
>> I'm not sure. Suppose tomorrow the type nick_name(1) is supported and a
>> customer begins using "sni-1". The day after tomorrow, Java is updated
>> and we have a string "nick_name" for it. I guess you will make "sni-1"
>> and "nick_home" equivalent so that the customer's app will still work?
>> If so, why not start doing it today for "sni-0" and "host_name"?
> 
> In fact, I don't quite understand how this is used. On the client side, when 
> the developer knows nick_name(1) is not yet supported by the current JDK, he 
> will use the "sni-1" type so that the SNI will be correctly encoded. Then, 
> JDK is updated and the name type is now "nick_name", but I believe the old 
> app should still work, so "sni-1" should be still accepted and be equivalent 
> to "nick_name".
> 
> On the server side, the old JDK and new JDK would return different values for 
> getServerNames(). It's likely that the user app will be broken if it cannot 
> successfully predict the "nick_name" name and already support it.
> 
> Therefore, seriously, I suggest we change Map to 
> Map. Since we already defined SSLParamters constant for the 
> types, the user won't feel uncomfortable.
> 
We have to consider two things, the type and the encoding method of the value.  
For oracle provider, we will not support unknown type in SSLParameters because 
we don't know the encoding method. We support unknown type in session but we 
require the value is encoded in utf-8.

I  do not prefer integer for the type because from the integer type I cannot 
tell whether the type is unknown or not, and then cannot make sure what is the 
proper encoding according. For example, the old application does not know 
"nick-name", so the type integer is 1 and the value encoded as UTF-8, but what 
if the encoding is not UTF-8 in the formal spec?  We will not be able to decode 
the value properly.

Xuelei

> Thanks
> Max
> 


Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Weijun Wang



4. I just noticed that the type can be also "sni-". What if someone
call setServerName("sni-0", "...")? Is it the same as calling
setServerName(SNI_HOST_NAME, "...")?


Unspecified behavior. Maybe not, because of the value encoding method.
We may throw exception in Oracle provider. Other providers may want to
support "sni-32" or "principal" for its private server name type.


I'm not sure. Suppose tomorrow the type nick_name(1) is supported and a
customer begins using "sni-1". The day after tomorrow, Java is updated
and we have a string "nick_name" for it. I guess you will make "sni-1"
and "nick_home" equivalent so that the customer's app will still work?
If so, why not start doing it today for "sni-0" and "host_name"?


In fact, I don't quite understand how this is used. On the client side, 
when the developer knows nick_name(1) is not yet supported by the 
current JDK, he will use the "sni-1" type so that the SNI will be 
correctly encoded. Then, JDK is updated and the name type is now 
"nick_name", but I believe the old app should still work, so "sni-1" 
should be still accepted and be equivalent to "nick_name".


On the server side, the old JDK and new JDK would return different 
values for getServerNames(). It's likely that the user app will be 
broken if it cannot successfully predict the "nick_name" name and 
already support it.


Therefore, seriously, I suggest we change Map to 
Map. Since we already defined SSLParamters constant for 
the types, the user won't feel uncomfortable.


Thanks
Max



Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Weijun Wang



On 08/15/2012 10:34 PM, Xuelei Fan wrote:

On 8/15/2012 10:06 PM, Weijun Wang wrote:

Some suggestions to SSLParameters:

1. Since you now decide to exclude the default SNI value from
getServerNames(), maybe you can emphasis this by using "user-provided
server name" in the spec.


I have a paragraph in setServerName:
  * Note that the returned Map of
  * {@link #setServerName(String, String)} does not contain the provider
  * default server name types and values.

Did you suggest to replace the "desired" with "user-provided" or
"user-specified"?


Yes.




2. At disableServerName(), maybe you can explicitly point out that "A
disabled server name type cannot be used as a part of the server name
indication in SSL/TLS handshaking, even if the provider has a default
value for this server name type".


OK.


3. isDisabledServerName, maybe isServerNameDisabled? Neither sounds
perfect.


I'd like to collect more votes on the name. ;-)


4. I just noticed that the type can be also "sni-". What if someone
call setServerName("sni-0", "...")? Is it the same as calling
setServerName(SNI_HOST_NAME, "...")?


Unspecified behavior. Maybe not, because of the value encoding method.
We may throw exception in Oracle provider. Other providers may want to
support "sni-32" or "principal" for its private server name type.


I'm not sure. Suppose tomorrow the type nick_name(1) is supported and a 
customer begins using "sni-1". The day after tomorrow, Java is updated 
and we have a string "nick_name" for it. I guess you will make "sni-1" 
and "nick_home" equivalent so that the customer's app will still work? 
If so, why not start doing it today for "sni-0" and "host_name"?


Thanks
Max



Thanks for the quick response!

Xuelei


Thanks
Max


On 08/15/2012 09:30 PM, Xuelei Fan wrote:

This paragraph was removed in the latest update. Please ignore the
webrev_spec.04 and previous webrevs, and review webrev_spec.05.

http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/

Thanks,
Xuelei

On 8/15/2012 3:24 PM, Bradford Wetmore wrote:




How would the ServerHello be generated until you call this method and
then start the handshaking on the returned SSLSocket?

One can use SSLEngine.wrap()/unwrap() to generate handshaking messages
from socket I/O stream.
   // accept a socket
   // read/write the I/O with SSLEngine
   // call this method


Switching between SSLSocket/SSLEngine like that would require
intentional misuse of the API.  You could make the same claim for
regular handshaking.

Brad


Xuelei


You said in a
previous internal conversation that SSLExplore would not generate any
ServerHello, so this can't be what you mean.  Are you talking about
layering another SSLSocket over a already layered SSLSocket?


2. "consumed network data is resumable" wasn't clear either.  To me
this
should mean that you can obtain the data which has already been read
from "s".


Yes, need wordsmithing here.


3. "Otherwise, the behavior of the return socket is not defined"
lost
me.  Does this mean that that SSLParameters and assorted settings
are
not otherwise defined?


See above example.


I think I need the above answered before I can comment further here.


I think you could delete this paragraph.

From your second email:


Thought more about the design, I would have to say that we cannot
return
the default value in sslParameters.getServerNames().  Otherwise,
the
following two block of codes look very weird to me:
 // case one:
1   SSLparameters sslParameters = sslSocket.getSSLParameters();
2   sslParameters.clearServerName("host_name");
3   Map names = sslParameters.getServerNames();
4   sslSocket.setSSLParameters(sslParameters);
5   sslParameters = sslSocket.getSSLParameters();
6   names = sslParameters.getServerNames();

In line 3, the returned map does not contain "host_name" entry.
But in
line 6, it may be expected that no "host_name" in the returned
map. But
if we want to return default values, line 6 do need to return a map
containing "host_name".  The behavior is pretty confusing. We may
want
to try avoid the confusion.


I'm not following your confusion, it seemed pretty
straightforward to
me, it works much like CipherSuites.  We have a set of ciphersuites
which are enabled by default.  We can turn some off by using
SSLParameters.  Expanding a bit on your example here, I'll describe
what
I think would happen internally/externally:

1SSLSocket sslSocket = mySSLSocketFactory.createSocket(
"www.example.com", 443);

mySSLSocketFactory sets any initial parameters as usual.
SSLSocketImpl
knows it's connecting to www.example.com and automatically stores
"host_name" -> "www.example.com" in its local host data (map or
separate
variables).

2   SSLparameters sslParameters = sslSocket.getSSLParameters();

SSLSocketImpl.getSSLParameters() creates a SSLParameters, and
sets the
hostmap to the one value "host_name" -> "www.example.com"

If the application want to get the "default values", they just pull
th

Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Sean Mullan

On 08/14/2012 10:45 PM, Xuelei Fan wrote:

I only reply on the items that I may need more review.

On 8/15/2012 7:54 AM, Brad Wetmore wrote:

SSLParameters.java
==
76:  Not sure why you want/need a LinkedHashMap with only one currently
defined NameType.  Even if there were multiple types, I don't think that
SNI requires an ordering.  You also mention this in
setAccessibleServerName, but not sure what purpose this serves.  I'm not
strongly against it, just wondering.


I am also not sure about the strong desire that the SNI should be
ordered. But Weijun prefers it to be ordered because he think the SNI in
RFC6066 is defined as an ordered sequence.

struct {
ServerName server_name_list<1..2^16-1>
} ServerNameList;


I've gone through RFC6066 pretty carefully, and I'm not seeing any
indication that this should be ordered.

In RFC 2246, if there is an ordering required, such as
cipher_suites/compression/certs/cert_requests, it's specifically called
out.  For any other "lists", it is not specified.

Section 7.4.1.2

The CipherSuite list, passed from the client to the server in the
client hello message, contains the combinations of cryptographic
algorithms supported by the client in order of the client's
preference (favorite choice first).

...deleted...

The client hello includes a list of compression algorithms supported
by the client, ordered according to the client's preference.

...deleted...

cipher_suites
This is a list of the cryptographic options supported by the
client, with the client's first preference first.

...deleted...

compression_methods
This is a list of the compression methods supported by the
client, sorted by client preference.

Section 7.4.2

certificate_list
This is a sequence (chain) of X.509v3 certificates. The sender's
certificate must come first in the list.

Section 7.4.4

certificate_types
   This field is a list of the types of certificates requested,
   sorted in order of the server's preference.

Weijun, did you see something else in your read of the spec that
indicates an ordering?  If not, maybe we should not put in the order
wording now.  If it turns out we do need it, we can always add that
wording later in a later release, but it will be impossible to remove it
if we add it now.


I think you are right. If Weijun has no other concerns, I will remove
related description.


Doesn't a "List" imply it is ordered? I guess I'm ok with not putting 
any specific wording in right now, since there's likely only going to be 
one entry anyway, but if subsequent standard name types are defined 
later, order might become important for some reason.


--Sean


Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Xuelei Fan
On 8/15/2012 10:06 PM, Weijun Wang wrote:
> Some suggestions to SSLParameters:
> 
> 1. Since you now decide to exclude the default SNI value from
> getServerNames(), maybe you can emphasis this by using "user-provided
> server name" in the spec.
> 
I have a paragraph in setServerName:
 * Note that the returned Map of
 * {@link #setServerName(String, String)} does not contain the provider
 * default server name types and values.

Did you suggest to replace the "desired" with "user-provided" or
"user-specified"?

> 2. At disableServerName(), maybe you can explicitly point out that "A
> disabled server name type cannot be used as a part of the server name
> indication in SSL/TLS handshaking, even if the provider has a default
> value for this server name type".
> 
OK.

> 3. isDisabledServerName, maybe isServerNameDisabled? Neither sounds
> perfect.
> 
I'd like to collect more votes on the name. ;-)

> 4. I just noticed that the type can be also "sni-". What if someone
> call setServerName("sni-0", "...")? Is it the same as calling
> setServerName(SNI_HOST_NAME, "...")?
> 
Unspecified behavior. Maybe not, because of the value encoding method.
We may throw exception in Oracle provider. Other providers may want to
support "sni-32" or "principal" for its private server name type.

Thanks for the quick response!

Xuelei

> Thanks
> Max
> 
> 
> On 08/15/2012 09:30 PM, Xuelei Fan wrote:
>> This paragraph was removed in the latest update. Please ignore the
>> webrev_spec.04 and previous webrevs, and review webrev_spec.05.
>>
>> http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/
>>
>> Thanks,
>> Xuelei
>>
>> On 8/15/2012 3:24 PM, Bradford Wetmore wrote:
>>>
>>>
> How would the ServerHello be generated until you call this method and
> then start the handshaking on the returned SSLSocket?
 One can use SSLEngine.wrap()/unwrap() to generate handshaking messages
 from socket I/O stream.
   // accept a socket
   // read/write the I/O with SSLEngine
   // call this method
>>>
>>> Switching between SSLSocket/SSLEngine like that would require
>>> intentional misuse of the API.  You could make the same claim for
>>> regular handshaking.
>>>
>>> Brad
>>>
 Xuelei

> You said in a
> previous internal conversation that SSLExplore would not generate any
> ServerHello, so this can't be what you mean.  Are you talking about
> layering another SSLSocket over a already layered SSLSocket?
>
>>> 2. "consumed network data is resumable" wasn't clear either.  To me
>>> this
>>> should mean that you can obtain the data which has already been read
>>> from "s".
>>>
>> Yes, need wordsmithing here.
>>
>>> 3. "Otherwise, the behavior of the return socket is not defined"
>>> lost
>>> me.  Does this mean that that SSLParameters and assorted settings
>>> are
>>> not otherwise defined?
>>>
>> See above example.
>
> I think I need the above answered before I can comment further here.
>
>>> I think you could delete this paragraph.
>>>
>>>From your second email:
>>>
 Thought more about the design, I would have to say that we cannot
 return
 the default value in sslParameters.getServerNames().  Otherwise,
 the
 following two block of codes look very weird to me:
 // case one:
 1   SSLparameters sslParameters = sslSocket.getSSLParameters();
 2   sslParameters.clearServerName("host_name");
 3   Map names = sslParameters.getServerNames();
 4   sslSocket.setSSLParameters(sslParameters);
 5   sslParameters = sslSocket.getSSLParameters();
 6   names = sslParameters.getServerNames();

 In line 3, the returned map does not contain "host_name" entry.
 But in
 line 6, it may be expected that no "host_name" in the returned
 map. But
 if we want to return default values, line 6 do need to return a map
 containing "host_name".  The behavior is pretty confusing. We may
 want
 to try avoid the confusion.
>>>
>>> I'm not following your confusion, it seemed pretty
>>> straightforward to
>>> me, it works much like CipherSuites.  We have a set of ciphersuites
>>> which are enabled by default.  We can turn some off by using
>>> SSLParameters.  Expanding a bit on your example here, I'll describe
>>> what
>>> I think would happen internally/externally:
>>>
>>> 1SSLSocket sslSocket = mySSLSocketFactory.createSocket(
>>>"www.example.com", 443);
>>>
>>> mySSLSocketFactory sets any initial parameters as usual.
>>> SSLSocketImpl
>>> knows it's connecting to www.example.com and automatically stores
>>> "host_name" -> "www.example.com" in its local host data (map or
>>> separate
>>> variables).
>>>
>>> 2   SSLparameters sslParameters = sslSocket.getSSLPa

Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Weijun Wang

Some suggestions to SSLParameters:

1. Since you now decide to exclude the default SNI value from 
getServerNames(), maybe you can emphasis this by using "user-provided 
server name" in the spec.


2. At disableServerName(), maybe you can explicitly point out that "A 
disabled server name type cannot be used as a part of the server name 
indication in SSL/TLS handshaking, even if the provider has a default 
value for this server name type".


3. isDisabledServerName, maybe isServerNameDisabled? Neither sounds perfect.

4. I just noticed that the type can be also "sni-". What if someone 
call setServerName("sni-0", "...")? Is it the same as calling 
setServerName(SNI_HOST_NAME, "...")?


Thanks
Max


On 08/15/2012 09:30 PM, Xuelei Fan wrote:

This paragraph was removed in the latest update. Please ignore the
webrev_spec.04 and previous webrevs, and review webrev_spec.05.

http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/

Thanks,
Xuelei

On 8/15/2012 3:24 PM, Bradford Wetmore wrote:




How would the ServerHello be generated until you call this method and
then start the handshaking on the returned SSLSocket?

One can use SSLEngine.wrap()/unwrap() to generate handshaking messages
from socket I/O stream.
  // accept a socket
  // read/write the I/O with SSLEngine
  // call this method


Switching between SSLSocket/SSLEngine like that would require
intentional misuse of the API.  You could make the same claim for
regular handshaking.

Brad


Xuelei


You said in a
previous internal conversation that SSLExplore would not generate any
ServerHello, so this can't be what you mean.  Are you talking about
layering another SSLSocket over a already layered SSLSocket?


2. "consumed network data is resumable" wasn't clear either.  To me
this
should mean that you can obtain the data which has already been read
from "s".


Yes, need wordsmithing here.


3. "Otherwise, the behavior of the return socket is not defined" lost
me.  Does this mean that that SSLParameters and assorted settings are
not otherwise defined?


See above example.


I think I need the above answered before I can comment further here.


I think you could delete this paragraph.

   From your second email:


Thought more about the design, I would have to say that we cannot
return
the default value in sslParameters.getServerNames().  Otherwise, the
following two block of codes look very weird to me:
// case one:
1   SSLparameters sslParameters = sslSocket.getSSLParameters();
2   sslParameters.clearServerName("host_name");
3   Map names = sslParameters.getServerNames();
4   sslSocket.setSSLParameters(sslParameters);
5   sslParameters = sslSocket.getSSLParameters();
6   names = sslParameters.getServerNames();

In line 3, the returned map does not contain "host_name" entry.
But in
line 6, it may be expected that no "host_name" in the returned
map. But
if we want to return default values, line 6 do need to return a map
containing "host_name".  The behavior is pretty confusing. We may
want
to try avoid the confusion.


I'm not following your confusion, it seemed pretty straightforward to
me, it works much like CipherSuites.  We have a set of ciphersuites
which are enabled by default.  We can turn some off by using
SSLParameters.  Expanding a bit on your example here, I'll describe
what
I think would happen internally/externally:

1SSLSocket sslSocket = mySSLSocketFactory.createSocket(
   "www.example.com", 443);

mySSLSocketFactory sets any initial parameters as usual.
SSLSocketImpl
knows it's connecting to www.example.com and automatically stores
"host_name" -> "www.example.com" in its local host data (map or
separate
variables).

2   SSLparameters sslParameters = sslSocket.getSSLParameters();

SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the
hostmap to the one value "host_name" -> "www.example.com"

If the application want to get the "default values", they just pull
them
out of the SSLParameters here

3   sslParameters.clearServerName("host_name");

Or sslParameters.setServerName("host_name", null)?

User just decided to clear it.  Ok, that's what we do.  It becomes an
empty map in SSLParameters.

4   Map names = sslParameters.getServerNames();

Returns empty Map.


As far as good.


5   sslSocket.setSSLParameters(sslParameters);

SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this
SSLParameters and as a result, clears it's internal "host_name" map to
null, and thus won't send anything out since it's empty.


We have problems here.  We need to support that if an application does
not specified host_name value, we should use default values.
 I.   SSLParameters sslParameters = new SSLParameters();
 II.  sslParameters.setCipherSuites(...);
 III. SSLSocket sslSocket =
 sslSocketFactory.createSocket("www.example.com", 443)
 IV.  sslSocket.setSSLParameters(sslParameters);

Before line IV and after line II, the sslParameters.getServerNames()
are
empty. In li

Re: (3rd Round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Xuelei Fan
On 8/15/2012 9:36 PM, Xuelei Fan wrote:
> Updated webrev according to recent feedbacks:
> http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/
> http://cr.openjdk.java.net./~xuelei/7068321/README_05.txt
> 
> The major differences:
> 1. add constant SSLParameters.SNI_HOST_NAME, and specify the format of
> unknown SNI type (sni-) in ExtendedSSLSession.
>Then we don't need to document the types and values in Java
> Cryptography Architecture Standard Algorithm Name Documentation.
> 
> 2. Other updates according to feedback from Brad, Weijun, and Sean.
>Please let me know I missed misunderstood something.
> 
missed or misunderstood ;-)

> Thanks,
> Xuelei
> 
> On 8/12/2012 8:50 PM, Xuelei Fan wrote:
>> Hi,
>>
>> Please review the spec of JEP 114, TLS Server Name Indication (SNI)
>> Extension.
>>
>> http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/
>>
>> Please read the README to help you understanding the the specification:
>>
>>http://cr.openjdk.java.net./~xuelei/7068321/README_04.txt
>>
>> The major differences comparing with previous webrev are:
>> 1. client mode and server mode will use separated API set.
>>For client, the related APIs are:
>>  setServerName(String type, String value)
>>  clearServerName(String type)
>>  disableServerName(String type)
>>  enableServerName(String type)
>>  isDisabledServerName(String type)
>>  getServerNames()
>>
>>For server side, the related APIs are:
>>  setServerNamePattern(String type, Pattern pattern)
>>  clearServerNamePattern(String type)
>>  getServerNamePatterns()
>>
>> 2. close the door to use the generated socket in client mode.
>>
>>SSLSocketFactory.createSocket(Socket s,
>>InputStream consumed, boolean autoClose)
>>
>>The returned socket was set in server mode.
>>
>> Regards,
>> Xuelei
>>
> 



Re: (3rd Round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Xuelei Fan
Updated webrev according to recent feedbacks:
http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/
http://cr.openjdk.java.net./~xuelei/7068321/README_05.txt

The major differences:
1. add constant SSLParameters.SNI_HOST_NAME, and specify the format of
unknown SNI type (sni-) in ExtendedSSLSession.
   Then we don't need to document the types and values in Java
Cryptography Architecture Standard Algorithm Name Documentation.

2. Other updates according to feedback from Brad, Weijun, and Sean.
   Please let me know I missed misunderstood something.

Thanks,
Xuelei

On 8/12/2012 8:50 PM, Xuelei Fan wrote:
> Hi,
> 
> Please review the spec of JEP 114, TLS Server Name Indication (SNI)
> Extension.
> 
> http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/
> 
> Please read the README to help you understanding the the specification:
> 
>http://cr.openjdk.java.net./~xuelei/7068321/README_04.txt
> 
> The major differences comparing with previous webrev are:
> 1. client mode and server mode will use separated API set.
>For client, the related APIs are:
>  setServerName(String type, String value)
>  clearServerName(String type)
>  disableServerName(String type)
>  enableServerName(String type)
>  isDisabledServerName(String type)
>  getServerNames()
> 
>For server side, the related APIs are:
>  setServerNamePattern(String type, Pattern pattern)
>  clearServerNamePattern(String type)
>  getServerNamePatterns()
> 
> 2. close the door to use the generated socket in client mode.
> 
>SSLSocketFactory.createSocket(Socket s,
>InputStream consumed, boolean autoClose)
> 
>The returned socket was set in server mode.
> 
> Regards,
> Xuelei
> 



hg: jdk8/tl/jdk: 7110151: Use underlying platform's zlib library for Java zlib support

2012-08-15 Thread ahughes
Changeset: 35e024c6a62c
Author:andrew
Date:  2012-08-15 14:35 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/35e024c6a62c

7110151: Use underlying platform's zlib library for Java zlib support
Summary: Make SYSTEM_ZLIB more flexible by using ZLIB_{CFLAGS,LIBS} and 
building on more than just MACOSX.
Reviewed-by: sherman, alanb

! make/com/sun/java/pack/Makefile
! make/common/Program.gmk
! make/common/shared/Defs-linux.gmk
! make/common/shared/Defs-macosx.gmk
! make/common/shared/Defs-solaris.gmk
! make/java/jli/Makefile
! make/java/zip/Makefile
! make/jdk_generic_profile.sh
! make/sun/splashscreen/Makefile
! src/share/native/com/sun/java/util/jar/pack/defines.h
! src/share/native/java/util/zip/Adler32.c
! src/share/native/java/util/zip/CRC32.c
! src/share/native/java/util/zip/Deflater.c
! src/share/native/java/util/zip/Inflater.c
! src/share/native/java/util/zip/zip_util.c



Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Xuelei Fan
This paragraph was removed in the latest update. Please ignore the
webrev_spec.04 and previous webrevs, and review webrev_spec.05.

http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.05/

Thanks,
Xuelei

On 8/15/2012 3:24 PM, Bradford Wetmore wrote:
> 
> 
>>> How would the ServerHello be generated until you call this method and
>>> then start the handshaking on the returned SSLSocket?
>> One can use SSLEngine.wrap()/unwrap() to generate handshaking messages
>> from socket I/O stream.
>>  // accept a socket
>>  // read/write the I/O with SSLEngine
>>  // call this method
> 
> Switching between SSLSocket/SSLEngine like that would require
> intentional misuse of the API.  You could make the same claim for
> regular handshaking.
> 
> Brad
> 
>> Xuelei
>>
>>> You said in a
>>> previous internal conversation that SSLExplore would not generate any
>>> ServerHello, so this can't be what you mean.  Are you talking about
>>> layering another SSLSocket over a already layered SSLSocket?
>>>
> 2. "consumed network data is resumable" wasn't clear either.  To me
> this
> should mean that you can obtain the data which has already been read
> from "s".
>
 Yes, need wordsmithing here.

> 3. "Otherwise, the behavior of the return socket is not defined" lost
> me.  Does this mean that that SSLParameters and assorted settings are
> not otherwise defined?
>
 See above example.
>>>
>>> I think I need the above answered before I can comment further here.
>>>
> I think you could delete this paragraph.
>
>   From your second email:
>
>> Thought more about the design, I would have to say that we cannot
>> return
>> the default value in sslParameters.getServerNames().  Otherwise, the
>> following two block of codes look very weird to me:
>>// case one:
>> 1   SSLparameters sslParameters = sslSocket.getSSLParameters();
>> 2   sslParameters.clearServerName("host_name");
>> 3   Map names = sslParameters.getServerNames();
>> 4   sslSocket.setSSLParameters(sslParameters);
>> 5   sslParameters = sslSocket.getSSLParameters();
>> 6   names = sslParameters.getServerNames();
>>
>> In line 3, the returned map does not contain "host_name" entry.
>> But in
>> line 6, it may be expected that no "host_name" in the returned
>> map. But
>> if we want to return default values, line 6 do need to return a map
>> containing "host_name".  The behavior is pretty confusing. We may
>> want
>> to try avoid the confusion.
>
> I'm not following your confusion, it seemed pretty straightforward to
> me, it works much like CipherSuites.  We have a set of ciphersuites
> which are enabled by default.  We can turn some off by using
> SSLParameters.  Expanding a bit on your example here, I'll describe
> what
> I think would happen internally/externally:
>
> 1SSLSocket sslSocket = mySSLSocketFactory.createSocket(
>   "www.example.com", 443);
>
> mySSLSocketFactory sets any initial parameters as usual. 
> SSLSocketImpl
> knows it's connecting to www.example.com and automatically stores
> "host_name" -> "www.example.com" in its local host data (map or
> separate
> variables).
>
> 2   SSLparameters sslParameters = sslSocket.getSSLParameters();
>
> SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the
> hostmap to the one value "host_name" -> "www.example.com"
>
> If the application want to get the "default values", they just pull
> them
> out of the SSLParameters here
>
> 3   sslParameters.clearServerName("host_name");
>
> Or sslParameters.setServerName("host_name", null)?
>
> User just decided to clear it.  Ok, that's what we do.  It becomes an
> empty map in SSLParameters.
>
> 4   Map names = sslParameters.getServerNames();
>
> Returns empty Map.
>
 As far as good.

> 5   sslSocket.setSSLParameters(sslParameters);
>
> SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this
> SSLParameters and as a result, clears it's internal "host_name" map to
> null, and thus won't send anything out since it's empty.
>
 We have problems here.  We need to support that if an application does
 not specified host_name value, we should use default values.
 I.   SSLParameters sslParameters = new SSLParameters();
 II.  sslParameters.setCipherSuites(...);
 III. SSLSocket sslSocket =
 sslSocketFactory.createSocket("www.example.com", 443)
 IV.  sslSocket.setSSLParameters(sslParameters);

 Before line IV and after line II, the sslParameters.getServerNames()
 are
 empty. In line IV, we need to make sure the internal "host_name",
 "www.example.com" is used as default value, and send it to server in
 SNI.  That's the default behavio

Re: Code review request: 7184246: Simplify Config.get() of krb5

2012-08-15 Thread Weijun Wang

Updated at

   http://cr.openjdk.java.net/~weijun/7184246/webrev.01

Changes:

1. String values (even if there is only one) for the same key are stored 
in a vector. Two methods are provided:


get(String... keys) returns the last value
getAll(String... keys) returns all values concatenated

so if you see

  [realms]
  R = {
kdc = k1
kdc = k2
  }

then get("realms","R","kdc") returns k2, getAll("realms","R","kdc") 
returns "k1 k2".


2. SCDynamicStore is updated to be consistent with above. I also break 
the getConfig() method into 2 so that I can write a test.


Thanks
Max

On 08/02/2012 10:14 PM, Weijun Wang wrote:

Hi Valerie

Please take a look at this

   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7184246

The code changes include:

1. Config.java:
a. Retrieve settings using .get(String... keys) now
b. Some changes to parsing. The sub-section depth can be at any
level. For compatibility reasons, multiple values for the same key are
only for [realms] and [capaths] sections.
c. Still using Hashtable and Vector because I don't want to make
changes to Mac's SCDynamicStoreConfig.m.

2. initStatic() methods in several classes that read krb5.conf settings
to static fields

3. All other old calls to getDefault() methods.

Thanks
Max



 Original Message 
7184246: Simplify Config.get() of krb5
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7184246

=== *Description*

This is about the internal class sun.security.krb5.Config.

If you want to get a value from inside krb5.conf, you can call
getDefault(String). This might be good to get a value from the
[libdefaults] section. However, the method was designed to be so smart
that it can recursively search for key/value pairs no matter where and
how deep it is.

For example, given a krb5.conf

[s1]
a=b

[s2]
c=d

[s3]
e = {
  f = g
}

getDefault("a") = "b", getDefault("c") = "d", and astonishingly,
getDefault("f") = "g".

I don't think this is a good design, for several reasons:

1. It depends on the order of sections if there are key/value pairs with
the same key in different sections.

2. It ignores wrong settings. For example, when doing a cross-realm
auth, the Realm.getRealmsList(from,to) is used to get a path which
should be defined in [capaths]. However, the method simply crawls
recursively into any subsection it found and won't notice the [capaths]
being mistakenly typed as [capath]

3. It lacks certain features. Because the function always return a
String (same with the getDefault(String,String) method), getDefault("e")
can only return a null. Therefore there is no way to find out the
existence of the subsection e unless we also know it contains a key f.

4. The current Config class needs to know what subsections contains more
subsections, and it hardcodes names like [capaths] and [realms].

In short, it's just too smart and becomes unsafe to use. I suggest
removing all this smartness and a user must use the full paths to get a
value, say,

kdc = config.get("realms", "SUN.COM", "kdc")

My proposed spec is:

1. The Config class should understand a krb5.conf without knowing any
specific section names. All it maintains is a Value, which can be either of

 String
 List
 TreeMap

Here I use TreeMap to preserve the order (might not be necessary).

2. The basic retrieval method will be

 Value get(String... key)

3. There are simpler methods if you already know what the type in your
case is

 String getAsString(String... key)
 List getAsStringList(String... key)

The compatibility risk will be low, and if there really comes a
compatibility issue, most likely it will be because the caller had
written his krb5.conf wrong.

One of the advantages of the original design is that when a key is
provided in both [libdefaults] and a given realm, the method can find it
anyway. This will be useful for keys like kdc_timeout, max_retries.
However, I think this automatic retrieval is confusing and error-prone,
I'd rather manually call the get() method twice.



Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Xuelei Fan
More comments about whether we are able to override the default value.

On 8/15/2012 10:45 AM, Xuelei Fan wrote:
>>> >> Thought more about the design, I would have to say that we cannot return
>>> >> the default value in sslParameters.getServerNames().  Otherwise, the
>>> >> following two block of codes look very weird to me:
>>> >>  // case one:
>>> >> 1   SSLparameters sslParameters = sslSocket.getSSLParameters();
>>> >> 2   sslParameters.clearServerName("host_name");
>>> >> 3   Map names = sslParameters.getServerNames();
>>> >> 4   sslSocket.setSSLParameters(sslParameters);
>>> >> 5   sslParameters = sslSocket.getSSLParameters();
>>> >> 6   names = sslParameters.getServerNames();
>>> >>
>>> >> In line 3, the returned map does not contain "host_name" entry. But in
>>> >> line 6, it may be expected that no "host_name" in the returned map. But
>>> >> if we want to return default values, line 6 do need to return a map
>>> >> containing "host_name".  The behavior is pretty confusing. We may want
>>> >> to try avoid the confusion.
>> > 
>> > I'm not following your confusion, it seemed pretty straightforward to
>> > me, it works much like CipherSuites.  We have a set of ciphersuites
>> > which are enabled by default.  We can turn some off by using
>> > SSLParameters.
Compatibility is my concerns here.

When the SSLParameters class was was introduced in JDK 6,
set/getCipherSuites() and set/getProtocols were new, so there was no
compatibility issue for these two pairs methods at that time, as they
were not used in old applications.

In JDK 7, we introduced two new methods, set/getAlgorithmConstraints().
We were luck that we cannot allow the override of default constraints
because of security consideration, and the concept of algorithm
constraints was new in JDK 7. So we needed not to consider the
compatibility issue too much between JDK 6 and 7 for this pair of methods.

However, things get changed for the Server Name Inidication (SNI),
because in JDK 7 we have already support default SNI extension implicit.
We have to consider the compatibility issues more between JDK 7 and JDK
8, we need to make sure the behaviors are consistent whenever we call
the set/getServerName(). If we allow override of default value, the
application may run into corner trap as the description in my previous mail.

That's the background of my thoughts. Hope it helps you understand the
current design.

Thanks,
Xuelei

>> >  Expanding a bit on your example here, I'll describe what
>> > I think would happen internally/externally:
>> > 
>> > 1SSLSocket sslSocket = mySSLSocketFactory.createSocket(
>> > "www.example.com", 443);
>> > 
>> > mySSLSocketFactory sets any initial parameters as usual.  SSLSocketImpl
>> > knows it's connecting to www.example.com and automatically stores
>> > "host_name" -> "www.example.com" in its local host data (map or separate
>> > variables).
>> > 
>> > 2   SSLparameters sslParameters = sslSocket.getSSLParameters();
>> > 
>> > SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the
>> > hostmap to the one value "host_name" -> "www.example.com"
>> > 
>> > If the application want to get the "default values", they just pull them
>> > out of the SSLParameters here
>> > 
>> > 3   sslParameters.clearServerName("host_name");
>> > 
>> > Or sslParameters.setServerName("host_name", null)?
>> > 
>> > User just decided to clear it.  Ok, that's what we do.  It becomes an
>> > empty map in SSLParameters.
>> > 
>> > 4   Map names = sslParameters.getServerNames();
>> > 
>> > Returns empty Map.
>> > 
> As far as good.
> 
>> > 5   sslSocket.setSSLParameters(sslParameters);
>> > 
>> > SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this
>> > SSLParameters and as a result, clears it's internal "host_name" map to
>> > null, and thus won't send anything out since it's empty.
>> > 
> We have problems here.  We need to support that if an application does
> not specified host_name value, we should use default values.
>   I.   SSLParameters sslParameters = new SSLParameters();
>   II.  sslParameters.setCipherSuites(...);
>   III. SSLSocket sslSocket =
>   sslSocketFactory.createSocket("www.example.com", 443)
>   IV.  sslSocket.setSSLParameters(sslParameters);
> 
> Before line IV and after line II, the sslParameters.getServerNames() are
> empty. In line IV, we need to make sure the internal "host_name",
> "www.example.com" is used as default value, and send it to server in
> SNI.  That's the default behaviors in JDK 7.  We cannot break it without
> strong desires.
> 
> I think it means that we cannot clear the internal "host_name" when the
> sslParameters.getServerNames() return empty.
> 
> Does it make sense to you?
> 
> Thanks,
> Xuelei
> 
>> > 6   sslParameters = sslSocket.getSSLParameters();
>> > 
>> > SSLSocketImpl.getSSLParameters() creates a SSLParameters, which sees
>> > that there's no name indication, so it creates an empty name map and
>> > stores in SSLParameters.
>> > 
>> 

Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Bradford Wetmore




How would the ServerHello be generated until you call this method and
then start the handshaking on the returned SSLSocket?

One can use SSLEngine.wrap()/unwrap() to generate handshaking messages
from socket I/O stream.
 // accept a socket
 // read/write the I/O with SSLEngine
 // call this method


Switching between SSLSocket/SSLEngine like that would require 
intentional misuse of the API.  You could make the same claim for 
regular handshaking.


Brad


Xuelei


You said in a
previous internal conversation that SSLExplore would not generate any
ServerHello, so this can't be what you mean.  Are you talking about
layering another SSLSocket over a already layered SSLSocket?


2. "consumed network data is resumable" wasn't clear either.  To me this
should mean that you can obtain the data which has already been read
from "s".


Yes, need wordsmithing here.


3. "Otherwise, the behavior of the return socket is not defined" lost
me.  Does this mean that that SSLParameters and assorted settings are
not otherwise defined?


See above example.


I think I need the above answered before I can comment further here.


I think you could delete this paragraph.

  From your second email:


Thought more about the design, I would have to say that we cannot
return
the default value in sslParameters.getServerNames().  Otherwise, the
following two block of codes look very weird to me:
   // case one:
1   SSLparameters sslParameters = sslSocket.getSSLParameters();
2   sslParameters.clearServerName("host_name");
3   Map names = sslParameters.getServerNames();
4   sslSocket.setSSLParameters(sslParameters);
5   sslParameters = sslSocket.getSSLParameters();
6   names = sslParameters.getServerNames();

In line 3, the returned map does not contain "host_name" entry. But in
line 6, it may be expected that no "host_name" in the returned map. But
if we want to return default values, line 6 do need to return a map
containing "host_name".  The behavior is pretty confusing. We may want
to try avoid the confusion.


I'm not following your confusion, it seemed pretty straightforward to
me, it works much like CipherSuites.  We have a set of ciphersuites
which are enabled by default.  We can turn some off by using
SSLParameters.  Expanding a bit on your example here, I'll describe what
I think would happen internally/externally:

1SSLSocket sslSocket = mySSLSocketFactory.createSocket(
  "www.example.com", 443);

mySSLSocketFactory sets any initial parameters as usual.  SSLSocketImpl
knows it's connecting to www.example.com and automatically stores
"host_name" -> "www.example.com" in its local host data (map or separate
variables).

2   SSLparameters sslParameters = sslSocket.getSSLParameters();

SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the
hostmap to the one value "host_name" -> "www.example.com"

If the application want to get the "default values", they just pull them
out of the SSLParameters here

3   sslParameters.clearServerName("host_name");

Or sslParameters.setServerName("host_name", null)?

User just decided to clear it.  Ok, that's what we do.  It becomes an
empty map in SSLParameters.

4   Map names = sslParameters.getServerNames();

Returns empty Map.


As far as good.


5   sslSocket.setSSLParameters(sslParameters);

SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this
SSLParameters and as a result, clears it's internal "host_name" map to
null, and thus won't send anything out since it's empty.


We have problems here.  We need to support that if an application does
not specified host_name value, we should use default values.
I.   SSLParameters sslParameters = new SSLParameters();
II.  sslParameters.setCipherSuites(...);
III. SSLSocket sslSocket =
sslSocketFactory.createSocket("www.example.com", 443)
IV.  sslSocket.setSSLParameters(sslParameters);

Before line IV and after line II, the sslParameters.getServerNames() are
empty. In line IV, we need to make sure the internal "host_name",
"www.example.com" is used as default value, and send it to server in
SNI.  That's the default behaviors in JDK 7.  We cannot break it without
strong desires.

I think it means that we cannot clear the internal "host_name" when the
sslParameters.getServerNames() return empty.

Does it make sense to you?


Ok, that's an issue, I was assuming people would generally get a
SSLParameters from the SSLSocket/SSLEngine, which would have
prepopulated values such as CipherSuites/Protocols and SNI values. Now I
hear what you're saying.

So in SSLSocket/SSLEngine, we have a very similar situation with the
CipherSuites/Protocols.  The way we did it there was if
SSLParameters.getCipherSuites() is null (that is, has not been set in
this instance), we let the default values stand (that is we did not call
SSLSocket.setEnabledCipherSuites()).  You could do something like that
with the SNI info.  I've always felt the setServerName/getServerName
should take/return the

Re: (2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

2012-08-15 Thread Xuelei Fan
On 8/15/2012 2:10 PM, Bradford Wetmore wrote:
> 
> 
> On 8/14/2012 7:45 PM, Xuelei Fan wrote:
> 
> 197:  You're not planning to process (e.g.
> ServerHandshaker/ClientHandshaker.process_message) the consumed
> handshaking bytes immediately during the createSocket call, are
> you? You
> still need to allow the user time to set the socket
> options/SSLParameters/etc.  I was expecting in this method you'd just
> suck in the consumed bytes into temporary storage, then create/return
> the socket, and then when the handshaking is started, you then read
> out
> from the temporary storage until you run out, then you switch to the
> Socket's InputStream.
>
 You're right. It is allowed to set more options in the returned socket
 before kick off handshake.

> 197:  This needs some wordsmithing here.  This method will produce the
> SSLSocket that will be consuming this data, so of course it has to be
> called first.
>
 I'm not sure I understand your point. Please comment it again with the
 revised APIs if you still have concerns.
>>>
>>> I just didn't understand much of this paragraph.
>>>
>>> 1. You have to call this method, then set up your parameters, then start
>>> your handshaking.  So the first half of this sentence doesn't apply.
>>>
>> Oh, I know your concerns. What I want to express is that before the
>> calling to method, the caller should not do real handshaking. The logic
>> I concerned looks like:
>>  // 1. accept a socket
>>  // 2. read ClientHello and reply ServerHello to output stream.
>>  // 3. call this method
>>  SSLSocket sslSocket = (SSLSocket)sslSocketFactory.createSocket(
>>  socke, inputStream, true);
>>
>> because the handshaking has started in step 2, then in step 3, we cannot
>> get a proper SSLSocket.
> 
> How would the ServerHello be generated until you call this method and
> then start the handshaking on the returned SSLSocket?
One can use SSLEngine.wrap()/unwrap() to generate handshaking messages
from socket I/O stream.
// accept a socket
// read/write the I/O with SSLEngine
// call this method

Xuelei

> You said in a
> previous internal conversation that SSLExplore would not generate any
> ServerHello, so this can't be what you mean.  Are you talking about
> layering another SSLSocket over a already layered SSLSocket?
> 
>>> 2. "consumed network data is resumable" wasn't clear either.  To me this
>>> should mean that you can obtain the data which has already been read
>>> from "s".
>>>
>> Yes, need wordsmithing here.
>>
>>> 3. "Otherwise, the behavior of the return socket is not defined" lost
>>> me.  Does this mean that that SSLParameters and assorted settings are
>>> not otherwise defined?
>>>
>> See above example.
> 
> I think I need the above answered before I can comment further here.
> 
>>> I think you could delete this paragraph.
>>>
>>>  From your second email:
>>>
 Thought more about the design, I would have to say that we cannot
 return
 the default value in sslParameters.getServerNames().  Otherwise, the
 following two block of codes look very weird to me:
   // case one:
 1   SSLparameters sslParameters = sslSocket.getSSLParameters();
 2   sslParameters.clearServerName("host_name");
 3   Map names = sslParameters.getServerNames();
 4   sslSocket.setSSLParameters(sslParameters);
 5   sslParameters = sslSocket.getSSLParameters();
 6   names = sslParameters.getServerNames();

 In line 3, the returned map does not contain "host_name" entry. But in
 line 6, it may be expected that no "host_name" in the returned map. But
 if we want to return default values, line 6 do need to return a map
 containing "host_name".  The behavior is pretty confusing. We may want
 to try avoid the confusion.
>>>
>>> I'm not following your confusion, it seemed pretty straightforward to
>>> me, it works much like CipherSuites.  We have a set of ciphersuites
>>> which are enabled by default.  We can turn some off by using
>>> SSLParameters.  Expanding a bit on your example here, I'll describe what
>>> I think would happen internally/externally:
>>>
>>> 1SSLSocket sslSocket = mySSLSocketFactory.createSocket(
>>>  "www.example.com", 443);
>>>
>>> mySSLSocketFactory sets any initial parameters as usual.  SSLSocketImpl
>>> knows it's connecting to www.example.com and automatically stores
>>> "host_name" -> "www.example.com" in its local host data (map or separate
>>> variables).
>>>
>>> 2   SSLparameters sslParameters = sslSocket.getSSLParameters();
>>>
>>> SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the
>>> hostmap to the one value "host_name" -> "www.example.com"
>>>
>>> If the application want to get the "default values", they just pull them
>>> out of the SSLParameters here
>>>
>>> 3   sslParameters.clearServerName("host_name");
>>>
>>> Or sslParameters.setServerName("