fixup! crypto: support loading private keys through STORE loaders

Author: panva

doc/api/crypto.md

@@ -4051,6 +4051,16 @@ 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.

lib/internal/crypto/keys.js

@@ -7,6 +7,7 @@ const {
   ObjectSetPrototypeOf,
   SafeSet,
   StringPrototypeIncludes,
+  StringPrototypeStartsWith,
   SymbolToStringTag,
   Uint8Array,
 } = primordials;
@@ -74,7 +75,11 @@ const {
   isArrayBufferView,
 } = require('internal/util/types');
 
-const { fileURLToPath, isURLInstance, URL } = require('internal/url');
+const {
+  fileURLToPath,
+  getURLHref,
+  isURLInstance,
+} = require('internal/url');
 
 const {
   customInspectSymbol: kInspect,
@@ -542,13 +547,12 @@ function getKeyTypes({ allowKeyObject, allowURL = false }) {
 }
 
 function prepareStorePrivateKey(url, name, passphrase, encoding, properties) {
-  const normalizedURL = new URL(url.href);
-  let uri = normalizedURL.href;
-  if (normalizedURL.protocol === 'file:')
-    uri = fileURLToPath(normalizedURL);
+  // 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);
 
-  // A subclass may override the `href` getter, so the value is not guaranteed
-  // to be a string even for a genuine URL instance.
   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

lib/internal/url.js

@@ -230,6 +230,11 @@ let setURLSearchParams;
  * @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;
@@ -821,6 +826,11 @@ class URL {
   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;
@@ -1734,6 +1744,7 @@ module.exports = {
   encodeStr,
   isURL,
   isURLInstance,
+  getURLHref,
 
   urlUpdateActions: updateActions,
   getURLOrigin,

test/parallel/test-crypto-key-store.js

@@ -233,6 +233,57 @@ const data = Buffer.from('hello store');
     { 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.