CMP app: fix file output of certs and cert lists on non-existing cert(s)

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/20035)

diff --git a/apps/cmp.c b/apps/cmp.c
index bc446a4654c03afb3e0b69ef0fc1a232d673c617..e44d32fd7f04965207920439471e31750538ec00 100644 (file)
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -1989,7 +1989,7 @@ static int write_cert(BIO *bio, X509 *cert)
  * where DER does not make much sense for writing more than one cert!
  * Returns number of written certificates on success, -1 on error.
  */
-static int save_free_certs(OSSL_CMP_CTX *ctx, STACK_OF(X509) *certs,
+static int save_free_certs(STACK_OF(X509) *certs,
                            const char *file, const char *desc)
 {
     BIO *bio = NULL;
@@ -2028,24 +2028,28 @@ static int save_free_certs(OSSL_CMP_CTX *ctx, STACK_OF(X509) *certs,
     return n;
 }
 
-static int delete_certfile(const char *file, const char *desc)
+static int delete_file(const char *file, const char *desc)
 {
     if (file == NULL)
         return 1;
 
     if (unlink(file) != 0 && errno != ENOENT) {
-        CMP_err2("Failed to delete %s, which should be done to indicate there is no %s cert",
+        CMP_err2("Failed to delete %s, which should be done to indicate there is no %s",
                  file, desc);
         return 0;
     }
     return 1;
 }
 
-static int save_cert(OSSL_CMP_CTX *ctx, X509 *cert,
-                     const char *file, const char *desc)
+static int save_cert_or_delete(X509 *cert, const char *file, const char *desc)
 {
-    if (file == NULL || cert == NULL) {
+    if (file == NULL)
         return 1;
+    if (cert == NULL) {
+        char desc_cert[80];
+
+        snprintf(desc_cert, sizeof(desc_cert), "%s certificate", desc);
+        return delete_file(file, desc_cert);
     } else {
         STACK_OF(X509) *certs = sk_X509_new_null();
 
@@ -2053,7 +2057,7 @@ static int save_cert(OSSL_CMP_CTX *ctx, X509 *cert,
             sk_X509_free(certs);
             return 0;
         }
-        return save_free_certs(ctx, certs, file, desc) >= 0;
+        return save_free_certs(certs, file, desc) >= 0;
     }
 }
 
@@ -2858,13 +2862,6 @@ int cmp_main(int argc, char **argv)
         goto err;
 
     ret = 0;
-    if (!delete_certfile(opt_srvcertout, "validated server")
-        || !delete_certfile(opt_certout, "enrolled")
-        || save_free_certs(NULL, NULL, opt_extracertsout, "extra") < 0
-        || save_free_certs(NULL, NULL, opt_cacertsout, "CA") < 0
-        || save_free_certs(NULL, NULL, opt_chainout, "chain") < 0)
-        goto err;
-
     if (!app_RAND_load())
         goto err;
 
@@ -3011,28 +3008,28 @@ int cmp_main(int argc, char **argv)
         default:
             break;
         }
-        if (OSSL_CMP_CTX_get_status(cmp_ctx) < OSSL_CMP_PKISTATUS_accepted)
+        if (OSSL_CMP_CTX_get_status(cmp_ctx) < OSSL_CMP_PKISTATUS_accepted) {
+            ret = 0;
             goto err; /* we got no response, maybe even did not send request */
-
+        }
         print_status();
-        if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_extraCertsIn(cmp_ctx),
-                            opt_extracertsout, "extra") < 0)
+        if (!save_cert_or_delete(OSSL_CMP_CTX_get0_validatedSrvCert(cmp_ctx),
+                                 opt_srvcertout, "validated server"))
             ret = 0;
         if (!ret)
             goto err;
         ret = 0;
-        if (!save_cert(cmp_ctx, OSSL_CMP_CTX_get0_validatedSrvCert(cmp_ctx),
-                       opt_srvcertout, "validated server"))
-            goto err;
-        if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_caPubs(cmp_ctx),
-                            opt_cacertsout, "CA") < 0)
-            goto err;
-        if (!save_cert(cmp_ctx, newcert, opt_certout, "enrolled"))
-            goto err;
-        if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_newChain(cmp_ctx),
-                            opt_chainout, "chain") < 0)
+        if (save_free_certs(OSSL_CMP_CTX_get1_extraCertsIn(cmp_ctx),
+                            opt_extracertsout, "extra") < 0)
             goto err;
-
+        if (newcert != NULL && (opt_cmd == CMP_IR || opt_cmd == CMP_CR
+                                || opt_cmd == CMP_KUR || opt_cmd == CMP_P10CR))
+            if (!save_cert_or_delete(newcert, opt_certout, "newly enrolled")
+                || save_free_certs(OSSL_CMP_CTX_get1_newChain(cmp_ctx),
+                                   opt_chainout, "chain") < 0
+                || save_free_certs(OSSL_CMP_CTX_get1_caPubs(cmp_ctx),
+                                   opt_cacertsout, "CA") < 0)
+                goto err;
         if (!OSSL_CMP_CTX_reinit(cmp_ctx))
             goto err;
     }

