(2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension
Xuelei Fan
xuelei.fan at oracle.com
Sun Aug 12 12:52:34 UTC 2012
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.
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.
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);
> 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;
> 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>;
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. 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.
> 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?
> 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.
>
> 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.
> 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
> 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.
Thanks,
Xuelei
More information about the security-dev
mailing list