On 12/8/2016 6:09 PM, David M. Lloyd wrote:
> How could it turn out to be incorrect, ...
Here is some potentials regarding the cipher suite and application protocol:
C1. select cipher suite at first, and then select the application protocol.
There is an issue if there is no application protocol suitable to the
selected cipher suites [Problem A].
This is what JDK-8170282 wants to address.
C2. select application protocol at first, and then select the cipher suite.
We have the getHandshakeApplicationProtocol() APIs that can be used to
detect if a cipher suite can work with the handshake application protocol.
There is an issue if there is no cipher suite suitable to the selected
application protocols [Problem B]
C3. select application protocol and cipher suite at the same time.
The application layer implementation and TLS implementation could be
very complicated. We don't want it.
With this update (JDK-8170282), we can provide two scenarios for
applications. If applications really care about Problem B, they can use
#C1; otherwise, #C2 is a more simple solution.
http://cr.openjdk.java.net/~vinnie/8170282/webrev.01/
The spec part:
setHandshakeApplicationProtocolSelector:
---------------------------------------
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 it supports the
following type parameters:
This method would work in server side only. You mention this point in
the apiNote part. I'd like to spec this point in the beginning part.
The setApplicationProtocols() method is still useful for client side to
configure the supported application protocols. In the 2nd paragraph, it
may be nice to mention the override only impact the server side behaviors.
Except the explicit client application protocols passed in as a
parameter, it's no very clear how much information the callback
implementation can rely on. For example, can the callback get the
negotiated protocol, the negotiated cipher suites and the negotiated
server name indicator? I'd like to have a implNote so as to suggest
some handshake session fields settings, so that a JSSE provider can have
a better sense about how to set the handshake session, and application
developer know what kind of information can be retrieved from the
handshake session reliably.
The implementation part:
ServerHandshaker.java
---------------------
The current application protocol selection scenarios looks like:
1. select an application protocol independently according to the
s/getetApplicationProtocols() spec. (line 536-560).
2. then select an application protocol suitable to the current session
context, according to the spec of
s/getHandshakeApplicationProtocolSelector() spec. (line 897-909)
3. if no application protocols was selected in #2, use the result in #1.
(linr 912-919)
4. if no application protocol was selected, fatal alert
(alert_no_application_protocol). (line 912)
5. if a wrong application protocol was selected, fatal alert
(alert_no_application_protocol). (line 913)
For #4, previous, server just don't response the ALPN extension (old
code line 895-897). For better interop, I think we'd better keep this
behavior although the spec suggests a fatal response (section 3.2, RFC
7301) . Application layer can handle it better if negotiated
application protocol is really required.
For #5, it is actually an issue of the callback implementation, I think.
The callback implement should select the application protocol from the
passed in arrays only. I'd like to use fatal "internal_error" alert
instead.
Fro #3, it is a little confusing to me, and is not strictly follow the
spec. The spec say "overrides" #1 (SSLEngine spec line 1341) and if #2
result is not available or illegal, the underlying impl "will determine"
(SSLEngine spec line 1354-1357). I think the spec is fine. To be more
simple, I would like to choose one method only, and don't join them
together. The joining may make the developer confusing about which
method exactly works in the underlying black box. Another side effect
is that the cipher suite selector may use the application protocol
selected in #1, and then the callback part (#20 may not work unbiasedly
as expected.
I was thinking that the scenarios may looks like:
N1. if callback is not set, select the application protocol
independently according to the s/getetApplicationProtocols() spec.
N2. if the callback is set, select the application protocol according to
the spec of s/getHandshakeApplicationProtocolSelector() spec.
N3. if no application protocol can be negotiated, don't use the ALPN
extension.
N4. if a wrong application protocol was selection, the server side run
into an internal problem, fatal with internal_error.
Xuelei
On 12/9/2016 1:08 PM, Vincent Ryan wrote:
Thanks for your detailed review Brad. I’ve generated a new webrev:
http://cr.openjdk.java.net/~vinnie/8170282/webrev.01/
On 9 Dec 2016, at 01:34, Bradford Wetmore <bradford.wetm...@oracle.com> wrote:
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
