1. openvpn
  2. Commit 716049923e3e70c3de938d6da5d05f529ec515b5
Browse code

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>

Arne Schwabe authored on 2021/05/21 00:11:40
Showing 4 changed files
src/openvpn/auth_token.c
History View file @ 7160499
... ... 
@@ -57,6 +57,7 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
57 57 
         return;
58 58 
     }
59 59
60 
+    int auth_token_state_flags = session->key[KS_PRIMARY].auth_token_state_flags;
60 61
61 62 
     const char *state;
62 63
... ... 
@@ -64,9 +65,9 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
64 64 
     {
65 65 
         state = "Initial";
66 66 
     }
67 
-    else if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
67 
+    else if (auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
68 68 
     {
69 
-        switch (multi->auth_token_state_flags & (AUTH_TOKEN_VALID_EMPTYUSER|AUTH_TOKEN_EXPIRED))
69 
+        switch (auth_token_state_flags & (AUTH_TOKEN_VALID_EMPTYUSER|AUTH_TOKEN_EXPIRED))
70 70 
         {
71 71 
             case 0:
72 72 
                 state = "Authenticated";
... ... 
@@ -98,8 +99,8 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
98 98
99 99 
     /* We had a valid session id before */
100 100 
     const char *session_id_source;
101 
-    if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK
102 
-        && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
101 
+    if (auth_token_state_flags & AUTH_TOKEN_HMAC_OK
102 
+        && !(auth_token_state_flags & AUTH_TOKEN_EXPIRED))
103 103 
     {
104 104 
         session_id_source = up->password;
105 105 
     }
... ... 
@@ -236,7 +237,8 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
236 236 
      * a new token with the empty username since we do not want to loose
237 237 
      * the information that the username cannot be trusted
238 238 
      */
239 
-    if (multi->auth_token_state_flags & AUTH_TOKEN_VALID_EMPTYUSER)
239 
+    struct key_state *ks = &multi->session[TM_ACTIVE].key[KS_PRIMARY];
240 
+    if (ks->auth_token_state_flags & AUTH_TOKEN_VALID_EMPTYUSER)
240 241 
     {
241 242 
         hmac_ctx_update(ctx, (const uint8_t *) "", 0);
242 243 
     }
src/openvpn/ssl_common.h
History View file @ 7160499
... ... 
@@ -182,6 +182,8 @@ enum auth_deferred_result {
182 182 
 struct key_state
183 183 
 {
184 184 
     int state;
185 
+    /** The state of the auth-token sent from the client */
186 
+    int auth_token_state_flags;
185 187
186 188 
     /**
187 189 
      * Key id for this key_state,  inherited from struct tls_session.
... ... 
@@ -598,8 +600,6 @@ struct tls_multi
598 598 
      * OpenVPN 3 clients sometimes wipes or replaces the username with a
599 599 
      * username hint from their config.
600 600 
      */
601 
-    int auth_token_state_flags;
602 
-    /**< The state of the auth-token sent from the client last time */
603 601
604 602 
     /* For P_DATA_V2 */
605 603 
     uint32_t peer_id;
src/openvpn/ssl_verify.c
History View file @ 7160499
... ... 
@@ -1484,7 +1484,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1484 1484 
      */
1485 1485 
     if (session->opt->auth_token_generate && is_auth_token(up->password))
1486 1486 
     {
1487 
-        multi->auth_token_state_flags = verify_auth_token(up, multi, session);
1487 
+        ks->auth_token_state_flags = verify_auth_token(up, multi, session);
1488 1488 
         if (session->opt->auth_token_call_auth)
1489 1489 
         {
1490 1490 
             /*
... ... 
@@ -1493,7 +1493,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1493 1493 
              * decide what to do with the result
1494 1494 
              */
1495 1495 
         }
1496 
-        else if (multi->auth_token_state_flags == AUTH_TOKEN_HMAC_OK)
1496 
+        else if (ks->auth_token_state_flags == AUTH_TOKEN_HMAC_OK)
1497 1497 
         {
1498 1498 
             /*
1499 1499 
              * 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,
1592 1592 
              * the initial timestamp and session id can be extracted from it
1593 1593 
              */
1594 1594 
             if (!multi->auth_token
1595 
-                && (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
1596 
-                && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
1595 
+                && (ks->auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
1596 
+                && !(ks->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
1597 1597 
             {
1598 1598 
                 multi->auth_token = strdup(up->password);
1599 1599 
             }
tests/unit_tests/openvpn/test_auth_token.c
History View file @ 7160499
... ... 
@@ -45,7 +45,7 @@ struct test_context {
45 45 
     struct tls_multi multi;
46 46 
     struct key_type kt;
47 47 
     struct user_pass up;
48 
-    struct tls_session session;
48 
+    struct tls_session *session;
49 49 
 };
50 50
51 51 
 /* Dummy functions that do nothing to mock the functionality */
... ... 
@@ -100,10 +100,11 @@ setup(void **state)
100 100 
     }
101 101 
     ctx->multi.opt.auth_token_generate = true;
102 102 
     ctx->multi.opt.auth_token_lifetime = 3000;
103 
+    ctx->session = &ctx->multi.session[TM_ACTIVE];
103 104
104 
-    ctx->session.opt = calloc(1, sizeof(struct tls_options));
105 
-    ctx->session.opt->renegotiate_seconds = 120;
106 
-    ctx->session.opt->auth_token_lifetime = 3000;
105 
+    ctx->session->opt = calloc(1, sizeof(struct tls_options));
106 
+    ctx->session->opt->renegotiate_seconds = 120;
107 
+    ctx->session->opt->auth_token_lifetime = 3000;
107 108
108 109 
     strcpy(ctx->up.username, "test user name");
109 110 
     strcpy(ctx->up.password, "ignored");
... ... 
@@ -122,7 +123,7 @@ teardown(void **state)
122 122 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
123 123 
     wipe_auth_token(&ctx->multi);
124 124
125 
-    free(ctx->session.opt);
125 
+    free(ctx->session->opt);
126 126 
     free(ctx);
127 127
128 128 
     return 0;
... ... 
@@ -135,7 +136,7 @@ auth_token_basic_test(void **state)
135 135
136 136 
     generate_auth_token(&ctx->up, &ctx->multi);
137 137 
     strcpy(ctx->up.password, ctx->multi.auth_token);
138 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
138 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
139 139 
                      AUTH_TOKEN_HMAC_OK);
140 140 
 }
141 141
... ... 
@@ -146,7 +147,7 @@ auth_token_fail_invalid_key(void **state)
146 146
147 147 
     generate_auth_token(&ctx->up, &ctx->multi);
148 148 
     strcpy(ctx->up.password, ctx->multi.auth_token);
149 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
149 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
150 150 
                      AUTH_TOKEN_HMAC_OK);
151 151
152 152 
     /* Change auth-token key */
... ... 
@@ -155,13 +156,13 @@ auth_token_fail_invalid_key(void **state)
155 155 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
156 156 
     init_key_ctx(&ctx->multi.opt.auth_token_key, &key, &ctx->kt, false, "TEST");
157 157
158 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session), 0);
158 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session), 0);
159 159
160 160 
     /* Load original test key again */
161 161 
     memset(&key, 0, sizeof(key));
162 162 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
163 163 
     init_key_ctx(&ctx->multi.opt.auth_token_key, &key, &ctx->kt, false, "TEST");
164 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
164 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
165 165 
                      AUTH_TOKEN_HMAC_OK);
166 166
167 167 
 }
... ... 
@@ -176,32 +177,32 @@ auth_token_test_timeout(void **state)
176 176 
     strcpy(ctx->up.password, ctx->multi.auth_token);
177 177
178 178 
     /* No time has passed */
179 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
179 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
180 180 
                      AUTH_TOKEN_HMAC_OK);
