Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
crypto: ensure exported webcrypto EC keys use uncompressed point format
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
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
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
@@ -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.