Ensure we only get the session from valid tokens for external-auth

This also improves is_auth_token to check for the correct length and
adds a few unit tests.

This fixes an assertion when a user would send auth token with the
right prefix but overall too short length.

Reported-By: Haiyang Huang <huanghaiyang83@gmail.com>
CVE: 2026-13122
Github: OpenVPN/openvpn-private-issues#118
Github: OpenVPN/openvpn-private-issues#128

Change-Id: I053cca5f42e3841cef8934c2114a8a159fbc0c29
Acked-by: MaxF <max@max-fillinger.net>

diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index eb8e27366957131bdb11851979f9f2bf82fe6752..f420588d8979ff1e8d181834d9e85565d8f31ae7 100644 (file)
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -87,7 +87,9 @@ fast hardware. SSL/TLS authentication must be used in this mode.
   external-auth option unless the client could authenticate in another
   acceptable way (e.g. client certificate), otherwise returning success
   will lead to authentication bypass (as does returning success on a wrong
-  password from a script).
+  password from a script). In the case that Expired token is accepted
+  the token will keep the session id and start time from the original
+  (expired) token.
 
   **Note:** the username for ``--auth-gen-token`` can be overridden by
   ``--override-username``. In this case the client will be pushed also the

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index d8ca125231d946179ee6d3bdd8a41d6b6ca8add7..bd902123274d371e20d62669024241e4c9c08f63 100644 (file)
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -19,14 +19,33 @@
 const char *auth_token_pem_name = "OpenVPN auth-token server key";
 
 #define AUTH_TOKEN_SESSION_ID_LEN        12
-#define AUTH_TOKEN_SESSION_ID_BASE64_LEN (AUTH_TOKEN_SESSION_ID_LEN * 8 / 6)
+#define AUTH_TOKEN_SESSION_ID_BASE64_LEN (OPENVPN_BASE64_LENGTH(AUTH_TOKEN_SESSION_ID_LEN))
 
-#if AUTH_TOKEN_SESSION_ID_LEN % 3
-#error AUTH_TOKEN_SESSION_ID_LEN needs to be multiple a 3
-#endif
+/* We want our token to be a multiple of 3 bytes to avoid the base64 padding */
+static_assert(AUTH_TOKEN_SESSION_ID_LEN % 3 == 0, "AUTH_TOKEN_SESSION_ID_LEN needs to be multiple a 3");
 
+#define AUTH_TOKEN_HMAC_LEN   SHA256_DIGEST_LENGTH
 /* Size of the data of the token (not b64 encoded and without prefix) */
-#define TOKEN_DATA_LEN (2 * sizeof(int64_t) + AUTH_TOKEN_SESSION_ID_LEN + 32)
+#define TOKEN_DATA_LEN        (2 * sizeof(int64_t) + AUTH_TOKEN_SESSION_ID_LEN + AUTH_TOKEN_HMAC_LEN)
+#define TOKEN_DATA_BASE64_LEN (OPENVPN_BASE64_LENGTH(TOKEN_DATA_LEN))
+
+
+#define TOTAL_SESSION_TOKEN_LEN (strlen(SESSION_ID_PREFIX) + TOKEN_DATA_BASE64_LEN)
+
+/* Ensure that TOKEN_DATA_LEN is a multiple of 3 so the we avoid the base64
+ * padding */
+static_assert(TOKEN_DATA_LEN % 3 == 0, "TOKEN_DATA_BASE64_LEN is not a multiple of 3");
+
+bool
+is_auth_token(const char *password)
+{
+    if (strlen(password) != TOTAL_SESSION_TOKEN_LEN)
+    {
+        return false;
+    }
+
+    return (memcmp_constant_time(SESSION_ID_PREFIX, password, strlen(SESSION_ID_PREFIX)) == 0);
+}
 
 static struct key_type
 auth_token_kt(void)
@@ -106,7 +125,6 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
      * and being a multiple of 4 ensure that it a multiple of bytes
      * in the encoding
      */
-
     char session_id[AUTH_TOKEN_SESSION_ID_LEN * 2] = { 0 };
     memcpy(session_id, session_id_source + strlen(SESSION_ID_PREFIX),
            AUTH_TOKEN_SESSION_ID_LEN * 8 / 6);