181 181
182 182 
     /* Token before validity, should be rejected */
183 183 
     now = 100000 - 100;
184 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
184 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
185 185 
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
186 186
187 187 
     /* Token still in validity, should be accepted */
188 
-    now = 100000 + 2*ctx->session.opt->renegotiate_seconds - 20;
189 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
188 
+    now = 100000 + 2*ctx->session->opt->renegotiate_seconds - 20;
189 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
190 190 
                      AUTH_TOKEN_HMAC_OK);
191 191
192 192 
     /* Token past validity, should be rejected */
193 
-    now = 100000 + 2*ctx->session.opt->renegotiate_seconds + 20;
194 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
193 
+    now = 100000 + 2*ctx->session->opt->renegotiate_seconds + 20;
194 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
195 195 
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
196 196
197 197 
     /* Check if the mode for a client that never updates its token works */
198 198 
     ctx->multi.auth_token_initial = strdup(ctx->up.password);
199 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
199 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
200 200 
                      AUTH_TOKEN_HMAC_OK);
201 201
202 202 
     /* But not when we reached our timeout */
203 
-    now = 100000 + ctx->session.opt->auth_token_lifetime + 1;
204 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
203 
+    now = 100000 + ctx->session->opt->auth_token_lifetime + 1;
204 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
205 205 
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
206 206
207 207 
     free(ctx->multi.auth_token_initial);
... ... 
@@ -209,22 +210,22 @@ auth_token_test_timeout(void **state)
209 209
210 210 
     /* regenerate the token util it hits the expiry */
211 211 
     now = 100000;
212 
-    while (now < 100000 + ctx->session.opt->auth_token_lifetime + 1)
212 
+    while (now < 100000 + ctx->session->opt->auth_token_lifetime + 1)
213 213 
     {
214 
-        assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
214 
+        assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
215 215 
                          AUTH_TOKEN_HMAC_OK);
216 216 
         generate_auth_token(&ctx->up, &ctx->multi);
217 217 
         strcpy(ctx->up.password, ctx->multi.auth_token);
