Attention is currently required from: flichtenheld. 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 (#3). 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 <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, 158 insertions(+), 10 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/72/872/3 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..a76de67 100644 --- a/doc/man-sections/server-options.rst +++ b/doc/man-sections/server-options.rst @@ -89,6 +89,12 @@ 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``. In this case the client will be pushed also the + ``--auth-token-user`` option and and an auth token that is valid for + that username instead of the original username that the client + authenticated with. + --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 +418,32 @@ 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 both the initial username of the client and the + overridden username are correctly handled when using ``--override-username`` + and the related 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..f51c42e 100644 --- a/src/openvpn/auth_token.c +++ b/src/openvpn/auth_token.c @@ -330,6 +330,7 @@ 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; diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index f426b46..3e0cbe1 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,48 @@ 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; + struct tls_session *session = &multi->session[TM_ACTIVE]; + + 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); + + /* Override also the common name if username should be set as common + * name */ + if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME)) + { + set_common_name(session, multi->locked_username); + tls_lock_common_name(multi); + } + + /* 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 +2810,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..e78c8e7 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) { @@ -85,10 +82,7 @@ } } -/* - * Set the given session's common_name - */ -static void +void set_common_name(struct tls_session *session, const char *common_name) { if (session->common_name) @@ -141,6 +135,7 @@ const char *cn = multi->session[TM_ACTIVE].common_name; if (cn && !multi->locked_cn) { + free(multi->locked_cn); multi->locked_cn = string_alloc(cn, NULL); } } @@ -153,7 +148,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, @@ -1613,6 +1611,17 @@ */ bool skip_auth = false; + /* Replace username early if override-username is in effect but only + * if client is sending the original username */ + if (multi->locked_original_username + && strncmp(up->username, multi->locked_original_username, sizeof(up->username)) != 0) + { + strncpy(up->username, multi->locked_username, sizeof(up->username)); + msg(D_MULTI_LOW, "TLS: Replacing client provided username '%s' with " + "username from override-user '%s'", up->username, + multi->locked_username); + } + /* * If server is configured with --auth-gen-token and the client sends * something that looks like an authentication token, this diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h index cd2ec24..eba3832 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]; @@ -146,6 +149,16 @@ */ const char *tls_common_name(const struct tls_multi *multi, const bool null); + +/** + * Sets the common name field for the given tunnel + * + * @param multi The tunnel to set the common name for + * @param common_name The name to set the common name to + */ +void +set_common_name(struct tls_session *session, const char *common_name); + /** * Returns the username field for the given tunnel * -- 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: 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