Skip to content

Commit ddd7eaf

Browse files
committed
crypto: support OpenSSL STORE private keys
Allow createPrivateKey() to load private keys from WHATWG URL objects through configured OpenSSL STORE providers. Reject built-in STORE loaders so this does not become a generic file or network URL loader, and gate the operation with the crypto.store permission. Mark resulting private KeyObjects as store-backed. Public rewrapping and private-key export are rejected, while provider-backed private operations use the existing EVP paths. Add hermetic STORE/keymgmt addon coverage and wire the pkcs11-provider SoftHSM test into shared-library CI for OpenSSL 3 and newer. Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent d001e26 commit ddd7eaf

37 files changed

Lines changed: 1839 additions & 33 deletions

.github/actions/build-shared/action.yml

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ inputs:
1313
description: Cachix auth token for nodejs.cachix.org.
1414
required: false
1515
default: ''
16+
pkcs11-store-provider-test:
17+
description: >
18+
Whether to make pkcs11-provider, SoftHSM, and OpenSC available to the
19+
regular test suite for the OpenSSL STORE provider-backed key test.
20+
required: false
21+
default: 'false'
1622

1723
runs:
1824
using: composite
@@ -51,15 +57,66 @@ runs:
5157
- name: Build Node.js and run tests
5258
shell: bash
5359
run: |
60+
dev_tools='[]'
61+
if [ "${{ inputs.pkcs11-store-provider-test }}" = "true" ]; then
62+
# The external STORE provider test dlopens both the OpenSSL provider
63+
# and the PKCS#11 module into the test Node.js process. Build both
64+
# against the same OpenSSL as the shared-OpenSSL matrix entry to avoid
65+
# mixing libcrypto ABIs from the nixpkgs default and the matrix build.
66+
# Keep this in sync with the pkcs11StoreProviderTest gate in
67+
# .github/workflows/test-shared.yml.
68+
openssl_attr="${OPENSSL_ATTR:-openssl_3_5}"
69+
shared_openssl="(import $TAR_DIR/tools/nix/openssl-matrix.nix { inherit pkgs; }).$openssl_attr"
70+
export NODE_TEST_PKCS11_PROVIDER_PACKAGE="$(
71+
nix-build \
72+
-I "nixpkgs=$TAR_DIR/tools/nix/pkgs.nix" \
73+
--no-out-link \
74+
-E "
75+
let
76+
pkgs = import <nixpkgs> {};
77+
openssl = $shared_openssl;
78+
in (pkgs.pkcs11-provider.override { inherit openssl; }).overrideAttrs (_: {
79+
doCheck = false;
80+
})
81+
"
82+
)"
83+
export NODE_TEST_SOFTHSM_PACKAGE="$(
84+
nix-build \
85+
-I "nixpkgs=$TAR_DIR/tools/nix/pkgs.nix" \
86+
--no-out-link \
87+
-E "
88+
let
89+
pkgs = import <nixpkgs> {};
90+
openssl = $shared_openssl;
91+
in (pkgs.softhsm.override { inherit openssl; }).overrideAttrs (_: {
92+
doCheck = false;
93+
})
94+
"
95+
)"
96+
dev_tools='with import <nixpkgs> {}; [ opensc ]'
97+
fi
98+
5499
nix-shell \
55100
-I "nixpkgs=$TAR_DIR/tools/nix/pkgs.nix" \
56101
--pure --keep TAR_DIR --keep FLAKY_TESTS \
102+
--keep NODE_TEST_PKCS11_PROVIDER_PACKAGE --keep NODE_TEST_SOFTHSM_PACKAGE \
57103
--keep SCCACHE_GHA_ENABLED --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN \
58104
--arg loadJSBuiltinsDynamically false \
59105
--arg ccache "${NIX_SCCACHE:-null}" \
60-
--arg devTools '[]' \
106+
--arg devTools "$dev_tools" \
61107
--arg benchmarkTools '[]' \
62108
${{ inputs.extra-nix-flags }} \
63109
--run '
64-
make -C "$TAR_DIR" run-ci -j4 V=1 TEST_CI_ARGS="-p actions --measure-flakiness 9 --skip-tests=$CI_SKIP_TESTS"
110+
set -e
111+
if [ "${{ inputs.pkcs11-store-provider-test }}" = "true" ]; then
112+
provider=$(find "$NODE_TEST_PKCS11_PROVIDER_PACKAGE" \( -path "*/lib/ossl-modules/pkcs11.so" -o -path "*/lib/ossl-modules/pkcs11.dylib" \) -print -quit)
113+
softhsm=$(find "$NODE_TEST_SOFTHSM_PACKAGE" \( -path "*/lib/softhsm/libsofthsm2.so" -o -path "*/lib/softhsm/libsofthsm2.dylib" \) -print -quit)
114+
test -n "$provider"
115+
test -n "$softhsm"
116+
export PATH="$NODE_TEST_SOFTHSM_PACKAGE/bin:$PATH"
117+
export NODE_TEST_PKCS11_PROVIDER=1
118+
export NODE_TEST_PKCS11_PROVIDER_MODULE="$provider"
119+
export NODE_TEST_SOFTHSM_MODULE="$softhsm"
120+
fi
121+
make -C "$TAR_DIR" run-ci -j4 V=1 TEST_CI_ARGS="-p actions --measure-flakiness 9 --skip-tests=${CI_SKIP_TESTS:-}"
65122
' "$TAR_DIR/shell.nix"

