diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst index 715473353..f1d2ec317 100644 --- a/doc/man-sections/server-options.rst +++ b/doc/man-sections/server-options.rst @@ -35,7 +35,7 @@ fast hardware. SSL/TLS authentication must be used in this mode. token is reached or after not being renewed for more than 2 \* ``reneg-sec`` seconds. Clients will be sent renewed tokens on every TLS renogiation to keep the client's token updated. This is done to - invalidate a token if a client is disconnected for a sufficently long + invalidate a token if a client is disconnected for a sufficiently long time, while at the same time permitting much longer token lifetimes for active clients. @@ -46,7 +46,7 @@ fast hardware. SSL/TLS authentication must be used in this mode. When the :code:`external-auth` keyword is present the normal authentication method will always be called even if auth-token succeeds. Normally other authentications method are skipped if auth-token - verification suceeds or fails. + verification succeeds or fails. This option postpones this decision to the external authentication methods and checks the validity of the account and do other checks. diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c index 0ea6d1832..d7c005256 100644 --- a/src/openvpn/auth_token.c +++ b/src/openvpn/auth_token.c @@ -21,6 +21,8 @@ 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) #if AUTH_TOKEN_SESSION_ID_LEN % 3 #error AUTH_TOKEN_SESSION_ID_LEN needs to be multiple a 3 #endif @@ -109,11 +111,11 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi, /* * No session before, generate a new session token for the new session */ - if (!multi->auth_token) + if (!multi->auth_token_initial) { generate_auth_token(up, multi); } - session_id_source = multi->auth_token; + session_id_source = multi->auth_token_initial; } /* * In the auth-token the auth token is already base64 encoded @@ -184,7 +186,7 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi) uint8_t sessid[AUTH_TOKEN_SESSION_ID_LEN]; - if (multi->auth_token) + if (multi->auth_token_initial) { /* 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 @@ -192,13 +194,16 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi) */ char old_tstamp_decode[9]; - /* - * reuse the same session id and timestamp and null terminate it at - * for base64 decode it only decodes the session id part of it - */ - char *old_sessid = multi->auth_token + strlen(SESSION_ID_PREFIX); + /* Make a copy of the string to not modify multi->auth_token_initial */ + char *initial_token_copy = string_alloc(multi->auth_token_initial, &gc); + + char *old_sessid = initial_token_copy + strlen(SESSION_ID_PREFIX); char *old_tsamp_initial = old_sessid + AUTH_TOKEN_SESSION_ID_LEN*8/6; + /* + * We null terminate the old token just after the session ID to let + * our base64 decode function only decode the session ID + */ old_tsamp_initial[12] = '\0'; ASSERT(openvpn_base64_decode(old_tsamp_initial, old_tstamp_decode, 9) == 9); @@ -211,11 +216,7 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi) initial_timestamp = *tstamp_ptr; old_tsamp_initial[0] = '\0'; - ASSERT(openvpn_base64_decode(old_sessid, sessid, AUTH_TOKEN_SESSION_ID_LEN)==AUTH_TOKEN_SESSION_ID_LEN); - - - /* free the auth-token, we will replace it with a new one */ - free(multi->auth_token); + ASSERT(openvpn_base64_decode(old_sessid, sessid, AUTH_TOKEN_SESSION_ID_LEN) == AUTH_TOKEN_SESSION_ID_LEN); } else if (!rand_bytes(sessid, AUTH_TOKEN_SESSION_ID_LEN)) { @@ -272,11 +273,22 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi) free(b64output); + /* free the auth-token if defined, we will replace it with a new one */ + free(multi->auth_token); multi->auth_token = strdup((char *)BPTR(&session_token)); dmsg(D_SHOW_KEYS, "Generated token for client: %s (%s)", multi->auth_token, up->username); + if (!multi->auth_token_initial) + { + /* + * Save the initial auth token to continue using the same session ID + * and timestamp in updates + */ + multi->auth_token_initial = strdup(multi->auth_token); + } + gc_free(&gc); } @@ -343,23 +355,17 @@ verify_auth_token(struct user_pass *up, struct tls_multi *multi, } else { - msg(M_WARN, "--auth-token-gen: HMAC on token from client failed (%s)", + msg(M_WARN, "--auth-gen-token: HMAC on token from client failed (%s)", up->username); return 0; } /* Accept session tokens that not expired are in the acceptable range * for renogiations */ - bool in_renog_time = now >= timestamp + bool in_renegotiation_time = now >= timestamp && now < timestamp + 2 * session->opt->renegotiate_seconds; - /* We could still have a client that does not update - * its auth-token, so also allow the initial auth-token */ - bool initialtoken = multi->auth_token_initial - && memcmp_constant_time(up->password, multi->auth_token_initial, - strlen(multi->auth_token_initial)) == 0; - - if (!in_renog_time && !initialtoken) + if (!in_renegotiation_time) { ret |= AUTH_TOKEN_EXPIRED; } @@ -383,7 +389,20 @@ verify_auth_token(struct user_pass *up, struct tls_multi *multi, { /* Tell client that the session token is expired */ auth_set_client_reason(multi, "SESSION: token expired"); - msg(M_INFO, "--auth-token-gen: auth-token from client expired"); + msg(M_INFO, "--auth-gen-token: auth-token from client expired"); + } + + /* Check that we do have the same session ID in the token as in our stored + * auth-token to ensure that it did not change. + * This also compares the prefix and session part of the + * tokens, which should be identical if the session ID stayed the same */ + if (multi->auth_token_initial + && memcmp_constant_time(multi->auth_token_initial, up->password, + strlen(SESSION_ID_PREFIX) + AUTH_TOKEN_SESSION_ID_BASE64_LEN)) + { + msg(M_WARN, "--auth-gen-token: session id in token changed (Rejecting " + "token."); + ret = 0; } return ret; } @@ -408,3 +427,27 @@ wipe_auth_token(struct tls_multi *multi) multi->auth_token_initial = NULL; } } + +void +resend_auth_token_renegotiation(struct tls_multi *multi, struct tls_session *session) +{ + /* + * 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 + */ + bool is_renegotiation = session->key[KS_PRIMARY].key_id != 0; + + if (multi->auth_token_initial && is_renegotiation) + { + /* + * We do not explicitly reschedule the sending of the + * control message here. This might delay this reply + * a few seconds but this message is not time critical + */ + send_push_reply_auth_token(multi); + } +} \ No newline at end of file diff --git a/src/openvpn/auth_token.h b/src/openvpn/auth_token.h index 73a00ddd7..de93a9413 100644 --- a/src/openvpn/auth_token.h +++ b/src/openvpn/auth_token.h @@ -117,7 +117,7 @@ void wipe_auth_token(struct tls_multi *multi); /** * Return if the password string has the format of a password. * - * This fuction will always read as many bytes as SESSION_ID_PREFIX is longer + * This function will always read as many bytes as SESSION_ID_PREFIX is longer * the caller needs ensure that password memory is at least that long (true for * calling with struct user_pass) * @param password @@ -129,4 +129,13 @@ is_auth_token(const char *password) return (memcmp_constant_time(SESSION_ID_PREFIX, password, strlen(SESSION_ID_PREFIX)) == 0); } +/** + * Checks if a client should be sent a new auth token to update its + * current auth-token + * @param multi Pointer the multi object of the TLS session + * @param session Pointer to the TLS session itself + */ +void +resend_auth_token_renegotiation(struct tls_multi *multi, struct tls_session *session); + #endif /* AUTH_TOKEN_H */ diff --git a/src/openvpn/push.c b/src/openvpn/push.c index f4957f147..53cb7ca6f 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -527,14 +527,6 @@ prepare_auth_token_push_reply(struct tls_multi *tls_multi, struct gc_arena *gc, push_option_fmt(gc, push_list, M_USAGE, "auth-token %s", tls_multi->auth_token); - if (!tls_multi->auth_token_initial) - { - /* - * Save the initial auth token for clients that ignore - * the updates to the token - */ - tls_multi->auth_token_initial = strdup(tls_multi->auth_token); - } } } diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 615ed69e5..e9b8d1b92 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3115,13 +3115,18 @@ tls_multi_process(struct tls_multi *multi, if (ks->state == S_ACTIVE && ks->authenticated == KS_AUTH_TRUE) { - /* This will move ks->state from S_ACTIVE to S_GENERATED_KEYS */ + /* Session is now fully authenticated. + * tls_session_generate_data_channel_keys will move ks->state + * from S_ACTIVE to S_GENERATED_KEYS */ if (!tls_session_generate_data_channel_keys(session)) { msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed"); ks->authenticated = KS_AUTH_FALSE; ks->state = S_ERROR; } + + /* Update auth token on the client if needed */ + resend_auth_token_renegotiation(multi, session); } } } diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 72eb55bd3..64c1d53f3 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -95,7 +95,10 @@ * completed while still within the * handshake window. Deferred auth and * client connect can still be pending. */ -#define S_GENERATED_KEYS 7 /**< The data channel keys have been generated */ +#define S_GENERATED_KEYS 7 /**< The data channel keys have been generated + * The TLS session is fully authenticated + * when reaching this state. */ + /* Note that earlier versions also had a S_OP_NORMAL state that was * virtually identical with S_ACTIVE and the code still assumes everything * >= S_ACTIVE to be fully operational */ @@ -596,8 +599,8 @@ struct tls_multi * user/pass authentications in this session. */ char *auth_token_initial; - /**< The first auth-token we sent to a client, for clients that do - * not update their auth-token (older OpenVPN3 core versions) + /**< The first auth-token we sent to a client. We use this to remember + * the session ID and initial timestamp when generating new auth-token. */ #define AUTH_TOKEN_HMAC_OK (1<<0) /**< Auth-token sent from client has valid hmac */ diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index bbb1878a3..913c95620 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -1512,6 +1512,15 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, if (session->opt->auth_token_generate && is_auth_token(up->password)) { 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) + { + multi->auth_token_initial = strdup(up->password); + } + if (session->opt->auth_token_call_auth) { /* @@ -1631,27 +1640,7 @@ 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, diff --git a/tests/unit_tests/openvpn/test_auth_token.c b/tests/unit_tests/openvpn/test_auth_token.c index 4030052e0..6bfad6efb 100644 --- a/tests/unit_tests/openvpn/test_auth_token.c +++ b/tests/unit_tests/openvpn/test_auth_token.c @@ -174,7 +174,10 @@ auth_token_test_timeout(void **state) now = 100000; generate_auth_token(&ctx->up, &ctx->multi); + strcpy(ctx->up.password, ctx->multi.auth_token); + free(ctx->multi.auth_token_initial); + ctx->multi.auth_token_initial = NULL; /* No time has passed */ assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session), @@ -195,11 +198,6 @@ auth_token_test_timeout(void **state) 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), - 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), @@ -244,10 +242,10 @@ auth_token_test_known_keys(void **state) now = 0; /* Preload the session id so the same session id is used here */ - ctx->multi.auth_token = strdup(now0key0); + ctx->multi.auth_token_initial = strdup(now0key0); /* Zero the hmac part to ensure we have a newly generated token */ - zerohmac(ctx->multi.auth_token); + zerohmac(ctx->multi.auth_token_initial); generate_auth_token(&ctx->up, &ctx->multi); @@ -268,6 +266,38 @@ setenv_str(struct env_set *es, const char *name, const char *value) } } +void +auth_token_test_session_mismatch(void **state) +{ + struct test_context *ctx = (struct test_context *) *state; + + /* Generate first auth token and check it is correct */ + 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), + AUTH_TOKEN_HMAC_OK); + + char *token_sessiona = strdup(ctx->multi.auth_token); + + /* Generate second token */ + wipe_auth_token(&ctx->multi); + + 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), + AUTH_TOKEN_HMAC_OK); + + assert_int_not_equal(0, memcmp(ctx->multi.auth_token_initial + strlen(SESSION_ID_PREFIX), + token_sessiona + strlen(SESSION_ID_PREFIX), + AUTH_TOKEN_SESSION_ID_BASE64_LEN)); + + /* The first token is valid but should trigger the invalid response since + * the session id is not the same */ + strcpy(ctx->up.password, token_sessiona); + assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session), 0); + free(token_sessiona); +} + static void auth_token_test_empty_user(void **state) { @@ -341,13 +371,13 @@ auth_token_test_random_keys(void **state) now = 0x5c331e9c; /* Preload the session id so the same session id is used here */ - ctx->multi.auth_token = strdup(random_token); + ctx->multi.auth_token_initial = strdup(random_token); free_key_ctx(&ctx->multi.opt.auth_token_key); auth_token_init_secret(&ctx->multi.opt.auth_token_key, random_key, true); /* Zero the hmac part to ensure we have a newly generated token */ - zerohmac(ctx->multi.auth_token); + zerohmac(ctx->multi.auth_token_initial); generate_auth_token(&ctx->up, &ctx->multi); @@ -385,6 +415,7 @@ 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) }; #if defined(ENABLE_CRYPTO_OPENSSL)