The branch master has been updated via d803930448476c3a6c50904b1cfb5ef20433652f (commit) via 99d680e6bcb7c5caaf8e94cc2c4dd7367e16d8f4 (commit) via a1e4c8ef8125bfff403d49d60a625f75b3074a79 (commit) from 2d956b320c910a90528e9a3aeb4cf48221dba246 (commit)
- Log ----------------------------------------------------------------- commit d803930448476c3a6c50904b1cfb5ef20433652f Author: Dr. David von Oheimb <david.von.ohe...@siemens.com> Date: Tue Mar 31 16:04:55 2020 +0200 Fix misleading error msg for PBM check w/o secret in OSSL_CMP_validate_msg() Reviewed-by: Matt Caswell <m...@openssl.org> Reviewed-by: David von Oheimb <david.von.ohe...@siemens.com> (Merged from https://github.com/openssl/openssl/pull/11448) commit 99d680e6bcb7c5caaf8e94cc2c4dd7367e16d8f4 Author: Dr. David von Oheimb <david.von.ohe...@siemens.com> Date: Tue Mar 31 13:26:32 2020 +0200 Fix error reporting glitch in X509_STORE_CTX_print_verify_cb() in t_x509.c Reviewed-by: Matt Caswell <m...@openssl.org> Reviewed-by: David von Oheimb <david.von.ohe...@siemens.com> (Merged from https://github.com/openssl/openssl/pull/11448) commit a1e4c8ef8125bfff403d49d60a625f75b3074a79 Author: Dr. David von Oheimb <david.von.ohe...@siemens.com> Date: Mon Mar 30 16:40:14 2020 +0200 Fix bugs in 3GPP exception checking and improve diagnostics in crypt/cmp/cmp_vfy.c Reviewed-by: Matt Caswell <m...@openssl.org> Reviewed-by: David von Oheimb <david.von.ohe...@siemens.com> (Merged from https://github.com/openssl/openssl/pull/11448) ----------------------------------------------------------------------- Summary of changes: crypto/cmp/cmp_err.c | 7 ++- crypto/cmp/cmp_vfy.c | 135 ++++++++++++++++++++++++++++++----------------- crypto/err/openssl.txt | 1 + crypto/x509/t_x509.c | 2 +- include/openssl/cmperr.h | 1 + 5 files changed, 93 insertions(+), 53 deletions(-) diff --git a/crypto/cmp/cmp_err.c b/crypto/cmp/cmp_err.c index d64d60bf1d..0f06fb3b42 100644 --- a/crypto/cmp/cmp_err.c +++ b/crypto/cmp/cmp_err.c @@ -33,6 +33,8 @@ static const ERR_STRING_DATA CMP_str_reasons[] = { "cert and key do not match"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_CHECKAFTER_OUT_OF_RANGE), "checkafter out of range"}, + {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_CHECKING_PBM_NO_SECRET_AVAILABLE), + "checking pbm no secret available"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ENCOUNTERED_KEYUPDATEWARNING), "encountered keyupdatewarning"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ENCOUNTERED_WAITING), @@ -64,8 +66,6 @@ static const ERR_STRING_DATA CMP_str_reasons[] = { {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_RR), "error creating rr"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_PARSING_PKISTATUS), "error parsing pkistatus"}, - {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_PARSING_PKISTATUS), - "error parsing pkistatus"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_PROCESSING_MESSAGE), "error processing message"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_PROTECTING_MESSAGE), @@ -110,8 +110,7 @@ static const ERR_STRING_DATA CMP_str_reasons[] = { {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_POLLING_FAILED), "polling failed"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_POTENTIALLY_INVALID_CERTIFICATE), "potentially invalid certificate"}, - {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_RECEIVED_ERROR), - "received error"}, + {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_RECEIVED_ERROR), "received error"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_RECIPNONCE_UNMATCHED), "recipnonce unmatched"}, {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_REQUEST_NOT_ACCEPTED), diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c index 73f93360d6..11688059da 100644 --- a/crypto/cmp/cmp_vfy.c +++ b/crypto/cmp/cmp_vfy.c @@ -167,6 +167,8 @@ int OSSL_CMP_validate_cert_path(OSSL_CMP_CTX *ctx, X509_STORE *trusted_store, CMPerr(0, CMP_R_POTENTIALLY_INVALID_CERTIFICATE); err: + /* directly output any fresh errors, needed for check_msg_find_cert() */ + OSSL_CMP_CTX_print_errors(ctx); X509_STORE_CTX_free(csc); return valid; } @@ -250,17 +252,22 @@ static int cert_acceptable(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg) { X509_STORE *ts = ctx->trusted; - char *sub, *iss; + int self_issued = X509_check_issued(cert, cert) == X509_V_OK; + char *str; X509_VERIFY_PARAM *vpm = ts != NULL ? X509_STORE_get0_param(ts) : NULL; int time_cmp; - ossl_cmp_log2(INFO, ctx, " considering %s %s with..", desc1, desc2); - if ((sub = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0)) != NULL) - ossl_cmp_log1(INFO, ctx, " subject = %s", sub); - if ((iss = X509_NAME_oneline(X509_get_issuer_name(cert), NULL, 0)) != NULL) - ossl_cmp_log1(INFO, ctx, " issuer = %s", iss); - OPENSSL_free(iss); - OPENSSL_free(sub); + ossl_cmp_log3(INFO, ctx, " considering %s%s %s with..", + self_issued ? "self-issued ": "", desc1, desc2); + if ((str = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0)) != NULL) + ossl_cmp_log1(INFO, ctx, " subject = %s", str); + OPENSSL_free(str); + if (!self_issued) { + str = X509_NAME_oneline(X509_get_issuer_name(cert), NULL, 0); + if (str != NULL) + ossl_cmp_log1(INFO, ctx, " issuer = %s", str); + OPENSSL_free(str); + } if (already_checked(cert, already_checked1) || already_checked(cert, already_checked2)) { @@ -284,7 +291,7 @@ static int cert_acceptable(OSSL_CMP_CTX *ctx, if (!check_kid(ctx, cert, msg->header->senderKID)) return 0; /* acceptable also if there is no senderKID in msg header */ - ossl_cmp_info(ctx, " cert is acceptable"); + ossl_cmp_info(ctx, " cert seems acceptable"); return 1; } @@ -295,38 +302,49 @@ static int check_msg_valid_cert(OSSL_CMP_CTX *ctx, X509_STORE *store, ossl_cmp_warn(ctx, "msg signature verification failed"); return 0; } - if (!OSSL_CMP_validate_cert_path(ctx, store, scrt)) { - ossl_cmp_warn(ctx, "cert path validation failed"); - return 0; - } - return 1; + if (OSSL_CMP_validate_cert_path(ctx, store, scrt)) + return 1; + + ossl_cmp_warn(ctx, + "msg signature validates but cert path validation failed"); + return 0; } /* * Exceptional handling for 3GPP TS 33.310 [3G/LTE Network Domain Security - * (NDS); Authentication Framework (AF)], only to use for IP and if the ctx - * option is explicitly set: use self-issued certificates from extraCerts as - * trust anchor to validate sender cert and msg - + * (NDS); Authentication Framework (AF)], only to use for IP messages + * and if the ctx option is explicitly set: use self-issued certificates + * from extraCerts as trust anchor to validate sender cert and msg - * provided it also can validate the newly enrolled certificate */ static int check_msg_valid_cert_3gpp(OSSL_CMP_CTX *ctx, X509 *scrt, const OSSL_CMP_MSG *msg) { int valid = 0; - X509_STORE *store = X509_STORE_new(); + X509_STORE *store; - if (store != NULL /* store does not include CRLs */ - && ossl_cmp_X509_STORE_add1_certs(store, msg->extraCerts, - 1 /* self-issued only */)) - valid = check_msg_valid_cert(ctx, store, scrt, msg); - if (valid) { + if (!ctx->permitTAInExtraCertsForIR) + return 0; + + if ((store = X509_STORE_new()) == NULL + || !ossl_cmp_X509_STORE_add1_certs(store, msg->extraCerts, + 1 /* self-issued only */)) + goto err; + + /* store does not include CRLs */ + valid = OSSL_CMP_validate_cert_path(ctx, store, scrt); + if (!valid) { + ossl_cmp_warn(ctx, + "also exceptional 3GPP mode cert path validation failed"); + } else { /* - * verify that the newly enrolled certificate (which is assumed to have - * rid == 0) can also be validated with the same trusted store + * verify that the newly enrolled certificate (which assumed rid == + * OSSL_CMP_CERTREQID) can also be validated with the same trusted store */ EVP_PKEY *privkey = OSSL_CMP_CTX_get0_newPkey(ctx, 1); OSSL_CMP_CERTRESPONSE *crep = - ossl_cmp_certrepmessage_get0_certresponse(msg->body->value.ip, 0); + ossl_cmp_certrepmessage_get0_certresponse(msg->body->value.ip, + OSSL_CMP_CERTREQID); X509 *newcrt = ossl_cmp_certresponse_get1_certificate(privkey, crep); /* * maybe better use get_cert_status() from cmp_client.c, which catches @@ -335,6 +353,8 @@ static int check_msg_valid_cert_3gpp(OSSL_CMP_CTX *ctx, X509 *scrt, valid = OSSL_CMP_validate_cert_path(ctx, store, newcrt); X509_free(newcrt); } + + err: X509_STORE_free(store); return valid; } @@ -393,8 +413,13 @@ static int check_msg_all_certs(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg, { int ret = 0; + if (mode_3gpp + && ((!ctx->permitTAInExtraCertsForIR + || ossl_cmp_msg_get_bodytype(msg) != OSSL_CMP_PKIBODY_IP))) + return 0; + ossl_cmp_info(ctx, - mode_3gpp ? "failed; trying now 3GPP mode trusting extraCerts" + mode_3gpp ? "normal mode failed; trying now 3GPP mode trusting extraCerts" : "trying first normal mode using trust store"); if (check_msg_with_certs(ctx, msg->extraCerts, "extraCerts", NULL, NULL, msg, mode_3gpp)) @@ -418,6 +443,12 @@ static int check_msg_all_certs(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg, return ret; } +static int no_log_cb(const char *func, const char *file, int line, + OSSL_CMP_severity level, const char *msg) +{ + return 1; +} + /* verify message signature with any acceptable and valid candidate cert */ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg) { @@ -436,49 +467,52 @@ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg) return 0; } + /* dump any hitherto errors to avoid confusion when printing further ones */ + OSSL_CMP_CTX_print_errors(ctx); + /* * try first cached scrt, used successfully earlier in same transaction, * for validating this and any further msgs where extraCerts may be left out */ - (void)ERR_set_mark(); - if (scrt != NULL - && cert_acceptable(ctx, "previously validated", "sender cert", scrt, - NULL, NULL, msg) - && (check_msg_valid_cert(ctx, ctx->trusted, scrt, msg) + if (scrt != NULL) { + (void)ERR_set_mark(); + ossl_cmp_info(ctx, + "trying to verify msg signature with previously validated cert"); + if (cert_acceptable(ctx, "previously validated", "sender cert", scrt, + NULL, NULL, msg) + && (check_msg_valid_cert(ctx, ctx->trusted, scrt, msg) || check_msg_valid_cert_3gpp(ctx, scrt, msg))) { + (void)ERR_pop_to_mark(); + return 1; + } (void)ERR_pop_to_mark(); - return 1; + /* cached sender cert has shown to be no more successfully usable */ + (void)ossl_cmp_ctx_set0_validatedSrvCert(ctx, NULL); } - (void)ERR_pop_to_mark(); - - /* release any cached sender cert that proved no more successfully usable */ - (void)ossl_cmp_ctx_set0_validatedSrvCert(ctx, NULL); /* enable clearing irrelevant errors in attempts to validate sender certs */ (void)ERR_set_mark(); - ctx->log_cb = NULL; /* temporarily disable logging diagnostic info */ - - if (check_msg_all_certs(ctx, msg, 0 /* using ctx->trusted */) - || check_msg_all_certs(ctx, msg, 1 /* 3gpp */)) { - /* discard any diagnostic info on trying to use certs */ - ctx->log_cb = backup_log_cb; /* restore any logging */ + ctx->log_cb = no_log_cb; /* temporarily disable logging */ + res = check_msg_all_certs(ctx, msg, 0 /* using ctx->trusted */) + || check_msg_all_certs(ctx, msg, 1 /* 3gpp */); + ctx->log_cb = backup_log_cb; + if (res) { + /* discard any diagnostic information on trying to use certs */ (void)ERR_pop_to_mark(); - res = 1; goto end; } /* failed finding a sender cert that verifies the message signature */ - ctx->log_cb = backup_log_cb; /* restore any logging */ (void)ERR_clear_last_mark(); sname = X509_NAME_oneline(sender->d.directoryName, NULL, 0); skid_str = skid == NULL ? NULL : OPENSSL_buf2hexstr(skid->data, skid->length); if (ctx->log_cb != NULL) { - ossl_cmp_info(ctx, "verifying msg signature with valid cert that.."); + ossl_cmp_info(ctx, "trying to verify msg signature with a valid cert that.."); if (sname != NULL) - ossl_cmp_log1(INFO, ctx, "matches msg sender name = %s", sname); + ossl_cmp_log1(INFO, ctx, "matches msg sender = %s", sname); if (skid_str != NULL) - ossl_cmp_log1(INFO, ctx, "matches msg senderKID = %s", skid_str); + ossl_cmp_log1(INFO, ctx, "matches msg senderKID = %s", skid_str); else ossl_cmp_info(ctx, "while msg header does not contain senderKID"); /* re-do the above checks (just) for adding diagnostic information */ @@ -543,6 +577,11 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg) switch (nid) { /* 5.1.3.1. Shared Secret Information */ case NID_id_PasswordBasedMAC: + if (ctx->secretValue == 0) { + CMPerr(0, CMP_R_CHECKING_PBM_NO_SECRET_AVAILABLE); + break; + } + if (verify_PBMAC(msg, ctx->secretValue)) { /* * RFC 4210, 5.3.2: 'Note that if the PKI Message Protection is diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index f467ea909f..80b92f8476 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -2088,6 +2088,7 @@ CMP_R_CERTREQMSG_NOT_FOUND:157:certreqmsg not found CMP_R_CERTRESPONSE_NOT_FOUND:113:certresponse not found CMP_R_CERT_AND_KEY_DO_NOT_MATCH:114:cert and key do not match CMP_R_CHECKAFTER_OUT_OF_RANGE:181:checkafter out of range +CMP_R_CHECKING_PBM_NO_SECRET_AVAILABLE:166:checking pbm no secret available CMP_R_ENCOUNTERED_KEYUPDATEWARNING:176:encountered keyupdatewarning CMP_R_ENCOUNTERED_WAITING:162:encountered waiting CMP_R_ERROR_CALCULATING_PROTECTION:115:error calculating protection diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c index 6ef979c4ff..16dc6eaaf2 100644 --- a/crypto/x509/t_x509.c +++ b/crypto/x509/t_x509.c @@ -472,7 +472,7 @@ int X509_STORE_CTX_print_verify_cb(int ok, X509_STORE_CTX *ctx) BIO_printf(bio, "certs in trust store:\n"); print_store_certs(bio, X509_STORE_CTX_get0_store(ctx)); } - CMPerr(0, X509_R_CERTIFICATE_VERIFICATION_FAILED); + X509err(0, X509_R_CERTIFICATE_VERIFICATION_FAILED); ERR_add_error_mem_bio("\n", bio); BIO_free(bio); } diff --git a/include/openssl/cmperr.h b/include/openssl/cmperr.h index c11f372ab5..312fa52932 100644 --- a/include/openssl/cmperr.h +++ b/include/openssl/cmperr.h @@ -44,6 +44,7 @@ int ERR_load_CMP_strings(void); # define CMP_R_CERTRESPONSE_NOT_FOUND 113 # define CMP_R_CERT_AND_KEY_DO_NOT_MATCH 114 # define CMP_R_CHECKAFTER_OUT_OF_RANGE 181 +# define CMP_R_CHECKING_PBM_NO_SECRET_AVAILABLE 166 # define CMP_R_ENCOUNTERED_KEYUPDATEWARNING 176 # define CMP_R_ENCOUNTERED_WAITING 162 # define CMP_R_ERROR_CALCULATING_PROTECTION 115