bugfix in apps/cmp.c and cmp_client.c: inconsistencies on retrieving extraCerts in...

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12822)

diff --git a/apps/cmp.c b/apps/cmp.c
index 3e7b010fcb720d259c517f56b3a31ea36d73a08e..1c3b592f7f3e679f277abd8b6a5769d09c0522ac 100644 (file)
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -2818,27 +2818,27 @@ int cmp_main(int argc, char **argv)
         switch (opt_cmd) {
         case CMP_IR:
             newcert = OSSL_CMP_exec_IR_ses(cmp_ctx);
-            if (newcert == NULL)
-                goto err;
+            if (newcert != NULL)
+                ret = 1;
             break;
         case CMP_KUR:
             newcert = OSSL_CMP_exec_KUR_ses(cmp_ctx);
-            if (newcert == NULL)
-                goto err;
+            if (newcert != NULL)
+                ret = 1;
             break;
         case CMP_CR:
             newcert = OSSL_CMP_exec_CR_ses(cmp_ctx);
-            if (newcert == NULL)
-                goto err;
+            if (newcert != NULL)
+                ret = 1;
             break;
         case CMP_P10CR:
             newcert = OSSL_CMP_exec_P10CR_ses(cmp_ctx);
-            if (newcert == NULL)
-                goto err;
+            if (newcert != NULL)
+                ret = 1;
             break;
         case CMP_RR:
-            if (OSSL_CMP_exec_RR_ses(cmp_ctx) == NULL)
-                goto err;
+            if (OSSL_CMP_exec_RR_ses(cmp_ctx) != NULL)
+                ret = 1;
             break;
         case CMP_GENM:
             {
@@ -2852,10 +2852,11 @@ int cmp_main(int argc, char **argv)
                     OSSL_CMP_CTX_push0_genm_ITAV(cmp_ctx, itav);
                 }
 
-                if ((itavs = OSSL_CMP_exec_GENM_ses(cmp_ctx)) == NULL)
-                    goto err;
-                print_itavs(itavs);
-                sk_OSSL_CMP_ITAV_pop_free(itavs, OSSL_CMP_ITAV_free);
+                if ((itavs = OSSL_CMP_exec_GENM_ses(cmp_ctx)) != NULL) {
+                    print_itavs(itavs);
+                    sk_OSSL_CMP_ITAV_pop_free(itavs, OSSL_CMP_ITAV_free);
+                    ret = 1;
+                }
                 break;
             }
         default:
@@ -2863,7 +2864,7 @@ int cmp_main(int argc, char **argv)
         }
 
         {
-            /* print PKIStatusInfo (this is in case there has been no error) */
+            /* print PKIStatusInfo */
             int status = OSSL_CMP_CTX_get_status(cmp_ctx);
             char *buf = app_malloc(OSSL_CMP_PKISI_BUFLEN, "PKIStatusInfo buf");
             const char *string =
@@ -2885,11 +2886,14 @@ int cmp_main(int argc, char **argv)
             OPENSSL_free(buf);
         }
 
-        if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_caPubs(cmp_ctx),
-                            opt_cacertsout, "CA") < 0)
-            goto err;
         if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_extraCertsIn(cmp_ctx),
                             opt_extracertsout, "extra") < 0)
