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 #19205)

(cherry picked from commit 19ddcc4)

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;

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 */
 }

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)

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;

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;

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;

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.

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

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)