diff --git a/android/src/main/java/org/conscrypt/Platform.java b/android/src/main/java/org/conscrypt/Platform.java index c4e447a49..1bdf9ccc5 100644 --- a/android/src/main/java/org/conscrypt/Platform.java +++ b/android/src/main/java/org/conscrypt/Platform.java @@ -254,6 +254,9 @@ private static void setSSLParametersOnImpl(SSLParameters params, SSLParametersIm Method m_getUseCipherSuitesOrder = params.getClass().getMethod("getUseCipherSuitesOrder"); impl.setUseCipherSuitesOrder((boolean) m_getUseCipherSuitesOrder.invoke(params)); + + Method getNamedGroupsMethod = params.getClass().getMethod("getNamedGroups"); + impl.setNamedGroups((String[]) getNamedGroupsMethod.invoke(params)); } public static void setSSLParameters( @@ -323,6 +326,9 @@ private static void getSSLParametersFromImpl(SSLParameters params, SSLParameters Method m_setUseCipherSuitesOrder = params.getClass().getMethod("setUseCipherSuitesOrder", boolean.class); m_setUseCipherSuitesOrder.invoke(params, impl.getUseCipherSuitesOrder()); + + Method setNamedGroupsMethod = params.getClass().getMethod("setNamedGroups", String[].class); + setNamedGroupsMethod.invoke(params, (Object[]) impl.getNamedGroups()); } public static void getSSLParameters( diff --git a/common/src/main/java/org/conscrypt/NativeSsl.java b/common/src/main/java/org/conscrypt/NativeSsl.java index 6c21a076a..ebf915ed9 100644 --- a/common/src/main/java/org/conscrypt/NativeSsl.java +++ b/common/src/main/java/org/conscrypt/NativeSsl.java @@ -386,14 +386,25 @@ void initialize(String hostname, OpenSSLKey channelIdPrivateKey) throws IOExcept ssl, this, parameters.enabledCipherSuites, parameters.enabledProtocols); } - String namedGroupsProperty = System.getProperty("jdk.tls.namedGroups"); - if (namedGroupsProperty == null || namedGroupsProperty.isEmpty()) { - // If the property is not set or empty, use the default named groups. See: - // https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html - setDefaultNamedGroups(ssl, this); + String[] paramsNamedGroups = parameters.getNamedGroups(); + // - If the named groups are null, we use the default groups. + // - If the named groups are not null, it overrides the default groups. + // - Unknown curves are ignored. + // See: + // https://docs.oracle.com/en/java/javase/25/docs/api/java.base/javax/net/ssl/SSLParameters.html#getNamedGroups() + if (paramsNamedGroups != null) { + NativeCrypto.SSL_set1_groups(ssl, this, toBoringSslGroups(paramsNamedGroups)); } else { - int[] groups = parseNamedGroupsProperty(namedGroupsProperty); - NativeCrypto.SSL_set1_groups(ssl, this, groups); + // Use default named group. + String namedGroupsProperty = System.getProperty("jdk.tls.namedGroups"); + if (namedGroupsProperty == null || namedGroupsProperty.isEmpty()) { + // If the property is not set or empty, use the default named groups. See: + // https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html + setDefaultNamedGroups(ssl, this); + } else { + int[] groups = parseNamedGroupsProperty(namedGroupsProperty); + NativeCrypto.SSL_set1_groups(ssl, this, groups); + } } if (parameters.applicationProtocols.length > 0) { diff --git a/common/src/main/java/org/conscrypt/SSLParametersImpl.java b/common/src/main/java/org/conscrypt/SSLParametersImpl.java index f2056f2bd..f78b8691b 100644 --- a/common/src/main/java/org/conscrypt/SSLParametersImpl.java +++ b/common/src/main/java/org/conscrypt/SSLParametersImpl.java @@ -82,6 +82,8 @@ final class SSLParametersImpl implements Cloneable { // cannot be customized, so for simplicity this field never contains any TLS 1.3 suites. String[] enabledCipherSuites; + String[] namedGroups; + // if the peer with this parameters tuned to work in client mode private boolean client_mode = true; // if the peer with this parameters tuned to require client authentication @@ -363,6 +365,21 @@ void setEnabledProtocols(String[] protocols) { enabledProtocols = NativeCrypto.checkEnabledProtocols(filteredProtocols).clone(); } + void setNamedGroups(String[] namedGroups) { + if (namedGroups == null) { + this.namedGroups = null; + return; + } + this.namedGroups = namedGroups.clone(); + } + + String[] getNamedGroups() { + if (namedGroups == null) { + return null; + } + return this.namedGroups.clone(); + } + /* * Sets the list of ALPN protocols. */ diff --git a/common/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java b/common/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java index 7b5e6132b..bd021b70f 100644 --- a/common/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java +++ b/common/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java @@ -833,20 +833,32 @@ public void setSSLParameters_invalidCipherSuite_throwsIllegalArgumentException() } } + boolean sslParametersSupportsNamedGroups() throws SecurityException { + try { + Method unused = SSLParameters.class.getMethod("getNamedGroups"); + return true; + } catch (NoSuchMethodException e) { + return false; + } + } + @Test - public void setAndGetSSLParameters_withSetNamedGroups_isIgnored() throws Exception { + public void setAndGetSSLParameters_withSetNamedGroups_worksIfSupported() throws Exception { SSLSocketFactory sf = (SSLSocketFactory) SSLSocketFactory.getDefault(); try (SSLSocket ssl = (SSLSocket) sf.createSocket()) { SSLParameters parameters = new SSLParameters( new String[] {"TLS_AES_128_GCM_SHA256"}, new String[] {"TLSv1.3"}); + assertArrayEquals(null, getNamedGroupsOrNull(ssl.getSSLParameters())); + setNamedGroups(parameters, new String[] {"foo", "bar"}); ssl.setSSLParameters(parameters); SSLParameters sslParameters = ssl.getSSLParameters(); - // getNamedGroups currently returns null because setNamedGroups is not supported. - // This is allowed, see: - // https://docs.oracle.com/en/java/javase/24/docs/api/java.base/javax/net/ssl/SSLParameters.html#getNamedGroups() - assertArrayEquals(null, getNamedGroupsOrNull(sslParameters)); + if (sslParametersSupportsNamedGroups()) { + assertArrayEquals(new String[] {"foo", "bar"}, getNamedGroupsOrNull(sslParameters)); + } else { + assertArrayEquals(null, getNamedGroupsOrNull(sslParameters)); + } } } @@ -984,6 +996,184 @@ public void handshake_namedGroupsProperty_usesFirstKnownEntry() throws Exception context.close(); } + @Test + public void handshake_p256IsSupportedByDefault() throws Exception { + TestSSLContext context = TestSSLContext.create(); + final SSLSocket client = (SSLSocket) context.clientContext.getSocketFactory().createSocket( + context.host, context.port); + final SSLSocket server = (SSLSocket) context.serverSocket.accept(); + Future s = runAsync(() -> { + server.startHandshake(); + return null; + }); + Future c = runAsync(() -> { + SSLParameters parameters = client.getSSLParameters(); + setNamedGroups(parameters, new String[] {"P-256"}); + client.setSSLParameters(parameters); + client.startHandshake(); + return null; + }); + s.get(); + c.get(); + // By default, BoringSSL uses X25519, P-256, and P-384. + if (sslParametersSupportsNamedGroups()) { + // If the client requests P-256, it will be chosen. + assertEquals("P-256", getCurveName(client)); + assertEquals("P-256", getCurveName(server)); + } else { + // Otherwise, X25519 gets priority. + assertEquals("X25519", getCurveName(client)); + assertEquals("X25519", getCurveName(server)); + } + client.close(); + server.close(); + context.close(); + } + + @Test + public void handshake_p384IsSupportedByDefault() throws Exception { + TestSSLContext context = TestSSLContext.create(); + final SSLSocket client = (SSLSocket) context.clientContext.getSocketFactory().createSocket( + context.host, context.port); + final SSLSocket server = (SSLSocket) context.serverSocket.accept(); + Future s = runAsync(() -> { + SSLParameters parameters = server.getSSLParameters(); + // secp384r1 is an alias for P-384. + setNamedGroups(parameters, new String[] {"secp384r1"}); + server.setSSLParameters(parameters); + server.startHandshake(); + return null; + }); + Future c = runAsync(() -> { + client.startHandshake(); + return null; + }); + s.get(); + c.get(); + // By default, BoringSSL uses X25519, P-256, and P-384. + if (sslParametersSupportsNamedGroups()) { + // If the client requests P-384, it will be chosen. + assertEquals("P-384", getCurveName(client)); + assertEquals("P-384", getCurveName(server)); + } else { + // Otherwise, X25519 gets priority. + assertEquals("X25519", getCurveName(client)); + assertEquals("X25519", getCurveName(server)); + } + client.close(); + server.close(); + context.close(); + } + + @Test + public void handshake_setsNamedGroups_usesFirstServerNamedGroupThatClientSupports() + throws Exception { + TestSSLContext context = TestSSLContext.create(); + final SSLSocket client = (SSLSocket) context.clientContext.getSocketFactory().createSocket( + context.host, context.port); + final SSLSocket server = (SSLSocket) context.serverSocket.accept(); + Future s = runAsync(() -> { + SSLParameters parameters = server.getSSLParameters(); + setNamedGroups(parameters, new String[] {"P-384", "X25519"}); + server.setSSLParameters(parameters); + server.startHandshake(); + return null; + }); + Future c = runAsync(() -> { + SSLParameters parameters = client.getSSLParameters(); + setNamedGroups(parameters, new String[] {"P-521", "X25519", "P-384"}); + client.setSSLParameters(parameters); + client.startHandshake(); + return null; + }); + s.get(); + c.get(); + if (sslParametersSupportsNamedGroups()) { + // P-384 is the first named group in the server's list that both support. + assertEquals("P-384", getCurveName(client)); + assertEquals("P-384", getCurveName(server)); + } else { + // The defaults are used, and X25519 gets priority. + assertEquals("X25519", getCurveName(client)); + assertEquals("X25519", getCurveName(server)); + } + client.close(); + server.close(); + context.close(); + } + + @Test + public void handshake_withX25519MLKEM768_works() throws Exception { + TestSSLContext context = TestSSLContext.create(); + final SSLSocket client = (SSLSocket) context.clientContext.getSocketFactory().createSocket( + context.host, context.port); + final SSLSocket server = (SSLSocket) context.serverSocket.accept(); + Future s = runAsync(() -> { + SSLParameters parameters = server.getSSLParameters(); + setNamedGroups(parameters, new String[] {"X25519MLKEM768"}); + server.setSSLParameters(parameters); + server.startHandshake(); + return null; + }); + Future c = runAsync(() -> { + SSLParameters parameters = client.getSSLParameters(); + setNamedGroups(parameters, new String[] {"X25519MLKEM768"}); + client.setSSLParameters(parameters); + client.startHandshake(); + return null; + }); + s.get(); + c.get(); + if (sslParametersSupportsNamedGroups()) { + assertEquals("X25519MLKEM768", getCurveName(client)); + assertEquals("X25519MLKEM768", getCurveName(server)); + } else { + // The defaults are used, and X25519 gets priority. + assertEquals("X25519", getCurveName(client)); + assertEquals("X25519", getCurveName(server)); + } + client.close(); + server.close(); + context.close(); + } + + @Test + public void handshake_namedGroupsDontIntersect_throwsException() throws Exception { + TestSSLContext context = TestSSLContext.create(); + final SSLSocket client = (SSLSocket) context.clientContext.getSocketFactory().createSocket( + context.host, context.port); + final SSLSocket server = (SSLSocket) context.serverSocket.accept(); + Future s = runAsync(() -> { + SSLParameters parameters = server.getSSLParameters(); + setNamedGroups(parameters, new String[] {"X25519", "P-384"}); + server.setSSLParameters(parameters); + server.startHandshake(); + return null; + }); + Future c = runAsync(() -> { + SSLParameters parameters = client.getSSLParameters(); + setNamedGroups(parameters, new String[] {"P-256", "P-521"}); + client.setSSLParameters(parameters); + client.startHandshake(); + return null; + }); + if (sslParametersSupportsNamedGroups()) { + ExecutionException serverException = assertThrows(ExecutionException.class, s::get); + assertTrue(serverException.getCause() instanceof SSLHandshakeException); + ExecutionException clientException = assertThrows(ExecutionException.class, c::get); + assertTrue(clientException.getCause() instanceof SSLHandshakeException); + } else { + s.get(); + c.get(); + // The defaults are used, and X25519 gets priority. + assertEquals("X25519", getCurveName(client)); + assertEquals("X25519", getCurveName(server)); + } + client.close(); + server.close(); + context.close(); + } + @Test public void handshake_namedGroupsProperty_failsIfAllValuesAreInvalid() throws Exception { System.setProperty("jdk.tls.namedGroups", "invalid,invalid2"); diff --git a/openjdk/src/main/java/org/conscrypt/Java9PlatformUtil.java b/openjdk/src/main/java/org/conscrypt/Java9PlatformUtil.java index 6e3fae650..aef049c06 100644 --- a/openjdk/src/main/java/org/conscrypt/Java9PlatformUtil.java +++ b/openjdk/src/main/java/org/conscrypt/Java9PlatformUtil.java @@ -47,14 +47,26 @@ final class Java9PlatformUtil { static void setSSLParameters( SSLParameters src, SSLParametersImpl dest, AbstractConscryptSocket socket) { Java8PlatformUtil.setSSLParameters(src, dest, socket); - + try { + Method getNamedGroupsMethod = src.getClass().getMethod("getNamedGroups"); + dest.setNamedGroups((String[]) getNamedGroupsMethod.invoke(src)); + } catch (ReflectiveOperationException | SecurityException e) { + // Method is not available. Ignore. + } dest.setApplicationProtocols(getApplicationProtocols(src)); } static void getSSLParameters( SSLParameters dest, SSLParametersImpl src, AbstractConscryptSocket socket) { Java8PlatformUtil.getSSLParameters(dest, src, socket); - + try { + String[] namedGroups = src.getNamedGroups(); + Method setNamedGroupsMethod = + dest.getClass().getMethod("setNamedGroups", String[].class); + setNamedGroupsMethod.invoke(dest, (Object) namedGroups); + } catch (ReflectiveOperationException | SecurityException e) { + // Method is not available. Ignore. + } setApplicationProtocols(dest, src.getApplicationProtocols()); } @@ -62,6 +74,13 @@ static void setSSLParameters( SSLParameters src, SSLParametersImpl dest, ConscryptEngine engine) { Java8PlatformUtil.setSSLParameters(src, dest, engine); + try { + Method getNamedGroupsMethod = src.getClass().getMethod("getNamedGroups"); + dest.setNamedGroups((String[]) getNamedGroupsMethod.invoke(src)); + } catch (ReflectiveOperationException | SecurityException e) { + // Method is not available. Ignore. + } + dest.setApplicationProtocols(getApplicationProtocols(src)); } @@ -69,6 +88,15 @@ static void getSSLParameters( SSLParameters dest, SSLParametersImpl src, ConscryptEngine engine) { Java8PlatformUtil.getSSLParameters(dest, src, engine); + try { + String[] namedGroups = src.getNamedGroups(); + Method setNamedGroupsMethod = + dest.getClass().getMethod("setNamedGroups", String[].class); + setNamedGroupsMethod.invoke(dest, (Object) namedGroups); + } catch (ReflectiveOperationException | SecurityException e) { + // Method is not available. Ignore. + } + setApplicationProtocols(dest, src.getApplicationProtocols()); } diff --git a/platform/src/main/java/org/conscrypt/Platform.java b/platform/src/main/java/org/conscrypt/Platform.java index e98ece7c3..a34be6da9 100644 --- a/platform/src/main/java/org/conscrypt/Platform.java +++ b/platform/src/main/java/org/conscrypt/Platform.java @@ -166,6 +166,10 @@ static void setSSLParameters( SSLParameters params, SSLParametersImpl impl, AbstractConscryptSocket socket) { impl.setEndpointIdentificationAlgorithm(params.getEndpointIdentificationAlgorithm()); impl.setUseCipherSuitesOrder(params.getUseCipherSuitesOrder()); + + Method getNamedGroupsMethod = params.getClass().getMethod("getNamedGroups"); + impl.setNamedGroups((String[]) getNamedGroupsMethod.invoke(params)); + List serverNames = params.getServerNames(); if (serverNames != null) { for (SNIServerName serverName : serverNames) { @@ -182,6 +186,10 @@ static void getSSLParameters( SSLParameters params, SSLParametersImpl impl, AbstractConscryptSocket socket) { params.setEndpointIdentificationAlgorithm(impl.getEndpointIdentificationAlgorithm()); params.setUseCipherSuitesOrder(impl.getUseCipherSuitesOrder()); + + Method setNamedGroupsMethod = params.getClass().getMethod("setNamedGroups", String[].class); + setNamedGroupsMethod.invoke(params, (Object[]) impl.getNamedGroups()); + if (impl.getUseSni() && AddressUtils.isValidSniHostname(socket.getHostname())) { params.setServerNames(Collections.singletonList( new SNIHostName(socket.getHostname()))); @@ -193,6 +201,10 @@ static void setSSLParameters( SSLParameters params, SSLParametersImpl impl, ConscryptEngine engine) { impl.setEndpointIdentificationAlgorithm(params.getEndpointIdentificationAlgorithm()); impl.setUseCipherSuitesOrder(params.getUseCipherSuitesOrder()); + + Method getNamedGroupsMethod = params.getClass().getMethod("getNamedGroups"); + impl.setNamedGroups((String[]) getNamedGroupsMethod.invoke(params)); + List serverNames = params.getServerNames(); if (serverNames != null) { for (SNIServerName serverName : serverNames) { @@ -209,6 +221,10 @@ static void getSSLParameters( SSLParameters params, SSLParametersImpl impl, ConscryptEngine engine) { params.setEndpointIdentificationAlgorithm(impl.getEndpointIdentificationAlgorithm()); params.setUseCipherSuitesOrder(impl.getUseCipherSuitesOrder()); + + Method setNamedGroupsMethod = params.getClass().getMethod("setNamedGroups", String[].class); + setNamedGroupsMethod.invoke(params, (Object[]) impl.getNamedGroups()); + if (impl.getUseSni() && AddressUtils.isValidSniHostname(engine.getHostname())) { params.setServerNames(Collections.singletonList( new SNIHostName(engine.getHostname())));