diff --git a/doc/man1/openssl-cmp.pod.in b/doc/man1/openssl-cmp.pod.in
index a27af9f645dcff7b23dd95af04288ad69fc91329..f3bdb55e24bd35a8bb44a1378faa69dbd3a31ab0 100644 (file)
--- a/doc/man1/openssl-cmp.pod.in
+++ b/doc/man1/openssl-cmp.pod.in
@@ -268,7 +268,7 @@ L<openssl-passphrase-options(1)>.
 
 X509 Distinguished Name (DN) of subject to use in the requested certificate
 template.
-If the NULL-DN (C<"/">) is given then no subject is placed in the template.
+If the NULL-DN (C</>) is given then no subject is placed in the template.
 Default is the subject DN of any PKCS#10 CSR given with the B<-csr> option.
 For KUR, a further fallback is the subject DN
 of the reference certificate (see B<-oldcert>) if provided.
@@ -291,7 +291,7 @@ C</DC=org/DC=OpenSSL/DC=users/UID=123456+CN=John Doe>
 
 X509 issuer Distinguished Name (DN) of the CA server
 to place in the requested certificate template in IR/CR/KUR.
-If the NULL-DN (C<"/">) is given then no issuer is placed in the template.
+If the NULL-DN (C</>) is given then no issuer is placed in the template.
 
 If provided and neither B<-recipient> nor B<-srvcert> is given,
 the issuer DN is used as fallback recipient of outgoing CMP messages.
@@ -390,11 +390,11 @@ B<WARNING:> This leads to behavior violating RFC 4210.
 
 =item B<-certout> I<filename>
 
-The file where the newly enrolled certificate should be saved.
+The file where any newly enrolled certificate should be saved.
 
 =item B<-chainout> I<filename>
 
-The file where the chain of the newly enrolled certificate should be saved.
+The file where the chain of any newly enrolled certificate should be saved.
 
 =back
 
@@ -629,16 +629,18 @@ with a signature key."
 
 The file where to save the successfully validated certificate, if any,
 that the CMP server used for signature-based response message protection.
+If there is no such certificate, typically because the protection was MAC-based,
+this is indicated by deleting the file (if it existed).
 
 =item B<-extracertsout> I<filename>
 
-The file where to save all certificates contained in the extraCerts field
-of the last received response message (except for pollRep and PKIConf).
+The file where to save the list of certificates contained in the extraCerts
+field of the last received response message that is not a pollRep nor PKIConf.
 
 =item B<-cacertsout> I<filename>
 
-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.
+The file where to save the list of CA certificates contained in the caPubs field
+if a positive certificate response (i.e., IP, CP, or KUP) message was received.
 
 =back
 

diff --git a/test/recipes/80-test_cmp_http_data/test_enrollment.csv b/test/recipes/80-test_cmp_http_data/test_enrollment.csv
index 53bb162b9e9851bb98ab917e64c302c13c9616eb..2f21d3d424270270c05a344a9d07a5ed51cb1e82 100644 (file)
--- a/test/recipes/80-test_cmp_http_data/test_enrollment.csv
+++ b/test/recipes/80-test_cmp_http_data/test_enrollment.csv
@@ -108,3 +108,4 @@ TODO,p10cr wrong csr, -section,, -cmd,p10cr, -newkey,new.key,, -newkeypass,pass:
 0,kur wrong oldcert, -section,, -cmd,kur, -newkey,new.key,, -newkeypass,pass:,,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,_RESULT_DIR/test.certout_kur6.pem,, -out_trusted,root.crt,, -oldcert,root.crt,BLANK,,,,,-server,_SERVER_HOST:_KUR_PORT
 0,kur empty oldcert file, -section,, -cmd,kur, -newkey,new.key,, -newkeypass,pass:,,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,_RESULT_DIR/test.certout_kur7.pem,, -out_trusted,root.crt,, -oldcert,empty.txt,BLANK,,,,,-server,_SERVER_HOST:_KUR_PORT
 0,kur without cert and oldcert, -section,, -cmd,kur, -newkey,new.key,, -newkeypass,pass:,,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,_RESULT_DIR/test.certout_kur8.pem,, -out_trusted,root.crt,, -cert,"""",BLANK,,,,,-server,_SERVER_HOST:_KUR_PORT
+1,kur certout overwriting oldcert, -section,, -cmd,kur, -newkey,new.key,, -newkeypass,pass:,,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,_RESULT_DIR/test.certout_newkey.pem,, -out_trusted,root.crt,, -oldcert,_RESULT_DIR/test.certout_newkey.pem,BLANK,,,,-server,_SERVER_HOST:_KUR_PORT