From a0745e2be6635ffdf286ba5bc3bd867c8d4152a9 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Fri, 28 Aug 2020 12:11:31 +0200 Subject: [PATCH] Clean up CMP chain building for CMP signer, TLS client, and newly enrolled certs * Use strenghtened cert chain building, verifying chain using optional trust store while making sure that no certificate status (e.g., CRL) checks are done * Use OSSL_CMP_certConf_cb() by default and move its doc to OSSL_CMP_CTX_new.pod * Simplify certificate and cert store loading in apps/cmp.c Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/12741) --- apps/cmp.c | 255 +++++++++------------ crypto/cmp/cmp_client.c | 46 +++- crypto/cmp/cmp_ctx.c | 13 +- crypto/cmp/cmp_local.h | 1 + crypto/cmp/cmp_protect.c | 47 ++-- doc/internal/man3/ossl_cmp_msg_protect.pod | 2 +- doc/man1/openssl-cmp.pod.in | 1 + doc/man3/OSSL_CMP_CTX_new.pod | 19 +- doc/man3/OSSL_CMP_exec_certreq.pod | 15 +- 9 files changed, 201 insertions(+), 198 deletions(-) diff --git a/apps/cmp.c b/apps/cmp.c index f9b50fc659..7524930b8f 100644 --- a/apps/cmp.c +++ b/apps/cmp.c @@ -888,23 +888,6 @@ static OSSL_CMP_MSG *read_write_req_resp(OSSL_CMP_CTX *ctx, return res; } -static int set1_store_parameters(X509_STORE *ts) -{ - if (ts == NULL) - return 0; - - /* copy vpm to store */ - if (!X509_STORE_set1_param(ts, vpm /* may be NULL */)) { - BIO_printf(bio_err, "error setting verification parameters\n"); - OSSL_CMP_CTX_print_errors(cmp_ctx); - return 0; - } - - X509_STORE_set_verify_cb(ts, X509_STORE_CTX_print_verify_cb); - - return 1; -} - static int set_name(const char *str, int (*set_fn) (OSSL_CMP_CTX *ctx, const X509_NAME *name), OSSL_CMP_CTX *ctx, const char *desc) @@ -990,6 +973,24 @@ static X509_STORE *load_certstore(char *input, const char *desc) return store; } +static X509_STORE *load_trusted(char *input, int for_new_cert, const char *desc) +{ + X509_STORE *ts = load_certstore(input, desc); + + if (ts == NULL) + return NULL; + X509_STORE_set_verify_cb(ts, X509_STORE_CTX_print_verify_cb); + + /* copy vpm to store */ + if (X509_STORE_set1_param(ts, vpm /* may be NULL */) + && (for_new_cert || truststore_set_host_etc(ts, NULL))) + return ts; + BIO_printf(bio_err, "error setting verification parameters\n"); + OSSL_CMP_CTX_print_errors(cmp_ctx); + X509_STORE_free(ts); + return NULL; +} + /* TODO potentially move to apps/lib/apps.c */ static STACK_OF(X509) *load_certs_multifile(char *files, const char *pass, const char *desc) @@ -1025,31 +1026,20 @@ static STACK_OF(X509) *load_certs_multifile(char *files, } typedef int (*add_X509_stack_fn_t)(void *ctx, const STACK_OF(X509) *certs); -typedef int (*add_X509_fn_t)(void *ctx, const X509 *cert); static int setup_certs(char *files, const char *desc, void *ctx, - add_X509_stack_fn_t addn_fn, add_X509_fn_t add1_fn) + add_X509_stack_fn_t set1_fn) { - int ret = 1; + STACK_OF(X509) *certs; + int ok; - if (files != NULL) { - STACK_OF(X509) *certs = load_certs_multifile(files, opt_otherpass, - desc); - if (certs == NULL) { - ret = 0; - } else { - if (addn_fn != NULL) { - ret = (*addn_fn)(ctx, certs); - } else { - int i; - - for (i = 0; i < sk_X509_num(certs /* may be NULL */); i++) - ret &= (*add1_fn)(ctx, sk_X509_value(certs, i)); - } - sk_X509_pop_free(certs, X509_free); - } - } - return ret; + if (files == NULL) + return 1; + if ((certs = load_certs_multifile(files, opt_otherpass, desc)) == NULL) + return 0; + ok = (*set1_fn)(ctx, certs); + sk_X509_pop_free(certs, X509_free); + return ok; } @@ -1175,13 +1165,9 @@ static OSSL_CMP_SRV_CTX *setup_srv_ctx(ENGINE *engine) if (opt_srv_trusted != NULL) { X509_STORE *ts = - load_certstore(opt_srv_trusted, "certificates trusted by server"); + load_trusted(opt_srv_trusted, 0, "certs trusted by server"); - if (ts == NULL) - goto err; - if (!set1_store_parameters(ts) - || !truststore_set_host_etc(ts, NULL) - || !OSSL_CMP_CTX_set0_trustedStore(ctx, ts)) { + if (ts == NULL || !OSSL_CMP_CTX_set0_trustedStore(ctx, ts)) { X509_STORE_free(ts); goto err; } @@ -1190,7 +1176,7 @@ static OSSL_CMP_SRV_CTX *setup_srv_ctx(ENGINE *engine) } if (!setup_certs(opt_srv_untrusted, "untrusted certificates for mock server", ctx, - (add_X509_stack_fn_t)OSSL_CMP_CTX_set1_untrusted, NULL)) + (add_X509_stack_fn_t)OSSL_CMP_CTX_set1_untrusted)) goto err; if (opt_rsp_cert == NULL) { @@ -1212,12 +1198,10 @@ static OSSL_CMP_SRV_CTX *setup_srv_ctx(ENGINE *engine) /* TODO find a cleaner solution not requiring type casts */ if (!setup_certs(opt_rsp_extracerts, "CMP extra certificates for mock server", srv_ctx, - (add_X509_stack_fn_t)ossl_cmp_mock_srv_set1_chainOut, - NULL)) + (add_X509_stack_fn_t)ossl_cmp_mock_srv_set1_chainOut)) goto err; if (!setup_certs(opt_rsp_capubs, "caPubs for mock server", srv_ctx, - (add_X509_stack_fn_t)ossl_cmp_mock_srv_set1_caPubsOut, - NULL)) + (add_X509_stack_fn_t)ossl_cmp_mock_srv_set1_caPubsOut)) goto err; (void)ossl_cmp_mock_srv_set_pollCount(srv_ctx, opt_poll_count); (void)ossl_cmp_mock_srv_set_checkAfterTime(srv_ctx, opt_check_after); @@ -1271,16 +1255,15 @@ static OSSL_CMP_SRV_CTX *setup_srv_ctx(ENGINE *engine) static int setup_verification_ctx(OSSL_CMP_CTX *ctx) { if (!setup_certs(opt_untrusted, "untrusted certificates", ctx, - (add_X509_stack_fn_t)OSSL_CMP_CTX_set1_untrusted, - NULL)) - goto err; + (add_X509_stack_fn_t)OSSL_CMP_CTX_set1_untrusted)) + return 0; if (opt_srvcert != NULL || opt_trusted != NULL) { - X509_STORE *ts = NULL; + X509 *srvcert; + X509_STORE *ts; + int ok; if (opt_srvcert != NULL) { - X509 *srvcert; - if (opt_trusted != NULL) { CMP_warn("-trusted option is ignored since -srvcert option is present"); opt_trusted = NULL; @@ -1291,33 +1274,22 @@ static int setup_verification_ctx(OSSL_CMP_CTX *ctx) } srvcert = load_cert_pwd(opt_srvcert, opt_otherpass, "directly trusted CMP server certificate"); - if (srvcert == NULL) - /* - * opt_otherpass is needed in case - * opt_srvcert is an encrypted PKCS#12 file - */ - goto err; - if (!OSSL_CMP_CTX_set1_srvCert(ctx, srvcert)) { - X509_free(srvcert); - goto oom; - } + ok = srvcert != NULL && OSSL_CMP_CTX_set1_srvCert(ctx, srvcert); X509_free(srvcert); - if ((ts = X509_STORE_new()) == NULL) - goto oom; + if (!ok) + return 0; } - if (opt_trusted != NULL - && (ts = load_certstore(opt_trusted, "trusted certificates")) - == NULL) - goto err; - if (!set1_store_parameters(ts) /* also copies vpm */ - /* - * clear any expected host/ip/email address; - * opt_expect_sender is used instead - */ - || !truststore_set_host_etc(ts, NULL) - || !OSSL_CMP_CTX_set0_trustedStore(ctx, ts)) { - X509_STORE_free(ts); - goto oom; + if (opt_trusted != NULL) { + /* + * the 0 arg below clears any expected host/ip/email address; + * opt_expect_sender is used instead + */ + ts = load_trusted(opt_trusted, 0, "certs trusted by client"); + + if (ts == NULL || !OSSL_CMP_CTX_set0_trustedStore(ctx, ts)) { + X509_STORE_free(ts); + return 0; + } } } @@ -1330,14 +1302,11 @@ static int setup_verification_ctx(OSSL_CMP_CTX *ctx) if (opt_out_trusted != NULL) { /* for use in OSSL_CMP_certConf_cb() */ X509_VERIFY_PARAM *out_vpm = NULL; X509_STORE *out_trusted = - load_certstore(opt_out_trusted, - "trusted certs for verifying newly enrolled cert"); + load_trusted(opt_out_trusted, 1, + "trusted certs for verifying newly enrolled cert"); if (out_trusted == NULL) - goto err; - /* any -verify_hostname, -verify_ip, and -verify_email apply here */ - if (!set1_store_parameters(out_trusted)) - goto oom; + return 0; /* ignore any -attime here, new certs are current anyway */ out_vpm = X509_STORE_get0_param(out_trusted); X509_VERIFY_PARAM_clear_flags(out_vpm, X509_V_FLAG_USE_CHECK_TIME); @@ -1351,14 +1320,7 @@ static int setup_verification_ctx(OSSL_CMP_CTX *ctx) if (opt_implicit_confirm) (void)OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_IMPLICIT_CONFIRM, 1); - (void)OSSL_CMP_CTX_set_certConf_cb(ctx, OSSL_CMP_certConf_cb); - return 1; - - oom: - CMP_err("out of memory"); - err: - return 0; } #ifndef OPENSSL_NO_SOCK @@ -1368,7 +1330,7 @@ static int setup_verification_ctx(OSSL_CMP_CTX *ctx) */ static SSL_CTX *setup_ssl_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) { - STACK_OF(X509) *untrusted_certs = OSSL_CMP_CTX_get0_untrusted(ctx); + STACK_OF(X509) *untrusted = OSSL_CMP_CTX_get0_untrusted(ctx); EVP_PKEY *pkey = NULL; X509_STORE *trust_store = NULL; SSL_CTX *ssl_ctx; @@ -1412,21 +1374,37 @@ static SSL_CTX *setup_ssl_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) sk_X509_pop_free(certs, X509_free); goto err; } - for (i = 0; i < sk_X509_num(untrusted_certs); i++) { - cert = sk_X509_value(untrusted_certs, i); + for (i = 0; i < sk_X509_num(untrusted); i++) { + cert = sk_X509_value(untrusted, i); if (!SSL_CTX_add1_chain_cert(ssl_ctx, cert)) { CMP_err("could not add untrusted cert to TLS client cert chain"); goto err; } } - CMP_debug("trying to build cert chain for own TLS cert"); - if (SSL_CTX_build_cert_chain(ssl_ctx, - SSL_BUILD_CHAIN_FLAG_UNTRUSTED | - SSL_BUILD_CHAIN_FLAG_NO_ROOT)) { - CMP_debug("succeeded building cert chain for own TLS cert"); - } else { - OSSL_CMP_CTX_print_errors(ctx); - CMP_warn("could not build cert chain for own TLS cert"); + + { + X509_VERIFY_PARAM *tls_vpm = NULL; + unsigned long bak_flags = 0; /* compiler warns without init */ + + if (trust_store != NULL) { + tls_vpm = X509_STORE_get0_param(trust_store); + bak_flags = X509_VERIFY_PARAM_get_flags(tls_vpm); + /* disable any cert status/revocation checking etc. */ + X509_VERIFY_PARAM_clear_flags(tls_vpm, + ~(X509_V_FLAG_USE_CHECK_TIME + | X509_V_FLAG_NO_CHECK_TIME)); + } + CMP_debug("trying to build cert chain for own TLS cert"); + if (SSL_CTX_build_cert_chain(ssl_ctx, + SSL_BUILD_CHAIN_FLAG_UNTRUSTED | + SSL_BUILD_CHAIN_FLAG_NO_ROOT)) { + CMP_debug("success building cert chain for own TLS cert"); + } else { + OSSL_CMP_CTX_print_errors(ctx); + CMP_warn("could not build cert chain for own TLS cert"); + } + if (trust_store != NULL) + X509_VERIFY_PARAM_set_flags(tls_vpm, bak_flags); } /* If present we append to the list also the certs from opt_tls_extra */ @@ -1503,17 +1481,17 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) { if (!opt_unprotected_requests && opt_secret == NULL && opt_cert == NULL) { CMP_err("must give client credentials unless -unprotected_requests is set"); - goto err; + return 0; } if (opt_ref == NULL && opt_cert == NULL && opt_subject == NULL) { /* cert or subject should determine the sender */ CMP_err("must give -ref if no -cert and no -subject given"); - goto err; + return 0; } if (!opt_secret && ((opt_cert == NULL) != (opt_key == NULL))) { CMP_err("must give both -cert and -key options or neither"); - goto err; + return 0; } if (opt_secret != NULL) { char *pass_string = get_passwd(opt_secret, "PBMAC"); @@ -1526,7 +1504,7 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) strlen(pass_string)); clear_free(pass_string); if (res == 0) - goto err; + return 0; } if (opt_cert != NULL || opt_key != NULL) CMP_warn("no signature-based protection used since -secret is given"); @@ -1534,7 +1512,7 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) if (opt_ref != NULL && !OSSL_CMP_CTX_set1_referenceValue(ctx, (unsigned char *)opt_ref, strlen(opt_ref))) - goto err; + return 0; if (opt_key != NULL) { EVP_PKEY *pkey = load_key_pwd(opt_key, opt_keyform, opt_keypass, engine, @@ -1542,7 +1520,7 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) if (pkey == NULL || !OSSL_CMP_CTX_set1_pkey(ctx, pkey)) { EVP_PKEY_free(pkey); - goto err; + return 0; } EVP_PKEY_free(pkey); } @@ -1553,38 +1531,35 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) X509 *cert; STACK_OF(X509) *certs = NULL; X509_STORE *own_trusted = NULL; - int ok = 0; + int ok; if (!load_cert_certs(opt_cert, &cert, &certs, 0, opt_keypass, "CMP client certificate (optionally with chain)")) /* opt_keypass is needed if opt_cert is an encrypted PKCS#12 file */ - goto err; + return 0; ok = OSSL_CMP_CTX_set1_cert(ctx, cert); X509_free(cert); if (!ok) { CMP_err("out of memory"); } else { if (opt_own_trusted != NULL) { - own_trusted = load_certstore(opt_own_trusted, - "trusted certs for verifying own CMP signer cert"); - ok = own_trusted != NULL - && set1_store_parameters(own_trusted) - && truststore_set_host_etc(own_trusted, NULL); + own_trusted = load_trusted(opt_own_trusted, 0, + "trusted certs for verifying own CMP signer cert"); + ok = own_trusted != NULL; } ok = ok && OSSL_CMP_CTX_build_cert_chain(ctx, own_trusted, certs); } X509_STORE_free(own_trusted); sk_X509_pop_free(certs, X509_free); if (!ok) - goto err; + return 0; } else if (opt_own_trusted != NULL) { CMP_warn("-own_trusted option is ignored without -cert"); } if (!setup_certs(opt_extracerts, "extra certificates for CMP", ctx, - (add_X509_stack_fn_t)OSSL_CMP_CTX_set1_extraCertsOut, - NULL)) - goto err; + (add_X509_stack_fn_t)OSSL_CMP_CTX_set1_extraCertsOut)) + return 0; cleanse(opt_otherpass); if (opt_unprotected_requests) @@ -1595,12 +1570,12 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) if (digest == NID_undef) { CMP_err1("digest algorithm name not recognized: '%s'", opt_digest); - goto err; + return 0; } if (!OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_DIGEST_ALGNID, digest) || !OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_OWF_ALGNID, digest)) { CMP_err1("digest algorithm name not supported: '%s'", opt_digest); - goto err; + return 0; } } @@ -1608,14 +1583,11 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) int mac = OBJ_ln2nid(opt_mac); if (mac == NID_undef) { CMP_err1("MAC algorithm name not recognized: '%s'", opt_mac); - goto err; + return 0; } (void)OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_MAC_ALGNID, mac); } return 1; - - err: - return 0; } /* @@ -1629,7 +1601,7 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) CMP_warn("no -subject given, neither -oldcert nor -cert available as default"); if (!set_name(opt_subject, OSSL_CMP_CTX_set1_subjectName, ctx, "subject") || !set_name(opt_issuer, OSSL_CMP_CTX_set1_issuer, ctx, "issuer")) - goto err; + return 0; if (opt_newkey != NULL) { const char *file = opt_newkey; @@ -1647,7 +1619,7 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) cleanse(opt_newkeypass); if (pkey == NULL || !OSSL_CMP_CTX_set0_newPkey(ctx, priv, pkey)) { EVP_PKEY_free(pkey); - goto err; + return 0; } } @@ -1655,12 +1627,12 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) && !OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_VALIDITY_DAYS, opt_days)) { CMP_err("could not set requested cert validity period"); - goto err; + return 0; } if (opt_policies != NULL && opt_policy_oids != NULL) { CMP_err("cannot have policies both via -policies and via -policy_oids"); - goto err; + return 0; } if (opt_reqexts != NULL || opt_policies != NULL) { @@ -1668,7 +1640,7 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) X509_EXTENSIONS *exts = sk_X509_EXTENSION_new_null(); if (exts == NULL) - goto err; + return 0; X509V3_set_ctx(&ext_ctx, NULL, NULL, NULL, NULL, 0); X509V3_set_nconf(&ext_ctx, conf); if (opt_reqexts != NULL @@ -1676,24 +1648,24 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) CMP_err1("cannot load certificate request extension section '%s'", opt_reqexts); sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free); - goto err; + return 0; } if (opt_policies != NULL && !X509V3_EXT_add_nconf_sk(conf, &ext_ctx, opt_policies, &exts)) { CMP_err1("cannot load policy cert request extension section '%s'", opt_policies); sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free); - goto err; + return 0; } OSSL_CMP_CTX_set0_reqExtensions(ctx, exts); } if (OSSL_CMP_CTX_reqExtensions_have_SAN(ctx) && opt_sans != NULL) { CMP_err("cannot have Subject Alternative Names both via -reqexts and via -sans"); - goto err; + return 0; } if (!set_gennames(ctx, opt_sans, "Subject Alternative Name")) - goto err; + return 0; if (opt_san_nodefault) { if (opt_sans != NULL) @@ -1715,19 +1687,19 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) if ((policy = OBJ_txt2obj(opt_policy_oids, 1)) == 0) { CMP_err1("unknown policy OID '%s'", opt_policy_oids); - goto err; + return 0; } if ((pinfo = POLICYINFO_new()) == NULL) { ASN1_OBJECT_free(policy); - goto err; + return 0; } pinfo->policyid = policy; if (!OSSL_CMP_CTX_push0_policy(ctx, pinfo)) { CMP_err1("cannot add policy with OID '%s'", opt_policy_oids); POLICYINFO_free(pinfo); - goto err; + return 0; } opt_policy_oids = next; } @@ -1743,7 +1715,7 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) load_csr_autofmt(opt_csr, "PKCS#10 CSR for p10cr"); if (csr == NULL) - goto err; + return 0; if (!OSSL_CMP_CTX_set1_p10CSR(ctx, csr)) { X509_REQ_free(csr); goto oom; @@ -1758,7 +1730,7 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) /* opt_keypass is needed if opt_oldcert is an encrypted PKCS#12 file */ if (oldcert == NULL) - goto err; + return 0; if (!OSSL_CMP_CTX_set1_oldCert(ctx, oldcert)) { X509_free(oldcert); goto oom; @@ -1774,7 +1746,6 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine) oom: CMP_err("out of memory"); - err: return 0; } diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c index d5a4f3ced5..4f8a9e2444 100644 --- a/crypto/cmp/cmp_client.c +++ b/crypto/cmp/cmp_client.c @@ -22,6 +22,7 @@ #include "openssl/cmp_util.h" DEFINE_STACK_OF(ASN1_UTF8STRING) +DEFINE_STACK_OF(X509) DEFINE_STACK_OF(X509_CRL) DEFINE_STACK_OF(OSSL_CMP_CERTRESPONSE) DEFINE_STACK_OF(OSSL_CMP_PKISI) @@ -487,14 +488,35 @@ int OSSL_CMP_certConf_cb(OSSL_CMP_CTX *ctx, X509 *cert, int fail_info, const char **text) { X509_STORE *out_trusted = OSSL_CMP_CTX_get_certConf_cb_arg(ctx); + STACK_OF(X509) *chain = NULL; (void)text; /* make (artificial) use of var to prevent compiler warning */ if (fail_info != 0) /* accept any error flagged by CMP core library */ return fail_info; - if (out_trusted != NULL - && !OSSL_CMP_validate_cert_path(ctx, out_trusted, cert)) - fail_info = 1 << OSSL_CMP_PKIFAILUREINFO_incorrectData; + ossl_cmp_debug(ctx, "trying to build chain for newly enrolled cert"); + chain = ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, + out_trusted /* may be NULL */, + ctx->untrusted, cert); + if (sk_X509_num(chain) > 0) + X509_free(sk_X509_shift(chain)); /* remove leaf (EE) cert */ + if (out_trusted != NULL) { + if (chain == NULL) { + ossl_cmp_err(ctx, "failed building chain for newly enrolled cert"); + fail_info = 1 << OSSL_CMP_PKIFAILUREINFO_incorrectData; + } else { + ossl_cmp_debug(ctx, + "succeeded building proper chain for newly enrolled cert"); + } + } else if (chain == NULL) { + ossl_cmp_warn(ctx, "could not build approximate chain for newly enrolled cert, resorting to received extraCerts"); + chain = OSSL_CMP_CTX_get1_extraCertsIn(ctx); + } else { + ossl_cmp_debug(ctx, + "success building approximate chain for newly enrolled cert"); + } + (void)ossl_cmp_ctx_set1_newChain(ctx, chain); + sk_X509_pop_free(chain, X509_free); return fail_info; } @@ -515,10 +537,14 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid, const char *txt = NULL; OSSL_CMP_CERTREPMESSAGE *crepmsg; OSSL_CMP_CERTRESPONSE *crep; + OSSL_CMP_certConf_cb_t cb; X509 *cert; char *subj = NULL; int ret = 1; + if (!ossl_assert(ctx != NULL)) + return 0; + retry: crepmsg = (*resp)->body->value.ip; /* same for cp and kup */ if (sk_OSSL_CMP_CERTRESPONSE_num(crepmsg->response) > 1) { @@ -584,21 +610,19 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid, * OSSL_CMP_PKISTATUS_rejection, fail_info, txt) * not throwing CMP_R_CERTIFICATE_NOT_ACCEPTED with txt * not returning 0 - * since we better leave this for any ctx->certConf_cb to decide + * since we better leave this for the certConf_cb to decide */ } /* - * Execute the certification checking callback function possibly set in ctx, + * Execute the certification checking callback function, * which can determine whether to accept a newly enrolled certificate. * It may overrule the pre-decision reflected in 'fail_info' and '*txt'. */ - if (ctx->certConf_cb - && (fail_info = ctx->certConf_cb(ctx, ctx->newCert, - fail_info, &txt)) != 0) { - if (txt == NULL) - txt = "CMP client application did not accept it"; - } + cb = ctx->certConf_cb != NULL ? ctx->certConf_cb : OSSL_CMP_certConf_cb; + if ((fail_info = cb(ctx, ctx->newCert, fail_info, &txt)) != 0 + && txt == NULL) + txt = "CMP client did not accept it"; if (fail_info != 0) /* immediately log error before any certConf exchange */ ossl_cmp_log1(ERROR, ctx, "rejecting newly enrolled cert with subject: %s", subj); diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c index 5b61108f8b..6bbd3510c7 100644 --- a/crypto/cmp/cmp_ctx.c +++ b/crypto/cmp/cmp_ctx.c @@ -189,6 +189,7 @@ void OSSL_CMP_CTX_free(OSSL_CMP_CTX *ctx) sk_X509_pop_free(ctx->untrusted, X509_free); X509_free(ctx->cert); + sk_X509_pop_free(ctx->chain, X509_free); EVP_PKEY_free(ctx->pkey); ASN1_OCTET_STRING_free(ctx->referenceValue); if (ctx->secretValue != NULL) @@ -489,11 +490,7 @@ int ossl_cmp_ctx_set1_newChain(OSSL_CMP_CTX *ctx, STACK_OF(X509) *newChain) return (ctx->newChain = X509_chain_up_ref(newChain)) != NULL; } -/* - * Returns the stack of certificates received in a response message. - * The stack is duplicated so the caller must handle freeing it! - * Returns pointer to created stack on success, NULL on error - */ +/* Returns the stack of extraCerts received in CertRepMessage, NULL on error */ STACK_OF(X509) *OSSL_CMP_CTX_get1_extraCertsIn(const OSSL_CMP_CTX *ctx) { if (ctx == NULL) { @@ -523,7 +520,7 @@ int ossl_cmp_ctx_set1_extraCertsIn(OSSL_CMP_CTX *ctx, } /* - * Duplicate and set the given stack as the new stack of X509 + * Copies any given stack as the new stack of X509 * certificates to send out in the extraCerts field. */ int OSSL_CMP_CTX_set1_extraCertsOut(OSSL_CMP_CTX *ctx, @@ -596,7 +593,7 @@ STACK_OF(X509) *OSSL_CMP_CTX_get1_caPubs(const OSSL_CMP_CTX *ctx) } /* - * Duplicate and copy the given stack of certificates to the given + * Copies any given stack of certificates to the given * OSSL_CMP_CTX structure so that they may be retrieved later. */ int ossl_cmp_ctx_set1_caPubs(OSSL_CMP_CTX *ctx, STACK_OF(X509) *caPubs) @@ -766,7 +763,7 @@ int OSSL_CMP_CTX_build_cert_chain(OSSL_CMP_CTX *ctx, X509_STORE *own_trusted, return 0; } ossl_cmp_debug(ctx, "success building chain for own CMP signer cert"); - sk_X509_pop_free(chain, X509_free); /* TODO(3.0) replace this by 'ctx->chain = chain;' when ctx->chain is available */ + ctx->chain = chain; return 1; } diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h index d5ac7a521d..434f9e093f 100644 --- a/crypto/cmp/cmp_local.h +++ b/crypto/cmp/cmp_local.h @@ -71,6 +71,7 @@ struct ossl_cmp_ctx_st { /* client authentication */ int unprotectedSend; /* send unprotected PKI messages */ X509 *cert; /* protection cert used to identify and sign for MSG_SIG_ALG */ + STACK_OF(X509) *chain; /* (cached) chain of protection cert including it */ EVP_PKEY *pkey; /* the key pair corresponding to cert */ ASN1_OCTET_STRING *referenceValue; /* optional user name for MSG_MAC_ALG */ ASN1_OCTET_STRING *secretValue; /* password/shared secret for MSG_MAC_ALG */ diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c index b65de09517..6313cc94ce 100644 --- a/crypto/cmp/cmp_protect.c +++ b/crypto/cmp/cmp_protect.c @@ -139,32 +139,37 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg) && (msg->extraCerts = sk_X509_new_null()) == NULL) return 0; - if (ctx->cert != NULL && ctx->pkey != NULL) { - /* make sure that our own cert is included in the first position */ - if (!X509_add_cert(msg->extraCerts, ctx->cert, - X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP - | X509_ADD_FLAG_PREPEND)) - return 0; - /* if we have untrusted certs, try to add intermediate certs */ - if (ctx->untrusted != NULL) { - STACK_OF(X509) *chain; - int res; + /* Add first ctx->cert and its chain if using signature-based protection */ + if (!ctx->unprotectedSend && ctx->secretValue == NULL) { + int flags_prepend = X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP + | X509_ADD_FLAG_PREPEND | X509_ADD_FLAG_NO_SS; + /* if not yet done try to build chain using available untrusted certs */ + if (ctx->chain == NULL) { ossl_cmp_debug(ctx, "trying to build chain for own CMP signer cert"); - chain = ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, NULL, - ctx->untrusted, ctx->cert); - res = X509_add_certs(msg->extraCerts, chain, - X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP - | X509_ADD_FLAG_NO_SS); - sk_X509_pop_free(chain, X509_free); - if (res == 0) { - ossl_cmp_err(ctx, - "could not build chain for own CMP signer cert"); - return 0; + ctx->chain = + ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, NULL, + ctx->untrusted, ctx->cert); + if (ctx->chain != NULL) { + ossl_cmp_debug(ctx, + "success building chain for own CMP signer cert"); + } else { + /* dump errors to avoid confusion when printing further ones */ + OSSL_CMP_CTX_print_errors(ctx); + ossl_cmp_warn(ctx, + "could not build chain for own CMP signer cert"); } + } + if (ctx->chain != NULL) { + if (!X509_add_certs(msg->extraCerts, ctx->chain, flags_prepend)) + return 0; + } else { + /* make sure that at least our own signer cert is included first */ + if (!X509_add_cert(msg->extraCerts, ctx->cert, flags_prepend)) + return 0; ossl_cmp_debug(ctx, - "succeeded building chain for own CMP signer cert"); + "fallback: adding just own CMP signer cert"); } } diff --git a/doc/internal/man3/ossl_cmp_msg_protect.pod b/doc/internal/man3/ossl_cmp_msg_protect.pod index 39f5146530..0a6b70fe9d 100644 --- a/doc/internal/man3/ossl_cmp_msg_protect.pod +++ b/doc/internal/man3/ossl_cmp_msg_protect.pod @@ -46,7 +46,7 @@ It also sets the protectionAlg field in the message header accordingly. ossl_cmp_msg_add_extraCerts() adds elements to the extraCerts field in I. If signature-based message protection is used it adds first the CMP signer cert ctx->cert and then its chain ctx->chain. If this chain is not present in I -tries to build it using ctx->untrusted_certs and caches the result in ctx->chain. +tries to build it using ctx->untrusted and caches the result in ctx->chain. In any case all the certificates explicitly specified to be sent out (i.e., IextraCertsOut>) are added. Note that it will NOT add the root certificate of the chain, i.e, the trust anchor (unless it is part of extraCertsOut). diff --git a/doc/man1/openssl-cmp.pod.in b/doc/man1/openssl-cmp.pod.in index 623e3f7dee..75ee82211d 100644 --- a/doc/man1/openssl-cmp.pod.in +++ b/doc/man1/openssl-cmp.pod.in @@ -681,6 +681,7 @@ Defaults to C as per RFC 4210. =item B<-extracerts> I Certificates to append in the extraCerts field when sending messages. +They can be used as the default CMP signer certificate chain to include. Multiple filenames or URLs may be given, separated by commas and/or whitespace (where in the latter case the whole argument must be enclosed in "..."). diff --git a/doc/man3/OSSL_CMP_CTX_new.pod b/doc/man3/OSSL_CMP_CTX_new.pod index 972cef9047..246c302685 100644 --- a/doc/man3/OSSL_CMP_CTX_new.pod +++ b/doc/man3/OSSL_CMP_CTX_new.pod @@ -48,6 +48,7 @@ OSSL_CMP_CTX_set1_oldCert, OSSL_CMP_CTX_set1_p10CSR, OSSL_CMP_CTX_push0_genm_ITAV, OSSL_CMP_certConf_cb_t, +OSSL_CMP_certConf_cb, OSSL_CMP_CTX_set_certConf_cb, OSSL_CMP_CTX_set_certConf_cb_arg, OSSL_CMP_CTX_get_certConf_cb_arg, @@ -137,6 +138,8 @@ OSSL_CMP_CTX_set1_senderNonce /* certificate confirmation: */ typedef int (*OSSL_CMP_certConf_cb_t)(OSSL_CMP_CTX *ctx, X509 *cert, int fail_info, const char **txt); + int OSSL_CMP_certConf_cb(OSSL_CMP_CTX *ctx, X509 *cert, int fail_info, + const char **text); int OSSL_CMP_CTX_set_certConf_cb(OSSL_CMP_CTX *ctx, OSSL_CMP_certConf_cb_t cb); int OSSL_CMP_CTX_set_certConf_cb_arg(OSSL_CMP_CTX *ctx, void *arg); void *OSSL_CMP_CTX_get_certConf_cb_arg(const OSSL_CMP_CTX *ctx); @@ -430,7 +433,7 @@ list of untrusted certs, which may be empty if unset. OSSL_CMP_CTX_set1_cert() sets the certificate used for CMP message protection. The public key of this B must correspond to -the private key set via B. +the private key set before or thereafter via B. When using signature-based protection of CMP request messages this "protection certificate" will be included first in the extraCerts field. The subject of this B will be used as the sender field of outgoing @@ -552,6 +555,16 @@ OSSL_CMP_CTX_set1_p10CSR() sets the PKCS#10 CSR to be used in P10CR. OSSL_CMP_CTX_push0_genm_ITAV() adds B to the stack in the B which will be the body of a General Message sent with this context. +OSSL_CMP_certConf_cb() is the default certificate confirmation callback function. +If the callback argument is not NULL it must point to a trust store. +In this case the function checks that the newly enrolled certificate can be +verified using this trust store and untrusted certificates from the B, +which have been augmented by the list of extraCerts received. +If the callback argument is NULL the function tries building an approximate +chain as far as possible using the same untrusted certificates from the B, +and if this fails it takes the received extraCerts as fallback. +The resulting cert chain can be retrieved using OSSL_CMP_CTX_get1_newChain(). + OSSL_CMP_CTX_set_certConf_cb() sets the callback used for evaluating the newly enrolled certificate before the library sends, depending on its result, a positive or negative certConf message to the server. The callback has type @@ -644,6 +657,10 @@ OSSL_CMP_CTX_get_status(), and OSSL_CMP_CTX_get_failInfoCode() return the intended value as described above or -1 on error. +OSSL_CMP_certConf_cb() returns B if it is not equal to B<0>, +else B<0> on successful validation, +or else a bit field with the B bit set. + All other functions return 1 on success, 0 on error. =head1 EXAMPLES diff --git a/doc/man3/OSSL_CMP_exec_certreq.pod b/doc/man3/OSSL_CMP_exec_certreq.pod index 098b60ae61..55fa73f563 100644 --- a/doc/man3/OSSL_CMP_exec_certreq.pod +++ b/doc/man3/OSSL_CMP_exec_certreq.pod @@ -13,8 +13,7 @@ OSSL_CMP_P10CR, OSSL_CMP_KUR, OSSL_CMP_try_certreq, OSSL_CMP_exec_RR_ses, -OSSL_CMP_exec_GENM_ses, -OSSL_CMP_certConf_cb +OSSL_CMP_exec_GENM_ses - functions implementing CMP client transactions =head1 SYNOPSIS @@ -33,8 +32,6 @@ OSSL_CMP_certConf_cb #define OSSL_CMP_KUR int OSSL_CMP_try_certreq(OSSL_CMP_CTX *ctx, int req_type, const OSSL_CRMF_MSG *crm, int *checkAfter); - int OSSL_CMP_certConf_cb(OSSL_CMP_CTX *ctx, X509 *cert, int fail_info, - const char **text); X509 *OSSL_CMP_exec_RR_ses(OSSL_CMP_CTX *ctx); STACK_OF(OSSL_CMP_ITAV) *OSSL_CMP_exec_GENM_ses(OSSL_CMP_CTX *ctx); @@ -101,12 +98,6 @@ If the caller decides to abort the pending certificate request and provides a negative value as the B argument then OSSL_CMP_try_certreq() aborts the CMP transaction by sending an error message to the server. -OSSL_CMP_certConf_cb() is a basic certificate confirmation callback validating -that the new certificate can be verified with the trusted/untrusted certificates -in B. -As there is no requirement in RFC 4210 that the certificate can be -validated by the client, this callback is not set by default in the context. - OSSL_CMP_exec_RR_ses() requests the revocation of the certificate specified in the B using L. RFC 4210 is vague in which PKIStatus should be returned by the server. @@ -146,10 +137,6 @@ In the latter case L yields NULL and the output parameter B has been used to assign the received value unless B is NULL. -OSSL_CMP_certConf_cb() returns B if it is not equal to B<0>, -else B<0> on successful validation, -or else a bit field with the B bit set. - OSSL_CMP_exec_RR_ses() returns the pointer to the revoked certificate on success, B on error. This pointer will be freed implicitly by OSSL_CMP_CTX_free(). -- 2.34.1