Hello plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1442?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+2 by plaisthos
Change subject: remove ENABLE_X509ALTUSERNAME conditional
......................................................................
remove ENABLE_X509ALTUSERNAME conditional
This is one of the #ifdef producing compile-time variants that make the
code harder to read and harder to test. The extra code size due to
turning it on is marginal.
The mbedTLS backend does not (yet) support it. To cope with that,
add a minimum function x509_username_field_ext_supported() that always
returns "false", and omit the --x509-username-field from the help
text if ENABLE_CRYPTO_MBEDTLS. Implement this on another day.
Github: closes OpenVPN/openvpn#917
Change-Id: I3f661cf305c52652e430b8d219df5186dd8ea4f7
Signed-off-by: Gert Doering <[email protected]>
Acked-by: Arne Schwabe <[email protected]>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1442
Message-Id: <[email protected]>
URL:
https://www.mail-archive.com/[email protected]/msg35237.html
Signed-off-by: Gert Doering <[email protected]>
---
M CMakeLists.txt
M Changes.rst
M config.h.cmake.in
M configure.ac
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/ssl_common.h
M src/openvpn/ssl_verify_backend.h
M src/openvpn/ssl_verify_mbedtls.c
M src/openvpn/ssl_verify_openssl.c
11 files changed, 10 insertions(+), 50 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/42/1442/2
diff --git a/CMakeLists.txt b/CMakeLists.txt
index bdad173..e25ecae 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -325,8 +325,6 @@
target_link_libraries(${target} PUBLIC ${wolfssl_LINK_LIBRARIES})
target_include_directories(${target} PRIVATE
${wolfssl_INCLUDE_DIRS}/wolfssl)
else ()
- set(ENABLE_X509ALTUSERNAME YES)
-
find_package(OpenSSL REQUIRED)
target_link_libraries(${target} PUBLIC OpenSSL::SSL OpenSSL::Crypto)
if (WIN32)
@@ -365,10 +363,8 @@
elseif (${WOLFSSL})
set(ENABLE_CRYPTO_OPENSSL YES)
set(ENABLE_CRYPTO_WOLFSSL YES)
- set(ENABLE_X509ALTUSERNAME YES)
else ()
set(ENABLE_CRYPTO_OPENSSL YES)
- set(ENABLE_X509ALTUSERNAME YES)
endif ()
include_directories(${CMAKE_CURRENT_SOURCE_DIR} src/compat include)
diff --git a/Changes.rst b/Changes.rst
index c8a3058..18d601f 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -346,6 +346,9 @@
- The ``test-crypto`` option no longer requires a ``--secret`` argument and
will automatically generate a random key.
+- The configure-time option ``--enable-x509-alt-username`` is no longer
+ conditional, and always-on (GH: OpenVPN/openvpn#917).
+
Deprecated features
-------------------
diff --git a/config.h.cmake.in b/config.h.cmake.in
index 53976a7..ddf6174 100644
--- a/config.h.cmake.in
+++ b/config.h.cmake.in
@@ -62,9 +62,6 @@
/* Enable systemd integration */
/* #undef ENABLE_SYSTEMD */
-/* Enable --x509-username-field feature */
-#cmakedefine ENABLE_X509ALTUSERNAME
-
/* Define to 1 if you have the <arpa/inet.h> header file. */
#cmakedefine HAVE_ARPA_INET_H 1
diff --git a/configure.ac b/configure.ac
index 63d4d6e..2d8de7c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -88,13 +88,6 @@
)
AC_ARG_ENABLE(
- [x509-alt-username],
- [AS_HELP_STRING([--enable-x509-alt-username], [enable the
--x509-username-field feature @<:@default=no@:>@])],
- ,
- [enable_x509_alt_username="no"]
-)
-
-AC_ARG_ENABLE(
[dns-updown-by-default],
[AS_HELP_STRING([--disable-dns-updown-by-default], [disable running
--dns-updown by default @<:@default=yes@:>@])],
,
@@ -1172,15 +1165,6 @@
fi
AC_MSG_RESULT([${GIT_CHECKOUT}])
-dnl enable --x509-username-field feature if requested
-if test "${enable_x509_alt_username}" = "yes"; then
- if test "${with_crypto_library}" = "mbedtls" ; then
- AC_MSG_ERROR([mbed TLS does not support the
--x509-username-field feature])
- fi
-
- AC_DEFINE([ENABLE_X509ALTUSERNAME], [1], [Enable --x509-username-field
feature])
-fi
-
test "${enable_management}" = "yes" && AC_DEFINE([ENABLE_MANAGEMENT], [1],
[Enable management server capability])
test "${enable_debug}" = "yes" && AC_DEFINE([ENABLE_DEBUG], [1], [Enable
debugging support])
test "${enable_small}" = "yes" && AC_DEFINE([ENABLE_SMALL], [1], [Enable
smaller executable size])
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 4c23170..70c0b5d 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3358,11 +3358,7 @@
to.verify_hash_algo = options->verify_hash_algo;
to.verify_hash_depth = options->verify_hash_depth;
to.verify_hash_no_ca = options->verify_hash_no_ca;
-#ifdef ENABLE_X509ALTUSERNAME
memcpy(to.x509_username_field, options->x509_username_field,
sizeof(to.x509_username_field));
-#else
- to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
-#endif
to.es = c->c2.es;
to.net_ctx = &c->net_ctx;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3d86fc4..cede758 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -595,8 +595,6 @@
#ifndef ENABLE_CRYPTO_MBEDTLS
"--pkcs12 file : PKCS#12 file containing local private key, local
certificate\n"
" and optionally the root CA certificate.\n"
-#endif
-#ifdef ENABLE_X509ALTUSERNAME
"--x509-username-field : Field in x509 certificate containing the
username.\n"
" Default is CN in the Subject field.\n"
#endif
@@ -885,9 +883,7 @@
o->transition_window = 3600;
o->tls_cert_profile = NULL;
o->ecdh_curve = NULL;
-#ifdef ENABLE_X509ALTUSERNAME
o->x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
-#endif
#ifdef ENABLE_PKCS11
o->pkcs11_pin_cache_period = -1;
#endif /* ENABLE_PKCS11 */
@@ -9069,7 +9065,6 @@
VERIFY_PERMISSION(OPT_P_GENERAL);
x509_track_add(&options->x509_track, p[1], msglevel, &options->gc);
}
-#ifdef ENABLE_X509ALTUSERNAME
else if (streq(p[0], "x509-username-field") && p[1])
{
VERIFY_PERMISSION(OPT_P_GENERAL);
@@ -9084,7 +9079,6 @@
options->x509_username_field[j - 1] = p[j];
}
}
-#endif /* ENABLE_X509ALTUSERNAME */
#ifdef ENABLE_PKCS11
else if (streq(p[0], "show-pkcs11-ids") && !p[3])
{
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 9d89426..16cfdb5 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -651,10 +651,8 @@
* within n seconds of handshake initiation. */
int handshake_window;
-#ifdef ENABLE_X509ALTUSERNAME
/* Field list used to be the username in X509 cert. */
char *x509_username_field[MAX_PARMS];
-#endif
/* Old key allowed to live n seconds after new key goes active */
int transition_window;
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 1cf254f..fba01bb 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -362,11 +362,7 @@
int verify_hash_depth;
bool verify_hash_no_ca;
hash_algo_type verify_hash_algo;
-#ifdef ENABLE_X509ALTUSERNAME
char *x509_username_field[MAX_PARMS];
-#else
- char *x509_username_field[2];
-#endif
/* struct crypto_option flags */
unsigned int crypto_flags;
diff --git a/src/openvpn/ssl_verify_backend.h b/src/openvpn/ssl_verify_backend.h
index 9cad320..0a2de03 100644
--- a/src/openvpn/ssl_verify_backend.h
+++ b/src/openvpn/ssl_verify_backend.h
@@ -113,7 +113,7 @@
/*
* Retrieve the certificate's username from the specified field.
*
- * If the field is prepended with ext: and ENABLE_X509ALTUSERNAME is enabled,
+ * If the field is prepended with ext: is enabled,
* it will be loaded from an X.509 extension
*
* @param cn Buffer to return the common name in.
@@ -126,15 +126,12 @@
result_t backend_x509_get_username(char *common_name, size_t 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_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index 74b21b0..80396f8 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -123,9 +123,12 @@
return 0;
}
-#ifdef ENABLE_X509ALTUSERNAME
-#warning "X509 alt user name not yet supported for mbed TLS"
-#endif
+/* not supported for mbedTLS yet */
+bool
+x509_username_field_ext_supported(const char *fieldname)
+{
+ return false;
+}
result_t
backend_x509_get_username(char *cn, size_t cn_len, char *x509_username_field,
mbedtls_x509_crt *cert)
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index 32e55e6..4f6573d 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -111,7 +111,6 @@
return ret;
}
-#ifdef ENABLE_X509ALTUSERNAME
bool
x509_username_field_ext_supported(const char *fieldname)
{
@@ -180,7 +179,6 @@
}
return retval;
}
-#endif /* ENABLE_X509ALTUSERNAME */
/*
* Extract a field from an X509 subject name.
@@ -252,7 +250,6 @@
result_t
backend_x509_get_username(char *common_name, size_t cn_len, char
*x509_username_field, X509 *peer_cert)
{
-#ifdef ENABLE_X509ALTUSERNAME
if (strncmp("ext:", x509_username_field, 4) == 0)
{
if (!extract_x509_extension(peer_cert, x509_username_field + 4,
common_name, cn_len))
@@ -275,7 +272,6 @@
gc_free(&gc);
}
else
-#endif /* ifdef ENABLE_X509ALTUSERNAME */
{
X509_NAME *x509_subject_name = X509_get_subject_name(peer_cert);
if (x509_subject_name == NULL)
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1442?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3f661cf305c52652e430b8d219df5186dd8ea4f7
Gerrit-Change-Number: 1442
Gerrit-PatchSet: 2
Gerrit-Owner: cron2 <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
