Streamline the CMP request session API, adding the generalized OSSL_CMP_exec_certreq()

Fixes #12395

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12431)

7 files changed, 45 insertions, 57 deletions

diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c
index f38d8651f4..37473c7a6c 100644
--- a/crypto/cmp/cmp_client.c
+++ b/crypto/cmp/cmp_client.c
@@ -630,7 +630,8 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
     return ret;
 }
 
-int OSSL_CMP_try_certreq(OSSL_CMP_CTX *ctx, int req_type, int *checkAfter)
+int OSSL_CMP_try_certreq(OSSL_CMP_CTX *ctx, int req_type,
+                         const OSSL_CRMF_MSG *crm, int *checkAfter)
 {
     OSSL_CMP_MSG *req = NULL;
     OSSL_CMP_MSG *rep = NULL;
@@ -652,7 +653,7 @@ int OSSL_CMP_try_certreq(OSSL_CMP_CTX *ctx, int req_type, int *checkAfter)
         if (ctx->total_timeout > 0) /* else ctx->end_time is not used */
             ctx->end_time = time(NULL) + ctx->total_timeout;
 
-        req = ossl_cmp_certReq_new(ctx, req_type, 0 /* req_err */);
+        req = ossl_cmp_certreq_new(ctx, req_type, crm);
         if (req == NULL) /* also checks if all necessary options are set */
             return 0;
 
@@ -685,18 +686,26 @@ int OSSL_CMP_try_certreq(OSSL_CMP_CTX *ctx, int req_type, int *checkAfter)
  * TODO: another function to request two certificates at once should be created.
  * Returns pointer to received certificate, or NULL if none was received.
  */
-static X509 *do_certreq_seq(OSSL_CMP_CTX *ctx, int req_type, int req_err,
-                            int rep_type)
+X509 *OSSL_CMP_exec_certreq(OSSL_CMP_CTX *ctx, int req_type,
+                            const OSSL_CRMF_MSG *crm)
 {
+
     OSSL_CMP_MSG *req = NULL;
     OSSL_CMP_MSG *rep = NULL;
-    int rid = (req_type == OSSL_CMP_PKIBODY_P10CR) ? -1 : OSSL_CMP_CERTREQID;
+    int is_p10 = req_type == OSSL_CMP_PKIBODY_P10CR;
+    int rid = is_p10 ? -1 : OSSL_CMP_CERTREQID;
+    int rep_type = is_p10 ? OSSL_CMP_PKIBODY_CP : req_type + 1;
     X509 *result = NULL;
 
     if (ctx == NULL) {
         CMPerr(0, CMP_R_NULL_ARGUMENT);
         return NULL;
     }
+    if (is_p10 && crm != NULL) {
+        CMPerr(0, CMP_R_INVALID_ARGS);
+        return NULL;
+    }
+
     ctx->status = -1;
     if (!ossl_cmp_ctx_set0_newCert(ctx, NULL))
         return NULL;
@@ -705,7 +714,7 @@ static X509 *do_certreq_seq(OSSL_CMP_CTX *ctx, int req_type, int req_err,
         ctx->end_time = time(NULL) + ctx->total_timeout;
 
     /* OSSL_CMP_certreq_new() also checks if all necessary options are set */
-    if ((req = ossl_cmp_certReq_new(ctx, req_type, req_err)) == NULL)
+    if ((req = ossl_cmp_certreq_new(ctx, req_type, crm)) == NULL)
         goto err;
 
     if (!send_receive_check(ctx, req, &rep, rep_type))
@@ -722,30 +731,6 @@ static X509 *do_certreq_seq(OSSL_CMP_CTX *ctx, int req_type, int req_err,
     return result;
 }
 
-X509 *OSSL_CMP_exec_IR_ses(OSSL_CMP_CTX *ctx)
-{
-    return do_certreq_seq(ctx, OSSL_CMP_PKIBODY_IR,
-                          CMP_R_ERROR_CREATING_IR, OSSL_CMP_PKIBODY_IP);
-}
-
-X509 *OSSL_CMP_exec_CR_ses(OSSL_CMP_CTX *ctx)
-{
-    return do_certreq_seq(ctx, OSSL_CMP_PKIBODY_CR,
-                          CMP_R_ERROR_CREATING_CR, OSSL_CMP_PKIBODY_CP);
-}
-
-X509 *OSSL_CMP_exec_KUR_ses(OSSL_CMP_CTX *ctx)
-{
-    return do_certreq_seq(ctx, OSSL_CMP_PKIBODY_KUR,
-                          CMP_R_ERROR_CREATING_KUR, OSSL_CMP_PKIBODY_KUP);
-}
-
-X509 *OSSL_CMP_exec_P10CR_ses(OSSL_CMP_CTX *ctx)
-{
-    return do_certreq_seq(ctx, OSSL_CMP_PKIBODY_P10CR,
-                          CMP_R_ERROR_CREATING_P10CR, OSSL_CMP_PKIBODY_CP);
-}
-
 X509 *OSSL_CMP_exec_RR_ses(OSSL_CMP_CTX *ctx)
 {
     OSSL_CMP_MSG *rr = NULL;
diff --git a/crypto/cmp/cmp_err.c b/crypto/cmp/cmp_err.c
index 1ee1002233..87d0f0f1b0 100644
--- a/crypto/cmp/cmp_err.c
+++ b/crypto/cmp/cmp_err.c
@@ -45,17 +45,14 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     "error creating certconf"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_CERTREP),
     "error creating certrep"},
-    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_CR), "error creating cr"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_CERTREQ),
+    "error creating certreq"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_ERROR),
     "error creating error"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_GENM),
     "error creating genm"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_GENP),
     "error creating genp"},
