Merge pull request #4002 from kabuobeid/smime_fixes

S/MIME: Canonicalize multi-part messages before hashing.  Improve error checking in wc_MIME_parse_headers.

certs/test/include.am

@@ -48,6 +48,7 @@ EXTRA_DIST += \
          certs/test/server-localhost.der \
          certs/test/server-localhost.pem \
          certs/test/smime-test.p7s \
+         certs/test/smime-test-canon.p7s \
          certs/test/smime-test-multipart.p7s \
          certs/test/smime-test-multipart-badsig.p7s \
          certs/crl/server-goodaltCrl.pem \

certs/test/smime-test-canon.p7s

@@ -0,0 +1,63 @@
+MIME-Version: 1.0
+Content-Type: multipart/signed; protocol="application/x-pkcs7-signature"; micalg="sha-256"; boundary="----7DB22CE8F405BFCE81603E2CD3794291"
+
+This is an S/MIME signed message
+
+------7DB22CE8F405BFCE81603E2CD3794291
+Content-Type: text/plain
+
+
+
+This text/plain content has been signed with canonicalized CRLF line endings,
+
+but it is added to the multipart message with non canonicalized line endings.
+
+
+
+In order to match the hash, it will need to be canonicalized first.
+------7DB22CE8F405BFCE81603E2CD3794291
+Content-Type: application/x-pkcs7-signature; name="smime.p7s"
+Content-Transfer-Encoding: base64
+Content-Disposition: attachment; filename="smime.p7s"
+
+MIIH3gYJKoZIhvcNAQcCoIIHzzCCB8sCAQExDzANBglghkgBZQMEAgEFADALBgkq
+hkiG9w0BBwGgggTtMIIE6TCCA9GgAwIBAgIJAKrTP6wYCjdNMA0GCSqGSIb3DQEB
+CwUAMIGUMQswCQYDVQQGEwJVUzEQMA4GA1UECAwHTW9udGFuYTEQMA4GA1UEBwwH
+Qm96ZW1hbjERMA8GA1UECgwIU2F3dG9vdGgxEzARBgNVBAsMCkNvbnN1bHRpbmcx
+GDAWBgNVBAMMD3d3dy53b2xmc3NsLmNvbTEfMB0GCSqGSIb3DQEJARYQaW5mb0B3
+b2xmc3NsLmNvbTAeFw0yMTAyMTAxOTQ5NTJaFw0yMzExMDcxOTQ5NTJaMIGUMQsw
+CQYDVQQGEwJVUzEQMA4GA1UECAwHTW9udGFuYTEQMA4GA1UEBwwHQm96ZW1hbjER
+MA8GA1UECgwIU2F3dG9vdGgxEzARBgNVBAsMCkNvbnN1bHRpbmcxGDAWBgNVBAMM
+D3d3dy53b2xmc3NsLmNvbTEfMB0GCSqGSIb3DQEJARYQaW5mb0B3b2xmc3NsLmNv
+bTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL8Myi0Ush6EQlvNOB9K
+8k11EPG2NZ/fyn0DmNOs3gNm7irx2LB9bgdUCxCYIU2AyxIg58xP3kV9yXJ3MurK
+kLtpUhADL6jzlcXxi2JWG+9nb6QQQZWtCpvjpcCw0nB2UDBbqOgILHztp6J6jTgp
+HKzH7fJ8lbCVgn1JXDjNdyXvvYB1U5Q8PcpjW58VtdMdEy8Z0TzbdjrMuH3J5cLX
+2kBv2CHccxtCLVOc/hr8fat6Nj+Y3oR8BWfOahQ4h6nxjLVoy2h/cSAr9aBj9VYv
+oybSt2+xWhfXOJkI/pNYb/7DE0kIFgunTWcAUjFnI06Y7VFFHbkE2Qvs2CizS73t
+NnkCAwEAAaOCATowggE2MB0GA1UdDgQWBBQnjmcRdMMmHT/tM2OzpNgdMOXo1TCB
+yQYDVR0jBIHBMIG+gBQnjmcRdMMmHT/tM2OzpNgdMOXo1aGBmqSBlzCBlDELMAkG
+A1UEBhMCVVMxEDAOBgNVBAgMB01vbnRhbmExEDAOBgNVBAcMB0JvemVtYW4xETAP
+BgNVBAoMCFNhd3Rvb3RoMRMwEQYDVQQLDApDb25zdWx0aW5nMRgwFgYDVQQDDA93
+d3cud29sZnNzbC5jb20xHzAdBgkqhkiG9w0BCQEWEGluZm9Ad29sZnNzbC5jb22C
+CQCq0z+sGAo3TTAMBgNVHRMEBTADAQH/MBwGA1UdEQQVMBOCC2V4YW1wbGUuY29t
+hwR/AAABMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjANBgkqhkiG9w0B
+AQsFAAOCAQEAYpjIWM9WA4ZbG3FJfQUDXeAIhq3bSt6rIpaow1lowTeQQN+9idC8
+2o7vh7LCYlLhGikXapaZyE7YMv640Vw7CsI8X6EemH/OiSYhH2ScFXqc7/sdhWr6
+mM6oqavDosDrh+28Id/zB1uu/UDUriDQdooxCqJifGENzl2aHuQgiFFJ+3epzU3G
+v1SZM+9LoHNwbS7ZPQj2EjkxaMZhXEG1G/Q4ffy+c2Yt98pbLFsxqs/2fzDkEiyO
+1jhR5kXu1drDg9btXuzWthSzk1nhVUp/BN/OZdTfGE/dtEV/plYwxAVEmJ1PJm2E
+gKBe7SPRSIcOBQaRO7A8u4yPPHtMT6HKmDGCArUwggKxAgEBMIGiMIGUMQswCQYD
+VQQGEwJVUzEQMA4GA1UECAwHTW9udGFuYTEQMA4GA1UEBwwHQm96ZW1hbjERMA8G
+A1UECgwIU2F3dG9vdGgxEzARBgNVBAsMCkNvbnN1bHRpbmcxGDAWBgNVBAMMD3d3
+dy53b2xmc3NsLmNvbTEfMB0GCSqGSIb3DQEJARYQaW5mb0B3b2xmc3NsLmNvbQIJ
+AKrTP6wYCjdNMA0GCWCGSAFlAwQCAQUAoIHkMBgGCSqGSIb3DQEJAzELBgkqhkiG
+9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTIxMDQzMDIyMDIxNVowLwYJKoZIhvcNAQkE
+MSIEIK/ck17trnj+dm7BHjYDU6Vr84PBHws8jt5J5kmOBzeFMHkGCSqGSIb3DQEJ
+DzFsMGowCwYJYIZIAWUDBAEqMAsGCWCGSAFlAwQBFjALBglghkgBZQMEAQIwCgYI
+KoZIhvcNAwcwDgYIKoZIhvcNAwICAgCAMA0GCCqGSIb3DQMCAgFAMAcGBSsOAwIH
+MA0GCCqGSIb3DQMCAgEoMA0GCSqGSIb3DQEBAQUABIIBAIpnnr7ts2RcUhyYH0g8
+H55w+mgaX+BEZ6dPbR0Db3VIwueDXhsyyp+JQtfRo4DUPgHQtGKFFk5PsAnv122l
+dMEdEx90hwtzdM1GAg/8PIMJnsKFlz+Yc2F6WsTGdCKasCpr4Tz/UiiztdCi3eJn
+UGGlChnuZgUS0LRRNfSmZGdnAjVdk8yammEUFclS09z+t9NT+2UIMHeZWfsMRSit
+fHjnvzbMtxF32uRbZdALMV2b8nW56y6al+/lMvj0074l4VKmKuH3Vj/EnC2PQ9xl

