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

Xuelei Fan xuelei.fan at oracle.com
Wed Aug 15 02:45:48 UTC 2012

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 servernamelist<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 ciphersuites/compression/certs/certrequests, 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... ciphersuites This is a list of the cryptographic options supported by the client, with the client's first preference first. ...deleted... compressionmethods This is a list of the compression methods supported by the client, sorted by client preference. Section 7.4.2 certificatelist This is a sequence (chain) of X.509v3 certificates. The sender's certificate must come first in the list. Section 7.4.4 certificatetypes 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.

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 hostname type? For example, a table with: type value ==== ===== hostname ASCII representation of a DNS hostname Sorry, I did misunderstand your point. Yes, the doc will looks like your above example.

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. I mean the "host" param is the server host, and the "port" param is the server port, and the returned value is connected to the server "host" and server "port", so it should be a client. But, it is not straightforward. I'm OK to make it more clear.

197: You're not planning to process (e.g. ServerHandshaker/ClientHandshaker.processmessage) 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.

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 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("hostname"); 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 "hostname" entry. But in line 6, it may be expected that no "hostname" in the returned map. But if we want to return default values, line 6 do need to return a map containing "hostname". 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 "hostname" -> "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 "hostname" -> "www.example.com" If the application want to get the "default values", they just pull them out of the SSLParameters here 3 sslParameters.clearServerName("hostname"); Or sslParameters.setServerName("hostname", 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 "hostname" 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. 7 names = sslParameters.getServerNames(); returns empty. It's no longer the default value, because they have specifically set the value. HTH, Brad

More information about the security-dev mailing list