From: George Joseph Date: Wed, 18 Jun 2025 20:38:08 +0000 (-0600) Subject: res_stir_shaken.so: Handle X5U certificate chains. X-Git-Tag: 21.10.0-rc1~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6770dc31b7ecca8b56e238837a3da96a13954595;p=thirdparty%2Fasterisk.git res_stir_shaken.so: Handle X5U certificate chains. The verification process will now load a full certificate chain retrieved via the X5U URL instead of loading only the end user cert. * Renamed crypto_load_cert_from_file() and crypto_load_cert_from_memory() to crypto_load_cert_chain_from_file() and crypto_load_cert_chain_from_memory() respectively. * The two load functions now continue to load certs from the file or memory PEMs and store them in a separate stack of untrusted certs specific to the current verification context. * crypto_is_cert_trusted() now uses the stack of untrusted certs that were extracted from the PEM in addition to any untrusted certs that were passed in from the configuration (and any CA certs passed in from the config of course). Resolves: #1272 UserNote: The STIR/SHAKEN verification process will now load a full certificate chain retrieved via the X5U URL instead of loading only the end user cert. (cherry picked from commit ec2591c60b63e05d1b74eb5a80408974fed0f2a4) --- diff --git a/res/res_stir_shaken/attestation_config.c b/res/res_stir_shaken/attestation_config.c index 46ebb90399..1a0a22826a 100644 --- a/res/res_stir_shaken/attestation_config.c +++ b/res/res_stir_shaken/attestation_config.c @@ -164,8 +164,8 @@ int as_check_common_config(const char *id, struct attestation_cfg_common *acfg_c acfg_common->public_cert_url); } - public_cert = crypto_load_cert_from_memory(public_cert_data, - public_cert_len); + public_cert = crypto_load_cert_chain_from_memory(public_cert_data, + public_cert_len, NULL); if (!public_cert) { SCOPE_EXIT_LOG_RTN_VALUE(-1, LOG_ERROR, "%s: public_cert '%s' could not be parsed as a certificate\n", id, acfg_common->public_cert_url); diff --git a/res/res_stir_shaken/common_config.c b/res/res_stir_shaken/common_config.c index 0cbd9b5035..33e2d188a9 100644 --- a/res/res_stir_shaken/common_config.c +++ b/res/res_stir_shaken/common_config.c @@ -306,6 +306,7 @@ static char *cli_verify_cert(struct ast_cli_entry *e, int cmd, struct ast_cli_ar RAII_VAR(struct verification_cfg *, vs_cfg, NULL, ao2_cleanup); struct crypto_cert_store *tcs; X509 *cert = NULL; + STACK_OF(X509) *cert_chain = NULL; const char *errmsg = NULL; switch(cmd) { @@ -347,18 +348,21 @@ static char *cli_verify_cert(struct ast_cli_entry *e, int cmd, struct ast_cli_ar tcs = vs_cfg->vcfg_common.tcs; } - cert = crypto_load_cert_from_file(a->argv[3]); + cert = crypto_load_cert_chain_from_file(a->argv[3], &cert_chain); if (!cert) { ast_cli(a->fd, "Failed to load certificate from %s. See log for details\n", a->argv[3]); return CLI_SUCCESS; } - if (crypto_is_cert_trusted(tcs, cert, &errmsg)) { + if (crypto_is_cert_trusted(tcs, cert, cert_chain, &errmsg)) { ast_cli(a->fd, "Certificate %s trusted\n", a->argv[3]); } else { ast_cli(a->fd, "Certificate %s NOT trusted: %s\n", a->argv[3], errmsg); } X509_free(cert); + if (cert_chain) { + sk_X509_pop_free(cert_chain, X509_free); + } return CLI_SUCCESS; } diff --git a/res/res_stir_shaken/crypto_utils.c b/res/res_stir_shaken/crypto_utils.c index f00d72f9b3..b1671c1593 100644 --- a/res/res_stir_shaken/crypto_utils.c +++ b/res/res_stir_shaken/crypto_utils.c @@ -186,10 +186,24 @@ X509_CRL *crypto_load_crl_from_file(const char *filename) return crl; } -X509 *crypto_load_cert_from_file(const char *filename) +#define debug_cert_chain(level, cert_chain) \ +({ \ + int i; \ + char subj[1024]; \ + if (cert_chain && sk_X509_num(cert_chain) > 0) { \ + for (i = 0; i < sk_X509_num(cert_chain); i++) { \ + X509 *cert = sk_X509_value(cert_chain, i); \ + subj[0] = '\0'; \ + X509_NAME_oneline(X509_get_subject_name(cert), subj, 1024); \ + ast_debug(level, "Chain cert %d: '%s'\n", i, subj); \ + } \ + } \ +}) + +X509 *crypto_load_cert_chain_from_file(const char *filename, STACK_OF(X509) **cert_chain) { FILE *fp; - X509 *cert = NULL; + X509 *end_cert = NULL; if (ast_strlen_zero(filename)) { ast_log(LOG_ERROR, "filename was null or empty\n"); @@ -202,18 +216,58 @@ X509 *crypto_load_cert_from_file(const char *filename) return NULL; } - cert = PEM_read_X509(fp, &cert, NULL, NULL); - fclose(fp); - if (!cert) { - crypto_log_openssl(LOG_ERROR, "Failed to create cert from %s\n", filename); + end_cert = PEM_read_X509(fp, &end_cert, NULL, NULL); + if (!end_cert) { + crypto_log_openssl(LOG_ERROR, "Failed to create end_cert from %s\n", filename); + fclose(fp); + return NULL; + } + + /* + * If the caller provided a stack, we will read the chain certs + * (if any) into it. + */ + if (cert_chain) { + X509 *chain_cert = NULL; + + *cert_chain = sk_X509_new_null(); + while ((chain_cert = PEM_read_X509(fp, &chain_cert, NULL, NULL)) != NULL) { + if (sk_X509_push(*cert_chain, chain_cert) <= 0) { + crypto_log_openssl(LOG_ERROR, "Failed to add chain cert from %s to list\n", + filename); + fclose(fp); + X509_free(end_cert); + sk_X509_pop_free(*cert_chain, X509_free); + return NULL; + } + /* chain_cert needs to be reset to NULL after every call to PEM_read_X509 */ + chain_cert = NULL; + } + } + + if (DEBUG_ATLEAST(4)) { + char subj[1024]; + + X509_NAME_oneline(X509_get_subject_name(end_cert), subj, 1024); + ast_debug(4, "Opened end cert '%s' from '%s'\n", subj, filename); + + if (cert_chain && *cert_chain) { + debug_cert_chain(4, *cert_chain); + } else { + ast_debug(4, "No chain certs found in '%s'\n", filename); + } } - return cert; + + fclose(fp); + + return end_cert; } -X509 *crypto_load_cert_from_memory(const char *buffer, size_t size) +X509 *crypto_load_cert_chain_from_memory(const char *buffer, size_t size, + STACK_OF(X509) **cert_chain) { RAII_VAR(BIO *, bio, NULL, BIO_free_all); - X509 *cert = NULL; + X509 *end_cert = NULL; if (ast_strlen_zero(buffer) || size <= 0) { ast_log(LOG_ERROR, "buffer was null or empty\n"); @@ -226,11 +280,46 @@ X509 *crypto_load_cert_from_memory(const char *buffer, size_t size) return NULL; } - cert = PEM_read_bio_X509(bio, NULL, NULL, NULL); - if (!cert) { - crypto_log_openssl(LOG_ERROR, "Failed to create cert from BIO\n"); + end_cert = PEM_read_bio_X509(bio, NULL, NULL, NULL); + if (!end_cert) { + crypto_log_openssl(LOG_ERROR, "Failed to create end_cert from BIO\n"); + return NULL; } - return cert; + + /* + * If the caller provided a stack, we will read the chain certs + * (if any) into it. + */ + if (cert_chain) { + X509 *chain_cert = NULL; + + *cert_chain = sk_X509_new_null(); + while ((chain_cert = PEM_read_bio_X509(bio, &chain_cert, NULL, NULL)) != NULL) { + if (sk_X509_push(*cert_chain, chain_cert) <= 0) { + crypto_log_openssl(LOG_ERROR, "Failed to add chain cert from BIO to list\n"); + X509_free(end_cert); + sk_X509_pop_free(*cert_chain, X509_free); + return NULL; + } + /* chain_cert needs to be reset to NULL after every call to PEM_read_X509 */ + chain_cert = NULL; + } + } + + if (DEBUG_ATLEAST(4)) { + char subj[1024]; + + X509_NAME_oneline(X509_get_subject_name(end_cert), subj, 1024); + ast_debug(4, "Opened end cert '%s' from BIO\n", subj); + + if (cert_chain && *cert_chain) { + debug_cert_chain(4, *cert_chain); + } else { + ast_debug(4, "No chain certs found in BIO\n"); + } + } + + return end_cert; } static EVP_PKEY *load_private_key_from_memory(const char *buffer, size_t size) @@ -441,7 +530,7 @@ static int crypto_load_store_from_cert_file(X509_STORE *store, const char *file) return -1; } - cert = crypto_load_cert_from_file(file); + cert = crypto_load_cert_chain_from_file(file, NULL); if (!cert) { return -1; } @@ -737,9 +826,11 @@ int crypto_is_cert_time_valid(X509*cert, time_t reftime) X509_cmp_time(notafter, &reftime) > 0); } -int crypto_is_cert_trusted(struct crypto_cert_store *store, X509 *cert, const char **err_msg) +int crypto_is_cert_trusted(struct crypto_cert_store *store, X509 *cert, + STACK_OF(X509) *cert_chain, const char **err_msg) { X509_STORE_CTX *verify_ctx = NULL; + RAII_VAR(STACK_OF(X509) *, untrusted_stack, NULL, sk_X509_free); int rc = 0; if (!(verify_ctx = X509_STORE_CTX_new())) { @@ -747,7 +838,45 @@ int crypto_is_cert_trusted(struct crypto_cert_store *store, X509 *cert, const ch return 0; } - if (X509_STORE_CTX_init(verify_ctx, store->certs, cert, store->untrusted_stack) != 1) { + if (cert_chain && sk_X509_num(cert_chain) > 0) { + int untrusted_count = store->untrusted_stack ? sk_X509_num(store->untrusted_stack) : 0; + int i = 0; + + untrusted_stack = sk_X509_dup(cert_chain); + if (!untrusted_stack) { + crypto_log_openssl(LOG_ERROR, "Unable to duplicate untrusted stack\n"); + X509_STORE_CTX_free(verify_ctx); + return 0; + } + /* + * If store->untrusted_stack was NULL for some reason then + * untrusted_count will be 0 so the loop will never run. + */ + for (i = 0; i < untrusted_count; i++) { + X509 *c = sk_X509_value(store->untrusted_stack, i); + if (sk_X509_push(untrusted_stack, c) <= 0) { + crypto_log_openssl(LOG_ERROR, "Unable to push untrusted cert onto stack\n"); + sk_X509_free(untrusted_stack); + X509_STORE_CTX_free(verify_ctx); + return 0; + } + } + /* + * store->untrusted_stack should always be allocated even if empty + * but we'll make sure. + */ + } else if (store->untrusted_stack){ + /* This is a dead simple shallow clone */ + ast_debug(4, "cert_chain had no certs\n"); + untrusted_stack = sk_X509_dup(store->untrusted_stack); + if (!untrusted_stack) { + crypto_log_openssl(LOG_ERROR, "Unable to duplicate untrusted stack\n"); + X509_STORE_CTX_free(verify_ctx); + return 0; + } + } + + if (X509_STORE_CTX_init(verify_ctx, store->certs, cert, untrusted_stack) != 1) { X509_STORE_CTX_cleanup(verify_ctx); X509_STORE_CTX_free(verify_ctx); crypto_log_openssl(LOG_ERROR, "Unable to initialize verify_ctx\n"); @@ -760,6 +889,7 @@ int crypto_is_cert_trusted(struct crypto_cert_store *store, X509 *cert, const ch int err = X509_STORE_CTX_get_error(verify_ctx); *err_msg = X509_verify_cert_error_string(err); } + X509_STORE_CTX_cleanup(verify_ctx); X509_STORE_CTX_free(verify_ctx); diff --git a/res/res_stir_shaken/crypto_utils.h b/res/res_stir_shaken/crypto_utils.h index 692f25abb9..2780ba15c4 100644 --- a/res/res_stir_shaken/crypto_utils.h +++ b/res/res_stir_shaken/crypto_utils.h @@ -74,13 +74,19 @@ ASN1_OCTET_STRING *crypto_get_cert_extension_data(X509 *cert, int nid, const char *short_name); /*! - * \brief Load an X509 Cert from a file + * \brief Load an X509 Cert and any chained certs from a file * * \param filename PEM file + * \param chain_stack The address of a STACK_OF(X509) pointer to receive the + * chain of certificates if any. + * + * \note The caller is responsible for freeing the cert_chain stack and + * any certs in it. * * \returns X509* or NULL on error */ -X509 *crypto_load_cert_from_file(const char *filename); +X509 *crypto_load_cert_chain_from_file(const char *filename, + STACK_OF(X509) **chain_stack); /*! * \brief Load an X509 CRL from a PEM file @@ -116,15 +122,21 @@ EVP_PKEY *crypto_load_private_key_from_memory(const char *buffer, size_t size); int crypto_has_private_key_from_memory(const char *buffer, size_t size); /*! - * \brief Load an X509 Cert from a NULL terminated buffer + * \brief Load an X509 Cert and any chained certs from a NULL terminated buffer * * \param buffer containing the cert * \param size size of the buffer. * May be -1 if the buffer is NULL terminated. + * \param chain_stack The address of a STACK_OF(X509) pointer to receive the + * chain of certificates if any. + * + * \note The caller is responsible for freeing the cert_chain stack and + * any certs in it. * * \returns X509* or NULL on error */ -X509 *crypto_load_cert_from_memory(const char *buffer, size_t size); +X509 *crypto_load_cert_chain_from_memory(const char *buffer, size_t size, + STACK_OF(X509) **cert_chain); /*! * \brief Retrieve RAW public key from cert @@ -292,12 +304,14 @@ int crypto_is_cert_time_valid(X509 *cert, time_t reftime); * * \param store The CA store to check against * \param cert The cert to check + * \param cert_chain An untrusted certificate chain that may have accompanied the cert. * \param err_msg Optional pointer to a const char * * * \retval 1 Cert is trusted * \retval 0 Cert is not trusted */ -int crypto_is_cert_trusted(struct crypto_cert_store *store, X509 *cert, const char **err_msg); +int crypto_is_cert_trusted(struct crypto_cert_store *store, X509 *cert, + STACK_OF(X509) *cert_chain, const char **err_msg); /*! * \brief Return a time_t for an ASN1_TIME diff --git a/res/res_stir_shaken/verification.c b/res/res_stir_shaken/verification.c index 04f003504e..ab137535fd 100644 --- a/res/res_stir_shaken/verification.c +++ b/res/res_stir_shaken/verification.c @@ -345,7 +345,8 @@ static enum ast_stir_shaken_vs_response_code check_cert( } ast_trace(3,"%s: Checking ctx against CA ctx\n", ctx->tag); - res = crypto_is_cert_trusted(ctx->eprofile->vcfg_common.tcs, ctx->xcert, &err_msg); + res = crypto_is_cert_trusted(ctx->eprofile->vcfg_common.tcs, ctx->xcert, + ctx->cert_chain, &err_msg); if (!res) { SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_VS_CERT_NOT_TRUSTED, LOG_ERROR, "%s: Cert '%s' not trusted: %s\n", @@ -429,8 +430,8 @@ static enum ast_stir_shaken_vs_response_code retrieve_cert_from_url( ctx->tag, ctx->public_url); } - ctx->xcert = crypto_load_cert_from_memory(write_data->stream_buffer, - write_data->stream_bytes_downloaded); + ctx->xcert = crypto_load_cert_chain_from_memory(write_data->stream_buffer, + write_data->stream_bytes_downloaded, &ctx->cert_chain); if (!ctx->xcert) { SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_VS_CERT_CONTENTS_INVALID, LOG_ERROR, "%s: Cert '%s' was not parseable as an X509 certificate\n", @@ -524,7 +525,7 @@ static enum ast_stir_shaken_vs_response_code ctx->tag, ctx->filename, ctx->public_url); } - ctx->xcert = crypto_load_cert_from_file(ctx->filename); + ctx->xcert = crypto_load_cert_chain_from_file(ctx->filename, &ctx->cert_chain); if (!ctx->xcert) { cleanup_cert_from_astdb_and_fs(ctx); SCOPE_EXIT_RTN_VALUE(AST_STIR_SHAKEN_VS_CERT_CONTENTS_INVALID, @@ -651,6 +652,7 @@ static void ctx_destructor(void *obj) ast_free(ctx->raw_key); ast_string_field_free_memory(ctx); X509_free(ctx->xcert); + sk_X509_free(ctx->cert_chain); } enum ast_stir_shaken_vs_response_code diff --git a/res/res_stir_shaken/verification.h b/res/res_stir_shaken/verification.h index 65f5fb6970..b3b448d091 100644 --- a/res/res_stir_shaken/verification.h +++ b/res/res_stir_shaken/verification.h @@ -45,6 +45,7 @@ struct ast_stir_shaken_vs_ctx { unsigned char *raw_key; char expiration[32]; X509 *xcert; + STACK_OF(X509) *cert_chain; enum ast_stir_shaken_vs_response_code failure_reason; };