src/ssl.c

@@ -54446,11 +54446,14 @@ WOLFSSL_API PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in,
     int sectionLen = 0;
     int ret = -1;
     char* section = NULL;
+    char* canonLine = NULL;
+    char* canonSection = NULL;
     PKCS7* pkcs7 = NULL;
     word32 outLen = 0;
     byte* out = NULL;
     byte* outHead = NULL;
 
+    int canonPos = 0;
     int lineLen = 0;
     int remainLen = 0;
     byte isEnd = 0;
@@ -54478,8 +54481,7 @@ WOLFSSL_API PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in,
         goto error;
     }
 
-    section = (char*)XMALLOC((remainLen+1)*sizeof(char), NULL,
+    section = (char*)XMALLOC(remainLen+1, NULL, DYNAMIC_TYPE_PKCS7);
-                             DYNAMIC_TYPE_PKCS7);
     if (section == NULL) {
         goto error;
     }
@@ -54526,12 +54528,11 @@ WOLFSSL_API PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in,
             }
 
             boundLen = XSTRLEN(curParam->value) + 2;
-            boundary = (char*)XMALLOC((boundLen+1)*sizeof(char), NULL,
+            boundary = (char*)XMALLOC(boundLen+1, NULL, DYNAMIC_TYPE_PKCS7);
-                                      DYNAMIC_TYPE_PKCS7);
             if (boundary == NULL) {
                 goto error;
             }
