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 <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <d25ec73f-2ab0-31df-8cb6-7778000f4822@openvpn.net>
URL: non-public, embargoed
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 6fd51505e23b3cc335f16f1e97d6604ac5437953..55e7fedc0fccdb9d9ce69f17cfb82b5929c689e6 100644 (file)
--- 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
     {