@@ -172,7 +190,7 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
 
     if (multi->auth_token_initial)
     {
-        /* Just enough space to fit 8 bytes+ 1 extra to decode a non-padded
+        /* Just enough space to fit 8 bytes + 1 extra to decode a non-padded
          * base64 string (multiple of 3 bytes). 9 bytes => 12 bytes base64
          * bytes
          */
@@ -182,7 +200,7 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
         char *initial_token_copy = string_alloc(multi->auth_token_initial, &gc);
 
         char *old_sessid = initial_token_copy + strlen(SESSION_ID_PREFIX);
-        char *old_tstamp_initial = old_sessid + AUTH_TOKEN_SESSION_ID_LEN * 8 / 6;
+        char *old_tstamp_initial = old_sessid + AUTH_TOKEN_SESSION_ID_BASE64_LEN;
 
         /*
          * We null terminate the old token just after the session ID to let
@@ -284,7 +302,7 @@ check_hmac_token(hmac_ctx_t *ctx, const uint8_t *b64decoded, const char *usernam
     hmac_ctx_final(ctx, hmac_output);
 
     const uint8_t *hmac = b64decoded + TOKEN_DATA_LEN - 256 / 8;
-    return memcmp_constant_time(&hmac_output, hmac, 32) == 0;
+    return memcmp_constant_time(&hmac_output, hmac, AUTH_TOKEN_HMAC_LEN) == 0;
 }
 
 unsigned int

diff --git a/src/openvpn/auth_token.h b/src/openvpn/auth_token.h
index 7b66a8189c1ffb6998dd0e82e975e764bd650ee8..fd8317d80a8bb52cce3dd12b23597ed24808c40c 100644 (file)
--- a/src/openvpn/auth_token.h
+++ b/src/openvpn/auth_token.h
@@ -117,11 +117,8 @@ void wipe_auth_token(struct tls_multi *multi);
  * @param password
  * @return whether the password string starts with the session token prefix
  */
-static inline bool
-is_auth_token(const char *password)
-{
-    return (memcmp_constant_time(SESSION_ID_PREFIX, password, strlen(SESSION_ID_PREFIX)) == 0);
-}
+bool
+is_auth_token(const char *password);
 /**
  * Checks if a client should be sent a new auth token to update its
  * current auth-token

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 31ecf13ad6e0ff591dcf1d274b66494025f48e45..9197adfba1967a81737cdc73c049f7277813ca11 100644 (file)
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1651,10 +1651,10 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, struct tls_sessi
     {
         ks->auth_token_state_flags = verify_auth_token(up, multi, session);
 
-        /* If this is the first time we see an auth-token in this multi session,
-         * save it as initial auth token. This ensures using the
-         * same session ID and initial timestamp in new tokens */
-        if (!multi->auth_token_initial)
+        /* If this is a valid token and the first time we see an auth-token
+         * in this multi session, save it as initial auth token. This ensures
+         * using the same session ID and initial timestamp in new tokens */
+        if (!multi->auth_token_initial && (ks->auth_token_state_flags & AUTH_TOKEN_HMAC_OK))
         {
             multi->auth_token_initial = strdup(up->password);
         }

diff --git a/tests/unit_tests/openvpn/test_auth_token.c b/tests/unit_tests/openvpn/test_auth_token.c
index c7ba5fd825a30f062f06a709a9bd6b369a92ba97..9e7b5b929fdebd7f43fc189d1e11b3f9d6049a2e 100644 (file)
--- a/tests/unit_tests/openvpn/test_auth_token.c
+++ b/tests/unit_tests/openvpn/test_auth_token.c
@@ -83,6 +83,8 @@ static const char *random_key =
 static const char *random_token =
     "SESS_ID_AT_ThhRItzOKNKrh3dfAAAAAFwzHpwAAAAAXDMenDdrq0RoH3dkA1f7O3wO+7kZcx2DusVZrRmFlWQM9HOb";
 
+static const char *last_session_state_value;
+static char last_session_id_value[64];
 
 static int
 setup(void **state)
@@ -113,6 +115,10 @@ setup(void **state)
     init_key_ctx(&ctx->multi.opt.auth_token_key, &key, &ctx->kt, false, "TEST");
 
     now = 0;
+
+    CLEAR(last_session_id_value);
+    last_session_state_value = NULL;
+
     return 0;
 }
 
@@ -257,13 +263,17 @@ auth_token_test_known_keys(void **state)
     assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session), AUTH_TOKEN_HMAC_OK);
 }
 
-static const char *lastsesion_statevalue;
+
 void
 setenv_str(struct env_set *es, const char *name, const char *value)
 {
     if (streq(name, "session_state"))
     {
-        lastsesion_statevalue = value;
+        last_session_state_value = value;
+    }
+    else if (streq(name, "session_id"))
+    {
+        strncpy(last_session_id_value, value, sizeof(last_session_id_value));
     }
 }
 