-            XMEMSET(boundary, 0, (word32)((boundLen+1)*sizeof(char)));
+            XMEMSET(boundary, 0, (word32)(boundLen+1));
             boundary[0] = boundary[1] = '-';
             XSTRNCPY(&boundary[2], curParam->value, boundLen-2);
 
@@ -54553,38 +54554,59 @@ WOLFSSL_API PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in,
 
             section[0] = '\0';
             sectionLen = 0;
+            canonSection = (char*)XMALLOC(remainLen+1, NULL,
+                                          DYNAMIC_TYPE_PKCS7);
+            if (canonSection == NULL) {
+                goto error;
+            }
+
             lineLen = wolfSSL_BIO_gets(in, section, remainLen);
             while(XSTRNCMP(&section[sectionLen], boundary, boundLen) &&
                            remainLen > 0) {
+                canonLine = wc_MIME_canonicalize(&section[sectionLen]);
+                if (canonLine == NULL) {
+                    goto error;
+                }
+                XMEMCPY(&canonSection[canonPos], canonLine,
+                        (int)XSTRLEN(canonLine));
+                canonPos += XSTRLEN(canonLine);
+                XFREE(canonLine, NULL, DYNAMIC_TYPE_PKCS7);
+                canonLine = NULL;
+
                 sectionLen += lineLen;
                 remainLen -= lineLen;
+
                 lineLen = wolfSSL_BIO_gets(in, &section[sectionLen],
                                            remainLen);
                 if (lineLen <= 0) {
                     goto error;
                 }
             }
-            sectionLen--;
+
+            canonPos--;
             /* Strip the final trailing newline.  Support \r, \n or \r\n. */
