Skip to content

Commit

Permalink
crypto: ensure exported webcrypto EC keys use uncompressed point format
Browse files Browse the repository at this point in the history
The WebCrypto spec apparently mandates that EC keys must be exported in
uncompressed point format. This commit makes it so.

Fixes: #45859
PR-URL: #46021
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
  • Loading branch information
bnoordhuis authored and juanarbol committed Jan 31, 2023
1 parent e3f0194 commit 782b6f6
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 15 deletions.
45 changes: 43 additions & 2 deletions src/crypto/crypto_ec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -703,10 +703,51 @@ WebCryptoKeyExportStatus ECKeyExportTraits::DoExport(
if (key_data->GetKeyType() != kKeyTypePrivate)
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
return PKEY_PKCS8_Export(key_data.get(), out);
case kWebCryptoKeyFormatSPKI:
case kWebCryptoKeyFormatSPKI: {
if (key_data->GetKeyType() != kKeyTypePublic)
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
return PKEY_SPKI_Export(key_data.get(), out);

ManagedEVPPKey m_pkey = key_data->GetAsymmetricKey();
if (EVP_PKEY_id(m_pkey.get()) != EVP_PKEY_EC) {
return PKEY_SPKI_Export(key_data.get(), out);
} else {
// Ensure exported key is in uncompressed point format.
// The temporary EC key is so we can have i2d_PUBKEY_bio() write out
// the header but it is a somewhat silly hoop to jump through because
// the header is for all practical purposes a static 26 byte sequence
// where only the second byte changes.
Mutex::ScopedLock lock(*m_pkey.mutex());
const EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(m_pkey.get());
const EC_GROUP* group = EC_KEY_get0_group(ec_key);
const EC_POINT* point = EC_KEY_get0_public_key(ec_key);
const point_conversion_form_t form = POINT_CONVERSION_UNCOMPRESSED;
const size_t need =
EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr);
if (need == 0) return WebCryptoKeyExportStatus::FAILED;
ByteSource::Builder data(need);
const size_t have = EC_POINT_point2oct(
group, point, form, data.data<unsigned char>(), need, nullptr);
if (have == 0) return WebCryptoKeyExportStatus::FAILED;
ECKeyPointer ec(EC_KEY_new());
CHECK_EQ(1, EC_KEY_set_group(ec.get(), group));
ECPointPointer uncompressed(EC_POINT_new(group));
CHECK_EQ(1,
EC_POINT_oct2point(group,
uncompressed.get(),
data.data<unsigned char>(),
data.size(),
nullptr));
CHECK_EQ(1, EC_KEY_set_public_key(ec.get(), uncompressed.get()));
EVPKeyPointer pkey(EVP_PKEY_new());
CHECK_EQ(1, EVP_PKEY_set1_EC_KEY(pkey.get(), ec.get()));
BIOPointer bio(BIO_new(BIO_s_mem()));
CHECK(bio);
if (!i2d_PUBKEY_bio(bio.get(), pkey.get()))
return WebCryptoKeyExportStatus::FAILED;
*out = ByteSource::FromBIO(bio);
return WebCryptoKeyExportStatus::OK;
}
}
default:
UNREACHABLE();
}
Expand Down
13 changes: 13 additions & 0 deletions test/parallel/test-webcrypto-export-import-ec.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,19 @@ async function testImportRaw({ name, publicUsages }, namedCurve) {
await Promise.all(tests);
})().then(common.mustCall());


// https://github.com/nodejs/node/issues/45859
(async function() {
const compressed = Buffer.from([48, 57, 48, 19, 6, 7, 42, 134, 72, 206, 61, 2, 1, 6, 8, 42, 134, 72, 206, 61, 3, 1, 7, 3, 34, 0, 2, 210, 16, 176, 166, 249, 217, 240, 18, 134, 128, 88, 180, 63, 164, 244, 113, 1, 133, 67, 187, 160, 12, 146, 80, 223, 146, 87, 194, 172, 174, 93, 209]); // eslint-disable-line max-len
const uncompressed = Buffer.from([48, 89, 48, 19, 6, 7, 42, 134, 72, 206, 61, 2, 1, 6, 8, 42, 134, 72, 206, 61, 3, 1, 7, 3, 66, 0, 4, 210, 16, 176, 166, 249, 217, 240, 18, 134, 128, 88, 180, 63, 164, 244, 113, 1, 133, 67, 187, 160, 12, 146, 80, 223, 146, 87, 194, 172, 174, 93, 209, 206, 3, 117, 82, 212, 129, 69, 12, 227, 155, 77, 16, 149, 112, 27, 23, 91, 250, 179, 75, 142, 108, 9, 158, 24, 241, 193, 152, 53, 131, 97, 232]); // eslint-disable-line max-len
for (const name of ['ECDH', 'ECDSA']) {
const options = { name, namedCurve: 'P-256' };
const key = await subtle.importKey('spki', compressed, options, true, []);
const spki = await subtle.exportKey('spki', key);
assert.deepStrictEqual(uncompressed, Buffer.from(spki));
}
})().then(common.mustCall());

{
const rsaPublic = crypto.createPublicKey(
fixtures.readKey('rsa_public_2048.pem'));
Expand Down
13 changes: 0 additions & 13 deletions test/wpt/status/WebCryptoAPI.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,4 @@
{
"import_export/ec_importKey.https.any.js": {
"fail": {
"note": "Compressed point export should result in uncompressed point https://github.com/nodejs/node/issues/45859",
"expected": [
"Good parameters: P-256 bits (spki, buffer(59, compressed), {name: ECDSA, namedCurve: P-256}, true, [])",
"Good parameters: P-384 bits (spki, buffer(72, compressed), {name: ECDSA, namedCurve: P-384}, true, [])",
"Good parameters: P-521 bits (spki, buffer(90, compressed), {name: ECDSA, namedCurve: P-521}, true, [])",
"Good parameters: P-256 bits (spki, buffer(59, compressed), {name: ECDH, namedCurve: P-256}, true, [])",
"Good parameters: P-384 bits (spki, buffer(72, compressed), {name: ECDH, namedCurve: P-384}, true, [])",
"Good parameters: P-521 bits (spki, buffer(90, compressed), {name: ECDH, namedCurve: P-521}, true, [])"
]
}
},
"algorithm-discards-context.https.window.js": {
"skip": "Not relevant in Node.js context"
},
Expand Down

0 comments on commit 782b6f6

Please sign in to comment.