.github/workflows/test-shared.yml

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ jobs:
162162
name: Build and test Node.js
163163
with:
164164
cachix-auth-token: ${{ secrets.CACHIX_AUTH_TOKEN }}
165+
pkcs11-store-provider-test: 'true'
165166
extra-nix-flags: |
166167
--arg useSeparateDerivationForV8 true \
167168
${{ endsWith(matrix.system, '-darwin') && '--arg withAmaro false --arg withLief false --arg withSQLite false --arg withFFI false --arg extraConfigFlags ''["--without-inspector" "--without-node-options"]'' \' || '\' }}
@@ -229,9 +230,31 @@ jobs:
229230
echo "matrix=$(
230231
nix-instantiate --eval --strict --json -E "
231232
let
232-
matrix = import $TAR_DIR/tools/nix/openssl-matrix.nix {};
233+
pkgs = import $TAR_DIR/tools/nix/pkgs.nix {
234+
config.permittedInsecurePackages = [ \"openssl-1.1.1w\" ];
235+
};
236+
matrix = import $TAR_DIR/tools/nix/openssl-matrix.nix {
237+
inherit pkgs;
238+
};
233239
in
234-
builtins.map (attr: { inherit attr; inherit (builtins.getAttr attr matrix) name; }) (builtins.attrNames matrix)
240+
builtins.map (attr:
241+
let
242+
openssl = builtins.getAttr attr matrix;
243+
in
244+
{
245+
inherit attr;
246+
inherit (openssl) name;
247+
# The real pkcs11 STORE test needs pkcs11-provider and
248+
# SoftHSM built against the same OpenSSL as Node. The pinned
249+
# SoftHSM package currently builds against OpenSSL 3.x but
250+
# not OpenSSL 4.x, so keep this to the known-working range and
251+
# revisit when https://github.com/softhsm/SoftHSMv2/issues/868
252+
# is fixed and the pinned nixpkgs provider stack supports 4.x.
253+
pkcs11StoreProviderTest =
254+
openssl.pname == \"openssl\" &&
255+
pkgs.lib.versionAtLeast openssl.version \"3\" &&
256+
pkgs.lib.versionOlder openssl.version \"4\";
257+
}) (builtins.attrNames matrix)
235258
"
236259
)" >> "$GITHUB_OUTPUT"
237260
@@ -267,6 +290,7 @@ jobs:
267290
name: Build and test Node.js
268291
with:
269292
cachix-auth-token: ${{ secrets.CACHIX_AUTH_TOKEN }}
293+
pkcs11-store-provider-test: ${{ matrix.openssl.pkcs11StoreProviderTest }}
270294
# Override just the `openssl` attr of the default shared-lib set with
271295
# the matrix-selected nixpkgs attribute (e.g. `openssl_3_6`). All
272296
# other shared libs (brotli, cares, libuv, …) keep their defaults.

deps/ncrypto/ncrypto.cc

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,9 +2839,28 @@ std::optional<uint32_t> EVPKeyPointer::getBytesOfRS() const {
28392839
if (id == EVP_PKEY_DSA) {
28402840
const DSA* dsa_key = EVP_PKEY_get0_DSA(get());
28412841
// Both r and s are computed mod q, so their width is limited by that of q.
2842-
bits = BignumPointer::GetBitCount(DSA_get0_q(dsa_key));
2842+
if (dsa_key != nullptr) {
2843+
bits = BignumPointer::GetBitCount(DSA_get0_q(dsa_key));
2844+
#if OPENSSL_VERSION_MAJOR >= 3 && !defined(OPENSSL_IS_BORINGSSL)
2845+
} else if (EVP_PKEY_get_int_param(
2846+
get(), OSSL_PKEY_PARAM_FFC_QBITS, &bits) != 1) {
2847+
return std::nullopt;
2848+
#endif
2849+
} else {
2850+
return std::nullopt;
2851+
}
28432852
} else if (id == EVP_PKEY_EC) {
2844-
bits = EC_GROUP_order_bits(ECKeyPointer::GetGroup(*this));
2853+
const EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(get());
2854+
if (ec_key != nullptr) {
2855+
bits = EC_GROUP_order_bits(ECKeyPointer::GetGroup(ec_key));
2856+
#if OPENSSL_VERSION_MAJOR >= 3 && !defined(OPENSSL_IS_BORINGSSL)
2857+
} else if (EVP_PKEY_get_int_param(get(), OSSL_PKEY_PARAM_BITS, &bits) !=
2858+
1) {
2859+
return std::nullopt;
2860+
#endif
2861+
} else {
2862+
return std::nullopt;
2863+
}
28452864
} else {
28462865
return std::nullopt;
28472866
}

doc/api/cli.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,28 @@ This behavior also applies to `child_process.spawn()`, but in that case, the
191191
flags are propagated via the `NODE_OPTIONS` environment variable rather than
192192
directly through the process arguments.
193193

194+
### `--allow-crypto-store`
195+
196+
<!-- YAML
197+
added: REPLACEME
198+
-->
199+
200+
> Stability: 1.1 - Active development
201+
202+
When using the [Permission Model][], the process will not be able to load
203+
private keys from OpenSSL STORE provider URLs by default. Attempts to do so
204+
will throw an `ERR_ACCESS_DENIED` unless the user explicitly passes the
205+
`--allow-crypto-store` flag when starting Node.js.
206+
207+
This permission only applies to OpenSSL STORE provider URLs accepted by
208+
[`crypto.createPrivateKey()`][]. It does not grant access to Node.js file
209+
system or network APIs. Configured OpenSSL providers may still perform their
210+
own I/O, credential handling, hardware access, or daemon communication outside
211+
of Node.js `fs` and `net` permission scopes.
212+
213+
Node.js does not pass URL input to OpenSSL's built-in `default` or `base` STORE
214+
loaders, so local file STORE loading is not exposed through this API.
215+
194216
### `--allow-ffi`
195217

196218
<!-- YAML
@@ -2322,6 +2344,7 @@ following permissions are restricted:
23222344
* File System - manageable through
23232345
[`--allow-fs-read`][], [`--allow-fs-write`][] flags
23242346
* Network - manageable through [`--allow-net`][] flag
2347+
* OpenSSL STORE - manageable through [`--allow-crypto-store`][] flag
23252348
* Child Process - manageable through [`--allow-child-process`][] flag
23262349
* Worker Threads - manageable through [`--allow-worker`][] flag
23272350
* WASI - manageable through [`--allow-wasi`][] flag
@@ -3777,6 +3800,7 @@ one is included in the list below.
37773800

37783801
* `--allow-addons`
37793802
* `--allow-child-process`
3803+
* `--allow-crypto-store`
37803804
* `--allow-ffi`
37813805
* `--allow-fs-read`
37823806
* `--allow-fs-write`
@@ -4420,6 +4444,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
44204444
[`"type"`]: packages.md#type
44214445
[`--allow-addons`]: #--allow-addons
44224446
[`--allow-child-process`]: #--allow-child-process
4447+
[`--allow-crypto-store`]: #--allow-crypto-store
44234448
[`--allow-fs-read`]: #--allow-fs-read
44244449
[`--allow-fs-write`]: #--allow-fs-write
44254450
[`--allow-net`]: #--allow-net
@@ -4453,6 +4478,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
44534478
[`NO_COLOR`]: https://no-color.org
44544479
[`Web Storage`]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API
44554480
[`YoungGenerationSizeFromSemiSpaceSize`]: https://chromium.googlesource.com/v8/v8.git/+/refs/tags/10.3.129/src/heap/heap.cc#328
4481+
[`crypto.createPrivateKey()`]: crypto.md#cryptocreateprivatekeykey
44564482
[`dns.lookup()`]: dns.md#dnslookuphostname-options-callback
44574483
[`dns.setDefaultResultOrder()`]: dns.md#dnssetdefaultresultorderorder
44584484
[`dnsPromises.lookup()`]: dns.md#dnspromiseslookuphostname-options

doc/api/crypto.md

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2493,6 +2493,9 @@ added:
24932493

24942494
Converts a `KeyObject` instance to a `CryptoKey`.
24952495

2496+
Private keys loaded from OpenSSL STORE providers can only be converted with
2497+
`extractable` set to `false`.
2498+
24962499
### `keyObject.type`
24972500

24982501
<!-- YAML
@@ -3973,8 +3976,8 @@ changes:
39733976

39743977
<!--lint disable maximum-line-length remark-lint-->
39753978

3976-
* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView}
3977-
* `key` {string|ArrayBuffer|Buffer|TypedArray|DataView|Object} The key
3979+
* `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|URL}
3980+
* `key` {string|ArrayBuffer|Buffer|TypedArray|DataView|Object|URL} The key
39783981
material, either in PEM, DER, JWK, or raw format.
39793982
* `format` {string} Must be `'pem'`, `'der'`, `'jwk'`, `'raw-private'`,
39803983
or `'raw-seed'`. **Default:** `'pem'`.
@@ -3995,6 +3998,18 @@ Creates and returns a new key object containing a private key. If `key` is a
39953998
string or `Buffer`, `format` is assumed to be `'pem'`; otherwise, `key`
39963999
must be an object with the properties described above.
39974000

4001+
If `key` is a WHATWG {URL}, it is loaded through OpenSSL STORE providers.
4002+
Only provider-backed URLs are supported. Node.js does not pass URL input to
4003+
OpenSSL's built-in `default` or `base` STORE loaders, so local file STORE
4004+
loading is not exposed through this API. Provider-specific URL grammar and
4005+
parameters are handled by the configured provider.
4006+
4007+
When `key` is a {URL}, `passphrase` is passed separately to OpenSSL's password
4008+
callback and is not encoded into the URL.
4009+
4010+
Private keys loaded from OpenSSL STORE providers are not exportable through
4011+
Node.js, and cannot be passed where a public key is required.
4012+
39984013
If the private key is encrypted, a `passphrase` must be specified. The length
39994014
of the passphrase is limited to 1024 bytes.
40004015

@@ -4068,6 +4083,7 @@ returned `KeyObject` will be `'public'` and that the private key cannot be
40684083
extracted from the returned `KeyObject`. Similarly, if a `KeyObject` with type
40694084
`'private'` is given, a new `KeyObject` with type `'public'` will be returned
40704085
and it will be impossible to extract the private key from the returned object.
4086+
This does not apply to private keys loaded from OpenSSL STORE providers.
40714087

40724088
### `crypto.createSecretKey(key[, encoding])`
40734089

@@ -5381,6 +5397,7 @@ object, the `padding` property can be passed. Otherwise, this function uses
53815397

53825398
Because RSA public keys can be derived from private keys, a private key may
53835399
be passed instead of a public key.
5400+
This does not apply to private keys loaded from OpenSSL STORE providers.
53845401

53855402
### `crypto.publicEncrypt(key, buffer)`
53865403

@@ -5440,6 +5457,7 @@ object, the `padding` property can be passed. Otherwise, this function uses
54405457

54415458
Because RSA public keys can be derived from private keys, a private key may
54425459
be passed instead of a public key.
5460+
This does not apply to private keys loaded from OpenSSL STORE providers.
54435461

54445462
### `crypto.randomBytes(size[, callback])`
54455463

doc/api/errors.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,6 +1123,16 @@ added: v24.7.0
11231123
Attempted to use KEM operations while Node.js was not compiled with
11241124
OpenSSL with KEM support.
11251125

1126+
<a id="ERR_CRYPTO_KEY_NOT_EXPORTABLE"></a>
1127+
1128+
### `ERR_CRYPTO_KEY_NOT_EXPORTABLE`
1129+
1130+
<!-- YAML
1131+
added: REPLACEME
1132+
-->
1133+
1134+
A private key could not be exported because the key material is not exportable.
1135+
11261136
<a id="ERR_CRYPTO_OPERATION_FAILED"></a>
11271137

11281138
### `ERR_CRYPTO_OPERATION_FAILED`

doc/api/permissions.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,11 @@ Error: Access to this API has been restricted
6868
Allowing access to spawning a process and creating worker threads can be done
6969
using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.
7070

71-
To allow network access, use [`--allow-net`][] and for allowing native addons
72-
when using permission model, use the [`--allow-addons`][]
71+
To allow network access, use [`--allow-net`][]. To allow loading private keys
72+
from OpenSSL STORE provider URLs, use [`--allow-crypto-store`][]. This does not
73+
grant access to Node.js file system or network APIs, but configured OpenSSL
74+
providers may perform their own I/O outside those permission scopes. For
75+
allowing native addons when using permission model, use the [`--allow-addons`][]
7376
flag. For WASI, use the [`--allow-wasi`][] flag. For FFI, use the
7477
[`--allow-ffi`][] flag. The [`node:ffi`](ffi.md) module also requires the
7578
`--experimental-ffi` flag and is only available in builds with FFI support.
@@ -206,6 +209,7 @@ Example `node.config.json`:
206209
"allow-fs-write": ["./bar"],
207210
"allow-child-process": true,
208211
"allow-worker": true,
212+
"allow-crypto-store": true,
209213
"allow-net": true,
210214
"allow-addons": false,
211215
"allow-ffi": false
@@ -318,6 +322,7 @@ Developers relying on --permission to sandbox untrusted code should be aware tha
318322
[Security Policy]: https://github.com/nodejs/node/blob/main/SECURITY.md
319323
[`--allow-addons`]: cli.md#--allow-addons
320324
[`--allow-child-process`]: cli.md#--allow-child-process
325+
[`--allow-crypto-store`]: cli.md#--allow-crypto-store
321326
[`--allow-ffi`]: cli.md#--allow-ffi
322327
[`--allow-fs-read`]: cli.md#--allow-fs-read
323328
[`--allow-fs-write`]: cli.md#--allow-fs-write

doc/api/process.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3210,6 +3210,7 @@ The available scopes are the same as [`process.permission.has()`][]:
32103210
* `child` - Child process spawning operations
32113211
* `worker` - Worker thread spawning operation
32123212
* `net` - Network operations
3213+
* `crypto.store` - OpenSSL STORE provider private key loading
32133214
* `inspector` - Inspector operations
32143215
* `wasi` - WASI operations
32153216
* `addon` - Native addon operations

doc/node.1

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,20 @@ This behavior also applies to \fBchild_process.spawn()\fR, but in that case, the
135135
flags are propagated via the \fBNODE_OPTIONS\fR environment variable rather than
136136
directly through the process arguments.
137137
.
138+
.It Fl -allow-crypto-store
139+
When using the Permission Model, the process will not be able to load private
140+
keys from OpenSSL STORE provider URLs by default.
141+
Attempts to do so will throw an \fBERR_ACCESS_DENIED\fR unless the user
142+
explicitly passes the \fB--allow-crypto-store\fR flag when starting Node.js.
143+
This permission only applies to OpenSSL STORE provider URLs accepted by
144+
\fBcrypto.createPrivateKey()\fR. It does not grant access to Node.js file system
145+
or network APIs. Configured OpenSSL providers may still perform their own I/O,
146+
credential handling, hardware access, or daemon communication outside of
147+
Node.js \fBfs\fR and \fBnet\fR permission scopes.
148+
Node.js does not pass URL input to OpenSSL's built-in \fBdefault\fR or
149+
\fBbase\fR STORE loaders, so local file STORE loading is not exposed through
150+
this API.
151+
.
138152
.It Fl -allow-ffi
139153
When using the Permission Model, the process will not be able to use
140154
\fBnode:ffi\fR by default.
@@ -1152,6 +1166,8 @@ File System - manageable through
11521166
.It
11531167
Network - manageable through \fB--allow-net\fR flag
11541168
.It
1169+
OpenSSL STORE - manageable through \fB--allow-crypto-store\fR flag
1170+
.It
11551171
Child Process - manageable through \fB--allow-child-process\fR flag
11561172
.It
11571173
Worker Threads - manageable through \fB--allow-worker\fR flag
@@ -1875,6 +1891,8 @@ one is included in the list below.
18751891
.It
18761892
\fB--allow-child-process\fR
18771893
.It
1894+
\fB--allow-crypto-store\fR
1895+
.It
18781896
\fB--allow-ffi\fR
18791897
.It
18801898
\fB--allow-fs-read\fR

0 commit comments

Comments
 (0)