-            if (section[sectionLen] == '\n') {
+            if (canonSection[canonPos] == '\n') {
-                sectionLen--;
+                canonPos--;
-                if (section[sectionLen] == '\r') {
+                if (canonSection[canonPos] == '\r') {
-                    sectionLen--;
+                    canonPos--;
                 }
             }
-            else if (section[sectionLen] == '\r') {
+            else if (canonSection[canonPos] == '\r') {
-                sectionLen--;
+                canonPos--;
             }
-            section[sectionLen+1] = '\0';
+            canonSection[canonPos+1] = '\0';
 
             *bcont = wolfSSL_BIO_new(wolfSSL_BIO_s_mem());
-            ret = wolfSSL_BIO_write(*bcont, section, (int)XSTRLEN(section));
+            ret = wolfSSL_BIO_write(*bcont, canonSection,
-            if (ret != (int)XSTRLEN(section)) {
+                                    (int)XSTRLEN(canonSection));
+            if (ret != (int)XSTRLEN(canonSection)) {
                 goto error;
             }
             if ((bcontMemSz = wolfSSL_BIO_get_mem_data(*bcont, &bcontMem)) < 0) {
                 goto error;
             }
+            XFREE(canonSection, NULL, DYNAMIC_TYPE_PKCS7);
+            canonSection = NULL;
 
 
             wc_MIME_free_hdrs(allHdrs);
@@ -54641,6 +54663,7 @@ WOLFSSL_API PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in,
             }
 
             XFREE(boundary, NULL, DYNAMIC_TYPE_PKCS7);
+            boundary = NULL;
         }
     }
     else if (curHdr && (!XSTRNCMP(curHdr->body, kAppPkcs7Mime,
@@ -54709,6 +54732,8 @@ error:
     XFREE(boundary, NULL, DYNAMIC_TYPE_PKCS7);
     XFREE(outHead, NULL, DYNAMIC_TYPE_PKCS7);
     XFREE(section, NULL, DYNAMIC_TYPE_PKCS7);
+    if (canonSection != NULL)
+        XFREE(canonSection, NULL, DYNAMIC_TYPE_PKCS7);
     wolfSSL_BIO_free(*bcont);
 
     return NULL;

tests/api.c

@@ -39333,6 +39333,15 @@ static void test_wolfSSL_SMIME_read_PKCS7(void)
     pkcs7 = wolfSSL_SMIME_read_PKCS7(bio, &bcont);
     AssertNull(pkcs7);
     AssertIntEQ(wolfSSL_PKCS7_verify(pkcs7, NULL, NULL, bcont, NULL, PKCS7_NOVERIFY), SSL_FAILURE);
+    XFCLOSE(smimeTestFile);
+    if (bcont) BIO_free(bcont);
+    wolfSSL_PKCS7_free(pkcs7);
+
+    smimeTestFile = XFOPEN("./certs/test/smime-test-canon.p7s", "r");
+    AssertIntEQ(wolfSSL_BIO_set_fp(bio, smimeTestFile, BIO_CLOSE), SSL_SUCCESS);
+    pkcs7 = wolfSSL_SMIME_read_PKCS7(bio, &bcont);
+    AssertNotNull(pkcs7);
+    AssertIntEQ(wolfSSL_PKCS7_verify(pkcs7, NULL, NULL, bcont, NULL, PKCS7_NOVERIFY), SSL_SUCCESS);
     BIO_free(bio);
     if (bcont) BIO_free(bcont);
     wolfSSL_PKCS7_free(pkcs7);

wolfcrypt/src/asn.c

@@ -18547,13 +18547,17 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers)
         }
         start = end = 0;
         lineLen = XSTRLEN(curLine);
+        if (lineLen == 0) {
+            ret = BAD_FUNC_ARG;
+            goto error;
+        }
 
         for (pos = 0; pos < lineLen; pos++) {
             char cur = curLine[pos];
 
             if (mimeStatus == MIME_NAMEATTR && ((cur == ':' &&
                 mimeType == MIME_HDR) || (cur == '=' &&
-                mimeType == MIME_PARAM))) {
+                mimeType == MIME_PARAM)) && pos >= 1) {
                 mimeStatus = MIME_BODYVAL;
                 end = pos-1;
                 ret = wc_MIME_header_strip(curLine, &nameAttr, start, end);
@@ -18562,7 +18566,7 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers)
                 }
                 start = pos+1;
             }
