Reject duplicate -addext parameters

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/6636)

diff --git a/apps/req.c b/apps/req.c
index 7e7b994f0d4f1ad2d38eec491e70f9084e743acd..3d89f2000c7c59489eac88bba76ac958df352184 100644 (file)
--- a/apps/req.c
+++ b/apps/req.c
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <time.h>
 #include <string.h>
+#include <ctype.h>
 #include "apps.h"
 #include "progs.h"
 #include <openssl/bio.h>
@@ -23,6 +24,8 @@
 #include <openssl/objects.h>
 #include <openssl/pem.h>
 #include <openssl/bn.h>
+#include <openssl/bn.h>
+#include <openssl/lhash.h>
 #ifndef OPENSSL_NO_RSA
 # include <openssl/rsa.h>
 #endif
@@ -147,6 +150,68 @@ const OPTIONS req_options[] = {
     {NULL}
 };
 
+
+/*
+ * An LHASH of strings, where each string is an extension name.
+ */
+static unsigned long ext_name_hash(const OPENSSL_STRING *a)
+{
+    return OPENSSL_LH_strhash((const char *)a);
+}
+
+static int ext_name_cmp(const OPENSSL_STRING *a, const OPENSSL_STRING *b)
+{
+    return strcmp((const char *)a, (const char *)b);
+}
+
+static void exts_cleanup(OPENSSL_STRING *x)
+{
+    OPENSSL_free((char *)x);
+}
+
+/*
+ * Is the |kv| key already duplicated?  This is remarkably tricky to get
+ * right.  Return 0 if unique, -1 on runtime error; 1 if found or a syntax
+ * error.
+ */
+static int duplicated(LHASH_OF(OPENSSL_STRING) *addexts, char *kv)
+{
+    char *p;
+
+    /* Check syntax. */
+    if (strchr(kv, '=') == NULL)
+        return 1;
+
+    /* Skip leading whitespace, make a copy. */
+    while (*kv && isspace(*kv))
+        if (*++kv == '\0')
+            return 1;
+    if ((kv = OPENSSL_strdup(kv)) == NULL)
+        return -1;
+
+    /* Skip trailing space before the equal sign. */
+    for (p = strchr(kv, '='); p > kv; --p)
+        if (p[-1] != ' ' && p[-1] != '\t')
+            break;
+    if (p == kv) {
+        OPENSSL_free(kv);
+        return 1;
+    }
+    *p = '\0';
+
+    /* Finally have a clean "key"; see if it's there. */
+    if (lh_OPENSSL_STRING_retrieve(addexts, (OPENSSL_STRING*)kv) != NULL) {
+        BIO_printf(bio_err, "Extension \"%s\" repeated\n", kv);
+        OPENSSL_free(kv);
+        return 1;
+    }
+
+    /* Not found; add it. */
+    if (lh_OPENSSL_STRING_insert(addexts, (OPENSSL_STRING*)kv) == NULL)
+        return -1;
+    return 0;
+}
+
 int req_main(int argc, char **argv)
 {
     ASN1_INTEGER *serial = NULL;
@@ -155,6 +220,7 @@ int req_main(int argc, char **argv)
     EVP_PKEY *pkey = NULL;
     EVP_PKEY_CTX *genctx = NULL;
     STACK_OF(OPENSSL_STRING) *pkeyopts = NULL, *sigopts = NULL;
+    LHASH_OF(OPENSSL_STRING) *addexts = NULL;
     X509 *x509ss = NULL;
     X509_REQ *req = NULL;
     const EVP_CIPHER *cipher = NULL;
@@ -324,11 +390,17 @@ int req_main(int argc, char **argv)
             multirdn = 1;
             break;
         case OPT_ADDEXT:
-            if (addext_bio == NULL) {
+            p = opt_arg();
+            if (addexts == NULL) {
+                addexts = lh_OPENSSL_STRING_new(ext_name_hash, ext_name_cmp);
                 addext_bio = BIO_new(BIO_s_mem());
+                if (addexts == NULL || addext_bio == NULL)
+                    goto end;
             }
-            if (addext_bio == NULL
-                || BIO_printf(addext_bio, "%s\n", opt_arg()) < 0)
+            i = duplicated(addexts, p);
+            if (i == 1)
+                goto opthelp;
+            if (i < 0 || BIO_printf(addext_bio, "%s\n", opt_arg()) < 0)
                 goto end;
             break;
         case OPT_EXTENSIONS:
@@ -885,6 +957,8 @@ int req_main(int argc, char **argv)
     EVP_PKEY_CTX_free(genctx);
     sk_OPENSSL_STRING_free(pkeyopts);
     sk_OPENSSL_STRING_free(sigopts);
+    lh_OPENSSL_STRING_doall(addexts, exts_cleanup);
+    lh_OPENSSL_STRING_free(addexts);
 #ifndef OPENSSL_NO_ENGINE
     ENGINE_free(gen_eng);
 #endif

diff --git a/test/recipes/25-test_req.t b/test/recipes/25-test_req.t
index 82b9bf8aecbd8a49ba5ae1ac396a347b8f42c76a..fa79219e1d10112e7fb26708418e3c71dd93e432 100644 (file)
--- a/test/recipes/25-test_req.t
+++ b/test/recipes/25-test_req.t
@@ -15,13 +15,24 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/;
 
 setup("test_req");
 
-plan tests => 4;
+plan tests => 8;
 
 require_ok(srctop_file('test','recipes','tconversion.pl'));
 
 open RND, ">>", ".rnd";
 print RND "string to make the random number generator think it has randomness";
 close RND;
+
+# Check for duplicate -addext parameters
+my $val = "subjectAltName=DNS:example.com";
+my $val2 = " " . $val;
+my $val3 = $val;
+$val3 =~ s/=/    =/;
+ok(!run(app(["openssl", "req", "-new", "-addext", $val, "-addext", $val])));
+ok(!run(app(["openssl", "req", "-new", "-addext", $val, "-addext", $val2])));
+ok(!run(app(["openssl", "req", "-new", "-addext", $val, "-addext", $val3])));
+ok(!run(app(["openssl", "req", "-new", "-addext", $val2, "-addext", $val3])));
+
 subtest "generating certificate requests" => sub {
     my @req_new;
     if (disabled("rsa")) {