Merge pull request #1193 from avast/2024-02-14/update-authenticode-parser

Updated authenticode-parser to version from 2024-02-14

Author: metthal

deps/authenticode-parser/include/authenticode-parser/authenticode.h

@@ -124,6 +124,8 @@ typedef struct {
     char* digest_alg;        /* Name of the digest algorithm used */
     ByteArray digest;        /* Stored message digest */
     CertificateArray* chain; /* Certificate chain of the signer */
+    CertificateArray* certs; /* All certs stored inside Countersignature, this can be superset
+                                of chain in case of non PKCS9 countersignature*/
 } Countersignature;
 
 typedef struct {

deps/authenticode-parser/src/authenticode.c

@@ -114,23 +114,6 @@ static char* parse_program_name(ASN1_TYPE* spcAttr)
     return result;
 }
 
-/* Parses X509* certs into internal representation and inserts into CertificateArray
- * Array is assumed to have enough space to hold all certificates storted in the STACK */
-static void parse_certificates(const STACK_OF(X509) * certs, CertificateArray* result)
-{
-    int certCount = sk_X509_num(certs);
-    int i = 0;
-    for (; i < certCount; ++i) {
-        Certificate* cert = certificate_new(sk_X509_value(certs, i));
-        if (!cert)
-            break;
-
-        /* Write to the result */
-        result->certs[i] = cert;
-    }
-    result->count = i;
-}
-
 static void parse_nested_authenticode(PKCS7_SIGNER_INFO* si, AuthenticodeArray* result)
 {
     STACK_OF(X509_ATTRIBUTE)* attrs = PKCS7_get_attributes(si);
@@ -190,26 +173,6 @@ static void parse_pkcs9_countersig(PKCS7* p7, Authenticode* auth)
     }
 }
 
-/* Extracts X509 certificates from MS countersignature and stores them into result */
-static void extract_ms_counter_certs(const uint8_t* data, int len, CertificateArray* result)
-{
-    PKCS7* p7 = d2i_PKCS7(NULL, &data, len);
-    if (!p7)
-        return;
-
-    STACK_OF(X509)* certs = p7->d.sign->cert;
-    CertificateArray* certArr = certificate_array_new(sk_X509_num(certs));
-    if (!certArr) {
-        PKCS7_free(p7);
-        return;
-    }
-    parse_certificates(certs, certArr);
-    certificate_array_move(result, certArr);
-    certificate_array_free(certArr);
-
-    PKCS7_free(p7);
-}
-
 static void parse_ms_countersig(PKCS7* p7, Authenticode* auth)
 {
     PKCS7_SIGNER_INFO* si = sk_PKCS7_SIGNER_INFO_value(PKCS7_get_signer_info(p7), 0);
@@ -233,14 +196,14 @@ static void parse_ms_countersig(PKCS7* p7, Authenticode* auth)
         int len = nested->value.sequence->length;
         const uint8_t* data = nested->value.sequence->data;
 
-        Countersignature* sig = ms_countersig_new(data, len, si->enc_digest);
-        if (!sig)
+        Countersignature* csig = ms_countersig_new(data, len, si->enc_digest);
+        if (!csig)
             return;
 
+        countersignature_array_insert(auth->countersigs, csig);
         /* Because MS TimeStamp countersignature has it's own SET of certificates
          * extract it back into parent signature for consistency with PKCS9 */
-        countersignature_array_insert(auth->countersigs, sig);
-        extract_ms_counter_certs(data, len, auth->certs);
+        certificate_array_append(auth->certs, csig->certs);
     }
 }
 
@@ -338,7 +301,7 @@ AuthenticodeArray* authenticode_new(const uint8_t* data, int32_t len)
         auth->verify_flags = AUTHENTICODE_VFY_INTERNAL_ERROR;
         goto end;
     }
-    parse_certificates(certs, auth->certs);
+    parse_x509_certificates(certs, auth->certs);
 
     /* Get Signature content that contains the message digest and it's algorithm */
     SpcIndirectDataContent* dataContent = get_content(p7data->contents);

deps/authenticode-parser/src/certificate.c

@@ -330,10 +330,82 @@ Certificate* certificate_new(X509* x509)
     return result;
 }
 
