Rework OpenVPN auth-token support
PATCH 1/1 Rework OpenVPN auth-token support
Auth-token is documented as a token that the client will use instead of a auth-token. When using an external auth-token script and OTP authentication, it is useful to have this token survive on reconnect (e.g. mobile device roaming). On the other hand if the token does expire, the client should fall back to the previous authentication methd (e.g. user/pass) or ask for a OTP password again.
Behaviour of OpenVPN client (prior to this patch) is to never fallback to the previous authentication method. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
This patch changes the client behaviour:
- Treat a failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method
- Implement a new pushable option forget-token-reconnect that forces to forget an auth-token when reconnecting
- Sending IV_PROTO=3 to signal that it is safe to send this client an expiring auth-token
The behaviour of the server option auth-gen-token:
- Automatically push forget-token-reconnect to avoid a failed authentication after reconnect
- By default only send auth-token to clients that will gracefully handle auth-token to avoid having clients not able to reconnect
- Add a force option to auth-gen-token that allow to ignore if the client can handle auth-tokens
--- doc/openvpn.8 | 39 ++++++++++++++++++-- src/openvpn/init.c | 4 ++ src/openvpn/misc.c | 16 +++++--- src/openvpn/misc.h | 3 +- src/openvpn/options.c | 11 +++++- src/openvpn/options.h | 2 + src/openvpn/push.c | 95 ++++++++++++++++++++++++++++++++---------------- src/openvpn/ssl.c | 41 +++++++++++++-------- src/openvpn/ssl.h | 2 + src/openvpn/ssl_common.h | 2 + 10 files changed, 156 insertions(+), 59 deletions(-)
doc/openvpn.8 +36 -3
@@ -3058,7 +3058,8 @@ IV_LZO_STUB=1 \-\- if client was built with LZO stub capability IV_LZ4=1 \-\- if the client supports LZ4 compressions. -IV_PROTO=2 \-\- if the client supports peer\-id floating mechansim +IV_PROTO=3 \-\- if the client supports peer\-id floating mechansim (2 or greater) and +fallback to normal authentication after a failed auth-token authentication (3 or greater) IV_NCP=2 \-\- negotiable ciphers, client supports .B \-\-cipher
@@ -3664,7 +3665,7 @@ For a sample script that performs PAM authentication, see in the OpenVPN source distribution. .\"********************************************************* .TP -.B \-\-auth\-gen\-token [lifetime] +.B \-\-auth\-gen\-token [lifetime] [force] After successful user/password authentication, the OpenVPN server will with this option generate a temporary authentication token and push that to client. On the following
@@ -3684,6 +3685,12 @@ This feature is useful for environments which is configured to use One Time Passwords (OTP) as part of the user/password authentications and that authentication mechanism does not implement any auth\-token support. + +The +.B force +option will send auth-token even if the client does not signal +proper auth-token support on reconnect and will fail on reconnect with +failed authentication. This is the behaviour of versions prior to 2.4.6. .\"********************************************************* .TP .B \-\-opt\-verify
@@ -5337,7 +5344,33 @@ push "auth\-token UNIQUE_TOKEN_VALUE" into the file/buffer for dynamic configuration data. This will then make the OpenVPN server to push this value to the client, which replaces the local password with the -UNIQUE_TOKEN_VALUE. +UNIQUE_TOKEN_VALUE. If you want the client (2.4.6+) to forget the token when +reconnecting also use: + +.nf +.ft 3 +.in +4 +push "forget\-token\-reconnect" +.in -4 +.ft +.fi + +Newer clients (2.4.6+) will fall back to the original password method +after a failed auth-token attempt. Older clients will keep using the token +value and react acording to +.B \-\-auth-retry +. +.\********************************************************** +.TP +.B forget\-token\-reconnect + +Normally a client will retry an auth-token after a reconnect (e.g. USR1 triggered). +If the token fails, the client will fall back to the original method and make an additional +connection attempt. If the token is only valid for a single connection this option can be used +to skip the first attempt on reconnect. The +.B +auth\-gen\-token +option automatically pushes this option. .\"********************************************************* .TP .B \-\-tls\-verify cmd
src/openvpn/init.c +4 -0
@@ -2750,6 +2750,7 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.auth_user_pass_file = options->auth_user_pass_file; to.auth_token_generate = options->auth_token_generate; to.auth_token_lifetime = options->auth_token_lifetime; + to.auth_token_generate_force = options->auth_token_generate_force;#endif to.x509_track = options->x509_track;
@@ -3390,6 +3391,9 @@ do_close_tls(struct context *c)md_ctx_cleanup (c->c2.pulled_options_state);md_ctx_free (c->c2.pulled_options_state); } + +if (c->options.forget_token_on_reconnect) +ssl_clean_auth_token (); }/*
src/openvpn/misc.c +10 -6
@@ -1258,7 +1258,7 @@ purge_user_pass(struct user_pass *up, const bool force)* don't show warning if the pass has been replaced by a token: this is an * artificial "auth-nocache" */ - else if (!warn_shown && (!up->tokenized)) + else if (!warn_shown) {msg (M_WARN ,"WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this" ); warn_shown = true;
@@ -1266,14 +1266,18 @@ purge_user_pass(struct user_pass *up, const bool force) }void -set_auth_token (struct user_pass *up,const char *token) +set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token) { -if (token &&strlen (token) && up && up->defined && !up->nocache) + +if (token &&strlen (token) && up && up->defined) { -CLEAR (up->password); -strncpynt (up->password, token,USER_PASS_LEN ); - up->tokenized = true; +strncpynt (tk->password, token,USER_PASS_LEN ); +strncpynt (tk->username, up->username,USER_PASS_LEN ); + tk->defined = true; } + +/* Cleans user/pass for nocache */ +purge_user_pass (up, false); }/*
src/openvpn/misc.h +1 -2
@@ -168,7 +168,6 @@ struct user_pass {bool defined;bool nocache; -bool tokenized;/* true if password has been substituted by a token */ bool wait_for_push;/* true if this object is waiting for a push-reply */ /* max length of username/password */
@@ -250,7 +249,7 @@ void fail_user_pass(const char *prefix,void purge_user_pass (struct user_pass *up,const bool force); -void set_auth_token (struct user_pass *up,const char *token); +void set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token);/* * Process string received by untrusted peer before
src/openvpn/options.c +10 -1
@@ -883,6 +883,7 @@ init_options(struct options *o, const bool init_gc)/* P2MP server context features */ #if P2MP_SERVER o->auth_token_generate = false; + o->auth_token_generate_force = false;/* Set default --tmp-dir */ #ifdef _WIN32
@@ -1334,6 +1335,7 @@ show_p2mp_parms(const struct options *o)SHOW_STR (auth_user_pass_verify_script);SHOW_BOOL (auth_user_pass_verify_script_via_file);SHOW_BOOL (auth_token_generate); +SHOW_BOOL (auth_token_generate_force);SHOW_INT (auth_token_lifetime);#if PORT_SHARE SHOW_STR (port_share_host);
@@ -6718,11 +6720,13 @@ add_option(struct options *options, &options->auth_user_pass_verify_script, p[1], "auth-user-pass-verify", true); } -else if (streq (p[0 ],"auth-gen-token" )) +else if (streq (p[0 ],"auth-gen-token" ) && !p[3 ]) {VERIFY_PERMISSION (OPT_P_GENERAL ); options->auth_token_generate = true; options->auth_token_lifetime = p[1 ] ?positive_atoi (p[1 ]) :0 ; +if (p[2 ] &&streq (p[2 ],"force" )) + options->auth_token_generate_force = true; }else if (streq (p[0 ],"client-connect" ) && p[1 ]) {
@@ -7791,6 +7795,11 @@ add_option(struct options *options, }#endif } +else if (streq (p[0 ],"forget-token-reconnect" ) && !p[1 ]) + { +VERIFY_PERMISSION (OPT_P_GENERAL |OPT_P_ECHO ); + options->forget_token_on_reconnect = true; + }else if (streq (p[0 ],"single-session" ) && !p[1 ]) {VERIFY_PERMISSION (OPT_P_GENERAL );
src/openvpn/options.h +2 -0
@@ -443,6 +443,7 @@ struct optionsconst char *auth_user_pass_verify_script;bool auth_user_pass_verify_script_via_file;bool auth_token_generate; +bool auth_token_generate_force;unsigned int auth_token_lifetime;#if PORT_SHARE char *port_share_host;
@@ -457,6 +458,7 @@ struct optionsunsigned int push_option_types_found;const char *auth_user_pass_file;struct options_pre_pull *pre_pull; +bool forget_token_on_reconnect;int scheduled_exit_interval;
src/openvpn/push.c +64 -31
126 lines not on this page · raw message
src/openvpn/ssl.c +25 -16
87 lines not on this page · raw message
src/openvpn/ssl.h +2 -0
@@ -428,6 +428,8 @@ void ssl_purge_auth(const bool auth_user_pass_only);void ssl_set_auth_token (const char *token); +bool ssl_clean_auth_token (void ); +#ifdef ENABLE_CLIENT_CR /* * ssl_get_auth_challenge will parse the server-pushed auth-failed
src/openvpn/ssl_common.h +2 -0
@@ -299,6 +299,8 @@ struct tls_optionsconst char *auth_user_pass_file;bool auth_token_generate;/**< Generate auth-tokens on successful user/pass auth, * set via options->auth_token_generate. */ +bool auth_token_generate_force;/* Sent auth-tokens even if the client will not reconnect +* properly with them */ unsigned int auth_token_lifetime;/* use the client-config-dir as a positive authenticator */
Signature · 1 line
2.11.0
PATCH v2 1/1 Rework OpenVPN auth-token support
Auth-token is documented as a token that the client will use instead of a auth-token. When using an external auth-token script and OTP authentication, it is useful to have this token survive on reconnect (e.g. mobile device roaming). On the other hand if the token does expire, the client should fall back to the previous authentication methd (e.g. user/pass) or ask for a OTP password again.
Behaviour of OpenVPN client (prior to this patch) is to never fallback to the previous authentication method. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
This patch changes the client behaviour:
- Treat a failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method
- Implement a new pushable option forget-token-reconnect that forces to forget an auth-token when reconnecting
- Sending IV_PROTO=3 to signal that it is safe to send this client an expiring auth-token
The behaviour of the server option auth-gen-token:
- Automatically push forget-token-reconnect to avoid a failed authentication after reconnect
- By default only send auth-token to clients that will gracefully handle auth-token to avoid having clients not able to reconnect
- Add a force option to auth-gen-token that allow to ignore if the client can handle auth-tokens
Patch V2: properly formatted commit message, fix openvpn3 detection
--- doc/openvpn.8 | 39 ++++++++++++++++++-- src/openvpn/init.c | 4 ++ src/openvpn/misc.c | 16 +++++--- src/openvpn/misc.h | 3 +- src/openvpn/options.c | 11 +++++- src/openvpn/options.h | 2 + src/openvpn/push.c | 95 ++++++++++++++++++++++++++++++++---------------- src/openvpn/ssl.c | 41 +++++++++++++-------- src/openvpn/ssl.h | 2 + src/openvpn/ssl_common.h | 2 + 10 files changed, 156 insertions(+), 59 deletions(-)
doc/openvpn.8 +36 -3
@@ -3058,7 +3058,8 @@ IV_LZO_STUB=1 \-\- if client was built with LZO stub capability IV_LZ4=1 \-\- if the client supports LZ4 compressions. -IV_PROTO=2 \-\- if the client supports peer\-id floating mechansim +IV_PROTO=3 \-\- if the client supports peer\-id floating mechansim (2 or greater) and +fallback to normal authentication after a failed auth-token authentication (3 or greater) IV_NCP=2 \-\- negotiable ciphers, client supports .B \-\-cipher
@@ -3664,7 +3665,7 @@ For a sample script that performs PAM authentication, see in the OpenVPN source distribution. .\"********************************************************* .TP -.B \-\-auth\-gen\-token [lifetime] +.B \-\-auth\-gen\-token [lifetime] [force] After successful user/password authentication, the OpenVPN server will with this option generate a temporary authentication token and push that to client. On the following
@@ -3684,6 +3685,12 @@ This feature is useful for environments which is configured to use One Time Passwords (OTP) as part of the user/password authentications and that authentication mechanism does not implement any auth\-token support. + +The +.B force +option will send auth-token even if the client does not signal +proper auth-token support on reconnect and will fail on reconnect with +failed authentication. This is the behaviour of versions prior to 2.4.6. .\"********************************************************* .TP .B \-\-opt\-verify
@@ -5337,7 +5344,33 @@ push "auth\-token UNIQUE_TOKEN_VALUE" into the file/buffer for dynamic configuration data. This will then make the OpenVPN server to push this value to the client, which replaces the local password with the -UNIQUE_TOKEN_VALUE. +UNIQUE_TOKEN_VALUE. If you want the client (2.4.6+) to forget the token when +reconnecting also use: + +.nf +.ft 3 +.in +4 +push "forget\-token\-reconnect" +.in -4 +.ft +.fi + +Newer clients (2.4.6+) will fall back to the original password method +after a failed auth-token attempt. Older clients will keep using the token +value and react acording to +.B \-\-auth-retry +. +.\********************************************************** +.TP +.B forget\-token\-reconnect + +Normally a client will retry an auth-token after a reconnect (e.g. USR1 triggered). +If the token fails, the client will fall back to the original method and make an additional +connection attempt. If the token is only valid for a single connection this option can be used +to skip the first attempt on reconnect. The +.B +auth\-gen\-token +option automatically pushes this option. .\"********************************************************* .TP .B \-\-tls\-verify cmd
src/openvpn/init.c +4 -0
@@ -2750,6 +2750,7 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.auth_user_pass_file = options->auth_user_pass_file; to.auth_token_generate = options->auth_token_generate; to.auth_token_lifetime = options->auth_token_lifetime; + to.auth_token_generate_force = options->auth_token_generate_force;#endif to.x509_track = options->x509_track;
@@ -3390,6 +3391,9 @@ do_close_tls(struct context *c)md_ctx_cleanup (c->c2.pulled_options_state);md_ctx_free (c->c2.pulled_options_state); } + +if (c->options.forget_token_on_reconnect) +ssl_clean_auth_token (); }/*
src/openvpn/misc.c +10 -6
@@ -1258,7 +1258,7 @@ purge_user_pass(struct user_pass *up, const bool force)* don't show warning if the pass has been replaced by a token: this is an * artificial "auth-nocache" */ - else if (!warn_shown && (!up->tokenized)) + else if (!warn_shown) {msg (M_WARN ,"WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this" ); warn_shown = true;
@@ -1266,14 +1266,18 @@ purge_user_pass(struct user_pass *up, const bool force) }void -set_auth_token (struct user_pass *up,const char *token) +set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token) { -if (token &&strlen (token) && up && up->defined && !up->nocache) + +if (token &&strlen (token) && up && up->defined) { -CLEAR (up->password); -strncpynt (up->password, token,USER_PASS_LEN ); - up->tokenized = true; +strncpynt (tk->password, token,USER_PASS_LEN ); +strncpynt (tk->username, up->username,USER_PASS_LEN ); + tk->defined = true; } + +/* Cleans user/pass for nocache */ +purge_user_pass (up, false); }/*
src/openvpn/misc.h +1 -2
@@ -168,7 +168,6 @@ struct user_pass {bool defined;bool nocache; -bool tokenized;/* true if password has been substituted by a token */ bool wait_for_push;/* true if this object is waiting for a push-reply */ /* max length of username/password */
@@ -250,7 +249,7 @@ void fail_user_pass(const char *prefix,void purge_user_pass (struct user_pass *up,const bool force); -void set_auth_token (struct user_pass *up,const char *token); +void set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token);/* * Process string received by untrusted peer before
src/openvpn/options.c +10 -1
@@ -883,6 +883,7 @@ init_options(struct options *o, const bool init_gc)/* P2MP server context features */ #if P2MP_SERVER o->auth_token_generate = false; + o->auth_token_generate_force = false;/* Set default --tmp-dir */ #ifdef _WIN32
@@ -1334,6 +1335,7 @@ show_p2mp_parms(const struct options *o)SHOW_STR (auth_user_pass_verify_script);SHOW_BOOL (auth_user_pass_verify_script_via_file);SHOW_BOOL (auth_token_generate); +SHOW_BOOL (auth_token_generate_force);SHOW_INT (auth_token_lifetime);#if PORT_SHARE SHOW_STR (port_share_host);
@@ -6718,11 +6720,13 @@ add_option(struct options *options, &options->auth_user_pass_verify_script, p[1], "auth-user-pass-verify", true); } -else if (streq (p[0 ],"auth-gen-token" )) +else if (streq (p[0 ],"auth-gen-token" ) && !p[3 ]) {VERIFY_PERMISSION (OPT_P_GENERAL ); options->auth_token_generate = true; options->auth_token_lifetime = p[1 ] ?positive_atoi (p[1 ]) :0 ; +if (p[2 ] &&streq (p[2 ],"force" )) + options->auth_token_generate_force = true; }else if (streq (p[0 ],"client-connect" ) && p[1 ]) {
@@ -7791,6 +7795,11 @@ add_option(struct options *options, }#endif } +else if (streq (p[0 ],"forget-token-reconnect" ) && !p[1 ]) + { +VERIFY_PERMISSION (OPT_P_GENERAL |OPT_P_ECHO ); + options->forget_token_on_reconnect = true; + }else if (streq (p[0 ],"single-session" ) && !p[1 ]) {VERIFY_PERMISSION (OPT_P_GENERAL );
src/openvpn/options.h +2 -0
@@ -443,6 +443,7 @@ struct optionsconst char *auth_user_pass_verify_script;bool auth_user_pass_verify_script_via_file;bool auth_token_generate; +bool auth_token_generate_force;unsigned int auth_token_lifetime;#if PORT_SHARE char *port_share_host;
@@ -457,6 +458,7 @@ struct optionsunsigned int push_option_types_found;const char *auth_user_pass_file;struct options_pre_pull *pre_pull; +bool forget_token_on_reconnect;int scheduled_exit_interval;
src/openvpn/push.c +64 -31
126 lines not on this page · raw message
src/openvpn/ssl.c +25 -16
87 lines not on this page · raw message
src/openvpn/ssl.h +2 -0
@@ -428,6 +428,8 @@ void ssl_purge_auth(const bool auth_user_pass_only);void ssl_set_auth_token (const char *token); +bool ssl_clean_auth_token (void ); +#ifdef ENABLE_CLIENT_CR /* * ssl_get_auth_challenge will parse the server-pushed auth-failed
src/openvpn/ssl_common.h +2 -0
@@ -299,6 +299,8 @@ struct tls_optionsconst char *auth_user_pass_file;bool auth_token_generate;/**< Generate auth-tokens on successful user/pass auth, * set via options->auth_token_generate. */ +bool auth_token_generate_force;/* Sent auth-tokens even if the client will not reconnect +* properly with them */ unsigned int auth_token_lifetime;/* use the client-config-dir as a positive authenticator */
Signature · 1 line
2.11.0
Hi,
Based on the commit message this appears to cover all that is wrong with current auth-token implementation. I haven't carefully reviewed the code or tested it, but some initial remarks that looks relevant.
Quoted 7 lines · Arne Schwabe
Auth-token is documented as a token that the client will use instead of a auth-token. When using an external auth-token script and OTP authentication, it is useful to have this token survive on reconnect (e.g. mobile device roaming). On the other hand if the token does expire, the client should fall back to the previous authentication methd (e.g. user/pass) or ask for a OTP password again.
Quoted 5 lines
Behaviour of OpenVPN client (prior to this patch) is to never fallback to the previous authentication method. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
While this is mostly true, there is one point missing here. That being the main reason for this email, here goes:
While its true that, on SIGUSR1, the client does retry with auth-token it's only a minor nuisance: it leads to an AUTH_FAILED and then the client does go back to prompting for username/password (assuming auth-retry is enabled). The real sticky problem is when authentication fails during a TLS reneg due to an expired auth-token. The client won't realize why the reneg did not complete and will repeatedly try to renegotiate using the expired (or invalid) token. The reason for this is that the server does not send back an AUTH_FAILED message in such cases.
I want to stress this point: when the server sends back AUTH_FAILED, the client does behave somewhat sanely, but not otherwise. And on that count this patch appears to be lacking. It teaches the client to forget the token during SIGUSR1 restarts which is good, but does not teach the server to send back AUTH_FAILED in all instances of auth failures.
Quoted 5 lines
This patch changes the client behaviour:
- Treat a failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method
We could be more graceful here by not triggering USR1 -- clearing the token is enough as that will automatically cause the client to fall back to auth-user-pass prompting. I say graceful because some failures do not need a restart, only a re-attempt to authenticate with a valid username/password.
Quoted 3 lines
- Implement a new pushable option forget-token-reconnect that forces to forget an auth-token when reconnecting
I would have liked to see this as the default behaviour: IMO the "right" way to use auth-token is to expire it on reconnect but if some existing external scripts want it otherwise, fine...
Quoted 3 lines
- Sending IV_PROTO=3 to signal that it is safe to send this client an expiring auth-token
Once the server side auth-failure handling is fixed this will be less of a concern. But a new IV_PROTO value cant hurt.
Quoted 5 lines
The behaviour of the server option auth-gen-token:
- Automatically push forget-token-reconnect to avoid a failed authentication after reconnect
Yes. Unless "forget on reconnect" becomes the only possible client behaviour :)
Quoted 3 lines
- By default only send auth-token to clients that will gracefully handle auth-token to avoid having clients not able to reconnect
I suppose this is regarding expiring tokens, not permanent tokens.
Quoted 5 lines
- Add a force option to auth-gen-token that allow to ignore if the client can handle auth-tokens
Patch V2: properly formatted commit message, fix openvpn3 detection
... snipped...
Quoted 6 lines
src/openvpn/push.c +0
-0@@ -53,25 +53,31 @@ receive_auth_failed(struct context *c, const struct buffer *buffer)msg (M_VERB0 ,"AUTH: Received control message: %s" ,BSTR (buffer));
src/openvpn/push.c receive_auth_failed(struct context *c, const struct buffer *buffer) · line 60
Quoted 10 lines
src/openvpn/push.c +4
-3c->options.no_advance = true; -if (c->options.pull) - { -switch (auth_retry_get ()) +if (c->options.pull) { +/* Before checking how to react on AUTH_FAILED, first check if the failed authed might be +* the result of an expired auth-token */ +if (ssl_clean_auth_token ()) {
This is not going to work. The only time the server sends back AUTH_FAILED is during the initial connection. See that send_auth_failed() is called only during PUSH request processing[*]. So, failure due to token expiry that normally happens during a reneg[*] will not trigger AUTH_FAILED and the client will continue trying reneg until the previous TLS session expires (1 hour?). This is a basic limitation of the present implementation and I do not see it being addressed by the patch.
Other than this, the patch is looking good.
Selva
[*] Unless management-client-auth is in use.
Am 06.03.18 um 22:04 schrieb Selva Nair:
Quoted 33 lines
Hi,
Based on the commit message this appears to cover all that is wrong with current auth-token implementation. I haven't carefully reviewed the code or tested it, but some initial remarks that looks relevant.
Quoted 7 lines · Arne Schwabe
Auth-token is documented as a token that the client will use instead of a auth-token. When using an external auth-token script and OTP authentication, it is useful to have this token survive on reconnect (e.g. mobile device roaming). On the other hand if the token does expire, the client should fall back to the previous authentication methd (e.g. user/pass) or ask for a OTP password again.
Quoted 5 lines
Behaviour of OpenVPN client (prior to this patch) is to never fallback to the previous authentication method. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
While this is mostly true, there is one point missing here. That being the main reason for this email, here goes:
While its true that, on SIGUSR1, the client does retry with auth-token it's only a minor nuisance: it leads to an
AUTH_FAILEDand then the client does go back to prompting for username/password (assuming auth-retry is enabled). The real sticky problem is when authentication fails during a TLS reneg due to an expired auth-token. The client won't realize why the reneg did not complete and will repeatedly try to renegotiate using the expired (or invalid) token. The reason for this is that the server does not send back anAUTH_FAILEDmessage in such cases.
Quoted 5 lines
I want to stress this point: when the server sends back
AUTH_FAILED, the client does behave somewhat sanely, but not otherwise. And on that count this patch appears to be lacking. It teaches the client to forget the token during SIGUSR1 restarts which is good, but does not teach the server to send backAUTH_FAILEDin all instances of auth failures.
I think failed authentication during a renogiation is also a valid issue that this patch does not address and the behaviour is somewhat broken, also for non auth-token usage. I would prefer to fix that thing in a sperate patch. This patch mainly focuses on fixing auth-gen-token and client interaction as much as possible.
Quoted 12 lines
Quoted 5 lines
This patch changes the client behaviour:
- Treat a failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method
We could be more graceful here by not triggering USR1 -- clearing the token is enough as that will automatically cause the client to fall back to auth-user-pass prompting. I say graceful because some failures do not need a restart, only a re-attempt to authenticate with a valid username/password.
Currently this code is only triggered on inital connnection (as the server fails to send AUTH_FAILED as you noted before). I have not checked how easy it is for the client to send a second key_exchange message.
Quoted 7 lines
Quoted 3 lines
- Implement a new pushable option forget-token-reconnect that forces to forget an auth-token when reconnecting
I would have liked to see this as the default behaviour: IMO the "right" way to use auth-token is to expire it on reconnect but if some existing external scripts want it otherwise, fine...
That is something we should discuss. I did here err on the side of caution and not break existing setups since currently the only working that that does not break on reconnect is one where auth-tokens are valid after reconnect and changing this default would break those scnearios and in the past we have most times erred on the side of caution and compatibility.
Quoted 6 lines
Quoted 3 lines
- Sending IV_PROTO=3 to signal that it is safe to send this client an expiring auth-token
Once the server side auth-failure handling is fixed this will be less of a concern. But a new
IV_PROTOvalue cant hurt.
You still need the ability in the client to actually be able to forget and the server needs to know if the client supports forgetting and it is safe to send the auth-token.
Quoted 9 lines
Quoted 6 lines
The behaviour of the server option auth-gen-token:
- Automatically push forget-token-reconnect to avoid a failed authentication after reconnect
Yes. Unless "forget on reconnect" becomes the only possible client behaviour :)
Sure but then we should add a keep-token-reconnect to support "OTP with often roaming clients" scenario.
Quoted 6 lines
Quoted 4 lines
- By default only send auth-token to clients that will gracefully handle auth-token to avoid having clients not able to reconnect
I suppose this is regarding expiring tokens, not permanent tokens.
Yes, since auth-gen-token has no permanent tokens (they only valid for the session and after reconnect are invalid), they expire instantly when a client reconnects.
Quoted 10 lines
src/openvpn/push.c receive_auth_failed(struct context *c, const struct buffer *buffer) · line 60
Quoted 6 lines
src/openvpn/push.c +4
-1-switch (auth_retry_get ()) +if (c->options.pull) { +/* Before checking how to react on AUTH_FAILED, first check if the failed authed might be +* the result of an expired auth-token */ +if (ssl_clean_auth_token ()) {This is not going to work. The only time the server sends back
AUTH_FAILEDis during the initial connection. See thatsend_auth_failed()is called only during PUSH request processing[*].
At least it fixes a client's behaviour with this patch with server that has the default auth-gen-token without expiry set. :)
Quoted 5 lines
So, failure due to token expiry that normally happens during a reneg[*] will not trigger
AUTH_FAILEDand the client will continue trying reneg until the previous TLS session expires (1 hour?). This is a basic limitation of the present implementation and I do not see it being addressed by the patch.
I will look into sending AUTH_FAILED also without management-client-auth.
Arne
Quoted 7 lines · Arne Schwabe
Quoted 5 lines
So, failure due to token expiry that normally happens during a reneg[*] will not trigger
AUTH_FAILEDand the client will continue trying reneg until the previous TLS session expires (1 hour?). This is a basic limitation of the present implementation and I do not see it being addressed by the patch.I will look into sending
AUTH_FAILEDalso without management-client-auth.
Just a quick response. AUTH_FAILED can provide an explanation which the client can pick up and act upon. I did that in an earlier attempt, but it required lots of refactoring as the send_control() function depends on a few structs not being available where the token failures can be tackled.
See this mail thread for details:
Message-Id: [email protected]... Mail archive:
<https://www.mail-archive.com/[email protected]/msg12848.html>
This approach is what James initially recommended me to do, and this approach of providing a failure message back to the client should already be supported in OpenVPN 3 and the OpenVPN Connect clients. This messaging I believe is already used by the Access Server and Private Tunnel. OpenVPN 2 in client mode, could in these cases catch this "message" and ask for user credentials again.
In regards to having the tokens survive reconnect, I agree. But lets try to split this challenge into a few more minor patches.
The challenge I see with starting with a new IV_PROTO=3 is that this also requires support in the OpenVPN 3 library as well. I believe the "expiring auth-token" feature would be better handled with proper AUTH_FAILED signalling.
I'll have a closer look (hopefully) later today, just wanted to give this quick feedback before its too late.
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Inc
Hi,
Quoted 2 lines · Arne Schwabe
Am 06.03.18 um 22:04 schrieb Selva Nair:
..
Quoted 7 lines
Quoted 5 lines
I want to stress this point: when the server sends back
AUTH_FAILED, the client does behave somewhat sanely, but not otherwise. And on that count this patch appears to be lacking. It teaches the client to forget the token during SIGUSR1 restarts which is good, but does not teach the server to send backAUTH_FAILEDin all instances of auth failures.
Quoted 5 lines
I think failed authentication during a renogiation is also a valid issue that this patch does not address and the behaviour is somewhat broken, also for non auth-token usage. I would prefer to fix that thing in a sperate patch. This patch mainly focuses on fixing auth-gen-token and client interaction as much as possible.
That is fine. In that case let's not pretend that an expiring auth-token using auth-gen-token is a supported feature. Disable it in the server until we have the ability to send AUTH_FAILED in such cases.
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
This patch changes the client behaviour:
Quoted 9 lines
Quoted 2 lines
- Treat a failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method
We could be more graceful here by not triggering USR1 -- clearing the token is enough as that will automatically cause the client to fall back to auth-user-pass prompting. I say graceful because some failures do not need a restart, only a re-attempt to authenticate with a valid username/password.
Quoted 3 lines
Currently this code is only triggered on inital connnection (as the server fails to send
AUTH_FAILEDas you noted before). I have not checked how easy it is for the client to send a secondkey_exchangemessage.
Agreed SIGUSR1 is appropriate for initial failure, but initial failure happens with auth-token only because the client does not forget the token during SIGUSR1/SIGHUP reconnects. Once it learns to do that (as this patch does), there are no "initial failures" with auth-token (more on that below).
Quoted 10 lines
Quoted 9 lines
Quoted 5 lines
- Implement a new pushable option forget-token-reconnect that forces to forget an auth-token when reconnecting
I would have liked to see this as the default behaviour: IMO the "right" way to use auth-token is to expire it on reconnect but if some existing external scripts want it otherwise, fine...
Quoted 6 lines
That is something we should discuss. I did here err on the side of caution and not break existing setups since currently the only working that that does not break on reconnect is one where auth-tokens are valid after reconnect and changing this default would break those scnearios and in the past we have most times erred on the side of caution and compatibility.
Agreed, if external scripts that implement auth-token without using auth-gen-token wants to reuse the token, fine..
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
src/openvpn/push.c +4
-1-switch (auth_retry_get ()) +if (c->options.pull) { +/* Before checking how to react on AUTH_FAILED, first check if the failed authed might be +* the result of an expired auth-token */ +if (ssl_clean_auth_token ()) {
Quoted 6 lines
Quoted 3 lines
This is not going to work. The only time the server sends back
AUTH_FAILEDis during the initial connection. See thatsend_auth_failed()is called only during PUSH request processing[*].At least it fixes a client's behaviour with this patch with server that has the default auth-gen-token without expiry set. :)
Not really -- that gets fixed by forget-token-reconnect which is pushed when auth-gen-token is set. So I think this code path will never get exercised. As this patch will push forget-token-reconnect, every restart will call ssl_clean_auth_token() from init.c so the client will use the username/password, not any previously set token. As AUTH_FAILED is received only during such first attempts, there is no token to clean at that point and thus this will be skipped. It will "mysteriously" start to get exercised if/when we later fix AUTH_FAILED. So better leave it out of this patch. Or am I missing something?
Selva
A bit more thorough review this time.
On 05/03/18 16:50, Arne Schwabe wrote:
[...snip...]
Quoted 5 lines
This patch changes the client behaviour:
- Treat a failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method
Conceptually, this makes sense.
Quoted 2 lines
- Implement a new pushable option forget-token-reconnect that forces to forget an auth-token when reconnecting
This I am not happy about. We should use an AUTH_FAILED,SESSION:$message approach instead. This is what OpenVPN 3 based clients expects, and it should actually do (read: behave) as the previous point. OpenVPN 2 does not do that, today, so it would need to be taught that instead.
Quoted 2 lines
- Sending IV_PROTO=3 to signal that it is safe to send this client an expiring auth-token
And as we shouldn't need forget-token-reconnect, this would also not be needed.
Quoted 4 lines
The behaviour of the server option auth-gen-token:
- Automatically push forget-token-reconnect to avoid a failed authentication after reconnect
This should not be needed with the AUTH_FAILED,SESSION:$message approach.
Quoted 2 lines
- By default only send auth-token to clients that will gracefully handle auth-token to avoid having clients not able to reconnect
What do you mean with "gracefully handle auth-token"? By fixing OpenVPN 2's behaviour (as suggested above), it should behave gracefully by default.
And I'd be willing to /consider/ this fix even to OpenVPN 2.3 code base, for those who can't upgrade. But it will be a hard sell, though; OpenVPN 2.4 is over a year old already.
Quoted 2 lines
- Add a force option to auth-gen-token that allow to ignore if the client can handle auth-tokens
And this should also not be needed.
I'll admit I might see this with a bit too narrow perspective. But how I have understood this issue is that OpenVPN 2.x does not behave correctly as it doesn't understand *why* the authentication failed. If the client side would understand why auth failed, then it can query the user for credentials again - which I believe should resolve the current issues ... Or have I missed something?
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Inc
Hi,
...some good stuff snipped...
Quoted 6 lines
I'll admit I might see this with a bit too narrow perspective. But how I have understood this issue is that OpenVPN 2.x does not behave correctly as it doesn't understand *why* the authentication failed. If the client side would understand why auth failed, then it can query the user for credentials again - which I believe should resolve the current issues ... Or have I missed something?
I hope we are slowly spiralling towards a solution; not going around in circles...
Anyway, to reiterate: we currently have two issues. (i) client is not told when authentication fails during reneg
and (ii) client doesn't know that auth-gen-token's token is not reusable across reconnects
(SIGHUP and SIGUSR1).
Fixing (i) does not fix (ii). But (ii) easier to fix although we could keep arguing whether a forget-token-reconnect should be used or not etc..
(i) is the trickier, though I'm not convinced it requires so much refactoring.
Anyway, if there is an immediate solution to (i) it may be better to fix (ii) along with it. Else just fixing (ii) along the lines Arne has proposed looks like the way to go.
Selva
Quoted 11 lines · Selva Nair
Hi,
...some good stuff snipped...
Quoted 6 lines
I'll admit I might see this with a bit too narrow perspective. But how I have understood this issue is that OpenVPN 2.x does not behave correctly as it doesn't understand *why* the authentication failed. If the client side would understand why auth failed, then it can query the user for credentials again - which I believe should resolve the current issues ... Or have I missed something?
Quoted 8 lines
I hope we are slowly spiralling towards a solution; not going around in circles...
Anyway, to reiterate: we currently have two issues. (i) client is not told when authentication fails during reneg
and (ii) client doesn't know that auth-gen-token's token is not reusable across reconnects
(SIGHUP and SIGUSR1).
Okay, (i) is handled with AUTH_FAILED,SESSION messages today in OpenVPN 3. OpenVPN 2 should be taught to do the same - both server and client side.
In regards to (ii), that is an implementation detail which I would say is incorrect with --auth-gen-token. As I am the one who implemented that feature, I would rather have this fixed so auth-gen-token works across restarts.
Another new feature I can accept, is to enable rotating the auth-tokens. This is currently not possible. Not a need-to-have feature now, but more a nice-to-have.
Quoted 8 lines
Fixing (i) does not fix (ii). But (ii) easier to fix although we could keep arguing whether a forget-token-reconnect should be used or not etc..
(i) is the trickier, though I'm not convinced it requires so much refactoring.Anyway, if there is an immediate solution to (i) it may be better to fix (ii) along with it. Else just fixing (ii) along the lines Arne has proposed looks like the way to go.
Arne's approach with forget-token and IV_PROTO=3 diverges how OpenVPN 2 and OpenVPN 3 behaves. This is *not* an option for this issue.
The reason is that OpenVPN Access Server 2.5 is now shipping with an unmodified (to my knowledge, at least) upstream OpenVPN 2.4. This server integrates well with the OpenVPN Connect clients, which we have for Android, iOS, macOS and Windows (All OpenVPN 3 based). It would be sad if the community side fixes something which isn't really an issue (as part of the solution is already there); we're just doing it wrong currently. The current approach by Arne would also require support in OpenVPN 3 too; and I doubt this approach will get easily accepted there.
Also beware that more and more users will start deploying OpenVPN 3 based clients too; it is already widespread on iOS. And there are efforts on getting OpenVPN Connect into even more appstores for different platforms. On top of that, in addition to the new open source openvpn3-linux client, there will also be put some efforts to at least provide more mature reference implementation clients for Windows and macOS too. Plus Arne is also poking at using OpenVPN 3 in his Android client. Many of them will definitely connect against OpenVPN 2.x based servers; so OpenVPN 2 and OpenVPN 3 need to be well aligned.
(With that said, I'm not saying OpenVPN 3 supports all OpenVPN 2 features 100% today, but the most important features are there - like authentication).
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Inc
Hi,
Quoted 21 lines · David Sommerseth
Quoted 11 lines · Selva Nair
Hi,
...some good stuff snipped...
Quoted 6 lines
I'll admit I might see this with a bit too narrow perspective. But how I have understood this issue is that OpenVPN 2.x does not behave correctly as it doesn't understand *why* the authentication failed. If the client side would understand why auth failed, then it can query the user for credentials again - which I believe should resolve the current issues ... Or have I missed something?
Quoted 8 lines
I hope we are slowly spiralling towards a solution; not going around in circles...
Anyway, to reiterate: we currently have two issues. (i) client is not told when authentication fails during reneg
and (ii) client doesn't know that auth-gen-token's token is not reusable across reconnects
(SIGHUP and SIGUSR1).
Quoted 7 lines
Okay, (i) is handled with AUTH_FAILED,SESSION messages today in OpenVPN 3. OpenVPN 2 should be taught to do the same - both server and client side.
In regards to (ii), that is an implementation detail which I would say is incorrect with --auth-gen-token. As I am the one who implemented that feature, I would rather have this fixed so auth-gen-token works across restarts.
Well, IMO, fixing (i) is more important than (ii). It would also make it easy to add support for dynamic auth through scripts and plugin, for example. Currently not possible because a specially crafted "reason" for AUTH_FAILED cannot be sent back unless management-client-auth is used. Unrelated to auth-token but a sorely missing feature. That said, personally I prefer management-client-auth to using scripts and the former works fine as is.
Regarding (ii), reusing auth-token across SIGHUP and SIGUSR1 reconnects could have security implications. At the minimum we should treat auth-token as a high value asset on the client side and not send it to management (at least on the Windows GUI it currently shows up in plain text in the status log). In the current state of things, the client-side management has no use for the actual value of the token as it cant override the client daemon's handling of it.
Selva
The discussion has gone on a bit about this patch. I would like to step back and give an overview to make this mess better understandable as we have multiple problem mixed together.
Current client behaviour:
- (a) OpenVPN 3. Forgets auth-token on reconnect, can be told to forget auth-token during a session with AUTH_FAILED,SESSION
- (b) OpenVPN 2.x Once it gets an auth-token, the auth-token replace the password and the client will never anything else from that point on
Proposed new behaviour:
- (c) OpenVPN 2.x should behave exactly like OpenVPN 3
- (d) OpenVPN 2.x if not intrustucted otherwise will keep auth-token on reconnect but will forget it as soon as it gets an AUTH-FAILED (what my patch proposed)
Current OpenVPN server behaviour:
- (i) Custom client-connect script with auth-token can send an auth-token and can also check the same token on reconnect
- (ii) auth-gen-token sends an auth-token but the auth-token is only valid for the session. The token can also set to expiry earlier in which case the server fails to notify the client about th failed auth-token.
Currently working combinations are (i)+(a) and ii+b. OpenVpn 3 with custom script (i+b) will also work but will query for password/OTP password on every reconnect/network change.
Current problems:
- (1) OpenVPN 2.x server does only signal AUTH-FAILED on the initial tls session but not on later ones (I am ingoring the management special case here)
- (2) OpenVPN 2.x client does not understand AUTH_FAILED,SESSION
- (3) OpenVPN 2.x client (b) against an auth-gen-token server (ii):
Client will use the auth-token buth the server does not know anything anymore about the token and the authentication fails
Problem (1) and (2) are unproblematic as far as the discussion goes as there is no doubt that we should "just" implement them in 2.x.
Fixes for (3), client side:
- Using new behaviour (c), like OpenVPN 3. This will work but will make the custom script solution (i) worse and also has more real authentication on network changes. At least I see no reason why a token after an OTP password should be invalidated after a network change.
- Using new behaviour (d): Will continue to work with custom script as before. Will also be able to recover when connecting to to an *unpatched* auth-gen-token server but will need two attempts on a reconnect since the first fails with an AUTH_FAILED.
- In either (c) and (d) as new client behaviour, there should be a pushable option that allows the server to select the auth-token behaviour on reconnect (forget or keep)
Fixes for the server side for (3):
- implement persistent auth-tokens: Will allow old clients (b) to reconnect. Maybe change expire time to be "after not being used for x hours". Old clients will still fail to reconnect if the server is restarted unless tokens survive server restart.
- do not send auth-token to client that have behaviour (b). For this to happen the server needs to know if a client has behaviour (b) or not.
My suggestion here was to increase IV_PROTO to 3. The 3 here should only signal that the client has some way of dealng with an expiring token on reconnect and that the current behaviour of auth-gen-token is safe to use. It does not matter if the client recovers from AUTH-FAILED when a token is in use or if the client forgets the token. (minimal common behsaviour). IV_PROTO should also include "understand AUTH_FAILED,SESSION"
This has been a long mail but I hope it clear ups some of the confusion.
Arne
Hi,
Quoted 3 lines · Arne Schwabe
The discussion has gone on a bit about this patch. I would like to step back and give an overview to make this mess better understandable as we have multiple problem mixed together.
Since this summary is from March, and we've so far only fixed one aspect, but not these...
Quoted 2 lines
Problem (1) and (2) are unproblematic as far as the discussion goes as there is no doubt that we should "just" implement them in 2.x.
"just implement"... :-) -> I have opened a trac ticket as a reminder,
https://community.openvpn.net/openvpn/ticket/1147#ticket
gert
Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]...PATCH v3 1/1 Revert to original password authentication after failed auth-token
In reply to Arne SchwabeAuth-tokens can expire. For by reconnecting when the server uses auth-gen-toke.
Behaviour of OpenVPN client is to never fallback to the previous authentication method and continue using the auth-token. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
This patches changes the behaviour on failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method.
Patch V2: properly formatted commit message, fix openvpn3 detection
Patch V3: remove all server changes, include only minimal non intrusive client changes that only improve error recovery but don't change overall behaviour.
--- doc/openvpn.8 | 8 +++++++- src/openvpn/misc.c | 16 ++++++++++------ src/openvpn/misc.h | 3 +-- src/openvpn/ssl.c | 36 ++++++++++++++++++++++-------------- src/openvpn/ssl.h | 2 ++ 5 files changed, 42 insertions(+), 23 deletions(-)
doc/openvpn.8 +7 -1
@@ -5362,7 +5362,13 @@ push "auth\-token UNIQUE_TOKEN_VALUE" into the file/buffer for dynamic configuration data. This will then make the OpenVPN server to push this value to the client, which replaces the local password with the -UNIQUE_TOKEN_VALUE. +UNIQUE_TOKEN_VALUE. + +Newer clients (2.4.6+) will fall back to the original password method +after a failed auth. Older clients will keep using +the token value and react acording to +.B \-\-auth-retry +. .\"********************************************************* .TP .B \-\-tls\-verify cmd
src/openvpn/misc.c +10 -6
@@ -882,7 +882,7 @@ purge_user_pass(struct user_pass *up, const bool force)* don't show warning if the pass has been replaced by a token: this is an * artificial "auth-nocache" */ - else if (!warn_shown && (!up->tokenized)) + else if (!warn_shown) {msg (M_WARN ,"WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this" ); warn_shown = true;
@@ -890,14 +890,18 @@ purge_user_pass(struct user_pass *up, const bool force) }void -set_auth_token (struct user_pass *up,const char *token) +set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token) { -if (token &&strlen (token) && up && up->defined && !up->nocache) + +if (token &&strlen (token) && up && up->defined) { -CLEAR (up->password); -strncpynt (up->password, token,USER_PASS_LEN ); - up->tokenized = true; +strncpynt (tk->password, token,USER_PASS_LEN ); +strncpynt (tk->username, up->username,USER_PASS_LEN ); + tk->defined = true; } + +/* Cleans user/pass for nocache */ +purge_user_pass (up, false); }/*
src/openvpn/misc.h +1 -2
@@ -102,7 +102,6 @@ struct user_pass {bool defined;bool nocache; -bool tokenized;/* true if password has been substituted by a token */ bool wait_for_push;/* true if this object is waiting for a push-reply */ /* max length of username/password */
@@ -184,7 +183,7 @@ void fail_user_pass(const char *prefix,void purge_user_pass (struct user_pass *up,const bool force); -void set_auth_token (struct user_pass *up,const char *token); +void set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token);/* * Process string received by untrusted peer before
src/openvpn/ssl.c +22 -14
@@ -398,6 +398,7 @@ pem_password_callback(char *buf, int size, int rwflag, void *u)static bool auth_user_pass_enabled;/* GLOBAL */ static struct user_pass auth_user_pass;/* GLOBAL */ +static struct user_pass auth_token;/* GLOBAL */ #ifdef ENABLE_CLIENT_CR static char *auth_challenge;/* GLOBAL */
@@ -407,7 +408,7 @@ void auth_user_pass_setup(const char *auth_file, const struct static_challenge_info *sci) { auth_user_pass_enabled = true; -if (!auth_user_pass.defined) +if (!auth_user_pass.defined && !auth_token.defined) {#if AUTO_USERID get_user_pass_auto_userid (&auth_user_pass, auth_file);
@@ -449,7 +450,7 @@ ssl_set_auth_nocache(void) { passbuf.nocache = true; auth_user_pass.nocache = true; -/* wait for push-reply, because auth-token may invert nocache */ +/* wait for push-reply, because auth-token may still need the username */ auth_user_pass.wait_for_push = true; }
@@ -459,15 +460,18 @@ ssl_set_auth_nocache(void)void ssl_set_auth_token (const char *token) { -if (auth_user_pass.nocache) - { -msg (M_INFO , -"auth-token received, disabling auth-nocache for the " -"authentication token" ); - auth_user_pass.nocache = false; - } +set_auth_token (&auth_user_pass, &auth_token, token); +} -set_auth_token (&auth_user_pass, token); +/* +* Cleans an auth token and checks if it was active +*/ +bool +ssl_clean_auth_token (void ) +{ +bool wasdefined = auth_token.defined; +purge_user_pass (&auth_token, true); +return wasdefined; }/*
@@ -2377,19 +2381,23 @@ key_method_2_write(struct buffer *buf, struct tls_session *session)#else auth_user_pass_setup (session->opt->auth_user_pass_file,NULL );#endif -if (!write_string (buf, auth_user_pass.username,-1 )) +struct user_pass * up = &auth_user_pass; + +/* If we have a valid auth-token, send that instead of real username/password */ +if (auth_token.defined) + up = &auth_token; + +if (!write_string (buf, up->username,-1 )) { goto error; } -if (!write_string (buf, auth_user_pass.password,-1 )) +else if (!write_string (buf, up->password,-1 )) { goto error; }/* if auth-nocache was specified, the auth_user_pass object reaches * a "complete" state only after having received the push-reply * message. -* This is the case because auth-token statement in a push-reply would -* invert its nocache. * * For this reason, skip the purge operation here if no push-reply * message has been received yet.
src/openvpn/ssl.h +2 -0
@@ -428,6 +428,8 @@ void ssl_purge_auth(const bool auth_user_pass_only);void ssl_set_auth_token (const char *token); +bool ssl_clean_auth_token (void ); +#ifdef ENABLE_CLIENT_CR /* * ssl_get_auth_challenge will parse the server-pushed auth-failed
Signature · 1 line
2.16.2
PATCH v4 1/1 Revert to original password authentication after failed auth-token
In reply to Arne SchwabeAuth-tokens can expire. For by reconnecting when the server uses auth-gen-toke.
Behaviour of OpenVPN client is to never fallback to the previous authentication method and continue using the auth-token. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
This patches changes the behaviour on failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method.
Patch V2: properly formatted commit message, fix openvpn3 detection
Patch V3: remove all server changes, include only minimal non intrusive client changes that only improve error recovery but don't change overall behaviour.
Patch V4: forget add push.c to git index, now also included
--- doc/openvpn.8 | 8 +++++++- src/openvpn/misc.c | 16 ++++++++++------ src/openvpn/misc.h | 3 +-- src/openvpn/push.c | 16 ++++++++++++++-- src/openvpn/ssl.c | 36 ++++++++++++++++++++++-------------- src/openvpn/ssl.h | 2 ++ 6 files changed, 56 insertions(+), 25 deletions(-)
doc/openvpn.8 +7 -1
@@ -5362,7 +5362,13 @@ push "auth\-token UNIQUE_TOKEN_VALUE" into the file/buffer for dynamic configuration data. This will then make the OpenVPN server to push this value to the client, which replaces the local password with the -UNIQUE_TOKEN_VALUE. +UNIQUE_TOKEN_VALUE. + +Newer clients (2.4.6+) will fall back to the original password method +after a failed auth. Older clients will keep using +the token value and react acording to +.B \-\-auth-retry +. .\"********************************************************* .TP .B \-\-tls\-verify cmd
src/openvpn/misc.c +10 -6
@@ -882,7 +882,7 @@ purge_user_pass(struct user_pass *up, const bool force)* don't show warning if the pass has been replaced by a token: this is an * artificial "auth-nocache" */ - else if (!warn_shown && (!up->tokenized)) + else if (!warn_shown) {msg (M_WARN ,"WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this" ); warn_shown = true;
@@ -890,14 +890,18 @@ purge_user_pass(struct user_pass *up, const bool force) }void -set_auth_token (struct user_pass *up,const char *token) +set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token) { -if (token &&strlen (token) && up && up->defined && !up->nocache) + +if (token &&strlen (token) && up && up->defined) { -CLEAR (up->password); -strncpynt (up->password, token,USER_PASS_LEN ); - up->tokenized = true; +strncpynt (tk->password, token,USER_PASS_LEN ); +strncpynt (tk->username, up->username,USER_PASS_LEN ); + tk->defined = true; } + +/* Cleans user/pass for nocache */ +purge_user_pass (up, false); }/*
src/openvpn/misc.h +1 -2
@@ -102,7 +102,6 @@ struct user_pass {bool defined;bool nocache; -bool tokenized;/* true if password has been substituted by a token */ bool wait_for_push;/* true if this object is waiting for a push-reply */ /* max length of username/password */
@@ -184,7 +183,7 @@ void fail_user_pass(const char *prefix,void purge_user_pass (struct user_pass *up,const bool force); -void set_auth_token (struct user_pass *up,const char *token); +void set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token);/* * Process string received by untrusted peer before
src/openvpn/push.c +14 -2
@@ -55,8 +55,19 @@ receive_auth_failed(struct context *c, const struct buffer *buffer)if (c->options.pull) { -switch (auth_retry_get ()) +/* Before checking how to react on AUTH_FAILED, first check if the failed authed might be +* the result of an expired auth-token. +* Note that a server restart will trigger a generic AUTH_FAILED instead an AUTH_FAILED,SESSION so +* handle all AUTH_FAILED message identical for this scenario */ +if (ssl_clean_auth_token ()) { + c->sig->signal_received =SIGUSR1 ;/* SOFT-SIGUSR1 -- Auth failure error */ + c->sig->signal_text ="auth-failure (auth-token)" ; + } +else + { +switch (auth_retry_get ()) + {case AR_NONE : c->sig->signal_received =SIGTERM ;/* SOFT-SIGTERM -- Auth failure error */ break ;
@@ -70,8 +81,9 @@ receive_auth_failed(struct context *c, const struct buffer *buffer)default :ASSERT (0 ); + } + c->sig->signal_text ="auth-failure" ; } - c->sig->signal_text ="auth-failure" ;#ifdef ENABLE_MANAGEMENT if (management) {
src/openvpn/ssl.c +22 -14
@@ -398,6 +398,7 @@ pem_password_callback(char *buf, int size, int rwflag, void *u)static bool auth_user_pass_enabled;/* GLOBAL */ static struct user_pass auth_user_pass;/* GLOBAL */ +static struct user_pass auth_token;/* GLOBAL */ #ifdef ENABLE_CLIENT_CR static char *auth_challenge;/* GLOBAL */
@@ -407,7 +408,7 @@ void auth_user_pass_setup(const char *auth_file, const struct static_challenge_info *sci) { auth_user_pass_enabled = true; -if (!auth_user_pass.defined) +if (!auth_user_pass.defined && !auth_token.defined) {#if AUTO_USERID get_user_pass_auto_userid (&auth_user_pass, auth_file);
@@ -449,7 +450,7 @@ ssl_set_auth_nocache(void) { passbuf.nocache = true; auth_user_pass.nocache = true; -/* wait for push-reply, because auth-token may invert nocache */ +/* wait for push-reply, because auth-token may still need the username */ auth_user_pass.wait_for_push = true; }
@@ -459,15 +460,18 @@ ssl_set_auth_nocache(void)void ssl_set_auth_token (const char *token) { -if (auth_user_pass.nocache) - { -msg (M_INFO , -"auth-token received, disabling auth-nocache for the " -"authentication token" ); - auth_user_pass.nocache = false; - } +set_auth_token (&auth_user_pass, &auth_token, token); +} -set_auth_token (&auth_user_pass, token); +/* +* Cleans an auth token and checks if it was active +*/ +bool +ssl_clean_auth_token (void ) +{ +bool wasdefined = auth_token.defined; +purge_user_pass (&auth_token, true); +return wasdefined; }/*
@@ -2377,19 +2381,23 @@ key_method_2_write(struct buffer *buf, struct tls_session *session)#else auth_user_pass_setup (session->opt->auth_user_pass_file,NULL );#endif -if (!write_string (buf, auth_user_pass.username,-1 )) +struct user_pass * up = &auth_user_pass; + +/* If we have a valid auth-token, send that instead of real username/password */ +if (auth_token.defined) + up = &auth_token; + +if (!write_string (buf, up->username,-1 )) { goto error; } -if (!write_string (buf, auth_user_pass.password,-1 )) +else if (!write_string (buf, up->password,-1 )) { goto error; }/* if auth-nocache was specified, the auth_user_pass object reaches * a "complete" state only after having received the push-reply * message. -* This is the case because auth-token statement in a push-reply would -* invert its nocache. * * For this reason, skip the purge operation here if no push-reply * message has been received yet.
src/openvpn/ssl.h +2 -0
@@ -428,6 +428,8 @@ void ssl_purge_auth(const bool auth_user_pass_only);void ssl_set_auth_token (const char *token); +bool ssl_clean_auth_token (void ); +#ifdef ENABLE_CLIENT_CR /* * ssl_get_auth_challenge will parse the server-pushed auth-failed
Signature · 1 line
2.16.2
PATCH v5 1/1 Revert to original password authentication after failed auth-token
In reply to Arne SchwabeAuth-tokens can expire. For by reconnecting when the server uses auth-gen-toke.
Behaviour of OpenVPN client is to never fallback to the previous authentication method and continue using the auth-token. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
This patches changes the behaviour on failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method.
Patch V2: properly formatted commit message, fix openvpn3 detection
Patch V3: remove all server changes, include only minimal non intrusive client changes that only improve error recovery but don't change overall behaviour.
Patch V4: forget add push.c to git index, now also included
--- doc/openvpn.8 | 6 ++++++ src/openvpn/misc.c | 16 ++++++++++------ src/openvpn/misc.h | 4 ++-- src/openvpn/push.c | 17 +++++++++++++++-- src/openvpn/ssl.c | 39 +++++++++++++++++++++++++-------------- src/openvpn/ssl.h | 2 ++ 6 files changed, 60 insertions(+), 24 deletions(-)
doc/openvpn.8 +6 -0
@@ -5347,6 +5347,12 @@ into the file/buffer for dynamic configuration data. This will then make the OpenVPN server to push this value to the client, which replaces the local password with the UNIQUE_TOKEN_VALUE. + +Newer clients (2.4.7+) will fall back to the original password method +after a failed auth. Older clients will keep using the token value +and react acording to +.B \-\-auth-retry +. .\"********************************************************* .TP .B \-\-tls\-verify cmd
src/openvpn/misc.c +10 -6
@@ -469,7 +469,7 @@ purge_user_pass(struct user_pass *up, const bool force)* don't show warning if the pass has been replaced by a token: this is an * artificial "auth-nocache" */ - else if (!warn_shown && (!up->tokenized)) + else if (!warn_shown) {msg (M_WARN ,"WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this" ); warn_shown = true;
@@ -477,14 +477,18 @@ purge_user_pass(struct user_pass *up, const bool force) }void -set_auth_token (struct user_pass *up,const char *token) +set_auth_token (struct user_pass *up,struct user_pass *tk,const char *token) { -if (token &&strlen (token) && up && up->defined && !up->nocache) + +if (token &&strlen (token) && up && up->defined) { -CLEAR (up->password); -strncpynt (up->password, token,USER_PASS_LEN ); - up->tokenized = true; +strncpynt (tk->password, token,USER_PASS_LEN ); +strncpynt (tk->username, up->username,USER_PASS_LEN ); + tk->defined = true; } + +/* Cleans user/pass for nocache */ +purge_user_pass (up, false); }/*
src/openvpn/misc.h +2 -2
@@ -63,7 +63,6 @@ struct user_pass {bool defined;bool nocache; -bool tokenized;/* true if password has been substituted by a token */ bool wait_for_push;/* true if this object is waiting for a push-reply */ /* max length of username/password */
@@ -145,7 +144,8 @@ void fail_user_pass(const char *prefix,void purge_user_pass (struct user_pass *up,const bool force); -void set_auth_token (struct user_pass *up,const char *token); +void set_auth_token (struct user_pass *up,struct user_pass *tk, +const char *token);/* * Process string received by untrusted peer before
src/openvpn/push.c +15 -2
@@ -55,8 +55,20 @@ receive_auth_failed(struct context *c, const struct buffer *buffer)if (c->options.pull) { -switch (auth_retry_get ()) +/* Before checking how to react on AUTH_FAILED, first check if the +* failed auth might be the result of an expired auth-token. +* Note that a server restart will trigger a generic AUTH_FAILED +* instead an AUTH_FAILED,SESSION so handle all AUTH_FAILED message +* identical for this scenario */ +if (ssl_clean_auth_token ()) { + c->sig->signal_received =SIGUSR1 ;/* SOFT-SIGUSR1 -- Auth failure error */ + c->sig->signal_text ="auth-failure (auth-token)" ; + } +else + { +switch (auth_retry_get ()) + {case AR_NONE : c->sig->signal_received =SIGTERM ;/* SOFT-SIGTERM -- Auth failure error */ break ;
@@ -70,8 +82,9 @@ receive_auth_failed(struct context *c, const struct buffer *buffer)default :ASSERT (0 ); + } + c->sig->signal_text ="auth-failure" ; } - c->sig->signal_text ="auth-failure" ;#ifdef ENABLE_MANAGEMENT if (management) {
src/openvpn/ssl.c +25 -14
@@ -398,6 +398,7 @@ pem_password_callback(char *buf, int size, int rwflag, void *u)static bool auth_user_pass_enabled;/* GLOBAL */ static struct user_pass auth_user_pass;/* GLOBAL */ +static struct user_pass auth_token;/* GLOBAL */ #ifdef ENABLE_MANAGEMENT static char *auth_challenge;/* GLOBAL */
@@ -407,7 +408,7 @@ void auth_user_pass_setup(const char *auth_file, const struct static_challenge_info *sci) { auth_user_pass_enabled = true; -if (!auth_user_pass.defined) +if (!auth_user_pass.defined && !auth_token.defined) {#ifdef ENABLE_MANAGEMENT if (auth_challenge)/* dynamic challenge/response */
@@ -445,7 +446,7 @@ ssl_set_auth_nocache(void) { passbuf.nocache = true; auth_user_pass.nocache = true; -/* wait for push-reply, because auth-token may invert nocache */ +/* wait for push-reply, because auth-token may still need the username */ auth_user_pass.wait_for_push = true; }
@@ -455,15 +456,18 @@ ssl_set_auth_nocache(void)void ssl_set_auth_token (const char *token) { -if (auth_user_pass.nocache) - { -msg (M_INFO , -"auth-token received, disabling auth-nocache for the " -"authentication token" ); - auth_user_pass.nocache = false; - } +set_auth_token (&auth_user_pass, &auth_token, token); +} -set_auth_token (&auth_user_pass, token); +/* +* Cleans an auth token and checks if it was active +*/ +bool +ssl_clean_auth_token (void ) +{ +bool wasdefined = auth_token.defined; +purge_user_pass (&auth_token, true); +return wasdefined; }/*
@@ -2369,19 +2373,26 @@ key_method_2_write(struct buffer *buf, struct tls_session *session)#else auth_user_pass_setup (session->opt->auth_user_pass_file,NULL );#endif -if (!write_string (buf, auth_user_pass.username,-1 )) +struct user_pass *up = &auth_user_pass; + +/* +* If we have a valid auth-token, send that instead of real +* username/password +*/ +if (auth_token.defined) + up = &auth_token; + +if (!write_string (buf, up->username,-1 )) { goto error; } -if (!write_string (buf, auth_user_pass.password,-1 )) +else if (!write_string (buf, up->password,-1 )) { goto error; }/* if auth-nocache was specified, the auth_user_pass object reaches * a "complete" state only after having received the push-reply * message. -* This is the case because auth-token statement in a push-reply would -* invert its nocache. * * For this reason, skip the purge operation here if no push-reply * message has been received yet.
src/openvpn/ssl.h +2 -0
@@ -437,6 +437,8 @@ void ssl_set_auth_token(const char *token);*/ void ssl_purge_auth_challenge (void ); +bool ssl_clean_auth_token (void ); +void ssl_put_auth_challenge (const char *cr_str);#endif
Signature · 1 line
2.19.0
Re: [Openvpn-devel] [PATCH v5] Revert to original password authentication after failed auth-token
Am 10.10.18 um 16:30 schrieb Arne Schwabe:
Quoted 13 lines
Auth-tokens can expire. For by reconnecting when the server uses auth-gen-toke.
Behaviour of OpenVPN client is to never fallback to the previous authentication method and continue using the auth-token. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
This patches changes the behaviour on failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method.
Quoted 7 lines
Patch V2: properly formatted commit message, fix openvpn3 detection
Patch V3: remove all server changes, include only minimal non intrusive client changes that only improve error recovery but don't change overall behaviour.
Patch V4: forget add
push.cto git index, now also included
Patch V5: is fixing overlong lines and one minor style problem.
Arne
Re: [Openvpn-devel] [PATCH v5] Revert to original password authentication after failed auth-token
In reply to Arne SchwabeHi all,
Quoted 3 lines · Arne Schwabe
Auth-tokens can expire. For by reconnecting when the server uses auth-gen-toke.
The sentence above should be adjusted a bit before the patch is merged.
Quoted 12 lines
Behaviour of OpenVPN client is to never fallback to the previous authentication method and continue using the auth-token. Depending on auth-retry it either quit or tried endlessly with an expired token. Since auth-gen-token tokens expire on reconnect, a client will not survive a reconnect.
This patches changes the behaviour on failed auth when using an auth-token as a soft error (USR1) and clean the auth-token falling back to the original auth method.
Patch V2: properly formatted commit message, fix openvpn3 detection
Quoted 7 lines
Patch V3: remove all server changes, include only minimal non intrusive client changes that only improve error recovery but don't change overall behaviour.
Patch V4: forget add
push.cto git index, now also includedSigned-off-by Arne Schwabe <[email protected]...>
The patch does what it says and it nicely separates where the "password" and the "token" are stored.
This has the advantage of making the code slightly easier to follow.
I performed a simple test with a server and a client configured for using auth-gen-token:
- client connected to the server
- authentication with user/pass is successful and I see the token being pushed
- server is restarted
- client waits its ping timeout and then softly restarts
- client fails with AUTH_FAILED:
Fri Dec 7 17:03:08 2018 us=203818 AUTH: Received control message:AUTH_FAILED
Fri Dec 7 17:03:08 2018 us=204210 TCP/UDP: Closing socket Fri Dec 7 17:03:08 2018 us=204322 SIGUSR1[soft,auth-failure (auth-token)] received, process restarting
Fri Dec 7 17:03:08 2018 us=204391 Restart pause, 5 second(s)
- client softly reconnects
- authentication is successful with user/pass again and a new token is pushed.
Signature · 1 line
Antonio Quartulli
[Openvpn-devel] [PATCH applied] Re: Fallback to password authentication when auth-token fails
In reply to Arne SchwabeYour patch has been applied to the master and release/2.4 branch.
Commit messages amended according to the comittee proposal ;-)
Uncrustify will complain to us that the switch/case block in push.c grew an extra nesting level but indentation didn't change... but that's just whitespace, so fixed on the next uncrustify run.
I haven't really reviewed the code - trusting Antonio here. (I did look over the changes and didn't see anything glaringly broken, and it passes full t_client tests on both 2.4 and master - though these are not doing auth token tests today)
commit e61b401ac50d2a9cfabf0289811ad14cf3bd2751 (master) commit 004f13b60d87fe7815f4feee9aded22ae4eacbaf (release/2.4)
Author: Arne Schwabe Date: Wed Oct 10 16:30:51 2018 +0200
Fallback to password authentication when auth-token fails
Signed-off-by: Arne Schwabe <[email protected]...>
Acked-by: Antonio Quartulli <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg17718.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering