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)

5 files changed, 33 insertions, 19 deletions

diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c
index cffd258f18..25f179e107 100644
--- a/crypto/cmp/cmp_client.c
+++ b/crypto/cmp/cmp_client.c
@@ -93,7 +93,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;
@@ -356,7 +357,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)
 {
@@ -385,6 +389,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;
@@ -742,16 +748,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 eb0cf35f84..fd71ba099b 100644
--- a/crypto/cmp/cmp_ctx.c
+++ b/crypto/cmp/cmp_ctx.c
@@ -118,7 +118,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;
@@ -161,7 +161,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 dc33d8d0b3..6a6f30c1a1 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -462,7 +462,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 bf5c5fdb57..83bf4ac157 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);
 
@@ -593,8 +593,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:
@@ -609,7 +609,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 bb93ac9389..176c546f9c 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;