From: Steffan Karger Date: Mon, 19 Jun 2017 09:28:39 +0000 (+0200) Subject: Restrict --x509-alt-username extension types X-Git-Tag: v2.5_beta1~657 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d2a19185fd78030ce4a1bba6c9f83e0dac9e15a6;p=thirdparty%2Fopenvpn.git 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 Acked-by: Gert Doering Acked-by: David Sommerseth Acked-by: Guido Vranken 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 --- diff --git a/Changes.rst b/Changes.rst index 89cfae87d..6fa1c0c66 100644 --- a/Changes.rst +++ b/Changes.rst @@ -324,6 +324,9 @@ User-visible Changes - ``--verify-hash`` can now take an optional flag which changes the hashing algorithm. It can be either SHA1 or SHA256. The default if not provided is SHA1 to preserve backwards compatibility with existing configurations. +- Restrict the supported --x509-alt-username extension fields to subjectAltName + and issuerAltName. Other extensions probably didn't work anyway, and would + cause OpenVPN to crash when a client connects. Bugfixes -------- diff --git a/doc/openvpn.8 b/doc/openvpn.8 index 3f183e642..20bdd91b3 100644 --- a/doc/openvpn.8 +++ b/doc/openvpn.8 @@ -5307,6 +5307,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 76a855066..505c5b2e3 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8083,6 +8083,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 978e54fdf..e8eaabe9e 100644 --- a/src/openvpn/ssl_verify_backend.h +++ b/src/openvpn/ssl_verify_backend.h @@ -124,6 +124,14 @@ struct buffer x509_get_sha256_fingerprint(openvpn_x509_cert_t *cert, 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 7c1a481c4..08451f29a 100644 --- a/src/openvpn/ssl_verify_openssl.c +++ b/src/openvpn/ssl_verify_openssl.c @@ -113,16 +113,29 @@ 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); - extensions = (GENERAL_NAMES *)X509_get_ext_d2i(cert, nid, NULL, NULL); + if (!x509_username_field_ext_supported(fieldname)) + { + msg(D_TLS_ERRORS, + "ERROR: --x509-alt-username field 'ext:%s' not supported", + fieldname); + return false; + } + + int nid = OBJ_txt2nid(fieldname); + GENERAL_NAMES *extensions = X509_get_ext_d2i(cert, nid, NULL, NULL); if (extensions) { int numalts;