| Message ID | 20210520151148.2565578-2-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Delegated to: | Antonio Quartulli |
| Headers | show |
| Series | [Openvpn-devel,v2,1/9] Move auth_token_state from multi to key_state | expand |
Hi, On 20/05/2021 17:11, Arne Schwabe wrote: > When not using username and password (i.e. auth-user-pass) it can still make > to provide the client with an auth-token, e.g. for allowing a session to > continue after a reconnect without requiring 2FA again. > > However, without --auth-user-pass openvpn does not have a username and will > ignore any pushed auth-token command. > > This patch adds support for auth-token-user to set the username that should > be used for auth-token > > The spec of using auth-token-user base64-encoded-user are the ones that > OpenVPN3 already implements. > > Patch V2: Improve style, fix comments and commit message > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> This patch hasn't changed since the last time it was on the mailing list (and I acked it), so the same goes for this copy. @Arne I have a new question though: what is expected to happen is the --auth-token-user is specified in the global config? Is the same user supposed to be used with every client? Or is it just ignored? I am testing this case and I don't see the user being pushed to the client. Cheers,
Am 11.06.21 um 02:41 schrieb Antonio Quartulli: > Hi, > > On 20/05/2021 17:11, Arne Schwabe wrote: >> When not using username and password (i.e. auth-user-pass) it can still make >> to provide the client with an auth-token, e.g. for allowing a session to >> continue after a reconnect without requiring 2FA again. >> >> However, without --auth-user-pass openvpn does not have a username and will >> ignore any pushed auth-token command. >> >> This patch adds support for auth-token-user to set the username that should >> be used for auth-token >> >> The spec of using auth-token-user base64-encoded-user are the ones that >> OpenVPN3 already implements. >> >> Patch V2: Improve style, fix comments and commit message >> >> Signed-off-by: Arne Schwabe <arne@rfc2549.org> > > This patch hasn't changed since the last time it was on the mailing list > (and I acked it), so the same goes for this copy. > > @Arne I have a new question though: what is expected to happen is the > --auth-token-user is specified in the global config? > > Is the same user supposed to be used with every client? > Or is it just ignored? > > I am testing this case and I don't see the user being pushed to the client. > It is probably the same as with auth-token itself. OpenVPN will pick it up and use it but most times it is not every useful as auth-token should be pushed from the server. It is an artefact from how we parse things. Arne
Hi, On 11/06/2021 11:48, Arne Schwabe wrote: > Am 11.06.21 um 02:41 schrieb Antonio Quartulli: >> Hi, >> >> On 20/05/2021 17:11, Arne Schwabe wrote: >>> When not using username and password (i.e. auth-user-pass) it can still make >>> to provide the client with an auth-token, e.g. for allowing a session to >>> continue after a reconnect without requiring 2FA again. >>> >>> However, without --auth-user-pass openvpn does not have a username and will >>> ignore any pushed auth-token command. >>> >>> This patch adds support for auth-token-user to set the username that should >>> be used for auth-token >>> >>> The spec of using auth-token-user base64-encoded-user are the ones that >>> OpenVPN3 already implements. >>> >>> Patch V2: Improve style, fix comments and commit message >>> >>> Signed-off-by: Arne Schwabe <arne@rfc2549.org> >> >> This patch hasn't changed since the last time it was on the mailing list >> (and I acked it), so the same goes for this copy. >> >> @Arne I have a new question though: what is expected to happen is the >> --auth-token-user is specified in the global config? >> >> Is the same user supposed to be used with every client? >> Or is it just ignored? >> >> I am testing this case and I don't see the user being pushed to the client. >> > > It is probably the same as with auth-token itself. OpenVPN will pick it > up and use it but most times it is not every useful as auth-token should > be pushed from the server. It is an artefact from how we parse things. > Makes sense - it is jus interpreted locally, but there is not much sense. This said, the rest looks good. Acked-by: Antonio Quartulli <antonio@openvpn.net>
Your patch has been applied to the master and release/2.5 branch
(long-term compat).
I'm not exactly happy with this patch for a number of reasons, which is
why I was a bit reluctant to merge it. It does what it says on the lid,
the code is safe, and it got ACKs. So, merging.
That said, this is sort of a special case for a special case that
seems to have come out of OpenVPN Inc product marketing (do 2FA outside
OpenVPN and then notice that this doesn't actually work) - and it makes
the code even more complex by having this extra boolean to check in
a number of places... (a somewhat simpler approach could be to have
a "default username" - <NOT-SET> or such - and use that if up is not
defined)
Anyway. I have tested the "does auth-token, on reneg, and token
expiry still work?" bit fairly thoroughly - as this is code that took
us quite a while to get right - and it seems to still work. I have
not tested actually pushing "auth-token-user bla", but I have been
told Heiko and David have tested that path in earnest.
I have also taken the liberty to clean up the comments quite a bit
(seems Richard did not like this patch and did not spell-check :-) ).
commit b398aa37ca309948b481401adf0074ea5589eb2d (master)
commit d38d61111d08558e2f52cc9bcdc928ca9c4fca61 (release/2.5)
Author: Arne Schwabe
Date: Thu May 20 17:11:41 2021 +0200
Implement auth-token-user
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210520151148.2565578-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22417.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst index af21fbcd7..c5b7ad960 100644 --- a/doc/man-sections/client-options.rst +++ b/doc/man-sections/client-options.rst @@ -50,6 +50,14 @@ configuration. after a failed auth. Older clients will keep using the token value and react according to ``--auth-retry`` +--auth-token-user base64username + Companion option to ``--auth-token``. This options allows to override + the username used by the client when reauthenticating with the ``auth-token``. + It also allows to use ``--auth-token`` in setups that normally do not use + username and password. + + The username has to be base64 encoded. + --auth-user-pass Authenticate with server using username/password. diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 650daa0c6..29061cd6f 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -490,22 +490,49 @@ void set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token) { - if (strlen(token) && (up->defined || tk->defined)) + if (strlen(token)) { - /* auth-token has no password, so it needs the username - * either already set or copied from up */ strncpynt(tk->password, token, USER_PASS_LEN); - if (up->defined) + tk->token_defined = true; + + /* + * --auth-token has no username, so it needs the username + * either already set or copied from up, or later set by + * --auth-token-user + * + * Do not overwrite the username if already set to avoid + * overwriting an auth-token + */ + if (up->defined && !tk->defined) { strncpynt(tk->username, up->username, USER_PASS_LEN); + tk->defined = true; } - tk->defined = true; } /* Cleans user/pass for nocache */ purge_user_pass(up, false); } +void +set_auth_token_user(struct user_pass *tk, const char *username) +{ + if (strlen(username)) + { + /* Clear the username before decoding to ensure no old material is left + * and also allow decoding to not use all space to ensure the last byte is + * always 0 */ + CLEAR(tk->username); + int len = openvpn_base64_decode(username, tk->username, USER_PASS_LEN - 1); + tk->defined = len > 0; + if (!tk->defined) + { + msg(D_PUSH, "Error decoding auth-token-username"); + } + } +} + + /* * Process string received by untrusted peer before * printing to console or log file. diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index d9005353e..0d2d42489 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -56,6 +56,9 @@ const char *hostname_randomize(const char *hostname, struct gc_arena *gc); struct user_pass { bool defined; + /* For auth-token username and token can be set individually, so + * we this second bool to track if the token (password) is defined */ + bool token_defined; bool nocache; /* max length of username/password */ @@ -138,19 +141,31 @@ void fail_user_pass(const char *prefix, void purge_user_pass(struct user_pass *up, const bool force); /** - * Sets the auth-token to token if a username is available from either - * up or already present in tk. The method will also purge up if + * Sets the auth-token to token. Ff a username is available from either + * up or already present in tk is the auth-token that will be used as default + * username for the token. The method will also purge up if * the auth-nocache option is active. * * @param up (non Auth-token) Username/password * @param tk auth-token userpass to set - * @param token token to use as password for the + * @param token token to use as password for the auth-token * * @note all parameters to this function must not be null. */ void set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token); +/** + * Sets the auth-token username by base64 decoding the passed + * username + * + * @param tk auth-token userpass to set + * @param username base64 encoded username to set + * + * @note all parameters to this function must not be null. + */ +void set_auth_token_user(struct user_pass *tk, const char *username); + /* * Process string received by untrusted peer before * printing to console or log file. diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 5a6f37d7d..fa3ee50d6 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8308,6 +8308,11 @@ add_option(struct options *options, } #endif } + else if (streq(p[0], "auth-token-user") && p[1] && !p[2]) + { + VERIFY_PERMISSION(OPT_P_ECHO); + ssl_set_auth_token_user(p[1]); + } else if (streq(p[0], "single-session") && !p[1]) { VERIFY_PERMISSION(OPT_P_GENERAL); diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index e843b2152..7eb356ddf 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -446,6 +446,12 @@ ssl_set_auth_token(const char *token) set_auth_token(&auth_user_pass, &auth_token, token); } +void +ssl_set_auth_token_user(const char *username) +{ + set_auth_token_user(&auth_token, username); +} + /* * Cleans an auth token and checks if it was active */ @@ -2310,8 +2316,8 @@ key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct tls_sessi } } - /* write username/password if specified */ - if (auth_user_pass_enabled) + /* write username/password if specified or we are using a auth-token */ + if (auth_user_pass_enabled || (auth_token.token_defined && auth_token.defined)) { #ifdef ENABLE_MANAGEMENT auth_user_pass_setup(session->opt->auth_user_pass_file, session->opt->sci); @@ -2324,7 +2330,7 @@ key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct tls_sessi * If we have a valid auth-token, send that instead of real * username/password */ - if (auth_token.defined) + if (auth_token.token_defined && auth_token.defined) { up = &auth_token; } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 88f89876a..81cc22131 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -450,6 +450,8 @@ void ssl_purge_auth(const bool auth_user_pass_only); void ssl_set_auth_token(const char *token); +void ssl_set_auth_token_user(const char *username); + bool ssl_clean_auth_token(void); #ifdef ENABLE_MANAGEMENT
When not using username and password (i.e. auth-user-pass) it can still make to provide the client with an auth-token, e.g. for allowing a session to continue after a reconnect without requiring 2FA again. However, without --auth-user-pass openvpn does not have a username and will ignore any pushed auth-token command. This patch adds support for auth-token-user to set the username that should be used for auth-token The spec of using auth-token-user base64-encoded-user are the ones that OpenVPN3 already implements. Patch V2: Improve style, fix comments and commit message Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- doc/man-sections/client-options.rst | 8 +++++++ src/openvpn/misc.c | 37 +++++++++++++++++++++++++---- src/openvpn/misc.h | 21 +++++++++++++--- src/openvpn/options.c | 5 ++++ src/openvpn/ssl.c | 12 +++++++--- src/openvpn/ssl.h | 2 ++ 6 files changed, 74 insertions(+), 11 deletions(-)