From b6d8c173543ab8941cc6e3b2b6f9523ebbecab47 Mon Sep 17 00:00:00 2001 From: olszomal Date: Mon, 28 Apr 2025 11:12:04 +0200 Subject: [PATCH 1/2] Fix control flow and braces for engine and provider support --- osslsigncode.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/osslsigncode.c b/osslsigncode.c index 950c305..9f1e4d8 100644 --- a/osslsigncode.c +++ b/osslsigncode.c @@ -4226,12 +4226,12 @@ static int read_crypto_params(GLOBAL_OPTIONS *options) /* Try to use PKCS#12 container with certificates and the private key ('-pkcs12' option) */ if (options->pkcs12file) { load_objects_from_store(options->pkcs12file, options->pass, &options->pkey, options->certs, options->crls); - + } #if !defined(OPENSSL_NO_ENGINE) || OPENSSL_VERSION_NUMBER>=0x30000000L /* Security token */ #ifndef OPENSSL_NO_ENGINE /* PKCS#11 'dynamic' engine */ - } else if (options->p11engine) { + else if (options->p11engine) { if(!engine_load(options)) goto out; #endif /* OPENSSL_NO_ENGINE */ @@ -4241,17 +4241,16 @@ static int read_crypto_params(GLOBAL_OPTIONS *options) if ((options->provider && provider_load(options->provider)) || provider_load("pkcs11prov")) { load_objects_from_store(options->keyfile, options->pass, &options->pkey, NULL, NULL); load_objects_from_store(options->p11cert, options->pass, NULL, options->certs, NULL); - } else { + } else #endif /* OPENSSL_VERSION_NUMBER>=0x30000000L */ #ifndef OPENSSL_NO_ENGINE /* try to find and load libp11 'pkcs11' engine */ - if (!engine_load(options)) { + if (!engine_load(options)) goto out; #endif /* OPENSSL_NO_ENGINE */ - } - } + } #endif /* !defined(OPENSSL_NO_ENGINE) || OPENSSL_VERSION_NUMBER>=0x30000000L */ - } else { + else { /* Load the the private key ('-key' option) */ load_objects_from_store(options->keyfile, options->pass, &options->pkey, NULL, NULL); } @@ -4273,7 +4272,9 @@ static int read_crypto_params(GLOBAL_OPTIONS *options) if (sk_X509_num(options->certs) == 0 && !read_pkcs7_certfile(options)) { return 0; /* FAILED */ } +#if !defined(OPENSSL_NO_ENGINE) || OPENSSL_VERSION_NUMBER>=0x30000000L out: +#endif /* !defined(OPENSSL_NO_ENGINE) || OPENSSL_VERSION_NUMBER>=0x30000000L */ return (options->pkey && sk_X509_num(options->certs) > 0) ? 1 : 0; } From 276769a98f9ed6e4a5c6f5eee0be299a4bd5e963 Mon Sep 17 00:00:00 2001 From: olszomal Date: Wed, 30 Apr 2025 09:11:28 +0200 Subject: [PATCH 2/2] Improve PKCS#7 verification with OpenSSL 3.5 Enhanced verification logic for PKCS#7 signedData structures by introducing a dedicated `verify_pkcs7_data()` function. This update addresses compatibility with older OpenSSL versions (< 3.0.5) and ensures correct handling of detached signed content using a BIO buffer. The change enables support for PKCS#7 inner content (RFC 2315, section 7), as per OpenSSL PR#22575. Refactored timestamp and authenticode verification functions to reduce duplication and properly manage X509_STORE and X509_CRL structures. --- osslsigncode.c | 116 ++++++++++++++++++++++++++----------------------- 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/osslsigncode.c b/osslsigncode.c index 9f1e4d8..176a471 100644 --- a/osslsigncode.c +++ b/osslsigncode.c @@ -2216,7 +2216,6 @@ static int verify_timestamp_token(PKCS7 *p7, CMS_ContentInfo *timestamp) */ static int verify_timestamp(FILE_FORMAT_CTX *ctx, PKCS7 *p7, CMS_ContentInfo *timestamp, time_t time) { - X509_STORE *store; STACK_OF(CMS_SignerInfo) *sinfos; CMS_SignerInfo *cmssi; X509 *signer; @@ -2224,8 +2223,8 @@ static int verify_timestamp(FILE_FORMAT_CTX *ctx, PKCS7 *p7, CMS_ContentInfo *ti STACK_OF(X509_CRL) *crls = NULL; char *url = NULL; int verok = 0; + X509_STORE *store = X509_STORE_new(); - store = X509_STORE_new(); if (!store) goto out; if (x509_store_load_file(store, ctx->options->tsa_cafile)) { @@ -2240,12 +2239,10 @@ static int verify_timestamp(FILE_FORMAT_CTX *ctx, PKCS7 *p7, CMS_ContentInfo *ti */ if (!x509_store_set_time(store, time)) { fprintf(stderr, "Failed to set store time\n"); - X509_STORE_free(store); goto out; } } else { printf("Use the \"-TSA-CAfile\" option to add the Time-Stamp Authority certificates bundle to verify the Timestamp Server.\n"); - X509_STORE_free(store); goto out; } @@ -2255,14 +2252,12 @@ static int verify_timestamp(FILE_FORMAT_CTX *ctx, PKCS7 *p7, CMS_ContentInfo *ti STACK_OF(X509) *cms_certs; printf("CMS_verify error\n"); - X509_STORE_free(store); printf("\nFailed timestamp certificate chain retrieved from the signature:\n"); cms_certs = CMS_get1_certs(timestamp); print_certs_chain(cms_certs); sk_X509_pop_free(cms_certs, X509_free); goto out; } - X509_STORE_free(store); sinfos = CMS_get0_SignerInfos(timestamp); cmssi = sk_CMS_SignerInfo_value(sinfos, 0); @@ -2299,7 +2294,6 @@ static int verify_timestamp(FILE_FORMAT_CTX *ctx, PKCS7 *p7, CMS_ContentInfo *ti int crlok = verify_crl(ctx->options->tsa_cafile, ctx->options->tsa_crlfile, crls, signer, chain); sk_X509_pop_free(chain, X509_free); - sk_X509_CRL_pop_free(crls, X509_CRL_free); printf("Timestamp Server Signature CRL verification: %s\n", crlok ? "ok" : "failed"); if (!crlok) goto out; @@ -2317,6 +2311,8 @@ static int verify_timestamp(FILE_FORMAT_CTX *ctx, PKCS7 *p7, CMS_ContentInfo *ti } verok = 1; /* OK */ out: + X509_STORE_free(store); + sk_X509_CRL_pop_free(crls, X509_CRL_free); if (!verok) ERR_print_errors_fp(stderr); return verok; @@ -2344,6 +2340,60 @@ static int PKCS7_type_is_other(PKCS7 *p7) } #endif /* OPENSSL_VERSION_NUMBER<0x30000000L */ +/* + * [in] p7: PKCS#7 signature + * [in] store: X509_STORE + * [returns] 1 on error or 0 on success + */ +static int verify_pkcs7_data(PKCS7 *p7, X509_STORE *store) +{ + int verok = 0; +#if OPENSSL_VERSION_NUMBER<0x30500000L + BIO *bio = NULL; + PKCS7 *contents = p7->d.sign->contents; + + /* + * In the PKCS7_verify() function, the BIO *indata parameter refers to + * the signed data if the content is detached from p7. + * Otherwise, indata should be NULL, and then the signed data must be in p7. + * The OpenSSL error workaround is to put the inner content into BIO *indata parameter + * https://github.com/openssl/openssl/pull/22575 + */ + if (PKCS7_type_is_other(contents) && (contents->d.other != NULL) + && (contents->d.other->value.sequence != NULL) + && (contents->d.other->value.sequence->length > 0)) { + if (contents->d.other->type == V_ASN1_SEQUENCE) { + /* only verify the content of the sequence */ + const unsigned char *data = contents->d.other->value.sequence->data; + long len; + int inf, tag, class; + + inf = ASN1_get_object(&data, &len, &tag, &class, + contents->d.other->value.sequence->length); + if (inf != V_ASN1_CONSTRUCTED || tag != V_ASN1_SEQUENCE) { + fprintf(stderr, "Corrupted data content\n"); + X509_STORE_free(store); + return 0; /* FAILED */ + } + bio = BIO_new_mem_buf(data, (int)len); + } else { + /* verify the entire value */ + bio = BIO_new_mem_buf(contents->d.other->value.sequence->data, + contents->d.other->value.sequence->length); + } + } else { + fprintf(stderr, "Corrupted data content\n"); + X509_STORE_free(store); + return 0; /* FAILED */ + } + verok = PKCS7_verify(p7, NULL, store, bio, NULL, 0); + BIO_free(bio); +#else /* OPENSSL_VERSION_NUMBER<0x30500000L */ + verok = PKCS7_verify(p7, NULL, store, NULL, NULL, 0); +#endif /* OPENSSL_VERSION_NUMBER<0x30500000L */ + return verok; +} + /* * [in] ctx: structure holds input and output data * [in] p7: PKCS#7 signature @@ -2353,21 +2403,17 @@ static int PKCS7_type_is_other(PKCS7 *p7) */ static int verify_authenticode(FILE_FORMAT_CTX *ctx, PKCS7 *p7, time_t time, X509 *signer) { - X509_STORE *store; X509_CRL *crl = NULL; STACK_OF(X509_CRL) *crls = NULL; - BIO *bio = NULL; int verok = 0; char *url = NULL; - PKCS7 *contents = p7->d.sign->contents; + X509_STORE *store = X509_STORE_new(); - store = X509_STORE_new(); if (!store) goto out; if (!x509_store_load_file(store, ctx->options->cafile)) { fprintf(stderr, "Failed to add store lookup file\n"); - X509_STORE_free(store); goto out; } if (time != INVALID_TIME) { @@ -2375,7 +2421,6 @@ static int verify_authenticode(FILE_FORMAT_CTX *ctx, PKCS7 *p7, time_t time, X50 print_time_t(time); if (!x509_store_set_time(store, time)) { fprintf(stderr, "Failed to set signature time\n"); - X509_STORE_free(store); goto out; } } else if (ctx->options->time != INVALID_TIME) { @@ -2383,56 +2428,17 @@ static int verify_authenticode(FILE_FORMAT_CTX *ctx, PKCS7 *p7, time_t time, X50 print_time_t(ctx->options->time); if (!x509_store_set_time(store, ctx->options->time)) { fprintf(stderr, "Failed to set verifying time\n"); - X509_STORE_free(store); goto out; } } /* verify a PKCS#7 signedData structure */ - if (PKCS7_type_is_other(contents) && (contents->d.other != NULL) - && (contents->d.other->value.sequence != NULL) - && (contents->d.other->value.sequence->length > 0)) { - if (contents->d.other->type == V_ASN1_SEQUENCE) { - /* only verify the content of the sequence */ - const unsigned char *data = contents->d.other->value.sequence->data; - long len; - int inf, tag, class; - - inf = ASN1_get_object(&data, &len, &tag, &class, - contents->d.other->value.sequence->length); - if (inf != V_ASN1_CONSTRUCTED || tag != V_ASN1_SEQUENCE) { - fprintf(stderr, "Corrupted data content\n"); - X509_STORE_free(store); - goto out; - } - bio = BIO_new_mem_buf(data, (int)len); - } else { - /* verify the entire value */ - bio = BIO_new_mem_buf(contents->d.other->value.sequence->data, - contents->d.other->value.sequence->length); - } - } else { - fprintf(stderr, "Corrupted data content\n"); - X509_STORE_free(store); - goto out; - } printf("Signing certificate chain verified using:\n"); - /* - * In the PKCS7_verify() function, the BIO *indata parameter refers to - * the signed data if the content is detached from p7. - * Otherwise, indata should be NULL, and then the signed data must be in p7. - * The OpenSSL error workaround is to put the inner content into BIO *indata parameter - * https://github.com/openssl/openssl/pull/22575 - */ - if (!PKCS7_verify(p7, NULL, store, bio, NULL, 0)) { + if (!verify_pkcs7_data(p7, store)) { printf("PKCS7_verify error\n"); - X509_STORE_free(store); - BIO_free(bio); printf("Failed signing certificate chain retrieved from the signature:\n"); print_certs_chain(p7->d.sign->cert); goto out; } - X509_STORE_free(store); - BIO_free(bio); /* verify a Certificate Revocation List */ if (!ctx->options->ignore_crl) { @@ -2464,7 +2470,7 @@ static int verify_authenticode(FILE_FORMAT_CTX *ctx, PKCS7 *p7, time_t time, X50 STACK_OF(X509) *chain = p7->d.sign->cert; int crlok = verify_crl(ctx->options->cafile, ctx->options->crlfile, crls, signer, chain); - sk_X509_CRL_pop_free(crls, X509_CRL_free); + printf("Signature CRL verification: %s\n", crlok ? "ok" : "failed"); if (!crlok) goto out; @@ -2477,6 +2483,8 @@ static int verify_authenticode(FILE_FORMAT_CTX *ctx, PKCS7 *p7, time_t time, X50 verok = 1; /* OK */ out: + X509_STORE_free(store); + sk_X509_CRL_pop_free(crls, X509_CRL_free); if (!verok) ERR_print_errors_fp(stderr); return verok;