CMP app and doc: add -no_cache_extracerts option / OSSL_CMP_OPT_NO_CA…

…CHE_EXTRACERTS Reviewed-by: Shane Lontis <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from openssl#19948)
t-j-h · Jan 17, 2024 · 1caaf07 · 1caaf07
1 parent fd51437
commit 1caaf07
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 37 deletions.
diff --git a/apps/cmp.c b/apps/cmp.c
@@ -86,6 +86,7 @@ static char *opt_srvcert = NULL;
 static char *opt_expect_sender = NULL;
 static int opt_ignore_keyusage = 0;
 static int opt_unprotected_errors = 0;
+static int opt_no_cache_extracerts = 0;
 static char *opt_srvcertout = NULL;
 static char *opt_extracertsout = NULL;
 static char *opt_cacertsout = NULL;
@@ -231,7 +232,7 @@ typedef enum OPTION_choice {
 
     OPT_TRUSTED, OPT_UNTRUSTED, OPT_SRVCERT,
     OPT_EXPECT_SENDER,
-    OPT_IGNORE_KEYUSAGE, OPT_UNPROTECTED_ERRORS,
+    OPT_IGNORE_KEYUSAGE, OPT_UNPROTECTED_ERRORS, OPT_NO_CACHE_EXTRACERTS,
     OPT_SRVCERTOUT, OPT_EXTRACERTSOUT, OPT_CACERTSOUT,
     OPT_OLDWITHOLD, OPT_NEWWITHNEW, OPT_NEWWITHOLD, OPT_OLDWITHNEW,
 
@@ -408,6 +409,8 @@ const OPTIONS cmp_options[] = {
      "certificate responses (ip/cp/kup), revocation responses (rp), and PKIConf"},
     {OPT_MORE_STR, 0, 0,
      "WARNING: This setting leads to behavior allowing violation of RFC 4210"},
+    {"no_cache_extracerts", OPT_NO_CACHE_EXTRACERTS, '-',
+     "Do not keep certificates received in the extraCerts CMP message field"},
     { "srvcertout", OPT_SRVCERTOUT, 's',
       "File to save the server cert used and validated for CMP response protection"},
     {"extracertsout", OPT_EXTRACERTSOUT, 's',
@@ -612,6 +615,7 @@ static varref cmp_vars[] = { /* must be in same order as enumerated above! */
     {&opt_trusted}, {&opt_untrusted}, {&opt_srvcert},
     {&opt_expect_sender},
     {(char **)&opt_ignore_keyusage}, {(char **)&opt_unprotected_errors},
+    {(char **)&opt_no_cache_extracerts},
     {&opt_srvcertout}, {&opt_extracertsout}, {&opt_cacertsout},
     {&opt_oldwithold}, {&opt_newwithnew}, {&opt_newwithold}, {&opt_oldwithnew},
 
@@ -2638,6 +2642,9 @@ static int get_opts(int argc, char **argv)
         case OPT_UNPROTECTED_ERRORS:
             opt_unprotected_errors = 1;
             break;
+        case OPT_NO_CACHE_EXTRACERTS:
+            opt_no_cache_extracerts = 1;
+            break;
         case OPT_SRVCERTOUT:
             opt_srvcertout = opt_str();
             break;
@@ -3243,6 +3250,9 @@ int cmp_main(int argc, char **argv)
 
     if (opt_ignore_keyusage)
         (void)OSSL_CMP_CTX_set_option(cmp_ctx, OSSL_CMP_OPT_IGNORE_KEYUSAGE, 1);
+    if (opt_no_cache_extracerts)
+        (void)OSSL_CMP_CTX_set_option(cmp_ctx, OSSL_CMP_OPT_NO_CACHE_EXTRACERTS,
+                                      1);
 
     if (opt_use_mock_srv
 #if !defined(OPENSSL_NO_SOCK) && !defined(OPENSSL_NO_HTTP)

diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c
@@ -915,6 +915,9 @@ int OSSL_CMP_CTX_set_option(OSSL_CMP_CTX *ctx, int opt, int val)
     case OSSL_CMP_OPT_UNPROTECTED_ERRORS:
         ctx->unprotectedErrors = val;
         break;
+    case OSSL_CMP_OPT_NO_CACHE_EXTRACERTS:
+        ctx->noCacheExtraCerts = val;
+        break;
     case OSSL_CMP_OPT_VALIDITY_DAYS:
         ctx->days = val;
         break;
@@ -1000,6 +1003,8 @@ int OSSL_CMP_CTX_get_option(const OSSL_CMP_CTX *ctx, int opt)
         return ctx->unprotectedSend;
     case OSSL_CMP_OPT_UNPROTECTED_ERRORS:
         return ctx->unprotectedErrors;
+    case OSSL_CMP_OPT_NO_CACHE_EXTRACERTS:
+        return ctx->noCacheExtraCerts;
     case OSSL_CMP_OPT_VALIDITY_DAYS:
         return ctx->days;
     case OSSL_CMP_OPT_SUBJECTALTNAME_NODEFAULT:

diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
@@ -64,6 +64,7 @@ struct ossl_cmp_ctx_st {
      * certificate responses (ip/cp/kup), revocation responses (rp), and PKIConf
      */
     int unprotectedErrors;
+    int noCacheExtraCerts;
     X509 *srvCert; /* certificate used to identify the server */
     X509 *validatedSrvCert; /* caches any already validated server cert */
     X509_NAME *expected_sender; /* expected sender in header of response */

diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c
@@ -705,6 +705,7 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
 {
     OSSL_CMP_PKIHEADER *hdr;
     const X509_NAME *expected_sender;
+    int num_untrusted, num_added, res;
 
     if (!ossl_assert(ctx != NULL && msg != NULL && msg->header != NULL))
         return 0;
@@ -728,41 +729,54 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
         return 0;
     /* Note: if recipient was NULL-DN it could be learned here if needed */
 
-    if (sk_X509_num(msg->extraCerts) > 10)
-        ossl_cmp_warn(ctx,
-                      "received CMP message contains more than 10 extraCerts");
+    num_added = sk_X509_num(msg->extraCerts);
+    if (num_added > 10)
+        ossl_cmp_log1(WARN, ctx, "received CMP message contains %d extraCerts",
+                      num_added);
     /*
      * Store any provided extraCerts in ctx for use in OSSL_CMP_validate_msg()
      * and for future use, such that they are available to ctx->certConf_cb and
      * the peer does not need to send them again in the same transaction.
      * Note that it does not help validating the message before storing the
      * extraCerts because they do not belong to the protected msg part anyway.
-     * For efficiency, the extraCerts are prepended so they get used first.
+     * The extraCerts are prepended. Allows simple removal if they shall not be
+     * cached. Also they get used first, which is likely good for efficiency.
      */
-    if (!X509_add_certs(ctx->untrusted, msg->extraCerts,
-                        /* this allows self-signed certs */
-                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
-                        | X509_ADD_FLAG_PREPEND))
+    num_untrusted = ctx->untrusted == NULL ? 0 : sk_X509_num(ctx->untrusted);
+    res = ossl_x509_add_certs_new(&ctx->untrusted, msg->extraCerts,
+                                  /* this allows self-signed certs */
+                                  X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
+                                  | X509_ADD_FLAG_PREPEND);
+    num_added = (ctx->untrusted == NULL ? 0 : sk_X509_num(ctx->untrusted))
+        - num_untrusted;
+    if (!res) {
+        while (num_added-- > 0)
+            X509_free(sk_X509_shift(ctx->untrusted));
         return 0;
+    }
 
-    /* validate message protection */
-    if (hdr->protectionAlg != NULL) {
-        /* detect explicitly permitted exceptions for invalid protection */
-        if (!OSSL_CMP_validate_msg(ctx, msg)
-                && (cb == NULL || (*cb)(ctx, msg, 1, cb_arg) <= 0)) {
-#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-            ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_VALIDATING_PROTECTION);
-            return 0;
+    if (hdr->protectionAlg != NULL)
+        res = OSSL_CMP_validate_msg(ctx, msg)
+            /* explicitly permitted exceptions for invalid protection: */
+            || (cb != NULL && (*cb)(ctx, msg, 1, cb_arg) > 0);
+    else
+        /* explicitly permitted exceptions for missing protection: */
+        res = cb != NULL && (*cb)(ctx, msg, 0, cb_arg) > 0;
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+    res = 1; /* support more aggressive fuzzing by letting invalid msg pass */
 #endif
-        }
-    } else {
-        /* detect explicitly permitted exceptions for missing protection */
-        if (cb == NULL || (*cb)(ctx, msg, 0, cb_arg) <= 0) {
-#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+
+    /* remove extraCerts again if not caching */
+    if (ctx->noCacheExtraCerts)
+        while (num_added-- > 0)
+            X509_free(sk_X509_shift(ctx->untrusted));
+
+    if (!res) {
+        if (hdr->protectionAlg != NULL)
+            ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_VALIDATING_PROTECTION);
+        else
             ERR_raise(ERR_LIB_CMP, CMP_R_MISSING_PROTECTION);
-            return 0;
-#endif
-        }
+        return 0;
     }
 
     /* check CMP version number in header */
@@ -820,18 +834,6 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
     if (!ossl_cmp_ctx_set1_recipNonce(ctx, hdr->senderNonce))
         return 0;
 
-    /*
-     * Store any provided extraCerts in ctx for future use,
-     * such that they are available to ctx->certConf_cb and
-     * the peer does not need to send them again in the same transaction.
-     * For efficiency, the extraCerts are prepended so they get used first.
-     */
-    if (!X509_add_certs(ctx->untrusted, msg->extraCerts,
-                        /* this allows self-signed certs */
-                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
-                        | X509_ADD_FLAG_PREPEND))
-        return 0;
-
     if (ossl_cmp_hdr_get_protection_nid(hdr) == NID_id_PasswordBasedMAC) {
         /*
          * RFC 4210, 5.3.2: 'Note that if the PKI Message Protection is

diff --git a/doc/man1/openssl-cmp.pod.in b/doc/man1/openssl-cmp.pod.in
@@ -66,6 +66,7 @@ Server authentication options:
 [B<-expect_sender> I<name>]
 [B<-ignore_keyusage>]
 [B<-unprotected_errors>]
+[B<-no_cache_extracerts>]
 [B<-srvcertout> I<filename>]
 [B<-extracertsout> I<filename>]
 [B<-cacertsout> I<filename>]
@@ -670,6 +671,12 @@ with a signature key."
 
 =back
 
+=item B<-no_cache_extracerts>
+
+Do not cache certificates in the extraCerts field of CMP messages received.
+By default, they are kept as they may be helful for validating further messages.
+This option applies to both CMP clients and the mock server.
+
 =item B<-srvcertout> I<filename>
 
 The file where to save the successfully validated certificate, if any,

diff --git a/doc/man3/OSSL_CMP_CTX_new.pod b/doc/man3/OSSL_CMP_CTX_new.pod
@@ -344,6 +344,11 @@ RFC 4210.
         Allow retrieving a trust anchor from extraCerts and using that
         to validate the certificate chain of an IP message.
 
+=item B<OSSL_CMP_OPT_NO_CACHE_EXTRACERTS>
+
+        Do not cache certificates received in the extraCerts CMP message field.
+        Otherwise they are stored to potentially help validate further messages.
+
 =back
 
 OSSL_CMP_CTX_get_option() reads the current value of the given option
@@ -472,6 +477,8 @@ of intermediate CAs that may be useful for path construction for the own CMP
 signer certificate, for the own TLS certificate (if any), when verifying peer
 CMP protection certificates, and when verifying newly enrolled certificates.
 The reference counts of those certificates handled successfully are increased.
+This list of untrusted certificates in I<ctx> will get augmented by extraCerts
+in received CMP messages unless B<OSSL_CMP_OPT_NO_CACHE_EXTRACERTS> is set.
 
 OSSL_CMP_CTX_get0_untrusted() returns a pointer to the
 list of untrusted certs in I<ctx>, which may be empty if unset.

diff --git a/include/openssl/cmp.h.in b/include/openssl/cmp.h.in
@@ -310,6 +310,7 @@ const char *OSSL_CMP_CTX_get0_propq(const OSSL_CMP_CTX *ctx);
 #  define OSSL_CMP_OPT_DIGEST_ALGNID 34
 #  define OSSL_CMP_OPT_IGNORE_KEYUSAGE 35
 #  define OSSL_CMP_OPT_PERMIT_TA_IN_EXTRACERTS_FOR_IR 36
+#  define OSSL_CMP_OPT_NO_CACHE_EXTRACERTS 37
 int OSSL_CMP_CTX_set_option(OSSL_CMP_CTX *ctx, int opt, int val);
 int OSSL_CMP_CTX_get_option(const OSSL_CMP_CTX *ctx, int opt);
 /* CMP-specific callback for logging and outputting the error queue: */