Merge pull request #413 from PeculiarVentures/fix-rsa-pss

Fix consistency in signature algorithms and add tests

package.json

@@ -19,30 +19,30 @@
     "@rollup/plugin-alias": "^3.1.9",
     "@rollup/plugin-commonjs": "^22.0.1",
     "@rollup/plugin-node-resolve": "^13.3.0",
-    "@types/mocha": "^9.1.1",
-    "@types/node": "^18.18.9",
+    "@types/mocha": "^10.0.7",
+    "@types/node": "^20.14.11",
     "@typescript-eslint/eslint-plugin": "^5.32.0",
     "@typescript-eslint/parser": "^5.32.0",
     "assert": "^2.0.0",
     "emailjs-mime-builder": "^2.0.5",
     "emailjs-mime-parser": "^2.0.7",
     "eslint": "^8.21.0",
     "eslint-plugin-deprecation": "^1.3.2",
-    "mocha": "^10.0.0",
-    "nyc": "^15.1.0",
+    "mocha": "^10.7.0",
+    "nyc": "^17.0.0",
     "rollup": "^2.77.2",
     "rollup-plugin-dts": "^4.2.2",
     "rollup-plugin-typescript2": "^0.32.1",
     "ts-node": "^10.9.1",
-    "typescript": "^5.4.5"
+    "typescript": "^5.5.3"
   },
   "dependencies": {
     "@noble/hashes": "^1.4.0",
     "asn1js": "^3.0.5",
     "bytestreamjs": "^2.0.0",
     "pvtsutils": "^1.3.2",
     "pvutils": "^1.1.3",
-    "tslib": "^2.4.0"
+    "tslib": "^2.6.3"
   },
   "engines": {
     "node": ">=12.0.0"

src/CryptoEngine/CryptoEngine.ts

@@ -1783,25 +1783,30 @@ export class CryptoEngine extends AbstractCryptoEngine {
     // Initial variables
     const signatureAlgorithm = new AlgorithmIdentifier();
 
-    //#region Get a "default parameters" for current algorithm
+    //#region Get "default parameters" for the current algorithm
     const parameters = this.getAlgorithmParameters(privateKey.algorithm.name, "sign");
     if (!Object.keys(parameters.algorithm).length) {
       throw new Error("Parameter 'algorithm' is empty");
     }
+    // Use the hash from the privateKey.algorithm.hash.name for keys with hash algorithms (like RSA)
     const algorithm = parameters.algorithm as any; // TODO remove `as any`
-    algorithm.hash.name = hashAlgorithm;
+    if ("hash" in privateKey.algorithm && privateKey.algorithm.hash && (privateKey.algorithm.hash as Algorithm).name) {
+      algorithm.hash.name = (privateKey.algorithm.hash as Algorithm).name;
+    } else {
+      algorithm.hash.name = hashAlgorithm;
+    }
     //#endregion
 
-    //#region Fill internal structures base on "privateKey" and "hashAlgorithm"
+    //#region Fill internal structures based on "privateKey" and "hashAlgorithm"
     switch (privateKey.algorithm.name.toUpperCase()) {
       case "RSASSA-PKCS1-V1_5":
       case "ECDSA":
         signatureAlgorithm.algorithmId = this.getOIDByAlgorithm(algorithm, true);
         break;
       case "RSA-PSS":
         {
-          //#region Set "saltLength" as a length (in octets) of hash function result
-          switch (hashAlgorithm.toUpperCase()) {
+          //#region Set "saltLength" as the length (in octets) of the hash function result
+          switch (algorithm.hash.name.toUpperCase()) {
             case "SHA-256":
               algorithm.saltLength = 32;
               break;
@@ -1818,8 +1823,8 @@ export class CryptoEngine extends AbstractCryptoEngine {
           //#region Fill "RSASSA_PSS_params" object
           const paramsObject: Partial<IRSASSAPSSParams> = {};
 
-          if (hashAlgorithm.toUpperCase() !== "SHA-1") {
-            const hashAlgorithmOID = this.getOIDByAlgorithm({ name: hashAlgorithm }, true, "hashAlgorithm");
+          if (algorithm.hash.name.toUpperCase() !== "SHA-1") {
+            const hashAlgorithmOID = this.getOIDByAlgorithm({ name: algorithm.hash.name }, true, "hashAlgorithm");
 
             paramsObject.hashAlgorithm = new AlgorithmIdentifier({
               algorithmId: hashAlgorithmOID,

src/SignedData.ts

@@ -847,7 +847,12 @@ export class SignedData extends PkiObject implements ISignedData {
       }
       //#endregion
 
-      const verifyResult = await crypto.verifyWithPublicKey(data, signerInfo.signature, signerCert.subjectPublicKeyInfo, signerCert.signatureAlgorithm, shaAlgorithm);
+      const signAlg = crypto.getAlgorithmByOID(signerInfo.signatureAlgorithm.algorithmId, true);
+      // In some cases, the signature algorithm may not include a hash component.
+      // In such scenarios, we utilize the hash mechanism specified in signerInfo.digestAlgorithm.
+      const verifyResult = ("hash" in signAlg)
+        ? await crypto.verifyWithPublicKey(data, signerInfo.signature, signerCert.subjectPublicKeyInfo, signerCert.signatureAlgorithm)
+        : await crypto.verifyWithPublicKey(data, signerInfo.signature, signerCert.subjectPublicKeyInfo, signerCert.signatureAlgorithm, shaAlgorithm);
 
       //#region Make a final result
 
@@ -893,9 +898,18 @@ export class SignedData extends PkiObject implements ISignedData {
     //#region Initial checking
     if (!privateKey)
       throw new Error("Need to provide a private key for signing");
+
+    const signerInfo = this.signerInfos[signerIndex];
+    if (!signerInfo) {
+      throw new RangeError("SignerInfo index is out of range");
+    }
+
     //#endregion
 
-    //#region Simple check for supported algorithm
+    //#region Adjust hashAlgorithm based on privateKey if signedAttrs are missing
+    if (!signerInfo.signedAttrs?.attributes.length && "hash" in privateKey.algorithm && "hash" in privateKey.algorithm && privateKey.algorithm.hash) {
+      hashAlgorithm = (privateKey.algorithm.hash as Algorithm).name;
+    }
     const hashAlgorithmOID = crypto.getOIDByAlgorithm({ name: hashAlgorithm }, true, "hashAlgorithm");
     //#endregion
 
@@ -907,10 +921,6 @@ export class SignedData extends PkiObject implements ISignedData {
       }));
     }
 
-    const signerInfo = this.signerInfos[signerIndex];
-    if (!signerInfo) {
-      throw new RangeError("SignerInfo index is out of range");
-    }
     signerInfo.digestAlgorithm = new AlgorithmIdentifier({
       algorithmId: hashAlgorithmOID,
       algorithmParams: new asn1js.Null()

test/cmsSignedComplexExample.spec.ts

@@ -50,5 +50,33 @@ context("CMS Signed Complex Example", () => {
     const ok = await example.verifyCMSSigned(cmsSignedBuffer);
     assert.equal(ok, true, "CMS SignedData must be verified successfully");
   });
+
+  it("should sign data using SHA-256 for message digest and SHA-1 for signature hash, then verify the signature", async () => {
+    const cms = await example.createCMSSigned("SHA-256", "RSASSA-PKCS1-V1_5", dataBuffer, false, true, "SHA-1");
+    //#region Verify that the CMS SignedData can be decoded without errors
+    assert.doesNotThrow(() => {
+      const cmsContentSimpl = pkijs.ContentInfo.fromBER(cms.cmsSignedData);
+      new pkijs.SignedData({ schema: cmsContentSimpl.content });
+    });
+    //#endregion
+
+    // Verify the CMS SignedData using the provided certificate and original data buffer
+    const result = await example.verifyCMSSigned(cms.cmsSignedData, [cms.certificate], dataBuffer);
+    assert.equal(result, true, "The CMS SignedData should be verified successfully");
+  });
+
+  it("should sign data using SHA-256 for message digest and SHA-1 for signature hash without considering the message hash, then verify the signature", async () => {
+    const cms = await example.createCMSSigned("SHA-256", "RSASSA-PKCS1-V1_5", dataBuffer, false, false, "SHA-1");
+    //#region Verify that the CMS SignedData can be decoded without errors
+    assert.doesNotThrow(() => {
+      const cmsContentSimpl = pkijs.ContentInfo.fromBER(cms.cmsSignedData);
+      new pkijs.SignedData({ schema: cmsContentSimpl.content });
+    });
+    //#endregion
+
+    // Verify the CMS SignedData using the provided certificate and original data buffer
+    const result = await example.verifyCMSSigned(cms.cmsSignedData, [cms.certificate], dataBuffer);
+    assert.equal(result, true, "The CMS SignedData should be verified successfully");
+  });
 });
 

test/cmsSignedComplexExample.ts

@@ -5,11 +5,11 @@ import * as pkijs from "../src";
 /**
  * Create CMS_Signed
  */
-export async function createCMSSigned(hashAlg: string, signAlg: string, dataBuffer: ArrayBuffer, detachedSignature = false, addExt = false) {
+export async function createCMSSigned(hashAlg: string, signAlg: string, dataBuffer: ArrayBuffer, detachedSignature = false, addExt = false, signHashAlg?: string) {
   // Get a "crypto" extension
   const crypto = pkijs.getCrypto(true);
 
-  const certWithKey = await utils.createSelfSignedCertificate(hashAlg, signAlg);
+  const certWithKey = await utils.createSelfSignedCertificate(signHashAlg || hashAlg, signAlg);
 
   //#region Initialize CMS Signed Data structures and sign it
   const cmsSignedSimpl = new pkijs.SignedData({