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

crypto: ensure exported webcrypto EC keys use uncompressed point format #46021

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
39 changes: 38 additions & 1 deletion src/crypto/crypto_ec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,44 @@ WebCryptoKeyExportStatus ECKeyExportTraits::DoExport(
case kWebCryptoKeyFormatSPKI:
if (key_data->GetKeyType() != kKeyTypePublic)
return WebCryptoKeyExportStatus::INVALID_KEY_TYPE;
return PKEY_SPKI_Export(key_data.get(), out);
panva marked this conversation as resolved.
Show resolved Hide resolved
// 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.
{
panva marked this conversation as resolved.
Show resolved Hide resolved
ManagedEVPPKey m_pkey = key_data->GetAsymmetricKey();
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