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/ConscryptEngineSocket.java b/common/src/main/java/org/conscrypt/ConscryptEngineSocket.java index fdd434d7c..42f602366 100644 --- a/common/src/main/java/org/conscrypt/ConscryptEngineSocket.java +++ b/common/src/main/java/org/conscrypt/ConscryptEngineSocket.java @@ -432,7 +432,7 @@ public final void setEnabledProtocols(String[] protocols) { } @Override - final String getCurveNameForTesting() { + public String getCurveNameForTesting() { return engine.getCurveNameForTesting(); } diff --git a/common/src/main/java/org/conscrypt/ConscryptFileDescriptorSocket.java b/common/src/main/java/org/conscrypt/ConscryptFileDescriptorSocket.java index 62f30bea1..af0a34323 100644 --- a/common/src/main/java/org/conscrypt/ConscryptFileDescriptorSocket.java +++ b/common/src/main/java/org/conscrypt/ConscryptFileDescriptorSocket.java @@ -761,7 +761,7 @@ public final void setEnabledProtocols(String[] protocols) { } @Override - final String getCurveNameForTesting() { + public String getCurveNameForTesting() { return ssl.getCurveNameForTesting(); } diff --git a/common/src/main/java/org/conscrypt/NativeSsl.java b/common/src/main/java/org/conscrypt/NativeSsl.java index 762314d5a..748cade8c 100644 --- a/common/src/main/java/org/conscrypt/NativeSsl.java +++ b/common/src/main/java/org/conscrypt/NativeSsl.java @@ -278,6 +278,55 @@ byte[] getTlsChannelId() throws SSLException { return NativeCrypto.SSL_get_tls_channel_id(ssl, this); } + // Converts a Java "named group" to the corresponding BoringSSL group NID, or -1. + private static int toBoringSslGroupNid(String javaNamedGroup) { + if (javaNamedGroup.equals("X25519") || javaNamedGroup.equals("x25519")) { + return NativeConstants.NID_X25519; + } + if (javaNamedGroup.equals("P-256") || javaNamedGroup.equals("secp256r1")) { + return NativeConstants.NID_X9_62_prime256v1; + } + if (javaNamedGroup.equals("P-384") || javaNamedGroup.equals("secp384r1")) { + return NativeConstants.NID_secp384r1; + } + if (javaNamedGroup.equals("P-521") || javaNamedGroup.equals("secp521r1")) { + return NativeConstants.NID_secp521r1; + } + if (javaNamedGroup.equals("X25519MLKEM768")) { + return NativeConstants.NID_X25519MLKEM768; + } + if (javaNamedGroup.equals("X25519Kyber768Draft00")) { + return NativeConstants.NID_X25519Kyber768Draft00; + } + if (javaNamedGroup.equals("MLKEM1024")) { + return NativeConstants.NID_ML_KEM_1024; + } + return -1; // Unknown curve. + } + + // Default curves to use if namedGroups is null. + // Same curves as StandardNames.ELLIPTIC_CURVES_DEFAULT + private static final int[] DEFAULT_GROUPS = new int[] {NativeConstants.NID_X25519, + NativeConstants.NID_X9_62_prime256v1, NativeConstants.NID_secp384r1}; + + /** + * Converts a list of java named groups to an array of groups that can be passed to BoringSSL. + * + *

Unknown curves are ignored. + */ + public static int[] toBoringSslGroups(String[] javaNamedGroups) { + int[] outputGroups = new int[javaNamedGroups.length]; + int i = 0; + for (String javaNamedGroup : javaNamedGroups) { + int group = toBoringSslGroupNid(javaNamedGroup); + if (group > 0) { + outputGroups[i] = group; + } + i++; + } + return outputGroups; + } + void initialize(String hostname, OpenSSLKey channelIdPrivateKey) throws IOException { boolean enableSessionCreation = parameters.getEnableSessionCreation(); if (!enableSessionCreation) { @@ -320,6 +369,19 @@ void initialize(String hostname, OpenSSLKey channelIdPrivateKey) throws IOExcept ssl, this, parameters.enabledCipherSuites, parameters.enabledProtocols); } + 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) { + // Use the default curves. + NativeCrypto.SSL_set1_groups(ssl, this, DEFAULT_GROUPS); + } else { + NativeCrypto.SSL_set1_groups(ssl, this, toBoringSslGroups(paramsNamedGroups)); + } + if (parameters.applicationProtocols.length > 0) { NativeCrypto.setApplicationProtocols(ssl, this, isClient(), parameters.applicationProtocols); } diff --git a/common/src/main/java/org/conscrypt/OpenSSLSocketImpl.java b/common/src/main/java/org/conscrypt/OpenSSLSocketImpl.java index 5aabdc358..f0c2571e1 100644 --- a/common/src/main/java/org/conscrypt/OpenSSLSocketImpl.java +++ b/common/src/main/java/org/conscrypt/OpenSSLSocketImpl.java @@ -151,4 +151,6 @@ public final byte[] getAlpnSelectedProtocol() { public final void setAlpnProtocols(byte[] protocols) { setApplicationProtocols(SSLUtils.decodeProtocols(protocols == null ? EmptyArray.BYTE : protocols)); } + + @Override public abstract String getCurveNameForTesting(); } 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 083a10a9e..4ca4fcc7b 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 @@ -27,6 +27,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; +import org.conscrypt.OpenSSLSocketImpl; import org.conscrypt.TestUtils; import org.conscrypt.java.security.StandardNames; import org.conscrypt.java.security.TestKeyStore; @@ -61,6 +62,7 @@ import java.util.HashSet; import java.util.List; import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -93,6 +95,10 @@ public class SSLSocketTest { private final ExecutorService executor = Executors.newCachedThreadPool(t -> new Thread(threadGroup, t)); + String getCurveName(SSLSocket socket) { + return ((OpenSSLSocketImpl) socket).getCurveNameForTesting(); + } + /** * Returns the named groups, or null if the method is not available (older versions of * Java/Android). @@ -812,19 +818,21 @@ public void setSSLParameters_invalidCipherSuite_throwsIllegalArgumentException() } @Test - public void setAndGetSSLParameters_withSetNamedGroups_isIgnored() throws Exception { + public void setAndGetSSLParameters_withSetNamedGroups_works() 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())); + + // values passed to setNamedGroups are not validated, any strings work. 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)); + + assertArrayEquals(new String[] {"foo", "bar"}, getNamedGroupsOrNull(sslParameters)); } } @@ -911,6 +919,176 @@ public void test_SSLSocket_ClientHello_cipherSuites() throws Exception { getSSLSocketFactoriesToTest()); } + @Test + public void handshake_noNamedGroups_usesX25519() 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(() -> { + client.startHandshake(); + return null; + }); + s.get(); + c.get(); + // By default, BoringSSL uses X25519, P-256, and P-384, in this order. + // So X25519 gets priority. + assertEquals("X25519", getCurveName(client)); + assertEquals("X25519", getCurveName(server)); + client.close(); + server.close(); + 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 the client + // requests P-256, it will be chosen. + assertEquals("P-256", getCurveName(client)); + assertEquals("P-256", 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 the server only supports P-384, + // it will be chosen. + assertEquals("P-384", getCurveName(client)); + assertEquals("P-384", 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(); + assertEquals("P-384", getCurveName(client)); + assertEquals("P-384", 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(); + assertEquals("X25519MLKEM768", getCurveName(client)); + assertEquals("X25519MLKEM768", 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; + }); + ExecutionException serverException = assertThrows(ExecutionException.class, s::get); + assertTrue(serverException.getCause() instanceof SSLHandshakeException); + ExecutionException clientException = assertThrows(ExecutionException.class, c::get); + assertTrue(clientException.getCause() instanceof SSLHandshakeException); + client.close(); + server.close(); + context.close(); + } + @Test public void test_SSLSocket_ClientHello_supportedCurves() throws Exception { ForEachRunner.runNamed(sslSocketFactory -> { diff --git a/openjdk/src/main/java/org/conscrypt/Java9PlatformUtil.java b/openjdk/src/main/java/org/conscrypt/Java9PlatformUtil.java index 6e3fae650..fca6c175b 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) { + throw new RuntimeException("SSLParameters.getNamedGroups failed.", e); + } 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) { + throw new RuntimeException("SSLParameters.setNamedGroups failed.", e); + } 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) { + throw new RuntimeException("SSLParameters.getNamedGroups failed.", e); + } + 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) { + throw new RuntimeException("SSLParameters.setNamedGroups failed.", e); + } + 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())));