From 17876b43a46ae5cc21f17021a88376c5569dad1d Mon Sep 17 00:00:00 2001 From: Sergey Chernyshev Date: Thu, 25 Apr 2024 00:14:11 +0200 Subject: [PATCH] crypto: ensure valid point on elliptic curve in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/50234 Reviewed-By: Tobias Nießen Reviewed-By: Antoine du Hamel --- graal-nodejs/lib/internal/crypto/ec.js | 4 + graal-nodejs/src/crypto/crypto_keys.cc | 31 +++++++ graal-nodejs/src/crypto/crypto_keys.h | 3 + .../test-webcrypto-export-import-ec.js | 86 ++++++++++++++++--- 4 files changed, 114 insertions(+), 10 deletions(-) diff --git a/graal-nodejs/lib/internal/crypto/ec.js b/graal-nodejs/lib/internal/crypto/ec.js index aca99902cc7..ebfcecaff3c 100644 --- a/graal-nodejs/lib/internal/crypto/ec.js +++ b/graal-nodejs/lib/internal/crypto/ec.js @@ -264,6 +264,10 @@ async function ecImportKey( break; } + if (!keyObject[kHandle].checkEcKeyData()) { + throw lazyDOMException('Invalid keyData', 'DataError'); + } + const { namedCurve: checkNamedCurve, } = keyObject[kHandle].keyDetail({}); diff --git a/graal-nodejs/src/crypto/crypto_keys.cc b/graal-nodejs/src/crypto/crypto_keys.cc index 0be2283d601..fdf6cd45e97 100644 --- a/graal-nodejs/src/crypto/crypto_keys.cc +++ b/graal-nodejs/src/crypto/crypto_keys.cc @@ -908,6 +908,8 @@ v8::Local KeyObjectHandle::Initialize(Environment* env) { isolate, templ, "getSymmetricKeySize", GetSymmetricKeySize); SetProtoMethodNoSideEffect( isolate, templ, "getAsymmetricKeyType", GetAsymmetricKeyType); + SetProtoMethodNoSideEffect( + isolate, templ, "checkEcKeyData", CheckEcKeyData); SetProtoMethod(isolate, templ, "export", Export); SetProtoMethod(isolate, templ, "exportJwk", ExportJWK); SetProtoMethod(isolate, templ, "initECRaw", InitECRaw); @@ -927,6 +929,7 @@ void KeyObjectHandle::RegisterExternalReferences( registry->Register(Init); registry->Register(GetSymmetricKeySize); registry->Register(GetAsymmetricKeyType); + registry->Register(CheckEcKeyData); registry->Register(Export); registry->Register(ExportJWK); registry->Register(InitECRaw); @@ -1238,6 +1241,34 @@ void KeyObjectHandle::GetAsymmetricKeyType( args.GetReturnValue().Set(key->GetAsymmetricKeyType()); } +bool KeyObjectHandle::CheckEcKeyData() const { + MarkPopErrorOnReturn mark_pop_error_on_return; + + const ManagedEVPPKey& key = data_->GetAsymmetricKey(); + KeyType type = data_->GetKeyType(); + CHECK_NE(type, kKeyTypeSecret); + EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(key.get(), nullptr)); + CHECK(ctx); + CHECK_EQ(EVP_PKEY_id(key.get()), EVP_PKEY_EC); + + if (type == kKeyTypePrivate) { + return EVP_PKEY_check(ctx.get()) == 1; + } + +#if OPENSSL_VERSION_MAJOR >= 3 + return EVP_PKEY_public_check_quick(ctx.get()) == 1; +#else + return EVP_PKEY_public_check(ctx.get()) == 1; +#endif +} + +void KeyObjectHandle::CheckEcKeyData(const FunctionCallbackInfo& args) { + KeyObjectHandle* key; + ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder()); + + args.GetReturnValue().Set(key->CheckEcKeyData()); +} + void KeyObjectHandle::GetSymmetricKeySize( const FunctionCallbackInfo& args) { KeyObjectHandle* key; diff --git a/graal-nodejs/src/crypto/crypto_keys.h b/graal-nodejs/src/crypto/crypto_keys.h index eb4f5222670..820d26cc153 100644 --- a/graal-nodejs/src/crypto/crypto_keys.h +++ b/graal-nodejs/src/crypto/crypto_keys.h @@ -193,6 +193,9 @@ class KeyObjectHandle : public BaseObject { const v8::FunctionCallbackInfo& args); v8::Local GetAsymmetricKeyType() const; + static void CheckEcKeyData(const v8::FunctionCallbackInfo& args); + bool CheckEcKeyData() const; + static void GetSymmetricKeySize( const v8::FunctionCallbackInfo& args); diff --git a/graal-nodejs/test/parallel/test-webcrypto-export-import-ec.js b/graal-nodejs/test/parallel/test-webcrypto-export-import-ec.js index e18514800f5..1e9edb9d6a8 100644 --- a/graal-nodejs/test/parallel/test-webcrypto-export-import-ec.js +++ b/graal-nodejs/test/parallel/test-webcrypto-export-import-ec.js @@ -400,15 +400,81 @@ async function testImportRaw({ name, publicUsages }, namedCurve) { ['ECDSA', ['verify'], ['sign']], ['ECDH', [], ['deriveBits', 'deriveBits']], ]) { - assert.rejects(subtle.importKey( - 'spki', - rsaPublic.export({ format: 'der', type: 'spki' }), - { name, hash: 'SHA-256', namedCurve: 'P-256' }, - true, publicUsages), { message: /Invalid key type/ }); - assert.rejects(subtle.importKey( - 'pkcs8', - rsaPrivate.export({ format: 'der', type: 'pkcs8' }), - { name, hash: 'SHA-256', namedCurve: 'P-256' }, - true, privateUsages), { message: /Invalid key type/ }); + assert.rejects( + subtle.importKey( + 'spki', + rsaPublic.export({ format: 'der', type: 'spki' }), + { name, hash: 'SHA-256', namedCurve: 'P-256' }, + true, publicUsages), { message: /Invalid key type/ }, + ).then(common.mustCall()); + assert.rejects( + subtle.importKey( + 'pkcs8', + rsaPrivate.export({ format: 'der', type: 'pkcs8' }), + { name, hash: 'SHA-256', namedCurve: 'P-256' }, + true, privateUsages), { message: /Invalid key type/ }, + ).then(common.mustCall()); + } +} + +// Bad private keys +{ + for (const { namedCurve, key: pkcs8 } of [ + // The private key is exactly equal to the order, and the public key is + // private key * order. + { + namedCurve: 'P-256', + key: Buffer.from( + '3066020100301306072a8648ce3d020106082a8648ce3d030107044c304a0201' + + '010420ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc' + + '632551a12303210000ffffff00000000ffffffffffffffffbce6faada7179e84' + + 'f3b9cac2fc632551', 'hex'), + }, + // The private key is exactly equal to the order, and the public key is + // omitted. + { + namedCurve: 'P-256', + key: Buffer.from( + '3041020100301306072a8648ce3d020106082a8648ce3d030107042730250201' + + '010420ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc' + + '632551', 'hex'), + }, + // The private key is exactly equal to the order + 11, and the public key is + // private key * order. + { + namedCurve: 'P-521', + key: Buffer.from( + '3081ee020100301006072a8648ce3d020106052b810400230481d63081d30201' + + '01044201ffffffffffffffffffffffffffffffffffffffffffffffffffffffff' + + 'fffffffffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb7' + + '1e91386414a181890381860004008a75841259fdedff546f1a39573b4315cfed' + + '5dc7ed7c17849543ef2c54f2991652f3dbc5332663da1bd19b1aebe319108501' + + '5c024fa4c9a902ecc0e02dda0cdb9a0096fb303fcbba2129849d0ca877054fb2' + + '293add566210bd0493ed2e95d4e0b9b82b1bc8a90e8b42a4ab3892331914a953' + + '36dcac80e3f4819b5d58874f92ce48c808', 'hex'), + }, + // The private key is exactly equal to the order + 11, and the public key is + // omitted. + { + namedCurve: 'P-521', + key: Buffer.from( + '3060020100301006072a8648ce3d020106052b81040023044930470201010442' + + '01ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' + + 'fffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb71e9138' + + '6414', 'hex'), + }, + ]) { + for (const [name, privateUsages] of [ + ['ECDSA', ['sign']], + ['ECDH', ['deriveBits', 'deriveBits']], + ]) { + assert.rejects( + subtle.importKey( + 'pkcs8', + pkcs8, + { name, hash: 'SHA-256', namedCurve }, + true, privateUsages), { name: 'DataError', message: /Invalid keyData/ }, + ).then(common.mustCall()); + } } }