extend X509_REQ_add_extensions_nid() and this APPS/req to support aug…

…menting/overriding existing extensions

Fixes openssl#11169

crypto/x509/x509_req.c

@@ -168,14 +168,35 @@ 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
+            && X509at_delete_attr(req->req_info.attributes, loc) == NULL)
+        goto end;
+
     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;
 }
 

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<key=value> 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<x509v3_config(5)> manual page for details of the
 extension section format.
 

doc/man3/X509_REQ_get_extensions.pod

@@ -22,13 +22,15 @@ found in the attributes of I<req>.
 The returned list is empty if there are no such extensions in I<req>.
 The caller is responsible for freeing the list obtained.
 
-X509_REQ_add_extensions() adds to I<req> a list of X.509 extensions I<exts>,
-which must not be NULL, using the default B<NID_ext_req>.
-This function must not be called more than once on the same I<req>.
-
-X509_REQ_add_extensions_nid() is like X509_REQ_add_extensions()
-except that I<nid> is used to identify the extensions attribute.
-This function must not be called more than once with the same I<req> and I<nid>.
+X509_REQ_add_extensions_nid() adds to I<req> a list of X.509 extensions I<exts>,
+using I<nid> to identify the extensions attribute.
+I<req> is unchanged if I<exts> is NULL or an empty list.
+This function may be called more than once on the same I<req> and I<nid>.
+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<NID_ext_req> is used.
 
 =head1 RETURN VALUES
 

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"),

test/test.cnf

@@ -78,3 +78,6 @@ C  = UK
 O  = My Organization
 OU = My Unit
 CN = My Name
+
+[ reqexts ]
+keyUsage = critical,digitalSignature,keyEncipherment