Move auth_token_state from multi to key_state

The auth-token check is tied to the username/password that is coming
via a specific SSL session, so keep the state also in the key_state
structure.

This also ensures the auth_token_state is always set to 0 on a new
session since we clear the key_state object at the start of a new
SSL session.

This is a prerequisite patch to fix 2020-15078 in the following two
commits.

This also applies the changes to the auth_token_test.c. The change of
tls_session to a pointer is necessary since before that we had tls_session
not tied to the multi and had two tls_session used in the test. One
implicitly in tls_multi and one explicit one. Merge these to one.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210520151148.2565578-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22415.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index d571b686e0cd373a3fba9a1d1026629b5ddd6cd2..0ea6d18328842458b1c3e66269efaf9bec817ee9 100644 (file)
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -57,6 +57,7 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
         return;
     }
 
+    int auth_token_state_flags = session->key[KS_PRIMARY].auth_token_state_flags;
 
     const char *state;
 
@@ -64,9 +65,9 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
     {
         state = "Initial";
     }
-    else if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
+    else if (auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
     {
-        switch (multi->auth_token_state_flags & (AUTH_TOKEN_VALID_EMPTYUSER|AUTH_TOKEN_EXPIRED))
+        switch (auth_token_state_flags & (AUTH_TOKEN_VALID_EMPTYUSER|AUTH_TOKEN_EXPIRED))
         {
             case 0:
                 state = "Authenticated";
@@ -98,8 +99,8 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
 
     /* We had a valid session id before */
     const char *session_id_source;
-    if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK
-        && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
+    if (auth_token_state_flags & AUTH_TOKEN_HMAC_OK
+        && !(auth_token_state_flags & AUTH_TOKEN_EXPIRED))
     {
         session_id_source = up->password;
     }
@@ -236,7 +237,8 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
      * a new token with the empty username since we do not want to loose
      * the information that the username cannot be trusted
      */
-    if (multi->auth_token_state_flags & AUTH_TOKEN_VALID_EMPTYUSER)
+    struct key_state *ks = &multi->session[TM_ACTIVE].key[KS_PRIMARY];
+    if (ks->auth_token_state_flags & AUTH_TOKEN_VALID_EMPTYUSER)
     {
         hmac_ctx_update(ctx, (const uint8_t *) "", 0);
     }

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 9552a0f6c40ef4bdc3152f0f7f25e962658712e6..845d2243d09de35aac4386540afcc6664a73211a 100644 (file)
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -182,6 +182,8 @@ enum auth_deferred_result {
 struct key_state
 {
     int state;
+    /** The state of the auth-token sent from the client */
+    int auth_token_state_flags;
 
     /**
      * Key id for this key_state,  inherited from struct tls_session.
@@ -598,8 +600,6 @@ struct tls_multi
      * OpenVPN 3 clients sometimes wipes or replaces the username with a
      * username hint from their config.
      */
-    int auth_token_state_flags;
-    /**< The state of the auth-token sent from the client last time */
 
     /* For P_DATA_V2 */
     uint32_t peer_id;

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 74b418f2d78b239c64621765b3d999a43364ea71..10b555ceb479ba7866a0636e433057a0972ee98a 100644 (file)
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1484,7 +1484,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
      */
     if (session->opt->auth_token_generate && is_auth_token(up->password))
     {
-        multi->auth_token_state_flags = verify_auth_token(up, multi, session);
+        ks->auth_token_state_flags = verify_auth_token(up, multi, session);
         if (session->opt->auth_token_call_auth)
         {
             /*
@@ -1493,7 +1493,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
              * decide what to do with the result
              */
         }
-        else if (multi->auth_token_state_flags == AUTH_TOKEN_HMAC_OK)
+        else if (ks->auth_token_state_flags == AUTH_TOKEN_HMAC_OK)
         {
             /*
              * We do not want the EXPIRED or EMPTY USER flags here so check
@@ -1592,8 +1592,8 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
              * the initial timestamp and session id can be extracted from it
              */
             if (!multi->auth_token
-                && (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
-                && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
+                && (ks->auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
+                && !(ks->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
             {
                 multi->auth_token = strdup(up->password);
             }

diff --git a/tests/unit_tests/openvpn/test_auth_token.c b/tests/unit_tests/openvpn/test_auth_token.c
index 3f2f7714034120d0662212985f17b419c191694b..c75b00d81aee7485a74d1e37baadbe9cdcd335d0 100644 (file)
--- a/tests/unit_tests/openvpn/test_auth_token.c
+++ b/tests/unit_tests/openvpn/test_auth_token.c
@@ -45,7 +45,7 @@ struct test_context {
     struct tls_multi multi;
     struct key_type kt;
     struct user_pass up;
-    struct tls_session session;
+    struct tls_session *session;
 };
 
 /* Dummy functions that do nothing to mock the functionality */
@@ -100,10 +100,11 @@ setup(void **state)
     }
     ctx->multi.opt.auth_token_generate = true;
     ctx->multi.opt.auth_token_lifetime = 3000;
+    ctx->session = &ctx->multi.session[TM_ACTIVE];
 
-    ctx->session.opt = calloc(1, sizeof(struct tls_options));
-    ctx->session.opt->renegotiate_seconds = 120;
-    ctx->session.opt->auth_token_lifetime = 3000;
+    ctx->session->opt = calloc(1, sizeof(struct tls_options));
+    ctx->session->opt->renegotiate_seconds = 120;
+    ctx->session->opt->auth_token_lifetime = 3000;
 
     strcpy(ctx->up.username, "test user name");
     strcpy(ctx->up.password, "ignored");
@@ -122,7 +123,7 @@ teardown(void **state)
     free_key_ctx(&ctx->multi.opt.auth_token_key);
     wipe_auth_token(&ctx->multi);
 
-    free(ctx->session.opt);
+    free(ctx->session->opt);
     free(ctx);
 
     return 0;
@@ -135,7 +136,7 @@ auth_token_basic_test(void **state)
 
     generate_auth_token(&ctx->up, &ctx->multi);
     strcpy(ctx->up.password, ctx->multi.auth_token);
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 }
 
@@ -146,7 +147,7 @@ auth_token_fail_invalid_key(void **state)
 
     generate_auth_token(&ctx->up, &ctx->multi);
     strcpy(ctx->up.password, ctx->multi.auth_token);
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 
     /* Change auth-token key */
@@ -155,13 +156,13 @@ auth_token_fail_invalid_key(void **state)
     free_key_ctx(&ctx->multi.opt.auth_token_key);
     init_key_ctx(&ctx->multi.opt.auth_token_key, &key, &ctx->kt, false, "TEST");
 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session), 0);
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session), 0);
 
     /* Load original test key again */
     memset(&key, 0, sizeof(key));
     free_key_ctx(&ctx->multi.opt.auth_token_key);
     init_key_ctx(&ctx->multi.opt.auth_token_key, &key, &ctx->kt, false, "TEST");
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 
 }
@@ -176,32 +177,32 @@ auth_token_test_timeout(void **state)
     strcpy(ctx->up.password, ctx->multi.auth_token);
 
     /* No time has passed */
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 
     /* Token before validity, should be rejected */
     now = 100000 - 100;
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
 
     /* Token still in validity, should be accepted */
-    now = 100000 + 2*ctx->session.opt->renegotiate_seconds - 20;
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    now = 100000 + 2*ctx->session->opt->renegotiate_seconds - 20;
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 
     /* Token past validity, should be rejected */
-    now = 100000 + 2*ctx->session.opt->renegotiate_seconds + 20;
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    now = 100000 + 2*ctx->session->opt->renegotiate_seconds + 20;
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
 
     /* Check if the mode for a client that never updates its token works */
     ctx->multi.auth_token_initial = strdup(ctx->up.password);
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 
     /* But not when we reached our timeout */
-    now = 100000 + ctx->session.opt->auth_token_lifetime + 1;
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    now = 100000 + ctx->session->opt->auth_token_lifetime + 1;
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
 
     free(ctx->multi.auth_token_initial);
@@ -209,22 +210,22 @@ auth_token_test_timeout(void **state)
 
     /* regenerate the token util it hits the expiry */
     now = 100000;
-    while (now < 100000 + ctx->session.opt->auth_token_lifetime + 1)
+    while (now < 100000 + ctx->session->opt->auth_token_lifetime + 1)
     {
-        assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+        assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                          AUTH_TOKEN_HMAC_OK);
         generate_auth_token(&ctx->up, &ctx->multi);
         strcpy(ctx->up.password, ctx->multi.auth_token);
-        now += ctx->session.opt->renegotiate_seconds;
+        now += ctx->session->opt->renegotiate_seconds;
     }
 
 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
     ctx->multi.opt.auth_token_lifetime = 0;
 
     /* Non expiring token should be fine */
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 }
 
@@ -253,7 +254,7 @@ auth_token_test_known_keys(void **state)
     assert_string_equal(now0key0, ctx->multi.auth_token);
 
     strcpy(ctx->up.password, ctx->multi.auth_token);
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 }
 
@@ -277,25 +278,25 @@ auth_token_test_empty_user(void **state)
 
     generate_auth_token(&ctx->up, &ctx->multi);
     strcpy(ctx->up.password, ctx->multi.auth_token);
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK);
 
     now = 100000;
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
     strcpy(ctx->up.username, "test user name");
 
     now = 0;
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_VALID_EMPTYUSER);
 
     strcpy(ctx->up.username, "test user name");
     now = 100000;
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED|AUTH_TOKEN_VALID_EMPTYUSER);
 
     zerohmac(ctx->up.password);
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
                      0);
 }
 
