crypt32: CertComparePublicKeyInfo must ignore the leading zero

[Michael Karcher]

As the notes for RSA_CSP_PUBLICKEYBLOB on MSDN describe, the modulus
of an RSA public key *might* have a leading zero in the ASN.1 encoding.
CertComparePublicKeyInfo must not treat public keys which differ only
in the presence of the leading zero different, if the dwCertEncodingType
indicates an ASN.1 encoded public key.

A test for the behaviour of CertComparePublicKeyInfo with X509_ASN_ENCODING
is included. The testcases pass with this patch in wine as well as
cross-compiled on Windows XP SP3.

This patch makes makecert.exe from the Microsoft SDK/DDK run.
---
 dlls/crypt32/cert.c       |   63 ++++++++++++++++++++++++++++++++++++++------
 dlls/crypt32/tests/cert.c |   22 +++++++++++++++
 2 files changed, 76 insertions(+), 9 deletions(-)

diff --git a/dlls/crypt32/cert.c b/dlls/crypt32/cert.c
index 41d3998..3eb9e2d 100644
--- a/dlls/crypt32/cert.c
+++ b/dlls/crypt32/cert.c
@@ -787,18 +787,63 @@ BOOL WINAPI CertComparePublicKeyInfo(DWORD dwCertEncodingType,
     BOOL ret;
 
     TRACE("(%08x, %p, %p)\n", dwCertEncodingType, pPublicKey1, pPublicKey2);
-
-    if (pPublicKey1->PublicKey.cbData == pPublicKey2->PublicKey.cbData &&
-     pPublicKey1->PublicKey.cUnusedBits == pPublicKey2->PublicKey.cUnusedBits)
+    
+    switch (GET_CERT_ENCODING_TYPE(dwCertEncodingType))
     {
-        if (pPublicKey2->PublicKey.cbData)
-            ret = !memcmp(pPublicKey1->PublicKey.pbData,
-             pPublicKey2->PublicKey.pbData, pPublicKey1->PublicKey.cbData);
+    case 0:	/* Seems to mean "raw binary bits" */
+        if (pPublicKey1->PublicKey.cbData == pPublicKey2->PublicKey.cbData &&
+         pPublicKey1->PublicKey.cUnusedBits == pPublicKey2->PublicKey.cUnusedBits)
+        {
+          if (pPublicKey2->PublicKey.cbData)
+              ret = !memcmp(pPublicKey1->PublicKey.pbData,
+               pPublicKey2->PublicKey.pbData, pPublicKey1->PublicKey.cbData);
+          else
+              ret = TRUE;
+        }
         else
-            ret = TRUE;
-    }
-    else
+            ret = FALSE;
+        break;
+    default:
+        WARN("Unknown encoding type %08x\n", dwCertEncodingType);
+        /* FALLTHROUGH */
+    case X509_ASN_ENCODING:
+    {
+        BLOBHEADER *pblob1, *pblob2;
+        DWORD length;
         ret = FALSE;
+        if (CryptDecodeObject(dwCertEncodingType, RSA_CSP_PUBLICKEYBLOB,
+                    pPublicKey1->PublicKey.pbData, pPublicKey1->PublicKey.cbData,
+                    0, NULL, &length))
+        {
+            pblob1 = CryptMemAlloc(length);
+            if (CryptDecodeObject(dwCertEncodingType, RSA_CSP_PUBLICKEYBLOB,
+                    pPublicKey1->PublicKey.pbData, pPublicKey1->PublicKey.cbData,
+                    0, pblob1, &length))
+            {
+                if (CryptDecodeObject(dwCertEncodingType, RSA_CSP_PUBLICKEYBLOB,
+                            pPublicKey2->PublicKey.pbData, pPublicKey2->PublicKey.cbData,
+                            0, NULL, &length))
+                {
+                    pblob2 = CryptMemAlloc(length);
+                    if (CryptDecodeObject(dwCertEncodingType, RSA_CSP_PUBLICKEYBLOB,
+                            pPublicKey1->PublicKey.pbData, pPublicKey1->PublicKey.cbData,
+                            0, pblob2, &length))
+                    {
+                        /* The RSAPUBKEY structure directly follows the BLOBHEADER */
+                        RSAPUBKEY *pk1 = (LPVOID)(pblob1 + 1), 
+                                  *pk2 = (LPVOID)(pblob2 + 1);
+                        ret = (pk1->bitlen == pk2->bitlen) && (pk1->pubexp == pk2->pubexp)
+                                 && !memcmp(pk1 + 1, pk2 + 1, pk1->bitlen/8);
+                    }
+                    CryptMemFree(pblob2);
+                }
+            }
+            CryptMemFree(pblob1);
+        }
+                              
+        break;
+    }
+    }
     return ret;
 }
 
diff --git a/dlls/crypt32/tests/cert.c b/dlls/crypt32/tests/cert.c
index 3fce85e..5acdc0e 100644
--- a/dlls/crypt32/tests/cert.c
+++ b/dlls/crypt32/tests/cert.c
@@ -2437,6 +2437,8 @@ static void testComparePublicKeyInfo(void)
     static BYTE bits1[] = { 1, 0 };
     static BYTE bits2[] = { 0 };
     static BYTE bits3[] = { 1 };
+    static BYTE bits4[] = { 0x30,8, 2,1,0x81, 2,3,1,0,1 };
+    static BYTE bits5[] = { 0x30,9, 2,2,0,0x81, 2,3,1,0,1 };
 
     /* crashes
     ret = CertComparePublicKeyInfo(0, NULL, NULL);
@@ -2460,6 +2462,26 @@ static void testComparePublicKeyInfo(void)
     info2.PublicKey.cUnusedBits = 0;
     ret = CertComparePublicKeyInfo(0, &info1, &info2);
     ok(ret, "CertComparePublicKeyInfo failed: %08x\n", GetLastError());
+    info2.Algorithm.pszObjId = oid_rsa_rsa;
+    info1.PublicKey.cbData = sizeof(bits4);
+    info1.PublicKey.pbData = bits4;
+    info1.PublicKey.cUnusedBits = 0;
+    info2.PublicKey.cbData = sizeof(bits5);
+    info2.PublicKey.pbData = bits5;
+    info2.PublicKey.cUnusedBits = 0;
+    ret = CertComparePublicKeyInfo(0, &info1, &info2);
+    ok(!ret, "CertComparePublicKeyInfo: as raw binary: keys should be unequal\n");
+    ret = CertComparePublicKeyInfo(X509_ASN_ENCODING, &info1, &info2);
+    ok(ret, "CertComparePublicKeyInfo: as ASN.1 encoded: keys should be equal\n");
+    info1.PublicKey.cUnusedBits = 1;
+    info2.PublicKey.cUnusedBits = 5;
+    ret = CertComparePublicKeyInfo(X509_ASN_ENCODING, &info1, &info2);
+    ok(ret, "CertComparePublicKeyInfo: ASN.1 encoding should ignore cUnusedBits\n");
+    info1.PublicKey.cUnusedBits = 0;
+    info2.PublicKey.cUnusedBits = 0;
+    info1.PublicKey.cbData--; /* kill one byte, make ASN.1 encoded data invalid */
+    ret = CertComparePublicKeyInfo(X509_ASN_ENCODING, &info1, &info2);
+    ok(!ret, "CertComparePublicKeyInfo: comparing bad ASN.1 encoded key should fail\n");
     /* Even though they compare in their used bits, these do not compare */
     info1.PublicKey.cbData = sizeof(bits2);
     info1.PublicKey.pbData = bits2;
-- 
1.5.5.1