+void attributes_copy(Attributes* dst, Attributes* src)
+{
+    byte_array_init(&dst->country, src->country.data, src->country.len);
+    byte_array_init(&dst->organization, src->organization.data, src->organization.len);
+    byte_array_init(
+        &dst->organizationalUnit, src->organizationalUnit.data, src->organizationalUnit.len);
+    byte_array_init(&dst->nameQualifier, src->nameQualifier.data, src->nameQualifier.len);
+    byte_array_init(&dst->state, src->state.data, src->state.len);
+    byte_array_init(&dst->commonName, src->commonName.data, src->commonName.len);
+    byte_array_init(&dst->serialNumber, src->serialNumber.data, src->serialNumber.len);
+    byte_array_init(&dst->locality, src->locality.data, src->locality.len);
+    byte_array_init(&dst->title, src->title.data, src->title.len);
+    byte_array_init(&dst->surname, src->surname.data, src->surname.len);
+    byte_array_init(&dst->givenName, src->givenName.data, src->givenName.len);
+    byte_array_init(&dst->initials, src->initials.data, src->initials.len);
+    byte_array_init(&dst->pseudonym, src->pseudonym.data, src->pseudonym.len);
+    byte_array_init(
+        &dst->generationQualifier, src->generationQualifier.data, src->generationQualifier.len);
+    byte_array_init(&dst->emailAddress, src->emailAddress.data, src->emailAddress.len);
+}
+
+/* Parses X509* certs into internal representation and inserts into CertificateArray
+ * Array is assumed to have enough space to hold all certificates storted in the STACK */
+void parse_x509_certificates(const STACK_OF(X509) * certs, CertificateArray* result)
+{
+    int certCount = sk_X509_num(certs);
+    int i = 0;
+    for (; i < certCount; ++i) {
+        Certificate* cert = certificate_new(sk_X509_value(certs, i));
+        if (!cert)
+            break;
+
+        /* Write to the result */
+        result->certs[i] = cert;
+    }
+    result->count = i;
+}
+
+/* Creates deep copy of a certificate */
+Certificate* certificate_copy(Certificate* cert)
+{
+    if (!cert)
+        return NULL;
+
+    Certificate* result = (Certificate*)calloc(1, sizeof(*result));
+    if (!result)
+        return NULL;
+
+    result->version = cert->version;
+    result->issuer = strdup(cert->issuer);
+    result->subject = strdup(cert->subject);
+    result->serial = strdup(cert->serial);
+    result->not_after = cert->not_after;
+    result->not_before = cert->not_before;
+    result->sig_alg = strdup(cert->sig_alg);
+    result->sig_alg_oid = strdup(cert->sig_alg_oid);
+    result->key_alg = strdup(cert->key_alg);
+    result->key = strdup(cert->key);
+    byte_array_init(&result->sha1, cert->sha1.data, cert->sha1.len);
+    byte_array_init(&result->sha256, cert->sha256.data, cert->sha256.len);
+    attributes_copy(&result->issuer_attrs, &cert->issuer_attrs);
+    attributes_copy(&result->subject_attrs, &cert->subject_attrs);
+
+    return result;
+}
+
 /* Moves certificates from src to dst, returns 0 on success,
  * else 1. If error occurs, arguments are unchanged */
 int certificate_array_move(CertificateArray* dst, CertificateArray* src)
 {
+    if (!dst || !src)
+        return 1;
+
+    if (!src->certs || !src->count)
+        return 0;
+
     size_t newCount = dst->count + src->count;
 
     Certificate** tmp = (Certificate**)realloc(dst->certs, newCount * sizeof(Certificate*));
@@ -354,6 +426,32 @@ int certificate_array_move(CertificateArray* dst, CertificateArray* src)
     return 0;
 }
 
+/* Copies certificates from src and appends to dst, returns 0 on success,
+ * else 1. If error occurs, arguments are unchanged */
+int certificate_array_append(CertificateArray* dst, CertificateArray* src)
+{
+    if (!dst || !src)
+        return 1;
+
+    if (!src->certs || !src->count)
+        return 0;
+
+    size_t newCount = dst->count + src->count;
+
+    Certificate** tmp = (Certificate**)realloc(dst->certs, newCount * sizeof(Certificate*));
+    if (!tmp)
+        return 1;
+
+    dst->certs = tmp;
+
+    for (size_t i = 0; i < src->count; ++i)
+        dst->certs[i + dst->count] = certificate_copy(src->certs[i]);
+
+    dst->count = newCount;
+
+    return 0;
+}
+
 /* Allocates empty certificate array with reserved space for certCount certs */
 CertificateArray* certificate_array_new(int certCount)
 {

deps/authenticode-parser/src/certificate.h

@@ -31,10 +31,14 @@ extern "C" {
 #endif
 
 Certificate* certificate_new(X509* x509);
+Certificate* certificate_copy(Certificate* cert);
 void certificate_free(Certificate* cert);
 
+void parse_x509_certificates(const STACK_OF(X509) * certs, CertificateArray* result);
+
 CertificateArray* parse_signer_chain(X509* signer_cert, STACK_OF(X509) * certs);
 int certificate_array_move(CertificateArray* dst, CertificateArray* src);
+int certificate_array_append(CertificateArray* dst, CertificateArray* src);
 CertificateArray* certificate_array_new(int certCount);
 void certificate_array_free(CertificateArray* arr);
 

deps/authenticode-parser/src/countersignature.c

@@ -457,6 +457,9 @@ CountersignatureImpl* ms_countersig_impl_new(const uint8_t* data, long size)
         result->funcs = &FUNC_ARRAY_NAME_FOR_IMPL(pkcs7);
         result->pkcs7 = p7;
         return result;
+    } else if (p7) {
+        PKCS7_free(p7);
+        return NULL;
     }
 
     d = data;
@@ -527,6 +530,16 @@ Countersignature* ms_countersig_new(const uint8_t* data, long size, ASN1_STRING*
     }
 
     STACK_OF(X509)* certs = impl->funcs->get_certs(impl);
+
+    /* MS Counter signatures (PKCS7/CMS) can have extra certificates that are not part of a chain */
+    result->certs = certificate_array_new(sk_X509_num(certs));
+    if (!result->certs) {
+        result->verify_flags = AUTHENTICODE_VFY_INTERNAL_ERROR;
+        goto end;
+    }
+
+    parse_x509_certificates(certs, result->certs);
+
     result->chain = parse_signer_chain(signCert, certs);
 
     /* Imprint == digest */
@@ -642,6 +655,7 @@ void countersignature_free(Countersignature* sig)
         free(sig->digest_alg);
         free(sig->digest.data);
         certificate_array_free(sig->chain);
+        certificate_array_free(sig->certs);
         free(sig);
     }
 }