From fb0cbd9cfdbb8e8e3a22590febc6fd7cabc206db Mon Sep 17 00:00:00 2001 From: Miguel Aranda Date: Wed, 10 Dec 2025 11:31:47 +0000 Subject: [PATCH 1/5] Changes for google3 standardization --- common/src/jni/main/cpp/conscrypt/native_crypto.cc | 4 +--- .../org/conscrypt/java/security/KeyPairGeneratorTest.java | 3 +++ .../org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java | 4 +++- openjdk/src/test/java/org/conscrypt/ConscryptTest.java | 2 ++ 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/common/src/jni/main/cpp/conscrypt/native_crypto.cc b/common/src/jni/main/cpp/conscrypt/native_crypto.cc index 8a8a5f17b..e3225b4d4 100644 --- a/common/src/jni/main/cpp/conscrypt/native_crypto.cc +++ b/common/src/jni/main/cpp/conscrypt/native_crypto.cc @@ -1548,7 +1548,7 @@ static jlong NativeCrypto_EVP_PKEY_from_private_seed(JNIEnv* env, jclass, jint p return reinterpret_cast(pkey.release()); } -static jbyteArray NativeCrypto_EVP_PKEY_get_private_seed(JNIEnv* env, jclass cls, jobject pkeyRef) { +static jbyteArray NativeCrypto_EVP_PKEY_get_private_seed(JNIEnv* env, jclass, jobject pkeyRef) { CHECK_ERROR_QUEUE_ON_RETURN; JNI_TRACE("EVP_PKEY_get_private_seed(%p)", pkeyRef); @@ -11752,8 +11752,6 @@ static void NativeCrypto_SSL_CTX_set_spake_credential( jbyteArray id_verifier_array, jboolean is_client, jint handshake_limit, jlong ssl_ctx_address, CONSCRYPT_UNUSED jobject holder) { CHECK_ERROR_QUEUE_ON_RETURN; - JNI_TRACE("SSL_CTX_set_spake_credential(%p, %p, %p, %p, %d, %d, %ld)", context, pw_array, - id_prover_array, id_verifier_array, is_client, handshake_limit, ssl_ctx_address); SSL_CTX* ssl_ctx = to_SSL_CTX(env, ssl_ctx_address, true); diff --git a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java index 3409e8810..444fd14e9 100644 --- a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java +++ b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java @@ -81,6 +81,7 @@ public void test_getInstance() throws Exception { .skipProvider("SunPKCS11-NSS") .run(new ServiceTester.Test() { @Override + // @SuppressWarnings("InsecureCryptoUsage") public void test(Provider provider, String algorithm) throws Exception { AlgorithmParameterSpec params = null; @@ -295,6 +296,7 @@ private void test_Key(KeyPairGenerator kpg, Key k) throws Exception { test_KeyWithAllKeyFactories(k); } + // @SuppressWarnings("InsecureCryptoUsage") private void test_KeyWithAllKeyFactories(Key k) throws Exception { byte[] encoded = k.getEncoded(); @@ -440,6 +442,7 @@ private static DHParameterSpec getDHParams() { }); @Test + // @SuppressWarnings("InsecureCryptoUsage") public void testDSAGeneratorWithParams() throws Exception { final DSAParameterSpec dsaSpec = new DSAParameterSpec(DSA_P, DSA_Q, DSA_G); diff --git a/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java b/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java index 7f760ea79..4bed6c242 100644 --- a/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java +++ b/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java @@ -118,7 +118,8 @@ public void failedUrlConnect() throws Exception { Future future = executor.submit(server.run(op)); HttpsURLConnection connection = server.tlsConnection("/file"); - int response = connection.getResponseCode(); + // google3-add: broken HTTPS hostname verification + int response = connection.getResponseCode(); assertEquals(404, response); future.get(2000, TimeUnit.MILLISECONDS); @@ -151,6 +152,7 @@ public void urlReadTimeout() throws Exception { Future future = executor.submit(server.run(op)); HttpsURLConnection connection = server.tlsConnection("/file"); + // google3-add: broken HTTPS hostname verification connection.setConnectTimeout(0); connection.setReadTimeout(1000); diff --git a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java index 44533ce94..1823b086a 100644 --- a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java +++ b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java @@ -27,6 +27,7 @@ import javax.net.ssl.SSLContext; import org.conscrypt.java.security.StandardNames; +// import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -38,6 +39,7 @@ public class ConscryptTest { * This confirms that the version machinery is working. */ @Test + // @Ignore("Failing on google3. TODO(b/309186591)") public void testVersionIsSensible() { Conscrypt.Version version = Conscrypt.version(); assertNotNull(version); From e24fb09f48eea1f74c5b4977ff5ec5edf2ee4db4 Mon Sep 17 00:00:00 2001 From: Miguel Aranda Date: Wed, 10 Dec 2025 11:34:53 +0000 Subject: [PATCH 2/5] Whitespace corrections --- .../org/conscrypt/java/security/KeyPairGeneratorTest.java | 2 +- .../org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java index 444fd14e9..541cbf2b0 100644 --- a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java +++ b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java @@ -81,7 +81,7 @@ public void test_getInstance() throws Exception { .skipProvider("SunPKCS11-NSS") .run(new ServiceTester.Test() { @Override - // @SuppressWarnings("InsecureCryptoUsage") + // @SuppressWarnings("InsecureCryptoUsage") public void test(Provider provider, String algorithm) throws Exception { AlgorithmParameterSpec params = null; diff --git a/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java b/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java index 4bed6c242..1c256e426 100644 --- a/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java +++ b/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java @@ -119,7 +119,7 @@ public void failedUrlConnect() throws Exception { HttpsURLConnection connection = server.tlsConnection("/file"); // google3-add: broken HTTPS hostname verification - int response = connection.getResponseCode(); + int response = connection.getResponseCode(); assertEquals(404, response); future.get(2000, TimeUnit.MILLISECONDS); @@ -152,7 +152,7 @@ public void urlReadTimeout() throws Exception { Future future = executor.submit(server.run(op)); HttpsURLConnection connection = server.tlsConnection("/file"); - // google3-add: broken HTTPS hostname verification + // google3-add: broken HTTPS hostname verification connection.setConnectTimeout(0); connection.setReadTimeout(1000); From f1ded7a592dfefe79a48e1f6ff6ad9d38bf05c7e Mon Sep 17 00:00:00 2001 From: Miguel Aranda Date: Thu, 11 Dec 2025 14:42:59 +0000 Subject: [PATCH 3/5] Clang-format --- .../java/security/KeyPairGeneratorTest.java | 104 +++++++++--------- .../java/org/conscrypt/ConscryptTest.java | 12 +- 2 files changed, 59 insertions(+), 57 deletions(-) diff --git a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java index fd1a04acd..5eb42e312 100644 --- a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java +++ b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java @@ -64,58 +64,60 @@ public class KeyPairGeneratorTest { @Test public void test_getInstance() throws Exception { - ServiceTester.test("KeyPairGenerator") - // Do not test AndroidKeyStore Provider. It does not accept vanilla public keys for - // signature verification. It's OKish not to test here because it's tested by - // cts/tests/tests/keystore. - .skipProvider("AndroidKeyStore") - // The SunEC provider tries to pass a sun-only AlgorithmParameterSpec to the default - // AlgorithmParameters:EC when its KeyPairGenerator is initialized. Since Conscrypt - // is the highest-ranked provider when running our tests, its implementation of - // AlgorithmParameters:EC is returned, and it doesn't understand the special - // AlgorithmParameterSpec, so the KeyPairGenerator can't be initialized. - .skipProvider("SunEC") - // The SunPKCS11-NSS provider on OpenJDK 7 attempts to delegate to the SunEC provider, - // which doesn't exist on OpenJDK 7, and thus totally fails. This appears to be a bug - // introduced into later revisions of OpenJDK 7. - .skipProvider("SunPKCS11-NSS") - .run(new ServiceTester.Test() { - @Override - // @SuppressWarnings("InsecureCryptoUsage") - public void test(Provider provider, String algorithm) throws Exception { - AlgorithmParameterSpec params = null; - - if ("DH".equals(algorithm) || "DiffieHellman".equalsIgnoreCase(algorithm)) { - params = getDHParams(); - } - // KeyPairGenerator.getInstance(String) - KeyPairGenerator kpg1 = KeyPairGenerator.getInstance(algorithm); - assertEquals(algorithm, kpg1.getAlgorithm()); - if (params != null) { - kpg1.initialize(params); + ServiceTester + .test("KeyPairGenerator") + // Do not test AndroidKeyStore Provider. It does not accept vanilla public keys for + // signature verification. It's OKish not to test here because it's tested by + // cts/tests/tests/keystore. + .skipProvider("AndroidKeyStore") + // The SunEC provider tries to pass a sun-only AlgorithmParameterSpec to the default + // AlgorithmParameters:EC when its KeyPairGenerator is initialized. Since Conscrypt + // is the highest-ranked provider when running our tests, its implementation of + // AlgorithmParameters:EC is returned, and it doesn't understand the special + // AlgorithmParameterSpec, so the KeyPairGenerator can't be initialized. + .skipProvider("SunEC") + // The SunPKCS11-NSS provider on OpenJDK 7 attempts to delegate to the SunEC + // provider, which doesn't exist on OpenJDK 7, and thus totally fails. This appears + // to be a bug introduced into later revisions of OpenJDK 7. + .skipProvider("SunPKCS11-NSS") + .run(new ServiceTester.Test() { + @Override + + // @SuppressWarnings("InsecureCryptoUsage") + public void test(Provider provider, String algorithm) throws Exception { + AlgorithmParameterSpec params = null; + + if ("DH".equals(algorithm) || "DiffieHellman".equalsIgnoreCase(algorithm)) { + params = getDHParams(); + } + // KeyPairGenerator.getInstance(String) + KeyPairGenerator kpg1 = KeyPairGenerator.getInstance(algorithm); + assertEquals(algorithm, kpg1.getAlgorithm()); + if (params != null) { + kpg1.initialize(params); + } + test_KeyPairGenerator(kpg1); + + // KeyPairGenerator.getInstance(String, Provider) + KeyPairGenerator kpg2 = KeyPairGenerator.getInstance(algorithm, provider); + assertEquals(algorithm, kpg2.getAlgorithm()); + assertEquals(provider, kpg2.getProvider()); + if (params != null) { + kpg2.initialize(params); + } + test_KeyPairGenerator(kpg2); + + // KeyPairGenerator.getInstance(String, String) + KeyPairGenerator kpg3 = + KeyPairGenerator.getInstance(algorithm, provider.getName()); + assertEquals(algorithm, kpg3.getAlgorithm()); + assertEquals(provider, kpg3.getProvider()); + if (params != null) { + kpg3.initialize(params); + } + test_KeyPairGenerator(kpg3); } - test_KeyPairGenerator(kpg1); - - // KeyPairGenerator.getInstance(String, Provider) - KeyPairGenerator kpg2 = KeyPairGenerator.getInstance(algorithm, provider); - assertEquals(algorithm, kpg2.getAlgorithm()); - assertEquals(provider, kpg2.getProvider()); - if (params != null) { - kpg2.initialize(params); - } - test_KeyPairGenerator(kpg2); - - // KeyPairGenerator.getInstance(String, String) - KeyPairGenerator kpg3 = KeyPairGenerator.getInstance(algorithm, - provider.getName()); - assertEquals(algorithm, kpg3.getAlgorithm()); - assertEquals(provider, kpg3.getProvider()); - if (params != null) { - kpg3.initialize(params); - } - test_KeyPairGenerator(kpg3); - } - }); + }); } private static final Map> KEY_SIZES = new HashMap<>(); diff --git a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java index 1823b086a..3f9cec81a 100644 --- a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java +++ b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java @@ -73,8 +73,7 @@ public void buildTls13WithoutTrustManager() throws Exception { @Test public void buildInvalid() { try { - Conscrypt.newProviderBuilder() - .defaultTlsProtocol("invalid").build(); + Conscrypt.newProviderBuilder().defaultTlsProtocol("invalid").build(); fail(); } catch (IllegalArgumentException e) { // Expected. @@ -83,10 +82,11 @@ public void buildInvalid() { private void buildProvider(String defaultProtocol, boolean withTrustManager) throws Exception { Provider provider = Conscrypt.newProviderBuilder() - .setName("test name") - .provideTrustManager(withTrustManager) - .defaultTlsProtocol(defaultProtocol) - .build(); + .setName("test name") + .provideTrustManager(withTrustManager) + .defaultTlsProtocol(defaultProtocol) + + .build(); assertEquals("test name", provider.getName()); assertEquals(withTrustManager, provider.containsKey("TrustManagerFactory.PKIX")); From 2eb42e5b74be42160a0535016048a8833d20960a Mon Sep 17 00:00:00 2001 From: Miguel Aranda Date: Thu, 11 Dec 2025 14:57:29 +0000 Subject: [PATCH 4/5] Change imports --- openjdk/src/test/java/org/conscrypt/ConscryptTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java index 3f9cec81a..e5dc02bad 100644 --- a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java +++ b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java @@ -22,16 +22,17 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import java.security.Provider; -import java.security.Security; -import javax.net.ssl.SSLContext; - import org.conscrypt.java.security.StandardNames; // import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import java.security.Provider; +import java.security.Security; + +import javax.net.ssl.SSLContext; + @RunWith(JUnit4.class) public class ConscryptTest { From 6b0841de2b408fe431fdccdb8629593861e2c559 Mon Sep 17 00:00:00 2001 From: Miguel Aranda Date: Thu, 11 Dec 2025 15:55:54 +0000 Subject: [PATCH 5/5] Minor fixes --- .../org/conscrypt/java/security/KeyPairGeneratorTest.java | 7 +++---- .../conscrypt/javax/net/ssl/HttpsURLConnectionTest.java | 4 ++-- openjdk/src/test/java/org/conscrypt/ConscryptTest.java | 5 ++--- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java index 5eb42e312..67390586e 100644 --- a/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java +++ b/common/src/test/java/org/conscrypt/java/security/KeyPairGeneratorTest.java @@ -82,8 +82,7 @@ public void test_getInstance() throws Exception { .skipProvider("SunPKCS11-NSS") .run(new ServiceTester.Test() { @Override - - // @SuppressWarnings("InsecureCryptoUsage") + // g3-add: @SuppressWarnings("InsecureCryptoUsage") public void test(Provider provider, String algorithm) throws Exception { AlgorithmParameterSpec params = null; @@ -304,7 +303,7 @@ private void test_Key(KeyPairGenerator kpg, Key k) throws Exception { test_KeyWithAllKeyFactories(k); } - // @SuppressWarnings("InsecureCryptoUsage") + // g3-add: @SuppressWarnings("InsecureCryptoUsage") private void test_KeyWithAllKeyFactories(Key k) throws Exception { byte[] encoded = k.getEncoded(); @@ -450,7 +449,7 @@ private static DHParameterSpec getDHParams() { }); @Test - // @SuppressWarnings("InsecureCryptoUsage") + // g3-add: @SuppressWarnings("InsecureCryptoUsage") public void testDSAGeneratorWithParams() throws Exception { final DSAParameterSpec dsaSpec = new DSAParameterSpec(DSA_P, DSA_Q, DSA_G); diff --git a/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java b/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java index 1c256e426..aed3dc9e4 100644 --- a/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java +++ b/common/src/test/java/org/conscrypt/javax/net/ssl/HttpsURLConnectionTest.java @@ -118,7 +118,7 @@ public void failedUrlConnect() throws Exception { Future future = executor.submit(server.run(op)); HttpsURLConnection connection = server.tlsConnection("/file"); - // google3-add: broken HTTPS hostname verification + // g3-add: broken HTTPS hostname verification int response = connection.getResponseCode(); assertEquals(404, response); @@ -152,7 +152,7 @@ public void urlReadTimeout() throws Exception { Future future = executor.submit(server.run(op)); HttpsURLConnection connection = server.tlsConnection("/file"); - // google3-add: broken HTTPS hostname verification + // g3-add: broken HTTPS hostname verification connection.setConnectTimeout(0); connection.setReadTimeout(1000); diff --git a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java index e5dc02bad..9b58a96ea 100644 --- a/openjdk/src/test/java/org/conscrypt/ConscryptTest.java +++ b/openjdk/src/test/java/org/conscrypt/ConscryptTest.java @@ -23,7 +23,7 @@ import static org.junit.Assert.fail; import org.conscrypt.java.security.StandardNames; -// import org.junit.Ignore; +// g3-add: import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -40,7 +40,7 @@ public class ConscryptTest { * This confirms that the version machinery is working. */ @Test - // @Ignore("Failing on google3. TODO(b/309186591)") + // g3-add: @Ignore("Failing on google3. TODO(b/309186591)") public void testVersionIsSensible() { Conscrypt.Version version = Conscrypt.version(); assertNotNull(version); @@ -86,7 +86,6 @@ private void buildProvider(String defaultProtocol, boolean withTrustManager) thr .setName("test name") .provideTrustManager(withTrustManager) .defaultTlsProtocol(defaultProtocol) - .build(); assertEquals("test name", provider.getName());