On 6/2/2015 11:23 PM, Xuelei Fan wrote:
src/java.base/share/classes/javax/net/ssl/ExtendedSSLSession.java
=================================================================
List<String> getReceivedApplicationProtocols()
----------------------------------------------
C1. It might be useful to know the client requested application
protocols in some circumstance. Better to set the application protocols
in client side either.
For ALPN, the client supplies the list, the server chooses. For NPN,
the server supplies the list, the client chooses. So it's a list of
received (requested) protocols. I've changed it for now, not a big deal
to me.
I'd like to use:
- * Gets the application protocol value(s) received from the peer
- * for this connection.
+ * Gets a {@link List} of requested application protocol value(s)
+ * for this connection.
I've never seen a link inside an opening sentence. I have seen @code,
but there's only 6 in the entire java namespace. The
- public List<String> getReceivedApplicationProtocols() {
+ public List<String> getRequesedApplicationProtocols() {
I'll assume that was supposed to be requested. :)
C2. The return value would better to be immutable, and better to
describe the preference per RFC 7301. Maybe looks like:
- * @return the non-null list of application protocol names
+ * @return the non-null immutable list of application protocol
+ * names, in descending order of preference. The returned
+ * list may be empty if no application protocols were
+ * requested.
Done.
src/java.base/share/classes/javax/net/ssl/SSLParameters.java
============================================================
List<String> getApplicationProtocols()
--------------------------------------
C3. Better to indicate explicitly that this method only apply to client
mode.
See above.
C4. The method description is not instinctive enough for an application
developer. Can we use words to indicate the purpose of the setting?
For example:
- * Gets the list of application-level protocol names that could
- * be sent to the SSL/TLS peer.
+ * Gets the {@link List} of application layer protocol names that
+ * can be negotiated over SSL/TLS/DTLS protocols.
Changed, except for the @link.
C5. Prefer to use immutable return value:
- * @return a non-null list of application protocol names.
+ * @return a non-null immutable list of application protocol names.
Changed.
C6. Nice to have a link for the standard protocol names.
I wasn't planning on having a list of standard protocol names.
See below.
void setApplicationProtocols(List<String> protocols)
----------------------------------------------------
C7. see C3.
C8. see C4.
Changed.
s/getApplicationProtocolSelector()
----------------------------------
C9. The use of SSLFunction<SSLBase, String> make the implementation of
protocol selector and JSSE provider implementation complicated.
From the spec, looks like the selector may want to know address/ports,
SSL protocol versions or the negotiated cipher suit. As would require
that before use this selector, the handshaking must negotiate the
protocol version and the cipher suite. That's a specific JSSE
implementation requirement. It does not sound like a reasonable behavior.
The implement of the selector is not straightforward, I think.
Per section 4, RFC 7301:
"... The
"application_layer_protocol_negotiation" ServerHello extension is
intended to be definitive for the connection (until the connection is
renegotiated) and is sent in plaintext to permit network elements to
provide differentiated service for the connection when the TCP or UDP
port number is not definitive for the application-layer protocol to
be used in the connection. By placing ownership of protocol
selection on the server, ALPN facilitates scenarios in which
certificate selection or connection rerouting may be based on the
negotiated protocol."
Per my understanding, application protocol should be negotiated before
cipher suite and protocol version negotiated. And the connection may be
rerouted (even to different machines) for further operation. The
requested application protocols list should be the only information for
the selection of a suitable application protocol.
Based on that, I think it is more simple to use Simone's proposal:
@FunctionalInterface
interface ApplicationProtocolSelector
{
String select(List<String> protocols) throws SSLException;
}
Hence, no need for a SSLBase any more.
There's been a lot of discussion this morning, I'll return to this later
when I've had a chance to parse it.
public static final String AP_HTTP_1_1 = "http/1.1";
public static final String AP_H2 = "h2";
----------------------------------------
C10. I understand why the constants are defined here. However, usually,
we don't define standard names in JSSE APIs. Instead, we normally use
Oracle standard names documentation.
This is not really a Standard Name in our normal sense/usage. Usually
it's a mapping from a name string to some type of value (e.g. TLSv1 ->
0x0301, "SSL_RSA_WITH_3DES_EDE_CBC_SHA" -> 0x000a, "SHA1withRSA" ->
1.2.840.113549.1.1.5. In this case, it's the actual value, and will
depends on the decoding.
src/java.base/share/classes/javax/net/ssl/SSLSession.java
=========================================================
String getCipherSuite()
-----------------------
Pretty hard to use this method with the new specification. It's not a
necessary update, see #C9.
I'll come back to this as per above.
Brad
Hope it helps!
Xuelei
On 6/3/2015 8:56 AM, Bradford Wetmore wrote:
Hi,
I have just posted the second iteration of the ALPN proposal which
hopefully addresses all of the comments raised here. It is in javadoc
format, but things can certainly be adjusted:
http://cr.openjdk.java.net/~wetmore/8051498/webrev.00/
Please see the archive [1] for previous design discussion. I will be
writing up usage examples in the next few days.
The significant changes:
ExtendedSSLSession
public List<String> getReceivedApplicationProtocols() {
This will allow applications to examine which protocol names were
sent.
SSLParameters
mentions both ALPN/NPN as possible protocols. I removed the
discussion about "server" and "client" since ALPN/NPN essentially
reverse the roles.
mentions "appropriate character sets" for String-byte[] conversions
such as UTF-8 for ALPN.
The application protocol selector is now a @FunctionalInterface
(i.e. lambda-ready) called SSLFunction. It is to throw an
SSLException if no values are supported, or null if you want to
treat as an unknown extension.
Defined constants for HTTP/1.1 and HTTP/2.
SSLSession
Called out that getHandshakeSession's ciphersuite may vary until
selected.
SSLBase
A new marker interface that SSLEngine/SSLSocket will implement.
This will allow for a single SSLFunction instead of having
SSLFunctionSSLEngine and SSLFunctionSSLSocket. It does require
that the lambda do a instanceof, unless we were to move the common
Socket/Engine APIs into this class.
Thanks,
Brad
[1]
http://mail.openjdk.java.net/pipermail/security-dev/2015-May/012183.html
