From 69d3c81ca5c6cb03b0d1d1063fe6a2fa731ff461 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Tue, 13 Sep 2022 15:43:59 +0200 Subject: CMP: fix status held in OSSL_CMP_CTX, in particular for genp messages On this occasion, replace magic constants by mnemonic ones; update doc Reviewed-by: Tomas Mraz Reviewed-by: Todd Short Reviewed-by: David von Oheimb (Merged from https://github.com/openssl/openssl/pull/19205) (cherry picked from commit 19ddcc4cbb43464493a4b82332a1ab96da823451) --- apps/cmp.c | 66 +++++++++++++++++++------------------- crypto/cmp/cmp_client.c | 29 +++++++++++------ crypto/cmp/cmp_ctx.c | 4 +-- crypto/cmp/cmp_msg.c | 2 +- crypto/cmp/cmp_server.c | 12 +++---- crypto/cmp/cmp_status.c | 5 ++- doc/man3/OSSL_CMP_CTX_new.pod | 26 +++++++++++++-- doc/man3/OSSL_CMP_exec_certreq.pod | 9 +++--- include/openssl/cmp.h.in | 15 +++++---- 9 files changed, 103 insertions(+), 65 deletions(-) diff --git a/apps/cmp.c b/apps/cmp.c index f98e5ab938..9b9e405bb2 100644 --- a/apps/cmp.c +++ b/apps/cmp.c @@ -2689,8 +2689,8 @@ static int cmp_server(OSSL_CMP_CTX *srv_cmp_ctx) { (void)OSSL_CMP_CTX_set1_senderNonce(srv_cmp_ctx, NULL); } if (!ret || !keep_alive - || OSSL_CMP_CTX_get_status(srv_cmp_ctx) == -1 - /* transaction closed by OSSL_CMP_CTX_server_perform() */) { + || OSSL_CMP_CTX_get_status(srv_cmp_ctx) != OSSL_CMP_PKISTATUS_trans + /* transaction closed by OSSL_CMP_CTX_server_perform() */) { BIO_free_all(cbio); cbio = NULL; } @@ -2702,6 +2702,35 @@ static int cmp_server(OSSL_CMP_CTX *srv_cmp_ctx) { } #endif +static void print_status(void) +{ + /* print PKIStatusInfo */ + int status = OSSL_CMP_CTX_get_status(cmp_ctx); + char *buf = app_malloc(OSSL_CMP_PKISI_BUFLEN, "PKIStatusInfo buf"); + const char *string = + OSSL_CMP_CTX_snprint_PKIStatus(cmp_ctx, buf, OSSL_CMP_PKISI_BUFLEN); + const char *from = "", *server = ""; + +#ifndef OPENSSL_NO_SOCK + if (opt_server != NULL) { + from = " from "; + server = opt_server; + } +#endif + CMP_print(bio_err, + status == OSSL_CMP_PKISTATUS_accepted + ? OSSL_CMP_LOG_INFO : + status == OSSL_CMP_PKISTATUS_rejection + || status == OSSL_CMP_PKISTATUS_waiting + ? OSSL_CMP_LOG_ERR : OSSL_CMP_LOG_WARNING, + status == OSSL_CMP_PKISTATUS_accepted ? "info" : + status == OSSL_CMP_PKISTATUS_rejection ? "server error" : + status == OSSL_CMP_PKISTATUS_waiting ? "internal error" + : "warning", "received%s%s %s", from, server, + string != NULL ? string : ""); + OPENSSL_free(buf); +} + int cmp_main(int argc, char **argv) { char *configfile = NULL; @@ -2924,39 +2953,10 @@ int cmp_main(int argc, char **argv) default: break; } - if (OSSL_CMP_CTX_get_status(cmp_ctx) < 0) + if (OSSL_CMP_CTX_get_status(cmp_ctx) < OSSL_CMP_PKISTATUS_accepted) goto err; /* we got no response, maybe even did not send request */ - { - /* print PKIStatusInfo */ - int status = OSSL_CMP_CTX_get_status(cmp_ctx); - char *buf = app_malloc(OSSL_CMP_PKISI_BUFLEN, "PKIStatusInfo buf"); - const char *string = - OSSL_CMP_CTX_snprint_PKIStatus(cmp_ctx, buf, - OSSL_CMP_PKISI_BUFLEN); - const char *from = "", *server = ""; - -#ifndef OPENSSL_NO_SOCK - if (opt_server != NULL) { - from = " from "; - server = opt_server; - } -#endif - CMP_print(bio_err, - status == OSSL_CMP_PKISTATUS_accepted - ? OSSL_CMP_LOG_INFO : - status == OSSL_CMP_PKISTATUS_rejection - || status == OSSL_CMP_PKISTATUS_waiting - ? OSSL_CMP_LOG_ERR : OSSL_CMP_LOG_WARNING, - status == OSSL_CMP_PKISTATUS_accepted ? "info" : - status == OSSL_CMP_PKISTATUS_rejection ? "server error" : - status == OSSL_CMP_PKISTATUS_waiting ? "internal error" - : "warning", - "received%s%s %s", from, server, - string != NULL ? string : ""); - OPENSSL_free(buf); - } - + print_status(); if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_extraCertsIn(cmp_ctx), opt_extracertsout, "extra") < 0) ret = 0; diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c index c7674ce088..607f5dafd6 100644 --- a/crypto/cmp/cmp_client.c +++ b/crypto/cmp/cmp_client.c @@ -94,7 +94,8 @@ static int save_statusInfo(OSSL_CMP_CTX *ctx, OSSL_CMP_PKISI *si) if (!ossl_assert(ctx != NULL && si != NULL)) return 0; - if ((ctx->status = ossl_cmp_pkisi_get_status(si)) < 0) + ctx->status = ossl_cmp_pkisi_get_status(si); + if (ctx->status < OSSL_CMP_PKISTATUS_accepted) return 0; ctx->failInfoCode = 0; @@ -357,7 +358,10 @@ static int poll_for_response(OSSL_CMP_CTX *ctx, int sleep, int rid, return 0; } -/* Send certConf for IR, CR or KUR sequences and check response */ +/* + * Send certConf for IR, CR or KUR sequences and check response, + * not modifying ctx->status during the certConf exchange + */ int ossl_cmp_exchange_certConf(OSSL_CMP_CTX *ctx, int fail_info, const char *txt) { @@ -386,6 +390,7 @@ int ossl_cmp_exchange_error(OSSL_CMP_CTX *ctx, int status, int fail_info, OSSL_CMP_MSG *PKIconf = NULL; int res = 0; + /* not overwriting ctx->status on error exchange */ if ((si = OSSL_CMP_STATUSINFO_new(status, fail_info, txt)) == NULL) goto err; /* ossl_cmp_error_new() also checks if all necessary options are set */ @@ -643,7 +648,7 @@ static int initial_certreq(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *req; int res; - ctx->status = -1; + ctx->status = OSSL_CMP_PKISTATUS_request; if (!ossl_cmp_ctx_set0_newCert(ctx, NULL)) return 0; @@ -654,6 +659,7 @@ static int initial_certreq(OSSL_CMP_CTX *ctx, if ((req = ossl_cmp_certreq_new(ctx, req_type, crm)) == NULL) return 0; + ctx->status = OSSL_CMP_PKISTATUS_trans; res = send_receive_check(ctx, req, p_rep, rep_type); OSSL_CMP_MSG_free(req); return res; @@ -743,16 +749,17 @@ int OSSL_CMP_exec_RR_ses(OSSL_CMP_CTX *ctx) ERR_raise(ERR_LIB_CMP, CMP_R_INVALID_ARGS); return 0; } + ctx->status = OSSL_CMP_PKISTATUS_request; if (ctx->oldCert == NULL && ctx->p10CSR == NULL) { ERR_raise(ERR_LIB_CMP, CMP_R_MISSING_REFERENCE_CERT); return 0; } - ctx->status = -1; /* OSSL_CMP_rr_new() also checks if all necessary options are set */ if ((rr = ossl_cmp_rr_new(ctx)) == NULL) goto end; + ctx->status = OSSL_CMP_PKISTATUS_trans; if (!send_receive_check(ctx, rr, &rp, OSSL_CMP_PKIBODY_RP)) goto end; @@ -861,27 +868,31 @@ STACK_OF(OSSL_CMP_ITAV) *OSSL_CMP_exec_GENM_ses(OSSL_CMP_CTX *ctx) { OSSL_CMP_MSG *genm; OSSL_CMP_MSG *genp = NULL; - STACK_OF(OSSL_CMP_ITAV) *rcvd_itavs = NULL; + STACK_OF(OSSL_CMP_ITAV) *itavs = NULL; if (ctx == NULL) { ERR_raise(ERR_LIB_CMP, CMP_R_INVALID_ARGS); - return 0; + return NULL; } - ctx->status = -1; + ctx->status = OSSL_CMP_PKISTATUS_request; if ((genm = ossl_cmp_genm_new(ctx)) == NULL) goto err; + ctx->status = OSSL_CMP_PKISTATUS_trans; if (!send_receive_check(ctx, genm, &genp, OSSL_CMP_PKIBODY_GENP)) goto err; + ctx->status = OSSL_CMP_PKISTATUS_accepted; + itavs = genp->body->value.genp; + if (itavs == NULL) + itavs = sk_OSSL_CMP_ITAV_new_null(); /* received stack of itavs not to be freed with the genp */ - rcvd_itavs = genp->body->value.genp; genp->body->value.genp = NULL; err: OSSL_CMP_MSG_free(genm); OSSL_CMP_MSG_free(genp); - return rcvd_itavs; /* recv_itavs == NULL indicates an error */ + return itavs; /* NULL indicates error case */ } diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c index f514ab27e0..5c0f51f163 100644 --- a/crypto/cmp/cmp_ctx.c +++ b/crypto/cmp/cmp_ctx.c @@ -112,7 +112,7 @@ OSSL_CMP_CTX *OSSL_CMP_CTX_new(OSSL_LIB_CTX *libctx, const char *propq) ctx->log_verbosity = OSSL_CMP_LOG_INFO; - ctx->status = -1; + ctx->status = OSSL_CMP_PKISTATUS_unspecified; ctx->failInfoCode = -1; ctx->keep_alive = 1; @@ -155,7 +155,7 @@ int OSSL_CMP_CTX_reinit(OSSL_CMP_CTX *ctx) ossl_cmp_debug(ctx, "disconnected from CMP server"); ctx->http_ctx = NULL; } - ctx->status = -1; + ctx->status = OSSL_CMP_PKISTATUS_unspecified; ctx->failInfoCode = -1; return ossl_cmp_ctx_set0_statusString(ctx, NULL) diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c index 9890fc3dcb..5526f99eca 100644 --- a/crypto/cmp/cmp_msg.c +++ b/crypto/cmp/cmp_msg.c @@ -463,7 +463,7 @@ OSSL_CMP_MSG *ossl_cmp_certrep_new(OSSL_CMP_CTX *ctx, int bodytype, OSSL_CMP_MSG *msg = NULL; OSSL_CMP_CERTREPMESSAGE *repMsg = NULL; OSSL_CMP_CERTRESPONSE *resp = NULL; - int status = -1; + int status = OSSL_CMP_PKISTATUS_unspecified; if (!ossl_assert(ctx != NULL && si != NULL)) return NULL; diff --git a/crypto/cmp/cmp_server.c b/crypto/cmp/cmp_server.c index 7ce4662aee..946c32c45e 100644 --- a/crypto/cmp/cmp_server.c +++ b/crypto/cmp/cmp_server.c @@ -338,7 +338,7 @@ static OSSL_CMP_MSG *process_certConf(OSSL_CMP_SRV_CTX *srv_ctx, num = sk_OSSL_CMP_CERTSTATUS_num(ccc); if (OSSL_CMP_CTX_get_option(ctx, OSSL_CMP_OPT_IMPLICIT_CONFIRM) == 1 - || ctx->status != -2 /* transaction not open */) { + || ctx->status != OSSL_CMP_PKISTATUS_trans) { ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_UNEXPECTED_CERTCONF); return NULL; } @@ -359,8 +359,8 @@ static OSSL_CMP_MSG *process_certConf(OSSL_CMP_SRV_CTX *srv_ctx, if (!srv_ctx->process_certConf(srv_ctx, req, certReqId, certHash, si)) return NULL; /* reason code may be: CMP_R_CERTHASH_UNMATCHED */ - if (si != NULL && ossl_cmp_pkisi_get_status(si) - != OSSL_CMP_PKISTATUS_accepted) { + if (si != NULL + && ossl_cmp_pkisi_get_status(si) != OSSL_CMP_PKISTATUS_accepted) { int pki_status = ossl_cmp_pkisi_get_status(si); const char *str = ossl_cmp_PKIStatus_to_string(pki_status); @@ -595,8 +595,8 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx, else ossl_cmp_log(ERR, ctx, "cannot send proper CMP response"); - /* possibly close the transaction */ - ctx->status = -2; /* this indicates transaction is open */ + /* determine whether to keep the transaction open or not */ + ctx->status = OSSL_CMP_PKISTATUS_trans; switch (rsp_type) { case OSSL_CMP_PKIBODY_IP: case OSSL_CMP_PKIBODY_CP: @@ -611,7 +611,7 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx, case OSSL_CMP_PKIBODY_ERROR: (void)OSSL_CMP_CTX_set1_transactionID(ctx, NULL); (void)OSSL_CMP_CTX_set1_senderNonce(ctx, NULL); - ctx->status = -1; /* transaction closed */ + ctx->status = OSSL_CMP_PKISTATUS_unspecified; /* transaction closed */ default: /* not closing transaction in other cases */ break; diff --git a/crypto/cmp/cmp_status.c b/crypto/cmp/cmp_status.c index 46be6b6899..ffde72c3f8 100644 --- a/crypto/cmp/cmp_status.c +++ b/crypto/cmp/cmp_status.c @@ -189,7 +189,10 @@ char *snprint_PKIStatusInfo_parts(int status, int fail_info, printed_chars = BIO_snprintf(write_ptr, bufsize, "%s", status_string); ADVANCE_BUFFER; - /* failInfo is optional and may be empty */ + /* + * failInfo is optional and may be empty; + * if present, print failInfo before statusString because it is more concise + */ if (fail_info != 0) { printed_chars = BIO_snprintf(write_ptr, bufsize, "; PKIFailureInfo: "); ADVANCE_BUFFER; diff --git a/doc/man3/OSSL_CMP_CTX_new.pod b/doc/man3/OSSL_CMP_CTX_new.pod index 8ac5b815be..7b33dd0e4f 100644 --- a/doc/man3/OSSL_CMP_CTX_new.pod +++ b/doc/man3/OSSL_CMP_CTX_new.pod @@ -612,9 +612,29 @@ OSSL_CMP_CTX_get_certConf_cb_arg() gets the argument, respectively the pointer to a structure containing arguments, previously set by OSSL_CMP_CTX_set_certConf_cb_arg(), or NULL if unset. -OSSL_CMP_CTX_get_status() returns the PKIstatus from the last received -CertRepMessage or Revocation Response or error message, or -1 if unset. -For server contexts it returns -2 if a transaction is open, else -1. +OSSL_CMP_CTX_get_status() returns for client contexts the PKIstatus from +the last received CertRepMessage or Revocation Response or error message: +=item B on sucessful receipt of a GENP message: + +=over 4 + +=item B + +if an IR/CR/KUR/RR/GENM request message could not be produced, + +=item B + +on a transmission error or transaction error for this type of request, and + +=item B + +if no such request was attempted or OSSL_CMP_CTX_reinit() has been called. + +=back + +For server contexts it returns +B if a transaction is open, +otherwise B. OSSL_CMP_CTX_get0_statusString() returns the statusString from the last received CertRepMessage or Revocation Response or error message, or NULL if unset. diff --git a/doc/man3/OSSL_CMP_exec_certreq.pod b/doc/man3/OSSL_CMP_exec_certreq.pod index 60e2cf0f22..b0d81c7c41 100644 --- a/doc/man3/OSSL_CMP_exec_certreq.pod +++ b/doc/man3/OSSL_CMP_exec_certreq.pod @@ -109,8 +109,9 @@ make no sense for revocation and thus are treated as an error as well. OSSL_CMP_exec_GENM_ses() sends a general message containing the sequence of infoType and infoValue pairs (InfoTypeAndValue; short: B) -provided in the I using L. -It returns the list of Bs received in the GenRep. +optionally provided in the I using L. +On success it records in I the status B +and returns the list of Bs received in the GENP message. This can be used, for instance, to poll for CRLs or CA Key Updates. See RFC 4210 section 5.3.19 and appendix E.5 for details. @@ -139,8 +140,8 @@ assign the received value unless I is NULL. OSSL_CMP_exec_RR_ses() returns 1 on success, 0 on error. -OSSL_CMP_exec_GENM_ses() returns a -pointer to the received B sequence on success, NULL on error. +OSSL_CMP_exec_GENM_ses() returns NULL on error, +otherwise a pointer to the sequence of B received, which may be empty. This pointer must be freed by the caller. =head1 EXAMPLES diff --git a/include/openssl/cmp.h.in b/include/openssl/cmp.h.in index b47344215b..43666a0d0e 100644 --- a/include/openssl/cmp.h.in +++ b/include/openssl/cmp.h.in @@ -194,13 +194,16 @@ typedef ASN1_BIT_STRING OSSL_CMP_PKIFAILUREINFO; * -- CertReqMsg * } */ -# define OSSL_CMP_PKISTATUS_accepted 0 -# define OSSL_CMP_PKISTATUS_grantedWithMods 1 -# define OSSL_CMP_PKISTATUS_rejection 2 -# define OSSL_CMP_PKISTATUS_waiting 3 -# define OSSL_CMP_PKISTATUS_revocationWarning 4 +# define OSSL_CMP_PKISTATUS_request -3 +# define OSSL_CMP_PKISTATUS_trans -2 +# define OSSL_CMP_PKISTATUS_unspecified -1 +# define OSSL_CMP_PKISTATUS_accepted 0 +# define OSSL_CMP_PKISTATUS_grantedWithMods 1 +# define OSSL_CMP_PKISTATUS_rejection 2 +# define OSSL_CMP_PKISTATUS_waiting 3 +# define OSSL_CMP_PKISTATUS_revocationWarning 4 # define OSSL_CMP_PKISTATUS_revocationNotification 5 -# define OSSL_CMP_PKISTATUS_keyUpdateWarning 6 +# define OSSL_CMP_PKISTATUS_keyUpdateWarning 6 typedef ASN1_INTEGER OSSL_CMP_PKISTATUS; DECLARE_ASN1_ITEM(OSSL_CMP_PKISTATUS) -- cgit v1.2.3