diff options
| -rw-r--r-- | debian/patches/CVE-2020-15078-0.patch | 276 | ||||
| -rw-r--r-- | debian/patches/CVE-2020-15078-1.patch | 390 | ||||
| -rw-r--r-- | debian/patches/CVE-2020-15078-2.patch | 125 | ||||
| -rw-r--r-- | debian/patches/CVE-2020-15078-3.patch | 51 | ||||
| -rw-r--r-- | debian/patches/series | 4 |
5 files changed, 846 insertions, 0 deletions
diff --git a/debian/patches/CVE-2020-15078-0.patch b/debian/patches/CVE-2020-15078-0.patch new file mode 100644 index 0000000..d048994 --- /dev/null +++ b/debian/patches/CVE-2020-15078-0.patch @@ -0,0 +1,276 @@ +From 145110101b70599cb9adcf4ed071856daac9c8af Mon Sep 17 00:00:00 2001 +From: Arne Schwabe <arne@rfc2549.org> +Date: Sun, 28 Mar 2021 14:02:40 +0200 +Subject: [PATCH] Move context_auth from context_2 to tls_multi and name it + multi_state + +context_2 and tls_multi have the same life cycle for TLS connections +but so this move does not affect behaviour of the variable. + +OpenVPN TLS multi code has a grown a lot more complex and code that +handles multi objects needs to know the state that the object is in. +Since not all code has access to the context_2 struct, the code that +does not have access is often not checking the state directly but +checks other parts of multi that have been affected from a state +change. + +This patch also renames it to multi_state as this variable represents +the multi state machine status rather than just the state of the connect +authentication (more upcoming patches will move other states +into this variable). + +Patch V2: also rename context_auth to multi_state, explain a bit why this + change is done. +Patch V3: Add comments for c2->multi NULL check forwarding. Fix compile + with ENABLE_ASYNC_PUSH. + +Signed-off-by: Arne Schwabe <arne@rfc2549.org> +Acked-by: Antonio Quartulli <antonio@openvpn.net> +Acked-by: Gert Doering <gert@greenie.muc.de> +Message-Id: <20210418160111.1494779-1-arne@rfc2549.org> +URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22155.html +Signed-off-by: Gert Doering <gert@greenie.muc.de> +(backported from commit 0767d5b447044e4cdcfd198058aef1f85f63bbe6) +--- + src/openvpn/forward.c | 10 ++++++---- + src/openvpn/multi.c | 28 ++++++++++++++-------------- + src/openvpn/openvpn.h | 14 -------------- + src/openvpn/push.c | 5 +++-- + src/openvpn/ssl_common.h | 14 ++++++++++++++ + 5 files changed, 37 insertions(+), 34 deletions(-) + +diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c +index 7ed8d0d75..fd7412f73 100644 +--- a/src/openvpn/forward.c ++++ b/src/openvpn/forward.c +@@ -526,9 +526,10 @@ encrypt_sign(struct context *c, bool comp_frag) + + /* + * Drop non-TLS outgoing packet if client-connect script/plugin +- * has not yet succeeded. ++ * has not yet succeeded. In non-TLS mode tls_multi is not defined ++ * and we always pass packets. + */ +- if (c->c2.context_auth != CAS_SUCCEEDED) ++ if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED) + { + c->c2.buf.len = 0; + } +@@ -973,9 +974,10 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo + + /* + * Drop non-TLS packet if client-connect script/plugin and cipher selection +- * has not yet succeeded. ++ * has not yet succeeded. In non-TLS mode tls_multi is not defined ++ * and we always pass packets. + */ +- if (c->c2.context_auth != CAS_SUCCEEDED) ++ if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED) + { + c->c2.buf.len = 0; + } +diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c +index 137381805..599ffd86d 100644 +--- a/src/openvpn/multi.c ++++ b/src/openvpn/multi.c +@@ -678,7 +678,7 @@ multi_close_instance(struct multi_context *m, + #ifdef MANAGEMENT_DEF_AUTH + set_cc_config(mi, NULL); + #endif +- if (mi->context.c2.context_auth == CAS_SUCCEEDED) ++ if (mi->context.c2.tls_multi->multi_state == CAS_SUCCEEDED) + { + multi_client_disconnect_script(mi); + } +@@ -788,7 +788,7 @@ multi_create_instance(struct multi_context *m, const struct mroute_addr *real) + goto err; + } + +- mi->context.c2.context_auth = CAS_PENDING; ++ mi->context.c2.tls_multi->multi_state = CAS_PENDING; + + if (hash_n_elements(m->hash) >= m->max_clients) + { +@@ -2436,18 +2436,18 @@ multi_client_connect_late_setup(struct multi_context *m, + mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local; + + /* set context-level authentication flag */ +- mi->context.c2.context_auth = CAS_SUCCEEDED; ++ mi->context.c2.tls_multi->multi_state = CAS_SUCCEEDED; + + /* authentication complete, calculate dynamic client specific options */ + if (!multi_client_set_protocol_options(&mi->context)) + { +- mi->context.c2.context_auth = CAS_FAILED; ++ mi->context.c2.tls_multi->multi_state = CAS_FAILED; + } + /* Generate data channel keys only if setting protocol options + * has not failed */ + else if (!multi_client_generate_tls_keys(&mi->context)) + { +- mi->context.c2.context_auth = CAS_FAILED; ++ mi->context.c2.tls_multi->multi_state = CAS_FAILED; + } + + /* send push reply if ready */ +@@ -2595,7 +2595,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) + + /* We are only called for the CAS_PENDING_x states, so we + * can ignore other states here */ +- bool from_deferred = (mi->context.c2.context_auth != CAS_PENDING); ++ bool from_deferred = (mi->context.c2.tls_multi->multi_state != CAS_PENDING); + + int *cur_handler_index = &mi->client_connect_defer_state.cur_handler_index; + unsigned int *option_types_found = +@@ -2607,7 +2607,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) + *cur_handler_index = 0; + *option_types_found = 0; + /* Initially we have no handler that has returned a result */ +- mi->context.c2.context_auth = CAS_PENDING_DEFERRED; ++ mi->context.c2.tls_multi->multi_state = CAS_PENDING_DEFERRED; + + multi_client_connect_early_setup(m, mi); + } +@@ -2630,7 +2630,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) + * Remember that we already had at least one handler + * returning a result should we go to into deferred state + */ +- mi->context.c2.context_auth = CAS_PENDING_DEFERRED_PARTIAL; ++ mi->context.c2.tls_multi->multi_state = CAS_PENDING_DEFERRED_PARTIAL; + break; + + case CC_RET_SKIPPED: +@@ -2682,12 +2682,12 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) + { + /* run the disconnect script if we had a connect script that + * did not fail */ +- if (mi->context.c2.context_auth == CAS_PENDING_DEFERRED_PARTIAL) ++ if (mi->context.c2.tls_multi->multi_state == CAS_PENDING_DEFERRED_PARTIAL) + { + multi_client_disconnect_script(mi); + } + +- mi->context.c2.context_auth = CAS_FAILED; ++ mi->context.c2.tls_multi->multi_state = CAS_FAILED; + } + + /* increment number of current authenticated clients */ +@@ -2990,13 +2990,13 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns + { + /* connection is "established" when SSL/TLS key negotiation succeeds + * and (if specified) auth user/pass succeeds */ +- if (is_cas_pending(mi->context.c2.context_auth) ++ if (is_cas_pending(mi->context.c2.tls_multi->multi_state) + && CONNECTION_ESTABLISHED(&mi->context)) + { + multi_connection_established(m, mi); + } + #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH) +- if (is_cas_pending(mi->context.c2.context_auth) ++ if (is_cas_pending(mi->context.c2.tls_multi->multi_state) + && mi->client_connect_defer_state.deferred_ret_file) + { + add_inotify_file_watch(m, mi, m->top.c2.inotify_fd, +@@ -3953,7 +3953,7 @@ management_client_auth(void *arg, + { + if (auth) + { +- if (is_cas_pending(mi->context.c2.context_auth)) ++ if (is_cas_pending(mi->context.c2.tls_multi->multi_state)) + { + set_cc_config(mi, cc_config); + cc_config_owned = false; +@@ -3965,7 +3965,7 @@ management_client_auth(void *arg, + { + msg(D_MULTI_LOW, "MULTI: connection rejected: %s, CLI:%s", reason, np(client_reason)); + } +- if (!is_cas_pending(mi->context.c2.context_auth)) ++ if (!is_cas_pending(mi->context.c2.tls_multi->multi_state)) + { + send_auth_failed(&mi->context, client_reason); /* mid-session reauth failed */ + multi_schedule_context_wakeup(m, mi); +diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h +index a7b597749..d131ac59e 100644 +--- a/src/openvpn/openvpn.h ++++ b/src/openvpn/openvpn.h +@@ -211,17 +211,6 @@ struct context_1 + }; + + +-/* client authentication state, CAS_SUCCEEDED must be 0 since +- * non multi code path still checks this variable but does not initialise it +- * so the code depends on zero initialisation */ +-enum client_connect_status { +- CAS_SUCCEEDED=0, +- CAS_PENDING, +- CAS_PENDING_DEFERRED, +- CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/ +- CAS_FAILED, +-}; +- + static inline bool + is_cas_pending(enum client_connect_status cas) + { +@@ -458,9 +447,6 @@ struct context_2 + int push_ifconfig_ipv6_netbits; + struct in6_addr push_ifconfig_ipv6_remote; + +- +- enum client_connect_status context_auth; +- + struct event_timeout push_request_interval; + int n_sent_push_requests; + bool did_pre_pull_restore; +diff --git a/src/openvpn/push.c b/src/openvpn/push.c +index e0d2eeaf2..c47f4c8b6 100644 +--- a/src/openvpn/push.c ++++ b/src/openvpn/push.c +@@ -733,13 +733,14 @@ process_incoming_push_request(struct context *c) + { + int ret = PUSH_MSG_ERROR; + +- if (tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED || c->c2.context_auth == CAS_FAILED) ++ if (tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED ++ || c->c2.tls_multi->multi_state == CAS_FAILED) + { + const char *client_reason = tls_client_reason(c->c2.tls_multi); + send_auth_failed(c, client_reason); + ret = PUSH_MSG_AUTH_FAILURE; + } +- else if (c->c2.context_auth == CAS_SUCCEEDED) ++ else if (c->c2.tls_multi->multi_state == CAS_SUCCEEDED) + { + time_t now; + +diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h +index a703f97cd..06c32ac1d 100644 +--- a/src/openvpn/ssl_common.h ++++ b/src/openvpn/ssl_common.h +@@ -479,6 +479,19 @@ struct tls_session + */ + #define KEY_SCAN_SIZE 3 + ++ ++/* client authentication state, CAS_SUCCEEDED must be 0 since ++ * non multi code path still checks this variable but does not initialise it ++ * so the code depends on zero initialisation */ ++enum client_connect_status { ++ CAS_SUCCEEDED=0, ++ CAS_PENDING, ++ CAS_PENDING_DEFERRED, ++ CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/ ++ CAS_FAILED, ++}; ++ ++ + /** + * Security parameter state for a single VPN tunnel. + * @ingroup control_processor +@@ -519,6 +532,7 @@ struct tls_multi + + int n_sessions; /**< Number of sessions negotiated thus + * far. */ ++ enum client_connect_status multi_state; + + /* + * Number of errors. diff --git a/debian/patches/CVE-2020-15078-1.patch b/debian/patches/CVE-2020-15078-1.patch new file mode 100644 index 0000000..9acfc27 --- /dev/null +++ b/debian/patches/CVE-2020-15078-1.patch @@ -0,0 +1,390 @@ +From 3aca477a1b58714754fea3a26d0892fffc51db6b Mon Sep 17 00:00:00 2001 +From: Arne Schwabe <arne@rfc2549.org> +Date: Sat, 27 Mar 2021 18:47:24 +0100 +Subject: [PATCH] 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. + +2nd patch, squashed into the first one: + +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. + +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> +--- + doc/man-sections/example-fingerprint.rst | 0 + src/openvpn/auth_token.c | 12 +-- + src/openvpn/ssl_common.h | 4 +- + src/openvpn/ssl_verify.c | 8 +- + tests/unit_tests/openvpn/test_auth_token.c | 91 +++++++++++----------- + 5 files changed, 60 insertions(+), 55 deletions(-) + create mode 100644 doc/man-sections/example-fingerprint.rst + +diff --git a/doc/man-sections/example-fingerprint.rst b/doc/man-sections/example-fingerprint.rst +new file mode 100644 +index 000000000..e69de29bb +diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c +index cc70c06c3..0ea6d1832 100644 +--- 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 06c32ac1d..d6fd50bd3 100644 +--- a/src/openvpn/ssl_common.h ++++ b/src/openvpn/ssl_common.h +@@ -166,6 +166,8 @@ enum ks_auth_state { + 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. +@@ -582,8 +584,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 33115eb6c..6fd51505e 100644 +--- a/src/openvpn/ssl_verify.c ++++ b/src/openvpn/ssl_verify.c +@@ -1269,7 +1269,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) + { + /* +@@ -1278,7 +1278,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 +@@ -1373,8 +1373,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 dbde86318..69fc1f8c9 100644 +--- 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 diff --git a/debian/patches/CVE-2020-15078-2.patch b/debian/patches/CVE-2020-15078-2.patch new file mode 100644 index 0000000..4bd7df4 --- /dev/null +++ b/debian/patches/CVE-2020-15078-2.patch @@ -0,0 +1,125 @@ +From 3d18e308c4e7e6f7ab7c2826c70d2d07b031c18a Mon Sep 17 00:00:00 2001 +From: Arne Schwabe <arne@rfc2549.org> +Date: Sat, 27 Mar 2021 19:35:44 +0100 +Subject: [PATCH] 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> +--- + src/openvpn/ssl_verify.c | 64 +++++++++++++++++++++++++++------------- + 1 file changed, 43 insertions(+), 21 deletions(-) + +diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c +index 6fd51505e..55e7fedc0 100644 +--- 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 + { diff --git a/debian/patches/CVE-2020-15078-3.patch b/debian/patches/CVE-2020-15078-3.patch new file mode 100644 index 0000000..6d4414a --- /dev/null +++ b/debian/patches/CVE-2020-15078-3.patch @@ -0,0 +1,51 @@ +From f7b3bf067ffce72e7de49a4174fd17a3a83f0573 Mon Sep 17 00:00:00 2001 +From: Arne Schwabe <arne@rfc2549.org> +Date: Tue, 6 Apr 2021 00:14:47 +0200 +Subject: [PATCH] Ensure key state is authenticated before sending push reply + +This ensures that the key state is authenticated when sending +a push reply. + +This bug allows - under very specific circumstances - to trick a +server using delayed authentication (plugin or management) into +returning a PUSH_REPLY before the AUTH_FAILED message, which can +possibly be used to gather information about a VPN setup. + +In combination with "--auth-gen-token" or user-specific token auth +solutions it can be possible to 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: Gert Doering <gert@greenie.muc.de> +Message-Id: <d25ec73f-2ab0-31df-8cb6-7778000f4822@openvpn.net> +URL: non-public, embargoed +Signed-off-by: Gert Doering <gert@greenie.muc.de> +--- + src/openvpn/push.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/openvpn/push.c b/src/openvpn/push.c +index c47f4c8b6..2147aca0c 100644 +--- a/src/openvpn/push.c ++++ b/src/openvpn/push.c +@@ -732,6 +732,7 @@ int + process_incoming_push_request(struct context *c) + { + int ret = PUSH_MSG_ERROR; ++ struct key_state *ks = &c->c2.tls_multi->session[TM_ACTIVE].key[KS_PRIMARY]; + + if (tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED + || c->c2.tls_multi->multi_state == CAS_FAILED) +@@ -740,7 +741,8 @@ process_incoming_push_request(struct context *c) + send_auth_failed(c, client_reason); + ret = PUSH_MSG_AUTH_FAILURE; + } +- else if (c->c2.tls_multi->multi_state == CAS_SUCCEEDED) ++ else if (c->c2.tls_multi->multi_state == CAS_SUCCEEDED ++ && ks->authenticated == KS_AUTH_TRUE) + { + time_t now; + diff --git a/debian/patches/series b/debian/patches/series index 55bae8e..692c800 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -5,3 +5,7 @@ openvpn-pkcs11warn.patch #kfreebsd_support.patch match-manpage-and-command-help.patch systemd.patch +CVE-2020-15078-0.patch +CVE-2020-15078-1.patch +CVE-2020-15078-2.patch +CVE-2020-15078-3.patch |