From f97c7a3704866a705797243235ba53fff704658e Mon Sep 17 00:00:00 2001 From: "Node.js GitHub Bot" Date: Sun, 2 Aug 2026 13:32:39 -0400 Subject: [PATCH 1/3] deps: update zlib to 1.3.2.1-motley-42c2f19 PR-URL: https://github.com/nodejs/node/pull/64744 Reviewed-By: Colin Ihrig Reviewed-By: Antoine du Hamel --- deps/zlib/google/zip_reader.cc | 45 ++++++++- deps/zlib/google/zip_reader.h | 5 + deps/zlib/google/zip_reader_unittest.cc | 117 ++++++++++++++++++++++++ src/zlib_version.h | 2 +- 4 files changed, 165 insertions(+), 4 deletions(-) diff --git a/deps/zlib/google/zip_reader.cc b/deps/zlib/google/zip_reader.cc index 002a1e3ed01d..7b5b28967cfb 100644 --- a/deps/zlib/google/zip_reader.cc +++ b/deps/zlib/google/zip_reader.cc @@ -16,6 +16,7 @@ #include "base/files/file.h" #include "base/files/file_util.h" #include "base/functional/bind.h" +#include "base/i18n/i18n_constants.h" #include "base/i18n/icu_string_conversions.h" #include "base/logging.h" #include "base/numerics/safe_conversions.h" @@ -307,25 +308,63 @@ bool ZipReader::OpenEntry() { DCHECK(path_in_zip[info.size_filename] == '\0'); entry_.path_in_original_encoding = path_in_zip.data(); + const char* const configured_encoding = + encoding_.empty() ? base::kCodepageUTF8 : encoding_.c_str(); + const char* entry_path_encoding = configured_encoding; + bool physical_path_is_directory = false; + bool physical_path_is_unsafe = false; + + // If an Info-ZIP Unicode Path Extra Field is present, the physical Central + // Directory path is about to be overridden. Decode and normalize it now into + // `entry_.physical_path` so consumers (e.g. Safe Browsing) can still see the + // name that other tools (e.g. Windows Explorer) would use for extraction. if (info.size_utf8_filename > 0) { + std::u16string physical_path_in_utf16; + if (!base::CodepageToUTF16(entry_.path_in_original_encoding, + configured_encoding, + base::OnStringConversionError::SUBSTITUTE, + &physical_path_in_utf16)) { + LOG(ERROR) << "Cannot convert path from encoding " << configured_encoding; + return false; + } + // Normalize() stores the normalized result in entry_.path; copy it before + // applying the Unicode Path Extra Field below. + Normalize(physical_path_in_utf16); + entry_.physical_path = entry_.path; + physical_path_is_directory = entry_.is_directory; + physical_path_is_unsafe = entry_.is_unsafe; + // Use the Info-ZIP Unicode Path Extra Field if present. DCHECK(info.utf8_filename[info.size_utf8_filename] == '\0'); entry_.path_in_original_encoding = info.utf8_filename; + entry_path_encoding = base::kCodepageUTF8; } // Convert path from original encoding to Unicode. std::u16string path_in_utf16; - const char* const encoding = encoding_.empty() ? "UTF-8" : encoding_.c_str(); - if (!base::CodepageToUTF16(entry_.path_in_original_encoding, encoding, + if (!base::CodepageToUTF16(entry_.path_in_original_encoding, + entry_path_encoding, base::OnStringConversionError::SUBSTITUTE, &path_in_utf16)) { - LOG(ERROR) << "Cannot convert path from encoding " << encoding; + LOG(ERROR) << "Cannot convert path from encoding " << entry_path_encoding; return false; } // Normalize path. Normalize(path_in_utf16); + if (info.size_utf8_filename > 0) { + // Treat an entry as a directory only if both names are directories; + // otherwise callers that analyze file entries should inspect it as a file. + entry_.is_directory = entry_.is_directory && physical_path_is_directory; + // Treat an entry as unsafe if either name is unsafe. + entry_.is_unsafe = entry_.is_unsafe || physical_path_is_unsafe; + } else { + // In the common case (no Unicode Path Extra Field) the physical path + // matches the effective path. + entry_.physical_path = entry_.path; + } + entry_.original_size = info.uncompressed_size; // The file content of this entry is encrypted if flag bit 0 is set. diff --git a/deps/zlib/google/zip_reader.h b/deps/zlib/google/zip_reader.h index 8bc4a21d27c3..af7536149f6f 100644 --- a/deps/zlib/google/zip_reader.h +++ b/deps/zlib/google/zip_reader.h @@ -130,6 +130,11 @@ class ZipReader { // ./a -> DOT/a base::FilePath path; + // Physical path from the ZIP Central Directory, before applying the + // Info-ZIP Unicode Path Extra Field. This is converted and normalized in + // the same way as `path`. + base::FilePath physical_path; + // Size of the original uncompressed file, or 0 if the entry is a directory. // This value should not be trusted, because it is stored as metadata in the // ZIP archive and can be different from the real uncompressed size. diff --git a/deps/zlib/google/zip_reader_unittest.cc b/deps/zlib/google/zip_reader_unittest.cc index 3de65688b7e1..578539ffbf76 100644 --- a/deps/zlib/google/zip_reader_unittest.cc +++ b/deps/zlib/google/zip_reader_unittest.cc @@ -998,6 +998,123 @@ TEST_F(ZipReaderTest, WrongFilenameLength) { reader.Next(); } +TEST_F(ZipReaderTest, UnicodePathExtraFieldPreservesPhysicalPath) { + static const char test_data[] = { + 0x50, 0x4b, 0x03, 0x04, 0x0a, 0x03, 0x00, 0x00, 0x00, 0x00, 0xd0, 0x71, + 0x91, 0x4e, 0x11, 0x2c, 0xf9, 0x51, 0x09, 0x00, 0x00, 0x00, 0x09, 0x00, + 0x00, 0x00, 0x0b, 0x00, 0x00, 0x00, 0x6d, 0x61, 0x6c, 0x77, 0x61, 0x72, + 0x65, 0x2e, 0x65, 0x78, 0x65, 0x54, 0x65, 0x73, 0x74, 0x20, 0x64, 0x61, + 0x74, 0x61, 0x50, 0x4b, 0x01, 0x02, 0x3f, 0x03, 0x0a, 0x03, 0x00, 0x00, + 0x00, 0x00, 0xd0, 0x71, 0x91, 0x4e, 0x11, 0x2c, 0xf9, 0x51, 0x09, 0x00, + 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x15, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x20, 0x80, 0xc9, 0x81, 0x00, 0x00, 0x00, 0x00, + 0x6d, 0x61, 0x6c, 0x77, 0x61, 0x72, 0x65, 0x2e, 0x65, 0x78, 0x65, 0x75, + 0x70, 0x11, 0x00, 0x01, 0xed, 0x4b, 0x16, 0x3c, 0x64, 0x6f, 0x77, 0x6e, + 0x6c, 0x6f, 0x61, 0x64, 0x2e, 0x74, 0x78, 0x74, 0x50, 0x4b, 0x05, 0x06, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x4e, 0x00, 0x00, 0x00, + 0x32, 0x00, 0x00, 0x00, 0x00, 0x00}; + + std::string test_string(test_data, sizeof(test_data)); + ZipReader reader; + ASSERT_TRUE(reader.OpenFromString(test_string)); + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + // The Unicode Path Extra Field overrides the Central Directory filename, + // but the original physical path is preserved separately. `is_unsafe` tracks + // path traversal safety, not whether the filename looks executable. + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("download.txt"), entry->path); + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("malware.exe"), + entry->physical_path); + EXPECT_FALSE(entry->is_directory); + EXPECT_FALSE(entry->is_unsafe); +} + +TEST_F(ZipReaderTest, UnicodePathExtraFieldUsesUtf8WithConfiguredEncoding) { + static constexpr uint8_t test_data[] = { + 0x50, 0x4b, 0x03, 0x04, 0x0a, 0x03, 0x00, 0x00, 0x00, 0x00, 0xd0, 0x71, + 0x91, 0x4e, 0x11, 0x2c, 0xf9, 0x51, 0x09, 0x00, 0x00, 0x00, 0x09, 0x00, + 0x00, 0x00, 0x0b, 0x00, 0x00, 0x00, 0x6d, 0x61, 0x6c, 0x77, 0x61, 0x72, + 0x65, 0x2e, 0x65, 0x78, 0x65, 0x54, 0x65, 0x73, 0x74, 0x20, 0x64, 0x61, + 0x74, 0x61, 0x50, 0x4b, 0x01, 0x02, 0x3f, 0x03, 0x0a, 0x03, 0x00, 0x00, + 0x00, 0x00, 0xd0, 0x71, 0x91, 0x4e, 0x11, 0x2c, 0xf9, 0x51, 0x09, 0x00, + 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x15, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x20, 0x80, 0xc9, 0x81, 0x00, 0x00, 0x00, 0x00, + 0x6d, 0x61, 0x6c, 0x77, 0x61, 0x72, 0x65, 0x2e, 0x65, 0x78, 0x65, 0x75, + 0x70, 0x11, 0x00, 0x01, 0xed, 0x4b, 0x16, 0x3c, 0x72, 0xc3, 0xa9, 0x73, + 0x75, 0x6d, 0xc3, 0xa9, 0x2e, 0x74, 0x78, 0x74, 0x50, 0x4b, 0x05, 0x06, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x4e, 0x00, 0x00, 0x00, + 0x32, 0x00, 0x00, 0x00, 0x00, 0x00}; + + std::string test_string(reinterpret_cast(test_data), + sizeof(test_data)); + ZipReader reader; + ASSERT_TRUE(reader.OpenFromString(test_string)); + reader.SetEncoding("windows-1252"); + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("résumé.txt"), entry->path); + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("malware.exe"), + entry->physical_path); +} + +TEST_F(ZipReaderTest, UnicodePathExtraFieldFileIfEitherPathIsFile) { + static const char test_data[] = { + 0x50, 0x4b, 0x03, 0x04, 0x0a, 0x03, 0x00, 0x00, 0x00, 0x00, 0xd0, 0x71, + 0x91, 0x4e, 0x11, 0x2c, 0xf9, 0x51, 0x09, 0x00, 0x00, 0x00, 0x09, 0x00, + 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x6d, 0x61, 0x6c, 0x77, 0x61, 0x72, + 0x65, 0x2e, 0x65, 0x78, 0x65, 0x2f, 0x54, 0x65, 0x73, 0x74, 0x20, 0x64, + 0x61, 0x74, 0x61, 0x50, 0x4b, 0x01, 0x02, 0x3f, 0x03, 0x0a, 0x03, 0x00, + 0x00, 0x00, 0x00, 0xd0, 0x71, 0x91, 0x4e, 0x11, 0x2c, 0xf9, 0x51, 0x09, + 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x14, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x80, 0xc9, 0x81, 0x00, 0x00, 0x00, + 0x00, 0x6d, 0x61, 0x6c, 0x77, 0x61, 0x72, 0x65, 0x2e, 0x65, 0x78, 0x65, + 0x2f, 0x75, 0x70, 0x10, 0x00, 0x01, 0x5a, 0x5a, 0x54, 0xa7, 0x6d, 0x61, + 0x6c, 0x77, 0x61, 0x72, 0x65, 0x2e, 0x65, 0x78, 0x65, 0x50, 0x4b, 0x05, + 0x06, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x4e, 0x00, 0x00, + 0x00, 0x33, 0x00, 0x00, 0x00, 0x00, 0x00}; + + std::string test_string(test_data, sizeof(test_data)); + ZipReader reader; + ASSERT_TRUE(reader.OpenFromString(test_string)); + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("malware.exe"), entry->path); + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("malware.exe/"), + entry->physical_path); + EXPECT_FALSE(entry->is_directory); + EXPECT_FALSE(entry->is_unsafe); +} + +TEST_F(ZipReaderTest, UnicodePathExtraFieldPreservesUnsafePhysicalPath) { + static const char test_data[] = { + 0x50, 0x4b, 0x03, 0x04, 0x0a, 0x03, 0x00, 0x00, 0x00, 0x00, 0xd0, 0x71, + 0x91, 0x4e, 0x11, 0x2c, 0xf9, 0x51, 0x09, 0x00, 0x00, 0x00, 0x09, 0x00, + 0x00, 0x00, 0x0e, 0x00, 0x00, 0x00, 0x6d, 0x61, 0x6c, + 0x77, 0x61, 0x72, 0x65, 0x2e, 0x65, 0x78, 0x65, 0x2f, + 0x2e, 0x2e, 0x54, 0x65, 0x73, 0x74, + 0x20, 0x64, 0x61, 0x74, 0x61, 0x50, 0x4b, 0x01, 0x02, 0x3f, 0x03, 0x0a, + 0x03, 0x00, 0x00, 0x00, 0x00, 0xd0, 0x71, 0x91, 0x4e, 0x11, 0x2c, 0xf9, + 0x51, 0x09, 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x0e, 0x00, 0x15, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x80, 0xc9, 0x81, 0x00, + 0x00, 0x00, 0x00, 0x6d, 0x61, 0x6c, 0x77, 0x61, 0x72, + 0x65, 0x2e, 0x65, 0x78, 0x65, 0x2f, 0x2e, 0x2e, 0x75, + 0x70, 0x11, 0x00, 0x01, 0x7c, 0xbc, 0xe2, 0x5d, 0x64, + 0x6f, 0x77, 0x6e, 0x6c, 0x6f, 0x61, 0x64, 0x2e, 0x74, + 0x78, 0x74, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x01, 0x00, 0x51, 0x00, 0x00, 0x00, 0x35, 0x00, 0x00, 0x00, 0x00, 0x00}; + + std::string test_string(test_data, sizeof(test_data)); + ZipReader reader; + ASSERT_TRUE(reader.OpenFromString(test_string)); + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("download.txt"), entry->path); + EXPECT_EQ(base::FilePath::FromUTF8Unsafe("malware.exe/UP"), + entry->physical_path); + EXPECT_FALSE(entry->is_directory); + EXPECT_TRUE(entry->is_unsafe); +} + class FileWriterDelegateTest : public ::testing::Test { protected: void SetUp() override { diff --git a/src/zlib_version.h b/src/zlib_version.h index a258bd21a55d..302282a2027e 100644 --- a/src/zlib_version.h +++ b/src/zlib_version.h @@ -2,5 +2,5 @@ // Refer to tools/dep_updaters/update-zlib.sh #ifndef SRC_ZLIB_VERSION_H_ #define SRC_ZLIB_VERSION_H_ -#define ZLIB_VERSION "1.3.2.1-motley-8b3aa8a" +#define ZLIB_VERSION "1.3.2.1-motley-42c2f19" #endif // SRC_ZLIB_VERSION_H_ From 565c3daebf97fd35265cf7fad92065cc2f3a3ff6 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 2 Aug 2026 22:18:09 +0200 Subject: [PATCH 2/3] crypto: support loading private keys through STORE loaders Accept WHATWG URL objects in private-key inputs and load referenced keys through OpenSSL STORE loaders. Pass optional property queries and passphrases while preserving provider-owned EVP_PKEY objects for ordinary KeyObject and CryptoKey operations. Signed-off-by: Filip Skokan PR-URL: https://github.com/nodejs/node/pull/63949 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- .github/workflows/build-shared.yml | 6 + .github/workflows/nix-changes.yml | 2 + .github/workflows/test-shared.yml | 1 + deps/ncrypto/ncrypto.cc | 201 +++++- deps/ncrypto/ncrypto.h | 28 +- doc/api/cli.md | 24 + doc/api/crypto.md | 80 ++- doc/api/permissions.md | 13 +- doc/api/process.md | 2 + doc/node-config-schema.json | 8 + doc/node.1 | 15 + lib/internal/crypto/keys.js | 114 +++- lib/internal/process/permission.js | 1 + lib/internal/process/pre_execution.js | 1 + lib/internal/url.js | 24 + node.gyp | 2 + shell.nix | 140 +++-- src/crypto/crypto_cipher.cc | 6 +- src/crypto/crypto_ec.cc | 5 +- src/crypto/crypto_keys.cc | 139 ++++- src/crypto/crypto_keys.h | 4 +- src/crypto/crypto_sig.cc | 167 ++++- src/env.cc | 4 + src/node_options.cc | 6 + src/node_options.h | 1 + src/permission/openssl_store_permission.cc | 31 + src/permission/openssl_store_permission.h | 34 + src/permission/permission.cc | 8 + src/permission/permission.h | 1 + src/permission/permission_base.h | 6 +- test/parallel/test-crypto-key-store-pkcs11.js | 585 ++++++++++++++++++ test/parallel/test-crypto-key-store.js | 303 +++++++++ test/parallel/test-crypto-sign-verify.js | 45 +- test/parallel/test-permission-has.js | 1 + .../parallel/test-permission-openssl-store.js | 93 +++ .../parallel/test-permission-warning-flags.js | 1 + tools/nix/pkcs11.nix | 128 ++++ typings/internalBinding/crypto.d.ts | 11 +- 38 files changed, 2102 insertions(+), 139 deletions(-) create mode 100644 src/permission/openssl_store_permission.cc create mode 100644 src/permission/openssl_store_permission.h create mode 100644 test/parallel/test-crypto-key-store-pkcs11.js create mode 100644 test/parallel/test-crypto-key-store.js create mode 100644 test/parallel/test-permission-openssl-store.js create mode 100644 tools/nix/pkcs11.nix diff --git a/.github/workflows/build-shared.yml b/.github/workflows/build-shared.yml index 1cd660daadde..c858c34cd18b 100644 --- a/.github/workflows/build-shared.yml +++ b/.github/workflows/build-shared.yml @@ -22,6 +22,11 @@ on: required: false type: string default: '' + pkcs11-store-test: + description: Whether to enable the PKCS#11-backed crypto STORE test + required: false + type: boolean + default: false secrets: CACHIX_AUTH_TOKEN: description: Cachix auth token for nodejs.cachix.org. @@ -84,6 +89,7 @@ jobs: --arg ccache "${NIX_SCCACHE:-null}" \ --arg devTools '[]' \ --arg benchmarkTools '[]' \ + --arg pkcs11 ${{ inputs.pkcs11-store-test }} \ ${{ inputs.extra-nix-flags }} \ --run ' make -C "$TAR_DIR" run-ci -j4 V=1 TEST_CI_ARGS="-p actions --measure-flakiness 9 --skip-tests=$CI_SKIP_TESTS" diff --git a/.github/workflows/nix-changes.yml b/.github/workflows/nix-changes.yml index ddf192603590..819199414377 100644 --- a/.github/workflows/nix-changes.yml +++ b/.github/workflows/nix-changes.yml @@ -69,6 +69,7 @@ jobs: ++ builtins.attrValues ( { inherit (import {}) nixfmt-tree sccache; } // import ./tools/nix/openssl-matrix.nix {} + // import ./tools/nix/pkcs11.nix {} )")" \ | xargs nix-store --realise \ | xargs nix-store --query --requisites \ @@ -77,6 +78,7 @@ jobs: - name: Compute requisites before change shell: bash # See https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference, we want the pipefail option. + # TODO(panva): add `// import ./tools/nix/pkcs11.nix {}` once landed run: | git reset HEAD^ --hard nix-store --query --references "$( diff --git a/.github/workflows/test-shared.yml b/.github/workflows/test-shared.yml index 077e9e21a981..ac76d5b3f32e 100644 --- a/.github/workflows/test-shared.yml +++ b/.github/workflows/test-shared.yml @@ -246,6 +246,7 @@ jobs: with: runner: ubuntu-24.04-arm v8-nar: ${{ needs.build-aarch64-linux-v8.outputs.local-cache && 'libv8-aarch64-linux.nar' }} + pkcs11-store-test: ${{ matrix.openssl.attr == 'openssl_3_5' }} # Override just the `openssl` attr of the default shared-lib set with # the matrix-selected nixpkgs attribute (e.g. `openssl_3_6`). All # other shared libs (brotli, cares, libuv, …) keep their defaults. diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index bed4d48d118d..b73bdc84cf54 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -4,13 +4,13 @@ #include #include #include +#include #include #include #include #if NCRYPTO_USE_BORINGSSL_EVP_DO_ALL_FALLBACK #include #include -#include #endif #include #include @@ -21,6 +21,8 @@ #include #include #include +#include +#include #if OPENSSL_WITH_ARGON2 #include #endif @@ -76,6 +78,17 @@ using BignumCtxPointer = DeleteFnPtr; using BignumGenCallbackPointer = DeleteFnPtr; using NetscapeSPKIPointer = DeleteFnPtr; +#if NCRYPTO_USE_OPENSSL3_PROVIDER +using X509PubKeyPointer = DeleteFnPtr; +// OSSL_STORE_close() returns int, so it needs a void-returning adapter to be +// usable as a DeleteFnPtr deleter. +void CloseStoreCtx(OSSL_STORE_CTX* ctx) { + OSSL_STORE_close(ctx); +} +using StoreCtxPointer = DeleteFnPtr; +using UIMethodPointer = DeleteFnPtr; +#endif + const EVP_CIPHER* GetCipherCtxCipher(const EVP_CIPHER_CTX* ctx) { #if NCRYPTO_USE_OPENSSL3_PROVIDER return EVP_CIPHER_CTX_get0_cipher(ctx); @@ -332,7 +345,7 @@ ClearErrorOnReturn::~ClearErrorOnReturn() { ERR_clear_error(); } -int ClearErrorOnReturn::peekError() { +unsigned long ClearErrorOnReturn::peekError() { // NOLINT(runtime/int) return ERR_peek_error(); } @@ -346,7 +359,7 @@ MarkPopErrorOnReturn::~MarkPopErrorOnReturn() { ERR_pop_to_mark(); } -int MarkPopErrorOnReturn::peekError() { +unsigned long MarkPopErrorOnReturn::peekError() { // NOLINT(runtime/int) return ERR_peek_error(); } @@ -840,6 +853,7 @@ int NoPasswordCallback(char* buf, int size, int rwflag, void* u) { int PasswordCallback(char* buf, int size, int rwflag, void* u) { auto passphrase = static_cast*>(u); + if (size <= 0) return -1; if (passphrase != nullptr) { size_t buflen = static_cast(size); size_t len = passphrase->len; @@ -851,6 +865,31 @@ int PasswordCallback(char* buf, int size, int rwflag, void* u) { return -1; } +#if NCRYPTO_USE_OPENSSL3_PROVIDER +namespace { +struct StorePassphraseData { + Buffer passphrase{.data = nullptr, .len = 0}; + bool has_passphrase = false; + bool missing_passphrase = false; +}; + +int StorePasswordCallback(char* buf, int size, int rwflag, void* u) { + auto data = static_cast(u); + if (data == nullptr || !data->has_passphrase) { + if (data != nullptr) data->missing_passphrase = true; + return -1; + } + + if (size <= 0) return -1; + size_t buflen = static_cast(size); + size_t len = data->passphrase.len; + if (buflen < len) return -1; + memcpy(buf, reinterpret_cast(data->passphrase.data), len); + return len; +} +} // namespace +#endif + // Algorithm: http://howardhinnant.github.io/date_algorithms.html constexpr int days_from_epoch(int y, unsigned m, unsigned d) { y -= m <= 2; @@ -3584,7 +3623,7 @@ EVPKeyPointer::ParseKeyResult EVPKeyPointer::TryParsePrivateKey( const Buffer& buffer) { static constexpr auto keyOrError = [](EVPKeyPointer pkey, bool had_passphrase = false) { - if (int err = ERR_peek_error()) { + if (unsigned long err = ERR_peek_error()) { // NOLINT(runtime/int) if (ERR_GET_LIB(err) == ERR_LIB_PEM && ERR_GET_REASON(err) == PEM_R_BAD_PASSWORD_READ && !had_passphrase) { return ParseKeyResult(PKParseError::NEED_PASSPHRASE); @@ -3644,6 +3683,99 @@ EVPKeyPointer::ParseKeyResult EVPKeyPointer::TryParsePrivateKey( }; } +EVPKeyPointer::ParseKeyResult EVPKeyPointer::TryLoadPrivateKeyFromStore( + const StorePrivateKeyConfig& config) { +#if !NCRYPTO_USE_OPENSSL3_PROVIDER + return ParseKeyResult(PKParseError::FAILED); +#else + // The error queue is left populated on failure so the caller can surface a + // `code` and an `opensslErrorStack`, matching TryParsePrivateKey(), and is + // cleared on success because decoders leave entries behind either way. + std::string uri_str(config.uri); + std::string properties_str; + const char* properties = nullptr; + if (config.properties.has_value() && !config.properties->empty()) { + properties_str.assign(config.properties->data(), config.properties->size()); + properties = properties_str.c_str(); + } + + // config.passphrase outlives this call, so no copy is needed. + Buffer passbuf{.data = nullptr, .len = 0}; + if (config.passphrase.has_value()) { + passbuf.data = const_cast(config.passphrase->data); + passbuf.len = config.passphrase->len; + } + StorePassphraseData passphrase_data{ + .passphrase = passbuf, + .has_passphrase = config.passphrase.has_value(), + }; + // Declared before ctx so that reverse destruction closes the store first; + // it holds both for its lifetime. + UIMethodPointer ui_method( + UI_UTIL_wrap_read_pem_callback(StorePasswordCallback, 0)); + if (!ui_method) return ParseKeyResult(PKParseError::FAILED); + + // Errors from loaders that declined the URI are retained oldest-first, so the + // newest entry is the loader that actually handled it. Must run before ctx is + // destroyed, since OSSL_STORE_close() can push errors of its own. + const auto failed = [&](bool missing_passphrase) { + if (missing_passphrase) + return ParseKeyResult(PKParseError::NEED_PASSPHRASE); + return ParseKeyResult(PKParseError::FAILED, ERR_peek_last_error()); + }; + + const OSSL_PARAM store_params[] = {OSSL_PARAM_END}; + StoreCtxPointer ctx(OSSL_STORE_open_ex(uri_str.c_str(), + nullptr, + properties, + ui_method.get(), + &passphrase_data, + store_params, + nullptr, + nullptr)); + if (!ctx) return failed(passphrase_data.missing_passphrase); + + if (!OSSL_STORE_expect(ctx.get(), OSSL_STORE_INFO_PKEY)) { + return failed(passphrase_data.missing_passphrase); + } + + EVPKeyPointer pkey; + bool store_error = false; + while (!OSSL_STORE_eof(ctx.get())) { + OSSL_STORE_INFO* info = OSSL_STORE_load(ctx.get()); + if (info == nullptr) { + if (OSSL_STORE_error(ctx.get())) { + store_error = true; + break; + } + continue; + } + if (OSSL_STORE_INFO_get_type(info) == OSSL_STORE_INFO_PKEY) { + EVP_PKEY* raw_pkey = OSSL_STORE_INFO_get1_PKEY(info); + if (raw_pkey != nullptr) { + pkey = EVPKeyPointer(raw_pkey); + } else { + store_error = true; + } + } + OSSL_STORE_INFO_free(info); + if (pkey || store_error) break; + } + + // missing_passphrase is sticky, so a key that loaded anyway wins over it. + if (pkey) { + ctx.reset(); + ERR_clear_error(); + return ParseKeyResult(std::move(pkey)); + } + + if (passphrase_data.missing_passphrase || store_error) { + return failed(passphrase_data.missing_passphrase); + } + return ParseKeyResult(PKParseError::NOT_RECOGNIZED); +#endif +} + Result EVPKeyPointer::writePrivateKey( const PrivateKeyEncodingConfig& config) const { if (config.format == PKFormatType::JWK) { @@ -3685,6 +3817,8 @@ Result EVPKeyPointer::writePrivateKey( #else RSA* rsa = EVP_PKEY_get0_RSA(get()); #endif + if (rsa == nullptr) return Result(false); + switch (config.format) { case PKFormatType::PEM: { err = PEM_write_bio_RSAPrivateKey( @@ -3760,6 +3894,8 @@ Result EVPKeyPointer::writePrivateKey( #else EC_KEY* ec = EVP_PKEY_get0_EC_KEY(get()); #endif + if (ec == nullptr) return Result(false); + switch (config.format) { case PKFormatType::PEM: { err = PEM_write_bio_ECPrivateKey( @@ -3826,6 +3962,8 @@ Result EVPKeyPointer::writePublicKey( #else RSA* rsa = EVP_PKEY_get0_RSA(get()); #endif + if (rsa == nullptr) return Result(false); + if (config.format == ncrypto::EVPKeyPointer::PKFormatType::PEM) { // Encode PKCS#1 as PEM. if (PEM_write_bio_RSAPublicKey(bio.get(), rsa) != 1) { @@ -3854,10 +3992,28 @@ Result EVPKeyPointer::writePublicKey( if (config.format == ncrypto::EVPKeyPointer::PKFormatType::PEM) { // Encode SPKI as PEM. +#if NCRYPTO_USE_OPENSSL3_PROVIDER + // Build the SubjectPublicKeyInfo wrapper explicitly before PEM encoding. + // Provider-backed keys can fail the direct PEM_write_bio_PUBKEY() path even + // when OpenSSL can materialize the public wrapper with X509_PUBKEY_set(). + X509_PUBKEY* pubkey = nullptr; + if (X509_PUBKEY_set(&pubkey, get()) != 1) { + X509_PUBKEY_free(pubkey); + return Result(false, + mark_pop_error_on_return.peekError()); + } + X509PubKeyPointer pubkey_ptr(pubkey); + if (PEM_write_bio_X509_PUBKEY(bio.get(), pubkey_ptr.get()) != 1) { + return Result(false, + mark_pop_error_on_return.peekError()); + } +#else + // Non-OpenSSL >= 3 builds do not all declare PEM_write_bio_X509_PUBKEY(). if (PEM_write_bio_PUBKEY(bio.get(), get()) != 1) { return Result(false, mark_pop_error_on_return.peekError()); } +#endif return bio; } @@ -3928,21 +4084,37 @@ std::optional EVPKeyPointer::getBytesOfRS() const { bits = BignumPointer::GetBitCount(q.get()); #else const DSA* dsa_key = EVP_PKEY_get0_DSA(get()); + bool has_bits = false; // Both r and s are computed mod q, so their width is limited by that of q. - bits = BignumPointer::GetBitCount(DSA_get0_q(dsa_key)); + if (dsa_key != nullptr) { + const BIGNUM* q = DSA_get0_q(dsa_key); + if (q != nullptr) { + bits = BignumPointer::GetBitCount(q); + has_bits = true; + } + } + if (!has_bits) return std::nullopt; #endif } else if (id == EVP_PKEY_EC) { #if NCRYPTO_USE_OPENSSL3_PROVIDER Ec ec(get()); if (!ec) return std::nullopt; - bits = EC_GROUP_order_bits(ec.getGroup()); + const EC_GROUP* group = ec.getGroup(); + if (group == nullptr) return std::nullopt; + bits = EC_GROUP_order_bits(group); #else - bits = EC_GROUP_order_bits(ECKeyPointer::GetGroup(*this)); + const EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(get()); + if (ec_key == nullptr) return std::nullopt; + const EC_GROUP* group = ECKeyPointer::GetGroup(ec_key); + if (group == nullptr) return std::nullopt; + bits = EC_GROUP_order_bits(group); #endif } else { return std::nullopt; } + if (bits <= 0) return std::nullopt; + return (bits + 7) / 8; } @@ -3981,12 +4153,12 @@ EVPKeyPointer::operator Dsa() const { bool EVPKeyPointer::validateDsaParameters() const { if (!pkey_) return false; - /* Validate DSA2 parameters from FIPS 186-4 */ #if OPENSSL_VERSION_MAJOR >= 3 if (EVP_default_properties_is_fips_enabled(nullptr) && EVP_PKEY_DSA == id()) { #else if (FIPS_mode() && EVP_PKEY_DSA == id()) { #endif + // Validate DSA2 parameters from FIPS 186-4. #if NCRYPTO_USE_OPENSSL3_PROVIDER DeleteFnPtr p; DeleteFnPtr q; @@ -3998,9 +4170,11 @@ bool EVPKeyPointer::validateDsaParameters() const { const BIGNUM* q_value = q.get(); #else const DSA* dsa = EVP_PKEY_get0_DSA(pkey_.get()); + if (dsa == nullptr) return false; const BIGNUM* p; const BIGNUM* q; DSA_get0_pqg(dsa, &p, &q, nullptr); + if (p == nullptr || q == nullptr) return false; const BIGNUM* p_value = p; const BIGNUM* q_value = q; #endif @@ -6445,9 +6619,14 @@ DataPointer EVPMDCtxPointer::sign( bool EVPMDCtxPointer::verify(const Buffer& buf, const Buffer& sig) const { - if (!ctx_) return false; - int ret = EVP_DigestVerify(ctx_.get(), sig.data, sig.len, buf.data, buf.len); - return ret == 1; + return verifyOneShot(buf, sig) == 1; +} + +int EVPMDCtxPointer::verifyOneShot( + const Buffer& buf, + const Buffer& sig) const { + if (!ctx_) return -1; + return EVP_DigestVerify(ctx_.get(), sig.data, sig.len, buf.data, buf.len); } EVPMDCtxPointer EVPMDCtxPointer::New() { diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index 6fb6b384fd4f..53302394f38b 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -284,7 +284,7 @@ class ClearErrorOnReturn final { NCRYPTO_DISALLOW_COPY_AND_MOVE(ClearErrorOnReturn) NCRYPTO_DISALLOW_NEW_DELETE() - int peekError(); + unsigned long peekError(); // NOLINT(runtime/int) private: CryptoErrorList* errors_; @@ -302,7 +302,7 @@ class MarkPopErrorOnReturn final { NCRYPTO_DISALLOW_COPY_AND_MOVE(MarkPopErrorOnReturn) NCRYPTO_DISALLOW_NEW_DELETE() - int peekError(); + unsigned long peekError(); // NOLINT(runtime/int) private: CryptoErrorList* errors_; @@ -315,9 +315,11 @@ struct Result final { const bool has_value; T value; std::optional error = std::nullopt; - std::optional openssl_error = std::nullopt; + // NOLINTNEXTLINE(runtime/int) -- matches ERR_peek_error() + std::optional openssl_error = std::nullopt; Result(T&& value) : has_value(true), value(std::move(value)) {} - Result(E&& error, std::optional openssl_error = std::nullopt) + // NOLINTNEXTLINE(runtime/int) -- matches ERR_peek_error() + Result(E&& error, std::optional openssl_error = std::nullopt) : has_value(false), error(std::move(error)), openssl_error(std::move(openssl_error)) {} @@ -1046,6 +1048,7 @@ class EVPKeyPointer final { RAW_PUBLIC, RAW_PRIVATE, RAW_SEED, + STORE, }; enum class PKParseError { NOT_RECOGNIZED, NEED_PASSPHRASE, FAILED }; @@ -1078,6 +1081,12 @@ class EVPKeyPointer final { PrivateKeyEncodingConfig& operator=(const PrivateKeyEncodingConfig&); }; + struct StorePrivateKeyConfig { + std::string_view uri; + std::optional properties = std::nullopt; + std::optional> passphrase = std::nullopt; + }; + static ParseKeyResult TryParsePublicKey( const PublicKeyEncodingConfig& config, const Buffer& buffer); @@ -1089,6 +1098,14 @@ class EVPKeyPointer final { const PrivateKeyEncodingConfig& config, const Buffer& buffer); + // Loads a private key through an OpenSSL STORE loader using the configured + // URI (e.g. "file:", a provider-backed scheme such as "pkcs11:"). The + // optional passphrase is used as the PIN/passphrase for encrypted or + // token-protected keys. + // Returns NOT_RECOGNIZED when no private key is found at the URI. + static ParseKeyResult TryLoadPrivateKeyFromStore( + const StorePrivateKeyConfig& config); + EVPKeyPointer() = default; explicit EVPKeyPointer(EVP_PKEY* pkey); EVPKeyPointer(EVPKeyPointer&& other) noexcept; @@ -1681,6 +1698,9 @@ class EVPMDCtxPointer final { DataPointer sign(const Buffer& buf) const; bool verify(const Buffer& buf, const Buffer& sig) const; + // Unlike verify(), preserves EVP_DigestVerify()'s three-way result. + int verifyOneShot(const Buffer& buf, + const Buffer& sig) const; const EVP_MD* getDigest() const; size_t getDigestSize() const; diff --git a/doc/api/cli.md b/doc/api/cli.md index 7318e51f9707..3e8dcd4f27e5 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -361,6 +361,25 @@ Error: connect ERR_ACCESS_DENIED Access to this API has been restricted. Use --a } ``` +### `--allow-openssl-store` + + + +> Stability: 1.1 - Active development + +When using the [Permission Model][], the process will not be able to use +OpenSSL STORE loaders by default, for example to load a private key from a +{URL} passed to [`crypto.createPrivateKey()`][]. Attempts to do so will throw +an `ERR_ACCESS_DENIED` unless the user explicitly passes the +`--allow-openssl-store` flag. This permission can be dropped at runtime via +[`permission.drop()`][]. + +This flag grants broad authority to configured OpenSSL STORE loaders. A loader +may access files, devices, tokens, or the network. Access performed by a loader +is not constrained by the `fs.read`, `fs.write`, or `net` permission scopes. + ### `--allow-wasi` -* `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} +* `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|URL} * `dsaEncoding` {string} * `padding` {integer} * `saltLength` {integer} @@ -3978,6 +3978,10 @@ input.on('readable', () => { -* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView} - * `key` {string|ArrayBuffer|Buffer|TypedArray|DataView|Object} The key - material, either in PEM, DER, JWK, or raw format. +* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|URL} + * `key` {string|ArrayBuffer|Buffer|TypedArray|DataView|Object|URL} The key + material, either in PEM, DER, JWK, or raw format, or a {URL} referencing an + object for an OpenSSL STORE loader. * `format` {string} Must be `'pem'`, `'der'`, `'jwk'`, `'raw-private'`, or `'raw-seed'`. **Default:** `'pem'`. * `type` {string} Must be `'pkcs1'`, `'pkcs8'` or `'sec1'`. This option is required only if the `format` is `'der'` and ignored otherwise. - * `passphrase` {string | Buffer} The passphrase to use for decryption. + * `passphrase` {string | Buffer} The passphrase to use for decryption. When + `key` is a {URL}, this is the optional PIN/passphrase forwarded to the + STORE loader. + * `properties` {string} The optional OpenSSL property query used when + fetching the STORE loader for a {URL} key. * `encoding` {string} The string encoding to use when `key` is a string. * `asymmetricKeyType` {string} Required when `format` is `'raw-private'` or `'raw-seed'` and ignored otherwise. @@ -4033,6 +4042,46 @@ must be an object with the properties described above. If the private key is encrypted, a `passphrase` must be specified. The length of the passphrase is limited to 1024 bytes. +#### Private keys from OpenSSL STORE loaders + +> Stability: 1.1 - Active development + +If `key` is a {URL} (or an object whose `key` is a {URL}), the private key is +loaded through an OpenSSL STORE loader. The URL is passed to OpenSSL as a URI, +for example a `file:` URI or a provider-backed scheme such as `pkcs11:`. When +the [Permission Model][] is enabled, [`--allow-openssl-store`][] is required. + +> **Warning**: A URI scheme does not pin an OpenSSL STORE loader or prove where +> the returned key came from. Node.js forwards the URI to OpenSSL, which chooses +> loaders according to its version and configuration. For example, OpenSSL may +> offer an opaque URI such as `pkcs11:object=...` (one without `//` after the +> scheme) to its `file` loader before trying the `pkcs11` loader. If the complete +> URI is a valid local path and that file exists, it may be loaded instead. +> Node.js does not verify which loader supplied the key. Do not rely on a +> provider-specific URI scheme as proof that a key came from that provider or +> from a hardware device. + +Configured OpenSSL STORE loaders have broad authority and may access files, +devices, tokens, or the network. Access performed by a loader is not constrained +by the `fs.read`, `fs.write`, or `net` permission scopes. + +When a {URL} is used, `format`, `type`, `asymmetricKeyType`, and `namedCurve` +are ignored even when those options would otherwise depend on each other, such +as `type` with `format: 'der'` or `namedCurve` with +`asymmetricKeyType: 'ec'`. The input is passed to the STORE loader as a URI, +not handled as PEM, DER, JWK, or raw key material. `passphrase` is still used as +the optional PIN/passphrase passed to the loader, and `encoding` applies if that +`passphrase` is a string. + +Use `passphrase` instead of embedding credentials in the URI passed to the +STORE loader. Node.js redacts the URI from its own permission-denial resource +and diagnostics. Errors reported by OpenSSL or a provider after loading begins +may include the URI. + +When `properties` is specified with a {URL} key, it is passed to OpenSSL as the +property query for selecting the STORE loader. It is not appended to the URL and +is distinct from provider-specific URI parameters. + ### `crypto.createPublicKey(key)` -* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} Private Key +* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|URL} Private Key * `ciphertext` {ArrayBuffer|Buffer|TypedArray|DataView} * `callback` {Function} * `err` {Error} @@ -4221,7 +4274,7 @@ changes: --> * `options` {Object} - * `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} + * `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|URL} * `publicKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} * `callback` {Function} * `err` {Error} @@ -5310,7 +5363,7 @@ changes: -* `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} +* `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|URL} * `oaepHash` {string} The hash function to use for OAEP padding and MGF1. **Default:** `'sha1'` * `oaepLabel` {string|ArrayBuffer|Buffer|TypedArray|DataView} The label to @@ -5358,9 +5411,10 @@ changes: -* `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} - * `key` {string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} - A PEM encoded private key. +* `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|URL} + * `key` {string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|URL} + The private key material, a {KeyObject}, or a {URL} referencing an object + for an OpenSSL STORE loader. * `passphrase` {string|ArrayBuffer|Buffer|TypedArray|DataView} An optional passphrase for the private key. * `padding` {crypto.constants} An optional padding value defined in @@ -6219,7 +6273,7 @@ changes: * `algorithm` {string | null | undefined} * `data` {ArrayBuffer|Buffer|SharedArrayBuffer|TypedArray|DataView|string} -* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject} +* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|URL} * `callback` {Function} * `err` {Error} * `signature` {Buffer} @@ -6990,6 +7044,7 @@ See the [list of SSL OP Flags][] for details. [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf [OpenSSL's FIPS README file]: https://github.com/openssl/openssl/blob/openssl-3.0/README-FIPS.md [OpenSSL's SPKAC implementation]: https://www.openssl.org/docs/man3.0/man1/openssl-spkac.html +[Permission Model]: permissions.md#permission-model [RFC 1421]: https://www.rfc-editor.org/rfc/rfc1421.txt [RFC 2409]: https://www.rfc-editor.org/rfc/rfc2409.txt [RFC 2818]: https://www.rfc-editor.org/rfc/rfc2818.txt @@ -7003,6 +7058,7 @@ See the [list of SSL OP Flags][] for details. [RFC 8032]: https://www.rfc-editor.org/rfc/rfc8032.txt [RFC 9562]: https://www.rfc-editor.org/rfc/rfc9562.txt [Web Crypto API documentation]: webcrypto.md +[`--allow-openssl-store`]: cli.md#--allow-openssl-store [`BN_is_prime_ex`]: https://www.openssl.org/docs/man1.1.1/man3/BN_is_prime_ex.html [`Buffer`]: buffer.md [`DH_generate_key()`]: https://www.openssl.org/docs/man3.0/man3/DH_generate_key.html diff --git a/doc/api/permissions.md b/doc/api/permissions.md index 05a18db0457d..c387e899cf1a 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -85,6 +85,13 @@ flag. For WASI, use the [`--allow-wasi`][] flag. For FFI, use the [`--allow-ffi`][] flag. The [`node:ffi`](ffi.md) module also requires the `--experimental-ffi` flag and is only available in builds with FFI support. +To allow use of OpenSSL STORE loaders, for example to load a private key +from a {URL} passed to [`crypto.createPrivateKey()`][], use the +[`--allow-openssl-store`][] flag. +This flag grants broad authority to configured OpenSSL STORE loaders, which may +access files, devices, tokens, or the network. Access performed by a loader is +not constrained by the `fs.read`, `fs.write`, or `net` permission scopes. + #### Runtime API When enabling the Permission Model through the [`--permission`][] @@ -269,7 +276,8 @@ Example `node.config.json`: "allow-worker": true, "allow-net": true, "allow-addons": false, - "allow-ffi": false + "allow-ffi": false, + "allow-openssl-store": false } } ``` @@ -332,6 +340,7 @@ There are constraints you need to know before using this system: * File system access * WASI * FFI + * OpenSSL STORE loaders * The Permission Model is initialized after the Node.js environment is set up. However, certain flags such as `--env-file` or `--openssl-config` are designed to read files before environment initialization. As a result, such flags are @@ -383,9 +392,11 @@ Developers relying on --permission to sandbox untrusted code should be aware tha [`--allow-fs-read`]: cli.md#--allow-fs-read [`--allow-fs-write`]: cli.md#--allow-fs-write [`--allow-net`]: cli.md#--allow-net +[`--allow-openssl-store`]: cli.md#--allow-openssl-store [`--allow-wasi`]: cli.md#--allow-wasi [`--allow-worker`]: cli.md#--allow-worker [`--permission-audit`]: cli.md#--permission-audit [`--permission`]: cli.md#--permission +[`crypto.createPrivateKey()`]: crypto.md#cryptocreateprivatekeykey [`npx`]: https://docs.npmjs.com/cli/commands/npx [`permission.has()`]: process.md#processpermissionhasscope-reference diff --git a/doc/api/process.md b/doc/api/process.md index 48c7e868ae8f..68140744766b 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -3162,6 +3162,7 @@ The available scopes are: * `fs.read` - File System read operations * `fs.write` - File System write operations * `child` - Child process spawning operations +* `openssl.store` - Loading keys through OpenSSL STORE loaders * `worker` - Worker thread spawning operation * `ffi` - Foreign function interface operations @@ -3216,6 +3217,7 @@ The available scopes are the same as [`process.permission.has()`][]: * `fs.read` - File System read operations * `fs.write` - File System write operations * `child` - Child process spawning operations +* `openssl.store` - Loading keys through OpenSSL STORE loaders * `worker` - Worker thread spawning operation * `net` - Network operations * `inspector` - Inspector operations diff --git a/doc/node-config-schema.json b/doc/node-config-schema.json index 116f0c286818..df75fc3cf37a 100644 --- a/doc/node-config-schema.json +++ b/doc/node-config-schema.json @@ -103,6 +103,10 @@ "type": "boolean", "description": "allow use of network when any permissions are set" }, + "allow-openssl-store": { + "type": "boolean", + "description": "allow use of OpenSSL STORE loaders when any permissions are set" + }, "allow-wasi": { "type": "boolean", "description": "allow wasi when any permissions are set" @@ -864,6 +868,10 @@ "type": "boolean", "description": "allow use of network when any permissions are set" }, + "allow-openssl-store": { + "type": "boolean", + "description": "allow use of OpenSSL STORE loaders when any permissions are set" + }, "allow-wasi": { "type": "boolean", "description": "allow wasi when any permissions are set" diff --git a/doc/node.1 b/doc/node.1 index 05b80da80a43..ad5e0415a54e 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -222,6 +222,17 @@ Error: connect ERR_ACCESS_DENIED Access to this API has been restricted. Use --a } .Ed . +.It Fl -allow-openssl-store +When using the Permission Model, the process will not be able to use +OpenSSL STORE loaders by default, for example to load a private key from a +\fB\fR passed to \fBcrypto.createPrivateKey()\fR. Attempts to do so will throw +an \fBERR_ACCESS_DENIED\fR unless the user explicitly passes the +\fB--allow-openssl-store\fR flag. This permission can be dropped at runtime via +\fBpermission.drop()\fR. +This flag grants broad authority to configured OpenSSL STORE loaders. A loader +may access files, devices, tokens, or the network. Access performed by a loader +is not constrained by the \fBfs.read\fR, \fBfs.write\fR, or \fBnet\fR permission scopes. +. .It Fl -allow-wasi When using the Permission Model, the process will not be capable of creating any WASI instances by default. @@ -1183,6 +1194,8 @@ WASI - manageable through \fB--allow-wasi\fR flag Addons - manageable through \fB--allow-addons\fR flag .It FFI - manageable through \fB--allow-ffi\fR flag +.It +OpenSSL STORE loaders - manageable through \fB--allow-openssl-store\fR flag .El . .It Fl -permission-audit @@ -1929,6 +1942,8 @@ one is included in the list below. .It \fB--allow-net\fR .It +\fB--allow-openssl-store\fR +.It \fB--allow-wasi\fR .It \fB--allow-worker\fR diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index 4efc5a0d4e3e..45d7cd535013 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -6,6 +6,8 @@ const { ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, SafeSet, + StringPrototypeIncludes, + StringPrototypeStartsWith, SymbolToStringTag, Uint8Array, } = primordials; @@ -27,6 +29,7 @@ const { kKeyFormatRawPublic, kKeyFormatRawPrivate, kKeyFormatRawSeed, + kKeyFormatStore, kKeyEncodingPKCS1, kKeyEncodingPKCS8, kKeyEncodingSPKI, @@ -72,6 +75,12 @@ const { isArrayBufferView, } = require('internal/util/types'); +const { + fileURLToPath, + getURLHref, + isURLInstance, +} = require('internal/url'); + const { customInspectSymbol: kInspect, kEnumerableProperty, @@ -508,7 +517,7 @@ function validateAsymmetricKeyType(type, ctx, key) { if (ctx === kCreatePrivate) { throw new ERR_INVALID_ARG_TYPE( 'key', - ['string', 'ArrayBuffer', 'Buffer', 'TypedArray', 'DataView'], + getKeyTypes({ allowKeyObject: false, allowURL: true }), key, ); } @@ -523,37 +532,88 @@ function validateAsymmetricKeyType(type, ctx, key) { } } -function getKeyTypes(allowKeyObject, bufferOnly = false) { - const types = [ - 'ArrayBuffer', - 'Buffer', - 'TypedArray', - 'DataView', - 'string', // Only if bufferOnly == false - 'KeyObject', // Only if allowKeyObject == true && bufferOnly == false +const kKeyTypesBufferOnly = [ + 'ArrayBuffer', + 'Buffer', + 'TypedArray', + 'DataView', +]; + +function getKeyTypes({ allowKeyObject, allowURL = false }) { + return [ + ...kKeyTypesBufferOnly, + 'string', + ...(allowKeyObject ? ['KeyObject'] : []), + ...(allowURL ? ['URL'] : []), ]; - if (bufferOnly) { - return ArrayPrototypeSlice(types, 0, 4); - } else if (!allowKeyObject) { - return ArrayPrototypeSlice(types, 0, 5); - } - return types; } +function prepareStorePrivateKey(url, name, passphrase, encoding, properties) { + // Read the canonical serialization from private URL state. Public accessors + // can be overridden by subclasses or through prototype mutation. + let uri = getURLHref(url); + if (StringPrototypeStartsWith(uri, 'file:')) + uri = fileURLToPath(uri); + + validateString(uri, name); + // The URI is handed to OpenSSL as a NUL-terminated C string. Reject embedded + // NUL bytes, which `fileURLToPath()` happily decodes from `%00`, rather than + // letting the URI be silently truncated. + if (StringPrototypeIncludes(uri, '\u0000')) { + throw new ERR_INVALID_ARG_VALUE( + name, url, 'must be a URL without null bytes'); + } + + if (passphrase !== undefined) { + if (!isStringOrBuffer(passphrase)) { + throw new ERR_INVALID_ARG_VALUE(option('passphrase', name), passphrase); + } + passphrase = getArrayBufferOrView( + passphrase, + option('passphrase', name), + encoding); + } + + if (properties !== undefined) { + validateString(properties, option('properties', name)); + // Reaches OpenSSL as a NUL-terminated C string, same as the URI above. + if (StringPrototypeIncludes(properties, '\u0000')) { + throw new ERR_INVALID_ARG_VALUE( + option('properties', name), + properties, + 'must be a string without null bytes'); + } + } + + return { + data: { + uri, + properties: properties ?? null, + }, + format: kKeyFormatStore, + passphrase: passphrase ?? null, + }; +} function prepareAsymmetricKey(key, ctx, name = 'key') { + const isPrivateKeyInput = ctx === kConsumePrivate || ctx === kCreatePrivate; + if (isKeyObject(key)) { // Best case: A key object, as simple as that. const type = getKeyObjectType(key); validateAsymmetricKeyType(type, ctx, key); return { data: getKeyObjectHandle(key) }; } + if (isPrivateKeyInput && isURLInstance(key)) { + // A private key referenced by a URI for an OpenSSL STORE loader. + return prepareStorePrivateKey(key, name); + } if (isStringOrBuffer(key)) { // Expect PEM by default, mostly for backward compatibility. return { format: kKeyFormatPEM, data: getArrayBufferOrView(key, name) }; } if (typeof key === 'object') { - const { key: data, encoding, format } = key; + const { key: data, encoding, format, properties } = key; // The 'key' property can be a KeyObject as well to allow specifying // additional options such as padding along with the key. @@ -562,6 +622,16 @@ function prepareAsymmetricKey(key, ctx, name = 'key') { validateAsymmetricKeyType(type, ctx, data); return { data: getKeyObjectHandle(data) }; } + if (isPrivateKeyInput && isURLInstance(data)) { + // A private key referenced by a URI for an OpenSSL STORE loader, with + // an optional passphrase/PIN. + return prepareStorePrivateKey( + data, + name, + key.passphrase, + encoding, + properties); + } if (format === 'jwk') { validateObject(data, `${name}.key`); return { data, format: kKeyFormatJWK }; @@ -594,7 +664,10 @@ function prepareAsymmetricKey(key, ctx, name = 'key') { if (!isStringOrBuffer(data)) { throw new ERR_INVALID_ARG_TYPE( `${name}.key`, - getKeyTypes(ctx !== kCreatePrivate), + getKeyTypes({ + allowKeyObject: ctx !== kCreatePrivate, + allowURL: isPrivateKeyInput, + }), data); } @@ -608,7 +681,10 @@ function prepareAsymmetricKey(key, ctx, name = 'key') { throw new ERR_INVALID_ARG_TYPE( name, - getKeyTypes(ctx !== kCreatePrivate), + getKeyTypes({ + allowKeyObject: ctx !== kCreatePrivate, + allowURL: isPrivateKeyInput, + }), key); } @@ -634,7 +710,7 @@ function prepareSecretKey(key, encoding, bufferOnly = false) { !isAnyArrayBuffer(key)) { throw new ERR_INVALID_ARG_TYPE( 'key', - getKeyTypes(!bufferOnly, bufferOnly), + bufferOnly ? kKeyTypesBufferOnly : getKeyTypes({ allowKeyObject: true }), key); } return getArrayBufferOrView(key, 'key', encoding); diff --git a/lib/internal/process/permission.js b/lib/internal/process/permission.js index 78e10e6e15fd..8dd92b1709b2 100644 --- a/lib/internal/process/permission.js +++ b/lib/internal/process/permission.js @@ -70,6 +70,7 @@ module.exports = ObjectFreeze({ '--allow-inspector', '--allow-wasi', '--allow-worker', + '--allow-openssl-store', ]; if (_ffi) { diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 1d380db24111..9d5891e7a24a 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -697,6 +697,7 @@ function initializePermission() { { flag: '--allow-inspector', enabled: true, code: 'PERM0004' }, { flag: '--allow-wasi', enabled: true, code: 'PERM0005' }, { flag: '--allow-worker', enabled: true, code: 'PERM0006' }, + { flag: '--allow-openssl-store', enabled: true, code: 'PERM0007' }, ]; for (const { flag, enabled, code } of warnFlags) { if (enabled && getOptionValue(flag)) { diff --git a/lib/internal/url.js b/lib/internal/url.js index be20127b45f1..e96df6148f98 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -220,6 +220,21 @@ let setURLSearchParamsModified; let setURLSearchParamsContext; let getURLSearchParamsList; let setURLSearchParams; +/** + * Brand check for URL instances created by this realm's URL constructor. + * + * Unlike `isURL()`, which duck-types so that URL objects coming from other + * implementations are recognized, this cannot be satisfied by an ordinary + * object that merely exposes `href` and `protocol`. Use it where treating an + * attacker-supplied plain object as a URL would be a security concern. + * @type {(value: unknown) => value is URL} + */ +let isURLInstance; +/** + * Returns the canonical serialization of a URL from its private state. + * @type {(value: URL) => string} + */ +let getURLHref; class URLSearchParamsIterator { #target; @@ -809,6 +824,13 @@ class URL { #searchParamsModified; static { + isURLInstance = (value) => typeof value === 'object' && value !== null && #context in value; + + getURLHref = (value) => { + value.#ensureSearchParamsUpdated(); + return value.#context.href; + }; + setURLSearchParamsModified = (obj) => { // When URLSearchParams changes, we lazily update URL on the next read/write for performance. obj.#searchParamsModified = true; @@ -1721,6 +1743,8 @@ module.exports = { urlToHttpOptions, encodeStr, isURL, + isURLInstance, + getURLHref, urlUpdateActions: updateActions, getURLOrigin, diff --git a/node.gyp b/node.gyp index e88bac122b93..fd3da4164fb4 100644 --- a/node.gyp +++ b/node.gyp @@ -179,6 +179,7 @@ 'src/node_zlib.cc', 'src/path.cc', 'src/permission/child_process_permission.cc', + 'src/permission/openssl_store_permission.cc', 'src/permission/ffi_permission.cc', 'src/permission/fs_permission.cc', 'src/permission/inspector_permission.cc', @@ -314,6 +315,7 @@ 'src/node_worker.h', 'src/path.h', 'src/permission/child_process_permission.h', + 'src/permission/openssl_store_permission.h', 'src/permission/ffi_permission.h', 'src/permission/fs_permission.h', 'src/permission/inspector_permission.h', diff --git a/shell.nix b/shell.nix index b97bcac5d7a8..74a93d0e9ff4 100644 --- a/shell.nix +++ b/shell.nix @@ -34,6 +34,13 @@ } ), + # PKCS#11 fixture for `test/parallel/test-crypto-key-store-pkcs11.js`. `true` + # builds the one from tools/nix/pkcs11.nix against the OpenSSL being linked, + # which needs a shared OpenSSL >= 3 and a platform nixpkgs ships + # pkcs11-provider for. `false` and `null` disable it, any other value is used + # as the fixture itself. + pkcs11 ? false, + # dev tools (not needed to build Node.js, useful to maintain it) ncu-path ? null, # Provide this if you want to use a local version of NCU devTools ? import ./tools/nix/devTools.nix { inherit pkgs ncu-path; }, @@ -48,6 +55,16 @@ let useSharedTemporal = builtins.hasAttr "temporal_capi" sharedLibDeps; needsRustCompiler = withTemporal && !useSharedTemporal; + usePkcs11 = pkcs11 != false && pkcs11 != null; + pkcs11Fixture = + if pkcs11 == true then + import ./tools/nix/pkcs11.nix { + inherit pkgs; + inherit (sharedLibDeps) openssl; + } + else + pkcs11; + nativeBuildInputs = pkgs.nodejs-slim_latest.nativeBuildInputs ++ pkgs.lib.optionals needsRustCompiler [ @@ -71,66 +88,75 @@ let ++ pkgs.lib.optional (withTemporal && useSharedTemporal) "--shared-temporal_capi" ++ pkgs.lib.optional withPerfetto "--with-perfetto"; in -pkgs.mkShell { - inherit nativeBuildInputs; +pkgs.mkShell ( + { + inherit nativeBuildInputs; - buildInputs = - builtins.attrValues sharedLibDeps - ++ buildInputs - ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) ( - if useSeparateDerivationForV8 == true then - let - sharedLibsToMock = pkgs.callPackage ./tools/nix/non-v8-deps-mock.nix { }; - in - pkgs.callPackage ./tools/nix/v8.nix { - inherit nativeBuildInputs icu; + buildInputs = + builtins.attrValues sharedLibDeps + ++ buildInputs + ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) ( + if useSeparateDerivationForV8 == true then + let + sharedLibsToMock = pkgs.callPackage ./tools/nix/non-v8-deps-mock.nix { }; + in + pkgs.callPackage ./tools/nix/v8.nix { + inherit nativeBuildInputs icu; - configureFlags = configureFlags ++ sharedLibsToMock.configureFlags ++ [ "--ninja" ]; - buildInputs = buildInputs ++ [ sharedLibsToMock ]; - } - else - useSeparateDerivationForV8 - ); + configureFlags = configureFlags ++ sharedLibsToMock.configureFlags ++ [ "--ninja" ]; + buildInputs = buildInputs ++ [ sharedLibsToMock ]; + } + else + useSeparateDerivationForV8 + ); - packages = devTools ++ benchmarkTools ++ pkgs.lib.optional (ccache != null) ccache; + packages = devTools ++ benchmarkTools ++ pkgs.lib.optional (ccache != null) ccache; - shellHook = pkgs.lib.optionalString (ccache != null) '' - export CC="${pkgs.lib.getExe ccache} $CC" - export CXX="${pkgs.lib.getExe ccache} $CXX" - ''; + shellHook = pkgs.lib.optionalString (ccache != null) '' + export CC="${pkgs.lib.getExe ccache} $CC" + export CXX="${pkgs.lib.getExe ccache} $CXX" + ''; - BUILD_WITH = if (ninja != null) then "ninja" else "make"; - NINJA = pkgs.lib.optionalString (ninja != null) "${pkgs.lib.getExe ninja}"; - CONFIG_FLAGS = builtins.toString ( - configureFlags - ++ extraConfigFlags - ++ pkgs.lib.optional (ninja != null) "--ninja" - ++ pkgs.lib.optional (!withAmaro) "--without-amaro" - ++ pkgs.lib.optional (!withLief) "--without-lief" - ++ pkgs.lib.optional withQuic "--experimental-quic" - ++ pkgs.lib.optional (!withSQLite) "--without-sqlite" - ++ pkgs.lib.optional (!withFFI) "--without-ffi" - ++ pkgs.lib.optional (!withSSL) "--without-ssl" - ++ pkgs.lib.optional loadJSBuiltinsDynamically "--node-builtin-modules-path=${builtins.toString ./.}" - ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) "--without-bundled-v8" - ++ - pkgs.lib.concatMap - (name: [ - "--shared-${name}" - "--shared-${name}-libpath=${pkgs.lib.getLib sharedLibDeps.${name}}/lib" - "--shared-${name}-include=${pkgs.lib.getInclude sharedLibDeps.${name}}/include" - ]) - ( - builtins.attrNames ( - if (useSeparateDerivationForV8 != false) then - builtins.removeAttrs sharedLibDeps [ - "simdutf" - "temporal_capi" - ] - else - sharedLibDeps + BUILD_WITH = if (ninja != null) then "ninja" else "make"; + NINJA = pkgs.lib.optionalString (ninja != null) "${pkgs.lib.getExe ninja}"; + CONFIG_FLAGS = builtins.toString ( + configureFlags + ++ extraConfigFlags + ++ pkgs.lib.optional (ninja != null) "--ninja" + ++ pkgs.lib.optional (!withAmaro) "--without-amaro" + ++ pkgs.lib.optional (!withLief) "--without-lief" + ++ pkgs.lib.optional withQuic "--experimental-quic" + ++ pkgs.lib.optional (!withSQLite) "--without-sqlite" + ++ pkgs.lib.optional (!withFFI) "--without-ffi" + ++ pkgs.lib.optional (!withSSL) "--without-ssl" + ++ pkgs.lib.optional loadJSBuiltinsDynamically "--node-builtin-modules-path=${builtins.toString ./.}" + ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) "--without-bundled-v8" + ++ + pkgs.lib.concatMap + (name: [ + "--shared-${name}" + "--shared-${name}-libpath=${pkgs.lib.getLib sharedLibDeps.${name}}/lib" + "--shared-${name}-include=${pkgs.lib.getInclude sharedLibDeps.${name}}/include" + ]) + ( + builtins.attrNames ( + if (useSeparateDerivationForV8 != false) then + builtins.removeAttrs sharedLibDeps [ + "simdutf" + "temporal_capi" + ] + else + sharedLibDeps + ) ) - ) - ); - NOSQLITE = pkgs.lib.optionalString (!withSQLite) "1"; -} + ); + } + // pkgs.lib.optionalAttrs (!withSQLite) { + NOSQLITE = "1"; + } + // pkgs.lib.optionalAttrs usePkcs11 { + NODE_TEST_PKCS11_OPENSSL_CONF = "${pkcs11Fixture.opensslConf}"; + NODE_TEST_PKCS11_PIN = pkcs11Fixture.softhsmDir.pin; + NODE_TEST_PKCS11_SOFTHSM_DIR = "${pkcs11Fixture.softhsmDir}"; + } +) diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc index a165e8e3409d..92682328d0ba 100644 --- a/src/crypto/crypto_cipher.cc +++ b/src/crypto/crypto_cipher.cc @@ -837,7 +837,11 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); unsigned int offset = 0; - auto data = KeyObjectData::GetPublicOrPrivateKeyFromJs(args, &offset); + // TODO(panva): Use GetPrivateKeyFromJs() for private operations, then + // remove allow_private_key_store and URL handling from + // GetPublicOrPrivateKeyFromJs(). + auto data = KeyObjectData::GetPublicOrPrivateKeyFromJs( + args, &offset, operation == PublicKeyCipher::kPrivate); if (!data) return; const auto& pkey = data.GetAsymmetricKey(); if (!pkey) return; diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index 38f6831e7a9d..388931c11e79 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -475,13 +475,15 @@ bool ExportJWKEcKey(Environment* env, THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK EC key"); return false; } + // A provider-backed key need not expose its public point. + if (ec.getPublicKey() == nullptr) return false; const auto pub = ec.getPublicKey(); const auto group = ec.getGroup(); int degree_bits = EC_GROUP_get_degree(group); int degree_bytes = - (degree_bits / CHAR_BIT) + (7 + (degree_bits % CHAR_BIT)) / 8; + (degree_bits / CHAR_BIT) + (7 + (degree_bits % CHAR_BIT)) / 8; auto x = BignumPointer::New(); auto y = BignumPointer::New(); @@ -543,6 +545,7 @@ bool ExportJWKEcKey(Environment* env, if (key.GetKeyType() == kKeyTypePrivate) { auto pvt = ec.getPrivateKey(); + if (pvt == nullptr) return false; return SetEncodedValue(env, target, env->jwk_d_string(), pvt, degree_bytes) .IsJust(); } diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 111462463833..d8bdc93deff8 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -12,6 +12,7 @@ #include "memory_tracker-inl.h" #include "node.h" #include "node_buffer.h" +#include "permission/permission.h" #include "string_bytes.h" #include "threadpoolwork-inl.h" #include "util-inl.h" @@ -390,6 +391,12 @@ bool KeyObjectData::ToEncodedPublicKey( THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env); return false; } + // A provider-backed key need not expose its public point. + if (ec_key.getPublicKey() == nullptr) { + THROW_ERR_CRYPTO_OPERATION_FAILED(env, + "Failed to export EC public key"); + return false; + } auto form = static_cast(config.ec_point_form); const auto group = ec_key.getGroup(); const auto point = ec_key.getPublicKey(); @@ -441,7 +448,8 @@ bool KeyObjectData::ToEncodedPrivateKey( } const BIGNUM* private_key = ec_key.getPrivateKey(); if (private_key == nullptr) { - THROW_ERR_CRYPTO_OPERATION_FAILED(env, "Failed to get EC private key"); + THROW_ERR_CRYPTO_OPERATION_FAILED(env, + "Failed to export EC private key"); return false; } const auto group = ec_key.getGroup(); @@ -764,7 +772,103 @@ KeyObjectData KeyObjectData::GetPrivateKeyFromJs( bool allow_key_object) { Environment* env = Environment::GetCurrent(args); - // JWK format: data is a JS Object (not buffer), format int is JWK. + // Store descriptor: data is a { uri, properties } object, format int is + // STORE, and the passphrase slot carries an optional passphrase/PIN. + if (args[*offset]->IsObject() && !IsAnyBufferSource(args[*offset]) && + args[*offset + 1]->IsInt32() && + static_cast( + args[*offset + 1].As()->Value()) == + EVPKeyPointer::PKFormatType::STORE) { + Local store = args[*offset].As(); + Local uri_value; + if (!store + ->Get(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "uri")) + .ToLocal(&uri_value)) { + return {}; + } + CHECK(uri_value->IsString()); + Utf8Value uri(env->isolate(), uri_value); + + Local properties_value; + if (!store + ->Get(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "properties")) + .ToLocal(&properties_value)) { + return {}; + } + std::string properties_storage; + std::optional properties; + if (properties_value->IsString()) { + Utf8Value properties_string(env->isolate(), properties_value); + std::string_view properties_view = properties_string.ToStringView(); + properties_storage.assign(properties_view.data(), properties_view.size()); + properties = std::string_view(properties_storage); + } else { + CHECK(properties_value->IsNullOrUndefined()); + } + + // OpenSSLStore is a global permission. URIs passed to STORE loaders can + // contain credentials, so they must not be exposed through permission + // errors or diagnostics. + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kOpenSSLStore, "", KeyObjectData()); + + std::optional> passphrase_content; + std::optional> passphrase; + if (IsAnyBufferSource(args[*offset + 3])) { + passphrase_content.emplace(args[*offset + 3]); + if (!passphrase_content->CheckSizeInt32()) [[unlikely]] { + THROW_ERR_OUT_OF_RANGE(env, "passphrase is too big"); + return {}; + } + passphrase = ncrypto::Buffer{ + .data = passphrase_content->data(), + .len = passphrase_content->size(), + }; + } else { + CHECK(args[*offset + 3]->IsNullOrUndefined()); + } + + *offset += 5; + EVPKeyPointer::StorePrivateKeyConfig config{ + .uri = uri.ToStringView(), + .properties = properties, + .passphrase = passphrase, + }; + auto res = EVPKeyPointer::TryLoadPrivateKeyFromStore(config); + if (res) { + return CreateAsymmetric(KeyType::kKeyTypePrivate, std::move(res.value)); + } + switch (res.error.value()) { + case EVPKeyPointer::PKParseError::NEED_PASSPHRASE: + ERR_clear_error(); + THROW_ERR_MISSING_PASSPHRASE(env, + "Passphrase required for encrypted key"); + break; + case EVPKeyPointer::PKParseError::NOT_RECOGNIZED: + ERR_clear_error(); + THROW_ERR_CRYPTO_OPERATION_FAILED( + env, "No private key found through the OpenSSL STORE loader"); + break; + default: { + static constexpr const char* msg = + "Failed to load private key through an OpenSSL STORE loader"; + // A loader may report a failure without leaving anything in the error + // queue, in which case ThrowCryptoError() would produce a bare Error + // carrying no code at all. + if (res.openssl_error.value_or(0) == 0) { + THROW_ERR_CRYPTO_OPERATION_FAILED(env, msg); + } else { + ThrowCryptoError(env, res.openssl_error.value(), msg); + } + break; + } + } + return {}; + } + + // Object formats: data is a JS Object (not buffer), format int determines + // whether this is a JWK or an OpenSSL STORE loader descriptor. if (args[*offset]->IsObject() && !IsAnyBufferSource(args[*offset]) && args[*offset + 1]->IsInt32()) { auto format = static_cast( @@ -818,7 +922,9 @@ KeyObjectData KeyObjectData::GetPrivateKeyFromJs( } KeyObjectData KeyObjectData::GetPublicOrPrivateKeyFromJs( - const FunctionCallbackInfo& args, unsigned int* offset) { + const FunctionCallbackInfo& args, + unsigned int* offset, + bool allow_private_key_store) { Environment* env = Environment::GetCurrent(args); // JWK format: data is a JS Object (not buffer), format int is JWK. @@ -831,6 +937,15 @@ KeyObjectData KeyObjectData::GetPublicOrPrivateKeyFromJs( *offset += 5; return data; } + if (format == EVPKeyPointer::PKFormatType::STORE) { + if (allow_private_key_store) { + return GetPrivateKeyFromJs(args, offset, false); + } + THROW_ERR_INVALID_ARG_VALUE( + env, + "URLs for OpenSSL STORE loaders are only accepted for private keys"); + return {}; + } } if (args[*offset]->IsString() || IsAnyBufferSource(args[*offset])) { @@ -1467,8 +1582,11 @@ void KeyObjectHandle::ExportECPublicRaw( } ECKeyPointer ec_key(m_pkey); - if (!ec_key) { - return THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env); + if (!ec_key) return THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env); + // A provider-backed key need not expose its public point. + if (ec_key.getPublicKey() == nullptr) { + return THROW_ERR_CRYPTO_OPERATION_FAILED(env, + "Failed to export EC public key"); } CHECK(args[0]->IsInt32()); @@ -1500,14 +1618,12 @@ void KeyObjectHandle::ExportECPrivateRaw( } ECKeyPointer ec_key(m_pkey); - if (!ec_key) { - return THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env); - } + if (!ec_key) return THROW_ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(env); const BIGNUM* private_key = ec_key.getPrivateKey(); if (private_key == nullptr) { return THROW_ERR_CRYPTO_OPERATION_FAILED(env, - "Failed to get EC private key"); + "Failed to export EC private key"); } const auto group = ec_key.getGroup(); @@ -1567,6 +1683,8 @@ void KeyObjectHandle::ExportJWK( if (ExportJWKInner(env, key->Data(), args[0], args[1]->IsTrue())) { args.GetReturnValue().Set(args[0]); + } else if (!env->isolate()->HasPendingException()) { + THROW_ERR_CRYPTO_OPERATION_FAILED(env, "Failed to export JWK"); } } @@ -2038,6 +2156,8 @@ void Initialize(Environment* env, Local target) { static_cast(EVPKeyPointer::PKFormatType::RAW_PRIVATE); constexpr int kKeyFormatRawSeed = static_cast(EVPKeyPointer::PKFormatType::RAW_SEED); + constexpr int kKeyFormatStore = + static_cast(EVPKeyPointer::PKFormatType::STORE); constexpr auto kSigEncDER = DSASigEnc::DER; constexpr auto kSigEncP1363 = DSASigEnc::P1363; @@ -2084,6 +2204,7 @@ void Initialize(Environment* env, Local target) { NODE_DEFINE_CONSTANT(target, kKeyFormatRawPublic); NODE_DEFINE_CONSTANT(target, kKeyFormatRawPrivate); NODE_DEFINE_CONSTANT(target, kKeyFormatRawSeed); + NODE_DEFINE_CONSTANT(target, kKeyFormatStore); NODE_DEFINE_CONSTANT(target, kKeyTypeSecret); NODE_DEFINE_CONSTANT(target, kKeyTypePublic); NODE_DEFINE_CONSTANT(target, kKeyTypePrivate); diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h index 9c06648a8a3f..b09756f719cf 100644 --- a/src/crypto/crypto_keys.h +++ b/src/crypto/crypto_keys.h @@ -77,7 +77,9 @@ class KeyObjectData final : public MemoryRetainer { bool allow_key_object); static KeyObjectData GetPublicOrPrivateKeyFromJs( - const v8::FunctionCallbackInfo& args, unsigned int* offset); + const v8::FunctionCallbackInfo& args, + unsigned int* offset, + bool allow_private_key_store = false); static v8::Maybe GetPrivateKeyEncodingFromJs(const v8::FunctionCallbackInfo& args, diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 7a272b16ffd2..5e09477a6913 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -8,6 +8,10 @@ #include "env-inl.h" #include "memory_tracker-inl.h" #include "openssl/ec.h" +#if NCRYPTO_USE_OPENSSL3_PROVIDER +#include +#include +#endif #include "threadpoolwork-inl.h" #include "v8.h" @@ -18,6 +22,7 @@ using ncrypto::ClearErrorOnReturn; using ncrypto::DataPointer; using ncrypto::Digest; using ncrypto::ECDSASigPointer; +using ncrypto::ECKeyPointer; using ncrypto::EVPKeyCtxPointer; using ncrypto::EVPKeyPointer; using ncrypto::EVPMDCtxPointer; @@ -390,6 +395,113 @@ bool SupportsContextString(const EVPKeyPointer& key) { #endif return false; } + +// Returns true unless the key is known not to be SM2, so that a key whose curve +// cannot be determined opts out of the prehashed fallback rather than into it. +bool MayBeSM2Key(const EVPKeyPointer& key) { +#ifdef OPENSSL_IS_BORINGSSL + return false; +#else + if (key.id() == EVP_PKEY_SM2) return true; + if (key.id() != EVP_PKEY_EC) return false; + +#if NCRYPTO_USE_OPENSSL3_PROVIDER + // An ECKeyPointer would also need the public point, which a provider-backed + // key need not expose. + char group_name[64]; + size_t group_name_len = 0; + if (EVP_PKEY_get_utf8_string_param(key.get(), + OSSL_PKEY_PARAM_GROUP_NAME, + group_name, + sizeof(group_name), + &group_name_len) != 1) { + return true; + } + return OBJ_sn2nid(group_name) == NID_sm2 || + EC_curve_nist2nid(group_name) == NID_sm2; +#else + ECKeyPointer ec(key); + if (!ec) return true; + + const EC_GROUP* group = ec.getGroup(); + if (group == nullptr) return true; + return EC_GROUP_get_curve_name(group) == NID_sm2; +#endif +#endif +} + +bool CanUsePrehashedFallback(const EVPKeyPointer& key, + const Digest& digest, + bool has_context) { + if (!digest || has_context) return false; + + if (key.isRsaVariant()) return true; + + // SM2 digest signing first hashes the algorithm-specific Z value, so the + // lower-level prehashed sign/verify operation is not equivalent. + return key.isSigVariant() && !MayBeSM2Key(key); +} + +ByteSource SignPrehashed(Environment* env, + const EVPKeyPointer& key, + const Digest& digest, + const ByteSource& input, + int padding, + std::optional salt_length, + DSASigEnc dsa_encoding) { + EVPMDCtxPointer context = EVPMDCtxPointer::New(); + if (!context || !context.digestInit(digest) || !context.digestUpdate(input)) + [[unlikely]] { + return {}; + } + + auto data = context.digestFinal(context.getExpectedSize()); + if (!data) [[unlikely]] { + return {}; + } + + EVPKeyCtxPointer pkctx = key.newCtx(); + if (!pkctx || pkctx.initForSign() <= 0 || + !ApplyRSAOptions(key, pkctx.get(), padding, salt_length) || + !pkctx.setSignatureMd(context)) [[unlikely]] { + return {}; + } + + auto signature = pkctx.sign(data); + if (!signature) [[unlikely]] { + return {}; + } + + DCHECK(!signature.isSecure()); + auto out = ByteSource::Allocated(signature.release()); + if (UseP1363Encoding(key, dsa_encoding)) { + return ConvertSignatureToP1363(env, key, std::move(out)); + } + return out; +} + +bool VerifyPrehashed(const EVPKeyPointer& key, + const Digest& digest, + const ByteSource& input, + const ByteSource& signature, + int padding, + std::optional salt_length) { + EVPMDCtxPointer context = EVPMDCtxPointer::New(); + if (!context || !context.digestInit(digest) || !context.digestUpdate(input)) + [[unlikely]] { + return false; + } + + auto data = context.digestFinal(context.getExpectedSize()); + if (!data) [[unlikely]] { + return false; + } + + EVPKeyCtxPointer pkctx = key.newCtx(); + return pkctx && pkctx.initForVerify() > 0 && + ApplyRSAOptions(key, pkctx.get(), padding, salt_length) && + pkctx.setSignatureMd(context) && pkctx.verify(signature, data); +} } // namespace SignBase::Error SignBase::Init(const char* digest) { @@ -807,9 +919,6 @@ bool SignTraits::DeriveBits(Environment* env, ByteSource* out, CryptoJobMode mode, CryptoErrorStore* errors) { - auto context = EVPMDCtxPointer::New(); - if (!context) [[unlikely]] - return false; const auto& key = params.key.GetAsymmetricKey(); bool has_context = (params.flags & SignConfiguration::kHasContextString && @@ -821,6 +930,19 @@ bool SignTraits::DeriveBits(Environment* env, return false; } + int padding = params.flags & SignConfiguration::kHasPadding + ? params.padding + : key.getDefaultSignPadding(); + + std::optional salt_length = + params.flags & SignConfiguration::kHasSaltLength + ? std::optional(params.salt_length) + : std::nullopt; + + auto context = EVPMDCtxPointer::New(); + if (!context) [[unlikely]] + return false; + auto ctx = ([&] { if (has_context) { ncrypto::Buffer context_buf{ @@ -849,15 +971,6 @@ bool SignTraits::DeriveBits(Environment* env, return false; } - int padding = params.flags & SignConfiguration::kHasPadding - ? params.padding - : key.getDefaultSignPadding(); - - std::optional salt_length = - params.flags & SignConfiguration::kHasSaltLength - ? std::optional(params.salt_length) - : std::nullopt; - if (!ApplyRSAOptions(key, *ctx, padding, salt_length)) { return false; } @@ -873,6 +986,19 @@ bool SignTraits::DeriveBits(Environment* env, *out = ByteSource::Allocated(data.release()); } else { auto data = context.sign(params.data); + // Only evaluated on the failure path: CanUsePrehashedFallback() has to + // reconstruct EC key material to detect SM2, which is far too + // expensive to pay for on every successful sign. + if (!data && CanUsePrehashedFallback(key, params.digest, has_context)) { + *out = SignPrehashed(env, + key, + params.digest, + params.data, + padding, + salt_length, + params.dsa_encoding); + return static_cast(*out); + } if (!data) [[unlikely]] { return false; } @@ -890,9 +1016,24 @@ bool SignTraits::DeriveBits(Environment* env, case SignConfiguration::Mode::Verify: { auto buf = DataPointer::Alloc(1); static_cast(buf.get())[0] = 0; - if (context.verify(params.data, params.signature) && + // EVP_DigestVerify() documents 0 as a verification mismatch. In its + // Update/Final path, it maps a failed EVP_DigestVerifyUpdate() to -1. + // Some providers fail that combined operation but support raw + // verification of a precomputed digest, so only retry negative results. + // Retrying 0 would perform a second verification for every mismatch. + int verify_result = context.verifyOneShot(params.data, params.signature); + if (verify_result == 1 && !HasSmallOrderEdDsaPoint(key, params.signature)) { static_cast(buf.get())[0] = 1; + } else if (verify_result < 0 && + CanUsePrehashedFallback(key, params.digest, has_context) && + VerifyPrehashed(key, + params.digest, + params.data, + params.signature, + padding, + salt_length)) { + static_cast(buf.get())[0] = 1; } *out = ByteSource::Allocated(buf.release()); } diff --git a/src/env.cc b/src/env.cc index d6a859e7a354..87340112fbeb 100644 --- a/src/env.cc +++ b/src/env.cc @@ -939,6 +939,10 @@ Environment::Environment(IsolateData* isolate_data, if (!options_->allow_ffi) { permission()->Apply(this, {"*"}, permission::PermissionScope::kFFI); } + if (!options_->allow_openssl_store) { + permission()->Apply( + this, {"*"}, permission::PermissionScope::kOpenSSLStore); + } if (!options_->allow_worker_threads) { permission()->Apply( this, {"*"}, permission::PermissionScope::kWorkerThreads); diff --git a/src/node_options.cc b/src/node_options.cc index 383522863bb8..b9ed1b0c4d64 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -730,6 +730,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { kAllowedInEnvvar, false, OptionNamespaces::kPermissionNamespace); + AddOption("--allow-openssl-store", + "allow use of OpenSSL STORE loaders when any permissions are set", + &EnvironmentOptions::allow_openssl_store, + kAllowedInEnvvar, + false, + OptionNamespaces::kPermissionNamespace); AddOption("--experimental-vm-modules", "experimental ES Module support in vm module", &EnvironmentOptions::experimental_vm_modules, diff --git a/src/node_options.h b/src/node_options.h index fc758f2444aa..17faf4868bc1 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -153,6 +153,7 @@ class EnvironmentOptions : public Options { bool allow_net = false; bool allow_wasi = false; bool allow_ffi = false; + bool allow_openssl_store = false; bool allow_worker_threads = false; bool experimental_vm_modules = EXPERIMENTALS_DEFAULT_VALUE; bool async_context_frame = true; diff --git a/src/permission/openssl_store_permission.cc b/src/permission/openssl_store_permission.cc new file mode 100644 index 000000000000..fcae2772b3f5 --- /dev/null +++ b/src/permission/openssl_store_permission.cc @@ -0,0 +1,31 @@ +#include "permission/openssl_store_permission.h" + +#include +#include + +namespace node { + +namespace permission { + +// OpenSSLStorePermission manages a single global deny state for the use of +// OpenSSL STORE loaders. +void OpenSSLStorePermission::Apply(Environment* env, + const std::vector& allow, + PermissionScope scope) { + deny_all_ = true; +} + +void OpenSSLStorePermission::Drop(Environment* env, + PermissionScope scope, + const std::string_view& param) { + deny_all_ = true; +} + +bool OpenSSLStorePermission::is_granted(Environment* env, + PermissionScope perm, + const std::string_view& param) const { + return perm != PermissionScope::kOpenSSLStore || !deny_all_; +} + +} // namespace permission +} // namespace node diff --git a/src/permission/openssl_store_permission.h b/src/permission/openssl_store_permission.h new file mode 100644 index 000000000000..d64475228e18 --- /dev/null +++ b/src/permission/openssl_store_permission.h @@ -0,0 +1,34 @@ +#ifndef SRC_PERMISSION_OPENSSL_STORE_PERMISSION_H_ +#define SRC_PERMISSION_OPENSSL_STORE_PERMISSION_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include +#include "permission/permission_base.h" + +namespace node { + +namespace permission { + +class OpenSSLStorePermission final : public PermissionBase { + public: + void Apply(Environment* env, + const std::vector& allow, + PermissionScope scope) override; + void Drop(Environment* env, + PermissionScope scope, + const std::string_view& param = "") override; + bool is_granted(Environment* env, + PermissionScope perm, + const std::string_view& param = "") const override; + + private: + bool deny_all_ = false; +}; + +} // namespace permission + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#endif // SRC_PERMISSION_OPENSSL_STORE_PERMISSION_H_ diff --git a/src/permission/permission.cc b/src/permission/permission.cc index c593502d94eb..cf1174d85fa1 100644 --- a/src/permission/permission.cc +++ b/src/permission/permission.cc @@ -48,6 +48,8 @@ constexpr std::string_view GetDiagnosticsChannelName(PermissionScope scope) { return "node:permission-model:addon"; case PermissionScope::kFFI: return "node:permission-model:ffi"; + case PermissionScope::kOpenSSLStore: + return "node:permission-model:openssl-store"; default: return {}; } @@ -131,6 +133,8 @@ Permission::Permission() : enabled_(false), warning_only_(false) { std::shared_ptr net = std::make_shared(); std::shared_ptr addon = std::make_shared(); std::shared_ptr ffi = std::make_shared(); + std::shared_ptr openssl_store = + std::make_shared(); #define V(Name, _, __, ___) \ nodes_.insert(std::make_pair(PermissionScope::k##Name, fs)); FILESYSTEM_PERMISSIONS(V) @@ -163,6 +167,10 @@ Permission::Permission() : enabled_(false), warning_only_(false) { nodes_.insert(std::make_pair(PermissionScope::k##Name, ffi)); FFI_PERMISSIONS(V) #undef V +#define V(Name, _, __, ___) \ + nodes_.insert(std::make_pair(PermissionScope::k##Name, openssl_store)); + OPENSSL_STORE_PERMISSIONS(V) +#undef V } const char* GetErrorFlagSuggestion(node::permission::PermissionScope perm) { diff --git a/src/permission/permission.h b/src/permission/permission.h index 6a080fbe73c4..5597e5ce2445 100644 --- a/src/permission/permission.h +++ b/src/permission/permission.h @@ -12,6 +12,7 @@ #include "permission/fs_permission.h" #include "permission/inspector_permission.h" #include "permission/net_permission.h" +#include "permission/openssl_store_permission.h" #include "permission/permission_base.h" #include "permission/wasi_permission.h" #include "permission/worker_permission.h" diff --git a/src/permission/permission_base.h b/src/permission/permission_base.h index 11f4d6c6bfa6..66bfd33a8787 100644 --- a/src/permission/permission_base.h +++ b/src/permission/permission_base.h @@ -37,6 +37,9 @@ namespace permission { #define FFI_PERMISSIONS(V) V(FFI, "ffi", PermissionsRoot, "--allow-ffi") +#define OPENSSL_STORE_PERMISSIONS(V) \ + V(OpenSSLStore, "openssl.store", PermissionsRoot, "--allow-openssl-store") + #define PERMISSIONS(V) \ FILESYSTEM_PERMISSIONS(V) \ CHILD_PROCESS_PERMISSIONS(V) \ @@ -45,7 +48,8 @@ namespace permission { INSPECTOR_PERMISSIONS(V) \ NET_PERMISSIONS(V) \ ADDON_PERMISSIONS(V) \ - FFI_PERMISSIONS(V) + FFI_PERMISSIONS(V) \ + OPENSSL_STORE_PERMISSIONS(V) #define V(name, _, __, ___) k##name, enum class PermissionScope { diff --git a/test/parallel/test-crypto-key-store-pkcs11.js b/test/parallel/test-crypto-key-store-pkcs11.js new file mode 100644 index 000000000000..971f4909166b --- /dev/null +++ b/test/parallel/test-crypto-key-store-pkcs11.js @@ -0,0 +1,585 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const { hasOpenSSL } = require('../common/crypto'); +if (!hasOpenSSL(3, 0)) + common.skip('requires OpenSSL 3.x'); + +// The PKCS#11 token, the OpenSSL configuration that activates a provider for +// it, and the PIN that unlocks it are all provided by the environment. See +// tools/nix/pkcs11.nix for the fixture this repository ships, which `shell.nix` +// exports when instantiated with `--arg pkcs11 true`. +const kOpenSSLConfig = process.env.NODE_TEST_PKCS11_OPENSSL_CONF; +const kPin = process.env.NODE_TEST_PKCS11_PIN; +if (!kOpenSSLConfig || !kPin) + common.skip('missing a PKCS#11 provider test fixture'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const { + constants: { + RSA_PKCS1_PSS_PADDING, + }, + createPublicKey, + createPrivateKey, + createSign, + createVerify, + diffieHellman, + generateKeyPairSync, + sign, + verify, +} = require('crypto'); +const tmpdir = require('../common/tmpdir'); + +const { subtle } = globalThis.crypto; +const kData = Buffer.from( + Array.from({ length: 256 }, (_, i) => (i * 17 + 43) & 0xff)); +const kProperties = 'provider=pkcs11'; +const kExpectedPrivateExportFailure = + /Failed to encode private key|Failed to export JWK|Failed to export RSA private key|Failed to export EC .* key|Failed to get raw .* key|keymgmt export failure|not exportable|operation not supported|not supported|incompatible/i; + +// tools/nix/pkcs11.nix ships a SoftHSM directory holding the token and the +// configuration naming it. SoftHSM opens its token read-write, which a Nix +// store path can never be, so run from a writable copy of that directory; the +// configuration names the token relative to the working directory. A fixture +// configured by hand, a real HSM for instance, sets no directory and is used +// as it stands. +function softhsmOptions() { + const source = process.env.NODE_TEST_PKCS11_SOFTHSM_DIR; + if (!source) return {}; + + tmpdir.refresh(); + const cwd = tmpdir.resolve('softhsm'); + fs.cpSync(source, cwd, { recursive: true }); + fs.chmodSync(cwd, 0o700); + for (const entry of fs.readdirSync(cwd, { recursive: true })) { + fs.chmodSync(path.join(cwd, entry), 0o700); + } + + return { cwd, env: { SOFTHSM2_CONF: path.join(cwd, 'softhsm2.conf') } }; +} + +function runInChild() { + const { cwd, env } = softhsmOptions(); + const child = spawnSync(process.execPath, [ + `--openssl-config=${kOpenSSLConfig}`, + __filename, + ], { + cwd, + env: { ...process.env, ...env, NODE_TEST_PKCS11_CHILD: '1' }, + stdio: 'inherit', + }); + assert.strictEqual(child.status, 0); +} + +function privateKeyUrl(label) { + return new URL(`pkcs11:object=${label};type=private`); +} + +function loadPrivateKey(label) { + return createPrivateKey({ + key: privateKeyUrl(label), + passphrase: kPin, + properties: kProperties, + }); +} + +function assertKeyDetails(key, type, asymmetricKeyType) { + assert.strictEqual(key.type, type); + assert.strictEqual(key.asymmetricKeyType, asymmetricKeyType); + + switch (asymmetricKeyType) { + case 'rsa': + assert.strictEqual(key.asymmetricKeyDetails.modulusLength, 2048); + assert.strictEqual(key.asymmetricKeyDetails.publicExponent, 65537n); + break; + case 'ec': + assert.strictEqual(key.asymmetricKeyDetails.namedCurve, 'prime256v1'); + break; + case 'ed25519': + case 'ed448': + assert.deepStrictEqual(key.asymmetricKeyDetails, {}); + break; + default: + assert.fail(`unexpected asymmetric key type ${asymmetricKeyType}`); + } +} + +function assertDerivedPublicKey(privateKey, asymmetricKeyType) { + const publicKey = createPublicKey(privateKey); + assertKeyDetails(publicKey, 'public', asymmetricKeyType); + return publicKey; +} + +function assertPublicExports(publicKey) { + const spkiPem = publicKey.export({ format: 'pem', type: 'spki' }); + assert.strictEqual( + spkiPem.split('\n')[0], + '-----BEGIN PUBLIC KEY-----'); + + const spkiDer = publicKey.export({ format: 'der', type: 'spki' }); + assert(Buffer.isBuffer(spkiDer)); + assert(spkiDer.byteLength > 0); + + // The PEM must carry the same SubjectPublicKeyInfo the DER export produces. + // Encoding a provider-backed key through OpenSSL's PEM_write_bio_PUBKEY() + // yields a PKCS#1 body for RSA, which the label alone does not catch. + const pemBody = Buffer.from( + spkiPem.split('\n').filter((line) => !line.startsWith('---')).join(''), + 'base64'); + assert.deepStrictEqual(pemBody, spkiDer); +} + +function assertPrivateExportsRejected(privateKey, asymmetricKeyType) { + const specs = [ + { format: 'pem', type: 'pkcs8' }, + { format: 'der', type: 'pkcs8' }, + { format: 'jwk' }, + ]; + + switch (asymmetricKeyType) { + case 'rsa': + specs.push( + { format: 'pem', type: 'pkcs1' }, + { format: 'der', type: 'pkcs1' }); + break; + case 'ec': + specs.push( + { format: 'pem', type: 'sec1' }, + { format: 'der', type: 'sec1' }, + { format: 'raw-private' }); + break; + default: + specs.push({ format: 'raw-private' }); + } + + for (const options of specs) { + assert.throws(() => { + privateKey.export(options); + }, { + message: kExpectedPrivateExportFailure, + }); + } +} + +function assertOneShotSignVerify(digest, data, privateKey, options = {}) { + const publicKey = createPublicKey(privateKey); + const signKey = { key: privateKey, ...options }; + const verifyPublicKey = { key: publicKey, ...options }; + const verifyPrivateKey = { key: privateKey, ...options }; + + const signature = sign(digest, data, signKey); + assert(signature.byteLength > 0); + assert.strictEqual(verify(digest, data, verifyPublicKey, signature), true); + assert.strictEqual(verify(digest, data, verifyPrivateKey, signature), true); + + return signature; +} + +function assertStreamingSignOneShotVerify(digest, data, privateKey) { + const publicKey = createPublicKey(privateKey); + const signature = createSign(digest).update(data).sign(privateKey); + assert(signature.byteLength > 0); + assert.strictEqual(verify(digest, data, publicKey, signature), true); + assert.strictEqual(verify(digest, data, privateKey, signature), true); + + assert.strictEqual( + createVerify(digest).update(data).verify(publicKey, signature), + true); + assert.strictEqual( + createVerify(digest).update(data).verify(privateKey, signature), + true); +} + +// The one-shot sign and verify callbacks run the operation on the threadpool +// rather than on the main thread. PKCS#11 sessions are shared process-wide, so +// exercise that path explicitly instead of only the synchronous one. +async function assertAsyncSignVerify(digest, data, privateKey) { + const publicKey = createPublicKey(privateKey); + + const signature = await new Promise((resolve, reject) => { + sign(digest, data, privateKey, (err, sig) => { + if (err) reject(err); + else resolve(sig); + }); + }); + assert(signature.byteLength > 0); + + for (const key of [publicKey, privateKey]) { + assert.strictEqual(await new Promise((resolve, reject) => { + verify(digest, data, key, signature, (err, result) => { + if (err) reject(err); + else resolve(result); + }); + }), true); + } +} + +// A store-backed key and a second load of the same URI are the same key. +function assertKeyObjectEquality(privateKey, label) { + assert.strictEqual(privateKey.equals(loadPrivateKey(label)), true); + assert.strictEqual(privateKey.equals(loadPrivateKey('node-ec')), + label === 'node-ec'); +} + +function assertEcdh(privateKey) { + const peer = generateKeyPairSync('ec', { namedCurve: 'prime256v1' }); + + // The peer public key has to reach OpenSSL through the SPKI decoder, which + // is what happens in practice because a peer key arrives over the wire. A + // public KeyObject that the default provider produced directly, including + // createPublicKey() of this very key, is rejected by the PKCS#11 provider + // with CKR_ARGUMENTS_BAD even though it is byte-for-byte the same key. + const importSpki = (key) => createPublicKey({ + key: key.export({ format: 'der', type: 'spki' }), + format: 'der', + type: 'spki', + }); + + const publicKey = createPublicKey(privateKey); + const ours = diffieHellman({ + privateKey, + publicKey: importSpki(peer.publicKey), + }); + const theirs = diffieHellman({ + privateKey: peer.privateKey, + publicKey: importSpki(publicKey), + }); + + assert(ours.byteLength > 0); + assert.deepStrictEqual(ours, theirs); +} + +async function assertWebCryptoSignVerify( + privateKey, + publicKey, + algorithm, + privateUsages, + publicUsages, + signAlgorithm = algorithm.name, +) { + const privateCryptoKey = privateKey.toCryptoKey( + algorithm, + false, + privateUsages); + assert.strictEqual(privateCryptoKey.type, 'private'); + assert.strictEqual(privateCryptoKey.extractable, false); + assert.deepStrictEqual(privateCryptoKey.usages, privateUsages); + + await assert.rejects( + subtle.exportKey('pkcs8', privateCryptoKey), + { + name: 'InvalidAccessError', + message: /not extractable/i, + }); + + const publicCryptoKey = publicKey.toCryptoKey( + algorithm, + true, + publicUsages); + assert.strictEqual(publicCryptoKey.type, 'public'); + assert.strictEqual(publicCryptoKey.extractable, true); + assert.deepStrictEqual(publicCryptoKey.usages, publicUsages); + + const signature = await subtle.sign( + signAlgorithm, + privateCryptoKey, + kData); + assert(signature instanceof ArrayBuffer); + assert(signature.byteLength > 0); + assert.strictEqual( + await subtle.verify( + signAlgorithm, + publicCryptoKey, + signature, + kData), + true); + + try { + const exportedPublicKey = await subtle.exportKey('spki', publicCryptoKey); + assert(exportedPublicKey instanceof ArrayBuffer); + assert(exportedPublicKey.byteLength > 0); + } catch (err) { + assert.strictEqual(err.name, 'OperationError'); + assert.match(err.message, /operation-specific reason|not supported/i); + } +} + +async function assertPrivateCryptoKeyExportsRejected( + privateKey, + algorithm, + privateUsages, +) { + const privateCryptoKey = privateKey.toCryptoKey( + algorithm, + true, + privateUsages); + assert.strictEqual(privateCryptoKey.type, 'private'); + assert.strictEqual(privateCryptoKey.extractable, true); + assert.deepStrictEqual(privateCryptoKey.usages, privateUsages); + + for (const format of ['pkcs8', 'jwk']) { + await assert.rejects( + subtle.exportKey(format, privateCryptoKey), + (err) => { + assert(err.name === 'OperationError' || + err.code === 'ERR_CRYPTO_OPERATION_FAILED'); + assert.match(err.cause?.message ?? err.message, + kExpectedPrivateExportFailure); + return true; + }); + } +} + +function assertStoreOptions() { + assert.strictEqual( + createPrivateKey({ + key: privateKeyUrl('node-rsa'), + passphrase: kPin, + }).asymmetricKeyType, + 'rsa'); + + assert.strictEqual( + createPrivateKey({ + key: privateKeyUrl('node-rsa'), + passphrase: kPin, + properties: kProperties, + }).asymmetricKeyType, + 'rsa'); +} + +function assertChild(args, expectedStatus, stderrPattern, options = {}) { + const child = spawnSync(process.execPath, args, { + env: process.env, + encoding: 'utf8', + ...options, + }); + assert.strictEqual(child.signal, null); + assert.strictEqual(child.status, expectedStatus, child.stderr || child.stdout); + if (stderrPattern) assert.match(child.stderr, stderrPattern); +} + +function assertStoreLoadFailure(code, stderrPattern, options) { + assertChild([ + `--openssl-config=${kOpenSSLConfig}`, + '-e', + code, + ], 1, stderrPattern, options); +} + +function assertPassphraseHandling() { + // When no passphrase is supplied the provider falls back to prompting for a + // PIN through OpenSSL's default UI, which opens the terminal directly + // (/dev/tty, or "con" on Windows) rather than reading stdin. Detaching gives + // the child no controlling terminal, so the prompt cannot block. The result + // is the same either way, because Node has already recorded that no + // passphrase was available. + assertStoreLoadFailure(` + require('crypto').createPrivateKey({ + key: new URL('pkcs11:object=node-rsa;type=private'), + properties: ${JSON.stringify(kProperties)}, + }); + `, /ERR_MISSING_PASSPHRASE/, { detached: true }); + + assertStoreLoadFailure(` + require('crypto').createPrivateKey({ + key: new URL('pkcs11:object=node-rsa;type=private'), + passphrase: 'bad', + properties: ${JSON.stringify(kProperties)}, + }); + `, /Failed to load private key through an OpenSSL STORE loader/); +} + +function assertBadProperties() { + assertStoreLoadFailure(` + require('crypto').createPrivateKey({ + key: new URL('pkcs11:object=node-rsa;type=private'), + passphrase: ${JSON.stringify(kPin)}, + properties: 'provider=default', + }); + `, /Failed to load private key through an OpenSSL STORE loader|No such file or directory|unsupported/i); +} + +function assertPermissionModel() { + const code = ` + require('crypto').createPrivateKey({ + key: new URL('pkcs11:object=node-rsa;type=private'), + passphrase: ${JSON.stringify(kPin)}, + properties: ${JSON.stringify(kProperties)}, + }); + `; + + assertChild([ + `--openssl-config=${kOpenSSLConfig}`, + '--permission', + '--allow-fs-read=*', + '-e', + code, + ], 1, /ERR_ACCESS_DENIED/); + + assertChild([ + `--openssl-config=${kOpenSSLConfig}`, + '--permission', + '--allow-openssl-store', + '--allow-fs-read=*', + '-e', + code, + ], 0); +} + +function assertInlineSignWithStoreUrl(privateKey) { + const publicKey = createPublicKey(privateKey); + const signature = sign('sha256', kData, { + key: privateKeyUrl('node-rsa'), + passphrase: kPin, + properties: kProperties, + }); + assert(signature.byteLength > 0); + assert.strictEqual(verify('sha256', kData, publicKey, signature), true); +} + +async function testRsa() { + const privateKey = loadPrivateKey('node-rsa'); + assertKeyDetails(privateKey, 'private', 'rsa'); + + const publicKey = assertDerivedPublicKey(privateKey, 'rsa'); + assertOneShotSignVerify('sha256', kData, privateKey); + assertOneShotSignVerify('sha256', kData, privateKey, { + padding: RSA_PKCS1_PSS_PADDING, + saltLength: 32, + }); + assertStreamingSignOneShotVerify('sha256', kData, privateKey); + await assertAsyncSignVerify('sha256', kData, privateKey); + assertKeyObjectEquality(privateKey, 'node-rsa'); + assertInlineSignWithStoreUrl(privateKey); + assertPublicExports(publicKey); + assertPrivateExportsRejected(privateKey, 'rsa'); + await assertPrivateCryptoKeyExportsRejected( + privateKey, + { name: 'RSASSA-PKCS1-v1_5', hash: 'SHA-256' }, + ['sign']); + + await assertWebCryptoSignVerify( + privateKey, + publicKey, + { name: 'RSASSA-PKCS1-v1_5', hash: 'SHA-256' }, + ['sign'], + ['verify']); + + await assertWebCryptoSignVerify( + privateKey, + publicKey, + { name: 'RSA-PSS', hash: 'SHA-256' }, + ['sign'], + ['verify'], + { name: 'RSA-PSS', saltLength: 32 }); +} + +async function testEc() { + const privateKey = loadPrivateKey('node-ec'); + assertKeyDetails(privateKey, 'private', 'ec'); + + const publicKey = assertDerivedPublicKey(privateKey, 'ec'); + assertOneShotSignVerify('sha256', kData, privateKey); + assertOneShotSignVerify('sha256', kData, privateKey, { + dsaEncoding: 'ieee-p1363', + }); + assertStreamingSignOneShotVerify('sha256', kData, privateKey); + await assertAsyncSignVerify('sha256', kData, privateKey); + assertKeyObjectEquality(privateKey, 'node-ec'); + + assertPublicExports(publicKey); + assertPrivateExportsRejected(privateKey, 'ec'); + await assertPrivateCryptoKeyExportsRejected( + privateKey, + { name: 'ECDSA', namedCurve: 'P-256' }, + ['sign']); + await assertWebCryptoSignVerify( + privateKey, + publicKey, + { name: 'ECDSA', namedCurve: 'P-256' }, + ['sign'], + ['verify'], + { name: 'ECDSA', hash: 'SHA-256' }); +} + +async function testEd25519() { + const privateKey = loadPrivateKey('node-ed25519'); + assertKeyDetails(privateKey, 'private', 'ed25519'); + + const publicKey = assertDerivedPublicKey(privateKey, 'ed25519'); + assertOneShotSignVerify(null, kData, privateKey); + await assertAsyncSignVerify(null, kData, privateKey); + assertPublicExports(publicKey); + assertPrivateExportsRejected(privateKey, 'ed25519'); + await assertPrivateCryptoKeyExportsRejected( + privateKey, + { name: 'Ed25519' }, + ['sign']); + + await assertWebCryptoSignVerify( + privateKey, + publicKey, + { name: 'Ed25519' }, + ['sign'], + ['verify']); +} + +function testEcDiffieHellman() { + const privateKey = loadPrivateKey('node-ecdh'); + assertKeyDetails(privateKey, 'private', 'ec'); + + const publicKey = assertDerivedPublicKey(privateKey, 'ec'); + assertPublicExports(publicKey); + assertPrivateExportsRejected(privateKey, 'ec'); + assertEcdh(privateKey); +} + +async function testEd448() { + const privateKey = loadPrivateKey('node-ed448'); + assertKeyDetails(privateKey, 'private', 'ed448'); + + const publicKey = assertDerivedPublicKey(privateKey, 'ed448'); + assertOneShotSignVerify(null, kData, privateKey); + await assertAsyncSignVerify(null, kData, privateKey); + assertPublicExports(publicKey); + assertPrivateExportsRejected(privateKey, 'ed448'); + await assertPrivateCryptoKeyExportsRejected( + privateKey, + { name: 'Ed448' }, + ['sign']); + + await assertWebCryptoSignVerify( + privateKey, + publicKey, + { name: 'Ed448' }, + ['sign'], + ['verify']); +} + +async function runTest() { + assertStoreOptions(); + assertPassphraseHandling(); + assertBadProperties(); + assertPermissionModel(); + + await testRsa(); + await testEc(); + testEcDiffieHellman(); + await testEd25519(); + await testEd448(); +} + +if (process.env.NODE_TEST_PKCS11_CHILD === '1') { + runTest().then(common.mustCall()).catch((err) => { + process.nextTick(() => { + throw err; + }); + }); +} else { + runInChild(); +} diff --git a/test/parallel/test-crypto-key-store.js b/test/parallel/test-crypto-key-store.js new file mode 100644 index 000000000000..72826afaaeff --- /dev/null +++ b/test/parallel/test-crypto-key-store.js @@ -0,0 +1,303 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const { hasOpenSSL } = require('../common/crypto'); +if (!hasOpenSSL(3)) + common.skip('requires OpenSSL 3.x'); + +// Verifies that crypto.createPrivateKey() can pass a WHATWG URL (here a file: +// URI) to an OpenSSL STORE loader, and that the resulting KeyObject works for +// signing and verification. + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const { pathToFileURL } = require('url'); +const { + createPublicKey, + createPrivateKey, + createVerify, + decapsulate, + diffieHellman, + encapsulate, + generateKeyPairSync, + privateDecrypt, + privateEncrypt, + publicDecrypt, + publicEncrypt, + sign, + verify, +} = require('crypto'); +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const data = Buffer.from('hello store'); + +{ + const { privateKey, publicKey } = generateKeyPairSync('ed25519'); + const file = path.join(tmpdir.path, 'priv.pem'); + fs.writeFileSync(file, privateKey.export({ format: 'pem', type: 'pkcs8' })); + const url = pathToFileURL(file); + + const pk = createPrivateKey(url); + assert.strictEqual(pk.type, 'private'); + assert.strictEqual(pk.asymmetricKeyType, 'ed25519'); + + const sig = sign(null, data, pk); + assert.strictEqual(verify(null, data, publicKey, sig), true); + + const pkWithProperties = createPrivateKey({ key: url, properties: '' }); + assert.strictEqual(pkWithProperties.type, 'private'); + assert.strictEqual(pkWithProperties.asymmetricKeyType, 'ed25519'); + assert.strictEqual( + verify(null, data, publicKey, sign(null, data, pkWithProperties)), + true); + + // Passing the URL inline to sign() behaves like createPrivateKey(). + assert.strictEqual(verify(null, data, publicKey, sign(null, data, url)), true); + + assert.throws(() => createPrivateKey({ key: url, properties: 1 }), { + code: 'ERR_INVALID_ARG_TYPE', + }); +} + +{ + const { privateKey, publicKey } = generateKeyPairSync('rsa', { + modulusLength: 2048, + }); + const file = path.join(tmpdir.path, 'rsa.pem'); + fs.writeFileSync(file, privateKey.export({ format: 'pem', type: 'pkcs8' })); + const url = pathToFileURL(file); + const plaintext = Buffer.from('hello rsa store'); + + const ciphertext = publicEncrypt(publicKey, plaintext); + assert.deepStrictEqual(privateDecrypt(url, ciphertext), plaintext); + assert.deepStrictEqual(privateDecrypt({ key: url }, ciphertext), plaintext); + + const encrypted = privateEncrypt(url, plaintext); + assert.deepStrictEqual(publicDecrypt(publicKey, encrypted), plaintext); + + const encryptedFromObject = privateEncrypt({ key: url }, plaintext); + assert.deepStrictEqual(publicDecrypt(publicKey, encryptedFromObject), + plaintext); +} + +{ + const alice = generateKeyPairSync('x25519'); + const bob = generateKeyPairSync('x25519'); + const file = path.join(tmpdir.path, 'x25519.pem'); + fs.writeFileSync(file, alice.privateKey.export({ + format: 'pem', + type: 'pkcs8', + })); + const url = pathToFileURL(file); + + const expected = diffieHellman({ + privateKey: alice.privateKey, + publicKey: bob.publicKey, + }); + assert.deepStrictEqual( + diffieHellman({ privateKey: url, publicKey: bob.publicKey }), + expected); + + if (hasOpenSSL(3, 2)) { + const { sharedKey, ciphertext } = encapsulate(alice.publicKey); + assert.deepStrictEqual(decapsulate(url, ciphertext), sharedKey); + } +} + +{ + // Encrypted PKCS#8 with passphrase via { key: url, passphrase }. + const { privateKey, publicKey } = generateKeyPairSync('ed25519'); + const file = path.join(tmpdir.path, 'enc.pem'); + fs.writeFileSync(file, privateKey.export({ + format: 'pem', type: 'pkcs8', cipher: 'aes-256-cbc', passphrase: 'pw', + })); + const url = pathToFileURL(file); + + const sig = sign(null, data, { key: url, passphrase: Buffer.from('pw') }); + assert.strictEqual(verify(null, data, publicKey, sig), true); + + assert.throws(() => createPrivateKey(url), { + code: 'ERR_MISSING_PASSPHRASE', + }); + + assert.throws(() => createPrivateKey({ key: url, passphrase: 'bad' }), + common.expectsError({ + name: 'Error', + code: /^ERR_OSSL_/, + })); +} + +{ + // A URL is only accepted in private-key contexts. + const url = pathToFileURL(path.join(tmpdir.path, 'priv.pem')); + assert.throws(() => createPublicKey(url), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => createPublicKey({ key: url }), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => publicEncrypt(url, data), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => publicEncrypt({ key: url }, data), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => publicDecrypt(url, Buffer.alloc(0)), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => verify(null, data, url, Buffer.alloc(0)), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => verify(null, data, { key: url }, Buffer.alloc(0)), { + code: 'ERR_INVALID_ARG_TYPE', + }); + const verifier = createVerify('sha256'); + verifier.update(data); + assert.throws(() => verifier.verify(url, Buffer.alloc(0)), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => encapsulate(url), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => encapsulate({ key: url }), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws(() => diffieHellman({ + privateKey: createPrivateKey(url), + publicKey: url, + }), { + code: 'ERR_INVALID_ARG_TYPE', + }); + + assert.throws(() => createPrivateKey(1), { + code: 'ERR_INVALID_ARG_TYPE', + message: /URL/, + }); +} + +{ + // A readable URI that holds no private key is distinguishable from a URI + // that could not be opened at all. + const file = path.join(tmpdir.path, 'nope.pem'); + fs.writeFileSync(file, 'not a key'); + assert.throws(() => createPrivateKey(pathToFileURL(file)), { + code: 'ERR_CRYPTO_OPERATION_FAILED', + }); + + // OpenSSL has no reason string for system library errors, so error.code + // cannot be derived for these. Which entry becomes the message is not + // portable either: some builds leave a generic STORE `unsupported` on top of + // the error the loader itself raised. The reason is reported either way. + assert.throws( + () => createPrivateKey(pathToFileURL(path.join(tmpdir.path, 'missing.pem'))), + (err) => { + assert.match([err.message, ...err.opensslErrorStack ?? []].join('\n'), + /No such file or directory/); + return true; + }); +} + +{ + // Failures report the loader that actually handled the URI, not the `file` + // loader that OpenSSL always probes first for URIs without an authority. + assert.throws(() => createPrivateKey(new URL('pkcs11:object=nope')), { + code: 'ERR_OSSL_OSSL_STORE_UNSUPPORTED', + }); +} + +{ + // Only a genuine URL selects the STORE loader. A plain object that merely + // exposes `href` and `protocol` must not be treated as one, otherwise an + // attacker-supplied JWK could redirect the load to a URI of their choosing. + const file = path.join(tmpdir.path, 'priv.pem'); + const spoofed = { + kty: 'EC', + crv: 'P-256', + x: 'a', + y: 'b', + d: 'c', + href: pathToFileURL(file).href, + protocol: 'file:', + }; + assert.throws(() => createPrivateKey({ key: spoofed, format: 'jwk' }), { + code: 'ERR_CRYPTO_INVALID_JWK', + }); + assert.throws(() => createPrivateKey(spoofed), { + code: 'ERR_INVALID_ARG_TYPE', + }); + assert.throws( + () => sign(null, data, { key: spoofed, format: 'jwk' }), + { code: 'ERR_CRYPTO_INVALID_JWK' }); +} + +{ + // Read the URI from the URL's private state. Public accessors on a branded + // subclass must not redirect a previously created URL to a different key. + const trustedHref = pathToFileURL( + path.join(tmpdir.path, 'priv.pem')).href; + const redirectHref = pathToFileURL( + path.join(tmpdir.path, 'rsa.pem')).href; + class RedirectingURL extends URL { + get href() { return redirectHref; } + } + const subclassURL = new RedirectingURL(trustedHref); + assert.strictEqual( + URL.prototype.toString.call(subclassURL), trustedHref); + assert.strictEqual( + createPrivateKey(subclassURL).asymmetricKeyType, 'ed25519'); + + // The same applies when the accessor is replaced on URL.prototype. + const prototypeURL = new URL(trustedHref); + const hrefDescriptor = Object.getOwnPropertyDescriptor(URL.prototype, 'href'); + try { + Object.defineProperty(URL.prototype, 'href', { + ...hrefDescriptor, + get() { return redirectHref; }, + }); + assert.strictEqual( + URL.prototype.toString.call(prototypeURL), trustedHref); + assert.strictEqual( + createPrivateKey(prototypeURL).asymmetricKeyType, 'ed25519'); + } finally { + Object.defineProperty(URL.prototype, 'href', hrefDescriptor); + } + + // Replacing `protocol` must not turn an opaque STORE URI into a file path. + const filePathname = pathToFileURL( + path.join(tmpdir.path, 'priv.pem')).pathname; + const opaqueURL = new URL(`pkcs11:${filePathname}`); + const protocolDescriptor = Object.getOwnPropertyDescriptor( + URL.prototype, 'protocol'); + try { + Object.defineProperty(URL.prototype, 'protocol', { + ...protocolDescriptor, + get() { return 'file:'; }, + }); + assert.throws(() => createPrivateKey(opaqueURL), { + code: 'ERR_OSSL_OSSL_STORE_UNSUPPORTED', + }); + } finally { + Object.defineProperty(URL.prototype, 'protocol', protocolDescriptor); + } +} + +{ + // The URI is handed to OpenSSL as a NUL-terminated C string, so an embedded + // NUL must be rejected rather than silently truncating the path. + const file = path.join(tmpdir.path, 'priv.pem'); + assert.throws( + () => createPrivateKey(new URL(`${pathToFileURL(file).href}%00.txt`)), { + code: 'ERR_INVALID_ARG_VALUE', + }); + + // Same for the property query. + assert.throws(() => createPrivateKey({ + key: pathToFileURL(file), + properties: `provider=def${'\u0000'}ault`, + }), { + code: 'ERR_INVALID_ARG_VALUE', + }); +} diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js index a6a0d339d243..527c39b30786 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -625,6 +625,41 @@ if (hasOpenSSL(3, 2)) { assert.throws(() => crypto.verify(null, data, 'test', input), errObj); }); +// Preserve the current behavior from https://github.com/nodejs/node/issues/53761: +// one-shot verify does not accept SM2 signatures produced by the streaming path. +if (hasOpenSSL(3) && crypto.getHashes().includes('sm3')) { + const data = Buffer.from('AABB'); + const privateKey = crypto.createPrivateKey(`-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqBHM9VAYItBG0wawIBAQQgbjCNHopgvyGVfLaP +PamI9E9lf6jXT+xm1Pns1t/xQTihRANCAATV+I7HUGF2gC+miVl3JfjpoZaU2hrZ +QqHwKUNtIDE/uxxWNLBbYKaiLOWrbYA8skrWQWl3RkbXW4ZI28afRw9g +-----END PRIVATE KEY----- +`); + const publicKey = crypto.createPublicKey(`-----BEGIN PUBLIC KEY----- +MFkwEwYHKoZIzj0CAQYIKoEcz1UBgi0DQgAE1fiOx1BhdoAvpolZdyX46aGWlNoa +2UKh8ClDbSAxP7scVjSwW2Cmoizlq22APLJK1kFpd0ZG11uGSNvGn0cPYA== +-----END PUBLIC KEY-----`); + // Generate the signatures in-test so this checks API behavior rather than + // provider-version-specific SM2 signature fixtures. + const validOneShotSignature = crypto.sign('sm3', data, privateKey); + const streamingSign = crypto.createSign('sm3'); + streamingSign.update(data); + const streamingOnlySignature = streamingSign.sign(privateKey); + + assert.strictEqual( + crypto.verify('sm3', data, publicKey, validOneShotSignature), + true); + assert.strictEqual( + crypto.verify('sm3', data, publicKey, streamingOnlySignature), + false); + + const streamingVerify = crypto.createVerify('sm3'); + streamingVerify.update(data); + assert.strictEqual( + streamingVerify.verify(publicKey, streamingOnlySignature), + true); +} + { const data = Buffer.from('Hello world'); const keys = [['ec-key.pem', 64], ['dsa_private_1025.pem', 40]]; @@ -734,13 +769,9 @@ if (hasOpenSSL(3, 2)) { // RSA-PSS Sign test by verifying with 'openssl dgst -verify' -// Note: this particular test *must* be the last in this file as it will exit -// early if no openssl binary is found -{ - if (!opensslCli) { - common.skip('node compiled without OpenSSL CLI.'); - } - +if (!opensslCli) { + common.printSkipMessage('node compiled without OpenSSL CLI.'); +} else { const pubfile = fixtures.path('keys', 'rsa_public_2048.pem'); const privkey = fixtures.readKey('rsa_private_2048.pem'); diff --git a/test/parallel/test-permission-has.js b/test/parallel/test-permission-has.js index 838599735610..db5be830063b 100644 --- a/test/parallel/test-permission-has.js +++ b/test/parallel/test-permission-has.js @@ -30,6 +30,7 @@ const assert = require('assert'); assert.ok(!process.permission.has('fs')); assert.ok(process.permission.has('fs.read')); assert.ok(!process.permission.has('fs.write')); + assert.ok(!process.permission.has('openssl.store')); assert.ok(!process.permission.has('wasi')); assert.ok(!process.permission.has('worker')); assert.ok(!process.permission.has('inspector')); diff --git a/test/parallel/test-permission-openssl-store.js b/test/parallel/test-permission-openssl-store.js new file mode 100644 index 000000000000..ca2f240a9520 --- /dev/null +++ b/test/parallel/test-permission-openssl-store.js @@ -0,0 +1,93 @@ +// Flags: --permission --allow-fs-read=* --allow-fs-write=* --allow-openssl-store --allow-child-process +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const { hasOpenSSL3 } = require('../common/crypto'); +if (!hasOpenSSL3) + common.skip('requires OpenSSL 3.x'); + +// Verifies the openssl.store permission: allowed when --allow-openssl-store is +// set, can be dropped at runtime, and denied by default in a child process. + +const assert = require('assert'); +const dc = require('diagnostics_channel'); +const fs = require('fs'); +const path = require('path'); +const { pathToFileURL } = require('url'); +const { createPrivateKey, generateKeyPairSync } = require('crypto'); +const { spawnSync } = require('child_process'); +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const file = path.join(tmpdir.path, 'priv.pem'); +fs.writeFileSync( + file, generateKeyPairSync('ed25519').privateKey.export({ + format: 'pem', type: 'pkcs8', + })); +const url = pathToFileURL(file); + +assert.strictEqual(process.permission.has('openssl.store'), true); +assert.strictEqual(createPrivateKey(url).type, 'private'); +// A plain object that merely looks like a URL is not one, so it never reaches +// a STORE loader and cannot be used to sidestep this permission. +assert.throws(() => createPrivateKey({ + href: file, + protocol: 'pkcs11:', +}), { code: 'ERR_INVALID_ARG_TYPE' }); + +process.permission.drop('openssl.store'); +assert.strictEqual(process.permission.has('openssl.store'), false); + +const secret = 'store-permission-secret'; +const messages = []; +dc.subscribe('node:permission-model:openssl-store', (message) => { + messages.push(message); +}); +assert.throws( + () => createPrivateKey(new URL(`pkcs11:object=key;pin-value=${secret}`)), + (error) => { + assert.strictEqual(error.code, 'ERR_ACCESS_DENIED'); + assert.strictEqual(error.permission, 'OpenSSLStore'); + assert.strictEqual(error.resource, ''); + assert.doesNotMatch(error.stack, new RegExp(secret)); + return true; + }); +assert.strictEqual(messages.length, 1); +assert.strictEqual(messages[0].permission, 'OpenSSLStore'); +assert.strictEqual(messages[0].resource, ''); +assert.doesNotMatch(JSON.stringify(messages[0]), new RegExp(secret)); + +// Denied by default when the flag is not provided. +{ + const { status, stdout } = spawnSync(process.execPath, [ + '--permission', '--allow-fs-read=*', + '-e', `try { require('crypto').createPrivateKey(new URL(${JSON.stringify(url.href)})); console.log('LOADED'); } catch (e) { console.log(e.code, e.permission); }`, + ]); + assert.strictEqual(status, 0); + assert.match(stdout.toString(), /ERR_ACCESS_DENIED OpenSSLStore/); +} + +// openssl.store grants the STORE loader authority to access files even when +// fs.read is not granted. +{ + const { status, stdout } = spawnSync(process.execPath, [ + '--permission', '--allow-openssl-store', + '-e', `try { require('crypto').createPrivateKey(new URL(${JSON.stringify(url.href)})); console.log('LOADED'); } catch (e) { console.log(e.code, e.permission); }`, + ]); + assert.strictEqual(status, 0); + assert.match(stdout.toString(), /LOADED/); +} + +// OpenSSL tries the file loader before the loader identified by an opaque URI. +{ + const opaqueFile = 'pkcs11:priv.pem'; + fs.copyFileSync(file, path.join(tmpdir.path, opaqueFile)); + const { status, stdout } = spawnSync(process.execPath, [ + '--permission', '--allow-openssl-store', + '-e', `try { require('crypto').createPrivateKey(new URL(${JSON.stringify(opaqueFile)})); console.log('LOADED'); } catch (e) { console.log(e.code, e.permission); }`, + ], { cwd: tmpdir.path }); + assert.strictEqual(status, 0); + assert.match(stdout.toString(), /LOADED/); +} diff --git a/test/parallel/test-permission-warning-flags.js b/test/parallel/test-permission-warning-flags.js index 89deaf46fcb4..e2c17286983a 100644 --- a/test/parallel/test-permission-warning-flags.js +++ b/test/parallel/test-permission-warning-flags.js @@ -10,6 +10,7 @@ const warnFlags = [ '--allow-inspector', '--allow-wasi', '--allow-worker', + '--allow-openssl-store', ]; if (process.config.variables.node_use_ffi) { diff --git a/tools/nix/pkcs11.nix b/tools/nix/pkcs11.nix new file mode 100644 index 000000000000..eb6165b7bd99 --- /dev/null +++ b/tools/nix/pkcs11.nix @@ -0,0 +1,128 @@ +# PKCS#11 fixture for `test/parallel/test-crypto-key-store-pkcs11.js`. +# +# Builds a SoftHSM2 token holding the key material that test expects, together +# with the OpenSSL configuration that activates pkcs11-provider against it. +# `shell.nix` exposes the result through the `NODE_TEST_PKCS11_*` environment +# variables when it is instantiated with `--arg pkcs11 true`. +{ + pkgs ? import ./pkgs.nix { }, + + # pkcs11-provider is dlopen'd into the libcrypto Node.js itself links, so it + # has to be built against that very OpenSSL. SoftHSM links OpenSSL too; + # building it against the same one keeps a single libcrypto in the process. + openssl ? (import ./sharedLibDeps.nix { inherit pkgs; }).openssl, + + pin ? "1234", +}: + +let + # An override that yields the very same derivation is free, the package still + # comes from the binary cache. Skipping pkcs11-provider's test suite is what + # would cost that cache hit, so only do it once the override has forced a + # rebuild anyway. SoftHSM needs neither, it does not run tests at all. + pkcs11-provider = + let + inherit (pkgs) pkcs11-provider; + pkcs11-provider' = pkcs11-provider.override { inherit openssl; }; + in + if pkcs11-provider != pkcs11-provider' then + pkcs11-provider'.overrideAttrs { + doCheck = false; + } + else + pkcs11-provider; + + softhsm = pkgs.softhsm.override { inherit openssl; }; + + # OpenSSL names its provider modules after the platform's shared library + # extension, while SoftHSM is a libtool module and keeps the `.so` suffix + # everywhere, macOS included. + providerModule = "${pkcs11-provider}/lib/ossl-modules/pkcs11${pkgs.stdenv.hostPlatform.extensions.sharedLibrary}"; + softhsmModule = "${softhsm}/lib/softhsm/libsofthsm2.so"; + + # SoftHSM resolves a relative `directories.tokendir` against the working + # directory, so naming it that way keeps this a plain store file rather than + # something that has to be generated once the token's location is known. + softhsmConf = pkgs.writeText "softhsm2.conf" '' + directories.tokendir = tokens + objectstore.backend = file + log.level = ERROR + slots.removable = false + ''; +in +{ + inherit + pkcs11-provider + softhsm + ; + + opensslConf = pkgs.writeText "openssl-pkcs11.cnf" '' + nodejs_conf = nodejs_init + + [nodejs_init] + providers = provider_sect + + [provider_sect] + default = default_sect + pkcs11 = pkcs11_sect + + [default_sect] + activate = 1 + + [pkcs11_sect] + module = ${providerModule} + pkcs11-module-path = ${softhsmModule} + pkcs11-module-quirks = no-deinit + activate = 1 + ''; + + # A SoftHSM working directory: the configuration above next to the token it + # names. SoftHSM opens its token read-write, which a store path can never be, + # so this is meant to be copied somewhere writable and used from there. + # + # `pkcs11-tool` comes from OpenSC and only fills the token here; neither it + # nor `softhsm2-util` is needed to run the test. + softhsmDir = + pkgs.runCommand "node-pkcs11-softhsm" + { + nativeBuildInputs = [ + softhsm + pkgs.opensc + ]; + + passthru = { + inherit pin; + }; + } + '' + export SOFTHSM2_CONF=${softhsmConf} + mkdir tokens + + softhsm2-util --init-token --free --label node-test \ + --pin "${pin}" --so-pin "${pin}" + + keypairgen() { + pkcs11-tool --module "${softhsmModule}" --login --pin "${pin}" \ + --keypairgen --key-type "$1" --id "$2" --label "$3" "$4" + } + + # Keep this fixture limited to key types that SoftHSM and + # pkcs11-provider can both generate and operate. Node's PQC APIs + # require OpenSSL >= 3.5, but that does not imply ML-DSA or ML-KEM + # support in this PKCS#11 stack. + # + # RSA decryption is likewise not covered: pkcs11-provider fails every + # padding mode with `provider asym cipher failure` even for a key whose + # PKCS#11 attributes include the decrypt usage, so + # crypto.privateDecrypt() cannot be exercised against this stack. + keypairgen RSA:2048 01 node-rsa --usage-sign + keypairgen EC:prime256v1 02 node-ec --usage-sign + keypairgen EC:ED25519 03 node-ed25519 --usage-sign + keypairgen EC:ED448 04 node-ed448 --usage-sign + keypairgen EC:prime256v1 05 node-ecdh --usage-derive + + mkdir -p "$out" + cp -R tokens "$out/tokens" + cp ${softhsmConf} "$out/softhsm2.conf" + ''; +} diff --git a/typings/internalBinding/crypto.d.ts b/typings/internalBinding/crypto.d.ts index d91c5018ba68..03ea3e74c641 100644 --- a/typings/internalBinding/crypto.d.ts +++ b/typings/internalBinding/crypto.d.ts @@ -9,6 +9,7 @@ declare namespace InternalCryptoBinding { type KeyFormatRawPublic = 3; type KeyFormatRawPrivate = 4; type KeyFormatRawSeed = 5; + type KeyFormatStore = 6; type PublicKeyFormat = KeyFormatDER | KeyFormatPEM | KeyFormatJWK | KeyFormatRawPublic | undefined; type PrivateKeyFormat = @@ -18,7 +19,12 @@ declare namespace InternalCryptoBinding { type KeyEncoding = string | number | null | undefined; type KeyPassphrase = ByteSource | null | undefined; type NamedCurve = string | null | undefined; - type PreparedAsymmetricKeyData = KeyObjectHandle | ByteSource | JwkKey; + interface StorePrivateKeyData { + uri: string; + properties: string | null; + } + type PreparedAsymmetricKeyData = + KeyObjectHandle | ByteSource | JwkKey | StorePrivateKeyData; type PreparedSecretKeyData = KeyObjectHandle | ByteSource; type CryptoJobAsyncMode = 0; type CryptoJobSyncMode = 1; @@ -126,7 +132,7 @@ declare namespace InternalCryptoBinding { type PreparedAsymmetricKeyArgs = [ keyData: PreparedAsymmetricKeyData, - keyFormat: KeyFormat, + keyFormat: KeyFormat | KeyFormatStore, keyType: KeyEncoding, keyPassphrase: KeyPassphrase, keyNamedCurve: NamedCurve, @@ -862,6 +868,7 @@ export interface CryptoBinding { kKeyFormatRawPrivate: InternalCryptoBinding.KeyFormatRawPrivate; kKeyFormatRawPublic: InternalCryptoBinding.KeyFormatRawPublic; kKeyFormatRawSeed: InternalCryptoBinding.KeyFormatRawSeed; + kKeyFormatStore: InternalCryptoBinding.KeyFormatStore; kKeyTypePrivate: number; kKeyTypePublic: number; kKeyTypeSecret: number; From f43086d1e4d67e0a19e08da146a889b7b11e105a Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:18:19 -0700 Subject: [PATCH 3/3] ffi: fix crash in refCallback and unrefCallback Both handlers called fn.ClearWeak() and fn.SetWeak() without checking whether the persistent handle was still populated. After the callback function is garbage collected following an earlier unrefCallback() call, the handle is empty and both V8 methods dereference a null slot. InvokeCallback() already guarded against this. Add the same check to both handlers and throw ERR_INVALID_ARG_VALUE, matching the existing behavior for a pointer that is not in the callback map. Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Assisted-by: claude:opus-5 PR-URL: https://github.com/nodejs/node/pull/64881 Fixes: https://github.com/nodejs/node/issues/64880 Reviewed-By: Matteo Collina Reviewed-By: Aviv Keller --- doc/api/ffi.md | 7 +++++++ src/node_ffi.cc | 17 ++++++++++++++++ test/ffi/test-ffi-weakref-calls.js | 31 ++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/doc/api/ffi.md b/doc/api/ffi.md index db040d78ba5e..e7e5f74e23c3 100644 --- a/doc/api/ffi.md +++ b/doc/api/ffi.md @@ -460,6 +460,10 @@ memory. Keeps the callback strongly referenced by JavaScript. +Throws `ERR_INVALID_ARG_VALUE` if the callback function has already been +garbage collected after a previous `library.unrefCallback(pointer)` call, since +a collected function cannot be referenced again. + ### `library.unrefCallback(pointer)` * `pointer` {bigint} @@ -470,6 +474,9 @@ If the callback function is later garbage collected, subsequent native invocations become a no-op. Non-void return values are zero-initialized before returning to native code. +Throws `ERR_INVALID_ARG_VALUE` if the callback function has already been +garbage collected. + ## Calling native functions Argument conversion depends on the declared FFI type. diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 17b4c13c4552..6cce5e38e8e3 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -1127,6 +1127,15 @@ void DynamicLibrary::RefCallback(const FunctionCallbackInfo& args) { return; } + // The callback function may already have been collected after a previous + // unrefCallback() call. The persistent handle is empty in that case, and + // ClearWeak() on an empty handle dereferences a null slot. There is also no + // function left to make strong again. + if (existing->second->fn.IsEmpty()) { + THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found"); + return; + } + existing->second->fn.ClearWeak(); } @@ -1158,6 +1167,14 @@ void DynamicLibrary::UnrefCallback(const FunctionCallbackInfo& args) { return; } + // The callback function may already have been collected by a previous + // unrefCallback() call. The persistent handle is empty in that case, and + // SetWeak() on an empty handle dereferences a null slot. + if (existing->second->fn.IsEmpty()) { + THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found"); + return; + } + existing->second->fn.SetWeak(); } diff --git a/test/ffi/test-ffi-weakref-calls.js b/test/ffi/test-ffi-weakref-calls.js index 1a2a2eaeab87..b158ee810776 100644 --- a/test/ffi/test-ffi-weakref-calls.js +++ b/test/ffi/test-ffi-weakref-calls.js @@ -51,6 +51,37 @@ test('ffi refCallback retains callback function', async (t) => { lib.unregisterCallback(pointer); }); +test('callback ref/unref throw after callback function is collected', async (t) => { + const { lib } = ffi.dlopen(libraryPath, fixtureSymbols); + t.after(() => lib.close()); + + let callback = () => 1; + const ref = new WeakRef(callback); + const pointer = lib.registerCallback( + { arguments: ['i32'], return: 'i32' }, + callback, + ); + + lib.unrefCallback(pointer); + callback = null; + + await gcUntil( + 'callback ref/unref throw after callback function is collected', + () => ref.deref() === undefined, + ); + + t.assert.throws(() => lib.unrefCallback(pointer), { + code: 'ERR_INVALID_ARG_VALUE', + message: /Callback not found/, + }); + t.assert.throws(() => lib.refCallback(pointer), { + code: 'ERR_INVALID_ARG_VALUE', + message: /Callback not found/, + }); + + lib.unregisterCallback(pointer); +}); + test('callback ref/unref/unregister throw when library is closed', (t) => { const { lib } = ffi.dlopen(libraryPath, fixtureSymbols); const callback = lib.registerCallback(() => {});