[9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

[Bradford Wetmore]

Hi Vinnie,

On 12/8/2016 2:18 PM, Vincent Ryan wrote:
> The Java Servlet Expect Group reported that they have identified a specific HTTP2 server use-case that cannot
> be easily addressed using the existing ALPN APIs.
>
> This changeset fixes that problem. It supports a new callback mechanism to allow TLS server applications
> to set an application protocol during the TLS handshake. Specifically it allows the cipher suite chosen by the
> TLS protocol implementation to be examined by the TLS server application before it sets the application protocol.
> Additional TLS parameters are also available for inspection in the callback function.
>
> This new mechanism is available only to TLS server applications. TLS clients will continue to use the existing ALPN APIs.

Technically, the API could be used for NPN (though it's pretty much 
dead), so that would be a listing the options on the server side, and 
selection on client side.

> Bug: https://bugs.openjdk.java.net/browse/JDK-8170282
> Webrev: http://cr.openjdk.java.net/~vinnie/8170282/webrev.00/

SSLEngineImpl.java/SSLSocketImpl.java
=====================================
Please use the standard handshaker initialization pattern where the 
Handshaker is initialized with all of the data/mechanisms needed for a 
complete handshake.  This will will ensure consistent handshake results 
and avoid potential strange race conditions.  (There's some 
corresponding API comments below.)

You would register your callback after the 
handshaker.setApplicationProtocols() calls at (currently) line 444 in 
the SSLEngineImpl code.


SSLEngine.java/SSLSocket.java
=============================
I would suggest putting an introduction to this addition in the class 
description section, that application values can be set using 
SSLParameters.setApplication...() and selected with the default 
algorithm, or that a more accurate selection mechanism can be created by 
registering the callback that could look at any Handshake in progress 
and make appropriate decisions.

1339:
Registers the callback function that selects an application protocol
value during the SSL/TLS/DTLS handshake.
->
Registers a callback function that selects an application protocol
value for a SSL/TLS/DTLS handshake.  The function overrides any values 
set using {@link SSLParameters#setApplicationProtocols 
SSLParameters.setApplicationProtocols}.

and remove the corresponding section from the return docs (in the {@code 
String section}).

the function's first argument enables the current
handshake settings to be inspected.<br>
->
the function's first argument allows the current SSLEngine(SSLSocket) to 
be inspected, including the handshake session and configuration 
settings.<br>

If null is returned, or a value that was not advertised
then the underlying protocol will determine what action
to take.
(For example, ALPN will send a "no_application_protocol"
alert and terminate the connection.)
->
If the return value is null (no value chosen) or is a value that was not 
advertised by the peer, the underlying protocol will determine what 
action to take.  (For example, ALPN will send a 
"no_application_protocol" alert and terminate the connection.)

Also, TLS should be configured with parameters that
->
Also, this SSLEngine(SSLSocket) should be configured with parameters that

@param selector the callback function, or null to de-register.
->
@param selector the callback function, or null to disable the callback 
functionality.

Retrieves the callback function that selects an application protocol
value during the SSL/TLS/DTLS handshake.
->
Retrieves the callback function that selects an application protocol
value during a SSL/TLS/DTLS handshake.

     This method should be called by TLS protocol implementations during
     the TLS handshake. The callback function should not be called until
     after the cipher suite has been selected.

I would suggest removing this apiNote entirely.  I expect only 
applications will call this method, so the first sentence is not 
necessary since it's up to the implementation how it wants to store the 
BiFunction.  I expect that when the handshaker is initialized, the 
current BiFunction will be assigned to it, and thus can't be changed for 
the current handshake/Handshaker in progress.  The second sentence ties 
an ordering to the selection of ciphersuite and ALPN value, and will tie 
our hands if we ever revisit the group protocol/ciphersuite/SNI/ALPN 
selection discussion.

ServerHandshaker.java
=====================
I was expecting that the ALPN callback logic would be an update for our 
current ALPN decision logic.  If a callback was set, use it, else look 
at defined strings from the SSLParameters, else fail.  e.g.

     ALPNExtension clientHelloALPN = (ALPNExtension)
         mesg.extensions.get(ExtensionType.EXT_ALPN);

     if (clientHelloALPN != null) {
         List<String> protocols = clientHelloALPN.getPeerAPs();
         if (applicationSelector != null) {
             applicationProtocol =
                 selector.apply(SSLEngine/SSLSocket, peerAPs);
         } else if (localApl.length > 0)) {
             // Intersect the requested and the locally supported,
             // and save for later.  Use server preference order
             for (String ap : localApl) {
                 ...deleted...
             }
             applicationProtocol = negotiatedValue;
         }
         if (negotiatedValue == null) {
             fatalSE(Alerts.alert_no_application_protocol,
                 new SSLHandshakeException(
                     "No matching ALPN values"));
         }
     }

Thanks.

Brad