Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support "raw" format for XEC KeySpecs. #1187

Merged
merged 3 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions common/src/jni/main/cpp/conscrypt/jniutil.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, you will never get this error in the function you've added. This error exists because OpenSSL uses a single type for public and private keys, and so we have to rely on a runtime check. But EVP_parse_private_key will never give you a public key.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I did contrive a scenario for this, because the current XDH usage I have seen is apps storing raw XEC keys, so it's not entirely infeasible that a bug or user error causes them to accidentally wrap public key with a PKCS#8 preamble and later try to decode it.

Tracing the x25519 part of the code, it looks like that would get at least as far as x25519_set_priv_raw and X25519_public_from_private (which would presumably generate rubbish) but then I guess the EVP code would error out?

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:
Expand Down
42 changes: 42 additions & 0 deletions common/src/jni/main/cpp/conscrypt/native_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,47 @@ static jlong NativeCrypto_EVP_parse_private_key(JNIEnv* env, jclass, jbyteArray
return reinterpret_cast<uintptr_t>(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);
prbprbprb marked this conversation as resolved.
Show resolved Hide resolved
if (key_ptr == 0) {
return nullptr;
}
bssl::UniquePtr<EVP_PKEY> pkey(reinterpret_cast<EVP_PKEY*>(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<jbyteArray> byteArray(env, env->NewByteArray(static_cast<jsize>(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<uint8_t *>(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)
*/
Expand Down Expand Up @@ -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"),
Expand Down
35 changes: 25 additions & 10 deletions common/src/main/java/org/conscrypt/ArrayUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

package org.conscrypt;

import java.util.Arrays;

/**
* Compatibility utility for Arrays.
*/
final class ArrayUtils {
public final class ArrayUtils {
private ArrayUtils() {}

/**
Expand All @@ -33,19 +35,32 @@
}
}

static String[] concatValues(String[] a1, String... values) {
@SafeVarargs
static <T> T[] concatValues(T[] a1, T... values) {
return concat (a1, values);

Check failure on line 40 in common/src/main/java/org/conscrypt/ArrayUtils.java

View workflow job for this annotation

GitHub Actions / build (windows-latest)

warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter values

Check failure on line 40 in common/src/main/java/org/conscrypt/ArrayUtils.java

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest)

warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter values

Check failure on line 40 in common/src/main/java/org/conscrypt/ArrayUtils.java

View workflow job for this annotation

GitHub Actions / build (macos-latest)

warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter values
}

static String[] concat(String[] a1, String[] a2) {
String[] result = new String[a1.length + a2.length];
int offset = 0;
for (int i = 0; i < a1.length; i++, offset++) {
result[offset] = a1[i];
static <T> T[] concat(T[] a1, T[] a2) {
T[] result = Arrays.copyOf(a1, a1.length + a2.length);
System.arraycopy(a2, 0, result, a1.length, a2.length);
return result;
}

public static byte[] concat(byte[] a1, byte[] a2) {
byte[] result = Arrays.copyOf(a1, a1.length + a2.length);
System.arraycopy(a2, 0, result, a1.length, a2.length);
return result;
}

static boolean startsWith(byte[] array, byte[] startsWith) {
if (array.length < startsWith.length) {
return false;
}
for (int i = 0; i < a2.length; i++, offset++) {
result[offset] = a2[i];
for (int i = 0; i < startsWith.length; i++) {
if (array[i] != startsWith[i]) {
return false;
}
}
return result;
return true;
}
}
3 changes: 3 additions & 0 deletions common/src/main/java/org/conscrypt/NativeCrypto.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
79 changes: 45 additions & 34 deletions common/src/main/java/org/conscrypt/OpenSSLX25519PrivateKey.java
Original file line number Diff line number Diff line change
@@ -1,51 +1,66 @@
/*
* Copyright 2022 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 org.conscrypt.OpenSSLX509CertificateFactory.ParsingException;

import java.security.InvalidKeyException;
import java.security.PrivateKey;
import java.security.spec.EncodedKeySpec;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.PKCS8EncodedKeySpec;
import java.text.ParseException;
import java.util.Arrays;

public class OpenSSLX25519PrivateKey implements OpenSSLX25519Key, PrivateKey {
private static final long serialVersionUID = -3136201500221850916L;
private static final byte[] PKCS8_PREAMBLE = new byte[]{
0x30, 0x2e, 0x02, 0x01, 0x00, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, 0x6e, 0x04, 0x22, 0x04, 0x20,
};

private static final byte[] PKCS8_PREAMBLE_WITH_NULL = new byte[] {
0x30, 0x30, 0x02, 0x01, 0x00, 0x30, 0x07, 0x06, 0x03, 0x2B, 0x65, 0x6E, 0x05, 0x00, 0x04, 0x22, 0x04, 0x20,
0x30, 0x2e, // Sequence: 46 bytes
0x02, 0x01, 0x00, // Integer: 0 (version)
0x30, 0x05, // Sequence: 5 bytes
0x06, 0x03, 0x2b, 0x65, 0x6e, // OID: 1.3.101.110 (X25519)
0x04, 0x22, 0x04, 0x20, // Octet string: 32 bytes
// Key bytes follow directly
};

private byte[] uCoordinate;

public OpenSSLX25519PrivateKey(PKCS8EncodedKeySpec keySpec) throws InvalidKeySpecException {
public OpenSSLX25519PrivateKey(EncodedKeySpec keySpec)
throws InvalidKeySpecException {
byte[] encoded = keySpec.getEncoded();
if (encoded == null || !"PKCS#8".equals(keySpec.getFormat())) {
throw new InvalidKeySpecException("Key must be encoded in PKCS#8 format");
}

int preambleLength = matchesPreamble(PKCS8_PREAMBLE, encoded) | matchesPreamble(PKCS8_PREAMBLE_WITH_NULL, encoded);
if (preambleLength == 0) {
throw new InvalidKeySpecException("Key size is not correct size");
}

uCoordinate = Arrays.copyOfRange(encoded, PKCS8_PREAMBLE.length, encoded.length);
}

private static int matchesPreamble(byte[] preamble, byte[] encoded) {
prbprbprb marked this conversation as resolved.
Show resolved Hide resolved
if (encoded.length != (preamble.length + X25519_KEY_SIZE_BYTES)) {
return 0;
}
int cmp = 0;
for (int i = 0; i < preamble.length; i++) {
cmp |= encoded[i] ^ preamble[i];
if ("PKCS#8".equals(keySpec.getFormat())) {
try {
uCoordinate = NativeCrypto.EVP_raw_X25519_private_key(encoded);
} catch (InvalidKeyException | ParsingException e) {
throw new InvalidKeySpecException(e);
}
} else if ("raw".equalsIgnoreCase(keySpec.getFormat())) {
uCoordinate = encoded;
} else {
throw new InvalidKeySpecException("Encoding must be in PKCS#8 or raw format");
}
if (cmp != 0) {
return 0;
if (uCoordinate.length != X25519_KEY_SIZE_BYTES) {
throw new InvalidKeySpecException("Invalid key size");
}
return preamble.length;
}

public OpenSSLX25519PrivateKey(byte[] coordinateBytes) {
if (coordinateBytes.length != X25519_KEY_SIZE_BYTES) {
throw new IllegalArgumentException("Invalid key size");
}
uCoordinate = coordinateBytes.clone();
}

Expand All @@ -64,18 +79,14 @@ public byte[] getEncoded() {
if (uCoordinate == null) {
throw new IllegalStateException("key is destroyed");
}

byte[] encoded = Arrays.copyOf(PKCS8_PREAMBLE, PKCS8_PREAMBLE.length + uCoordinate.length);
System.arraycopy(uCoordinate, 0, encoded, PKCS8_PREAMBLE.length, uCoordinate.length);
return encoded;
return ArrayUtils.concat(PKCS8_PREAMBLE, uCoordinate);
}

@Override
public byte[] getU() {
if (uCoordinate == null) {
throw new IllegalStateException("key is destroyed");
}

return uCoordinate.clone();
}

Expand Down
78 changes: 43 additions & 35 deletions common/src/main/java/org/conscrypt/OpenSSLX25519PublicKey.java
Original file line number Diff line number Diff line change
@@ -1,52 +1,64 @@
/*
* Copyright 2022 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 java.security.PublicKey;
import java.security.spec.EncodedKeySpec;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.X509EncodedKeySpec;
import java.util.Arrays;

public class OpenSSLX25519PublicKey implements OpenSSLX25519Key, PublicKey {
private static final long serialVersionUID = 453861992373478445L;

private static final byte[] X509_PREAMBLE = new byte[] {
0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, 0x6e, 0x03, 0x21, 0x00,
};

private static final byte[] X509_PREAMBLE_WITH_NULL = new byte[] {
0x30, 0x2C, 0x30, 0x07, 0x06, 0x03, 0x2B, 0x65, 0x6E, 0x05, 0x00, 0x03, 0x21, 0x00,
0x30, 0x2a, // Sequence: 42 bytes
0x30, 0x05, // Sequence: 5 bytes
0x06, 0x03, 0x2b, 0x65, 0x6e, // OID: 1.3.101.110 (X25519)
0x03, 0x21, 0x00, // Bit string: 256 bits
// Key bytes follow directly
};

private final byte[] uCoordinate;

public OpenSSLX25519PublicKey(X509EncodedKeySpec keySpec) throws InvalidKeySpecException {
public OpenSSLX25519PublicKey(EncodedKeySpec keySpec) throws InvalidKeySpecException {
byte[] encoded = keySpec.getEncoded();
if (encoded == null || !"X.509".equals(keySpec.getFormat())) {
throw new InvalidKeySpecException("Encoding must be in X.509 format");
}

int preambleLength = matchesPreamble(X509_PREAMBLE, encoded) | matchesPreamble(X509_PREAMBLE_WITH_NULL, encoded);
if (preambleLength == 0) {
throw new InvalidKeySpecException("Key size is not correct size");
}

uCoordinate = Arrays.copyOfRange(encoded, preambleLength, encoded.length);
}

private static int matchesPreamble(byte[] preamble, byte[] encoded) {
if (encoded.length != (preamble.length + X25519_KEY_SIZE_BYTES)) {
return 0;
}
int cmp = 0;
for (int i = 0; i < preamble.length; i++) {
cmp |= encoded[i] ^ preamble[i];
}
if (cmp != 0) {
return 0;
if ("X.509".equals(keySpec.getFormat())) {
if (!ArrayUtils.startsWith(encoded, X509_PREAMBLE)) {
throw new InvalidKeySpecException("Invalid format");
}
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");
}
return preamble.length;
}

public OpenSSLX25519PublicKey(byte[] coordinateBytes) {
if (coordinateBytes.length != X25519_KEY_SIZE_BYTES) {
throw new IllegalArgumentException("Invalid key size");
}
uCoordinate = coordinateBytes.clone();
}

Expand All @@ -66,17 +78,14 @@ public byte[] getEncoded() {
throw new IllegalStateException("key is destroyed");
}

byte[] encoded = Arrays.copyOf(X509_PREAMBLE, X509_PREAMBLE.length + X25519_KEY_SIZE_BYTES);
System.arraycopy(uCoordinate, 0, encoded, X509_PREAMBLE.length, uCoordinate.length);
return encoded;
return (ArrayUtils.concat(X509_PREAMBLE, uCoordinate));
}

@Override
public byte[] getU() {
if (uCoordinate == null) {
throw new IllegalStateException("key is destroyed");
}

return uCoordinate.clone();
}

Expand All @@ -97,7 +106,6 @@ public int hashCode() {
if (uCoordinate == null) {
throw new IllegalStateException("key is destroyed");
}

return Arrays.hashCode(uCoordinate);
}
}
Loading
Loading