@@ -304,30 +305,32 @@ auth_token_test_env(void **state)
 {
     struct test_context *ctx = (struct test_context *) *state;
 
-    ctx->multi.auth_token_state_flags = 0;
+    struct key_state *ks = &ctx->multi.session[TM_ACTIVE].key[KS_PRIMARY];
+
+    ks->auth_token_state_flags = 0;
     ctx->multi.auth_token = NULL;
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
     assert_string_equal(lastsesion_statevalue, "Initial");
 
-    ctx->multi.auth_token_state_flags = 0;
+    ks->auth_token_state_flags = 0;
     strcpy(ctx->up.password, now0key0);
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
     assert_string_equal(lastsesion_statevalue, "Invalid");
 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK;
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK;
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
     assert_string_equal(lastsesion_statevalue, "Authenticated");
 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED;
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED;
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
     assert_string_equal(lastsesion_statevalue, "Expired");
 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_VALID_EMPTYUSER;
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_VALID_EMPTYUSER;
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
     assert_string_equal(lastsesion_statevalue, "AuthenticatedEmptyUser");
 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED|AUTH_TOKEN_VALID_EMPTYUSER;
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED|AUTH_TOKEN_VALID_EMPTYUSER;
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
     assert_string_equal(lastsesion_statevalue, "ExpiredEmptyUser");
 }
 
@@ -351,7 +354,7 @@ auth_token_test_random_keys(void **state)
     assert_string_equal(random_token, ctx->multi.auth_token);
 
     strcpy(ctx->up.password, ctx->multi.auth_token);
-    assert_true(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
+    assert_true(verify_auth_token(&ctx->up, &ctx->multi, ctx->session));
 }
 
 
@@ -363,11 +366,11 @@ auth_token_test_key_load(void **state)
     free_key_ctx(&ctx->multi.opt.auth_token_key);
     auth_token_init_secret(&ctx->multi.opt.auth_token_key, zeroinline, true);
     strcpy(ctx->up.password, now0key0);
-    assert_true(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
+    assert_true(verify_auth_token(&ctx->up, &ctx->multi, ctx->session));
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
     auth_token_init_secret(&ctx->multi.opt.auth_token_key, allx01inline, true);
-    assert_false(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
+    assert_false(verify_auth_token(&ctx->up, &ctx->multi, ctx->session));
 }
 
 int