Skip to content

Improve PKCS#7 verification with OpenSSL 3.5 #429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mtrojnar merged 2 commits into from
May 1, 2025
Merged

Improve PKCS#7 verification with OpenSSL 3.5 #429

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6d8c17
Fix control flow and braces for engine and provider support
olszomal Apr 28, 2025
276769a
Improve PKCS#7 verification with OpenSSL 3.5
olszomal Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 70 additions & 61 deletions osslsigncode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2216,16 +2216,15 @@ 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;
X509_CRL *crl = NULL;
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)) {
Expand All @@ -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;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -2353,86 +2403,42 @@ 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) {
printf("Signature verification time: ");
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) {
printf("Signature verification time: ");
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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -4226,12 +4234,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 */
Expand All @@ -4241,17 +4249,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);
}
Expand All @@ -4273,7 +4280,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;
}

Expand Down