-    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_IR), "error creating ir"},
-    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_KUR), "error creating kur"},
-    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_P10CR),
-    "error creating p10cr"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_PKICONF),
     "error creating pkiconf"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_CREATING_POLLREP),
@@ -90,6 +87,7 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     "missing key input for creating protection"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_KEY_USAGE_DIGITALSIGNATURE),
     "missing key usage digitalsignature"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_P10CSR), "missing p10csr"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_PRIVATE_KEY),
     "missing private key"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_PROTECTION), "missing protection"},
diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
index 0d874ae785..92f192bb5f 100644
--- a/crypto/cmp/cmp_local.h
+++ b/crypto/cmp/cmp_local.h
@@ -855,9 +855,9 @@ const char *ossl_cmp_bodytype_to_string(int type);
 int ossl_cmp_msg_set_bodytype(OSSL_CMP_MSG *msg, int type);
 int ossl_cmp_msg_get_bodytype(const OSSL_CMP_MSG *msg);
 OSSL_CMP_MSG *ossl_cmp_msg_create(OSSL_CMP_CTX *ctx, int bodytype);
-OSSL_CMP_MSG *ossl_cmp_certReq_new(OSSL_CMP_CTX *ctx, int bodytype,
-                                   int err_code);
-OSSL_CMP_MSG *ossl_cmp_certRep_new(OSSL_CMP_CTX *ctx, int bodytype,
+OSSL_CMP_MSG *ossl_cmp_certreq_new(OSSL_CMP_CTX *ctx, int bodytype,
+                                   const OSSL_CRMF_MSG *crm);
+OSSL_CMP_MSG *ossl_cmp_certrep_new(OSSL_CMP_CTX *ctx, int bodytype,
                                    int certReqId, OSSL_CMP_PKISI *si,
                                    X509 *cert, STACK_OF(X509) *chain,
                                    STACK_OF(X509) *caPubs, int encrypted,
diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index c5a9dbccf8..290a4ee10f 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -128,7 +128,7 @@ OSSL_CMP_MSG *ossl_cmp_msg_create(OSSL_CMP_CTX *ctx, int bodytype)
 
     case OSSL_CMP_PKIBODY_P10CR:
         if (ctx->p10CSR == NULL) {
-            CMPerr(0, CMP_R_ERROR_CREATING_P10CR);
+            CMPerr(0, CMP_R_MISSING_P10CSR);
             goto err;
         }
         if ((msg->body->value.p10cr = X509_REQ_dup(ctx->p10CSR)) == NULL)
@@ -321,10 +321,11 @@ static OSSL_CRMF_MSG *crm_new(OSSL_CMP_CTX *ctx, int bodytype, int rid)
     return crm;
 }
 
-OSSL_CMP_MSG *ossl_cmp_certReq_new(OSSL_CMP_CTX *ctx, int type, int err_code)
+OSSL_CMP_MSG *ossl_cmp_certreq_new(OSSL_CMP_CTX *ctx, int type,
+                                   const OSSL_CRMF_MSG *crm)
 {
     OSSL_CMP_MSG *msg;
-    OSSL_CRMF_MSG *crm = NULL;
+    OSSL_CRMF_MSG *local_crm = NULL;
 
     if (!ossl_assert(ctx != NULL))
         return NULL;
@@ -353,13 +354,20 @@ OSSL_CMP_MSG *ossl_cmp_certReq_new(OSSL_CMP_CTX *ctx, int type, int err_code)
             CMPerr(0, CMP_R_MISSING_PRIVATE_KEY);
             goto err;
         }
-        if ((crm = crm_new(ctx, type, OSSL_CMP_CERTREQID)) == NULL
-            || !OSSL_CRMF_MSG_create_popo(crm, privkey, ctx->digest,
-                                          ctx->popoMethod)
-            /* value.ir is same for cr and kur */
-            || !sk_OSSL_CRMF_MSG_push(msg->body->value.ir, crm))
+        if (crm == NULL) {
+            if ((local_crm = crm_new(ctx, type, OSSL_CMP_CERTREQID)) == NULL
+                || !OSSL_CRMF_MSG_create_popo(local_crm, privkey, ctx->digest,
+                                              ctx->popoMethod))
+                goto err;
+        } else {
+            if ((local_crm = OSSL_CRMF_MSG_dup(crm)) == NULL)
+                goto err;
+        }
+
+        /* value.ir is same for cr and kur */
+        if (!sk_OSSL_CRMF_MSG_push(msg->body->value.ir, local_crm))
             goto err;
-        crm = NULL;
+        local_crm = NULL;
         /* TODO: here optional 2nd certreqmsg could be pushed to the stack */
     }
 
