Accept RFC 5958 v2 OneAsymmetricKey in WebCrypto OKP pkcs8 import

scripts/build/deps/boringssl.ts

@@ -24,7 +24,7 @@ import { quote } from "../shell.ts";
 import type { Dependency, DirectBuild } from "../source.ts";
 import { depSourceDir } from "../source.ts";
 
-const BORINGSSL_COMMIT = "1a41b9025c2c0a37edd07ff10f6944f03e028522";
+const BORINGSSL_COMMIT = "ac93f352e3e4c58db88cc941aecd19a710391011";
 
 export const boringssl: Dependency = {
   name: "boringssl",

src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.cpp

@@ -30,13 +30,6 @@
 
 namespace WebCore {
 
-size_t bytesUsedToEncodedLength(uint8_t octet)
-{
-    if (octet < MaxLengthInOneByte)
-        return 1;
-    return octet - MaxLengthInOneByte + 1;
-}
-
 size_t extraBytesNeededForEncodedLength(size_t length)
 {
     if (!length)

src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.h

@@ -46,7 +46,6 @@ inline constexpr unsigned char Version[] = { 0x02, 0x01, 0x00 };
 inline constexpr unsigned char InitialOctet = 0x00;
 inline constexpr size_t MaxLengthInOneByte = 128;
 
-size_t bytesUsedToEncodedLength(uint8_t);
 size_t extraBytesNeededForEncodedLength(size_t);
 void addEncodedASN1Length(Vector<uint8_t>&, size_t);
 size_t bytesNeededForEncodedLength(size_t);

src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp

@@ -34,27 +34,22 @@
 #include <openssl/curve25519.h>
 #include "CommonCryptoDERUtilities.h"
 #include "OpenSSLCryptoUniquePtr.h"
+#include <openssl/bytestring.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
 
 namespace WebCore {
 
-// The OID scans below are hand-rolled byte compares and cannot tell a well-formed
-// key of another type from malformed bytes, so ask the real parser before a caller
-// reports "Invalid key type" rather than "Invalid keyData".
-static bool parsesAsSubjectPublicKeyInfo(const Vector<uint8_t>& keyData)
+static int namedCurveToNID(CryptoKeyOKP::NamedCurve namedCurve)
 {
-    const uint8_t* ptr = keyData.begin();
-    return !!EvpPKeyPtr(d2i_PUBKEY(nullptr, &ptr, keyData.size()));
-}
-
-static bool parsesAsPrivateKeyInfo(const Vector<uint8_t>& keyData)
-{
-    const uint8_t* ptr = keyData.begin();
-    auto p8inf = PKCS8PrivKeyInfoPtr(d2i_PKCS8_PRIV_KEY_INFO(nullptr, &ptr, keyData.size()));
-    if (!p8inf)
-        return false;
-    return !!EvpPKeyPtr(EVP_PKCS82PKEY(p8inf.get()));
+    switch (namedCurve) {
+    case CryptoKeyOKP::NamedCurve::X25519:
+        return EVP_PKEY_X25519;
+    case CryptoKeyOKP::NamedCurve::Ed25519:
+        return EVP_PKEY_ED25519;
+    }
+    ASSERT_NOT_REACHED();
+    return EVP_PKEY_NONE;
 }
 
 bool CryptoKeyOKP::isPlatformSupportedCurve(NamedCurve namedCurve)
@@ -95,70 +90,26 @@ std::optional<CryptoKeyPair> CryptoKeyOKP::platformGeneratePair(CryptoAlgorithmI
 // For all of the OIDs, the parameters MUST be absent.
 RefPtr<CryptoKeyOKP> CryptoKeyOKP::importSpki(CryptoAlgorithmIdentifier identifier, NamedCurve namedCurve, Vector<uint8_t>&& keyData, bool extractable, CryptoKeyUsageBitmap usages, bool* keyTypeMismatch)
 {
-    // FIXME: We should use the underlying crypto library to import PKCS8 OKP keys.
-
-    // Read SEQUENCE
-    size_t index = 1;
-    if (keyData.size() < index + 1)
+    CBS cbs;
+    CBS_init(&cbs, keyData.begin(), keyData.size());
+    EvpPKeyPtr pkey(EVP_parse_public_key(&cbs));
+    if (!pkey || CBS_len(&cbs) != 0)
         return nullptr;
-
-    // Read length and SEQUENCE
-    // FIXME: Check length is 5 + 1 + 1 + 1 + keyByteSize.
-    index += bytesUsedToEncodedLength(keyData[index]) + 1;
-    if (keyData.size() < index + 1)
-        return nullptr;
-
-    // Read length
-    // FIXME: Check length is 5.
-    index += bytesUsedToEncodedLength(keyData[index]);
-    if (keyData.size() < index + 5)
-        return nullptr;
-
-    // Read OID
-    // FIXME: spec says this is 1 3 101 11X but WPT tests expect 6 3 43 101 11X.
-    auto reportKeyTypeMismatch = [&] {
-        if (keyTypeMismatch && parsesAsSubjectPublicKeyInfo(keyData))
+    if (EVP_PKEY_id(pkey.get()) != namedCurveToNID(namedCurve)) {
+        if (keyTypeMismatch)
             *keyTypeMismatch = true;
-    };
-    if (keyData[index++] != 6 || keyData[index++] != 3 || keyData[index++] != 43 || keyData[index++] != 101) {
-        reportKeyTypeMismatch();
         return nullptr;
     }
 
-    switch (namedCurve) {
-    case NamedCurve::X25519:
-        if (keyData[index++] != 110) {
-            reportKeyTypeMismatch();
-            return nullptr;
-        }
-        break;
-    case NamedCurve::Ed25519:
-        if (keyData[index++] != 112) {
-            reportKeyTypeMismatch();
-            return nullptr;
-        }
-        break;
-    };
-
-    // Read BIT STRING
-    if (keyData.size() < index + 2)
-        return nullptr;
-    if (keyData[index++] != 3)
-        return nullptr;
-
-    // Read length
-    // FIXME: Check length is keyByteSize + 1.
-    index += bytesUsedToEncodedLength(keyData[index]);
-
-    if (keyData.size() < index + 1)
+    size_t rawLen = 0;
+    if (EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &rawLen) != 1)
         return nullptr;
-
-    // Initial octet
-    if (!!keyData[index])
+    Vector<uint8_t> raw(rawLen);
+    if (EVP_PKEY_get_raw_public_key(pkey.get(), raw.begin(), &rawLen) != 1)
         return nullptr;
-    ++index;
+    raw.shrink(rawLen);
 
-    return create(identifier, namedCurve, CryptoKeyType::Public, std::span<const uint8_t> { keyData.begin() + index, keyData.size() - index }, extractable, usages);
+    return create(identifier, namedCurve, CryptoKeyType::Public, WTF::move(raw), extractable, usages);
 }
 
 constexpr uint8_t OKPOIDFirstByte = 6;
@@ -213,8 +164,9 @@ ExceptionOr<Vector<uint8_t>> CryptoKeyOKP::exportSpki() const
     return WTF::move(result);
 }
 
-// Per https://www.ietf.org/rfc/rfc5280.txt
-// PrivateKeyInfo ::= SEQUENCE { version INTEGER, privateKeyAlgorithm AlgorithmIdentifier, privateKey OCTET STRING }
+// Per https://www.rfc-editor.org/rfc/rfc5958 (OneAsymmetricKey, superseding RFC 5208 PrivateKeyInfo)
+// OneAsymmetricKey ::= SEQUENCE { version INTEGER, privateKeyAlgorithm AlgorithmIdentifier, privateKey OCTET STRING,
+//                                 attributes [0] IMPLICIT Attributes OPTIONAL, publicKey [1] IMPLICIT BIT STRING OPTIONAL }
 // AlgorithmIdentifier  ::= SEQUENCE { algorithm OBJECT IDENTIFIER, parameters ANY DEFINED BY algorithm OPTIONAL }
 // Per https://www.rfc-editor.org/rfc/rfc8410
 // id-X25519    OBJECT IDENTIFIER ::= { 1 3 101 110 }
@@ -224,78 +176,29 @@ ExceptionOr<Vector<uint8_t>> CryptoKeyOKP::exportSpki() const
 // For all of the OIDs, the parameters MUST be absent.
 RefPtr<CryptoKeyOKP> CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identifier, NamedCurve namedCurve, Vector<uint8_t>&& keyData, bool extractable, CryptoKeyUsageBitmap usages, bool* keyTypeMismatch)
 {
-    // FIXME: We should use the underlying crypto library to import PKCS8 OKP keys.
-
-    // Read SEQUENCE
-    size_t index = 1;
-    if (keyData.size() < index + 1)
-        return nullptr;
-
-    // Read length
-    index += bytesUsedToEncodedLength(keyData[index]);
-    if (keyData.size() < index + 1)
-        return nullptr;
-
-    // Read version
-    index += 3;
-    if (keyData.size() < index + 1)
-        return nullptr;
-
-    // Read SEQUENCE
-    index += bytesUsedToEncodedLength(keyData[index]);
-    if (keyData.size() < index + 1)
+    // RFC 5958 v2 OneAsymmetricKey support comes from oven-sh/boringssl#10
+    const uint8_t* ptr = keyData.begin();
+    auto p8 = PKCS8PrivKeyInfoPtr(d2i_PKCS8_PRIV_KEY_INFO(nullptr, &ptr, keyData.size()));
+    if (!p8 || ptr != keyData.end())
         return nullptr;
-
-    // Read length
-    index += bytesUsedToEncodedLength(keyData[index]);
-    if (keyData.size() < index + 5)
+    EvpPKeyPtr pkey(EVP_PKCS82PKEY(p8.get()));
+    if (!pkey)
         return nullptr;
-
-    // Read OID
-    auto reportKeyTypeMismatch = [&] {
-        if (keyTypeMismatch && parsesAsPrivateKeyInfo(keyData))
+    if (EVP_PKEY_id(pkey.get()) != namedCurveToNID(namedCurve)) {
+        if (keyTypeMismatch)
             *keyTypeMismatch = true;
-    };
-    if (keyData[index++] != OKPOIDFirstByte || keyData[index++] != OKPOIDSecondByte || keyData[index++] != OKPOIDThirdByte || keyData[index++] != OKPOIDFourthByte) {
-        reportKeyTypeMismatch();
         return nullptr;
     }
 
-    switch (namedCurve) {
-    case NamedCurve::X25519:
-        if (keyData[index++] != OKPOIDX25519Byte) {
-            reportKeyTypeMismatch();
-            return nullptr;
-        }
-        break;
-    case NamedCurve::Ed25519:
-        if (keyData[index++] != OKPOIDEd25519Byte) {
-            reportKeyTypeMismatch();
-            return nullptr;
-        }
-        break;
-    };
-
-    // Read OCTET STRING
-    if (keyData.size() < index + 2)
-        return nullptr;
-
-    if (keyData[index++] != 4)
-        return nullptr;
-
-    index += bytesUsedToEncodedLength(keyData[index]);
-    if (keyData.size() < index + 2)
+    size_t rawLen = 0;
+    if (EVP_PKEY_get_raw_private_key(pkey.get(), nullptr, &rawLen) != 1)
         return nullptr;
-
-    // Read OCTET STRING
-    if (keyData[index++] != 4)
-        return nullptr;
-
-    index += bytesUsedToEncodedLength(keyData[index]);
-    if (keyData.size() < index + 1)
+    Vector<uint8_t> raw(rawLen);
+    if (EVP_PKEY_get_raw_private_key(pkey.get(), raw.begin(), &rawLen) != 1)
         return nullptr;
+    raw.shrink(rawLen);
 
-    return create(identifier, namedCurve, CryptoKeyType::Private, std::span<const uint8_t> { keyData.begin() + index, keyData.size() - index }, extractable, usages);
+    return create(identifier, namedCurve, CryptoKeyType::Private, WTF::move(raw), extractable, usages);
 }
 
 ExceptionOr<Vector<uint8_t>> CryptoKeyOKP::exportPkcs8() const

test/js/node/crypto/crypto.key-objects.test.ts

@@ -1800,3 +1800,54 @@ test("ECDSA should work", async () => {
 function randomProp() {
   return "prop" + crypto.randomUUID().replace(/-/g, "");
 }
+
+// https://github.com/oven-sh/bun/issues/35432 — BoringSSL rejected v2
+// OneAsymmetricKey (publicKey [1] / version 1) on both its template and CBS
+// parsers; oven-sh/boringssl#10 makes both tolerate it, matching OpenSSL.
+describe("createPrivateKey with RFC 5958 v2 OneAsymmetricKey", () => {
+  // RFC 8032 section 7.1 test vector 1
+  const seed = "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60";
+  const pub = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a";
+  const pubField = "812100" + pub;
+  const spki = Buffer.from("302a300506032b6570032100" + pub, "hex");
+  const der = (version: number, tail: string) => {
+    const body = "0201" + version.toString(16).padStart(2, "0") + "300506032b657004220420" + seed + tail;
+    return Buffer.from("30" + (body.length / 2).toString(16).padStart(2, "0") + body, "hex");
+  };
+
+  const accepts: [string, Buffer][] = [
+    ["v2 with attributes and publicKey", der(1, "a000" + pubField)],
+    ["v2 with publicKey only", der(1, pubField)],
+    ["v2 with attributes only", der(1, "a000")],
+    ["v2 with neither optional field", der(1, "")],
+    ["v1", der(0, "")],
+  ];
+  it.each(accepts)("accepts %s", (_name, key) => {
+    const privateKey = createPrivateKey({ key, format: "der", type: "pkcs8" });
+    expect(privateKey.asymmetricKeyType).toBe("ed25519");
+    const sig = sign(null, Buffer.from("OneAsymmetricKey"), privateKey);
+    expect(
+      verify(null, Buffer.from("OneAsymmetricKey"), createPublicKey({ key: spki, format: "der", type: "spki" }), sig),
+    ).toBe(true);
+  });
+
+  const rejects: [string, Buffer, string][] = [
+    ["v1 with publicKey", der(0, pubField), "ERR_OSSL_DECODE_ERROR"],
+    ["version > v2", der(2, pubField), "ERR_OSSL_DECODE_ERROR"],
+    ["empty publicKey BIT STRING", der(1, "8100"), "ERR_OSSL_ASN1_STRING_TOO_SHORT"],
+    ["publicKey BIT STRING with bad padding octet", der(1, "810108"), "ERR_OSSL_ASN1_INVALID_BIT_STRING_BITS_LEFT"],
+    ["duplicate publicKey", der(1, "a000" + "810100" + "810100"), "ERR_OSSL_ASN1_SEQUENCE_LENGTH_MISMATCH"],
+    ["publicKey before attributes", der(1, "810100" + "a000"), "ERR_OSSL_ASN1_SEQUENCE_LENGTH_MISMATCH"],
+    ["unknown trailing [2]", der(1, "a000" + "810100" + "820100"), "ERR_OSSL_ASN1_SEQUENCE_LENGTH_MISMATCH"],
+    ["malformed attribute body", der(1, "a001ff"), "ERR_OSSL_ASN1_DECODE_ERROR"],
+  ];
+  it.each(rejects)("rejects %s", (_name, key, code) => {
+    let error: any;
+    try {
+      createPrivateKey({ key, format: "der", type: "pkcs8" });
+    } catch (e) {
+      error = e;
+    }
+    expect(error?.code).toBe(code);
+  });
+});