Browse code

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.

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>

Steffan Karger authored on 2017/06/19 18:28:39
Showing 5 changed files
... ...
@@ -324,6 +324,9 @@ User-visible Changes
324 324
 - ``--verify-hash`` can now take an optional flag which changes the hashing
325 325
   algorithm. It can be either SHA1 or SHA256.  The default if not provided is
326 326
   SHA1 to preserve backwards compatibility with existing configurations.
327
+- Restrict the supported --x509-alt-username extension fields to subjectAltName
328
+  and issuerAltName.  Other extensions probably didn't work anyway, and would
329
+  cause OpenVPN to crash when a client connects.
327 330
 
328 331
 Bugfixes
329 332
 --------
... ...
@@ -5307,6 +5307,8 @@ option will match against the chosen
5307 5307
 .B fieldname
5308 5308
 instead of the Common Name.
5309 5309
 
5310
+Only the subjectAltName and issuerAltName X.509 extensions are supported.
5311
+
5310 5312
 .B Please note:
5311 5313
 This option has a feature which will convert an all-lowercase
5312 5314
 .B fieldname
... ...
@@ -8083,6 +8083,10 @@ add_option(struct options *options,
8083 8083
                     "configuration", p[1]);
8084 8084
             }
8085 8085
         }
8086
+        else if (!x509_username_field_ext_supported(s+4))
8087
+        {
8088
+            msg(msglevel, "Unsupported x509-username-field extension: %s", s);
8089
+        }
8086 8090
         options->x509_username_field = p[1];
8087 8091
     }
8088 8092
 #endif /* ENABLE_X509ALTUSERNAME */
... ...
@@ -124,6 +124,14 @@ struct buffer x509_get_sha256_fingerprint(openvpn_x509_cert_t *cert,
124 124
 result_t backend_x509_get_username(char *common_name, int cn_len,
125 125
                                    char *x509_username_field, openvpn_x509_cert_t *peer_cert);
126 126
 
127
+#ifdef ENABLE_X509ALTUSERNAME
128
+/**
129
+ * Return true iff the supplied extension field is supported by the
130
+ * --x509-username-field option.
131
+ */
132
+bool x509_username_field_ext_supported(const char *extname);
133
+#endif
134
+
127 135
 /*
128 136
  * Return the certificate's serial number in decimal string representation.
129 137
  *
... ...
@@ -113,16 +113,29 @@ cleanup:
113 113
 }
114 114
 
115 115
 #ifdef ENABLE_X509ALTUSERNAME
116
+bool x509_username_field_ext_supported(const char *fieldname)
117
+{
118
+    int nid = OBJ_txt2nid(fieldname);
119
+    return nid == NID_subject_alt_name || nid == NID_issuer_alt_name;
120
+}
121
+
116 122
 static
117 123
 bool
118 124
 extract_x509_extension(X509 *cert, char *fieldname, char *out, int size)
119 125
 {
120 126
     bool retval = false;
121 127
     char *buf = 0;
122
-    GENERAL_NAMES *extensions;
123
-    int nid = OBJ_txt2nid(fieldname);
124 128
 
125
-    extensions = (GENERAL_NAMES *)X509_get_ext_d2i(cert, nid, NULL, NULL);
129
+    if (!x509_username_field_ext_supported(fieldname))
130
+    {
131
+        msg(D_TLS_ERRORS,
132
+            "ERROR: --x509-alt-username field 'ext:%s' not supported",
133
+            fieldname);
134
+        return false;
135
+    }
136
+
137
+    int nid = OBJ_txt2nid(fieldname);
138
+    GENERAL_NAMES *extensions = X509_get_ext_d2i(cert, nid, NULL, NULL);
126 139
     if (extensions)
127 140
     {
128 141
         int numalts;