diff --git a/common/src/jni/main/cpp/conscrypt/jniutil.cc b/common/src/jni/main/cpp/conscrypt/jniutil.cc index ba696969d..1826d5aa4 100644 --- a/common/src/jni/main/cpp/conscrypt/jniutil.cc +++ b/common/src/jni/main/cpp/conscrypt/jniutil.cc @@ -352,12 +352,14 @@ int throwForEvpError(JNIEnv* env, int reason, const char* message, case EVP_R_MISSING_PARAMETERS: case EVP_R_INVALID_PEER_KEY: case EVP_R_DECODE_ERROR: + case EVP_R_NOT_A_PRIVATE_KEY: return throwInvalidKeyException(env, message); break; case EVP_R_UNSUPPORTED_ALGORITHM: return throwNoSuchAlgorithmException(env, message); break; case EVP_R_INVALID_BUFFER_SIZE: + case EVP_R_BUFFER_TOO_SMALL: return throwIllegalArgumentException(env, message); break; default: diff --git a/common/src/jni/main/cpp/conscrypt/native_crypto.cc b/common/src/jni/main/cpp/conscrypt/native_crypto.cc index ebbd644a6..d0bbc2e30 100644 --- a/common/src/jni/main/cpp/conscrypt/native_crypto.cc +++ b/common/src/jni/main/cpp/conscrypt/native_crypto.cc @@ -1228,6 +1228,47 @@ static jlong NativeCrypto_EVP_parse_private_key(JNIEnv* env, jclass, jbyteArray return reinterpret_cast(pkey.release()); } + +static jbyteArray NativeCrypto_EVP_raw_X25519_private_key( + JNIEnv* env, jclass cls, jbyteArray keyJavaBytes) { + CHECK_ERROR_QUEUE_ON_RETURN; + JNI_TRACE("NativeCrypto_EVP_raw_X25519_private_key(%p)", keyJavaBytes); + + jlong key_ptr = NativeCrypto_EVP_parse_private_key(env, cls, keyJavaBytes); + if (key_ptr == 0) { + return nullptr; + } + bssl::UniquePtr pkey(reinterpret_cast(key_ptr)); + if (EVP_PKEY_id(pkey.get()) != EVP_PKEY_X25519) { + conscrypt::jniutil::throwInvalidKeyException(env, "Invalid key type"); + return nullptr; + } + + size_t key_length = X25519_PRIVATE_KEY_LEN; + ScopedLocalRef byteArray(env, env->NewByteArray(static_cast(key_length))); + if (byteArray.get() == nullptr) { + conscrypt::jniutil::throwOutOfMemory(env, "Allocating byte[]"); + JNI_TRACE("NativeCrypto_EVP_raw_X25519_private_key: byte array creation failed"); + return nullptr; + } + + ScopedByteArrayRW bytes(env, byteArray.get()); + if (bytes.get() == nullptr) { + conscrypt::jniutil::throwOutOfMemory(env, "Allocating scoped byte array"); + JNI_TRACE("NativeCrypto_EVP_raw_X25519_private_key: scoped byte array failed"); + return nullptr; + } + + if (EVP_PKEY_get_raw_private_key( + pkey.get(), reinterpret_cast(bytes.get()),&key_length) == 0) { + conscrypt::jniutil::throwExceptionFromBoringSSLError(env, "EVP_PKEY_get_raw_private_key"); + return nullptr; + } + jbyteArray result = byteArray.release(); + JNI_TRACE("bytes=%p NativeCrypto_EVP_raw_X25519_private_key => %p", keyJavaBytes, result); + return result; +} + /* * static native byte[] EVP_marshal_public_key(long) */ @@ -10994,6 +11035,7 @@ static JNINativeMethod sNativeCryptoMethods[] = { CONSCRYPT_NATIVE_METHOD(EVP_PKEY_cmp, "(" REF_EVP_PKEY REF_EVP_PKEY ")I"), CONSCRYPT_NATIVE_METHOD(EVP_marshal_private_key, "(" REF_EVP_PKEY ")[B"), CONSCRYPT_NATIVE_METHOD(EVP_parse_private_key, "([B)J"), + CONSCRYPT_NATIVE_METHOD(EVP_raw_X25519_private_key, "([B)[B"), CONSCRYPT_NATIVE_METHOD(EVP_marshal_public_key, "(" REF_EVP_PKEY ")[B"), CONSCRYPT_NATIVE_METHOD(EVP_parse_public_key, "([B)J"), CONSCRYPT_NATIVE_METHOD(PEM_read_bio_PUBKEY, "(J)J"), diff --git a/common/src/main/java/org/conscrypt/NativeCrypto.java b/common/src/main/java/org/conscrypt/NativeCrypto.java index e87a23bfc..a83278e5f 100644 --- a/common/src/main/java/org/conscrypt/NativeCrypto.java +++ b/common/src/main/java/org/conscrypt/NativeCrypto.java @@ -103,6 +103,9 @@ static native long EVP_PKEY_new_RSA(byte[] n, byte[] e, byte[] d, byte[] p, byte static native byte[] EVP_marshal_public_key(NativeRef.EVP_PKEY pkey); + static native byte[] EVP_raw_X25519_private_key(byte[] data) + throws ParsingException, InvalidKeyException; + static native long EVP_parse_public_key(byte[] data) throws ParsingException; static native long PEM_read_bio_PUBKEY(long bioCtx); diff --git a/common/src/main/java/org/conscrypt/OpenSSLX25519PrivateKey.java b/common/src/main/java/org/conscrypt/OpenSSLX25519PrivateKey.java index 2ae11eb6d..69f0f3770 100644 --- a/common/src/main/java/org/conscrypt/OpenSSLX25519PrivateKey.java +++ b/common/src/main/java/org/conscrypt/OpenSSLX25519PrivateKey.java @@ -16,9 +16,13 @@ package org.conscrypt; +import org.conscrypt.OpenSSLX509CertificateFactory.ParsingException; + +import java.security.InvalidKeyException; import java.security.PrivateKey; import java.security.spec.EncodedKeySpec; import java.security.spec.InvalidKeySpecException; +import java.text.ParseException; import java.util.Arrays; public class OpenSSLX25519PrivateKey implements OpenSSLX25519Key, PrivateKey { @@ -34,13 +38,15 @@ public class OpenSSLX25519PrivateKey implements OpenSSLX25519Key, PrivateKey { private byte[] uCoordinate; - public OpenSSLX25519PrivateKey(EncodedKeySpec keySpec) throws InvalidKeySpecException { + public OpenSSLX25519PrivateKey(EncodedKeySpec keySpec) + throws InvalidKeySpecException { byte[] encoded = keySpec.getEncoded(); if ("PKCS#8".equals(keySpec.getFormat())) { - if (!ArrayUtils.startsWith(encoded, PKCS8_PREAMBLE)) { - throw new InvalidKeySpecException("Invalid PKCS#8 data"); + try { + uCoordinate = NativeCrypto.EVP_raw_X25519_private_key(encoded); + } catch (InvalidKeyException | ParsingException e) { + throw new InvalidKeySpecException(e); } - uCoordinate = Arrays.copyOfRange(encoded, PKCS8_PREAMBLE.length, encoded.length); } else if ("raw".equalsIgnoreCase(keySpec.getFormat())) { uCoordinate = encoded; } else { diff --git a/common/src/main/java/org/conscrypt/OpenSSLX25519PublicKey.java b/common/src/main/java/org/conscrypt/OpenSSLX25519PublicKey.java index 79702e5c1..11e99f85b 100644 --- a/common/src/main/java/org/conscrypt/OpenSSLX25519PublicKey.java +++ b/common/src/main/java/org/conscrypt/OpenSSLX25519PublicKey.java @@ -40,15 +40,19 @@ public OpenSSLX25519PublicKey(EncodedKeySpec keySpec) throws InvalidKeySpecExcep if (!ArrayUtils.startsWith(encoded, X509_PREAMBLE)) { throw new InvalidKeySpecException("Invalid format"); } - uCoordinate = Arrays.copyOfRange(encoded, X509_PREAMBLE.length, encoded.length); + int totalLength = X509_PREAMBLE.length + X25519_KEY_SIZE_BYTES; + if (encoded.length < totalLength) { + throw new InvalidKeySpecException("Invalid key size"); + } + uCoordinate = Arrays.copyOfRange(encoded, X509_PREAMBLE.length, totalLength); } else if ("raw".equalsIgnoreCase(keySpec.getFormat())) { + if (encoded.length != X25519_KEY_SIZE_BYTES) { + throw new InvalidKeySpecException("Invalid key size"); + } uCoordinate = encoded; } else { throw new InvalidKeySpecException("Encoding must be in X.509 or raw format"); } - if (uCoordinate.length != X25519_KEY_SIZE_BYTES) { - throw new InvalidKeySpecException("Invalid key size"); - } } public OpenSSLX25519PublicKey(byte[] coordinateBytes) { diff --git a/common/src/test/java/org/conscrypt/javax/crypto/XdhKeyTest.java b/common/src/test/java/org/conscrypt/javax/crypto/XdhKeyTest.java index 35e5f5582..beeaaf91b 100644 --- a/common/src/test/java/org/conscrypt/javax/crypto/XdhKeyTest.java +++ b/common/src/test/java/org/conscrypt/javax/crypto/XdhKeyTest.java @@ -21,7 +21,9 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import java.security.KeyFactory; import java.security.KeyPair; +import java.security.KeyPairGenerator; import java.security.PrivateKey; import java.security.PublicKey; import java.security.spec.InvalidKeySpecException; @@ -87,13 +89,20 @@ public void publicKey_X509() throws Exception { assertNotSame(publicKey, copy); assertKeysWork(copy, privateKey); - // Flip a bit in the X.509 preamble + assertThrows(InvalidKeySpecException.class, + () -> new OpenSSLX25519PublicKey(new X509EncodedKeySpec(privateKeyBytes))); assertThrows(InvalidKeySpecException.class, () -> new OpenSSLX25519PublicKey(new X509EncodedKeySpec(flipBit(x509bytes)))); assertThrows(InvalidKeySpecException.class, () -> new OpenSSLX25519PublicKey(new X509EncodedKeySpec(loseOneByte(x509bytes)))); - assertThrows(InvalidKeySpecException.class, - () -> new OpenSSLX25519PublicKey(new X509EncodedKeySpec(gainOneByte(x509bytes)))); + + // Should ignore extra data for better JCA compatibility. + copy = new OpenSSLX25519PublicKey(new X509EncodedKeySpec(gainOneByte(x509bytes))); + assertEquals(publicKey, copy); + assertNotSame(publicKey, copy); + assertKeysWork(copy, privateKey); + + } @Test @@ -124,7 +133,7 @@ public void privateKey_RawSpec() throws Exception { } @Test - public void privateKey_X509() throws Exception { + public void privateKey_PKCS8() throws Exception { assertEquals("PKCS#8", privateKey.getFormat()); byte[] pkcs8Bytes = privateKey.getEncoded(); @@ -134,12 +143,23 @@ public void privateKey_X509() throws Exception { assertNotSame(privateKey, copy); assertKeysWork(publicKey, copy); + assertThrows(InvalidKeySpecException.class, () -> + new OpenSSLX25519PrivateKey(new X509EncodedKeySpec(pkcs8Bytes))); assertThrows(InvalidKeySpecException.class, () -> new OpenSSLX25519PrivateKey(new PKCS8EncodedKeySpec(flipBit(pkcs8Bytes)))); assertThrows(InvalidKeySpecException.class, () -> new OpenSSLX25519PrivateKey(new PKCS8EncodedKeySpec(loseOneByte(pkcs8Bytes)))); - assertThrows(InvalidKeySpecException.class, () -> - new OpenSSLX25519PrivateKey(new PKCS8EncodedKeySpec(gainOneByte(pkcs8Bytes)))); + + // EVP_parse_private_key ignores extra data for JCA compatibility. + copy = new OpenSSLX25519PrivateKey(new PKCS8EncodedKeySpec(gainOneByte(pkcs8Bytes))); + assertEquals(privateKey, copy); + assertNotSame(privateKey, copy); + assertKeysWork(publicKey, copy); + + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA"); + KeyPair keyPair = keyPairGenerator.generateKeyPair(); + assertThrows(InvalidKeySpecException.class, () -> new OpenSSLX25519PrivateKey( + new PKCS8EncodedKeySpec(keyPair.getPrivate().getEncoded()))); } @Test diff --git a/testing/src/main/java/org/conscrypt/TestUtils.java b/testing/src/main/java/org/conscrypt/TestUtils.java index 07fdd7150..503102d48 100644 --- a/testing/src/main/java/org/conscrypt/TestUtils.java +++ b/testing/src/main/java/org/conscrypt/TestUtils.java @@ -348,7 +348,6 @@ public static Class conscryptClass(String simpleName) throws ClassNotFoundExc // Return a Class by name or null public static Class findClass(String name) { - ClassNotFoundException ex = null; try { return Class.forName(name); } catch (ClassNotFoundException ignored) {