crypto/cmp: fix CertReqId to use in p10cr transactions acc. to RFC 4210

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/20298)

4 files changed, 48 insertions, 26 deletions

diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c
index 7395c65647..74d544ab0e 100644
--- a/crypto/cmp/cmp_client.c
+++ b/crypto/cmp/cmp_client.c
@@ -63,10 +63,10 @@ static int unprotected_exception(const OSSL_CMP_CTX *ctx,
         break;
     default:
         if (IS_CREP(rcvd_type)) {
+            int any_rid = OSSL_CMP_CERTREQID_NONE;
             OSSL_CMP_CERTREPMESSAGE *crepmsg = rep->body->value.ip;
             OSSL_CMP_CERTRESPONSE *crep =
-                ossl_cmp_certrepmessage_get0_certresponse(crepmsg,
-                                                          -1 /* any rid */);
+                ossl_cmp_certrepmessage_get0_certresponse(crepmsg, any_rid);
 
             if (sk_OSSL_CMP_CERTRESPONSE_num(crepmsg->response) > 1)
                 return -1;
@@ -356,15 +356,16 @@ static int poll_for_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
  * 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)
+int ossl_cmp_exchange_certConf(OSSL_CMP_CTX *ctx, int certReqId,
+                               int fail_info, const char *txt)
 {
     OSSL_CMP_MSG *certConf;
     OSSL_CMP_MSG *PKIconf = NULL;
     int res = 0;
 
     /* OSSL_CMP_certConf_new() also checks if all necessary options are set */
-    if ((certConf = ossl_cmp_certConf_new(ctx, fail_info, txt)) == NULL)
+    certConf = ossl_cmp_certConf_new(ctx, certReqId, fail_info, txt);
+    if (certConf == NULL)
         goto err;
 
     res = send_receive_check(ctx, certConf, &PKIconf, OSSL_CMP_PKIBODY_PKICONF);
@@ -549,6 +550,7 @@ int OSSL_CMP_certConf_cb(OSSL_CMP_CTX *ctx, X509 *cert, int fail_info,
 
 /*-
  * Perform the generic handling of certificate responses for IR/CR/KUR/P10CR.
+ * |rid| must be OSSL_CMP_CERTREQID_NONE if not available, namely for p10cr
  * Returns -1 on receiving pollRep if sleep == 0, setting the checkAfter value.
  * Returns 1 on success and provides the received PKIMESSAGE in *resp.
  * Returns 0 on error (which includes the case that timeout has been reached).
@@ -582,10 +584,9 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
         return 0;
     if (!save_statusInfo(ctx, crep->status))
         return 0;
-    if (rid == -1) {
-        /* for OSSL_CMP_PKIBODY_P10CR learn CertReqId from response */
+    if (rid == OSSL_CMP_CERTREQID_NONE) { /* used for OSSL_CMP_PKIBODY_P10CR */
         rid = ossl_cmp_asn1_get_int(crep->certReqId);
-        if (rid == -1) {
+        if (rid != OSSL_CMP_CERTREQID_NONE) {
             ERR_raise(ERR_LIB_CMP, CMP_R_BAD_REQUEST_ID);
             return 0;
         }
@@ -649,7 +650,7 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
                       "rejecting newly enrolled cert with subject: %s", subj);
     if (!ctx->disableConfirm
             && !ossl_cmp_hdr_has_implicitConfirm((*resp)->header)) {
-        if (!ossl_cmp_exchange_certConf(ctx, fail_info, txt))
+        if (!ossl_cmp_exchange_certConf(ctx, rid, fail_info, txt))
             ret = 0;
     }
 
@@ -691,7 +692,7 @@ int OSSL_CMP_try_certreq(OSSL_CMP_CTX *ctx, int req_type,
 {
     OSSL_CMP_MSG *rep = NULL;
     int is_p10 = req_type == OSSL_CMP_PKIBODY_P10CR;
-    int rid = is_p10 ? -1 : OSSL_CMP_CERTREQID;
+    int rid = is_p10 ? OSSL_CMP_CERTREQID_NONE : OSSL_CMP_CERTREQID;
     int rep_type = is_p10 ? OSSL_CMP_PKIBODY_CP : req_type + 1;
     int res = 0;
 
@@ -732,7 +733,7 @@ X509 *OSSL_CMP_exec_certreq(OSSL_CMP_CTX *ctx, int req_type,
 {
     OSSL_CMP_MSG *rep = NULL;
     int is_p10 = req_type == OSSL_CMP_PKIBODY_P10CR;
-    int rid = is_p10 ? -1 : OSSL_CMP_CERTREQID;
+    int rid = is_p10 ? OSSL_CMP_CERTREQID_NONE : OSSL_CMP_CERTREQID;
     int rep_type = is_p10 ? OSSL_CMP_PKIBODY_CP : req_type + 1;
     X509 *result = NULL;
 
diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
index c227d7f152..7f69ad025f 100644
--- a/crypto/cmp/cmp_local.h
+++ b/crypto/cmp/cmp_local.h
@@ -855,7 +855,9 @@ int ossl_cmp_hdr_init(OSSL_CMP_CTX *ctx, OSSL_CMP_PKIHEADER *hdr);
 # define OSSL_CMP_PKIBODY_POLLREP  26
 # define OSSL_CMP_PKIBODY_TYPE_MAX OSSL_CMP_PKIBODY_POLLREP
 /* certReqId for the first - and so far only - certificate request */
-# define OSSL_CMP_CERTREQID 0
+# define OSSL_CMP_CERTREQID         0
+# define OSSL_CMP_CERTREQID_NONE    -1
+# define OSSL_CMP_CERTREQID_INVALID -2
 /* sequence id for the first - and so far only - revocation request */
 # define OSSL_CMP_REVREQSID 0
 int ossl_cmp_msg_set0_libctx(OSSL_CMP_MSG *msg, OSSL_LIB_CTX *libctx,
@@ -888,8 +890,8 @@ OSSL_CMP_MSG *ossl_cmp_error_new(OSSL_CMP_CTX *ctx, const OSSL_CMP_PKISI *si,
                                  int unprotected);
 int ossl_cmp_certstatus_set0_certHash(OSSL_CMP_CERTSTATUS *certStatus,
                                       ASN1_OCTET_STRING *hash);
-OSSL_CMP_MSG *ossl_cmp_certConf_new(OSSL_CMP_CTX *ctx, int fail_info,
-                                    const char *text);
+OSSL_CMP_MSG *ossl_cmp_certConf_new(OSSL_CMP_CTX *ctx, int certReqId,
+                                    int fail_info, const char *text);
 OSSL_CMP_MSG *ossl_cmp_pollReq_new(OSSL_CMP_CTX *ctx, int crid);
 OSSL_CMP_MSG *ossl_cmp_pollRep_new(OSSL_CMP_CTX *ctx, int crid,
                                    int64_t poll_after);
@@ -925,8 +927,8 @@ int ossl_cmp_verify_popo(const OSSL_CMP_CTX *ctx,
                          const OSSL_CMP_MSG *msg, int accept_RAVerified);
 
 /* from cmp_client.c */
-int ossl_cmp_exchange_certConf(OSSL_CMP_CTX *ctx, int fail_info,
-                               const char *txt);
+int ossl_cmp_exchange_certConf(OSSL_CMP_CTX *ctx, int certReqId,
+                               int fail_info, const char *txt);
 int ossl_cmp_exchange_error(OSSL_CMP_CTX *ctx, int status, int fail_info,
                             const char *txt, int errorCode, const char *detail);
 
diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index f9cffcc3b9..1920f19048 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -793,8 +793,8 @@ int ossl_cmp_certstatus_set0_certHash(OSSL_CMP_CERTSTATUS *certStatus,
     return 1;
 }
 
-OSSL_CMP_MSG *ossl_cmp_certConf_new(OSSL_CMP_CTX *ctx, int fail_info,
-                                    const char *text)
+OSSL_CMP_MSG *ossl_cmp_certConf_new(OSSL_CMP_CTX *ctx, int certReqId,
+                                    int fail_info, const char *text)
 {
     OSSL_CMP_MSG *msg = NULL;
     OSSL_CMP_CERTSTATUS *certStatus = NULL;
@@ -803,7 +803,9 @@ OSSL_CMP_MSG *ossl_cmp_certConf_new(OSSL_CMP_CTX *ctx, int fail_info,
     ASN1_OCTET_STRING *certHash = NULL;
     OSSL_CMP_PKISI *sinfo;
 
-    if (!ossl_assert(ctx != NULL && ctx->newCert != NULL))
+    if (!ossl_assert(ctx != NULL && ctx->newCert != NULL
+                     && (certReqId == OSSL_CMP_CERTREQID
+                         || certReqId == OSSL_CMP_CERTREQID_NONE)))
         return NULL;
 
     if ((unsigned)fail_info > OSSL_CMP_PKIFAILUREINFO_MAX_BIT_PATTERN) {
@@ -821,9 +823,11 @@ OSSL_CMP_MSG *ossl_cmp_certConf_new(OSSL_CMP_CTX *ctx, int fail_info,
         OSSL_CMP_CERTSTATUS_free(certStatus);
         goto err;
     }
+
     /* set the ID of the certReq */
-    if (!ASN1_INTEGER_set(certStatus->certReqId, OSSL_CMP_CERTREQID))
+    if (!ASN1_INTEGER_set(certStatus->certReqId, certReqId))
         goto err;
+
     certStatus->hashAlg = NULL;
     /*
      * The hash of the certificate, using the same hash algorithm
@@ -977,12 +981,12 @@ static int suitable_rid(const ASN1_INTEGER *certReqId, int rid)
 {
     int trid;
 
-    if (rid == -1)
+    if (rid == OSSL_CMP_CERTREQID_NONE)
         return 1;
 
     trid = ossl_cmp_asn1_get_int(certReqId);
 
-    if (trid == -1) {
+    if (trid == OSSL_CMP_CERTREQID_NONE) {
         ERR_raise(ERR_LIB_CMP, CMP_R_BAD_REQUEST_ID);
         return 0;
     }
diff --git a/crypto/cmp/cmp_server.c b/crypto/cmp/cmp_server.c
index 05e8cb1955..ce85dbe2f4 100644
--- a/crypto/cmp/cmp_server.c
+++ b/crypto/cmp/cmp_server.c
@@ -22,8 +22,9 @@
 /* the context for the generic CMP server */
 struct ossl_cmp_srv_ctx_st
 {
-    OSSL_CMP_CTX *ctx; /* Client CMP context, partly reused for srv */
-    void *custom_ctx;  /* pointer to specific server context */
+    void *custom_ctx;  /* pointer to application-specific server context */
+    OSSL_CMP_CTX *ctx; /* Client CMP context, reusing transactionID etc. */
+    int certReqId; /* id of last ir/cr/kur, OSSL_CMP_CERTREQID_NONE for p10cr */
 
     OSSL_CMP_SRV_cert_request_cb_t process_cert_request;
     OSSL_CMP_SRV_rr_cb_t process_rr;
@@ -57,6 +58,7 @@ OSSL_CMP_SRV_CTX *OSSL_CMP_SRV_CTX_new(OSSL_LIB_CTX *libctx, const char *propq)
 
     if ((ctx->ctx = OSSL_CMP_CTX_new(libctx, propq)) == NULL)
         goto err;
+    ctx->certReqId = OSSL_CMP_CERTREQID_INVALID;
 
     /* all other elements are initialized to 0 or NULL, respectively */
     return ctx;
@@ -184,7 +186,7 @@ static OSSL_CMP_MSG *process_cert_request(OSSL_CMP_SRV_CTX *srv_ctx,
     }
 
     if (OSSL_CMP_MSG_get_bodytype(req) == OSSL_CMP_PKIBODY_P10CR) {
-        certReqId = OSSL_CMP_CERTREQID;
+        certReqId = OSSL_CMP_CERTREQID_NONE; /* p10cr does not include an Id */
         p10cr = req->body->value.p10cr;
     } else {
         OSSL_CRMF_MSGS *reqs = req->body->value.ir; /* same for cr and kur */
@@ -199,7 +201,12 @@ static OSSL_CMP_MSG *process_cert_request(OSSL_CMP_SRV_CTX *srv_ctx,
             return NULL;
         }
         certReqId = OSSL_CRMF_MSG_get_certReqId(crm);
+        if (certReqId != OSSL_CMP_CERTREQID) {
+            ERR_raise(ERR_LIB_CMP, CMP_R_BAD_REQUEST_ID);
+            return 0;
+        }
     }
+    srv_ctx->certReqId = certReqId;
 
     if (!ossl_cmp_verify_popo(srv_ctx->ctx, req, srv_ctx->acceptRAVerified)) {
         /* Proof of possession could not be verified */
@@ -357,6 +364,10 @@ static OSSL_CMP_MSG *process_certConf(OSSL_CMP_SRV_CTX *srv_ctx,
         ASN1_OCTET_STRING *certHash = status->certHash;
         OSSL_CMP_PKISI *si = status->statusInfo;
 
+        if (certReqId != srv_ctx->certReqId) {
+            ERR_raise(ERR_LIB_CMP, CMP_R_BAD_REQUEST_ID);
+            return NULL;
+        }
         if (!srv_ctx->process_certConf(srv_ctx, req, certReqId, certHash, si))
             return NULL; /* reason code may be: CMP_R_CERTHASH_UNMATCHED */
 
@@ -395,8 +406,12 @@ static OSSL_CMP_MSG *process_pollReq(OSSL_CMP_SRV_CTX *srv_ctx,
         return NULL;
     }
 
-    pr = sk_OSSL_CMP_POLLREQ_value(prc, 0);
+    pr = sk_OSSL_CMP_POLLREQ_value(prc, OSSL_CMP_CERTREQID);
     certReqId = ossl_cmp_asn1_get_int(pr->certReqId);
+    if (certReqId != srv_ctx->certReqId) {
+        ERR_raise(ERR_LIB_CMP, CMP_R_BAD_REQUEST_ID);
+        return NULL;
+    }
     if (!srv_ctx->process_pollReq(srv_ctx, req, certReqId,
                                   &certReq, &check_after))
         return NULL;