(2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
Brad Wetmore
bradford.wetmore at oracle.com
Tue Aug 14 23:54:00 UTC 2012
On 8/12/2012 5:52 AM, Xuelei Fan wrote:
> I revised the APIs to separate the differnt server name values and will
> send it in another thread.
> http://cr.openjdk.java.net./~xuelei/7068321/webrev_spec.04/
> http://cr.openjdk.java.net./~xuelei/7068321/README_04.txt
>
> Only reply on those items we may have different opinions.
I'll address the 3rd Round's CR in another email. This will contain
just responses to your comments to keep the message flow in the archives.
> On 8/10/2012 8:37 AM, Brad Wetmore wrote:
>>
>> SSLParameters.java
>> ==================
>>
>> One general question before we get to specifics. Your current default
>> behavior of the SunJSSE is to add a SNI extension if we have the value
>> available. So if we call:
>>
>> sslSocket = socketFactory.createSocket("www.example.com", 443);
>> sslp = sslParameters.getSSLParameters();
>>
>> will this sslParameters ever contain a map with preinstalled "host_name"
>> set to "www.example.com", or will it be empty? I think the answer will
>> be empty. This API is just a way to force setting the value if an
>> implementation select an unwanted value.
>>
> No, it is not empty. The default value will appear in the SSLParameters
> in my prototype implementation.
That's good to know, that will help make things much easier to
understand, rather than a "null" value which is interpreted in a
undefined way underneath. I'll keep this in mind in the next review.
Although I just found your second email. Talk more there.
> But that's an interesting topic to discuss as there are a few concerns I
> have to consider when I design the APIs.
>
> There are public SSLParameters constructors. As means that an instance
> of SSLParameters is not always got from a SSLSocket or SSLEngine
> instance. Then we are not always able to have the *default* value of an
> instance of SSLSocket/SSLEngine into SSLParameters if it is not bound to
> SSLSocket/SSLEngine. So we still need to discuss about how the user
> specified values work with the default ones.
>
> SSLParameters sslParameters = new SSLParameters();
> ...
> sslSocket.setSSLParameters(sslParameters);
We have the same situation with
ciphersuites/protocols/endpointAlgs/AlgorithmConstraints. I was
expecting you'd do the same thing as the last two, that is, they're
interpreted at the SSLSocketImpl level. If they're null (not set), then
something similar needs to happen here. I'll also keep that in mind in
the next review.
>> 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.
>> 295: So what does value = "" mean now? Will that send a SNI extension
>> with an empty string, or will no SNI entry be sent? If null is passed,
>> then it's up to the provider to decide what to do, but there doesn't
>> seem to be a way to shut off SNI like in your old proposal.
>>
> The APIs was revised. But even in the new APIs, it is still a concern.
> According to the spec of host_name, a string length of a hostname should
> be greater than 1:
>
> opaque HostName<1..2^16-1>;
Yes, of course, I missed that! Ok, I'll wait to see what your new API
looks like.
> The SSLParameters will not check the validity of the value. The checking
> will be checked later during handshaking. With the revised APIs, empty
> string is not valid. An exception will be thrown during the preparing of
> SNI extension.
>
>> 308: What kinds of things are you thinking of writing in the Additional
>> Standard Names doc for "value"?
>>
> That was a hot discussion, I think we had talked about it a lot.
You missed my original point, sorry for dragging you into that
discussion. I was talking about the "value" field not the "name" field.
In that section, you indicated that the "value" field was going to be
talked about in the Standard Algorithm Name document. I was wondering
what you were going to be talking about there, just that this needs to
be an ASCII value of a DNS hostname if it's a host_name type? For
example, a table with:
type value
==== =====
host_name ASCII representation of a DNS hostname
> This
> API is not necessary to be forward compatibility with new name type of
> server name indication. We may be able to use subclasses of "ServerName"
> class, or ServerNameType enum for SSLParameters. As will make the
> methods look straightforward. For example:
> sslParameters.setServerName(ServerName);
> or:
> sslParameters.setServerName(ServerNameType.HOST_NAME,
> "www.example.com");
>
> But we lose the flexibility to support new defined server name type in
> the future if we don't define new APIs. For example, when IETF defines a
> new server name type, "cert_principal", we would have to add a new
> subclass of ServerName to represent "cert_principal", or extend the
> ServerNameType with more enum items. Otherwise, we cannot support
> "cert_principal".
>
> It's not the biggest concerns of mine. Although it is not that
> flexible, but we still can extend the APIs in the future.
>
> My biggest concerns is that we need the forward compatibility in
> ExtendedSSLSession.getRequestedServerNames():
>
> public Map<String, String> getRequestedServerNames();
>
> We need to be able to parser unknown server name types in server mode.
> Otherwise, we are not able to get the requested server names.
>
> Even we have to use the string-type-and-string-value style in
> ExtendedSSLSession, I want to keep it consistent in SSLParameters,
> rather than define new "ServerName" class or "ServerNameType" enum.
>
> I think that's also the traditional approach in JSSE to make forward
> compatibility, e.g, the TLS protocols, the cipher suites, etc.
>
>> Would it be sufficient to just say "this is the SNI hostname value"?
>
> See above, not all values are for host_name. The value format may be
> different, we need to describe the format in an additional doc.
This was my question! ;)
>> Even though you have a link to java.util.regex.Pattern in the actual
>> API, you might also mention here in textual form that these are
>> java.util.regex patterns, and add a @link java.util.regex. Makes it
>> more clear where to go to learn about how to specify patterns.
>>
>
>> 387: This is a pattern and not a String, so only listing
>> setAccessibleServerName() isn't quite complete. You might also list
>> "and also see the Pattern page for details on specifying regular
>> expression patterns".
>
> IMHO, I did not see strong necessary of the description. The parameter
> is not a string regular expression, it is an instance of Pattern. When
> one want to use the Pattern class, definitely, he knows it an instance
> related java.util.regex patterns.
>
> It is a pattern and not a string, why we should describe the string
> regular expression here?
I withdraw my comment. They can click through using the Pattern if
needed. I might suggest capitalizing Pattern here though.
"accessibhle" -> "accessible"
I might also suggest:
"is used to match alternative server names" ->
"is used to match server names"
since it's for the main and alternate names.
>> 379: So what does "" mean now? Will an empty string SNI extension be
>> accepted? Or we will
>>
> See aove, empty string ("") is invalid, the application will run into
> exception during handshaking for empty string hostname.
Ok. Will look at this in your next rev.
>> SSLSocketFactory.java
>> =====================
>>
>> Since you're leaving the door open for this socket being either client
>> or server, shouldn't the API then be similar to the existing layered
>> socket one, just including the additional InputStream here:
>>
>> public abstract Socket createSocket(Socket s, InputStream is,
>> String host, int port, boolean autoClose)
>>
>> We might be able to glean SNI information from the host here.
>>
> To make it more straightforward, I closed the door for this socket being
> in client mode in the revised APIs.
As you like. I would think adding an InputStream to the existing API is
just as straightforward also for learning the API, but since we would
only expect this to be used in server mode, your point has merit.
>> 171: Interesting, the existing layered createSocket doesn't mention
>> anything about whether client or server mode, and that you must set the
>> mode or suffer with the default. Might want to file a bug and CCC this
>> also.
>>
> Not really need the description of the mode. The existing description
> has already implied that the return socket must be in client mode.
>
> * @param host the server host
> * @param port the server port
> * @return a socket connected to the specified host and port
You mean because it's a "named host"? I can kind of buy that. But
currently it's kind of subtle that it will be in client mode. I was
thinking we should make it more clear.
>> 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.
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".
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?
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<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.
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.
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.
7 names = sslParameters.getServerNames();
returns empty.
It's no longer the default value, because they have specifically set the
value.
HTH,
Brad
> Thanks,
> Xuelei
>
More information about the security-dev
mailing list