On 08/15/2012 10:45 AM, 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.

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.

No, I didn't read anything else. However, I cannot think of a reason why we will need to remove it later.


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

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<String, String> 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:

1    SSLSocket 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<String, String> 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?

This is how I understand this problem: There are two ways you get a SSLParameters object with an empty server name map.

1. SSLparameters sslParameters = sslSocket.getSSLParameters();
   sslParameters.clearServerName("host_name");

2. SSLParameters sslParameters = new SSLParameters();

Now these 2 objects are identical. If you call sslSocket.setSSLParameters(sslParameters) now, what should it do? By looking at 1, it seems you cleared something; but by 2, you touched nothing and it should go default.


Thanks
Max


Thanks,
Xuelei

Reply via email to