From 084d3afd26cc20b41241b70b6c709b76d2a334a5 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Tue, 6 Jul 2021 12:23:51 +0200 Subject: [PATCH] Compensate for CMP-related TODOs removed by PR #15539 Reviewed-by: Dmitry Belyavskiy Reviewed-by: Hugo Landau Reviewed-by: David von Oheimb (Merged from https://github.com/openssl/openssl/pull/16006) --- apps/lib/cmp_mock_srv.c | 8 ++++---- crypto/cmp/cmp_local.h | 3 ++- crypto/cmp/cmp_protect.c | 1 + crypto/cmp/cmp_server.c | 5 +++++ crypto/cmp/cmp_vfy.c | 1 + crypto/crmf/crmf_asn.c | 1 + crypto/crmf/crmf_lib.c | 12 ++++++++++++ crypto/crmf/crmf_local.h | 3 ++- crypto/crmf/crmf_pbm.c | 2 ++ 9 files changed, 30 insertions(+), 6 deletions(-) diff --git a/apps/lib/cmp_mock_srv.c b/apps/lib/cmp_mock_srv.c index d890f7fde0..7e6f99fd52 100644 --- a/apps/lib/cmp_mock_srv.c +++ b/apps/lib/cmp_mock_srv.c @@ -264,6 +264,7 @@ static OSSL_CMP_PKISI *process_cert_request(OSSL_CMP_SRV_CTX *srv_ctx, if (ctx->certOut != NULL && (*certOut = X509_dup(ctx->certOut)) == NULL) + /* Should better return a cert produced from data in request template */ goto err; if (ctx->chainOut != NULL && (*chainOut = X509_chain_up_ref(ctx->chainOut)) == NULL) @@ -371,10 +372,9 @@ static void process_error(OSSL_CMP_SRV_CTX *srv_ctx, const OSSL_CMP_MSG *error, for (i = 0; i < sk_ASN1_UTF8STRING_num(errorDetails); i++) { if (i > 0) BIO_printf(bio_err, ", "); - BIO_printf(bio_err, "\""); - ASN1_STRING_print(bio_err, - sk_ASN1_UTF8STRING_value(errorDetails, i)); - BIO_printf(bio_err, "\""); + ASN1_STRING_print_ex(bio_err, + sk_ASN1_UTF8STRING_value(errorDetails, i), + ASN1_STRFLGS_ESC_QUOTE); } BIO_printf(bio_err, "\n"); } diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h index a20eeac9dc..c227d7f152 100644 --- a/crypto/cmp/cmp_local.h +++ b/crypto/cmp/cmp_local.h @@ -118,7 +118,7 @@ struct ossl_cmp_ctx_st { int revocationReason; /* revocation reason code to be included in RR */ STACK_OF(OSSL_CMP_ITAV) *genm_ITAVs; /* content of general message */ - /* result returned in responses */ + /* result returned in responses, so far supporting only one certResponse */ int status; /* PKIStatus of last received IP/CP/KUP/RP/error or -1 */ OSSL_CMP_PKIFREETEXT *statusString; /* of last IP/CP/KUP/RP/error */ int failInfoCode; /* failInfoCode of last received IP/CP/KUP/error, or -1 */ @@ -710,6 +710,7 @@ DECLARE_ASN1_FUNCTIONS(OSSL_CMP_PROTECTEDPART) * } -- or HMAC [RFC2104, RFC2202]) */ /*- + * Not supported: * id-DHBasedMac OBJECT IDENTIFIER ::= {1 2 840 113533 7 66 30} * DHBMParameter ::= SEQUENCE { * owf AlgorithmIdentifier, diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c index 7ff46a6dc1..76b9e55d3d 100644 --- a/crypto/cmp/cmp_protect.c +++ b/crypto/cmp/cmp_protect.c @@ -242,6 +242,7 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg) /* * For the case of re-protection remove pre-existing protection. + * Does not remove any pre-existing extraCerts. */ X509_ALGOR_free(msg->header->protectionAlg); msg->header->protectionAlg = NULL; diff --git a/crypto/cmp/cmp_server.c b/crypto/cmp/cmp_server.c index 83bf4ac157..a48631d267 100644 --- a/crypto/cmp/cmp_server.c +++ b/crypto/cmp/cmp_server.c @@ -228,6 +228,7 @@ static OSSL_CMP_MSG *process_cert_request(OSSL_CMP_SRV_CTX *srv_ctx, msg = ossl_cmp_certrep_new(srv_ctx->ctx, bodytype, certReqId, si, certOut, NULL /* enc */, chainOut, caPubs, srv_ctx->sendUnprotectedErrors); + /* When supporting OSSL_CRMF_POPO_KEYENC, "enc" will need to be set */ if (msg == NULL) ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_CREATING_CERTREP); @@ -553,6 +554,7 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx, rsp = process_pollReq(srv_ctx, req); break; default: + /* Other request message types are not supported */ ERR_raise(ERR_LIB_CMP, CMP_R_UNEXPECTED_PKIBODY); break; } @@ -564,6 +566,7 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx, int flags = 0; unsigned long err = ERR_peek_error_data(&data, &flags); int fail_info = 1 << OSSL_CMP_PKIFAILUREINFO_badRequest; + /* fail_info is not very specific */ OSSL_CMP_PKISI *si = NULL; if (ctx->transactionID == NULL) { @@ -607,6 +610,8 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx, case OSSL_CMP_PKIBODY_PKICONF: case OSSL_CMP_PKIBODY_GENP: case OSSL_CMP_PKIBODY_ERROR: + /* Other terminating response message types are not supported */ + /* Prepare for next transaction, ignoring any errors here: */ (void)OSSL_CMP_CTX_set1_transactionID(ctx, NULL); (void)OSSL_CMP_CTX_set1_senderNonce(ctx, NULL); ctx->status = OSSL_CMP_PKISTATUS_unspecified; /* transaction closed */ diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c index cc71c6ce04..d41e9e742e 100644 --- a/crypto/cmp/cmp_vfy.c +++ b/crypto/cmp/cmp_vfy.c @@ -456,6 +456,7 @@ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg) if (sender == NULL || msg->body == NULL) return 0; /* other NULL cases already have been checked */ if (sender->type != GEN_DIRNAME) { + /* So far, only X509_NAME is supported */ ERR_raise(ERR_LIB_CMP, CMP_R_SENDER_GENERALNAME_TYPE_NOT_SUPPORTED); return 0; } diff --git a/crypto/crmf/crmf_asn.c b/crypto/crmf/crmf_asn.c index 3a5bc3e71c..85b4213934 100644 --- a/crypto/crmf/crmf_asn.c +++ b/crypto/crmf/crmf_asn.c @@ -84,6 +84,7 @@ ASN1_CHOICE(OSSL_CRMF_POPOPRIVKEY) = { ASN1_IMP(OSSL_CRMF_POPOPRIVKEY, value.dhMAC, ASN1_BIT_STRING, 2), ASN1_IMP(OSSL_CRMF_POPOPRIVKEY, value.agreeMAC, OSSL_CRMF_PKMACVALUE, 3), ASN1_IMP(OSSL_CRMF_POPOPRIVKEY, value.encryptedKey, ASN1_NULL, 4), + /* When supported, ASN1_NULL needs to be replaced by CMS_ENVELOPEDDATA */ } ASN1_CHOICE_END(OSSL_CRMF_POPOPRIVKEY) IMPLEMENT_ASN1_FUNCTIONS(OSSL_CRMF_POPOPRIVKEY) diff --git a/crypto/crmf/crmf_lib.c b/crypto/crmf/crmf_lib.c index 4e90cbe02c..e57192fc83 100644 --- a/crypto/crmf/crmf_lib.c +++ b/crypto/crmf/crmf_lib.c @@ -505,6 +505,12 @@ int OSSL_CRMF_MSGS_verify_popo(const OSSL_CRMF_MSGS *reqs, ERR_raise(ERR_LIB_CRMF, CRMF_R_POPO_INCONSISTENT_PUBLIC_KEY); return 0; } + + /* + * Should check at this point the contents of the authInfo sub-field + * as requested in FR #19807 according to RFC 4211 section 4.1. + */ + it = ASN1_ITEM_rptr(OSSL_CRMF_POPOSIGNINGKEYINPUT); asn = sig->poposkInput; } else { @@ -521,6 +527,12 @@ int OSSL_CRMF_MSGS_verify_popo(const OSSL_CRMF_MSGS *reqs, return 0; break; case OSSL_CRMF_POPO_KEYENC: + /* + * When OSSL_CMP_certrep_new() supports encrypted certs, + * should return 1 if the type of req->popo->value.keyEncipherment + * is OSSL_CRMF_POPOPRIVKEY_SUBSEQUENTMESSAGE and + * its value.subsequentMessage == OSSL_CRMF_SUBSEQUENTMESSAGE_ENCRCERT + */ case OSSL_CRMF_POPO_KEYAGREE: default: ERR_raise(ERR_LIB_CRMF, CRMF_R_UNSUPPORTED_POPO_METHOD); diff --git a/crypto/crmf/crmf_local.h b/crypto/crmf/crmf_local.h index f8d028442b..e8937b4231 100644 --- a/crypto/crmf/crmf_local.h +++ b/crypto/crmf/crmf_local.h @@ -188,6 +188,7 @@ typedef struct ossl_crmf_popoprivkey_st { ASN1_BIT_STRING *dhMAC; /* 2 */ /* Deprecated */ OSSL_CRMF_PKMACVALUE *agreeMAC; /* 3 */ ASN1_NULL *encryptedKey; /* 4 */ + /* When supported, ASN1_NULL needs to be replaced by CMS_ENVELOPEDDATA */ } value; } OSSL_CRMF_POPOPRIVKEY; DECLARE_ASN1_FUNCTIONS(OSSL_CRMF_POPOPRIVKEY) @@ -329,7 +330,7 @@ struct ossl_crmf_certtemplate_st { struct ossl_crmf_certrequest_st { ASN1_INTEGER *certReqId; OSSL_CRMF_CERTTEMPLATE *certTemplate; - STACK_OF(OSSL_CRMF_ATTRIBUTETYPEANDVALUE) *controls; + STACK_OF(OSSL_CRMF_ATTRIBUTETYPEANDVALUE /* Controls expanded */) *controls; } /* OSSL_CRMF_CERTREQUEST */; DECLARE_ASN1_FUNCTIONS(OSSL_CRMF_CERTREQUEST) DECLARE_ASN1_DUP_FUNCTION(OSSL_CRMF_CERTREQUEST) diff --git a/crypto/crmf/crmf_pbm.c b/crypto/crmf/crmf_pbm.c index ec32e30598..d4c7af38cb 100644 --- a/crypto/crmf/crmf_pbm.c +++ b/crypto/crmf/crmf_pbm.c @@ -123,6 +123,7 @@ OSSL_CRMF_PBMPARAMETER *OSSL_CRMF_pbmp_new(OSSL_LIB_CTX *libctx, size_t slen, * |outlen| if not NULL, will set variable to the length of the mac on success * returns 1 on success, 0 on error */ +/* could be combined with other MAC calculations in the library */ int OSSL_CRMF_pbm_new(OSSL_LIB_CTX *libctx, const char *propq, const OSSL_CRMF_PBMPARAMETER *pbmp, const unsigned char *msg, size_t msglen, @@ -203,6 +204,7 @@ int OSSL_CRMF_pbm_new(OSSL_LIB_CTX *libctx, const char *propq, ERR_raise(ERR_LIB_CRMF, CRMF_R_UNSUPPORTED_ALGORITHM); goto err; } + /* could be generalized to allow non-HMAC: */ if (EVP_Q_mac(libctx, "HMAC", propq, hmac_mdname, NULL, basekey, bklen, msg, msglen, mac_res, EVP_MAX_MD_SIZE, outlen) == NULL) goto err; -- 2.34.1