218 
-        now += ctx->session.opt->renegotiate_seconds;
218 
+        now += ctx->session->opt->renegotiate_seconds;
219 219 
     }
220 220
221 221
222 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
222 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
223 223 
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
224 224 
     ctx->multi.opt.auth_token_lifetime = 0;
225 225
226 226 
     /* Non expiring token should be fine */
227 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
227 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
228 228 
                      AUTH_TOKEN_HMAC_OK);
229 229 
 }
230 230
... ... 
@@ -253,7 +254,7 @@ auth_token_test_known_keys(void **state)
253 253 
     assert_string_equal(now0key0, ctx->multi.auth_token);
254 254
255 255 
     strcpy(ctx->up.password, ctx->multi.auth_token);
256 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
256 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
257 257 
                      AUTH_TOKEN_HMAC_OK);
258 258 
 }
259 259
... ... 
@@ -277,25 +278,25 @@ auth_token_test_empty_user(void **state)
277 277
278 278 
     generate_auth_token(&ctx->up, &ctx->multi);
279 279 
     strcpy(ctx->up.password, ctx->multi.auth_token);
280 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
280 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
281 281 
                      AUTH_TOKEN_HMAC_OK);
282 282
283 283 
     now = 100000;
284 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
284 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
285 285 
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED);
286 286 
     strcpy(ctx->up.username, "test user name");
287 287
288 288 
     now = 0;
289 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
289 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
290 290 
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_VALID_EMPTYUSER);
291 291
292 292 
     strcpy(ctx->up.username, "test user name");
293 293 
     now = 100000;
294 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
294 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
295 295 
                      AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED|AUTH_TOKEN_VALID_EMPTYUSER);
296 296
297 297 
     zerohmac(ctx->up.password);
298 
-    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session),
298 
+    assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
299 299 
                      0);
300 300 
 }
301 301
... ... 
@@ -304,30 +305,32 @@ auth_token_test_env(void **state)
304 304 
 {
305 305 
     struct test_context *ctx = (struct test_context *) *state;
306 306
307 
-    ctx->multi.auth_token_state_flags = 0;
307 
+    struct key_state *ks = &ctx->multi.session[TM_ACTIVE].key[KS_PRIMARY];
308 
+
309 
+    ks->auth_token_state_flags = 0;
308 310 
     ctx->multi.auth_token = NULL;
309 
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
311 
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
310 312 
     assert_string_equal(lastsesion_statevalue, "Initial");
311 313
312 
-    ctx->multi.auth_token_state_flags = 0;
314 
+    ks->auth_token_state_flags = 0;
313 315 
     strcpy(ctx->up.password, now0key0);
314 
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
316 
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
315 317 
     assert_string_equal(lastsesion_statevalue, "Invalid");
316 318
317 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK;
318 
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
319 
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK;
320 
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
319 321 
     assert_string_equal(lastsesion_statevalue, "Authenticated");
320 322
321 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED;
322 
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
323 
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED;
324 
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
323 325 
     assert_string_equal(lastsesion_statevalue, "Expired");
324 326
325 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_VALID_EMPTYUSER;
326 
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
327 
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_VALID_EMPTYUSER;
328 
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
327 329 
     assert_string_equal(lastsesion_statevalue, "AuthenticatedEmptyUser");
328 330
329 
-    ctx->multi.auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED|AUTH_TOKEN_VALID_EMPTYUSER;
330 
-    add_session_token_env(&ctx->session, &ctx->multi, &ctx->up);
331 
+    ks->auth_token_state_flags = AUTH_TOKEN_HMAC_OK|AUTH_TOKEN_EXPIRED|AUTH_TOKEN_VALID_EMPTYUSER;
332 
+    add_session_token_env(ctx->session, &ctx->multi, &ctx->up);
331 333 
     assert_string_equal(lastsesion_statevalue, "ExpiredEmptyUser");
332 334 
 }
333 335
... ... 
@@ -351,7 +354,7 @@ auth_token_test_random_keys(void **state)
351 351 
     assert_string_equal(random_token, ctx->multi.auth_token);
352 352
353 353 
     strcpy(ctx->up.password, ctx->multi.auth_token);
354 
-    assert_true(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
354 
+    assert_true(verify_auth_token(&ctx->up, &ctx->multi, ctx->session));
355 355 
 }
356 356
357 357
... ... 
@@ -363,11 +366,11 @@ auth_token_test_key_load(void **state)
363 363 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
364 364 
     auth_token_init_secret(&ctx->multi.opt.auth_token_key, zeroinline, true);
365 365 
     strcpy(ctx->up.password, now0key0);
366 
-    assert_true(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
366 
+    assert_true(verify_auth_token(&ctx->up, &ctx->multi, ctx->session));
367 367
368 368 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
369 369 
     auth_token_init_secret(&ctx->multi.opt.auth_token_key, allx01inline, true);
370 
-    assert_false(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
370 
+    assert_false(verify_auth_token(&ctx->up, &ctx->multi, ctx->session));
371 371 
 }
372 372
373 373 
 int