update as per review

apps/cmp.c

@@ -146,6 +146,8 @@ static int opt_revreason = CRL_REASON_NONE;
 /* credentials format */
 static char *opt_certform_s = "PEM";
 static int opt_certform = FORMAT_PEM;
+static char *opt_crlform_s = "DER";
+static int opt_crlform = FORMAT_ASN1;
 static char *opt_keyform_s = NULL;
 static int opt_keyform = FORMAT_UNDEF;
 static char *opt_otherpass = NULL;
@@ -247,7 +249,7 @@ typedef enum OPTION_choice {
     OPT_DIGEST, OPT_MAC, OPT_EXTRACERTS,
     OPT_UNPROTECTED_REQUESTS,
 
-    OPT_CERTFORM, OPT_KEYFORM,
+    OPT_CERTFORM, OPT_CRLFORM, OPT_KEYFORM,
     OPT_OTHERPASS,
 #ifndef OPENSSL_NO_ENGINE
     OPT_ENGINE,
@@ -434,7 +436,7 @@ const OPTIONS cmp_options[] = {
     { "oldwithnew", OPT_OLDWITHNEW, 's',
       "File to save OldWithNew cert received in genp of type rootCaKeyUpdate"},
     { "crlcert", OPT_CRLCERT, 's',
-      "certificate to request CRL update for in genm of type crlStatusList"},
+      "certificate to request a CRL for in genm of type crlStatusList"},
     { "oldcrl", OPT_OLDCRL, 's',
       "CRL to request update for in genm of type crlStatusList"},
     { "crlout", OPT_CRLOUT, 's',
@@ -470,6 +472,8 @@ const OPTIONS cmp_options[] = {
     OPT_SECTION("Credentials format"),
     {"certform", OPT_CERTFORM, 's',
      "Format (PEM or DER) to use when saving a certificate to a file. Default PEM"},
+     {"crlform", OPT_CRLFORM, 's',
+     "Format (PEM or DER) to use when saving a CRL to a file. Default DER"},
     {"keyform", OPT_KEYFORM, 's',
      "Format of the key input (ENGINE, other values ignored)"},
     {"otherpass", OPT_OTHERPASS, 's',
@@ -643,7 +647,7 @@ static varref cmp_vars[] = { /* must be in same order as enumerated above! */
     {&opt_digest}, {&opt_mac}, {&opt_extracerts},
     {(char **)&opt_unprotected_requests},
 
-    {&opt_certform_s}, {&opt_keyform_s},
+    {&opt_certform_s}, {&opt_crlform_s}, {&opt_keyform_s},
     {&opt_otherpass},
 #ifndef OPENSSL_NO_ENGINE
     {&opt_engine},
@@ -1085,6 +1089,11 @@ static int transform_opts(void)
         CMP_err("unknown option given for certificate storing format");
         return 0;
     }
+    if (opt_crlform_s != NULL
+            && !opt_format(opt_crlform_s, OPT_FMT_PEMDER, &opt_crlform)) {
+        CMP_err("unknown option given for CRL storing format");
+        return 0;
+    }
 
     return 1;
 }
@@ -1946,20 +1955,20 @@ static int add_certProfile(OSSL_CMP_CTX *ctx, const char *name)
 
     if ((sk = sk_ASN1_UTF8STRING_new_reserve(NULL, 1)) == NULL)
         return 0;
-    if ((utf8string = ASN1_UTF8STRING_new()) == NULL)
-        goto err;
-    if (!ASN1_STRING_set(utf8string, name, (int)strlen(name))) {
-        ASN1_STRING_free(utf8string);
-        goto err;
-    }
-    /* Due to sk_ASN1_UTF8STRING_new_reserve(NULL, 1), this surely succeeds: */
-    (void)sk_ASN1_UTF8STRING_push(sk, utf8string);
-    if ((itav = OSSL_CMP_ITAV_new0_certProfile(sk)) == NULL)
-        goto err;
-    if (OSSL_CMP_CTX_push0_geninfo_ITAV(ctx, itav))
-        return 1;
-    OSSL_CMP_ITAV_free(itav);
-    return 0;
+   if ((utf8string = ASN1_UTF8STRING_new()) == NULL)
+       goto err;
+   if (!ASN1_STRING_set(utf8string, name, (int)strlen(name))) {
+       ASN1_STRING_free(utf8string);
+       goto err;
+   }
+   /* Due to sk_ASN1_UTF8STRING_new_reserve(NULL, 1), this surely succeeds: */
+   (void)sk_ASN1_UTF8STRING_push(sk, utf8string);
+   if ((itav = OSSL_CMP_ITAV_new0_certProfile(sk)) == NULL)
+       goto err;
+   if (OSSL_CMP_CTX_push0_geninfo_ITAV(ctx, itav))
+       return 1;
+   OSSL_CMP_ITAV_free(itav);
+   return 0;
 
  err:
     sk_ASN1_UTF8STRING_pop_free(sk, ASN1_UTF8STRING_free);
@@ -2004,7 +2013,7 @@ static int handle_opt_geninfo(OSSL_CMP_CTX *ctx)
             if (*ptr != '\0') {
                 if (*ptr != ',') {
                     CMP_err1("Missing ',' or end of -geninfo arg after int at %.40s",
-                             ptr);
+                        ptr);
                     goto err;
                 }
                 ptr++;
@@ -2281,13 +2290,13 @@ static int write_cert(BIO *bio, X509 *cert)
 
 static int write_crl(BIO *bio, X509_CRL *crl)
 {
-    if (opt_certform != FORMAT_PEM && opt_certform != FORMAT_ASN1) {
+    if (opt_crlform != FORMAT_PEM && opt_crlform != FORMAT_ASN1) {
         BIO_printf(bio_err, "error: unsupported type '%s' for writing CRLs\n",
-                   opt_certform_s);
+                   opt_crlform_s);
         return 0;
     }
 
-    return opt_certform == FORMAT_PEM ? PEM_write_bio_X509_CRL(bio, crl)
+    return opt_crlform == FORMAT_PEM ? PEM_write_bio_X509_CRL(bio, crl)
                                       : i2d_X509_CRL_bio(bio, crl);
 }
 
@@ -2338,26 +2347,26 @@ static int save_free_certs(STACK_OF(X509) *certs,
     return n;
 }
 
-static int save_free_crl(X509_CRL *crl,
-                         const char *file, const char *desc)
+static int save_crl(X509_CRL *crl,
+                    const char *file, const char *desc)
 {
     BIO *bio = NULL;
     int res = 0;
 
     if (file == NULL)
-        goto end;
+        return 1;
     if (crl != NULL)
-        CMP_info2("received %s CRL, saving to file '%s'", desc, file);
+        CMP_info2("received %s, saving to file '%s'", desc, file);
 
     if ((bio = BIO_new(BIO_s_file())) == NULL
             || !BIO_write_filename(bio, (char *)file)) {
-        CMP_err2("could not open file '%s' for writing %s CRL",
+        CMP_err2("could not open file '%s' for writing %s",
                  file, desc);
         goto end;
     }
 
     if (!write_crl(bio, crl)) {
-        CMP_err2("cannot write %s crl to file '%s'", desc, file);
+        CMP_err2("cannot write %s to file '%s'", desc, file);
         goto end;
     }
     res = 1;
@@ -2404,14 +2413,7 @@ static int save_crl_or_delete(X509_CRL *crl, const char *file, const char *desc)
 {
     if (file == NULL)
         return 1;
-    if (crl == NULL) {
-        char desc_crl[80];
-
-        BIO_snprintf(desc_crl, sizeof(desc_crl), "%s", desc);
-        return delete_file(file, desc_crl);
-    } else {
-        return save_free_crl(crl, file, desc);
-    }
+    return (crl == NULL) ? delete_file(file, desc) : save_crl(crl, file, desc);
 }
 
 static int print_itavs(const STACK_OF(OSSL_CMP_ITAV) *itavs)
@@ -2916,6 +2918,9 @@ static int get_opts(int argc, char **argv)
         case OPT_CERTFORM:
             opt_certform_s = opt_str();
             break;
+        case OPT_CRLFORM:
+            opt_crlform_s = opt_str();
+            break;
         case OPT_KEYFORM:
             opt_keyform_s = opt_str();
             break;
@@ -3253,18 +3258,14 @@ static int do_genm(OSSL_CMP_CTX *ctx)
             return 0;
         }
 
-        if (opt_crlcert == NULL) {
-            CMP_warn("No -crlcert given, will use data from -oldcrl");
-        } else {
+        if (opt_crlcert != NULL) {
             crlcert = load_cert_pwd(opt_crlcert, opt_otherpass,
                                     "Cert for genm with -infotype crlStatusList");
             if (crlcert == NULL)
                 goto end_crlupd;
         }
 
-        if (opt_oldcrl == NULL) {
-            CMP_warn("No -oldcrl given, will use data from -crlcert");
-        } else {
+        if (opt_oldcrl != NULL) {
             oldcrl = load_crl(opt_oldcrl, FORMAT_UNDEF, 0,
                               "CRL for genm with -infotype crlStatusList");
             if (oldcrl == NULL)

apps/lib/cmp_mock_srv.c

@@ -413,8 +413,8 @@ static int check_client_crl(const STACK_OF(OSSL_CMP_CRLSTATUS) *crlStatusList,
                             const X509_CRL *crl)
 {
     OSSL_CMP_CRLSTATUS *crlstatus;
-    DIST_POINT_NAME *distpoint;
-    GENERAL_NAMES *gen;
+    DIST_POINT_NAME *dpn;
+    GENERAL_NAMES *issuer;
     ASN1_TIME *thisupd = NULL;
 
     if (sk_OSSL_CMP_CRLSTATUS_num(crlStatusList) != 1) {
@@ -425,11 +425,11 @@ static int check_client_crl(const STACK_OF(OSSL_CMP_CRLSTATUS) *crlStatusList,
         return 0;
 
     crlstatus = sk_OSSL_CMP_CRLSTATUS_value(crlStatusList, 0);
-    if (!OSSL_CMP_CRLSTATUS_get0(crlstatus, &distpoint, &gen, &thisupd))
+    if (!OSSL_CMP_CRLSTATUS_get0(crlstatus, &dpn, &issuer, &thisupd))
         return -1;
 
-    if (gen != NULL) {
-        GENERAL_NAME *gn = sk_GENERAL_NAME_value(gen, 0);
+    if (issuer != NULL) {
+        GENERAL_NAME *gn = sk_GENERAL_NAME_value(issuer, 0);
 
         if (gn != NULL && gn->type == GEN_DIRNAME) {
             X509_NAME *gen_name = gn->d.dirn;
@@ -438,6 +438,9 @@ static int check_client_crl(const STACK_OF(OSSL_CMP_CRLSTATUS) *crlStatusList,
                 ERR_raise(ERR_LIB_CMP, CMP_R_UNKNOWN_CRL_ISSUER);
                 return -1;
             }
+        } else {  
+            ERR_raise(ERR_LIB_CMP, CMP_R_SENDER_GENERALNAME_TYPE_NOT_SUPPORTED);  
+            return -1; /* error according to RFC 9483 section 4.3.4 */  
         }
     }
 

crypto/cmp/cmp_asn.c

@@ -322,7 +322,7 @@ OSSL_CMP_ITAV *OSSL_CMP_ITAV_new_rootCaKeyUpdate(const X509 *newWithNew,
     itav->infoValue.rootCaKeyUpdate = upd;
     return itav;
 
- err:
+    err:
     OSSL_CMP_ROOTCAKEYUPDATE_free(upd);
     return NULL;
 }
@@ -459,8 +459,9 @@ OSSL_CMP_CRLSTATUS *OSSL_CMP_CRLSTATUS_create(const X509_CRL *crl,
     int i, NID_akid = NID_authority_key_identifier;
 
     /*
-     * Note: X509{,_CRL}_get_ext_d2i(..., NID, ..., NULL) return the 1st extension
-     * with the given NID that is available, if any. If there are more, this is an error.
+     * Note:
+     * X509{,_CRL}_get_ext_d2i(..., NID, ..., NULL) return the 1st extension with
+     * given NID that is available, if any. If there are more, this is an error.
      */
     if (cert != NULL) {
         crldps = X509_get_ext_d2i(cert, NID_crl_distribution_points, NULL, NULL);

crypto/cmp/cmp_genm.c

@@ -373,12 +373,10 @@ int OSSL_CMP_get1_crlUpdate(OSSL_CMP_CTX *ctx, const X509 *crlcert,
 
     if ((req = OSSL_CMP_ITAV_new0_crlStatusList(list)) == NULL)
         goto end;
-
     status = NULL;
     list = NULL;
 
-    itav = get_genm_itav(ctx, req, NID_id_it_crls, "crl");
-    if (itav == NULL)
+    if ((itav = get_genm_itav(ctx, req, NID_id_it_crls, "crl")) == NULL)
         goto end;
 
     if (!OSSL_CMP_ITAV_get0_crls(itav, &crls))

doc/man1/openssl-cmp.pod.in

@@ -94,6 +94,7 @@ Client authentication and protection options:
 Credentials format options:
 
 [B<-certform> I<PEM|DER>]
+[B<-crlform> I<PEM|DER>]
 [B<-keyform> I<PEM|DER|P12|ENGINE>]
 [B<-otherpass> I<arg>]
 {- $OpenSSL::safe::opt_engine_synopsis -}{- $OpenSSL::safe::opt_provider_synopsis -}
@@ -251,7 +252,8 @@ ITAV B<infoType>s is printed to stdout.
 
 Set InfoType name to use for requesting specific info in B<genm>,
 e.g., C<signKeyPairTypes>.
-So far, there is specific support for C<caCerts> and C<rootCaCert>.
+So far, there is specific support for C<caCerts>, C<rootCaCert>
+and C<crlStatusList>.
 
 =item B<-profile> I<name>
 
@@ -741,11 +743,14 @@ If on success no such cert was received, this is indicated by deleting the file.
 
 Certificate used for specifying a CRL issuer when requesting a CRL 
 in a genm message with infoType C<crlStatusList>.
+Any available distribution point name is preferred over issuer names.
 
 =item B<-oldcrl> I<filename>
 
 CRL used for specifying a CRL issuer when requesting a CRL 
 in a genm message with infoType C<crlStatusList>.
+Any available distribution point name is preferred over issuer names.
+If also -crlcrt is given, its data is preferred over data from -oldcrl.
 
 =item B<-crlout> I<filename>
 
@@ -877,6 +882,11 @@ Send request messages without CMP-level protection.
 File format to use when saving a certificate to a file.
 Default value is PEM.
 
+=item B<-crlform> I<PEM|DER>
+
+File format to use when saving a CRL to a file.
+Default value is DER.
+
 =item B<-keyform> I<PEM|DER|P12|ENGINE>
 
 The format of the key input; unspecified by default.
@@ -1162,7 +1172,7 @@ Certificate to be returned as mock enrollment result.
 
 =item B<-rsp_crl> I<filename>|I<uri>
 
-CRL to be returned in genp of type crls.
+CRL to be returned in genp of type C<crls>.
 
 =item B<-rsp_extracerts> I<filenames>|I<uris>
 
@@ -1461,7 +1471,8 @@ The B<cmp> application was added in OpenSSL 3.0.
 
 The B<-engine> option was deprecated in OpenSSL 3.0.
 
-The B<-profile> option was added in OpenSSL 3.3.
+B<-profile>, B<-crlcert>, B<-oldcrl>, B<-crlout>, B<-crlform>
+and B<-rsp_crl> options were added in OpenSSL 3.3.
 
 =head1 COPYRIGHT
 

doc/man3/GENERAL_NAME.pod

@@ -21,12 +21,10 @@ and populates it based on provided X509_NAME I<src> which can be NULL.
 I<tgt> must not be NULL. If successful, I<*tgt> will be set to point
 to the newly created GENERAL_NAME.
 
-=head1 NOTES
 =head1 RETURN VALUES
 
 GENERAL_NAME_set1_X509_NAME() return 1 on success, 0 on error.
 
-=head1 SEE ALSO
 =head1 HISTORY
 
 GENERAL_NAME_set1_X509_NAME() was added in OpenSSL 3.3.

doc/man3/OSSL_CMP_ITAV_new_caCerts.pod

@@ -93,7 +93,6 @@ OSSL_CMP_CRLSTATUS_create() is a high-level variant of OSSL_CMP_CRLSTATUS_new1()
 It fills the thisUpdate field with a copy of the thisUpdate field of I<crl> if present.
 It fills the CRLSource field with a copy of the first data item found using the I<crl>
 and/or I<cert> parameters as follows.
-The CRLSource field is filled with the first data item found in them as follows.
 Any available distribution point name is preferred over issuer names.
 Data from I<cert>, if present, is preferred over data from I<crl>.
 If no distribution point names are available,

doc/man3/OSSL_CMP_exec_certreq.pod

@@ -42,7 +42,8 @@ OSSL_CMP_get1_crlUpdate
  int OSSL_CMP_get1_rootCaKeyUpdate(OSSL_CMP_CTX *ctx,
                                    const X509 *oldWithOld, X509 **newWithNew,
                                    X509 **newWithOld, X509 **oldWithNew);
- int OSSL_CMP_get1_crlUpdate(OSSL_CMP_CTX *ctx, const X509_CRL *last_crl,
+ int OSSL_CMP_get1_crlUpdate(OSSL_CMP_CTX *ctx, const X509 *crlcert,
+                             const X509_CRL *last_crl,
                              X509_CRL **crl);
 
 =head1 DESCRIPTION
@@ -162,8 +163,9 @@ the weakest trust in any of the certificates in the trust store of I<ctx>.
 
 OSSL_CMP_get1_crlUpdate() uses a genm request message with infoType crlStatusList
 to obtain CRL from the CMP server referenced by I<ctx> in a genp response message
-with infoType crls. It uses oldcert referenced by I<ctx> and I<last_crl> to create
-request. On success it assigns to I<*crl> the CRL received.
+with infoType crls. It uses I<last_crl> and I<crlcert> to create  
+a request with a status field as described for L<OSSL_CMP_CRLSTATUS_create(3)>. 
+On success it assigns to I<*crl> the CRL received.
 NULL means that no CRL was provided by the server.
 The CRL obtained this way must be freed by the caller.
 
@@ -221,7 +223,7 @@ L<OSSL_CMP_CTX_new(3)>, L<OSSL_CMP_CTX_free(3)>,
 L<OSSL_CMP_CTX_set1_subjectName(3)>, L<OSSL_CMP_CTX_set0_newPkey(3)>,
 L<OSSL_CMP_CTX_set1_p10CSR(3)>, L<OSSL_CMP_CTX_set1_oldCert(3)>,
 L<OSSL_CMP_CTX_get0_newCert(3)>, L<OSSL_CMP_CTX_push0_genm_ITAV(3)>,
-L<OSSL_CMP_MSG_http_perform(3)>
+L<OSSL_CMP_MSG_http_perform(3)>, L<OSSL_CMP_CRLSTATUS_create(3)>
 
 =head1 HISTORY
 

include/crypto/cmperr.h

@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 2020-2024 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2020-2023 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy