CMP: On NULL-DN subject or issuer input omit field in cert template

Also improve diagnostics on inconsistent cert request input in apps/cmp.c,
add trace output for transactionIDs on new sessions,
and update the documentation in openssl-cmp.pod.in.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14018)

4 files changed, 99 insertions, 50 deletions

diff --git a/apps/cmp.c b/apps/cmp.c
index d04af4177b..40815930cf 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -1610,11 +1610,84 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine)
             && opt_csr == NULL && opt_oldcert == NULL && opt_cert == NULL
             && opt_cmd != CMP_RR && opt_cmd != CMP_GENM)
         CMP_warn("no -subject given; no -csr or -oldcert or -cert available for fallback");
-    if (!set_name(opt_subject, OSSL_CMP_CTX_set1_subjectName, ctx, "subject")
-            || !set_name(opt_issuer, OSSL_CMP_CTX_set1_issuer, ctx, "issuer"))
+
+    if (opt_cmd == CMP_IR || opt_cmd == CMP_CR || opt_cmd == CMP_KUR) {
+        if (opt_newkey == NULL && opt_key == NULL && opt_csr == NULL) {
+            CMP_err("missing -newkey (or -key) to be certified and no -csr given");
+            return 0;
+        }
+        if (opt_certout == NULL) {
+            CMP_err("-certout not given, nowhere to save newly enrolled certificate");
+            return 0;
+        }
+        if (!set_name(opt_subject, OSSL_CMP_CTX_set1_subjectName, ctx, "subject")
+                || !set_name(opt_issuer, OSSL_CMP_CTX_set1_issuer, ctx, "issuer"))
+            return 0;
+    } else {
+        const char *msg = "option is ignored for commands other than 'ir', 'cr', and 'kur'";
+
+        if (opt_subject != NULL) {
+            if (opt_ref == NULL && opt_cert == NULL) {
+                /* use subject as default sender unless oldcert subject is used */
+                if (!set_name(opt_subject, OSSL_CMP_CTX_set1_subjectName, ctx, "subject"))
+                    return 0;
+            } else {
+                CMP_warn1("-subject %s since -ref or -cert is given", msg);
+            }
+        }
+        if (opt_issuer != NULL)
+            CMP_warn1("-issuer %s", msg);
+        if (opt_reqexts != NULL)
+            CMP_warn1("-reqexts %s", msg);
+        if (opt_san_nodefault)
+            CMP_warn1("-san_nodefault %s", msg);
+        if (opt_sans != NULL)
+            CMP_warn1("-sans %s", msg);
+        if (opt_policies != NULL)
+            CMP_warn1("-policies %s", msg);
+        if (opt_policy_oids != NULL)
+            CMP_warn1("-policy_oids %s", msg);
+    }
+    if (opt_cmd == CMP_KUR) {
+        char *ref_cert = opt_oldcert != NULL ? opt_oldcert : opt_cert;
+
+        if (ref_cert == NULL && opt_csr == NULL) {
+            CMP_err("missing -oldcert for certificate to be updated and no -csr given");
+            return 0;
+        }
+        if (opt_subject != NULL)
+            CMP_warn2("given -subject '%s' overrides the subject of '%s' for KUR",
+                      opt_subject, ref_cert != NULL ? ref_cert : opt_csr);
+    }
+    if (opt_cmd == CMP_RR) {
+        if (opt_oldcert == NULL && opt_csr == NULL) {
+            CMP_err("missing -oldcert for certificate to be revoked and no -csr given");
+            return 0;
+        }
+        if (opt_oldcert != NULL && opt_csr != NULL)
+            CMP_warn("ignoring -csr since certificate to be revoked is given");
+    }
+    if (opt_cmd == CMP_P10CR && opt_csr == NULL) {
+        CMP_err("missing PKCS#10 CSR for p10cr");
         return 0;
+    }
 
-    if (opt_newkey != NULL) {
+    if (opt_recipient == NULL && opt_srvcert == NULL && opt_issuer == NULL
+            && opt_oldcert == NULL && opt_cert == NULL)
+        CMP_warn("missing -recipient, -srvcert, -issuer, -oldcert or -cert; recipient will be set to \"NULL-DN\"");
+
+    if (opt_cmd == CMP_P10CR || opt_cmd == CMP_RR) {
+        const char *msg = "option is ignored for 'p10cr' and 'rr' commands";
+
+        if (opt_newkeypass != NULL)
+            CMP_warn1("-newkeytype %s", msg);
+        if (opt_newkey != NULL)
+            CMP_warn1("-newkey %s", msg);
+        if (opt_days != 0)
+            CMP_warn1("-days %s", msg);
+        if (opt_popo != OSSL_CRMF_POPO_NONE - 1)
+            CMP_warn1("-popo %s", msg);
+    } else if (opt_newkey != NULL) {
         const char *file = opt_newkey;
         const int format = opt_keyform;
         const char *pass = opt_newkeypass;
@@ -1884,44 +1957,6 @@ static int setup_client_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine)
     if (!transform_opts())
         goto err;
 
-    if (opt_cmd == CMP_IR || opt_cmd == CMP_CR || opt_cmd == CMP_KUR) {
-        if (opt_newkey == NULL && opt_key == NULL && opt_csr == NULL) {
-            CMP_err("missing -newkey (or -key) to be certified");
-            goto err;
-        }
-        if (opt_certout == NULL) {
-            CMP_err("-certout not given, nowhere to save certificate");
-            goto err;
-        }
-    }
-    if (opt_cmd == CMP_KUR) {
-        char *ref_cert = opt_oldcert != NULL ? opt_oldcert : opt_cert;
-
-        if (ref_cert == NULL && opt_csr == NULL) {
-            CMP_err("missing -oldcert for certificate to be updated and no fallback -csr given");
-            goto err;
-        }
-        if (opt_subject != NULL)
-            CMP_warn2("given -subject '%s' overrides the subject of '%s' for KUR",
-                      opt_subject, ref_cert != NULL ? ref_cert : opt_csr);
-    }
-    if (opt_cmd == CMP_RR) {
-        if (opt_oldcert == NULL && opt_csr == NULL) {
-            CMP_err("missing -oldcert for certificate to be revoked and no fallback -csr given");
-            goto err;
-        }
-        if (opt_oldcert != NULL && opt_csr != NULL)
-            CMP_warn("ignoring -csr since certificate to be revoked is given");
-    }
-    if (opt_cmd == CMP_P10CR && opt_csr == NULL) {
-        CMP_err("missing PKCS#10 CSR for p10cr");
-        goto err;
-    }
-
-    if (opt_recipient == NULL && opt_srvcert == NULL && opt_issuer == NULL
-            && opt_oldcert == NULL && opt_cert == NULL)
-        CMP_warn("missing -recipient, -srvcert, -issuer, -oldcert or -cert; recipient will be set to \"NULL-DN\"");
-
     if (opt_infotype_s != NULL) {
         char id_buf[100] = "id-it-";
 
diff --git a/crypto/cmp/cmp_hdr.c b/crypto/cmp/cmp_hdr.c
index 5882d9c9de..58b07dd8b2 100644
--- a/crypto/cmp/cmp_hdr.c
+++ b/crypto/cmp/cmp_hdr.c
@@ -72,15 +72,11 @@ ASN1_OCTET_STRING *OSSL_CMP_HDR_get0_recipNonce(const OSSL_CMP_PKIHEADER *hdr)
     return hdr->recipNonce;
 }
 
+/* a NULL-DN as an empty sequence of RDNs */
 int ossl_cmp_general_name_is_NULL_DN(GENERAL_NAME *name)
 {
-    X509_NAME *null = X509_NAME_new();
-    int res = name == NULL || null == NULL
-        || (name->type == GEN_DIRNAME
-                && X509_NAME_cmp(name->d.directoryName, null) == 0);
-
-    X509_NAME_free(null);
-    return res;
+    return name == NULL
+        || (name->type == GEN_DIRNAME && IS_NULL_DN(name->d.directoryName));
 }
 
 /* assign to *tgt a copy of src (which may be NULL to indicate an empty DN) */
@@ -273,6 +269,20 @@ int ossl_cmp_hdr_has_implicitConfirm(const OSSL_CMP_PKIHEADER *hdr)
  */
 int ossl_cmp_hdr_set_transactionID(OSSL_CMP_CTX *ctx, OSSL_CMP_PKIHEADER *hdr)
 {
+    if (ctx->transactionID == NULL) {
+        char *tid;
+
+        if (!set_random(&ctx->transactionID, ctx,
+                        OSSL_CMP_TRANSACTIONID_LENGTH))
+            return 0;
+        tid = OPENSSL_buf2hexstr(ctx->transactionID->data,
+                                 ctx->transactionID->length);
+        if (tid != NULL)
+            ossl_cmp_log1(DEBUG, ctx,
+                          "Starting new transaction with ID=%s", tid);
+        OPENSSL_free(tid);
+    }
+
     if (ctx->transactionID == NULL
         && !set_random(&ctx->transactionID, ctx, OSSL_CMP_TRANSACTIONID_LENGTH))
         return 0;
diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
index a4d3cf9ea4..c5f4fd198d 100644
--- a/crypto/cmp/cmp_local.h
+++ b/crypto/cmp/cmp_local.h
@@ -25,6 +25,8 @@
 # include <openssl/x509v3.h>
 # include "crypto/x509.h"
 
+#define IS_NULL_DN(name) (X509_NAME_get_entry(name, 0) == NULL)
+
 /*
  * this structure is used to store the context for CMP sessions
  */
@@ -778,6 +780,7 @@ int ossl_cmp_print_log(OSSL_CMP_severity level, const OSSL_CMP_CTX *ctx,
 # define ossl_cmp_warn(ctx, msg)  ossl_cmp_log(WARN,  ctx, msg)
 # define ossl_cmp_info(ctx, msg)  ossl_cmp_log(INFO,  ctx, msg)
 # define ossl_cmp_debug(ctx, msg) ossl_cmp_log(DEBUG, ctx, msg)
+# define ossl_cmp_trace(ctx, msg) ossl_cmp_log(TRACE, ctx, msg)
 int ossl_cmp_ctx_set0_validatedSrvCert(OSSL_CMP_CTX *ctx, X509 *cert);
 int ossl_cmp_ctx_set_status(OSSL_CMP_CTX *ctx, int status);
 int ossl_cmp_ctx_set0_statusString(OSSL_CMP_CTX *ctx,
diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index 8514336801..09b2d7b03b 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -218,7 +218,7 @@ static const X509_NAME *determine_subj(OSSL_CMP_CTX *ctx,
                                        int for_KUR)
 {
     if (ctx->subjectName != NULL)
-        return ctx->subjectName;
+        return IS_NULL_DN(ctx->subjectName) ? NULL : ctx->subjectName;
 
     if (ref_subj != NULL && (for_KUR || !HAS_SAN(ctx)))
         /*
@@ -241,7 +241,8 @@ OSSL_CRMF_MSG *OSSL_CMP_CTX_setup_CRM(OSSL_CMP_CTX *ctx, int for_KUR, int rid)
         refcert != NULL ? X509_get_subject_name(refcert) : NULL;
     const X509_NAME *subject = determine_subj(ctx, ref_subj, for_KUR);
     const X509_NAME *issuer = ctx->issuer != NULL || refcert == NULL
-        ? ctx->issuer : X509_get_issuer_name(refcert);
+        ? (IS_NULL_DN(ctx->issuer) ? NULL : ctx->issuer)
+        : X509_get_issuer_name(refcert);
     int crit = ctx->setSubjectAltNameCritical || subject == NULL;
     /* RFC5280: subjectAltName MUST be critical if subject is null */
     X509_EXTENSIONS *exts = NULL;