crypto/cmp/,apps/lib/cmp_mock_srv.c: various improvements on delayed …

…delivery

apps/lib/cmp_mock_srv.c

@@ -183,15 +183,15 @@ int ossl_cmp_mock_srv_set_checkAfterTime(OSSL_CMP_SRV_CTX *srv_ctx, int sec)
     return 1;
 }
 
-static int delayed_delivery(OSSL_CMP_SRV_CTX *srv_ctx,
-                            const OSSL_CMP_MSG *req)
+/* determine whether to delay response to (non-polling) request */
+static int delayed_delivery(OSSL_CMP_SRV_CTX *srv_ctx, const OSSL_CMP_MSG *req)
 {
     mock_srv_ctx *ctx = OSSL_CMP_SRV_CTX_get0_custom_ctx(srv_ctx);
     int req_type = OSSL_CMP_MSG_get_bodytype(req);
 
     if (ctx == NULL || req == NULL) {
         ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT);
-        return 0;
+        return -1;
     }
 
     /*
@@ -211,11 +211,10 @@ static int delayed_delivery(OSSL_CMP_SRV_CTX *srv_ctx,
         if (ctx->req != NULL) { /* TODO: move this check to cmp_server.c */
             /* already in polling mode */
             ERR_raise(ERR_LIB_CMP, CMP_R_UNEXPECTED_PKIBODY);
-            return 0;
+            return -1;
         }
         if ((ctx->req = OSSL_CMP_MSG_dup(req)) == NULL)
             return -1;
-
         return 1;
     }
     return 0;
@@ -236,17 +235,17 @@ static int refcert_cmp(const X509 *refcert,
         && (ref_serial == NULL || ASN1_INTEGER_cmp(serial, ref_serial) == 0);
 }
 
-/* Reset dynamic variable in case of incomplete tansaction */
-static int reset_transaction(OSSL_CMP_SRV_CTX *srv_ctx)
+/* reset the state that belongs to a transaction */
+static int clean_transaction(OSSL_CMP_SRV_CTX *srv_ctx,
+                             ossl_unused const ASN1_OCTET_STRING *id)
 {
-    mock_srv_ctx *ctx = NULL;
+    mock_srv_ctx *ctx = OSSL_CMP_SRV_CTX_get0_custom_ctx(srv_ctx);
 
-    if (srv_ctx == NULL) {
+    if (ctx == NULL) {
         ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT);
         return 0;
     }
 
-    ctx = OSSL_CMP_SRV_CTX_get0_custom_ctx(srv_ctx);
     ctx->curr_pollCount = 0;
     OSSL_CMP_MSG_free(ctx->req);
     ctx->req = NULL;
@@ -497,6 +496,7 @@ static int process_certConf(OSSL_CMP_SRV_CTX *srv_ctx,
     return 1;
 }
 
+/* return 0 on failure, 1 on success, setting *req or otherwise *check_after */
 static int process_pollReq(OSSL_CMP_SRV_CTX *srv_ctx,
                            const OSSL_CMP_MSG *pollReq,
                            ossl_unused int certReqId,
@@ -509,16 +509,15 @@ static int process_pollReq(OSSL_CMP_SRV_CTX *srv_ctx,
         ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT);
         return 0;
     }
+    *req = NULL;
+
     if (ctx->sendError == 1
             || ctx->sendError == OSSL_CMP_MSG_get_bodytype(pollReq)) {
-        *req = NULL;
         ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_PROCESSING_MESSAGE);
         return 0;
     }
-    if (ctx->req == NULL) { /* TODO: move this check to cmp_server.c */
-        /* not currently in polling mode */
-        *req = NULL;
-        ERR_raise(ERR_LIB_CMP, CMP_R_UNEXPECTED_PKIBODY);
+    if (ctx->req == NULL) { /* not currently in polling mode */
+        ERR_raise(ERR_LIB_CMP, CMP_R_UNEXPECTED_POLLREQ);
         return 0;
     }
 
@@ -528,7 +527,6 @@ static int process_pollReq(OSSL_CMP_SRV_CTX *srv_ctx,
         ctx->req = NULL;
         *check_after = 0;
     } else {
-        *req = NULL;
         *check_after = ctx->checkAfterTime;
     }
     return 1;
@@ -543,8 +541,8 @@ OSSL_CMP_SRV_CTX *ossl_cmp_mock_srv_new(OSSL_LIB_CTX *libctx, const char *propq)
             && OSSL_CMP_SRV_CTX_init(srv_ctx, ctx, process_cert_request,
                                      process_rr, process_genm, process_error,
                                      process_certConf, process_pollReq)
-            && OSSL_CMP_SRV_CTX_setup_polling(srv_ctx, reset_transaction,
-                                              delayed_delivery))
+            && OSSL_CMP_SRV_CTX_init_trans(srv_ctx,
+                                           delayed_delivery, clean_transaction))
         return srv_ctx;
 
     mock_srv_ctx_free(ctx);

crypto/cmp/cmp_client.c

@@ -113,7 +113,7 @@ static int save_statusInfo(OSSL_CMP_CTX *ctx, OSSL_CMP_PKISI *si)
     return 1;
 }
 
-static int is_crep_with_waiting(OSSL_CMP_MSG *resp, int rid)
+static int is_crep_with_waiting(const OSSL_CMP_MSG *resp, int rid)
 {
     OSSL_CMP_CERTREPMESSAGE *crepmsg;
     OSSL_CMP_CERTRESPONSE *crep;
@@ -210,11 +210,11 @@ static int send_receive_check(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *req,
         return 0;
 
     /*
-     * 'rep' can have the expected response type, which during polling is
-     * pollRep. When polling, also any other non-error response (the final
-     * response) is fine here. When not yet polling, delayed delivery may
-     * be started by an error with 'waiting' status (while it may also be
-     * started by an expected response type ip/cp/kup).
+     * rep can have the expected response type, which during polling is pollRep.
+     * When polling, also any other non-error response (the final response)
+     * is fine here. When not yet polling, delayed delivery may be initiated
+     * by the server returning an error message with 'waiting' status (or a
+     * response message of expected type ip/cp/kup with 'waiting' status).
      */
     if (bt == expected_type
         || (expected_type == OSSL_CMP_PKIBODY_POLLREP
@@ -272,8 +272,8 @@ static int send_receive_check(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *req,
  * Returns -1 on receiving pollRep if sleep == 0, setting the checkAfter value.
  * Returns 1 on success and provides the received PKIMESSAGE in *rep.
  *           In this case the caller is responsible for freeing *rep.
- * Returns 0 on error (which includes the case that timeout has been reached or
- *           received response with waiting status).
+ * Returns 0 on error (which includes the cases that timeout has been reached
+ *           or a response with 'waiting' status has been received).
  */
 static int poll_for_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
                              OSSL_CMP_MSG **rep, int *checkAfter)
@@ -364,7 +364,7 @@ static int poll_for_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
             }
         } else if (is_crep_with_waiting(prep, rid)
                    || ossl_cmp_is_error_with_waiting(prep)) {
-            /* status cannot be 'waiting' at this point */
+            /* received status must not be 'waiting' */
             (void)ossl_cmp_exchange_error(ctx, OSSL_CMP_PKISTATUS_rejection,
                                           OSSL_CMP_CTX_FAILINFO_badRequest,
                                           "polling already started",
@@ -393,12 +393,12 @@ static int poll_for_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
 }
 
 static int save_senderNonce_if_waiting(OSSL_CMP_CTX *ctx,
-                                       OSSL_CMP_MSG *rep, int rid)
+                                       const OSSL_CMP_MSG *rep, int rid)
 {
     /*
-     * LWCMP section 4.4 states: the senderNonce of the preceding request
-     * message because this value will be needed for checking the recipNonce
-     * of the final response to be received after polling.
+     * Lightweight CMP Profile section 4.4 states: the senderNonce of the
+     * preceding request message because this value will be needed for checking
+     * the recipNonce of the final response to be received after polling.
      */
     if ((is_crep_with_waiting(rep, rid)
          || ossl_cmp_is_error_with_waiting(rep))
@@ -409,8 +409,8 @@ static int save_senderNonce_if_waiting(OSSL_CMP_CTX *ctx,
 }
 
 /*
- * send request and get response possibly with polling initiated by error msg.
- * Polling for ip/cp/kup/ with 'waiting' status is handled elsewhere.
+ * Send request and get response possibly with polling initiated by error msg.
+ * Polling for ip/cp/kup/ with 'waiting' status is handled by cert_response().
  */
 static int send_receive_also_delayed(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *req,
                                      OSSL_CMP_MSG **rep, int expected_type)
@@ -420,20 +420,17 @@ static int send_receive_also_delayed(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *req,
         return 0;
 
     if (ossl_cmp_is_error_with_waiting(*rep)) {
-        if (!save_senderNonce_if_waiting(ctx, *rep, -1 /* rid */))
+        if (!save_senderNonce_if_waiting(ctx, *rep, OSSL_CMP_CERTREQID_NONE))
             return 0;
-        /*
-         * not modifying ctx->status during the certConf & error exchange,
-         * because these additional exchanges should not change the status.
-         */
+        /* not modifying ctx->status during certConf and error exchanges */
         if (expected_type != OSSL_CMP_PKIBODY_PKICONF
             && !save_statusInfo(ctx, (*rep)->body->value.error->pKIStatusInfo))
             return 0;
 
         OSSL_CMP_MSG_free(*rep);
         *rep = NULL;
 
-        if (poll_for_response(ctx, 1 /* can sleep */, -1 /* rid */,
+        if (poll_for_response(ctx, 1 /* can sleep */, OSSL_CMP_CERTREQID_NONE,
                               rep, NULL /* checkAfter */) <= 0) {
             ERR_raise(ERR_LIB_CMP, CMP_R_POLLING_FAILED);
             return 0;
@@ -462,8 +459,8 @@ int ossl_cmp_exchange_certConf(OSSL_CMP_CTX *ctx, int certReqId,
     if (certConf == NULL)
         goto err;
 
-    res = send_receive_also_delayed(ctx, certConf,
-                                    &PKIconf, OSSL_CMP_PKIBODY_PKICONF);
+    res = send_receive_also_delayed(ctx, certConf, &PKIconf,
+                                    OSSL_CMP_PKIBODY_PKICONF);
 
  err:
     OSSL_CMP_MSG_free(certConf);
@@ -683,10 +680,10 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
             return 0;
         si = crep->status;
 
-        if (rid ==  OSSL_CMP_CERTREQID_NONE) {
+        if (rid == OSSL_CMP_CERTREQID_NONE) {
             /* for OSSL_CMP_PKIBODY_P10CR learn CertReqId from response */
             rid = ossl_cmp_asn1_get_int(crep->certReqId);
-            if (rid ==  OSSL_CMP_CERTREQID_NONE) {
+            if (rid != OSSL_CMP_CERTREQID_NONE) {
                 ERR_raise(ERR_LIB_CMP, CMP_R_BAD_REQUEST_ID);
                 return 0;
             }
@@ -702,7 +699,11 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
         return 0;
 
     if (ossl_cmp_pkisi_get_status(si) == OSSL_CMP_PKISTATUS_waiting) {
-        /* here we allow different flavor of ip/cp/kup & error with waiting */
+        /*
+         * Here we allow both and error message with waiting indication
+         * as well as a certificate response with waiting indication, where
+         * its flavor (ip, cp, or kup) may not strictly match ir/cr/p10cr/kur.
+         */
         OSSL_CMP_MSG_free(*resp);
         *resp = NULL;
         if ((ret = poll_for_response(ctx, sleep, rid, resp, checkAfter)) != 0) {
@@ -715,12 +716,12 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
         }
     }
 
-    /* at this point, ip/cp/kup or error without waiting */
+    /* at this point, we have received ip/cp/kup/error without waiting */
     if (rcvd_type == OSSL_CMP_PKIBODY_ERROR) {
         ERR_raise(ERR_LIB_CMP, CMP_R_RECEIVED_ERROR);
         return 0;
     }
-    /* here we are strict on the flavor of ip/cp/kup */
+    /* here we are strict on the flavor of ip/cp/kup: must match request */
     if (rcvd_type != expected_type) {
         ERR_raise(ERR_LIB_CMP, CMP_R_UNEXPECTED_PKIBODY);
         return 0;
@@ -738,8 +739,7 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
      * if the CMP server returned certificates in the caPubs field, copy them
      * to the context so that they can be retrieved if necessary
      */
-    if (crepmsg != NULL
-            && crepmsg->caPubs != NULL
+    if (crepmsg != NULL && crepmsg->caPubs != NULL
             && !ossl_cmp_ctx_set1_caPubs(ctx, crepmsg->caPubs))
         return 0;
 

crypto/cmp/cmp_err.c

@@ -76,6 +76,7 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     "error validating protection"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_ERROR_VALIDATING_SIGNATURE),
     "error validating signature"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_EXPECTED_POLLREQ), "expected pollreq"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_FAILED_BUILDING_OWN_CHAIN),
     "failed building own chain"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_FAILED_EXTRACTING_PUBKEY),
@@ -147,6 +148,7 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNEXPECTED_PKIBODY), "unexpected pkibody"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNEXPECTED_PKISTATUS),
     "unexpected pkistatus"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNEXPECTED_POLLREQ), "unexpected pollreq"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNEXPECTED_PVNO), "unexpected pvno"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNKNOWN_ALGORITHM_ID),
     "unknown algorithm id"},
@@ -156,6 +158,8 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     "unsupported algorithm"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNSUPPORTED_KEY_TYPE),
     "unsupported key type"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNSUPPORTED_PKIBODY),
+    "unsupported pkibody"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNSUPPORTED_PROTECTION_ALG_DHBASEDMAC),
     "unsupported protection alg dhbasedmac"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_VALUE_TOO_LARGE), "value too large"},

crypto/cmp/cmp_msg.c

@@ -984,8 +984,7 @@ static int suitable_rid(const ASN1_INTEGER *certReqId, int rid)
         return 1;
 
     trid = ossl_cmp_asn1_get_int(certReqId);
-
-    if (trid == OSSL_CMP_CERTREQID_NONE) {
+    if (trid < OSSL_CMP_CERTREQID_NONE) {
         ERR_raise(ERR_LIB_CMP, CMP_R_BAD_REQUEST_ID);
         return 0;
     }