diff options
author | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2022-09-13 15:43:59 +0200 |
---|---|---|
committer | Dr. David von Oheimb <dev@ddvo.net> | 2022-11-24 14:11:56 +0100 |
commit | 69d3c81ca5c6cb03b0d1d1063fe6a2fa731ff461 (patch) | |
tree | 70201c5bb10c6e25e2a4978fbbd6f3181e70dbf1 | |
parent | c28c2e0c7e6544e91886b2825c9e5ebb7ff98a8f (diff) |
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 <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/19205)
(cherry picked from commit 19ddcc4cbb43464493a4b82332a1ab96da823451)
-rw-r--r-- | apps/cmp.c | 66 | ||||
-rw-r--r-- | crypto/cmp/cmp_client.c | 29 | ||||
-rw-r--r-- | crypto/cmp/cmp_ctx.c | 4 | ||||
-rw-r--r-- | crypto/cmp/cmp_msg.c | 2 | ||||
-rw-r--r-- | crypto/cmp/cmp_server.c | 12 | ||||
-rw-r--r-- | crypto/cmp/cmp_status.c | 5 | ||||
-rw-r--r-- | doc/man3/OSSL_CMP_CTX_new.pod | 26 | ||||
-rw-r--r-- | doc/man3/OSSL_CMP_exec_certreq.pod | 9 | ||||
-rw-r--r-- | 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 : "<unknown PKIStatus>"); + 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 : "<unknown PKIStatus>"); - 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<OSSL_CMP_PKISTATUS_accepted> on sucessful receipt of a GENP message: + +=over 4 + +=item B<OSSL_CMP_PKISTATUS_request> + +if an IR/CR/KUR/RR/GENM request message could not be produced, + +=item B<OSSL_CMP_PKISTATUS_trans> + +on a transmission error or transaction error for this type of request, and + +=item B<OSSL_CMP_PKISTATUS_unspecified> + +if no such request was attempted or OSSL_CMP_CTX_reinit() has been called. + +=back + +For server contexts it returns +B<OSSL_CMP_PKISTATUS_trans> if a transaction is open, +otherwise B<OSSL_CMP_PKISTATUS_unspecified>. 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<ITAV>) -provided in the I<ctx> using L<OSSL_CMP_CTX_push0_genm_ITAV(3)>. -It returns the list of B<ITAV>s received in the GenRep. +optionally provided in the I<ctx> using L<OSSL_CMP_CTX_push0_genm_ITAV(3)>. +On success it records in I<ctx> the status B<OSSL_CMP_PKISTATUS_accepted> +and returns the list of B<ITAV>s 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<checkAfter> 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<ITAV> sequence on success, NULL on error. +OSSL_CMP_exec_GENM_ses() returns NULL on error, +otherwise a pointer to the sequence of B<ITAV> 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) |