auth-gen-token: Inform client why auth-token was rejected
cover auth-gen-token: Inform client why auth-token was rejected
Previously when a client connection got closed due to the auth-token being rejected, the client did not receive any indication why. This patch-set ensures the client will receive such an indication.
The AUTH_FAILED statement which is used to reject and close client connections can contain a text string with a message aimed at the user. Some clients (like OpenVPN Connect) have also some kind of categorization. For this use-case, the SESSION category was chosen.
The rejection message to the client is sent using the send_auth_failed() function. This ensures the formatting is proper. The only detail we need to ensure is that the category and the message is separated by a colon (:).
This patch-set is split into two parts. The first parts refactors the code to provide the struct context object the whole way into key_method_2_read(). This is the place where verify_user_pass() is called. The reason we need the struct context object is that send_auth_failed() needs that to prepare a message to be pushed to the client.
The second patch just implements the rejection message and calling send_auth_failed() with the appropriate messages.
David Sommerseth (2):
Refactor to provide struct context object inside key_method_2_read()
auth-gen-token: Provide a rejection message back to the client on
failure
src/openvpn/forward.c | 2 +- src/openvpn/ssl.c | 30 +++++++++++++++++------------- src/openvpn/ssl.h | 6 +----- src/openvpn/ssl_verify.c | 13 +++++++++---- src/openvpn/ssl_verify.h | 2 +- 5 files changed, 29 insertions(+), 24 deletions(-)
Signature · 1 line
1.8.3.1
PATCH 1/2 Refactor to provide struct context object inside key_method_2_read()
This refactors the call chain of tls_multi_process(), tls_process() and key_method_2_read() so the the latter function have access to the struct context object.
This change is needed for the following patch where verify_user_pass() will need to call send_auth_failed(), which depends on direct access to the the context object.
--- src/openvpn/forward.c | 2 +- src/openvpn/ssl.c | 28 ++++++++++++++++------------ src/openvpn/ssl.h | 6 +----- 3 files changed, 18 insertions(+), 18 deletions(-)
src/openvpn/forward.c +1 -1
@@ -96,7 +96,7 @@ check_tls_dowork (struct context *c)if (interval_test (&c->c2.tmp_int)) { -const int tmp_status =tls_multi_process +const int tmp_status =tls_multi_process (c, &wakeup); (c->c2.tls_multi, &c->c2.to_link, &c->c2.to_link_addr,get_link_socket_info (c), &wakeup);if (tmp_status ==TLSMP_ACTIVE )
src/openvpn/ssl.c +16 -12
@@ -58,6 +58,8 @@#include "pkcs11.h" #include "route.h" +#include "forward.h" +#include "forward-inline.h" #include "ssl.h" #include "ssl_verify.h" #include "ssl_backend.h"
@@ -2191,8 +2193,9 @@ key_method_1_read (struct buffer *buf, struct tls_session *session) }static bool -key_method_2_read (struct buffer *buf,struct tls_multi *multi,struct tls_session *session) +key_method_2_read (struct buffer *buf,struct context *c,struct tls_session *session) { +struct tls_multi *multi = c->c2.tls_multi;struct key_state *ks = &session->key[KS_PRIMARY ];/* primary key */ int key_method_flags;
@@ -2382,11 +2385,10 @@ auth_deferred_expire_window (const struct tls_options *o)* want to send to our peer. */ static bool -tls_process (struct tls_multi *multi, +tls_process (struct context *c,struct tls_session *session,struct buffer *to_link,struct link_socket_actual **to_link_addr, -struct link_socket_info *to_link_socket_info,interval_t *wakeup) {struct gc_arena gc =gc_new ();
@@ -2395,6 +2397,8 @@ tls_process (struct tls_multi *multi,bool active = false;struct key_state *ks = &session->key[KS_PRIMARY ];/* primary key */ struct key_state *ks_lame = &session->key[KS_LAME_DUCK ];/* retiring key */ +struct tls_multi *multi = c->c2.tls_multi; +struct link_socket_info *to_link_socket_info =get_link_socket_info (c);/* Make sure we were initialized and that we're not in an error state */ ASSERT (ks->state !=S_UNDEF );
@@ -2425,6 +2429,7 @@ tls_process (struct tls_multi *multi,msg (D_TLS_DEBUG_LOW ,"TLS: tls_process: killed expiring key" ); } + do { update_time ();
@@ -2654,7 +2659,7 @@ tls_process (struct tls_multi *multi, }else if (session->opt->key_method ==2 ) { -if (!key_method_2_read (buf, multi, session)) +if (!key_method_2_read (buf, c, session)) goto error; }else
@@ -2775,11 +2780,7 @@ error:*/ int -tls_multi_process (struct tls_multi *multi, -struct buffer *to_link, -struct link_socket_actual **to_link_addr, -struct link_socket_info *to_link_socket_info, -interval_t *wakeup) +tls_multi_process (struct context *c,interval_t *wakeup) {struct gc_arena gc =gc_new ();int i;
@@ -2787,6 +2788,8 @@ tls_multi_process (struct tls_multi *multi,bool error = false;int tas; +ASSERT (c !=NULL ); +perf_push (PERF_TLS_MULTI_PROCESS );tls_clear_error ();
@@ -2795,6 +2798,8 @@ tls_multi_process (struct tls_multi *multi,* Process each session object having state of S_INITIAL or greater, * and which has a defined remote IP addr. */ +struct tls_multi *multi = c->c2.tls_multi; +struct link_socket_info *to_link_socket_info =get_link_socket_info (c);for (i =0 ; i <TM_SIZE ; ++i) {
@@ -2821,8 +2826,7 @@ tls_multi_process (struct tls_multi *multi,update_time (); -if (tls_process (multi, session, to_link, &tla, - to_link_socket_info, wakeup)) +if (tls_process (c, session, &c->c2.to_link, &tla, wakeup)) active =TLSMP_ACTIVE ;/*
@@ -2833,7 +2837,7 @@ tls_multi_process (struct tls_multi *multi,if (tla) { multi->to_link_addr = *tla; - *to_link_addr = &multi->to_link_addr; + c->c2.to_link_addr = &multi->to_link_addr; }/*
src/openvpn/ssl.h +1 -5
@@ -244,11 +244,7 @@ void tls_multi_free (struct tls_multi *multi, bool clear);* Basically decides if we should call tls_process for * the active or untrusted sessions. */ -int tls_multi_process (struct tls_multi *multi, -struct buffer *to_link, -struct link_socket_actual **to_link_addr, -struct link_socket_info *to_link_socket_info, -interval_t *wakeup); +int tls_multi_process (struct context *c,interval_t *wakeup);/**************************************************************************/
Signature · 1 line
1.8.3.1
A message that is not in the archive (sent off-list or not yet received): [email protected]...
Re: [Openvpn-devel] [PATCH 1/2] Refactor to provide struct context object inside key_method_2_read()
Quoted 34 lines · Arne Schwabe
Am 31.10.16 um 13:51 schrieb David Sommerseth:
Quoted 20 lines
This refactors the call chain of
tls_multi_process(),tls_process()andkey_method_2_read()so the the latter function have access to the struct context object.This change is needed for the following patch where
verify_user_pass()will need to callsend_auth_failed(), which depends on direct access to the the context object.Signed-off-by David Sommerseth <[email protected]...>--- src/openvpn/forward.c | 2 +- src/openvpn/ssl.c | 28 ++++++++++++++++------------ src/openvpn/ssl.h | 6 +----- 3 files changed, 18 insertions(+), 18 deletions(-)src/openvpn/forward.c +0
-0src/openvpn/forward.c check_tls_dowork (struct context *c) · line 101
Quoted 7 lines
src/openvpn/forward.c +1
-1@@ -96,7 +96,7 @@ check_tls_dowork (struct context *c)if (interval_test (&c->c2.tmp_int)) { -const int tmp_status = tls_multi_process +const int tmp_status =tls_multi_process (c, &wakeup); (c->c2.tls_multi, &c->c2.to_link, &c->c2.to_link_addr,This part of the patch looks just wrong. The
src/openvpn/forward.c +0
-0(c->c2.tls_multi, &c->c2.to_link, &c->c2.to_link_addr,get_link_socket_info (c), &wakeup);
Quoted 1 line
will stand alone.
Yeah, that gotta be a commit error. That said, I remember Steffan not agreeing with this re-factoring approach. As, we're passing quite some data around in these large and more global structs. We need to look into how to make send_auth_failed() work in a smaller set of context than the main 'c' context.
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Inc
PATCH 2/2 auth-gen-token: Provide a rejection message back to the client on failure
In reply to David SommersethThis improves the client experience when a auth-token is rejected or has expired. The message provided should be visible in all clients supporting parsing of AUTH_FAILED push messages.
--- src/openvpn/ssl.c | 2 +- src/openvpn/ssl_verify.c | 13 +++++++++---- src/openvpn/ssl_verify.h | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-)
src/openvpn/ssl.c +1 -1
@@ -2278,7 +2278,7 @@ key_method_2_read (struct buffer *buf, struct context *c, struct tls_session *se } } -verify_user_pass (up, multi, session); +verify_user_pass (up, c, session); }else {
src/openvpn/ssl_verify.c +9 -4
@@ -37,8 +37,10 @@#ifdef ENABLE_CRYPTO +#include "openvpn.h" #include "misc.h" #include "manage.h" +#include "push.h" #include "otime.h" #include "base64.h" #include "ssl_verify.h"
@@ -1107,12 +1109,13 @@ verify_user_pass_management (struct tls_session *session, const struct user_pass* Main username/password verification entry point */ void -verify_user_pass (struct user_pass *up,struct tls_multi *multi, +verify_user_pass (struct user_pass *up,struct context *c,struct tls_session *session) {int s1 =OPENVPN_PLUGIN_FUNC_SUCCESS ;bool s2 = true;struct key_state *ks = &session->key[KS_PRIMARY ];/* primary key */ +struct tls_multi *multi = c->c2.tls_multi;struct gc_arena gc =gc_new ();char *raw_username =NULL ;
@@ -1163,7 +1166,9 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, && (multi->auth_token_tstamp + session->opt->auth_token_lifetime) < now) {msg (D_HANDSHAKE ,"Auth-token for client expired\n" ); - ks->authenticated = false; +send_auth_failed (c, +"SESSION: Your session has expired," +" please reauthenticate" ); goto done; }
@@ -1175,12 +1180,12 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,free (multi->auth_token); multi->auth_token =NULL ; multi->auth_token_sent = false; - ks->authenticated = false; -tls_deauthenticate (multi);msg (D_TLS_ERRORS ,"TLS Auth Error: Auth-token verification " "failed for username '%s' %s" , up->username, (ssl_flags &SSLF_USERNAME_AS_COMMON_NAME ) ?"[CN SET]" :"" ); +send_auth_failed (c,"SESSION: The authentication token was invalid, " +"please reauthenticate" ); }else {
src/openvpn/ssl_verify.h +1 -1
@@ -187,7 +187,7 @@ tls_common_name_hash (const struct tls_multi *multi, const char **cn, uint32_t ** @param session The current TLS session * */ -void verify_user_pass (struct user_pass *up,struct tls_multi *multi, +void verify_user_pass (struct user_pass *up,struct context *c,struct tls_session *session);/**
Signature · 1 line
1.8.3.1