Attention is currently required from: flichtenheld.
Hello flichtenheld,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/913?usp=email
to look at the new patch set (#3).
Change subject: Use USER_PASS_LEN instead of TLS_USERNAME_LEN for
override-username
......................................................................
Use USER_PASS_LEN instead of TLS_USERNAME_LEN for override-username
Currently override-username is artificially restricted to the length of
TLS common-name (64) for the corner case of using username-as-common-name,
which we explicitly do not recommend to use.
Do away with that limitation and only error out on longer usernames when
username-as-common-name is actually in effect.
Change-Id: I1c2c050dd160746a0f8d9c234abe1e258bc8e48d
Signed-off-by: Arne Schwabe <a...@rfc2549.org>
---
M src/openvpn/multi.c
M src/openvpn/options.c
M src/openvpn/ssl_verify.c
M src/openvpn/ssl_verify.h
4 files changed, 42 insertions(+), 7 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/13/913/3
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a673ec1..a2d3fd1 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2705,6 +2705,12 @@
if (!multi->locked_original_username
&& strcmp(multi->locked_username, options->override_username) != 0)
{
+ /* Check if the username length is acceptable */
+ if (!ssl_verify_username_length(session, options->override_username))
+ {
+ return false;
+ }
+
multi->locked_original_username = multi->locked_username;
multi->locked_username = strdup(options->override_username);
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index ab56609..f89fc7d 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7875,10 +7875,10 @@
else if (streq(p[0], "override-username") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_INSTANCE);
- if (strlen(p[1]) > TLS_USERNAME_LEN)
+ if (strlen(p[1]) > USER_PASS_LEN)
{
msg(msglevel, "override-username exceeds the maximum length of %d "
- "characters", TLS_USERNAME_LEN);
+ "characters", USER_PASS_LEN);
/* disable the connection since ignoring the request to
* set another username might cause serious problems */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 5f8f1d3..d2cc3d1 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1568,6 +1568,24 @@
}
}
+bool
+ssl_verify_username_length(struct tls_session *session, const char *username)
+{
+ if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME)
+ && strlen(username) > TLS_USERNAME_LEN)
+ {
+ msg(D_TLS_ERRORS,
+ "TLS Auth Error: --username-as-common name specified and "
+ "username is longer than the maximum permitted Common Name "
+ "length of %d characters", TLS_USERNAME_LEN);
+ return false;
+ }
+ else
+ {
+ return true;
+ }
+}
+
/**
* Main username/password verification entry point
*
@@ -1689,15 +1707,12 @@
}
/* check sizing of username if it will become our common name */
- if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME)
- && strlen(up->username)>TLS_USERNAME_LEN)
+ if (!ssl_verify_username_length(session, up->username))
{
- msg(D_TLS_ERRORS,
- "TLS Auth Error: --username-as-common name specified and username
is longer than the maximum permitted Common Name length of %d characters",
- TLS_USERNAME_LEN);
plugin_status = OPENVPN_PLUGIN_FUNC_ERROR;
script_status = OPENVPN_PLUGIN_FUNC_ERROR;
}
+
/* auth succeeded? */
bool plugin_ok = plugin_status == OPENVPN_PLUGIN_FUNC_SUCCESS
|| plugin_status == OPENVPN_PLUGIN_FUNC_DEFERRED;
diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h
index eba3832..7a4d44a 100644
--- a/src/openvpn/ssl_verify.h
+++ b/src/openvpn/ssl_verify.h
@@ -192,6 +192,20 @@
struct tls_session *session);
+/**
+ * Checks if the username length is valid to use. This checks when
+ * username-as-common-name is active if the username is shorter than
+ * the maximum TLS common name length (64).
+ *
+ * It will also display an error message if the name is too long
+ *
+ * @param session current TLS session
+ * @param username username to check
+ * @return true if name is under limit or username-as-common-name
+ * is not active
+ */
+bool ssl_verify_username_length(struct tls_session *session,
+ const char *username);
/**
* Runs the --client-crresponse script if one is defined.
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/913?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I1c2c050dd160746a0f8d9c234abe1e258bc8e48d
Gerrit-Change-Number: 913
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