+            ret = 0;
+        if (!ret)
+            goto err;
+        ret = 0;
+        if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_caPubs(cmp_ctx),
+                            opt_cacertsout, "CA") < 0)
             goto err;
         if (newcert != NULL) {
             STACK_OF(X509) *certs = sk_X509_new_null();

diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c
index 4f8a9e24447443a79c853c6403b7dd358e679b2c..fe7168916aba8b644f18d9ae80185cf583eb189b 100644 (file)
--- a/crypto/cmp/cmp_client.c
+++ b/crypto/cmp/cmp_client.c
@@ -190,6 +190,11 @@ static int send_receive_check(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *req,
      */
     ossl_cmp_log1(INFO, ctx, "received %s", ossl_cmp_bodytype_to_string(bt));
 
+    /* copy received extraCerts to ctx->extraCertsIn so they can be retrieved */
+    if (bt != OSSL_CMP_PKIBODY_POLLREP && bt != OSSL_CMP_PKIBODY_PKICONF
+            && !ossl_cmp_ctx_set1_extraCertsIn(ctx, (*rep)->extraCerts))
+        return 0;
+
     if (!ossl_cmp_msg_check_update(ctx, *rep, unprotected_exception,
                                    expected_type))
         return 0;
@@ -470,7 +475,7 @@ static X509 *get1_cert_status(OSSL_CMP_CTX *ctx, int bodytype,
 /*-
  * Callback fn validating that the new certificate can be verified, using
  * ctx->certConf_cb_arg, which has been initialized using opt_out_trusted, and
- * ctx->untrusted, which at this point already contains ctx->extraCertsIn.
+ * ctx->untrusted, which at this point already contains msg->extraCerts.
  * Returns 0 on acceptance, else a bit field reflecting PKIFailureInfo.
  * Quoting from RFC 4210 section 5.1. Overall PKI Message:
  *     The extraCerts field can contain certificates that may be useful to
@@ -595,10 +600,6 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
             && !ossl_cmp_ctx_set1_caPubs(ctx, crepmsg->caPubs))
         return 0;
 
-    /* copy received extraCerts to ctx->extraCertsIn so they can be retrieved */
-    if (!ossl_cmp_ctx_set1_extraCertsIn(ctx, (*resp)->extraCerts))
-        return 0;
-
     subj = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0);
     if (rkey != NULL
         /* X509_check_private_key() also works if rkey is just public key */
@@ -606,8 +607,8 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int rid,
         fail_info = 1 << OSSL_CMP_PKIFAILUREINFO_incorrectData;
         txt = "public key in new certificate does not match our enrollment key";
         /*-
-         * not callling (void)ossl_cmp_exchange_error(ctx,
-         *                    OSSL_CMP_PKISTATUS_rejection, fail_info, txt)
+         * not calling (void)ossl_cmp_exchange_error(ctx,
+         *                   OSSL_CMP_PKISTATUS_rejection, fail_info, txt)
          * not throwing CMP_R_CERTIFICATE_NOT_ACCEPTED with txt
          * not returning 0
          * since we better leave this for the certConf_cb to decide

diff --git a/doc/man1/openssl-cmp.pod.in b/doc/man1/openssl-cmp.pod.in
index 75ee82211d082ce90a9582aaadadabde960b6fca..93897018938e4488d1b7687cf5a4b3cf1bc75ef4 100644 (file)
--- a/doc/man1/openssl-cmp.pod.in
+++ b/doc/man1/openssl-cmp.pod.in
@@ -590,13 +590,13 @@ with a signature key."
 
 =item B<-extracertsout> I<filename>
 
-The file where to save any extra certificates received in the extraCerts field
-of response messages.
+The file where to save all certificates contained in the extraCerts field
+of the last received response message (except for pollRep and PKIConf).
 
 =item B<-cacertsout> I<filename>
 
-The file where to save any CA certificates received in the caPubs field of
-Initialization Response (IP) messages.
+The file where to save any CA certificates contained in the caPubs field of
+the last received certificate response (i.e., IP, CP, or KUP) message.
 
 =back
 

diff --git a/doc/man3/OSSL_CMP_CTX_new.pod b/doc/man3/OSSL_CMP_CTX_new.pod
index f619a65d3f0555a3d9bf309efe72ed16b96b5592..d581556ff1014fd580ebbe896a5704cf54946cba 100644 (file)
--- a/doc/man3/OSSL_CMP_CTX_new.pod
+++ b/doc/man3/OSSL_CMP_CTX_new.pod
@@ -617,14 +617,14 @@ OSSL_CMP_CTX_get1_newChain() returns a pointer to a duplicate of the stack of
 X.509 certificates computed by OSSL_CMP_certConf_cb() (if this function has
 been called) on the last received certificate response message IP/CP/KUP.
 
-OSSL_CMP_CTX_get1_caPubs() returns a pointer to a duplicate of the stack of
+OSSL_CMP_CTX_get1_caPubs() returns a pointer to a duplicate of the list of
 X.509 certificates received in the caPubs field of last received certificate
 response message IP/CP/KUP.
 
-OSSL_CMP_CTX_get1_extraCertsIn() returns a pointer to a duplicate of the stack
-of X.509 certificates received in the last received nonempty extraCerts field.
-Returns an empty stack if no extraCerts have been received in the current
-transaction.
+OSSL_CMP_CTX_get1_extraCertsIn() returns a pointer to a duplicate of the list
+of X.509 certificates contained in the extraCerts field of the last received
+response message (except for pollRep and PKIConf), or
+an empty stack if no extraCerts have been received in the current transaction.
 
 OSSL_CMP_CTX_set1_transactionID() sets the given transaction ID in the given
 OSSL_CMP_CTX structure.