@@ -337,29 +347,50 @@ auth_token_test_env(void **state)
     ks->auth_token_state_flags = 0;
     ctx->multi.auth_token = NULL;
     add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
-    assert_string_equal(lastsesion_statevalue, "Initial");
+    assert_string_equal(last_session_state_value, "Initial");
 
+    /* This generated a fresh token with a new session ID,
+     * check that this session id is part of the multi initial token and
+     * found at the right index */
+    char *idx = strstr(ctx->multi.auth_token_initial, last_session_id_value);
+    assert_non_null(idx);
+
+    ptrdiff_t offset = idx - ctx->multi.auth_token_initial;
+    assert_int_equal(offset, 11);
+
+    CLEAR(last_session_id_value);
     ks->auth_token_state_flags = 0;
-    strcpy(ctx->up.password, now0key0);
+    strcpy(ctx->up.password, "somepassword");
+
+    free(ctx->multi.auth_token_initial);
+    ctx->multi.auth_token_initial = strdup(now0key0);
+
     add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
-    assert_string_equal(lastsesion_statevalue, "Invalid");
+    assert_string_equal(last_session_state_value, "Initial");
+    assert_string_equal(last_session_id_value, "0123456789abcdef");
+
+    CLEAR(last_session_id_value);
+    free(ctx->multi.auth_token_initial);
+    ctx->multi.auth_token_initial = NULL;
 
     ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK;
+    strcpy(ctx->up.password, now0key0);
     add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
-    assert_string_equal(lastsesion_statevalue, "Authenticated");
+    assert_string_equal(last_session_state_value, "Authenticated");
+    assert_string_equal(last_session_id_value, "0123456789abcdef");
 
     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");
+    assert_string_equal(last_session_state_value, "Expired");
 
     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");
+    assert_string_equal(last_session_state_value, "AuthenticatedEmptyUser");
 
     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");
+    assert_string_equal(last_session_state_value, "ExpiredEmptyUser");
 }
 
 static void
@@ -387,6 +418,45 @@ auth_token_test_random_keys(void **state)
 }
 
 
+static void
+auth_token_test_is_auth_token(void **state)
+{
+    /* Known-good tokens are recognised */
+    assert_true(is_auth_token(now0key0));
+    assert_true(is_auth_token(random_token));
+
+    /* Empty and clearly too-short strings are rejected */
+    assert_false(is_auth_token(""));
+    assert_false(is_auth_token("foobar"));
+    assert_false(is_auth_token(SESSION_ID_PREFIX));
+    assert_false(is_auth_token("SESS_ID_AT_Something_wild"));
+
+    /* Since all auth token have the same length, get the length from
+     * now0key0 */
+    const size_t auth_token_len = strlen(now0key0);
+
+    /* Right length, incorrect prefix */
+    char buf[256] = { 0 };
+    assert_true(auth_token_len < sizeof(buf));
+    memset(buf, 'A', auth_token_len);
+    buf[auth_token_len] = '\0';
+    assert_false(is_auth_token(buf));
+
+    /* Correct prefix to ensure the previous test is valid */
+    memcpy(buf, "SESS_ID_AT_", strlen("SESS_ID_AT_"));
+    assert_true(is_auth_token(buf));
+
+    /* Correct prefix, one byte too short */
+    buf[auth_token_len - 1] = '\0';
+    assert_false(is_auth_token(buf));
+
+    /* Correct prefix, one byte too long */
+    strcpy(buf, now0key0);
+    buf[auth_token_len] = 'A';
+    buf[auth_token_len + 1] = '\0';
+    assert_false(is_auth_token(buf));
+}
+
 static void
 auth_token_test_key_load(void **state)
 {
@@ -415,7 +485,8 @@ main(void)
         cmocka_unit_test_setup_teardown(auth_token_test_random_keys, setup, teardown),
         cmocka_unit_test_setup_teardown(auth_token_test_key_load, setup, teardown),
         cmocka_unit_test_setup_teardown(auth_token_test_timeout, setup, teardown),
-        cmocka_unit_test_setup_teardown(auth_token_test_session_mismatch, setup, teardown)
+        cmocka_unit_test_setup_teardown(auth_token_test_session_mismatch, setup, teardown),
+        cmocka_unit_test(auth_token_test_is_auth_token),
     };
 
     return cmocka_run_group_tests_name("auth-token tests", tests, NULL, NULL);