From: Dr. David von Oheimb Date: Tue, 13 Dec 2022 16:47:23 +0000 (+0100) Subject: CMP app: fix file output of certs and cert lists on non-existing cert(s) X-Git-Tag: openssl-3.2.0-alpha1~1454 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=60c3d732b7b634290e4ec5d7ca6fb9b0a37592bf;p=thirdparty%2Fopenssl.git CMP app: fix file output of certs and cert lists on non-existing cert(s) Reviewed-by: Tomas Mraz Reviewed-by: Todd Short Reviewed-by: David von Oheimb (Merged from https://github.com/openssl/openssl/pull/20035) --- diff --git a/apps/cmp.c b/apps/cmp.c index bc446a4654c..e44d32fd7f0 100644 --- 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 a27af9f645d..f3bdb55e24b 100644 --- a/doc/man1/openssl-cmp.pod.in +++ b/doc/man1/openssl-cmp.pod.in @@ -268,7 +268,7 @@ L. 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 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 This leads to behavior violating RFC 4210. =item B<-certout> I -The file where the newly enrolled certificate should be saved. +The file where any newly enrolled certificate should be saved. =item B<-chainout> I -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 -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 -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 53bb162b9e9..2f21d3d4242 100644 --- 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