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 all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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);
panva marked this conversation as resolved.
Show resolved Hide resolved

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