From 3d18e308c4e7e6f7ab7c2826c70d2d07b031c18a Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Sat, 27 Mar 2021 19:35:44 +0100 Subject: [PATCH] Ensure auth-token is only sent on a fully authenticated session This fixes the problem that if client authentication is deferred, we send an updated token before the authentication fully finished. Calling the new ssl_session_fully_authenticated from the two places that do the state transition to KS_AUTH_TRUE is a bit suboptimal but a cleaner solution requires more refactoring of the involved methods and state machines. This bug allows - under very specific circumstances - to trick a server using delayed authentication (plugin or management) *and* "--auth-gen-token" into returning a PUSH_REPLY before the AUTH_FAILED message, which can possibly be used to gather information about a VPN setup or even get access to a VPN with an otherwise-invalid account. CVE-2020-15078 has been assigned to acknowledge this risk. CVE: 2020-15078 Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli Message-Id: URL: non-public, embargoed Signed-off-by: Gert Doering --- src/openvpn/ssl_verify.c | 64 +++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 6fd51505e..55e7fedc0 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -906,6 +906,39 @@ key_state_test_auth_control_file(struct key_state *ks) #endif /* ifdef PLUGIN_DEF_AUTH */ +/* This function is called when a session's primary key state first becomes KS_TRUE */ +void ssl_session_fully_authenticated(struct tls_multi *multi, struct tls_session* session) +{ + struct key_state *ks = &session->key[KS_PRIMARY]; + if (ks->key_id == 0) + { + /* A key id of 0 indicates a new session and the client will + * get the auth-token as part of the initial push reply */ + return; + } + + /* + * Auth token already sent to client, update auth-token on client. + * The initial auth-token is sent as part of the push message, for this + * update we need to schedule an extra push message. + * + * Otherwise the auth-token get pushed out as part of the "normal" + * push-reply + */ + if (multi->auth_token_initial) + { + /* + * We do not explicitly schedule the sending of the + * control message here but control message are only + * postponed when the control channel is not yet fully + * established and furthermore since this is called in + * the middle of authentication, there are other messages + * (new data channel keys) that are sent anyway and will + * trigger scheduling + */ + send_push_reply_auth_token(multi); + } +} /* * Return current session authentication state. Return * value is TLS_AUTHENTICATION_x. @@ -975,6 +1008,12 @@ tls_authentication_status(struct tls_multi *multi, const int latency) case ACF_SUCCEEDED: case ACF_DISABLED: success = true; + /* i=0 is the TM_ACTIVE/KS_PRIMARY session */ + if (i == 0 && ks->authenticated == KS_AUTH_DEFERRED) + { + ssl_session_fully_authenticated(multi, + &multi->session[TM_ACTIVE]); + } ks->authenticated = KS_AUTH_TRUE; break; @@ -1385,31 +1424,14 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, */ generate_auth_token(up, multi); } - /* - * Auth token already sent to client, update auth-token on client. - * The initial auth-token is sent as part of the push message, for this - * update we need to schedule an extra push message. - * - * Otherwise the auth-token get pushed out as part of the "normal" - * push-reply - */ - if (multi->auth_token_initial) - { - /* - * We do not explicitly schedule the sending of the - * control message here but control message are only - * postponed when the control channel is not yet fully - * established and furthermore since this is called in - * the middle of authentication, there are other messages - * (new data channel keys) that are sent anyway and will - * trigger schedueling - */ - send_push_reply_auth_token(multi); - } msg(D_HANDSHAKE, "TLS: Username/Password authentication %s for username '%s' %s", (ks->authenticated == KS_AUTH_DEFERRED) ? "deferred" : "succeeded", up->username, (session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME) ? "[CN SET]" : ""); + if (ks->authenticated == KS_AUTH_TRUE) + { + ssl_session_fully_authenticated(multi, session); + } } else {