From 5998295908b86ccf522db2936a63506bc59c8ea6 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Fri, 9 Oct 2020 12:02:54 -0400 Subject: [PATCH] Support byte-encoded application protocols This will let GREASE'd ALPN values be handled correctly in JSS, despite the JDK not working well. Additionally, handle NSS's quirks w.r.t. ALPN protocol preference. Signed-off-by: Alexander Scheel --- org/mozilla/jss/nss/NextProtoResult.java | 2 +- org/mozilla/jss/ssl/javax/JSSEngine.java | 41 ++++++++--- .../jss/ssl/javax/JSSEngineReferenceImpl.java | 2 + org/mozilla/jss/ssl/javax/JSSParameters.java | 72 ++++++++++++++++--- org/mozilla/jss/tests/TestSSLEngine.java | 8 ++- 5 files changed, 102 insertions(+), 23 deletions(-) diff --git a/org/mozilla/jss/nss/NextProtoResult.java b/org/mozilla/jss/nss/NextProtoResult.java index 32b844d55..8324deae7 100644 --- a/org/mozilla/jss/nss/NextProtoResult.java +++ b/org/mozilla/jss/nss/NextProtoResult.java @@ -24,7 +24,7 @@ public NextProtoResult(int state_value, byte[] protocol) { } public String getProtocol() { - if (protocol == null || protocol.length == 0) { + if (protocol == null) { return null; } diff --git a/org/mozilla/jss/ssl/javax/JSSEngine.java b/org/mozilla/jss/ssl/javax/JSSEngine.java index 81ac09ad8..d2181c6da 100644 --- a/org/mozilla/jss/ssl/javax/JSSEngine.java +++ b/org/mozilla/jss/ssl/javax/JSSEngine.java @@ -190,7 +190,7 @@ public abstract class JSSEngine extends javax.net.ssl.SSLEngine { /** * Set of possible application protocols to negotiate. */ - protected String[] alpn_protocols; + protected byte[][] alpn_protocols; /** * Constructor for a JSSEngine, providing no hints for an internal @@ -392,8 +392,8 @@ public void setSSLParameters(SSLParameters params) { // When we have a non-zero number of ALPNs, use them in the // negotiation. - if (parsed.getApplicationProtocols() != null) { - setApplicationProtocols(parsed.getApplicationProtocols()); + if (parsed.getRawApplicationProtocols() != null) { + setApplicationProtocols(parsed.getRawApplicationProtocols()); } } @@ -921,6 +921,15 @@ public boolean getWantClientAuth() { * Set a specific list of protocols to negotiate next for ALPN support. */ public void setApplicationProtocols(String[] protocols) { + JSSParameters parser = new JSSParameters(); + parser.setApplicationProtocols(protocols); + alpn_protocols = parser.getRawApplicationProtocols(); + } + + /** + * Set a specific list of protocols to negotiate next for ALPN support. + */ + public void setApplicationProtocols(byte[][] protocols) { alpn_protocols = protocols; } @@ -951,19 +960,31 @@ public String getHandshakeApplicationProtocol() { */ public byte[] getALPNWireData() { int length = 0; - for (String protocol : alpn_protocols) { - length += 1 + protocol.getBytes(StandardCharsets.UTF_8).length; + for (byte[] protocol : alpn_protocols) { + length += 1 + protocol.length; } byte[] result = new byte[length]; int offset = 0; - for (String protocol : alpn_protocols) { - byte[] p_bytes = protocol.getBytes(StandardCharsets.UTF_8); - result[offset] = (byte) p_bytes.length; + // XXX: Handle custom encoding left over from NPN draft: NSS's + // SSL_SetNextProtoNego takes the first protocol and helpfully puts + // it at the end of the list for us... So when we're validating using + // the default ALPN callback handler, switch the first to the last to + // ensure we're passing it in the caller's specified order. + byte[] last = alpn_protocols[alpn_protocols.length - 1]; + result[offset] = (byte) last.length; + offset += 1; + System.arraycopy(last, 0, result, offset, last.length); + offset += last.length; + + // Now copy the rest of the protocols. + for (int index = 0; index < alpn_protocols.length - 1; index++) { + byte[] protocol = alpn_protocols[index]; + result[offset] = (byte) protocol.length; offset += 1; - System.arraycopy(p_bytes, 0, result, offset, p_bytes.length); - offset += p_bytes.length; + System.arraycopy(protocol, 0, result, offset, protocol.length); + offset += protocol.length; } return result; diff --git a/org/mozilla/jss/ssl/javax/JSSEngineReferenceImpl.java b/org/mozilla/jss/ssl/javax/JSSEngineReferenceImpl.java index 479b0a36c..93fe703c2 100644 --- a/org/mozilla/jss/ssl/javax/JSSEngineReferenceImpl.java +++ b/org/mozilla/jss/ssl/javax/JSSEngineReferenceImpl.java @@ -970,6 +970,8 @@ private void updateSession() { ret.state != SSLNextProtoState.SSL_NEXT_PROTO_NO_OVERLAP) { session.setNextProtocol(ret.getProtocol()); + } else { + session.setNextProtocol(""); } } } catch (Exception e) { diff --git a/org/mozilla/jss/ssl/javax/JSSParameters.java b/org/mozilla/jss/ssl/javax/JSSParameters.java index bd014ce73..0fac271a6 100644 --- a/org/mozilla/jss/ssl/javax/JSSParameters.java +++ b/org/mozilla/jss/ssl/javax/JSSParameters.java @@ -28,7 +28,7 @@ public class JSSParameters extends SSLParameters { private SSLVersionRange range; private String alias; private String hostname; - private String[] appProtocols; + private byte[][] appProtocols; public JSSParameters() { // Choose our default set of SSLParameters here; default to null @@ -205,20 +205,74 @@ public void setApplicationProtocols(String[] protocols) throws IllegalArgumentEx return; } - int index = 0; - for (String protocol : protocols) { - if (protocol != "" && protocol.getBytes(StandardCharsets.UTF_8).length > 255) { - String msg = "Invalid application protocol " + protocol; - msg += ": standard allows up to 255 characters but was "; - msg += protocol.length(); + byte[][] converted = new byte[protocols.length][]; + for (int index = 0; index < protocols.length; index++) { + if (protocols[index] == null) { + String msg = "Invalid application protocol (index: "; + msg += index + "): null"; throw new IllegalArgumentException(msg); } + + converted[index] = protocols[index].getBytes(StandardCharsets.UTF_8); } - appProtocols = protocols; + setApplicationProtocols(converted); } - public String[] getApplicationProtocols() { + public void setApplicationProtocols(byte[][] protocols) throws IllegalArgumentException { + if (protocols == null || protocols.length == 0) { + appProtocols = null; + return; + } + + int total_length = 0; + byte[][] result = new byte[protocols.length][]; + for (int index = 0; index < protocols.length; index++) { + byte[] protocol = protocols[index]; + if (protocol == null) { + String msg = "Invalid application protocol (index: "; + msg += index + "): null"; + throw new IllegalArgumentException(msg); + } + + if (protocol.length == 0 || protocol.length > 255) { + String msg = "Invalid application protocol (index: "; + msg += index + "): RFC 7301 allows up to 255 "; + msg += "characters but was " + protocol.length; + msg += ": " + new String(protocol); + throw new IllegalArgumentException(msg); + } + + result[index] = Arrays.copyOf(protocol, protocol.length); + total_length += 1 + protocol.length; + } + + // XXX: NSS's ssl3_ValidateAppProtocol doesn't validate the total + // length. Stop early. + if (total_length >= (1 << 16)) { + String msg = "Total length of encoded protocols exceeds encoded "; + msg += "maximum allowable by RFC 7301: " + total_length; + msg += ">= 65536"; + throw new IllegalArgumentException(msg); + } + + appProtocols = result; + } + + public byte[][] getRawApplicationProtocols() { return appProtocols; } + + public String[] getApplicationProtocols() { + if (appProtocols == null) { + return null; + } + + String[] result = new String[appProtocols.length]; + for (int index = 0; index < appProtocols.length; index++) { + result[index] = new String(appProtocols[index], StandardCharsets.UTF_8); + } + + return result; + } } diff --git a/org/mozilla/jss/tests/TestSSLEngine.java b/org/mozilla/jss/tests/TestSSLEngine.java index df1a29ebd..776da9fc0 100644 --- a/org/mozilla/jss/tests/TestSSLEngine.java +++ b/org/mozilla/jss/tests/TestSSLEngine.java @@ -1001,8 +1001,10 @@ public static void testALPNHandshake(SSLContext ctx, String server_alias) throws } try { - testBasicHandshake(client_eng, server_eng, false); - assert(server_eng.getApplicationProtocol().equals("h2")); + testInitialHandshake(client_eng, server_eng); + assert client_eng.getApplicationProtocol().equals("h2"); + assert server_eng.getApplicationProtocol().equals("h2"); + testClose(client_eng, server_eng); } catch (Exception e) { client_eng.cleanup(); server_eng.cleanup(); @@ -1046,7 +1048,7 @@ public static void testALPNEncoding() throws Exception { eng = new JSSEngineReferenceImpl(); eng.setApplicationProtocols(new String[] { "http/1.1", "spdy/2" }); - byte[] expectedHTTPSpdy = new byte[] { 0x08, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31, 0x06, 0x73, 0x70, 0x64, 0x79, 0x2f, 0x32 }; + byte[] expectedHTTPSpdy = new byte[] { 0x06, 0x73, 0x70, 0x64, 0x79, 0x2f, 0x32, 0x08, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31 }; assert Arrays.equals(eng.getALPNWireData(), expectedHTTPSpdy); }