[PATCH 0/1] MR3620: crypt32: Fix CMS SignedData encoding.

[Torge Matthies (＠tmatthies)]

Several items here were encoded conditionally even though they aren't marked as optional in the spec.

I put the tests into the same commit as the fix because the helper function used throughout the file, check_param, does not support marking only some of the tests inside of it as todo_wine.

[PATCH 1/1] crypt32: Fix CMS SignedData encoding.

[Torge Matthies]

From: Torge Matthies

Several items here were encoded conditionally even though they aren't marked
as optional in the spec.

Signed-off-by: Torge Matthies 
---
 dlls/crypt32/encode.c    | 52 ++++++++++++++--------------------------
 dlls/crypt32/tests/msg.c | 38 +++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 34 deletions(-)

diff --git a/dlls/crypt32/encode.c b/dlls/crypt32/encode.c
index adfdad96fd9..9e362b8db21 100644
--- a/dlls/crypt32/encode.c
+++ b/dlls/crypt32/encode.c
@@ -4345,30 +4345,19 @@ static BOOL WINAPI CRYPT_AsnEncodeCMSSignerInfo(DWORD dwCertEncodingType,
 BOOL CRYPT_AsnEncodeCMSSignedInfo(CRYPT_SIGNED_INFO *signedInfo, void *pvData,
  DWORD *pcbData)
 {
+    struct DERSetDescriptor digestAlgorithmsSet = { signedInfo->cSignerInfo,
+     signedInfo->rgSignerInfo, sizeof(CMSG_CMS_SIGNER_INFO),
+     offsetof(CMSG_CMS_SIGNER_INFO, HashAlgorithm),
+     CRYPT_AsnEncodeAlgorithmIdWithNullParams };
     struct AsnEncodeSequenceItem items[7] = {
      { &signedInfo->version, CRYPT_AsnEncodeInt, 0 },
+     { &digestAlgorithmsSet, CRYPT_DEREncodeItemsAsSet, 0 },
+     { &signedInfo->content, CRYPT_AsnEncodePKCSContentInfoInternal, 0 }
     };
-    struct DERSetDescriptor digestAlgorithmsSet = { 0 }, certSet = { 0 };
-    struct DERSetDescriptor crlSet = { 0 }, signerSet = { 0 };
+    struct DERSetDescriptor certSet = { 0 }, crlSet = { 0 }, signerSet = { 0 };
     struct AsnEncodeTagSwappedItem swapped[2] = { { 0 } };
-    DWORD cItem = 1, cSwapped = 0;
-    BOOL ret = TRUE;
+    DWORD cItem = 3, cSwapped = 0;

-    if (signedInfo->cSignerInfo)
-    {
-        digestAlgorithmsSet.cItems = signedInfo->cSignerInfo;
-        digestAlgorithmsSet.items = signedInfo->rgSignerInfo;
-        digestAlgorithmsSet.itemSize = sizeof(CMSG_CMS_SIGNER_INFO);
-        digestAlgorithmsSet.itemOffset =
-         offsetof(CMSG_CMS_SIGNER_INFO, HashAlgorithm);
-        digestAlgorithmsSet.encode = CRYPT_AsnEncodeAlgorithmIdWithNullParams;
-        items[cItem].pvStructInfo = &digestAlgorithmsSet;
-        items[cItem].encodeFunc = CRYPT_DEREncodeItemsAsSet;
-        cItem++;
-    }
-    items[cItem].pvStructInfo = &signedInfo->content;
-    items[cItem].encodeFunc = CRYPT_AsnEncodePKCSContentInfoInternal;
-    cItem++;
     if (signedInfo->cCertEncoded)
     {
         certSet.cItems = signedInfo->cCertEncoded;
@@ -4399,22 +4388,17 @@ BOOL CRYPT_AsnEncodeCMSSignedInfo(CRYPT_SIGNED_INFO *signedInfo, void *pvData,
         cSwapped++;
         cItem++;
     }
-    if (ret && signedInfo->cSignerInfo)
-    {
-        signerSet.cItems = signedInfo->cSignerInfo;
-        signerSet.items = signedInfo->rgSignerInfo;
-        signerSet.itemSize = sizeof(CMSG_CMS_SIGNER_INFO);
-        signerSet.itemOffset = 0;
-        signerSet.encode = CRYPT_AsnEncodeCMSSignerInfo;
-        items[cItem].pvStructInfo = &signerSet;
-        items[cItem].encodeFunc = CRYPT_DEREncodeItemsAsSet;
-        cItem++;
-    }
-    if (ret)
-        ret = CRYPT_AsnEncodeSequence(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
-         items, cItem, 0, NULL, pvData, pcbData);
+    signerSet.cItems = signedInfo->cSignerInfo;
+    signerSet.items = signedInfo->rgSignerInfo;
+    signerSet.itemSize = sizeof(CMSG_CMS_SIGNER_INFO);
+    signerSet.itemOffset = 0;
+    signerSet.encode = CRYPT_AsnEncodeCMSSignerInfo;
+    items[cItem].pvStructInfo = &signerSet;
+    items[cItem].encodeFunc = CRYPT_DEREncodeItemsAsSet;
+    cItem++;

-    return ret;
+    return CRYPT_AsnEncodeSequence(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+     items, cItem, 0, NULL, pvData, pcbData);
 }

static BOOL WINAPI CRYPT_AsnEncodeRecipientInfo(DWORD dwCertEncodingType,
diff --git a/dlls/crypt32/tests/msg.c b/dlls/crypt32/tests/msg.c
index 8b550ee8b88..fa48d978e17 100644
--- a/dlls/crypt32/tests/msg.c
+++ b/dlls/crypt32/tests/msg.c
@@ -1256,6 +1256,18 @@ static void test_signed_msg_update(void)
      CRYPT_DELETEKEYSET);
 }

+static const BYTE unsignedEmptyBareContent[] = {
+0x30,0x0b,0x02,0x01,0x01,0x31,0x00,0x30,0x02,0x06,0x00,0x31,0x00 };
+static const BYTE unsignedEmptyContent[] = {
+0x30,0x1a,0x06,0x09,0x2a,0x86,0x48,0x86,0xf7,0x0d,0x01,0x07,0x02,0xa0,0x0d,
+0x30,0x0b,0x02,0x01,0x01,0x31,0x00,0x30,0x02,0x06,0x00,0x31,0x00 };
+static const BYTE detachedUnsignedBareContent[] = {
+0x30,0x14,0x02,0x01,0x01,0x31,0x00,0x30,0x0b,0x06,0x09,0x2a,0x86,0x48,0x86,
+0xf7,0x0d,0x01,0x07,0x01,0x31,0x00 };
+static const BYTE detachedUnsignedContent[] = {
+0x30,0x23,0x06,0x09,0x2a,0x86,0x48,0x86,0xf7,0x0d,0x01,0x07,0x02,0xa0,0x16,
+0x30,0x14,0x02,0x01,0x01,0x31,0x00,0x30,0x0b,0x06,0x09,0x2a,0x86,0x48,0x86,
+0xf7,0x0d,0x01,0x07,0x01,0x31,0x00 };
 static const BYTE signedEmptyBareContent[] = {
 0x30,0x50,0x02,0x01,0x01,0x31,0x0e,0x30,0x0c,0x06,0x08,0x2a,0x86,0x48,0x86,
 0xf7,0x0d,0x02,0x05,0x05,0x00,0x30,0x02,0x06,0x00,0x31,0x37,0x30,0x35,0x02,
@@ -1590,6 +1602,32 @@ static void test_signed_msg_encoding(void)
     HCRYPTKEY key;
     DWORD size;

+    msg = CryptMsgOpenToEncode(PKCS_7_ASN_ENCODING,
+     CMSG_DETACHED_FLAG, CMSG_SIGNED, &signInfo, NULL, NULL);
+    ok(msg != NULL, "CryptMsgOpenToEncode failed: %lx\n", GetLastError());
+
+    check_param("detached unsigned empty bare content", msg,
+     CMSG_BARE_CONTENT_PARAM, unsignedEmptyBareContent,
+     sizeof(unsignedEmptyBareContent));
+    check_param("detached unsigned empty content", msg, CMSG_CONTENT_PARAM,
+     unsignedEmptyContent, sizeof(unsignedEmptyContent));
+    ret = CryptMsgUpdate(msg, msgData, sizeof(msgData), TRUE);
+    ok(ret, "CryptMsgUpdate failed: %lx\n", GetLastError());
+    check_param("detached unsigned bare content", msg, CMSG_BARE_CONTENT_PARAM,
+     detachedUnsignedBareContent, sizeof(detachedUnsignedBareContent));
+    check_param("detached unsigned content", msg, CMSG_CONTENT_PARAM,
+     detachedUnsignedContent, sizeof(detachedUnsignedContent));
+    SetLastError(0xdeadbeef);
+    ret = CryptMsgGetParam(msg, CMSG_COMPUTED_HASH_PARAM, 0, NULL, &size);
+    ok(!ret && GetLastError() == CRYPT_E_INVALID_INDEX,
+     "Expected CRYPT_E_INVALID_INDEX, got %lx\n", GetLastError());
+    SetLastError(0xdeadbeef);
+    ret = CryptMsgGetParam(msg, CMSG_ENCODED_SIGNER, 0, NULL, &size);
+    ok(!ret && GetLastError() == CRYPT_E_INVALID_INDEX,
+     "Expected CRYPT_E_INVALID_INDEX, got %lx\n", GetLastError());
+
+    CryptMsgClose(msg);
+
     certInfo.SerialNumber.cbData = sizeof(serialNum);
     certInfo.SerialNumber.pbData = serialNum;
     certInfo.Issuer.cbData = sizeof(encodedCommonName);