| Message ID | 20201130123928.21837-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Fix auth-token not being updated if auth-nocache is set | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> Thanks for digging into this - this was an annoying and hard to diagnose "sometimes, TLS reconnects fail for users where it *should* succeed due to tokens being used" problem (that openvpn considers tokens sensitive and never logs them didn't help pinpointing the issue :-) ). I tested this with a client built with lots of extra debug output, gen-auth-token and frequend tls-renegotiates - and indeed, "up->defined" goes to "0" after the first incoming token if auth-nocache is active - so, no *further* tokens are learned. With the patch, we also look at "tk->defined", which is *then* defined, and tokens work even in that case. Verified with a 2.4 client against a master server, including a master restart without having to re-enter credentials on the client. Yay :-) Your patch has been applied to the master, release/2.5 and release/2.4 branch (bugfix, identical code in all branches). commit fb789947ab1eba3e68fb8e4b3551d095a53962bd (master) commit 95e183723fc6571c73ed070b22923df2ce666af2 (release/2.5) commit f9b73042892c14b906772e72b3116d809457c721 (release/2.4) Author: Arne Schwabe Date: Mon Nov 30 13:39:28 2020 +0100 Fix auth-token not being updated if auth-nocache is set Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20201130123928.21837-1-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21291.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 1038b383..c0c72dd7 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -510,10 +510,15 @@ void set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token) { - if (token && strlen(token) && up && up->defined) + if (strlen(token) && (up->defined || tk->defined)) { + /* auth-token has no password, so it needs the username + * either already set or copied from up */ strncpynt(tk->password, token, USER_PASS_LEN); - strncpynt(tk->username, up->username, USER_PASS_LEN); + if (up->defined) + { + strncpynt(tk->username, up->username, USER_PASS_LEN); + } tk->defined = true; } diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index a03d94e2..8c3a1227 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -145,6 +145,17 @@ 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 + * 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 + * + * @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);
This fixes the auth-token not being updated if auth-nocache is set. Our set_auth_token method ensures that the auth-token always has a username but is a little bit too strict in the check. Also add doxygen documentation and remove null checks. We use this function only with non-null pointers and it makes it a bit nicer to read. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/misc.c | 9 +++++++-- src/openvpn/misc.h | 11 +++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-)