diff --git a/common/src/main/java/org/conscrypt/ConscryptEngineSocket.java b/common/src/main/java/org/conscrypt/ConscryptEngineSocket.java index fdd434d7c..db1376d68 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 final 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..c457a1636 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 final 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..6c21a076a 100644 --- a/common/src/main/java/org/conscrypt/NativeSsl.java +++ b/common/src/main/java/org/conscrypt/NativeSsl.java @@ -39,6 +39,7 @@ import java.security.cert.CertificateEncodingException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.Arrays; import java.util.HashSet; import java.util.Set; import java.util.concurrent.locks.ReadWriteLock; @@ -278,6 +279,71 @@ byte[] getTlsChannelId() throws SSLException { return NativeCrypto.SSL_get_tls_channel_id(ssl, this); } + private static int toBoringSslGroup(String javaNamedGroup) { + switch (javaNamedGroup) { + case "X25519": + case "x25519": + return NativeConstants.NID_X25519; + case "P-256": + case "secp256r1": + return NativeConstants.NID_X9_62_prime256v1; + case "P-384": + case "secp384r1": + return NativeConstants.NID_secp384r1; + case "P-521": + case "secp521r1": + return NativeConstants.NID_secp521r1; + case "X25519MLKEM768": + return NativeConstants.NID_X25519MLKEM768; + case "X25519Kyber768Draft00": + return NativeConstants.NID_X25519Kyber768Draft00; + case "MLKEM1024": + return NativeConstants.NID_ML_KEM_1024; + default: + return -1; // Unknown group. + } + } + + /** + * Converts a list of java named groups to an array of groups that can be passed to BoringSSL. + * + *

Unknown groups are ignored, as required by the API documentation: + * SSLParameters.getNamedGroups() + */ + static int[] toBoringSslGroups(String[] javaNamedGroups) { + int[] outputGroups = new int[javaNamedGroups.length]; + int i = 0; + for (String javaNamedGroup : javaNamedGroups) { + int group = toBoringSslGroup(javaNamedGroup); + if (group > 0) { + outputGroups[i] = group; + i++; + } + } + if (i == 0) { + throw new IllegalArgumentException( + "No valid known group found in: " + Arrays.toString(javaNamedGroups)); + } + if (i < javaNamedGroups.length) { + return Arrays.copyOf(outputGroups, i); + } + return outputGroups; + } + + static int[] parseNamedGroupsProperty(String namedGroupsProperty) { + if (namedGroupsProperty == null) { + throw new NullPointerException("namedGroupsProperty is null"); + } + String[] namedGroups = namedGroupsProperty.replace(" ", "").split(","); + return toBoringSslGroups(namedGroups); + } + + private static void setDefaultNamedGroups(long ssl, NativeSsl sslHolder) { + // When the groups list is empty, the default named groups are used. + NativeCrypto.SSL_set1_groups(ssl, sslHolder, new int[0]); + } + void initialize(String hostname, OpenSSLKey channelIdPrivateKey) throws IOException { boolean enableSessionCreation = parameters.getEnableSessionCreation(); if (!enableSessionCreation) { @@ -320,6 +386,16 @@ 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); + } else { + int[] groups = parseNamedGroupsProperty(namedGroupsProperty); + NativeCrypto.SSL_set1_groups(ssl, this, groups); + } + 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/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java b/common/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java index 083a10a9e..7b5e6132b 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; @@ -39,6 +40,7 @@ import org.conscrypt.tlswire.handshake.HelloExtension; import org.conscrypt.tlswire.util.TlsProtocolVersion; import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -61,6 +63,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 +96,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). @@ -117,8 +124,23 @@ void setNamedGroups(SSLParameters params, String[] namedGroups) { } } + // value of jdk.tls.namedGroups property before the test. null if the property was not set. + private String tlsNamedGroupsProperty; + + @Before + public void setUp() throws Exception { + tlsNamedGroupsProperty = System.getProperty("jdk.tls.namedGroups"); + } + @After public void teardown() throws InterruptedException { + // Restore the property to its original value, to make sure that the test does not + // have any side effects on other tests when setting the property. + if (tlsNamedGroupsProperty == null) { + System.clearProperty("jdk.tls.namedGroups"); + } else { + System.setProperty("jdk.tls.namedGroups", tlsNamedGroupsProperty); + } executor.shutdownNow(); assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS)); } @@ -911,6 +933,80 @@ public void test_SSLSocket_ClientHello_cipherSuites() throws Exception { getSSLSocketFactoriesToTest()); } + @Test + public void handshake_noNamedGroupsProperty_usesDefaultGroups() throws Exception { + System.clearProperty("jdk.tls.namedGroups"); + 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_namedGroupsProperty_usesFirstKnownEntry() throws Exception { + System.setProperty("jdk.tls.namedGroups", "X25519MLKEM768,X25519"); + + 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(); + assertEquals("X25519MLKEM768", getCurveName(client)); + assertEquals("X25519MLKEM768", getCurveName(server)); + client.close(); + server.close(); + context.close(); + } + + @Test + public void handshake_namedGroupsProperty_failsIfAllValuesAreInvalid() throws Exception { + System.setProperty("jdk.tls.namedGroups", "invalid,invalid2"); + + 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; + }); + assertThrows(ExecutionException.class, s::get); + assertThrows(ExecutionException.class, c::get); + client.close(); + server.close(); + context.close(); + } + @Test public void test_SSLSocket_ClientHello_supportedCurves() throws Exception { ForEachRunner.runNamed(sslSocketFactory -> { diff --git a/openjdk/src/test/java/org/conscrypt/ConscryptOpenJdkSuite.java b/openjdk/src/test/java/org/conscrypt/ConscryptOpenJdkSuite.java index 57f40041e..350f4b33d 100644 --- a/openjdk/src/test/java/org/conscrypt/ConscryptOpenJdkSuite.java +++ b/openjdk/src/test/java/org/conscrypt/ConscryptOpenJdkSuite.java @@ -105,6 +105,7 @@ MlDsaTest.class, NativeCryptoArgTest.class, NativeCryptoTest.class, + NativeSslTest.class, NativeRefTest.class, NativeSslSessionTest.class, OpenSSLKeyTest.class, diff --git a/openjdk/src/test/java/org/conscrypt/NativeSslTest.java b/openjdk/src/test/java/org/conscrypt/NativeSslTest.java new file mode 100644 index 000000000..5f15cfa7e --- /dev/null +++ b/openjdk/src/test/java/org/conscrypt/NativeSslTest.java @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2010 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.conscrypt; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class NativeSslTest { + @Test + public void toBoringSslGroups_convertsKnownValues() { + assertArrayEquals(new int[] {NativeConstants.NID_X25519}, + NativeSsl.toBoringSslGroups(new String[] {"X25519"})); + assertArrayEquals(new int[] {NativeConstants.NID_X25519}, + NativeSsl.toBoringSslGroups(new String[] {"x25519"})); + assertArrayEquals(new int[] {NativeConstants.NID_X9_62_prime256v1}, + NativeSsl.toBoringSslGroups(new String[] {"P-256"})); + assertArrayEquals(new int[] {NativeConstants.NID_X9_62_prime256v1}, + NativeSsl.toBoringSslGroups(new String[] {"secp256r1"})); + assertArrayEquals(new int[] {NativeConstants.NID_secp384r1}, + NativeSsl.toBoringSslGroups(new String[] {"P-384"})); + assertArrayEquals(new int[] {NativeConstants.NID_secp384r1}, + NativeSsl.toBoringSslGroups(new String[] {"secp384r1"})); + assertArrayEquals(new int[] {NativeConstants.NID_secp521r1}, + NativeSsl.toBoringSslGroups(new String[] {"P-521"})); + assertArrayEquals(new int[] {NativeConstants.NID_secp521r1}, + NativeSsl.toBoringSslGroups(new String[] {"secp521r1"})); + assertArrayEquals(new int[] {NativeConstants.NID_X25519MLKEM768}, + NativeSsl.toBoringSslGroups(new String[] {"X25519MLKEM768"})); + assertArrayEquals(new int[] {NativeConstants.NID_X25519Kyber768Draft00}, + NativeSsl.toBoringSslGroups(new String[] {"X25519Kyber768Draft00"})); + assertArrayEquals(new int[] {NativeConstants.NID_ML_KEM_1024}, + NativeSsl.toBoringSslGroups(new String[] {"MLKEM1024"})); + } + + @Test + public void toBoringSslGroups_convertsLists() { + assertArrayEquals( + new int[] {NativeConstants.NID_X25519, NativeConstants.NID_X9_62_prime256v1}, + NativeSsl.toBoringSslGroups(new String[] {"X25519", "P-256"})); + } + + @Test + public void toBoringSslGroups_ignoresUnknownValues() { + assertArrayEquals(new int[] {NativeConstants.NID_X25519}, + NativeSsl.toBoringSslGroups(new String[] {"Unknown", "X25519", "Unknown2"})); + } + + @Test + public void toBoringSslGroups_throwsIfNoKnownGroupsFound() { + assertThrows( + IllegalArgumentException.class, () -> NativeSsl.toBoringSslGroups(new String[] {})); + assertThrows(IllegalArgumentException.class, + () -> NativeSsl.toBoringSslGroups(new String[] {"Unknown"})); + assertThrows(IllegalArgumentException.class, + () -> NativeSsl.toBoringSslGroups(new String[] {"Unknown", "Unknown2"})); + } + + @Test + public void parseNamedGroupsProperty_parsesProperty() { + assertArrayEquals(new int[] {NativeConstants.NID_X25519}, + NativeSsl.parseNamedGroupsProperty("X25519")); + assertArrayEquals( + new int[] {NativeConstants.NID_X25519, NativeConstants.NID_X9_62_prime256v1}, + NativeSsl.parseNamedGroupsProperty("X25519,P-256")); + assertArrayEquals( + new int[] {NativeConstants.NID_X25519, NativeConstants.NID_X9_62_prime256v1}, + NativeSsl.parseNamedGroupsProperty("X25519, P-256")); + assertArrayEquals( + new int[] {NativeConstants.NID_X25519, NativeConstants.NID_X9_62_prime256v1}, + NativeSsl.parseNamedGroupsProperty("X25519,Unknown,P-256")); + } + + @Test + public void parseNamedGroupsProperty_throwsIfNoKnownGroupsFound() { + assertThrows(NullPointerException.class, () -> NativeSsl.parseNamedGroupsProperty(null)); + assertThrows(IllegalArgumentException.class, () -> NativeSsl.parseNamedGroupsProperty("")); + assertThrows(IllegalArgumentException.class, () -> NativeSsl.parseNamedGroupsProperty(",")); + assertThrows( + IllegalArgumentException.class, () -> NativeSsl.parseNamedGroupsProperty(" ,")); + assertThrows(IllegalArgumentException.class, + () -> NativeSsl.parseNamedGroupsProperty("Unknown")); + assertThrows(IllegalArgumentException.class, + () -> NativeSsl.parseNamedGroupsProperty("Unknown,Unknown2")); + } +}