From 01c350c30a86c70ea85503b66c57de111700cd8d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:12:27 +0000 Subject: [PATCH 01/13] Accept RFC 5958 v2 OneAsymmetricKey in WebCrypto OKP pkcs8 import The hand-rolled DER walker in CryptoKeyOKP::importPkcs8 ignored the encoded OCTET STRING lengths and took everything up to the end of the buffer as the private key. A v2 OneAsymmetricKey carrying the optional attributes [0] or publicKey [1] fields therefore produced an oversized seed and the import was rejected with DataError. Decode the DER lengths and bound the CurvePrivateKey by them, ignoring trailing optional fields. Fixes #35432 --- .../webcrypto/CryptoKeyOKPOpenSSL.cpp | 45 +++++++++---- test/js/web/crypto/web-crypto.test.ts | 65 +++++++++++++++++++ 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp index 72a03c958e3f..e95508c92f6d 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp @@ -48,6 +48,24 @@ static bool parsesAsSubjectPublicKeyInfo(const Vector& keyData) return !!EvpPKeyPtr(d2i_PUBKEY(nullptr, &ptr, keyData.size())); } +// Decodes the DER length at keyData[index] and advances index past the length +// bytes. Returns std::nullopt if the encoding is truncated or wider than size_t. +static std::optional readDERLength(const Vector& keyData, size_t& index) +{ + if (index >= keyData.size()) + return std::nullopt; + uint8_t firstByte = keyData[index++]; + if (firstByte < MaxLengthInOneByte) + return firstByte; + size_t numBytes = firstByte & 0x7f; + if (!numBytes || numBytes > sizeof(size_t) || keyData.size() - index < numBytes) + return std::nullopt; + size_t length = 0; + for (size_t i = 0; i < numBytes; ++i) + length = (length << 8) | keyData[index++]; + return length; +} + static bool parsesAsPrivateKeyInfo(const Vector& keyData) { const uint8_t* ptr = keyData.begin(); @@ -213,8 +231,9 @@ ExceptionOr> 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 } @@ -276,26 +295,30 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif break; }; - // Read OCTET STRING - if (keyData.size() < index + 2) + // Read privateKey OCTET STRING, bounded by its encoded length: RFC 5958 v2 + // OneAsymmetricKey may carry optional attributes [0] and publicKey [1] + // fields after it, which must not be consumed as key material. + if (keyData.size() < index + 1) return nullptr; if (keyData[index++] != 4) return nullptr; - index += bytesUsedToEncodedLength(keyData[index]); - if (keyData.size() < index + 2) + auto octetStringLength = readDERLength(keyData, index); + if (!octetStringLength || keyData.size() - index < *octetStringLength) return nullptr; + size_t octetStringEnd = index + *octetStringLength; - // Read OCTET STRING - if (keyData[index++] != 4) + // Read the wrapped CurvePrivateKey OCTET STRING; it must fill the outer + // OCTET STRING exactly. + if (octetStringEnd - index < 1 || keyData[index++] != 4) return nullptr; - index += bytesUsedToEncodedLength(keyData[index]); - if (keyData.size() < index + 1) + auto keyLength = readDERLength(keyData, index); + if (!keyLength || index > octetStringEnd || octetStringEnd - index != *keyLength) return nullptr; - return create(identifier, namedCurve, CryptoKeyType::Private, std::span { keyData.begin() + index, keyData.size() - index }, extractable, usages); + return create(identifier, namedCurve, CryptoKeyType::Private, std::span { keyData.begin() + index, *keyLength }, extractable, usages); } ExceptionOr> CryptoKeyOKP::exportPkcs8() const diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 71dd736229e2..f9e43d959ed5 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1079,6 +1079,71 @@ describe("OKP spki/pkcs8 cross-curve import", () => { }); }); +// The hand-rolled pkcs8 parser consumed everything after the CurvePrivateKey +// tag as key material, so the optional attributes [0] and publicKey [1] fields +// of an RFC 5958 v2 OneAsymmetricKey made the seed 69 bytes and the import was +// rejected. https://github.com/oven-sh/bun/issues/35432 +describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { + const fromHex = (hex: string) => Uint8Array.from(Buffer.from(hex, "hex")); + const der = (...parts: string[]) => { + const body = parts.join(""); + return fromHex("30" + (body.length / 2).toString(16).padStart(2, "0") + body); + }; + + // RFC 8032 section 7.1 test vector 1 + const edSeed = "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60"; + const edPub = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a"; + const edAlgorithm = "300506032b6570"; + // RFC 7748 section 6.1 + const xAlicePriv = "77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a"; + const xAlicePub = "8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a"; + const xBobPub = "de9edb7d7b7dc1b4d35b61c2ece435373f8343c85b78674dadfc7e146f882b4f"; + const xShared = "4a5d9d5ba4ce2de1728e3bf480350f25e07e21c947d19e3376f09b3c1e161742"; + const xAlgorithm = "300506032b656e"; + + const version2 = "020101"; + const emptyAttributes = "a000"; + const wrapSeed = (seed: string) => "04220420" + seed; + const publicKeyField = (pub: string) => "812100" + pub; + + const signsAndVerifies = async (pkcs8: Uint8Array) => { + const privateKey = await crypto.subtle.importKey("pkcs8", pkcs8, "Ed25519", false, ["sign"]); + const publicKey = await crypto.subtle.importKey("raw", fromHex(edPub), "Ed25519", false, ["verify"]); + const message = new TextEncoder().encode("RFC 5958 OneAsymmetricKey"); + const signature = await crypto.subtle.sign("Ed25519", privateKey, message); + return crypto.subtle.verify("Ed25519", publicKey, signature, message); + }; + + it("Ed25519 with attributes [0] and publicKey [1]", async () => { + const pkcs8 = der(version2, edAlgorithm, wrapSeed(edSeed), emptyAttributes, publicKeyField(edPub)); + expect(await signsAndVerifies(pkcs8)).toBe(true); + }); + + it("Ed25519 with only publicKey [1]", async () => { + expect(await signsAndVerifies(der(version2, edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub)))).toBe(true); + }); + + it("Ed25519 with only attributes [0]", async () => { + expect(await signsAndVerifies(der(version2, edAlgorithm, wrapSeed(edSeed), emptyAttributes))).toBe(true); + }); + + it("X25519 with attributes [0] and publicKey [1]", async () => { + const pkcs8 = der(version2, xAlgorithm, wrapSeed(xAlicePriv), emptyAttributes, publicKeyField(xAlicePub)); + const privateKey = await crypto.subtle.importKey("pkcs8", pkcs8, "X25519", false, ["deriveBits"]); + const bobKey = await crypto.subtle.importKey("raw", fromHex(xBobPub), "X25519", false, []); + const shared = await crypto.subtle.deriveBits({ name: "X25519", public: bobKey }, privateKey, 256); + expect(Buffer.from(shared).toString("hex")).toBe(xShared); + }); + + it("rejects a CurvePrivateKey that does not fill the privateKey OCTET STRING", async () => { + // inner OCTET STRING claims 32 bytes but the outer one only carries 16 + const truncated = der(version2, edAlgorithm, "04120420" + edSeed.slice(0, 32)); + await expect(crypto.subtle.importKey("pkcs8", truncated, "Ed25519", false, ["sign"])).rejects.toThrow( + "Invalid keyData", + ); + }); +}); + // importKey's empty-usages guard got Node's message; the same predicate in // generateKey, deriveKey's inner import and unwrapKey's inner import still // carried the empty-message SyntaxError. From c5050422f99cc4f7f9796f796e824977bbeb71d3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:31:26 +0000 Subject: [PATCH 02/13] Validate outer SEQUENCE and trailing optional fields in OKP pkcs8 import Bounding the seed by its OCTET STRING length made importPkcs8 silently ignore every suffix, including bytes outside the declared outer SEQUENCE and truncated optional TLVs that the oversized-seed check used to reject by accident. Require the outer SEQUENCE length to cover the input exactly and structurally consume the optional attributes [0] and publicKey [1] fields, in order and at most once each, rejecting anything else. --- .../webcrypto/CryptoKeyOKPOpenSSL.cpp | 30 +++++++++++++++---- test/js/web/crypto/web-crypto.test.ts | 25 ++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp index e95508c92f6d..6276d61fcc3b 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp @@ -245,13 +245,13 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif { // FIXME: We should use the underlying crypto library to import PKCS8 OKP keys. - // Read SEQUENCE + // Read SEQUENCE, whose encoded length must cover the rest of the input + if (keyData.isEmpty() || keyData[0] != SequenceMark) + return nullptr; size_t index = 1; - if (keyData.size() < index + 1) + auto sequenceLength = readDERLength(keyData, index); + if (!sequenceLength || keyData.size() - index != *sequenceLength) return nullptr; - - // Read length - index += bytesUsedToEncodedLength(keyData[index]); if (keyData.size() < index + 1) return nullptr; @@ -318,7 +318,25 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif if (!keyLength || index > octetStringEnd || octetStringEnd - index != *keyLength) return nullptr; - return create(identifier, namedCurve, CryptoKeyType::Private, std::span { keyData.begin() + index, *keyLength }, extractable, usages); + std::span privateKey { keyData.begin() + index, *keyLength }; + + // Consume the optional trailing fields, attributes [0] then publicKey [1], + // each at most once and in that order. Anything else left in the SEQUENCE + // is malformed. + index = octetStringEnd; + for (uint8_t allowedTag : { uint8_t { 0xa0 }, uint8_t { 0x81 } }) { + if (index >= keyData.size() || keyData[index] != allowedTag) + continue; + ++index; + auto fieldLength = readDERLength(keyData, index); + if (!fieldLength || keyData.size() - index < *fieldLength) + return nullptr; + index += *fieldLength; + } + if (index != keyData.size()) + return nullptr; + + return create(identifier, namedCurve, CryptoKeyType::Private, privateKey, extractable, usages); } ExceptionOr> CryptoKeyOKP::exportPkcs8() const diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index f9e43d959ed5..1674027f4a7b 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1142,6 +1142,31 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { "Invalid keyData", ); }); + + const expectRejected = async (pkcs8: Uint8Array) => { + await expect(crypto.subtle.importKey("pkcs8", pkcs8, "Ed25519", false, ["sign"])).rejects.toThrow( + "Invalid keyData", + ); + }; + const version1 = "020100"; + + it("rejects bytes outside the declared outer SEQUENCE", async () => { + // the trailing ff is past the 0x2e-byte SEQUENCE + await expectRejected(fromHex("302e" + version1 + edAlgorithm + wrapSeed(edSeed) + "ff")); + }); + + it("rejects a truncated optional field", async () => { + // attributes [0] header claims one content byte that is not present + await expectRejected(der(version1, edAlgorithm, wrapSeed(edSeed), "a001")); + }); + + it("rejects an unknown trailing field", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), "8200")); + }); + + it("rejects publicKey [1] before attributes [0]", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub), emptyAttributes)); + }); }); // importKey's empty-usages guard got Node's message; the same predicate in From 2def3a3b3576521e184eceb87aeb63a4c9c89de7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:42:01 +0000 Subject: [PATCH 03/13] Add duplicate-field and long-form length tests for OKP pkcs8 import --- test/js/web/crypto/web-crypto.test.ts | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 1674027f4a7b..42c664ed03b8 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1087,7 +1087,9 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { const fromHex = (hex: string) => Uint8Array.from(Buffer.from(hex, "hex")); const der = (...parts: string[]) => { const body = parts.join(""); - return fromHex("30" + (body.length / 2).toString(16).padStart(2, "0") + body); + const length = body.length / 2; + const lengthHex = length < 0x80 ? length.toString(16).padStart(2, "0") : "81" + length.toString(16); + return fromHex("30" + lengthHex + body); }; // RFC 8032 section 7.1 test vector 1 @@ -1167,6 +1169,22 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { it("rejects publicKey [1] before attributes [0]", async () => { await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub), emptyAttributes)); }); + + it("rejects duplicate attributes [0]", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), emptyAttributes, emptyAttributes)); + }); + + it("rejects duplicate publicKey [1]", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub), publicKeyField(edPub))); + }); + + it("accepts long-form lengths on the SEQUENCE and attributes [0]", async () => { + // 0x90 bytes of attributes push both lengths into two-byte form + const bigAttributes = "a08190" + Buffer.alloc(0x90).toString("hex"); + const pkcs8 = der(version2, edAlgorithm, wrapSeed(edSeed), bigAttributes, publicKeyField(edPub)); + expect(pkcs8[1]).toBe(0x81); // outer SEQUENCE length is long-form + expect(await signsAndVerifies(pkcs8)).toBe(true); + }); }); // importKey's empty-usages guard got Node's message; the same predicate in From 34a2ee42c2ccb406524ec108563b8759c8f05483 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:45:50 +0000 Subject: [PATCH 04/13] Reject lengths above 0xff in the test DER helper --- test/js/web/crypto/web-crypto.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 42c664ed03b8..916e675df61e 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1088,6 +1088,7 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { const der = (...parts: string[]) => { const body = parts.join(""); const length = body.length / 2; + if (length > 0xff) throw new Error("der helper only encodes lengths up to 0xff"); const lengthHex = length < 0x80 ? length.toString(16).padStart(2, "0") : "81" + length.toString(16); return fromHex("30" + lengthHex + body); }; From a154603781ad7b8960e7b95be8d7f339706de41f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:07:02 +0000 Subject: [PATCH 05/13] Validate OneAsymmetricKey version, algorithm and optional field contents Match Node (OpenSSL) on the RFC 5958 fields the structural pass let through: the version INTEGER must be v1 or v2 and v2 is required when publicKey [1] is present, the AlgorithmIdentifier must carry only the OID (RFC 8410 forbids parameters), each element of attributes [0] must parse as an Attribute (validated with d2i_X509_ATTRIBUTE), and the publicKey BIT STRING must start with a valid unused-bits octet. Verified against Node 26 on a 21-case accept/reject matrix. --- .../webcrypto/CryptoKeyOKPOpenSSL.cpp | 58 ++++++++++++++----- .../webcrypto/OpenSSLCryptoUniquePtr.h | 1 + test/js/web/crypto/web-crypto.test.ts | 44 +++++++++++++- 3 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp index 6276d61fcc3b..026a4a2b972a 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp @@ -75,6 +75,20 @@ static bool parsesAsPrivateKeyInfo(const Vector& keyData) return !!EvpPKeyPtr(EVP_PKCS82PKEY(p8inf.get())); } +// The attributes [0] field of a OneAsymmetricKey is SET OF Attribute; each +// element must be a well-formed Attribute, not just a bounded TLV. +static bool parsesAsAttributes(std::span data) +{ + const uint8_t* ptr = data.data(); + const uint8_t* end = ptr + data.size(); + while (ptr < end) { + auto attribute = X509AttributePtr(d2i_X509_ATTRIBUTE(nullptr, &ptr, end - ptr)); + if (!attribute) + return false; + } + return true; +} + bool CryptoKeyOKP::isPlatformSupportedCurve(NamedCurve namedCurve) { return namedCurve == NamedCurve::Ed25519 || namedCurve == NamedCurve::X25519; @@ -255,19 +269,22 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif if (keyData.size() < index + 1) return nullptr; - // Read version - index += 3; - if (keyData.size() < index + 1) + // Read version INTEGER: v1(0) or v2(1) per RFC 5958 + if (keyData.size() - index < 1 || keyData[index++] != IntegerMark) return nullptr; - - // Read SEQUENCE - index += bytesUsedToEncodedLength(keyData[index]); - if (keyData.size() < index + 1) + auto versionLength = readDERLength(keyData, index); + if (!versionLength || *versionLength != 1 || keyData.size() - index < 1) + return nullptr; + uint8_t version = keyData[index++]; + if (version > 1) return nullptr; - // Read length - index += bytesUsedToEncodedLength(keyData[index]); - if (keyData.size() < index + 5) + // Read AlgorithmIdentifier SEQUENCE; its only content is the 5-byte OID + // (RFC 8410: the parameters MUST be absent) + if (keyData.size() - index < 1 || keyData[index++] != SequenceMark) + return nullptr; + auto algorithmLength = readDERLength(keyData, index); + if (!algorithmLength || *algorithmLength != 5 || keyData.size() - index < 5) return nullptr; // Read OID @@ -320,17 +337,30 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif std::span privateKey { keyData.begin() + index, *keyLength }; - // Consume the optional trailing fields, attributes [0] then publicKey [1], + // Read the optional trailing fields, attributes [0] then publicKey [1], // each at most once and in that order. Anything else left in the SEQUENCE // is malformed. index = octetStringEnd; - for (uint8_t allowedTag : { uint8_t { 0xa0 }, uint8_t { 0x81 } }) { - if (index >= keyData.size() || keyData[index] != allowedTag) - continue; + if (index < keyData.size() && keyData[index] == 0xa0) { ++index; auto fieldLength = readDERLength(keyData, index); if (!fieldLength || keyData.size() - index < *fieldLength) return nullptr; + if (!parsesAsAttributes(std::span { keyData.begin() + index, *fieldLength })) + return nullptr; + index += *fieldLength; + } + if (index < keyData.size() && keyData[index] == 0x81) { + // publicKey [1] IMPLICIT BIT STRING; RFC 5958 requires v2 when present + if (version != 1) + return nullptr; + ++index; + auto fieldLength = readDERLength(keyData, index); + if (!fieldLength || keyData.size() - index < *fieldLength) + return nullptr; + // BIT STRING content starts with the unused-bits octet + if (*fieldLength < 1 || keyData[index] > 7) + return nullptr; index += *fieldLength; } if (index != keyData.size()) diff --git a/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h b/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h index 0ce30dee6b26..cdb2ab590b8e 100644 --- a/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h +++ b/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h @@ -79,6 +79,7 @@ DEFINE_OPENSSL_CRYPTO_PTR(BIGNUMPtr, BIGNUM, BN_clear_free) DEFINE_OPENSSL_CRYPTO_PTR(BNCtxPtr, BN_CTX, BN_CTX_free) DEFINE_OPENSSL_CRYPTO_PTR(ECDSASigPtr, ECDSA_SIG, ECDSA_SIG_free) DEFINE_OPENSSL_CRYPTO_PTR(X509Ptr, X509, X509_free) +DEFINE_OPENSSL_CRYPTO_PTR(X509AttributePtr, X509_ATTRIBUTE, X509_ATTRIBUTE_free) DEFINE_OPENSSL_CRYPTO_PTR(BIOPtr, BIO, BIO_free) DEFINE_OPENSSL_CRYPTO_PTR_FULL(ASN1SequencePtr, ASN1_SEQUENCE_ANY, sk_ASN1_TYPE_pop_free(ptr, ASN1_TYPE_free)) diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 916e675df61e..43962fc9261b 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1126,10 +1126,23 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { expect(await signsAndVerifies(der(version2, edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub)))).toBe(true); }); - it("Ed25519 with only attributes [0]", async () => { + it("Ed25519 v1 with only attributes [0]", async () => { + expect(await signsAndVerifies(der("020100", edAlgorithm, wrapSeed(edSeed), emptyAttributes))).toBe(true); + }); + + it("Ed25519 v2 with only attributes [0]", async () => { + // non-conformant (v2 without publicKey) but tolerated, matching Node expect(await signsAndVerifies(der(version2, edAlgorithm, wrapSeed(edSeed), emptyAttributes))).toBe(true); }); + it("Ed25519 with a localKeyId attribute", async () => { + // Attribute ::= SEQUENCE { localKeyId OID, SET { OCTET STRING } } + const attributes = "a014" + "3012" + "06092a864886f70d010915" + "3105" + "0403aabbcc"; + expect(await signsAndVerifies(der(version2, edAlgorithm, wrapSeed(edSeed), attributes, publicKeyField(edPub)))).toBe( + true, + ); + }); + it("X25519 with attributes [0] and publicKey [1]", async () => { const pkcs8 = der(version2, xAlgorithm, wrapSeed(xAlicePriv), emptyAttributes, publicKeyField(xAlicePub)); const privateKey = await crypto.subtle.importKey("pkcs8", pkcs8, "X25519", false, ["deriveBits"]); @@ -1179,9 +1192,34 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub), publicKeyField(edPub))); }); + it("rejects attributes [0] that are not a SET OF Attribute", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), "a001ff", publicKeyField(edPub))); + }); + + it("rejects an Attribute missing its type and values", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), "a0023000", publicKeyField(edPub))); + }); + + it("rejects a publicKey [1] BIT STRING without the unused-bits octet", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), "8100")); + }); + + it("rejects a v1 key carrying publicKey [1]", async () => { + await expectRejected(der(version1, edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub))); + }); + + it("rejects versions above v2", async () => { + await expectRejected(der("020102", edAlgorithm, wrapSeed(edSeed), publicKeyField(edPub))); + }); + + it("rejects AlgorithmIdentifier parameters", async () => { + // RFC 8410: parameters MUST be absent; this one carries a NULL + await expectRejected(der(version1, "300706032b65700500", wrapSeed(edSeed))); + }); + it("accepts long-form lengths on the SEQUENCE and attributes [0]", async () => { - // 0x90 bytes of attributes push both lengths into two-byte form - const bigAttributes = "a08190" + Buffer.alloc(0x90).toString("hex"); + // one localKeyId attribute with a 120-byte value pushes both lengths into two-byte form + const bigAttributes = "a0818a" + "308187" + "06092a864886f70d010915" + "317a" + "0478" + Buffer.alloc(120).toString("hex"); const pkcs8 = der(version2, edAlgorithm, wrapSeed(edSeed), bigAttributes, publicKeyField(edPub)); expect(pkcs8[1]).toBe(0x81); // outer SEQUENCE length is long-form expect(await signsAndVerifies(pkcs8)).toBe(true); From 0af587b920802e8126dfa009ad164a5e266dbb9c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:09:09 +0000 Subject: [PATCH 06/13] [autofix.ci] apply automated fixes --- test/js/web/crypto/web-crypto.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 43962fc9261b..b138cf8109ad 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1138,9 +1138,9 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { it("Ed25519 with a localKeyId attribute", async () => { // Attribute ::= SEQUENCE { localKeyId OID, SET { OCTET STRING } } const attributes = "a014" + "3012" + "06092a864886f70d010915" + "3105" + "0403aabbcc"; - expect(await signsAndVerifies(der(version2, edAlgorithm, wrapSeed(edSeed), attributes, publicKeyField(edPub)))).toBe( - true, - ); + expect( + await signsAndVerifies(der(version2, edAlgorithm, wrapSeed(edSeed), attributes, publicKeyField(edPub))), + ).toBe(true); }); it("X25519 with attributes [0] and publicKey [1]", async () => { @@ -1219,7 +1219,8 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { it("accepts long-form lengths on the SEQUENCE and attributes [0]", async () => { // one localKeyId attribute with a 120-byte value pushes both lengths into two-byte form - const bigAttributes = "a0818a" + "308187" + "06092a864886f70d010915" + "317a" + "0478" + Buffer.alloc(120).toString("hex"); + const bigAttributes = + "a0818a" + "308187" + "06092a864886f70d010915" + "317a" + "0478" + Buffer.alloc(120).toString("hex"); const pkcs8 = der(version2, edAlgorithm, wrapSeed(edSeed), bigAttributes, publicKeyField(edPub)); expect(pkcs8[1]).toBe(0x81); // outer SEQUENCE length is long-form expect(await signsAndVerifies(pkcs8)).toBe(true); From c521d6e75740f1a9f892ebb1b231c36fd3c1762d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:21:05 +0000 Subject: [PATCH 07/13] Report the key type mismatch when the AlgorithmIdentifier length differs RSA and EC AlgorithmIdentifiers are longer than the OKP one, so the new length check rejected them before the OID compare and a well-formed key of another type lost the 'Invalid key type' error in favor of the generic 'Invalid keyData'. --- .../webcrypto/CryptoKeyOKPOpenSSL.cpp | 15 ++++++++---- test/js/web/crypto/web-crypto.test.ts | 24 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp index 026a4a2b972a..d524e66c4f98 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp @@ -279,19 +279,24 @@ RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identif if (version > 1) return nullptr; + auto reportKeyTypeMismatch = [&] { + if (keyTypeMismatch && parsesAsPrivateKeyInfo(keyData)) + *keyTypeMismatch = true; + }; + // Read AlgorithmIdentifier SEQUENCE; its only content is the 5-byte OID // (RFC 8410: the parameters MUST be absent) if (keyData.size() - index < 1 || keyData[index++] != SequenceMark) return nullptr; auto algorithmLength = readDERLength(keyData, index); - if (!algorithmLength || *algorithmLength != 5 || keyData.size() - index < 5) + if (!algorithmLength || *algorithmLength != 5 || keyData.size() - index < 5) { + // a well-formed key of another type lands here (RSA and EC + // AlgorithmIdentifiers are longer), so keep the richer error + reportKeyTypeMismatch(); return nullptr; + } // Read OID - auto reportKeyTypeMismatch = [&] { - if (keyTypeMismatch && parsesAsPrivateKeyInfo(keyData)) - *keyTypeMismatch = true; - }; if (keyData[index++] != OKPOIDFirstByte || keyData[index++] != OKPOIDSecondByte || keyData[index++] != OKPOIDThirdByte || keyData[index++] != OKPOIDFourthByte) { reportKeyTypeMismatch(); return nullptr; diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index b138cf8109ad..ec0ee91bc242 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1077,6 +1077,30 @@ describe("OKP spki/pkcs8 cross-curve import", () => { ecdsaPkcs8: "DataError: Invalid key type", }); }); + + // RSA and EC AlgorithmIdentifiers are longer than the OKP one, so the pkcs8 + // importer's length check trips before the OID compare; it must still report + // the type mismatch for these well-formed keys. + it("RSA/EC pkcs8 imported as Ed25519 reports 'Invalid key type'", async () => { + const rsa = await crypto.subtle.generateKey( + { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, + true, + ["sign", "verify"], + ); + const ec = await crypto.subtle.generateKey({ name: "ECDSA", namedCurve: "P-256" }, true, ["sign", "verify"]); + expect({ + rsa: await rejection( + crypto.subtle.importKey("pkcs8", await crypto.subtle.exportKey("pkcs8", rsa.privateKey), "Ed25519", true, [ + "sign", + ]), + ), + ec: await rejection( + crypto.subtle.importKey("pkcs8", await crypto.subtle.exportKey("pkcs8", ec.privateKey), "Ed25519", true, [ + "sign", + ]), + ), + }).toEqual({ rsa: "DataError: Invalid key type", ec: "DataError: Invalid key type" }); + }); }); // The hand-rolled pkcs8 parser consumed everything after the CurvePrivateKey From d7dc8cc1adfd01e10396473afdb0f19c35eadd9f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:03:44 +0000 Subject: [PATCH 08/13] ci: retrigger From 5db75aebecc3a6de0c1f1601bad331d20e2902c8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:52:27 +0000 Subject: [PATCH 09/13] Use BoringSSL's PKCS8 parser for WebCrypto OKP imports With oven-sh/boringssl#10 the PKCS8_PRIV_KEY_INFO template and EVP_parse_private_key both accept RFC 5958 v2 OneAsymmetricKey, so the hand-rolled DER walkers in CryptoKeyOKP::importPkcs8/importSpki can go through d2i_PKCS8_PRIV_KEY_INFO + EVP_PKCS82PKEY and EVP_parse_public_key instead, matching the EC/RSA/AKP importers. This keeps Node's attribute-body validation and the existing key-type-mismatch reporting. The BoringSSL commit is bumped to the PR head. A createPrivateKey accept/reject matrix in crypto.key-objects.test.ts covers the node:crypto path (d2i + EVP_PKCS82PKEY), verified against Node 26.3. --- scripts/build/deps/boringssl.ts | 2 +- .../webcrypto/CryptoKeyOKPOpenSSL.cpp | 245 +++--------------- .../webcrypto/OpenSSLCryptoUniquePtr.h | 1 - .../js/node/crypto/crypto.key-objects.test.ts | 43 +++ test/js/web/crypto/web-crypto.test.ts | 10 +- 5 files changed, 84 insertions(+), 217 deletions(-) diff --git a/scripts/build/deps/boringssl.ts b/scripts/build/deps/boringssl.ts index 395739e710bc..970bb3e2032a 100644 --- a/scripts/build/deps/boringssl.ts +++ b/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", diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp index d524e66c4f98..ca968c2fbd9c 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp @@ -34,59 +34,22 @@ #include #include "CommonCryptoDERUtilities.h" #include "OpenSSLCryptoUniquePtr.h" +#include #include #include 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& keyData) +static int namedCurveToNID(CryptoKeyOKP::NamedCurve namedCurve) { - const uint8_t* ptr = keyData.begin(); - return !!EvpPKeyPtr(d2i_PUBKEY(nullptr, &ptr, keyData.size())); -} - -// Decodes the DER length at keyData[index] and advances index past the length -// bytes. Returns std::nullopt if the encoding is truncated or wider than size_t. -static std::optional readDERLength(const Vector& keyData, size_t& index) -{ - if (index >= keyData.size()) - return std::nullopt; - uint8_t firstByte = keyData[index++]; - if (firstByte < MaxLengthInOneByte) - return firstByte; - size_t numBytes = firstByte & 0x7f; - if (!numBytes || numBytes > sizeof(size_t) || keyData.size() - index < numBytes) - return std::nullopt; - size_t length = 0; - for (size_t i = 0; i < numBytes; ++i) - length = (length << 8) | keyData[index++]; - return length; -} - -static bool parsesAsPrivateKeyInfo(const Vector& 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())); -} - -// The attributes [0] field of a OneAsymmetricKey is SET OF Attribute; each -// element must be a well-formed Attribute, not just a bounded TLV. -static bool parsesAsAttributes(std::span data) -{ - const uint8_t* ptr = data.data(); - const uint8_t* end = ptr + data.size(); - while (ptr < end) { - auto attribute = X509AttributePtr(d2i_X509_ATTRIBUTE(nullptr, &ptr, end - ptr)); - if (!attribute) - return false; + switch (namedCurve) { + case CryptoKeyOKP::NamedCurve::X25519: + return EVP_PKEY_X25519; + case CryptoKeyOKP::NamedCurve::Ed25519: + return EVP_PKEY_ED25519; } - return true; + ASSERT_NOT_REACHED(); + return EVP_PKEY_NONE; } bool CryptoKeyOKP::isPlatformSupportedCurve(NamedCurve namedCurve) @@ -127,70 +90,26 @@ std::optional CryptoKeyOKP::platformGeneratePair(CryptoAlgorithmI // For all of the OIDs, the parameters MUST be absent. RefPtr CryptoKeyOKP::importSpki(CryptoAlgorithmIdentifier identifier, NamedCurve namedCurve, Vector&& 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) + size_t rawLen = 0; + if (EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &rawLen) != 1) 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) - return nullptr; - - // Initial octet - if (!!keyData[index]) + Vector 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 { keyData.begin() + index, keyData.size() - index }, extractable, usages); + return create(identifier, namedCurve, CryptoKeyType::Public, WTF::move(raw), extractable, usages); } constexpr uint8_t OKPOIDFirstByte = 6; @@ -257,121 +176,31 @@ ExceptionOr> CryptoKeyOKP::exportSpki() const // For all of the OIDs, the parameters MUST be absent. RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identifier, NamedCurve namedCurve, Vector&& keyData, bool extractable, CryptoKeyUsageBitmap usages, bool* keyTypeMismatch) { - // FIXME: We should use the underlying crypto library to import PKCS8 OKP keys. - - // Read SEQUENCE, whose encoded length must cover the rest of the input - if (keyData.isEmpty() || keyData[0] != SequenceMark) - return nullptr; - size_t index = 1; - auto sequenceLength = readDERLength(keyData, index); - if (!sequenceLength || keyData.size() - index != *sequenceLength) - return nullptr; - if (keyData.size() < index + 1) - return nullptr; - - // Read version INTEGER: v1(0) or v2(1) per RFC 5958 - if (keyData.size() - index < 1 || keyData[index++] != IntegerMark) - return nullptr; - auto versionLength = readDERLength(keyData, index); - if (!versionLength || *versionLength != 1 || keyData.size() - index < 1) + // The PKCS8_PRIV_KEY_INFO template accepts both v1 PrivateKeyInfo and v2 + // OneAsymmetricKey with the optional attributes [0] and publicKey [1] + // fields (oven-sh/boringssl#10), validating attribute bodies as OpenSSL does. + 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; - uint8_t version = keyData[index++]; - if (version > 1) + EvpPKeyPtr pkey(EVP_PKCS82PKEY(p8.get())); + if (!pkey) return nullptr; - - auto reportKeyTypeMismatch = [&] { - if (keyTypeMismatch && parsesAsPrivateKeyInfo(keyData)) + if (EVP_PKEY_id(pkey.get()) != namedCurveToNID(namedCurve)) { + if (keyTypeMismatch) *keyTypeMismatch = true; - }; - - // Read AlgorithmIdentifier SEQUENCE; its only content is the 5-byte OID - // (RFC 8410: the parameters MUST be absent) - if (keyData.size() - index < 1 || keyData[index++] != SequenceMark) - return nullptr; - auto algorithmLength = readDERLength(keyData, index); - if (!algorithmLength || *algorithmLength != 5 || keyData.size() - index < 5) { - // a well-formed key of another type lands here (RSA and EC - // AlgorithmIdentifiers are longer), so keep the richer error - reportKeyTypeMismatch(); return nullptr; } - // Read OID - 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 privateKey OCTET STRING, bounded by its encoded length: RFC 5958 v2 - // OneAsymmetricKey may carry optional attributes [0] and publicKey [1] - // fields after it, which must not be consumed as key material. - if (keyData.size() < index + 1) - return nullptr; - - if (keyData[index++] != 4) - return nullptr; - - auto octetStringLength = readDERLength(keyData, index); - if (!octetStringLength || keyData.size() - index < *octetStringLength) + size_t rawLen = 0; + if (EVP_PKEY_get_raw_private_key(pkey.get(), nullptr, &rawLen) != 1) return nullptr; - size_t octetStringEnd = index + *octetStringLength; - - // Read the wrapped CurvePrivateKey OCTET STRING; it must fill the outer - // OCTET STRING exactly. - if (octetStringEnd - index < 1 || keyData[index++] != 4) - return nullptr; - - auto keyLength = readDERLength(keyData, index); - if (!keyLength || index > octetStringEnd || octetStringEnd - index != *keyLength) - return nullptr; - - std::span privateKey { keyData.begin() + index, *keyLength }; - - // Read the optional trailing fields, attributes [0] then publicKey [1], - // each at most once and in that order. Anything else left in the SEQUENCE - // is malformed. - index = octetStringEnd; - if (index < keyData.size() && keyData[index] == 0xa0) { - ++index; - auto fieldLength = readDERLength(keyData, index); - if (!fieldLength || keyData.size() - index < *fieldLength) - return nullptr; - if (!parsesAsAttributes(std::span { keyData.begin() + index, *fieldLength })) - return nullptr; - index += *fieldLength; - } - if (index < keyData.size() && keyData[index] == 0x81) { - // publicKey [1] IMPLICIT BIT STRING; RFC 5958 requires v2 when present - if (version != 1) - return nullptr; - ++index; - auto fieldLength = readDERLength(keyData, index); - if (!fieldLength || keyData.size() - index < *fieldLength) - return nullptr; - // BIT STRING content starts with the unused-bits octet - if (*fieldLength < 1 || keyData[index] > 7) - return nullptr; - index += *fieldLength; - } - if (index != keyData.size()) + Vector 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, privateKey, extractable, usages); + return create(identifier, namedCurve, CryptoKeyType::Private, WTF::move(raw), extractable, usages); } ExceptionOr> CryptoKeyOKP::exportPkcs8() const diff --git a/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h b/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h index cdb2ab590b8e..0ce30dee6b26 100644 --- a/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h +++ b/src/jsc/bindings/webcrypto/OpenSSLCryptoUniquePtr.h @@ -79,7 +79,6 @@ DEFINE_OPENSSL_CRYPTO_PTR(BIGNUMPtr, BIGNUM, BN_clear_free) DEFINE_OPENSSL_CRYPTO_PTR(BNCtxPtr, BN_CTX, BN_CTX_free) DEFINE_OPENSSL_CRYPTO_PTR(ECDSASigPtr, ECDSA_SIG, ECDSA_SIG_free) DEFINE_OPENSSL_CRYPTO_PTR(X509Ptr, X509, X509_free) -DEFINE_OPENSSL_CRYPTO_PTR(X509AttributePtr, X509_ATTRIBUTE, X509_ATTRIBUTE_free) DEFINE_OPENSSL_CRYPTO_PTR(BIOPtr, BIO, BIO_free) DEFINE_OPENSSL_CRYPTO_PTR_FULL(ASN1SequencePtr, ASN1_SEQUENCE_ANY, sk_ASN1_TYPE_pop_free(ptr, ASN1_TYPE_free)) diff --git a/test/js/node/crypto/crypto.key-objects.test.ts b/test/js/node/crypto/crypto.key-objects.test.ts index 5f433d9cf161..df8f676d5fe9 100644 --- a/test/js/node/crypto/crypto.key-objects.test.ts +++ b/test/js/node/crypto/crypto.key-objects.test.ts @@ -1800,3 +1800,46 @@ 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][] = [ + ["v1 with publicKey", der(0, pubField)], + ["version > v2", der(2, pubField)], + ["empty publicKey BIT STRING", der(1, "8100")], + ["publicKey BIT STRING with bad padding octet", der(1, "810108")], + ["duplicate publicKey", der(1, "a000" + "810100" + "810100")], + ["publicKey before attributes", der(1, "810100" + "a000")], + ["unknown trailing [2]", der(1, "a000" + "810100" + "820100")], + ["malformed attribute body", der(1, "a001ff")], + ]; + it.each(rejects)("rejects %s", (_name, key) => { + expect(() => createPrivateKey({ key, format: "der", type: "pkcs8" })).toThrow(); + }); +}); diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index ec0ee91bc242..763f0ef68322 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1078,9 +1078,6 @@ describe("OKP spki/pkcs8 cross-curve import", () => { }); }); - // RSA and EC AlgorithmIdentifiers are longer than the OKP one, so the pkcs8 - // importer's length check trips before the OID compare; it must still report - // the type mismatch for these well-formed keys. it("RSA/EC pkcs8 imported as Ed25519 reports 'Invalid key type'", async () => { const rsa = await crypto.subtle.generateKey( { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, @@ -1103,10 +1100,9 @@ describe("OKP spki/pkcs8 cross-curve import", () => { }); }); -// The hand-rolled pkcs8 parser consumed everything after the CurvePrivateKey -// tag as key material, so the optional attributes [0] and publicKey [1] fields -// of an RFC 5958 v2 OneAsymmetricKey made the seed 69 bytes and the import was -// rejected. https://github.com/oven-sh/bun/issues/35432 +// https://github.com/oven-sh/bun/issues/35432 — the OKP pkcs8 importer used a +// hand-rolled DER walker that mishandled RFC 5958 v2 OneAsymmetricKey; it now +// goes through BoringSSL's PKCS8_PRIV_KEY_INFO template (oven-sh/boringssl#10). describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { const fromHex = (hex: string) => Uint8Array.from(Buffer.from(hex, "hex")); const der = (...parts: string[]) => { From 8d16dde2e062fc23a07b269d674a6737b7f3f6e7 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:54:34 +0000 Subject: [PATCH 10/13] [autofix.ci] apply automated fixes --- test/js/node/crypto/crypto.key-objects.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/js/node/crypto/crypto.key-objects.test.ts b/test/js/node/crypto/crypto.key-objects.test.ts index df8f676d5fe9..96691ff07d66 100644 --- a/test/js/node/crypto/crypto.key-objects.test.ts +++ b/test/js/node/crypto/crypto.key-objects.test.ts @@ -1826,7 +1826,9 @@ describe("createPrivateKey with RFC 5958 v2 OneAsymmetricKey", () => { 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); + expect( + verify(null, Buffer.from("OneAsymmetricKey"), createPublicKey({ key: spki, format: "der", type: "spki" }), sig), + ).toBe(true); }); const rejects: [string, Buffer][] = [ From 56f9542c2e9a307cfd87d6179f6908fa778ce731 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:16:11 +0000 Subject: [PATCH 11/13] Address review feedback on the BoringSSL import rework Delete bytesUsedToEncodedLength, whose last callers were the removed DER walkers. Cover the spki arm of the RSA/EC type-mismatch test and the new trailing-byte check, assert the exact ERR_OSSL_* code for each rejected OneAsymmetricKey encoding, and trim an oversized comment. --- .../webcrypto/CommonCryptoDERUtilities.cpp | 7 ---- .../webcrypto/CommonCryptoDERUtilities.h | 1 - .../webcrypto/CryptoKeyOKPOpenSSL.cpp | 4 +- .../js/node/crypto/crypto.key-objects.test.ts | 28 ++++++++------ test/js/web/crypto/web-crypto.test.ts | 38 +++++++++++++------ 5 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.cpp b/src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.cpp index 972e4683aac0..9e6f2f4e2036 100644 --- a/src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.cpp +++ b/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) diff --git a/src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.h b/src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.h index cab642580798..ea60e3c6bea6 100644 --- a/src/jsc/bindings/webcrypto/CommonCryptoDERUtilities.h +++ b/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&, size_t); size_t bytesNeededForEncodedLength(size_t); diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp index ca968c2fbd9c..1dbbeb459c5f 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cpp @@ -176,9 +176,7 @@ ExceptionOr> CryptoKeyOKP::exportSpki() const // For all of the OIDs, the parameters MUST be absent. RefPtr CryptoKeyOKP::importPkcs8(CryptoAlgorithmIdentifier identifier, NamedCurve namedCurve, Vector&& keyData, bool extractable, CryptoKeyUsageBitmap usages, bool* keyTypeMismatch) { - // The PKCS8_PRIV_KEY_INFO template accepts both v1 PrivateKeyInfo and v2 - // OneAsymmetricKey with the optional attributes [0] and publicKey [1] - // fields (oven-sh/boringssl#10), validating attribute bodies as OpenSSL does. + // 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()) diff --git a/test/js/node/crypto/crypto.key-objects.test.ts b/test/js/node/crypto/crypto.key-objects.test.ts index 96691ff07d66..bb4eff950f22 100644 --- a/test/js/node/crypto/crypto.key-objects.test.ts +++ b/test/js/node/crypto/crypto.key-objects.test.ts @@ -1831,17 +1831,23 @@ describe("createPrivateKey with RFC 5958 v2 OneAsymmetricKey", () => { ).toBe(true); }); - const rejects: [string, Buffer][] = [ - ["v1 with publicKey", der(0, pubField)], - ["version > v2", der(2, pubField)], - ["empty publicKey BIT STRING", der(1, "8100")], - ["publicKey BIT STRING with bad padding octet", der(1, "810108")], - ["duplicate publicKey", der(1, "a000" + "810100" + "810100")], - ["publicKey before attributes", der(1, "810100" + "a000")], - ["unknown trailing [2]", der(1, "a000" + "810100" + "820100")], - ["malformed attribute body", der(1, "a001ff")], + 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) => { - expect(() => createPrivateKey({ key, format: "der", type: "pkcs8" })).toThrow(); + 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); }); }); diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 763f0ef68322..7e0a49ebf615 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1078,25 +1078,39 @@ describe("OKP spki/pkcs8 cross-curve import", () => { }); }); - it("RSA/EC pkcs8 imported as Ed25519 reports 'Invalid key type'", async () => { + it("RSA/EC keys imported as Ed25519 report 'Invalid key type'", async () => { const rsa = await crypto.subtle.generateKey( { name: "RSASSA-PKCS1-v1_5", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: "SHA-256" }, true, ["sign", "verify"], ); const ec = await crypto.subtle.generateKey({ name: "ECDSA", namedCurve: "P-256" }, true, ["sign", "verify"]); - expect({ - rsa: await rejection( - crypto.subtle.importKey("pkcs8", await crypto.subtle.exportKey("pkcs8", rsa.privateKey), "Ed25519", true, [ - "sign", - ]), - ), - ec: await rejection( - crypto.subtle.importKey("pkcs8", await crypto.subtle.exportKey("pkcs8", ec.privateKey), "Ed25519", true, [ - "sign", + const importAsEd25519 = async (format: "pkcs8" | "spki", key: CryptoKey) => + rejection( + crypto.subtle.importKey(format, await crypto.subtle.exportKey(format, key), "Ed25519", true, [ + format === "pkcs8" ? "sign" : "verify", ]), - ), - }).toEqual({ rsa: "DataError: Invalid key type", ec: "DataError: Invalid key type" }); + ); + expect({ + rsaPkcs8: await importAsEd25519("pkcs8", rsa.privateKey), + rsaSpki: await importAsEd25519("spki", rsa.publicKey), + ecPkcs8: await importAsEd25519("pkcs8", ec.privateKey), + ecSpki: await importAsEd25519("spki", ec.publicKey), + }).toEqual({ + rsaPkcs8: "DataError: Invalid key type", + rsaSpki: "DataError: Invalid key type", + ecPkcs8: "DataError: Invalid key type", + ecSpki: "DataError: Invalid key type", + }); + }); + + it("rejects an Ed25519 spki with trailing bytes", async () => { + const ed = await crypto.subtle.generateKey("Ed25519", true, ["sign", "verify"]); + const spki = new Uint8Array(await crypto.subtle.exportKey("spki", ed.publicKey)); + const withTrailing = new Uint8Array([...spki, 0xff]); + expect(await rejection(crypto.subtle.importKey("spki", withTrailing, "Ed25519", true, ["verify"]))).toBe( + "DataError: Invalid keyData", + ); }); }); From 8d32ec81ed9743f8fce2aea0a6ee14a50487cd72 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:48:09 +0000 Subject: [PATCH 12/13] Update expected boringssl commit in process.versions test --- test/js/node/process/process.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index b191a4ed15ab..d882d5db0519 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -331,7 +331,7 @@ it("process.versions", () => { // These are the ACTUAL commits built into bun (not derived values, so // bumping a dep requires updating this test too). const expectedVersions = { - boringssl: "1a41b9025c2c0a37edd07ff10f6944f03e028522", + boringssl: "ac93f352e3e4c58db88cc941aecd19a710391011", libarchive: "ded82291ab41d5e355831b96b0e1ff49e24d8939", mimalloc: "acd9924a0af3ba7c341910b48815106f2944ffa0", picohttpparser: "066d2b1e9ab820703db0837a7255d92d30f0c9f5", From e88aeec58258cd5c668e37a4028b4cfa571606a2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 10 Aug 2026 11:45:06 +0000 Subject: [PATCH 13/13] Pin strict-DER policy for OKP pkcs8 structure, opaque publicKey contents The BoringSSL route rejects non-minimal BER lengths and nonzero BIT STRING padding bits while leaving the publicKey [1] contents uninterpreted, matching Deno and bun's node:crypto (which has always used the strict parser) rather than Node's BER-tolerant OpenSSL. Add the four distinguishing vectors as tests. --- test/js/web/crypto/web-crypto.test.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 7e0a49ebf615..aef2d7a7fb10 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1259,6 +1259,27 @@ describe("OKP pkcs8 import of RFC 5958 v2 OneAsymmetricKey", () => { expect(pkcs8[1]).toBe(0x81); // outer SEQUENCE length is long-form expect(await signsAndVerifies(pkcs8)).toBe(true); }); + + // Strict DER, like Deno and bun's own node:crypto (and unlike Node's + // BER-tolerant OpenSSL): non-minimal lengths and nonzero padding bits + // reject, while the publicKey [1] contents stay uninterpreted. + it("rejects a non-minimal outer SEQUENCE length", async () => { + await expectRejected(fromHex("30812e" + version1 + edAlgorithm + wrapSeed(edSeed))); + }); + + it("rejects a non-minimal privateKey OCTET STRING length", async () => { + await expectRejected(fromHex("302f" + version1 + edAlgorithm + "0481220420" + edSeed)); + }); + + it("rejects a publicKey [1] with nonzero padding bits", async () => { + await expectRejected(der(version2, edAlgorithm, wrapSeed(edSeed), "81020101")); + }); + + it("accepts a publicKey [1] whose contents are not a 32-byte key", async () => { + // the field is stored, not interpreted, matching Node and Deno + const pkcs8 = der(version2, edAlgorithm, wrapSeed(edSeed), "810b00" + Buffer.alloc(10).toString("hex")); + expect(await signsAndVerifies(pkcs8)).toBe(true); + }); }); // importKey's empty-usages guard got Node's message; the same predicate in