From ab0907453c19ee14e1aec3f992ae27f768f95012 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Thu, 4 Jul 2024 09:42:00 +0200 Subject: [PATCH] extend X509_REQ_add_extensions_nid() and thuis APPS/req to support augmenting/overriding existing extensions Fixes #11169 --- crypto/x509/x509_req.c | 31 +++++++++++++++++++++++++--- doc/man1/openssl-req.pod.in | 6 +++++- doc/man3/X509_REQ_get_extensions.pod | 16 +++++++------- test/recipes/25-test_req.t | 7 +++++-- test/test.cnf | 3 +++ 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/crypto/x509/x509_req.c b/crypto/x509/x509_req.c index 801ceea9e3892..dec70ae991ee6 100644 --- a/crypto/x509/x509_req.c +++ b/crypto/x509/x509_req.c @@ -168,14 +168,39 @@ int X509_REQ_add_extensions_nid(X509_REQ *req, int extlen; int rv = 0; unsigned char *ext = NULL; + STACK_OF(X509_EXTENSION) *mod_exts = NULL; + int loc; + + if (sk_X509_EXTENSION_num(exts) <= 0) + return 1; /* adding NULL or empty list of exts is a no-op */ + + loc = X509at_get_attr_by_NID(req->req_info.attributes, nid, -1); + if (loc != -1) { + if ((mod_exts = get_extensions_by_nid(req, nid)) == NULL) + return 0; + if (X509v3_add_extensions(&mod_exts, exts) == NULL) + goto end; + } /* Generate encoding of extensions */ - extlen = ASN1_item_i2d((const ASN1_VALUE *)exts, &ext, - ASN1_ITEM_rptr(X509_EXTENSIONS)); + extlen = ASN1_item_i2d((const ASN1_VALUE *) + (mod_exts == NULL ? exts : mod_exts), + &ext, ASN1_ITEM_rptr(X509_EXTENSIONS)); if (extlen <= 0) - return 0; + goto end; + if (mod_exts != NULL) { + X509_ATTRIBUTE *att = X509at_delete_attr(req->req_info.attributes, loc); + + if (att == NULL) + goto end; + X509_ATTRIBUTE_free(att); + } + rv = X509_REQ_add1_attr_by_NID(req, nid, V_ASN1_SEQUENCE, ext, extlen); OPENSSL_free(ext); + + end: + sk_X509_EXTENSION_pop_free(mod_exts, X509_EXTENSION_free); return rv; } diff --git a/doc/man1/openssl-req.pod.in b/doc/man1/openssl-req.pod.in index 808801348fd59..0eacfc51a4c4f 100644 --- a/doc/man1/openssl-req.pod.in +++ b/doc/man1/openssl-req.pod.in @@ -392,7 +392,11 @@ Add a specific extension to the certificate (if B<-x509> is in use) or certificate request. The argument must have the form of a C pair as it would appear in a config file. +If an extension is added using this option that has the same OID as one +defined in the extension section of the config file, it overrides that one. + This option can be given multiple times. +Doing so, the same key most not be given more than once. =item B<-precert> @@ -552,7 +556,7 @@ BMPStrings and UTF8Strings. This specifies the configuration file section containing a list of extensions to add to the certificate request. It can be overridden -by the B<-reqexts> command line switch. See the +by the B<-reqexts> (or B<-extensions>) command line switch. See the L manual page for details of the extension section format. diff --git a/doc/man3/X509_REQ_get_extensions.pod b/doc/man3/X509_REQ_get_extensions.pod index 73e2ea698a7b0..bd50faa63d155 100644 --- a/doc/man3/X509_REQ_get_extensions.pod +++ b/doc/man3/X509_REQ_get_extensions.pod @@ -22,13 +22,15 @@ found in the attributes of I. The returned list is empty if there are no such extensions in I. The caller is responsible for freeing the list obtained. -X509_REQ_add_extensions() adds to I a list of X.509 extensions I, -which must not be NULL, using the default B. -This function must not be called more than once on the same I. - -X509_REQ_add_extensions_nid() is like X509_REQ_add_extensions() -except that I is used to identify the extensions attribute. -This function must not be called more than once with the same I and I. +X509_REQ_add_extensions_nid() adds to I a list of X.509 extensions I, +using I to identify the extensions attribute. +I is unchanged if I is NULL or an empty list. +This function may be called more than once on the same I and I. +In such case any previous extensions are augmented, where an extension to be +added that has the same OID as a pre-existing one replaces this earlier one. + +X509_REQ_add_extensions() is like X509_REQ_add_extensions_nid() +except that the default B is used. =head1 RETURN VALUES diff --git a/test/recipes/25-test_req.t b/test/recipes/25-test_req.t index 872ed316fc73c..a0f1efdab1e0d 100644 --- a/test/recipes/25-test_req.t +++ b/test/recipes/25-test_req.t @@ -15,7 +15,7 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/; setup("test_req"); -plan tests => 109; +plan tests => 110; require_ok(srctop_file('test', 'recipes', 'tconversion.pl')); @@ -36,7 +36,7 @@ if (disabled("rsa")) { $ENV{MSYS2_ARG_CONV_EXCL} = "/CN="; # Check for duplicate -addext parameters, and one "working" case. -my @addext_args = ( "openssl", "req", "-new", "-out", "testreq.pem", +my @addext_args = ( "openssl", "req", "-new", "-out", "testreq-addexts.pem", "-key", srctop_file(@certs, "ee-key.pem"), "-config", srctop_file("test", "test.cnf"), @req_new ); my $val = "subjectAltName=DNS:example.com"; @@ -55,6 +55,9 @@ ok(!run(app([@addext_args, "-addext", $val2, "-addext", $val3]))); ok(run(app([@addext_args, "-addext", "SXNetID=1:one, 2:two, 3:three"]))); ok(run(app([@addext_args, "-addext", "subjectAltName=dirName:dirname_sec"]))); +ok(run(app([@addext_args, "-addext", "keyUsage=digitalSignature", + "-reqexts", "reqexts"]))); # referring to section in test.cnf + # If a CSR is provided with neither of -key or -CA/-CAkey, this should fail. ok(!run(app(["openssl", "req", "-x509", "-in", srctop_file(@certs, "x509-check.csr"), diff --git a/test/test.cnf b/test/test.cnf index 8f68982a9fa1f..e0135ca206594 100644 --- a/test/test.cnf +++ b/test/test.cnf @@ -78,3 +78,6 @@ C = UK O = My Organization OU = My Unit CN = My Name + +[ reqexts ] +keyUsage = critical,digitalSignature,keyEncipherment