Attention is currently required from: flichtenheld, plaisthos.
Hello flichtenheld,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/872?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld
Change subject: Implement override-username
......................................................................
Implement override-username
This is intended to allow using auth-gen-token in scenarios where the
client use certificates and multi-factor authentication.
It allows a client to successfully roam to a different server and have
a correct username and auth-token that can be accepted by that server as
fully authenticated user without requiring MFA again.
The scenario that this feature is probably most useful
when --management-client-auth is in use as in this mode the OpenVPN
server can accept clients without username/password but still use
--auth-gen-token with username and password to accept auth-token as
alternative authentication.
Change-Id: Ia4095518d5e4447992a2974e0d7a159d79ba6b6f
Signed-off-by: Arne Schwabe <[email protected]>
---
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, 132 insertions(+), 6 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/72/872/2
diff --git a/Changes.rst b/Changes.rst
index 16ae6fc..8543113 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
+ clients 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..79b1bfa 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -89,6 +89,9 @@
will lead to authentication bypass (as does returning success on a wrong
password from a script).
+ **Note:** the username for ``--auth-gen-token`` can be overridden by
+ ``--override-user``.
+
--auth-gen-token-secret file
Specifies a file that holds a secret for the HMAC used in
``--auth-gen-token`` If ``file`` is not present OpenVPN will generate a
@@ -412,6 +415,31 @@
This option requires that ``--disable-occ`` NOT be used.
+--override-username
+ Sets the username of a connection to the specified username. This username
+ will also be used by ``--auth-gen-token``. However, the overridden
+ username comes only into effect *after* the ``--client-config-dir`` has been
+ read and the ``--auth-user-pass-verify`` and ``--client-connect`` scripts
+ 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 ``--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.
+
--port-share args
Share OpenVPN TCP with another service
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..5d464e0 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,
};
+/**
+ * Overrides the locked username with the username of --override-username
+ * @param mi the multi instance that should be modified.
+ */
+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..93b8417 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -440,6 +440,8 @@
" Only valid in a client-specific config file.\n"
"--disable : Client is disabled.\n"
" Only valid in a client-specific config file.\n"
+ "--override-username: Overrides the client-specific username to be used.\n"
+ " Only valid in a client-specific config file.\n"
"--verify-client-cert [none|optional|require] : perform no, optional or\n"
" mandatory client certificate verification.\n"
" Default is to require the client to supply a
certificate.\n"
@@ -7847,6 +7849,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 cause 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..2efeb6a 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -625,7 +625,16 @@
* 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 we assume the client is using.
+ * Normally the username used for initial authentication unless
+ * overridden by --override-username */
char *locked_username;
+
+ /** The username that client initially used before being overridden
+ * 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: 2
Gerrit-Owner: plaisthos <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
