Attention is currently required from: flichtenheld.
Hello flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/872?usp=email
to review the following change.
Change subject: Implement override-user
......................................................................
Implement override-user
Change-Id: Ia4095518d5e4447992a2974e0d7a159d79ba6b6f
Signed-off-by: Arne Schwabe <a...@rfc2549.org>
---
M Changes.rst
M doc/man-sections/server-options.rst
M src/openvpn/auth_token.c
M src/openvpn/multi.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/push.c
M src/openvpn/ssl.c
M src/openvpn/ssl_common.h
M src/openvpn/ssl_verify.c
M src/openvpn/ssl_verify.h
11 files changed, 126 insertions(+), 6 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/72/872/1
diff --git a/Changes.rst b/Changes.rst
index 16ae6fc..d7def7c 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -36,6 +36,10 @@
add an allowed cipher without having to spell out the default ciphers.
+Allow overriding username with ``--override-username``
+ This is intended to allow using auth-gen-token in scenarios where the
+ client use certificates and multi-factor authentication.
+
Deprecated features
-------------------
``secret`` support has been removed by default.
diff --git a/doc/man-sections/server-options.rst
b/doc/man-sections/server-options.rst
index 3fe9862..2b2c262 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -670,6 +670,31 @@
AND the ``--auth-user-pass-verify`` script will need to succeed in order
for a client to be authenticated and accepted onto the VPN.
+--override-username
+ Sets the username of a connection the specified username. This username
+ will also be used by the by ``--auth-gen-token``. However, the overridden
+ username comes only into effect *after* the ``--client-connect``,
+ ``--client-config-dir`` and the ``--auth-user-pass-verify`` script have
+ been run.
+
+ Also ``username-as-common-name`` will use the client provided username
+ as common-name. It is recommended to avoid the use of the
+ ``--override-username`` option if the option ``username-as-common-name``
+ is being used.
+
+ The changed username will be picked up by the status output and also by
+ the the ``--auth-gen-token`` option. It will also be pushed to the client
+ using ``--auth-token-user``.
+
+ Special care has to be taken that the initial username of the client is
+ correctly handled with these options to avoid authentication/authorisation
+ bypasses.
+
+ This option is mainly intended for use cases that use certificates and
+ multi factor authentication and therefore do not provide a username that
+ can be used for ``--auth-gen-token`` to allow providing a username in
+ these scenarios.
+
--vlan-tagging
Server-only option. Turns the OpenVPN server instance into a switch that
understands VLAN-tagging, based on IEEE 802.1Q.
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index 3cf55e8..312474c 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -330,10 +330,19 @@
timestamp_initial = ntohll(timestamp_initial);
hmac_ctx_t *ctx = multi->opt.auth_token_key.hmac;
+
if (check_hmac_token(ctx, b64decoded, up->username))
{
ret |= AUTH_TOKEN_HMAC_OK;
}
+ else if (multi->locked_original_username && multi->locked_username
+ && check_hmac_token(ctx, b64decoded, multi->locked_username))
+ {
+ /* if the username has been overridden, check with the overridden
+ * username and ignore the sent username in case the client does not
+ * support auth-token-user */
+ ret |= AUTH_TOKEN_HMAC_OK;
+ }
else if (check_hmac_token(ctx, b64decoded, ""))
{
ret |= AUTH_TOKEN_HMAC_OK;
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f426b46..03e6b6e 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -42,7 +42,9 @@
#include "ssl_verify.h"
#include "ssl_ncp.h"
#include "vlan.h"
+#include "auth_token.h"
#include <inttypes.h>
+#include <string.h>
#include "memdbg.h"
@@ -2663,6 +2665,41 @@
NULL,
};
+/**
+ *
+ * @param mi
+ */
+static void
+override_locked_username(struct multi_instance *mi)
+{
+ struct tls_multi *multi = mi->context.c2.tls_multi;
+
+ struct options *options = &mi->context.options;
+
+ if (!multi->locked_original_username
+ && strcmp(multi->locked_username, options->override_username) != 0)
+ {
+ multi->locked_original_username = multi->locked_username;
+ multi->locked_username = strdup(options->override_username);
+
+ /* Regenerate the auth-token if enabled */
+ if (multi->auth_token_initial)
+ {
+ struct user_pass up;
+ CLEAR(up);
+ strncpynt(up.username, multi->locked_username,
+ sizeof(up.username));
+
+ generate_auth_token(&up, multi);
+ }
+
+ msg(D_MULTI_LOW, "MULTI: Note, override-username changes username "
+ "from '%s' to '%s'",
+ multi->locked_original_username,
+ multi->locked_username);
+
+ }
+}
/*
* Called as soon as the SSL/TLS connection is authenticated.
*
@@ -2766,6 +2803,11 @@
(*cur_handler_index)++;
}
+ if (mi->context.options.override_username)
+ {
+ override_locked_username(mi);
+ }
+
/* Check if we have forbidding options in the current mode */
if (dco_enabled(&mi->context.options)
&& !dco_check_option(D_MULTI_ERRORS, &mi->context.options))
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3ef4d78..bf966f3 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7847,6 +7847,23 @@
VERIFY_PERMISSION(OPT_P_INSTANCE);
options->disable = true;
}
+ else if (streq(p[0], "override-username") && p[1] && !p[2])
+ {
+ VERIFY_PERMISSION(OPT_P_INSTANCE);
+ if (strlen(p[1]) > TLS_USERNAME_LEN)
+ {
+ msg(msglevel, "override-username exceeds the maximum length of %d "
+ "characters", TLS_USERNAME_LEN);
+
+ /* disable the connection since ignoring the request to
+ * set another username might serious problems */
+ options->disable = true;
+ }
+ else
+ {
+ options->override_username = p[1];
+ }
+ }
else if (streq(p[0], "tcp-nodelay") && !p[1])
{
VERIFY_PERMISSION(OPT_P_GENERAL);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 55f12dd..274f282 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -492,6 +492,7 @@
const char *client_config_dir;
bool ccd_exclusive;
bool disable;
+ const char *override_username;
int n_bcast_buf;
int tcp_queue_limit;
struct iroute *iroutes;
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index a7cd3bf..5ccd7d8 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -595,9 +595,19 @@
*/
if (tls_multi->auth_token)
{
- push_option_fmt(gc, push_list, M_USAGE,
- "auth-token %s",
+ push_option_fmt(gc, push_list, M_USAGE, "auth-token %s",
tls_multi->auth_token);
+
+ char *base64user;
+ int ret = openvpn_base64_encode(tls_multi->locked_username,
+
(int)strlen(tls_multi->locked_username),
+ &base64user);
+ if (ret < USER_PASS_LEN && ret > 0)
+ {
+ push_option_fmt(gc, push_list, M_USAGE, "auth-token-user %s",
+ base64user);
+ }
+ free(base64user);
}
}
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index e4a7b57..e548299 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1260,6 +1260,7 @@
free(multi->peer_info);
free(multi->locked_cn);
free(multi->locked_username);
+ free(multi->locked_original_username);
cert_hash_free(multi->locked_cert_hash_set);
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index e092472..79d0575 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -625,7 +625,15 @@
* Our locked common name, username, and cert hashes (cannot change during
the life of this tls_multi object)
*/
char *locked_cn;
+
+ /** The locked username is the username that the client used for initial
+ * authentication */
char *locked_username;
+
+ /** The username that client initial used before being overrriden by
+ * by override-user */
+ char *locked_original_username;
+
struct cert_hash_set *locked_cert_hash_set;
/** Time of last when we updated the cached state of
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index e7d7ed6..158fe19 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -48,9 +48,6 @@
#include "push.h"
#include "ssl_util.h"
-/** Maximum length of common name */
-#define TLS_USERNAME_LEN 64
-
static void
string_mod_remap_name(char *str)
{
@@ -153,7 +150,10 @@
{
if (multi->locked_username)
{
- if (strcmp(username, multi->locked_username) != 0)
+ /* If the username has been overridden, we accept both the original
+ * username and the changed username */
+ if (strcmp(username, multi->locked_username) != 0
+ && (!multi->locked_original_username || strcmp(username,
multi->locked_original_username) != 0))
{
msg(D_TLS_ERRORS, "TLS Auth Error: username attempted to change
from '%s' to '%s' -- tunnel disabled",
multi->locked_username,
diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h
index cd2ec24..77e814a 100644
--- a/src/openvpn/ssl_verify.h
+++ b/src/openvpn/ssl_verify.h
@@ -51,6 +51,9 @@
/** Maximum certificate depth we will allow */
#define MAX_CERT_DEPTH 16
+/** Maximum length of common name */
+#define TLS_USERNAME_LEN 64
+
/** Structure containing the hash for a single certificate */
struct cert_hash {
unsigned char sha256_hash[256/8];
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/872?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: Ia4095518d5e4447992a2974e0d7a159d79ba6b6f
Gerrit-Change-Number: 872
Gerrit-PatchSet: 1
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: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