-            else if (mimeStatus == MIME_BODYVAL && cur == ';') {
+            else if (mimeStatus == MIME_BODYVAL && cur == ';' && pos >= 1) {
                 end = pos-1;
                 ret = wc_MIME_header_strip(curLine, &bodyVal, start, end);
                 if (ret) {
@@ -18570,7 +18574,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers)
                 }
                 if (mimeType == MIME_HDR) {
                     nextHdr->name = nameAttr;
+                    nameAttr = NULL;
                     nextHdr->body = bodyVal;
+                    bodyVal = NULL;
                     nextHdr->next = curHdr;
                     curHdr = nextHdr;
                     nextHdr = (MimeHdr*)XMALLOC(sizeof(MimeHdr), NULL,
@@ -18583,7 +18589,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers)
                 }
                 else {
                     nextParam->attribute = nameAttr;
+                    nameAttr = NULL;
                     nextParam->value = bodyVal;
+                    bodyVal = NULL;
                     nextParam->next = curHdr->params;
                     curHdr->params = nextParam;
                     nextParam = (MimeParam*)XMALLOC(sizeof(MimeParam), NULL,
@@ -18612,7 +18620,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers)
             }
             if (mimeType == MIME_HDR) {
                 nextHdr->name = nameAttr;
+                nameAttr = NULL;
                 nextHdr->body = bodyVal;
+                bodyVal = NULL;
                 nextHdr->next = curHdr;
                 curHdr = nextHdr;
                 nextHdr = (MimeHdr*)XMALLOC(sizeof(MimeHdr), NULL,
@@ -18624,7 +18634,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers)
                 XMEMSET(nextHdr, 0, (word32)sizeof(MimeHdr));
             } else {
                 nextParam->attribute = nameAttr;
+                nameAttr = NULL;
                 nextParam->value = bodyVal;
+                bodyVal = NULL;
                 nextParam->next = curHdr->params;
                 curHdr->params = nextParam;
                 nextParam = (MimeParam*)XMALLOC(sizeof(MimeParam), NULL,
@@ -18650,8 +18662,10 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers)
 error:
     wc_MIME_free_hdrs(curHdr);
     wc_MIME_free_hdrs(nextHdr);
-    XFREE(nameAttr, NULL, DYNAMIC_TYPE_PKCS7);
+    if (nameAttr != NULL)
-    XFREE(bodyVal, NULL, DYNAMIC_TYPE_PKCS7);
+        XFREE(nameAttr, NULL, DYNAMIC_TYPE_PKCS7);
+    if (bodyVal != NULL)
+        XFREE(bodyVal, NULL, DYNAMIC_TYPE_PKCS7);
     XFREE(nextParam, NULL, DYNAMIC_TYPE_PKCS7);
 
     return ret;
@@ -18742,6 +18756,41 @@ MimeParam* wc_MIME_find_param_attr(const char* attribute,
     return param;
 }
 
+/*****************************************************************************
+* wc_MIME_canonicalize - Canonicalize a line by converting all line endings
+* to CRLF.
+*
+* RETURNS:
+* returns a pointer to a canonicalized line on success, NULL on error.
+*/
+char* wc_MIME_canonicalize(const char* line)
+{
+    size_t end = 0;
+    char* canonLine = NULL;
+
+    if (line == NULL || XSTRLEN(line) == 0) {
+        return NULL;
+    }
+
+    end = XSTRLEN(line);
+    while (end >= 1 && ((line[end-1] == '\r') || (line[end-1] == '\n'))) {
+        end--;
+    }
+
+    /* Need 2 chars for \r\n and 1 for EOL */
+    canonLine = (char*)XMALLOC((end+3)*sizeof(char), NULL, DYNAMIC_TYPE_PKCS7);
+    if (canonLine == NULL) {
+        return NULL;
+    }
+
+    XSTRNCPY(canonLine, line, end);
+    canonLine[end] = '\r';
+    canonLine[end+1] = '\n';
+    canonLine[end+2] = '\0';
+
+    return canonLine;
+}
+
 /*****************************************************************************
 * wc_MIME_free_hdrs - Frees all MIME headers, parameters and strings starting from
 * the provided header pointer.

wolfssl/wolfcrypt/asn.h

@@ -1295,6 +1295,7 @@ WOLFSSL_LOCAL int wc_MIME_create_header(char* name, char* body, MimeHdr** hdr);
 WOLFSSL_LOCAL int wc_MIME_create_parameter(char* attribute, char* value, MimeParam** param);
 WOLFSSL_LOCAL MimeHdr* wc_MIME_find_header_name(const char* name, MimeHdr* hdr);
 WOLFSSL_LOCAL MimeParam* wc_MIME_find_param_attr(const char* attribute, MimeParam* param);
+WOLFSSL_LOCAL char* wc_MIME_canonicalize(const char* line);
 WOLFSSL_LOCAL int wc_MIME_free_hdrs(MimeHdr* head);
 #endif /* HAVE_SMIME */