Restrict --x509-alt-username extension types

The code never supported all extension types.  Make this explicit by only
allowing subjectAltName and issuerAltName (for which the current code does
work).

Using unsupported extension fields would most likely cause OpenVPN to crash
as soon as a client connects.  This does not have a real-world security
impact, as such a configuration would not be possible to use in practice.

This bug was discovered, analysed and reported to the OpenVPN team by
Guido Vranken.

Note: small code changes in to avoid "variable declaration in the middle
of a code block" when backporting changes to extract_x509_extension(),
as 2.3 can not assume a C99 compiler.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Acked-by: David Sommerseth <davids@openvpn.net>
Acked-by: Guido Vranken <guidovranken@gmail.com>
Message-Id: <1497864520-12219-5-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/search?l=mid&q=1497864520-12219-5-git-send-email-steffan.karger@fox-it.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit d2a19185fd78030ce4a1bba6c9f83e0dac9e15a6)

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 284e8e628dc35764908c432b83a2d9e1a16be5c5..109afe66717bf3f5ebc396e47f6f2f97159a6e62 100644 (file)
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -4981,6 +4981,8 @@ option will match against the chosen
 .B fieldname
 instead of the Common Name.
 
+Only the subjectAltName and issuerAltName X.509 extensions are supported.
+
 .B Please note:
 This option has a feature which will convert an all-lowercase
 .B fieldname

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index aff77c496a695439a6a15346a79c20ec4ef3cafd..6faa280825bc3291c0c586529b09f1ecbf9835e1 100644 (file)
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7023,6 +7023,10 @@ add_option (struct options *options,
                  "configuration", p[1]);
            }
        }
+      else if (!x509_username_field_ext_supported(s+4))
+        {
+          msg(msglevel, "Unsupported x509-username-field extension: %s", s);
+        }
       options->x509_username_field = p[1];
     }
 #endif /* ENABLE_X509ALTUSERNAME */

diff --git a/src/openvpn/ssl_verify_backend.h b/src/openvpn/ssl_verify_backend.h
index 488983f4d518889c8588b0b62ddf9fb5985abb69..1c8d43c4149cca420edf367da5214457e57c896e 100644 (file)
--- a/src/openvpn/ssl_verify_backend.h
+++ b/src/openvpn/ssl_verify_backend.h
@@ -112,6 +112,14 @@ unsigned char *x509_get_sha1_hash (openvpn_x509_cert_t *cert, struct gc_arena *g
 result_t backend_x509_get_username (char *common_name, int cn_len,
     char * x509_username_field, openvpn_x509_cert_t *peer_cert);
 
+#ifdef ENABLE_X509ALTUSERNAME
+/**
+ * Return true iff the supplied extension field is supported by the
+ * --x509-username-field option.
+ */
+bool x509_username_field_ext_supported(const char *extname);
+#endif
+
 /*
  * Return the certificate's serial number in decimal string representation.
  *

diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index 11eb7be51062fc48452861c31a95413b34fc3d99..ba18e683c4541f985bdb69befac99fd129e7e29d 100644 (file)
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -93,14 +93,30 @@ cleanup:
 }
 
 #ifdef ENABLE_X509ALTUSERNAME
+bool
+x509_username_field_ext_supported(const char *fieldname)
+{
+  int nid = OBJ_txt2nid(fieldname);
+  return nid == NID_subject_alt_name || nid == NID_issuer_alt_name;
+}
+
 static
 bool extract_x509_extension(X509 *cert, char *fieldname, char *out, int size)
 {
   bool retval = false;
   char *buf = 0;
   GENERAL_NAMES *extensions;
-  int nid = OBJ_txt2nid(fieldname);
+  int nid;
+
+  if (!x509_username_field_ext_supported(fieldname))
+    {
+      msg(D_TLS_ERRORS,
+          "ERROR: --x509-alt-username field 'ext:%s' not supported",
+          fieldname);
+      return false;
+    }
 
+  nid = OBJ_txt2nid(fieldname);
   extensions = (GENERAL_NAMES *)X509_get_ext_d2i(cert, nid, NULL, NULL);
   if ( extensions )
     {