@@ -369,14 +377,13 @@ OSSL_CMP_MSG *ossl_cmp_certReq_new(OSSL_CMP_CTX *ctx, int type, int err_code)
     return msg;
 
  err:
-    if (err_code != 0)
-        CMPerr(0, err_code);
-    OSSL_CRMF_MSG_free(crm);
+    CMPerr(0, CMP_R_ERROR_CREATING_CERTREQ);
+    OSSL_CRMF_MSG_free(local_crm);
     OSSL_CMP_MSG_free(msg);
     return NULL;
 }
 
-OSSL_CMP_MSG *ossl_cmp_certRep_new(OSSL_CMP_CTX *ctx, int bodytype,
+OSSL_CMP_MSG *ossl_cmp_certrep_new(OSSL_CMP_CTX *ctx, int bodytype,
                                    int certReqId, OSSL_CMP_PKISI *si,
                                    X509 *cert, STACK_OF(X509) *chain,
                                    STACK_OF(X509) *caPubs, int encrypted,
diff --git a/crypto/cmp/cmp_server.c b/crypto/cmp/cmp_server.c
index 8570885eed..a9a86cb5de 100644
--- a/crypto/cmp/cmp_server.c
+++ b/crypto/cmp/cmp_server.c
@@ -230,7 +230,7 @@ static OSSL_CMP_MSG *process_cert_request(OSSL_CMP_SRV_CTX *srv_ctx,
             goto err;
     }
 
-    msg = ossl_cmp_certRep_new(srv_ctx->ctx, bodytype, certReqId, si,
+    msg = ossl_cmp_certrep_new(srv_ctx->ctx, bodytype, certReqId, si,
                                certOut, chainOut, caPubs, 0 /* encrypted */,
                                srv_ctx->sendUnprotectedErrors);
     /*
diff --git a/crypto/crmf/crmf_asn.c b/crypto/crmf/crmf_asn.c
index 567cfaaeec..0f6de3ce8d 100644
--- a/crypto/crmf/crmf_asn.c
+++ b/crypto/crmf/crmf_asn.c
@@ -230,7 +230,7 @@ ASN1_SEQUENCE(OSSL_CRMF_MSG) = {
                          OSSL_CRMF_ATTRIBUTETYPEANDVALUE)
 } ASN1_SEQUENCE_END(OSSL_CRMF_MSG)
 IMPLEMENT_ASN1_FUNCTIONS(OSSL_CRMF_MSG)
-
+IMPLEMENT_ASN1_DUP_FUNCTION(OSSL_CRMF_MSG)
 
 ASN1_ITEM_TEMPLATE(OSSL_CRMF_MSGS) =
     ASN1_EX_TEMPLATE_TYPE(ASN1_TFLG_SEQUENCE_OF, 0,
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index a99648a1fd..0124d1d3ae 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2098,13 +2098,10 @@ CMP_R_ENCOUNTERED_WAITING:162:encountered waiting
 CMP_R_ERROR_CALCULATING_PROTECTION:115:error calculating protection
 CMP_R_ERROR_CREATING_CERTCONF:116:error creating certconf
 CMP_R_ERROR_CREATING_CERTREP:117:error creating certrep
-CMP_R_ERROR_CREATING_CR:163:error creating cr
+CMP_R_ERROR_CREATING_CERTREQ:163:error creating certreq
 CMP_R_ERROR_CREATING_ERROR:118:error creating error
 CMP_R_ERROR_CREATING_GENM:119:error creating genm
 CMP_R_ERROR_CREATING_GENP:120:error creating genp
-CMP_R_ERROR_CREATING_IR:164:error creating ir
-CMP_R_ERROR_CREATING_KUR:165:error creating kur
-CMP_R_ERROR_CREATING_P10CR:121:error creating p10cr
 CMP_R_ERROR_CREATING_PKICONF:122:error creating pkiconf
 CMP_R_ERROR_CREATING_POLLREP:123:error creating pollrep
 CMP_R_ERROR_CREATING_POLLREQ:124:error creating pollreq
@@ -2125,6 +2122,7 @@ CMP_R_INVALID_OPTION:174:invalid option
 CMP_R_MISSING_KEY_INPUT_FOR_CREATING_PROTECTION:130:\
 	missing key input for creating protection
 CMP_R_MISSING_KEY_USAGE_DIGITALSIGNATURE:142:missing key usage digitalsignature
+CMP_R_MISSING_P10CSR:121:missing p10csr
 CMP_R_MISSING_PRIVATE_KEY:131:missing private key
 CMP_R_MISSING_PROTECTION:143:missing protection
 CMP_R_MISSING_REFERENCE_CERT:168:missing reference cert