crypto: validate RSA-PSS saltLength in subtle.sign and subtle.verify

fixes: #52188 PR-URL: #52262 Fixes: #52188 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
nodejs · May 2, 2024 · 956e8fc · 956e8fc
1 parent e18fef6
commit 956e8fc
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 23 deletions.
diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  MathCeil,
   SafeSet,
   Uint8Array,
 } = primordials;
@@ -27,6 +28,7 @@ const {
 
 const {
   bigIntArrayToUnsignedInt,
+  getDigestSizeInBytes,
   getUsagesUnion,
   hasAnyNotIn,
   jobPromise,
@@ -306,35 +308,36 @@ async function rsaImportKey(
   }, keyUsages, extractable);
 }
 
-function rsaSignVerify(key, data, { saltLength }, signature) {
-  let padding;
-  if (key.algorithm.name === 'RSA-PSS') {
-    padding = RSA_PKCS1_PSS_PADDING;
-    // TODO(@jasnell): Validate maximum size of saltLength
-    // based on the key size:
-    //   Math.ceil((keySizeInBits - 1)/8) - digestSizeInBytes - 2
-    validateInt32(saltLength, 'algorithm.saltLength', -2);
-  }
-
+async function rsaSignVerify(key, data, { saltLength }, signature) {
   const mode = signature === undefined ? kSignJobModeSign : kSignJobModeVerify;
   const type = mode === kSignJobModeSign ? 'private' : 'public';
 
   if (key.type !== type)
     throw lazyDOMException(`Key must be a ${type} key`, 'InvalidAccessError');
 
-  return jobPromise(() => new SignJob(
-    kCryptoJobAsync,
-    signature === undefined ? kSignJobModeSign : kSignJobModeVerify,
-    key[kKeyObject][kHandle],
-    undefined,
-    undefined,
-    undefined,
-    data,
-    normalizeHashName(key.algorithm.hash.name),
-    saltLength,
-    padding,
-    undefined,
-    signature));
+  return jobPromise(() => {
+    if (key.algorithm.name === 'RSA-PSS') {
+      validateInt32(
+        saltLength,
+        'algorithm.saltLength',
+        0,
+        MathCeil((key.algorithm.modulusLength - 1) / 8) - getDigestSizeInBytes(key.algorithm.hash.name) - 2);
+    }
+
+    return new SignJob(
+      kCryptoJobAsync,
+      signature === undefined ? kSignJobModeSign : kSignJobModeVerify,
+      key[kKeyObject][kHandle],
+      undefined,
+      undefined,
+      undefined,
+      data,
+      normalizeHashName(key.algorithm.hash.name),
+      saltLength,
+      key.algorithm.name === 'RSA-PSS' ? RSA_PKCS1_PSS_PADDING : undefined,
+      undefined,
+      signature);
+  });
 }
 
 

diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js
@@ -517,6 +517,15 @@ function getBlockSize(name) {
   }
 }
 
+function getDigestSizeInBytes(name) {
+  switch (name) {
+    case 'SHA-1': return 20;
+    case 'SHA-256': return 32;
+    case 'SHA-384': return 48;
+    case 'SHA-512': return 64;
+  }
+}
+
 const kKeyOps = {
   sign: 1,
   verify: 2,
@@ -596,6 +605,7 @@ module.exports = {
   bigIntArrayToUnsignedBigInt,
   bigIntArrayToUnsignedInt,
   getBlockSize,
+  getDigestSizeInBytes,
   getStringOption,
   getUsagesUnion,
   secureHeapUsed,

diff --git a/test/parallel/test-webcrypto-sign-verify-rsa.js b/test/parallel/test-webcrypto-sign-verify-rsa.js
@@ -194,6 +194,35 @@ async function testSign({
     });
 }
 
+async function testSaltLength(keyLength, hash, hLen) {
+  const { publicKey, privateKey } = await subtle.generateKey({
+    name: 'RSA-PSS',
+    modulusLength: keyLength,
+    publicExponent: new Uint8Array([1, 0, 1]),
+    hash,
+  }, false, ['sign', 'verify']);
+
+  const data = Buffer.from('Hello, world!');
+  const max = keyLength / 8 - hLen - 2;
+
+  const signature = await subtle.sign({ name: 'RSA-PSS', saltLength: max }, privateKey, data);
+  await assert.rejects(
+    subtle.sign({ name: 'RSA-PSS', saltLength: max + 1 }, privateKey, data), (err) => {
+      assert.strictEqual(err.name, 'OperationError');
+      assert.strictEqual(err.cause?.code, 'ERR_OUT_OF_RANGE');
+      assert.strictEqual(err.cause?.message, `The value of "algorithm.saltLength" is out of range. It must be >= 0 && <= ${max}. Received ${max + 1}`);
+      return true;
+    });
+  await subtle.verify({ name: 'RSA-PSS', saltLength: max }, publicKey, signature, data);
+  await assert.rejects(
+    subtle.verify({ name: 'RSA-PSS', saltLength: max + 1 }, publicKey, signature, data), (err) => {
+      assert.strictEqual(err.name, 'OperationError');
+      assert.strictEqual(err.cause?.code, 'ERR_OUT_OF_RANGE');
+      assert.strictEqual(err.cause?.message, `The value of "algorithm.saltLength" is out of range. It must be >= 0 && <= ${max}. Received ${max + 1}`);
+      return true;
+    });
+}
+
 (async function() {
   const variations = [];
 
@@ -206,5 +235,11 @@ async function testSign({
     variations.push(testSign(vector));
   });
 
+  for (const keyLength of [1024, 2048]) {
+    for (const [hash, hLen] of [['SHA-1', 20], ['SHA-256', 32], ['SHA-384', 48], ['SHA-512', 64]]) {
+      variations.push(testSaltLength(keyLength, hash, hLen));
+    }
+  }
+
   await Promise.all(variations);
 })().then(common.mustCall());