TLS ALPN Proposal v5

[Bradford Wetmore]

On 9/23/2015 2:33 AM, Simone Bordet wrote:
> Hi,
>
> On Wed, Sep 23, 2015 at 7:04 AM, Bradford Wetmore
> <bradford.wetmore at oracle.com> wrote:
>>
>>> This new proposal still requires that ciphers are sorted in a way that
>>> matches the ApplicationProtocol order.
>>> Would be nice if, along with the HTTP/2 blacklist, there is a HTTP/2
>>> comparator that sorts ciphers putting the blacklisted ones at the end.
>>
>> Hm...is the sample code at the end of the initial class description
>> insufficient?  Adding a comparator seems a little heavyweight in that it
>> could require access to the ciphersuite internals and would add a lot of
>> complexity for this one known use case.  When TLSv1.3 is done, the blacklist
>> stuff in HTTP/2 goes away.
>
> Sure, but until TLS 1.3 widely deployed, applications will have to
> sort the ciphers to put HTTP/2 ones before the blacklisted ones.
> Providing this comparator is as trivial as providing
> ApplicationProtocol.HTTP2BLACKLIST, so I thought to mention it.

I learned something today: Collections/Arrays.sort()/others are stable. 
  (i.e. equal elements will not be reordered as a result of the sort) 
My expectation of "equals" was .equals(), not return value == 0.

I was concerned that such a Comparator might reorder valid H2 suites 
from what was passed in.  Thankfully, that's not the case, so I've added 
this Comparator.  There is a warning now about "consistency with 
equals()", as the Strings obvioulsy won't be equals() and thus sorted 
sets/maps just won't work.  (See the Comparator pages for further 
discussion.)

>>> I also don't understand why there are 2 methods for the protocol name
>>> ? What value does it bring to have 2 methods for the same thing ?
>>
>> Please see the IANA registry:
>>
>> http://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
>>
>> for RFC 7301:
>>
>>      http://www.rfc-editor.org/info/rfc7301
>>
>> getProtocolName() is the IANA/IETF textual representation of the protocol
>> name (i.e. "Protocol" column), for example "HTTP/1.1", "SPDY/3", and "HTTP/2
>> over TLS".  I suppose toString() could be used instead, but thought it might
>> eventually output additional ALPN value state.  I don't have any concrete
>> plans at this point.
>>
>> getNetworkSequence() is the identification sequence for the protocol (i.e.
>> "Identification Sequence" column), and represents the actual byte
>> identifiers that will travel the network in an ALPN extension.
>>
>>      0x68 0x74 0x74 0x70 0x2f 0x31 0x2e 0x31 ("http/1.1")
>>      0x73 0x70 0x64 0x79 0x2f 0x33 ("spdy/3")
>>      0x68 0x32 ("h2")
>>
>> When client wants to send the extension over the network, it grabs the
>> ApplicationProtocols values from the SSLParameters, then calls
>> getNetworkSequence() on each ApplicationProtocol to obtain the actual opaque
>> ProtocolName(1..2^8-1) to send.  Likewise on the server side, we match the
>> incoming active ALPN opaque values with the list of mutually agreeable ALPN
>> values.  And of course, send back the final selected value.
>
> Sure, but application will have to implement two methods instead of
> one, and AFAIU the JDK implementation is never calling
> getProtocolName() since it's just a description for humans.

I think that a textual name will be better than:

     // Output:  javax.net.ssl.ApplicationProtocol$1 at 1b9e1916

     System.out.println(ApplicationProtocol.H2);

and there's no UTF-8 ambiguity.

>> I've updated the webrev to include an SSLSocket test variant, and added a
>> few more comments.
>>
>>      http://cr.openjdk.java.net/~wetmore/8051498/webrev.14/
>>
>> Hopefully things are more clear now.  Thanks for your review/comments.
>
> I see now, thanks for the pointers !
>
> Indulge me a bit more below on the Map passed as parameter to
> ApplicationProtocol :)
>
> IIUC, by the time we are executing the code that calls
> ApplicationProtocol.match(), the TLS protocol is already chosen and
> it's available in SSLSession.

Not necessarily.  This could also be called before the connection has 
even been attempted, say if the client wants to determine if the 
proposed protocols or protocol/ciphersuites it wants to use are even 
allowed by an ApplicationProtocol.

> When remains is the transient value of cipher that is being chosen.
> Because we already have modified the API to support the application
> protocol transient value (by adding
> SSLEngine.getHandshakeApplicationProtocol()) to be used by
> KeyManagers, I was wondering if we cannot either:

CipherSuite is more of a Session value, so it should probably be part of 
the handshakeSSLSession.  We could set 
handshakeSSLSession.getCipherSuite() before calling the ALPN selector, 
or pass it in as part of the Map.

 > A) add: String SSLEngine.getHandshakeCipherSuite(), to be used by
 > ApplicationProtocol

That's kind of what we are doing already, but just in the 
ApplicationProtocol matcher instead so that it doesn't add extra methods 
to SSLSocket/SSLEngine.

And this doesn't really help the pre-connection situation where you want 
to query/filter out unacceptable values.

     http://cr.openjdk.java.net/~wetmore/8051498/webrev.15/

1.  New H2BLACKLISTCOMPARATOR

2.  Renamed the HTTP2BLACKLIST to H2BLACKLIST

Brad