Merged PR 7272939: Return SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE from SymCryptRsaPssVerify

+ Return SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE from SymCryptRsaPssVerify
+ Also change tests to alias the error codes so that
  multi-implementation test with SymCrypt and CNG do not break while the
  change to SymCrypt rolls out.
+ BCrypt callers should already handle STATUS_INVALID_SIGNATURE as this
  is the error code that they should expect from documentation

Related work items: #39175561

Author: samuel-lee-msft

inc/symcrypt.h

@@ -6329,7 +6329,7 @@ SymCryptRsaPssVerify(
 // pbHashValue) with the pkRsakey key using RSA PSS. The signature is input
 // in the pbSignature buffer.
 //
-// It returns SYMCRYPT_NO_ERROR if the verification suceeded or SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE
+// It returns SYMCRYPT_NO_ERROR if the verification succeeded or SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE
 // if it failed.
 //
 // nfSignature is the number format of the signature (i.e. the pbSignature buffer). Currently

lib/rsa_padding.c

@@ -1041,7 +1041,7 @@ SymCryptRsaPssVerifySignaturePadding(
     {
         if (pbPSSFormat[0] != 0)
         {
-            scError = SYMCRYPT_INVALID_ARGUMENT;
+            scError = SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE;
             goto cleanup;
         }
         pbPSSFormat++;
@@ -1063,7 +1063,7 @@ SymCryptRsaPssVerifySignaturePadding(
         pbPSSFormat[cbPSSFormat - 1] != 0xbc
         )
     {
-        scError = SYMCRYPT_INVALID_ARGUMENT;
+        scError = SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE;
         goto cleanup;
     }
 
@@ -1103,15 +1103,15 @@ SymCryptRsaPssVerifySignaturePadding(
     {
         if (pbDBMask[i] != 0x00)
         {
-            scError = SYMCRYPT_INVALID_ARGUMENT;
+            scError = SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE;
             goto cleanup;
         }
     }
 
     // ensure the 0x01 byte is part of DB
     if (pbDBMask[cbDB - cbSalt - 1] != 0x01)
     {
-        scError = SYMCRYPT_INVALID_ARGUMENT;
+        scError = SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE;
         goto cleanup;
     }
 
@@ -1127,7 +1127,7 @@ SymCryptRsaPssVerifySignaturePadding(
 
     if ( !SymCryptEqual(pbPSSFormat + cbDB, pbMPrimeHash, cbHashAlg) )
     {
-        scError = SYMCRYPT_INVALID_ARGUMENT;
+        scError = SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE;
         goto cleanup;
     }
 

unittest/lib/cng_implementations.cpp

@@ -1823,7 +1823,7 @@ RsaSignImp<ImpCng, AlgRsaSignPss>::verify(
     BCRYPT_PSS_PADDING_INFO paddingInfo;
     PCCNG_HASH_INFO pInfo;
 
-     pInfo = getHashInfo( pcstrHashAlgName);
+    pInfo = getHashInfo( pcstrHashAlgName);
     paddingInfo.pszAlgId = pInfo->wideName;
     paddingInfo.cbSalt = u32Other;
 
@@ -1836,6 +1836,17 @@ RsaSignImp<ImpCng, AlgRsaSignPss>::verify(
         (UINT32)cbSig,
         BCRYPT_PAD_PSS );
 
+    // saml 2022/04:
+    // In order to update error message returned from SymCryptRsaPssVerify and not break
+    // multi-implementation test of SymCrypt vs. CNG, we must map STATUS_INVALID_SIGNATURE
+    // to STATUS_INVALID_PARAMETER for now.
+    // Once both CNG and SymCrypt are updated reliably we can reintroduce testing that the two
+    // error responses cohere - but for now they won't.
+    if( ntStatus == STATUS_INVALID_SIGNATURE )
+    {
+        ntStatus = STATUS_INVALID_PARAMETER;
+    }
+
     return ntStatus;
 }
 

unittest/lib/sc_implementations.cpp

@@ -4440,9 +4440,13 @@ RsaSignImp<ImpSc, AlgRsaSignPss>::verify(
     case SYMCRYPT_NO_ERROR:
         ntStatus = STATUS_SUCCESS;
         break;
+        // saml 2022/04:
+        // In order to update error message returned from SymCryptRsaPssVerify and not break
+        // multi-implementation test of SymCrypt vs. CNG, we must map SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE
+        // to STATUS_INVALID_PARAMETER rather than STATUS_INVALID_SIGNATURE for now.
+        // Once both CNG and SymCrypt are updated reliably we can reintroduce testing that the two
+        // error responses cohere - but for now they won't.
     case SYMCRYPT_SIGNATURE_VERIFICATION_FAILURE:
-        ntStatus = STATUS_INVALID_SIGNATURE;
-        break;
     case SYMCRYPT_INVALID_ARGUMENT:
         ntStatus = STATUS_INVALID_PARAMETER;
         break;