From patchwork Thu May 20 05:11:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1828 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director8.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id 0Kq9F3N8pmAIbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:51 -0400 Received: from proxy2.mail.ord1d.rsapps.net ([172.30.191.6]) by director8.mail.ord1d.rsapps.net with LMTP id SNWJF3N8pmAHdgAAfY0hYg (envelope-from ) for ; Thu, 20 May 2021 11:12:51 -0400 Received: from smtp16.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy2.mail.ord1d.rsapps.net with LMTPS id SKooF3N8pmBpUgAAfawv4w (envelope-from ) for ; Thu, 20 May 2021 11:12:51 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp16.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d7aa5c8e-b97d-11eb-95ee-bc305bf036a4-1-1 Received: from [216.105.38.7] ([216.105.38.7:47758] helo=lists.sourceforge.net) by smtp16.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 09/1A-17694-27C76A06; Thu, 20 May 2021 11:12:51 -0400 Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1ljkLF-00065H-Mh; Thu, 20 May 2021 15:12:05 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ljkLC-00064N-Ea for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=rQKYSB8EFjYRMuJtOFYs/sDWAG1T2PWwyGImsQWwlbE=; b=iJSixp9ZTp8VGhZyPvhdLiW83x OpISHn7JNP1j1kRFdNIn/nQRwWHA8Ojn6kGSufq9Z8FcfAN7FeTYdotdEoJYUN0/2uNEoLosfBXZI VoZbOgODGSXGnZSGeDQAiAxICovgAGHznCVQRuo/S29+x1i1Q+UdH9dE6EOWPowuLND4=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject:To:From: Sender:Reply-To:Cc:Content-Type:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=rQKYSB8EFjYRMuJtOFYs/sDWAG1T2PWwyGImsQWwlbE=; b=f RT+kxodhJ0bPlEvdGmEbv8AWXcnytY51ePeDJNUKKwDneUsrVUdi7DotmvN5oyyHq9Hv5dKFfiYkB sCUtRvMio4ZC1odkav7TGE2g1P+bvJ7CPoWqowrwkKPsPCGBrm1JqOCj/39j+Vag4fwdd3aOnGT+6 Yn2uliQmeI6dDIVM=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1ljkL4-0008Ik-AI for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:03 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2E-7W for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565625 invoked by uid 10006); Thu, 20 May 2021 15:11:48 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:40 +0200 Message-Id: <20210520151148.2565578-1-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL4-0008Ik-AI Subject: [Openvpn-devel] [PATCH v2 1/9] Move auth_token_state from multi to key_state X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox 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 Acked-by: Antonio Quartulli --- 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 +++++++++++----------- 4 files changed, 60 insertions(+), 55 deletions(-) diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c index d571b686e..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 61cae7419..53325e556 100644 --- 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 0b41eea2d..7992a6eb9 100644 --- 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 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 From patchwork Thu May 20 05:11:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1825 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director15.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id yLBgK258pmD1bgAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:46 -0400 Received: from proxy20.mail.ord1d.rsapps.net ([172.30.191.6]) by director15.mail.ord1d.rsapps.net with LMTP id GAEMK258pmCrCQAAIcMcQg (envelope-from ) for ; Thu, 20 May 2021 11:12:46 -0400 Received: from smtp13.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy20.mail.ord1d.rsapps.net with LMTPS id CA+bKm58pmACbAAAsk8m8w (envelope-from ) for ; Thu, 20 May 2021 11:12:46 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp13.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d4f6b140-b97d-11eb-bb8f-bc305bf03494-1-1 Received: from [216.105.38.7] ([216.105.38.7:47698] helo=lists.sourceforge.net) by smtp13.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id CE/E2-04910-E6C76A06; Thu, 20 May 2021 11:12:46 -0400 Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1ljkL9-00063n-Am; Thu, 20 May 2021 15:11:59 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ljkL5-00063Z-Oj for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:55 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Jz/B1vqvL85YVauSGcUkOYes+H0HjsYzIo/sMmNVlMU=; b=g63vj+Jtn5EnnqPWyGqTFdlwle 4NBdOf4GEUfD8EOwk8GASEj+xgPexXERQup5tjLevHeBtb6mcxNFFGqDTzO/hRrc+cyOhaLwsMziM YaXNhh8ctGUs1O0BK0BBvn6T34HnUaD8SE64wZcODZ94SmHEa6Z45YQ/cVFN4Immmulw=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=Jz/B1vqvL85YVauSGcUkOYes+H0HjsYzIo/sMmNVlMU=; b=fOkJW0XxBx3yOFjTnLCv8LXwUe rP4yWaPWTfBZE18aaL40c+9RXOUL1Ifcj0FWR7k7REcKhnmUeL64NelHNLIphdjT/v36AZ1Oq6fsv 2w8Ay3G/czQFlvik11XuCozbd0Kf2/sJjX62yxbwCgM5pY5fFXurluBoqOgHsvojPnGw=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1ljkL5-00GeSk-AF for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:57 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2H-9i for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565628 invoked by uid 10006); Thu, 20 May 2021 15:11:48 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:41 +0200 Message-Id: <20210520151148.2565578-2-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL5-00GeSk-AF Subject: [Openvpn-devel] [PATCH v2 2/9] Implement auth-token-user X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox When not using username and password (i.e. auth-user-pass) it can still make to provide the client with an auth-token, e.g. for allowing a session to continue after a reconnect without requiring 2FA again. However, without --auth-user-pass openvpn does not have a username and will ignore any pushed auth-token command. This patch adds support for auth-token-user to set the username that should be used for auth-token The spec of using auth-token-user base64-encoded-user are the ones that OpenVPN3 already implements. Patch V2: Improve style, fix comments and commit message Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- doc/man-sections/client-options.rst | 8 +++++++ src/openvpn/misc.c | 37 +++++++++++++++++++++++++---- src/openvpn/misc.h | 21 +++++++++++++--- src/openvpn/options.c | 5 ++++ src/openvpn/ssl.c | 12 +++++++--- src/openvpn/ssl.h | 2 ++ 6 files changed, 74 insertions(+), 11 deletions(-) diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst index af21fbcd7..c5b7ad960 100644 --- a/doc/man-sections/client-options.rst +++ b/doc/man-sections/client-options.rst @@ -50,6 +50,14 @@ configuration. after a failed auth. Older clients will keep using the token value and react according to ``--auth-retry`` +--auth-token-user base64username + Companion option to ``--auth-token``. This options allows to override + the username used by the client when reauthenticating with the ``auth-token``. + It also allows to use ``--auth-token`` in setups that normally do not use + username and password. + + The username has to be base64 encoded. + --auth-user-pass Authenticate with server using username/password. diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 650daa0c6..29061cd6f 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -490,22 +490,49 @@ void set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token) { - if (strlen(token) && (up->defined || tk->defined)) + if (strlen(token)) { - /* auth-token has no password, so it needs the username - * either already set or copied from up */ strncpynt(tk->password, token, USER_PASS_LEN); - if (up->defined) + tk->token_defined = true; + + /* + * --auth-token has no username, so it needs the username + * either already set or copied from up, or later set by + * --auth-token-user + * + * Do not overwrite the username if already set to avoid + * overwriting an auth-token + */ + if (up->defined && !tk->defined) { strncpynt(tk->username, up->username, USER_PASS_LEN); + tk->defined = true; } - tk->defined = true; } /* Cleans user/pass for nocache */ purge_user_pass(up, false); } +void +set_auth_token_user(struct user_pass *tk, const char *username) +{ + if (strlen(username)) + { + /* Clear the username before decoding to ensure no old material is left + * and also allow decoding to not use all space to ensure the last byte is + * always 0 */ + CLEAR(tk->username); + int len = openvpn_base64_decode(username, tk->username, USER_PASS_LEN - 1); + tk->defined = len > 0; + if (!tk->defined) + { + msg(D_PUSH, "Error decoding auth-token-username"); + } + } +} + + /* * Process string received by untrusted peer before * printing to console or log file. diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index d9005353e..0d2d42489 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -56,6 +56,9 @@ const char *hostname_randomize(const char *hostname, struct gc_arena *gc); struct user_pass { bool defined; + /* For auth-token username and token can be set individually, so + * we this second bool to track if the token (password) is defined */ + bool token_defined; bool nocache; /* max length of username/password */ @@ -138,19 +141,31 @@ void fail_user_pass(const char *prefix, void purge_user_pass(struct user_pass *up, const bool force); /** - * Sets the auth-token to token if a username is available from either - * up or already present in tk. The method will also purge up if + * Sets the auth-token to token. Ff a username is available from either + * up or already present in tk is the auth-token that will be used as default + * username for the token. The method will also purge up if * the auth-nocache option is active. * * @param up (non Auth-token) Username/password * @param tk auth-token userpass to set - * @param token token to use as password for the + * @param token token to use as password for the auth-token * * @note all parameters to this function must not be null. */ void set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token); +/** + * Sets the auth-token username by base64 decoding the passed + * username + * + * @param tk auth-token userpass to set + * @param username base64 encoded username to set + * + * @note all parameters to this function must not be null. + */ +void set_auth_token_user(struct user_pass *tk, const char *username); + /* * Process string received by untrusted peer before * printing to console or log file. diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 5a6f37d7d..fa3ee50d6 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8308,6 +8308,11 @@ add_option(struct options *options, } #endif } + else if (streq(p[0], "auth-token-user") && p[1] && !p[2]) + { + VERIFY_PERMISSION(OPT_P_ECHO); + ssl_set_auth_token_user(p[1]); + } else if (streq(p[0], "single-session") && !p[1]) { VERIFY_PERMISSION(OPT_P_GENERAL); diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index e843b2152..7eb356ddf 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -446,6 +446,12 @@ ssl_set_auth_token(const char *token) set_auth_token(&auth_user_pass, &auth_token, token); } +void +ssl_set_auth_token_user(const char *username) +{ + set_auth_token_user(&auth_token, username); +} + /* * Cleans an auth token and checks if it was active */ @@ -2310,8 +2316,8 @@ key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct tls_sessi } } - /* write username/password if specified */ - if (auth_user_pass_enabled) + /* write username/password if specified or we are using a auth-token */ + if (auth_user_pass_enabled || (auth_token.token_defined && auth_token.defined)) { #ifdef ENABLE_MANAGEMENT auth_user_pass_setup(session->opt->auth_user_pass_file, session->opt->sci); @@ -2324,7 +2330,7 @@ key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct tls_sessi * If we have a valid auth-token, send that instead of real * username/password */ - if (auth_token.defined) + if (auth_token.token_defined && auth_token.defined) { up = &auth_token; } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 88f89876a..81cc22131 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -450,6 +450,8 @@ void ssl_purge_auth(const bool auth_user_pass_only); void ssl_set_auth_token(const char *token); +void ssl_set_auth_token_user(const char *username); + bool ssl_clean_auth_token(void); #ifdef ENABLE_MANAGEMENT From patchwork Thu May 20 05:11:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1826 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director12.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id eGs5Lm58pmAJbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:46 -0400 Received: from proxy7.mail.ord1d.rsapps.net ([172.30.191.6]) by director12.mail.ord1d.rsapps.net with LMTP id qEcULm58pmBMAwAAIasKDg (envelope-from ) for ; Thu, 20 May 2021 11:12:46 -0400 Received: from smtp36.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy7.mail.ord1d.rsapps.net with LMTPS id WPHsLW58pmC/OAAAMe1Fpw (envelope-from ) for ; Thu, 20 May 2021 11:12:46 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp36.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d447dc74-b97d-11eb-9a85-5452006630bd-1-1 Received: from [216.105.38.7] ([216.105.38.7:43234] helo=lists.sourceforge.net) by smtp36.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 5C/4F-10362-D6C76A06; Thu, 20 May 2021 11:12:45 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.92.3) (envelope-from ) id 1ljkL8-00044R-T5; Thu, 20 May 2021 15:11:58 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) (envelope-from ) id 1ljkL6-00044I-UU for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:56 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=XzS+IK6ZdD0Dqmu+1hpeYPhbY+m6F/LaBEoXSlakNPk=; b=TK4ySlkIYJZClYvtkCfqtGTDMx 30ERR49DD2m8wmVVCKHjLxHy0VHFriVM61d1KKngd14P9hglewF0AN2DnX/NjSTEVLNvgJ1XvPE8q RVfEQmmmaDaV6qMyE37uI3vnOGH2MqNOKguJqYj6UM3ts8YF/CVvaXb7fk8R9U0uV1rE=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=XzS+IK6ZdD0Dqmu+1hpeYPhbY+m6F/LaBEoXSlakNPk=; b=MveLkmGPZ5orZ9KhraKKepUjEH KexM6Jh36IOaxj6YZsAQyx/wxbjhVswvGIOd5wp/cDKq6p0GQ3SMjjsJax1RCjyVFnLLSJoMiEeCT mQJoifEWaPzxYLUPGTpKj0O2Q69qnsmppbn5NB6/XwTQvJSNTqLtuNRiMDyOIsWWUZq4=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1ljkL4-0008Im-AR for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:56 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2K-CF for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565632 invoked by uid 10006); Thu, 20 May 2021 15:11:48 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:42 +0200 Message-Id: <20210520151148.2565578-3-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL4-0008Im-AR Subject: [Openvpn-devel] [PATCH v2 3/9] Add connection_established as state in tls_multi->context_auth X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox the socket_info->connection_establish is set through link_socket_set_outgoing_addr when we reach FULL_SYNC. This patch introduces a new state in context_auth that replaces the connection_established state for TLS connections. This make the state machine easier to understand. Patch v2: fix p2p mode server without (without ncp) Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- src/openvpn/forward.c | 6 +++--- src/openvpn/forward.h | 13 ++++++++++++- src/openvpn/multi.c | 15 ++++++++------- src/openvpn/occ.c | 2 +- src/openvpn/openvpn.h | 4 +++- src/openvpn/push.c | 2 +- src/openvpn/ssl.c | 38 ++++++++++++++++++++++++++++---------- src/openvpn/ssl_common.h | 12 +++++++++--- 8 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index e302d8e0a..692ca7271 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -280,7 +280,7 @@ void check_connection_established(struct context *c) { - if (CONNECTION_ESTABLISHED(c)) + if (connection_established(c)) { /* if --pull was specified, send a push request to server */ if (c->c2.tls_multi && c->options.pull) @@ -536,7 +536,7 @@ encrypt_sign(struct context *c, bool comp_frag) * has not yet succeeded. In non-TLS tls_multi mode is not defined * and we always pass packets. */ - if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED) + if (c->c2.tls_multi && c->c2.tls_multi->multi_state < CAS_CONNECT_DONE) { c->c2.buf.len = 0; } @@ -971,7 +971,7 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo * has not yet succeeded. In non-TLS mode tls_multi is not defined * and we always pass packets. */ - if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED) + if (c->c2.tls_multi && c->c2.tls_multi->multi_state < CAS_CONNECT_DONE) { c->c2.buf.len = 0; } diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 2a67c1445..3461e6422 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -445,6 +445,17 @@ io_wait(struct context *c, const unsigned int flags) } } -#define CONNECTION_ESTABLISHED(c) (get_link_socket_info(c)->connection_established) +static inline bool +connection_established(struct context *c) +{ + if (c->c2.tls_multi) + { + return c->c2.tls_multi->multi_state >= CAS_CONNECT_DONE; + } + else + { + return get_link_socket_info(c)->connection_established; + } +} #endif /* FORWARD_H */ diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index def5dd40b..3f9710134 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -674,7 +674,8 @@ multi_close_instance(struct multi_context *m, #ifdef ENABLE_MANAGEMENT set_cc_config(mi, NULL); #endif - if (mi->context.c2.tls_multi->multi_state == CAS_SUCCEEDED) + + if (mi->context.c2.tls_multi->multi_state >= CAS_CONNECT_DONE) { multi_client_disconnect_script(mi); } @@ -775,7 +776,7 @@ multi_create_instance(struct multi_context *m, const struct mroute_addr *real) goto err; } - mi->context.c2.tls_multi->multi_state = CAS_PENDING; + mi->context.c2.tls_multi->multi_state = CAS_NOT_CONNECTED; if (hash_n_elements(m->hash) >= m->max_clients) { @@ -2407,7 +2408,7 @@ 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.tls_multi->multi_state = CAS_SUCCEEDED; + mi->context.c2.tls_multi->multi_state = CAS_CONNECT_DONE; /* authentication complete, calculate dynamic client specific options */ if (!multi_client_set_protocol_options(&mi->context)) @@ -2649,9 +2650,9 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) case CC_RET_DEFERRED: /* - * we already set client_connect_status to DEFERRED_RESULT or + * we already set multi_status to DEFERRED_RESULT or * DEFERRED_NO_RESULT. We just return - * from the function as having client_connect_status + * from the function as having multi_status */ return; @@ -3003,8 +3004,8 @@ 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.tls_multi->multi_state) - && CONNECTION_ESTABLISHED(&mi->context)) + + if (is_cas_pending(mi->context.c2.tls_multi->multi_state)) { multi_connection_established(m, mi); } diff --git a/src/openvpn/occ.c b/src/openvpn/occ.c index 3ff351aa0..c169838e1 100644 --- a/src/openvpn/occ.c +++ b/src/openvpn/occ.c @@ -185,7 +185,7 @@ check_send_occ_req_dowork(struct context *c) void check_send_occ_load_test_dowork(struct context *c) { - if (CONNECTION_ESTABLISHED(c)) + if (connection_established(c)) { const struct mtu_load_test *entry; diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index 6e4651cf5..2b74975fa 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -206,7 +206,7 @@ struct context_1 static inline bool -is_cas_pending(enum client_connect_status cas) +is_cas_pending(enum multi_status cas) { return cas == CAS_PENDING || cas == CAS_PENDING_DEFERRED || cas == CAS_PENDING_DEFERRED_PARTIAL; @@ -237,6 +237,8 @@ struct context_2 struct link_socket *link_socket; /* socket used for TCP/UDP connection to remote */ bool link_socket_owned; + + /** This variable is used instead link_socket->info for P2MP UDP childs */ struct link_socket_info *link_socket_info; const struct link_socket *accept_from; /* possibly do accept() on a parent link_socket */ diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 85f9488a5..2d621e472 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -867,7 +867,7 @@ 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_CONNECT_DONE) { time_t now; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 7eb356ddf..9f3f83f16 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2381,19 +2381,22 @@ key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct tls_sessi * If we're a p2mp server to allow NCP, the first key * generation is postponed until after the connect script finished and the * NCP options can be processed. Since that always happens at after connect - * script options are available the CAS_SUCCEEDED status is identical to - * NCP options are processed and we have no extra state for NCP finished. + * script options are available the CAS_CONNECT_DONE status is identical to + * NCP options are processed and do not wait for NCP being finished. */ - if (session->opt->server && (session->opt->mode != MODE_SERVER - || multi->multi_state == CAS_SUCCEEDED)) + if (ks->authenticated > KS_AUTH_FALSE && session->opt->server + && ((session->opt->mode == MODE_SERVER && multi->multi_state >= CAS_CONNECT_DONE) + || (session->opt->mode == MODE_POINT_TO_POINT && !session->opt->pull))) { - if (ks->authenticated > KS_AUTH_FALSE) + /* if key_id >= 1, is a renegotiation, so we use the already established + * parameters and do not need to delay anything. */ + + /* key-id == 0 and multi_state >= CAS_CONNECT_DONE is a special case of + * the server reusing the session of a reconnecting client. */ + if (!tls_session_generate_data_channel_keys(session)) { - if (!tls_session_generate_data_channel_keys(session)) - { - msg(D_TLS_ERRORS, "TLS Error: server generate_key_expansion failed"); - goto error; - } + msg(D_TLS_ERRORS, "TLS Error: server generate_key_expansion failed"); + goto error; } } @@ -2801,6 +2804,21 @@ tls_process(struct tls_multi *multi, /* Set outgoing address for data channel packets */ link_socket_set_outgoing_addr(to_link_socket_info, &ks->remote_addr, session->common_name, session->opt->es); + /* Check if we need to advance the tls_multi state machine */ + if (multi->multi_state == CAS_NOT_CONNECTED) + { + if (session->opt->mode == MODE_SERVER) + { + /* On a server we continue with running connect scripts next */ + multi->multi_state = CAS_PENDING; + } + else + { + /* Skip the connect script related states */ + multi->multi_state = CAS_CONNECT_DONE; + } + } + /* Flush any payload packets that were buffered before our state transitioned to S_ACTIVE */ flush_payload_buffer(ks); diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 53325e556..8a65ab984 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -504,12 +504,18 @@ struct tls_session /* 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_NOT_CONNECTED is the initial state for every context. When the *first* + * tls_session reaches S_ACTIVE, this state machine moves to CAS_PENDING (server) + * or CAS_CONNECT_DONE (client/p2p) as clients skip the stages associated with + * connect scripts/plugins */ +enum multi_status { + CAS_NOT_CONNECTED, CAS_PENDING, CAS_PENDING_DEFERRED, CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/ CAS_FAILED, + CAS_CONNECT_DONE, }; @@ -548,7 +554,7 @@ struct tls_multi int n_sessions; /**< Number of sessions negotiated thus * far. */ - enum client_connect_status multi_state; + enum multi_status multi_state; /* * Number of errors. From patchwork Thu May 20 05:11:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1830 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.28.255.1]) by backend30.mail.ord1d.rsapps.net with LMTP id uDuXJHR8pmASbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from proxy4.mail.ord1c.rsapps.net ([172.28.255.1]) by director11.mail.ord1d.rsapps.net with LMTP id kD18JHR8pmBfKwAAvGGmqA (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from smtp40.gate.ord1c ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy4.mail.ord1c.rsapps.net with LMTPS id yPYrJHR8pmBYfwAAjcXvpA (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp40.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d83f2e86-b97d-11eb-afb0-525400b3abc9-1-1 Received: from [216.105.38.7] ([216.105.38.7:47774] helo=lists.sourceforge.net) by smtp40.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id A5/58-04968-37C76A06; Thu, 20 May 2021 11:12:52 -0400 Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1ljkLF-000655-J1; Thu, 20 May 2021 15:12:05 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ljkLC-00064L-EH for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Kou6tw6Wz0WGjV4yntWT2lTeQy/+gBFqaZrSoAxtR1Y=; b=jViEYfCXsY7FysR7xyfZsQjbFA 1fRASFjWuGJJ3q6B+BLjtklD/wC6y+ZyfsOsyP6+rhVntLoAYAtSHAIYhByA2d9dRIW0CNhcczY4x 4Neri0GISCkP+zliSKJo1WfKIdq2JM8cvXcBtgBqKn2+ZaW3RWJns8WqSgMKYFLgG68I=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=Kou6tw6Wz0WGjV4yntWT2lTeQy/+gBFqaZrSoAxtR1Y=; b=ETJ1fVlqmRw1t9e61+t3TcV7iO Hts2FB4rZ54kXaOTE8p4QSxhONK3Pl8DRKxSMvv9P12QdSNug2/26x/mPu8adctICZd5L/QlQIQLf ONNWmBWWHIgMIx9zZXB/WqPNNUpGtAyncSf/zx65WV+9clSQqHZ8EsmCe0BL7TNJRQlA=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1ljkL4-0008In-Hp for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:03 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2N-EZ for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565635 invoked by uid 10006); Thu, 20 May 2021 15:11:48 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:43 +0200 Message-Id: <20210520151148.2565578-4-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL4-0008In-Hp Subject: [Openvpn-devel] [PATCH v2 4/9] Make waiting on auth an explicit state in the context state machine X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox Previously we relied on checking tls_authentication_status to check wether to determine if the context auth state is actually valid or not. This patch eliminates that check by introducing waiting on the authentication as extra state in the context auth, state machine. Signed-off-by: Arne Schwabe --- src/openvpn/multi.c | 5 ----- src/openvpn/ssl.c | 9 ++++++++- src/openvpn/ssl_common.h | 1 + 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 3f9710134..7cb9e86aa 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2596,11 +2596,6 @@ static const multi_client_connect_handler client_connect_handlers[] = { static void multi_connection_established(struct multi_context *m, struct multi_instance *mi) { - if (tls_authentication_status(mi->context.c2.tls_multi) != TLS_AUTHENTICATION_SUCCEEDED) - { - return; - } - /* We are only called for the CAS_PENDING_x states, so we * can ignore other states here */ bool from_deferred = (mi->context.c2.tls_multi->multi_state != CAS_PENDING); diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 9f3f83f16..fd64b8d4e 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2810,7 +2810,7 @@ tls_process(struct tls_multi *multi, if (session->opt->mode == MODE_SERVER) { /* On a server we continue with running connect scripts next */ - multi->multi_state = CAS_PENDING; + multi->multi_state = CAS_WAITING_AUTH; } else { @@ -3136,6 +3136,13 @@ tls_multi_process(struct tls_multi *multi, enum tls_auth_status tas = tls_authentication_status(multi); + /* If we have successfully authenticated and are still waiting for the authentication to finish + * move the state machine for the multi context forward */ + if (multi->multi_state == CAS_WAITING_AUTH && tas == TLS_AUTHENTICATION_SUCCEEDED) + { + multi->multi_state = CAS_PENDING; + } + /* * If lame duck session expires, kill it. */ diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 8a65ab984..66700bf68 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -511,6 +511,7 @@ struct tls_session * connect scripts/plugins */ enum multi_status { CAS_NOT_CONNECTED, + CAS_WAITING_AUTH, /**< TLS connection established but deferred auth not finished */ CAS_PENDING, CAS_PENDING_DEFERRED, CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/ From patchwork Thu May 20 05:11:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1827 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director7.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id kLngAnN8pmAQbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:51 -0400 Received: from proxy11.mail.ord1d.rsapps.net ([172.30.191.6]) by director7.mail.ord1d.rsapps.net with LMTP id QHygAnN8pmDuNgAAovjBpQ (envelope-from ) for ; Thu, 20 May 2021 11:12:51 -0400 Received: from smtp34.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy11.mail.ord1d.rsapps.net with LMTPS id 6HboJmZ8pmAGYQAAgKDEHA (envelope-from ) for ; Thu, 20 May 2021 11:12:38 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp34.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d7634790-b97d-11eb-88b7-545200247500-1-1 Received: from [216.105.38.7] ([216.105.38.7:43284] helo=lists.sourceforge.net) by smtp34.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 2D/04-06762-27C76A06; Thu, 20 May 2021 11:12:50 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.92.3) (envelope-from ) id 1ljkLF-00045q-71; Thu, 20 May 2021 15:12:05 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) (envelope-from ) id 1ljkLD-00045I-PO for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:03 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=GQoD5PxNfXGsBjvmTUCXkbDH/EU5Xmbm6g5t3kD3iC0=; b=SXLi8MQI93uI8ZkNMWpprpxqiF 8o+dGM/IG9v6399owemqRTtrjIfwbSBXMONuqyLn0UXr7A1eIKMxx/7JFoftznWCnh/bvF62x7ztE kXs5a0K1gUqBFDYtwFPqKmCW3aP8mhjHpCbR3z8ZI1yB7OGF2B3UVKDupi7aAG0WVHgE=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=GQoD5PxNfXGsBjvmTUCXkbDH/EU5Xmbm6g5t3kD3iC0=; b=JiGpe61wC1zdaDREvkCJFSKPg7 OF7j82d+UkrKEPzE1pi4W2Y9v9ckti4XFig9mPjc/l6gY/LOPMOMDnYo1Euy7HDvvPan3Q5FVzDr2 rvuu+hIAeQ53CKDf6b5Cs1LH3vBPzHZm8IKDk1wbnHfUSh3/+oMAO1aZ+eOgCnSLPXA8=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1ljkL5-00GeSm-Ma for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:04 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2Q-Gx for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565638 invoked by uid 10006); Thu, 20 May 2021 15:11:48 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:44 +0200 Message-Id: <20210520151148.2565578-5-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL5-00GeSm-Ma Subject: [Openvpn-devel] [PATCH v2 5/9] Extracting key_state deferred auth status update into function X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox This extract the update of a deferred key status into into own function. Patch v2: Do not ignore auth_deferred_expire. Minor format changes. Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- src/openvpn/ssl_verify.c | 96 ++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 34 deletions(-) diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 7992a6eb9..455a5cd1b 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -1073,6 +1073,60 @@ key_state_test_auth_control_file(struct auth_deferred_status *ads, bool cached) return ACF_DISABLED; } +/** + * This method takes a key_state and if updates the state + * of the key if it is deferred. + * @param cached If auth control files should be tried to be opened or th + * cached results should be used + * @param ks The key_state to update + */ +static void +update_key_auth_status(bool cached, struct key_state *ks) +{ + if (ks->authenticated == KS_AUTH_FALSE) + { + return; + } + else + { + enum auth_deferred_result auth_plugin = ACF_DISABLED; + enum auth_deferred_result auth_script = ACF_DISABLED; + enum auth_deferred_result auth_man = ACF_DISABLED; + auth_plugin = key_state_test_auth_control_file(&ks->plugin_auth, cached); + auth_script = key_state_test_auth_control_file(&ks->script_auth, cached); +#ifdef ENABLE_MANAGEMENT + auth_man = man_def_auth_test(ks); +#endif + ASSERT(auth_plugin < 4 && auth_script < 4 && auth_man < 4); + + if (auth_plugin == ACF_FAILED || auth_script == ACF_FAILED + || auth_man == ACF_FAILED) + { + ks->authenticated = KS_AUTH_FALSE; + return; + } + else if (auth_plugin == ACF_PENDING || auth_script == ACF_PENDING + || auth_man == ACF_PENDING) + { + if (now > ks->auth_deferred_expire) + { + /* Window to authenticate the key has expired, mark + * the key as unauthenticated */ + ks->authenticated = KS_AUTH_FALSE; + } + } + else + { + /* all auth states (auth_plugin, auth_script, auth_man) + * are either ACF_DISABLED or ACF_SUCCEDED now, which + * translates to "not checked" or "auth succeeded" + */ + ks->authenticated = KS_AUTH_TRUE; + } + } +} + + /** * The minimum times to have passed to update the cache. Older versions * of OpenVPN had code path that did not do any caching, so we start @@ -1115,46 +1169,20 @@ tls_authentication_status(struct tls_multi *multi) if (TLS_AUTHENTICATED(multi, ks)) { active++; + update_key_auth_status(cached, ks); + if (ks->authenticated == KS_AUTH_FALSE) { failed_auth = true; } - else + else if (ks->authenticated == KS_AUTH_DEFERRED) { - enum auth_deferred_result auth_plugin = ACF_DISABLED; - enum auth_deferred_result auth_script = ACF_DISABLED; - enum auth_deferred_result auth_man = ACF_DISABLED; - auth_plugin = key_state_test_auth_control_file(&ks->plugin_auth, cached); - auth_script = key_state_test_auth_control_file(&ks->script_auth, cached); -#ifdef ENABLE_MANAGEMENT - auth_man = man_def_auth_test(ks); -#endif - ASSERT(auth_plugin < 4 && auth_script < 4 && auth_man < 4); - if (auth_plugin == ACF_FAILED || auth_script == ACF_FAILED - || auth_man == ACF_FAILED) - { - ks->authenticated = KS_AUTH_FALSE; - failed_auth = true; - } - else if (auth_plugin == ACF_PENDING - || auth_script == ACF_PENDING - || auth_man == ACF_PENDING) - { - if (now < ks->auth_deferred_expire) - { - deferred = true; - } - } - else - { - /* all auth states (auth_plugin, auth_script, auth_man) - * are either ACF_DISABLED or ACF_SUCCEDED now, which - * translates to "not checked" or "auth succeeded" - */ - success = true; - ks->authenticated = KS_AUTH_TRUE; - } + deferred = true; + } + else if (ks->authenticated == KS_AUTH_TRUE) + { + success = true; } } } From patchwork Thu May 20 05:11:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1832 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director13.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id ADf4JY58pmApcAAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:13:18 -0400 Received: from proxy8.mail.ord1d.rsapps.net ([172.30.191.6]) by director13.mail.ord1d.rsapps.net with LMTP id CAO7JY58pmDmOAAA91zNiA (envelope-from ) for ; Thu, 20 May 2021 11:13:18 -0400 Received: from smtp40.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy8.mail.ord1d.rsapps.net with LMTPS id SNhzJY58pmDPSQAAGdz6CA (envelope-from ) for ; Thu, 20 May 2021 11:13:18 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp40.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: e7b7353e-b97d-11eb-afb0-525400b3abc9-1-1 Received: from [216.105.38.7] ([216.105.38.7:35040] helo=lists.sourceforge.net) by smtp40.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 46/A8-04968-D8C76A06; Thu, 20 May 2021 11:13:18 -0400 Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1ljkLb-0005rm-MF; Thu, 20 May 2021 15:12:27 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ljkLH-0005np-56 for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=lHVeNLjrYWawDPD+O7Cn0OJLEmTZzALmpEi00swht0k=; b=NhX6tKsiRMcA6aZJq6Ayj/vZ/L 42202rSlc7wAnh+yEehDYLhgWwkyl1YxiPOi8g4P3VR3mR+nNFTMhFOn3fiqaST1JSlaS1sIOWO4D ANQiO1H1BhRJWYDYLNuHGXEqRf6GlDYYvc3YS/5QLCzG0ZZnJWQG8cZcUmMSoFcWrg2M=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=lHVeNLjrYWawDPD+O7Cn0OJLEmTZzALmpEi00swht0k=; b=Zb51bh6K6Fum6miXmnup97MyA8 aEVMg5Nw8szKYh26jPp0Zi/hV/MDxQ0LolZiQe65nctCAqPQv9sNiCgZXsPQkW/qeV6jfifbAYvcM FvY7niBOhszhoGGkoWoKl73b2vZ7AoEiSxTK4xObMmx9I17NxuuVwAft8vqO12n+t9Fw=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1ljkL4-0008Ir-Gy for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:06 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2T-Jg for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565641 invoked by uid 10006); Thu, 20 May 2021 15:11:49 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:45 +0200 Message-Id: <20210520151148.2565578-6-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL4-0008Ir-Gy Subject: [Openvpn-devel] [PATCH v2 6/9] Introduce S_GENERATED_KEYS state and generate keys only when authenticated X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox Since generating data channel keys does not happen when we have reach the S_ACTIVE/S_GOT_KEY state anymore like it used to be before NCP, the state that data channel keys deserves its own state in the TLS session state machine. The changes done by this commit are rather intrusive since they move the key generation to a completely different place and also rely on the state machine to decide if keys should be generated rather than on the complicated conditions that were implemented in the key_method_2_write/read methods. A (intended) side effect of this change is that sessions that are still in deferred state (ks->authenticated == KS_DEFERRED) will not have data channel keys generated. This avoids corner cases where a not fully authenticated sessions might leak data. Signed-off-by: Arne Schwabe Patch v2: rebased Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- src/openvpn/forward.h | 2 +- src/openvpn/init.c | 1 + src/openvpn/ssl.c | 89 +++++++++++++++++----------------------- src/openvpn/ssl.h | 10 +++++ src/openvpn/ssl_common.h | 9 +++- 5 files changed, 57 insertions(+), 54 deletions(-) diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 3461e6422..b8760099e 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -450,7 +450,7 @@ connection_established(struct context *c) { if (c->c2.tls_multi) { - return c->c2.tls_multi->multi_state >= CAS_CONNECT_DONE; + return c->c2.tls_multi->multi_state >= CAS_WAITING_OPTIONS_IMPORT; } else { diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 49c742928..4335d4870 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2202,6 +2202,7 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found) } c->c2.do_up_ran = true; + c->c2.tls_multi->multi_state = CAS_CONNECT_DONE; } return true; } diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index fd64b8d4e..b28d8edf8 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -788,6 +788,9 @@ state_name(int state) case S_ERROR: return "S_ERROR"; + case S_GENERATED_KEYS: + return "S_GENERATED_KEYS"; + default: return "S_???"; } @@ -1840,13 +1843,13 @@ key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len) * This erases the source material used to generate the data channel keys, and * can thus be called only once per session. */ -static bool +bool tls_session_generate_data_channel_keys(struct tls_session *session) { bool ret = false; struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - if (ks->authenticated == KS_AUTH_FALSE) + if (ks->authenticated <= KS_AUTH_FALSE) { msg(D_TLS_ERRORS, "TLS Error: key_state not authenticated"); goto cleanup; @@ -1862,6 +1865,9 @@ tls_session_generate_data_channel_keys(struct tls_session *session) tls_limit_reneg_bytes(session->opt->key_type.cipher, &session->opt->renegotiate_bytes); + /* set the state of the keys for the session to generated */ + ks->state = S_GENERATED_KEYS; + ret = true; cleanup: secure_memzero(ks->key_src, sizeof(*ks->key_src)); @@ -2375,31 +2381,6 @@ key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct tls_sessi goto error; } - /* - * Generate tunnel keys if we're a TLS server. - * - * If we're a p2mp server to allow NCP, the first key - * generation is postponed until after the connect script finished and the - * NCP options can be processed. Since that always happens at after connect - * script options are available the CAS_CONNECT_DONE status is identical to - * NCP options are processed and do not wait for NCP being finished. - */ - if (ks->authenticated > KS_AUTH_FALSE && session->opt->server - && ((session->opt->mode == MODE_SERVER && multi->multi_state >= CAS_CONNECT_DONE) - || (session->opt->mode == MODE_POINT_TO_POINT && !session->opt->pull))) - { - /* if key_id >= 1, is a renegotiation, so we use the already established - * parameters and do not need to delay anything. */ - - /* key-id == 0 and multi_state >= CAS_CONNECT_DONE is a special case of - * the server reusing the session of a reconnecting client. */ - if (!tls_session_generate_data_channel_keys(session)) - { - msg(D_TLS_ERRORS, "TLS Error: server generate_key_expansion failed"); - goto error; - } - } - return true; error: @@ -2599,21 +2580,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio setenv_del(session->opt->es, "exported_keying_material"); } - - /* - * Generate tunnel keys if we're a client. - * If --pull is enabled, the first key generation is postponed until after the - * pull/push, so we can process pushed cipher directives. - */ - if (!session->opt->server && (!session->opt->pull || ks->key_id > 0)) - { - if (!tls_session_generate_data_channel_keys(session)) - { - msg(D_TLS_ERRORS, "TLS Error: client generate_key_expansion failed"); - goto error; - } - } - + gc_free(&gc); return true; @@ -2815,7 +2782,7 @@ tls_process(struct tls_multi *multi, else { /* Skip the connect script related states */ - multi->multi_state = CAS_CONNECT_DONE; + multi->multi_state = CAS_WAITING_OPTIONS_IMPORT; } } @@ -3138,6 +3105,27 @@ tls_multi_process(struct tls_multi *multi, /* If we have successfully authenticated and are still waiting for the authentication to finish * move the state machine for the multi context forward */ + + if (multi->multi_state >= CAS_CONNECT_DONE) + { + for (int i = 0; i < TM_SIZE; ++i) + { + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; + + if (ks->state == S_ACTIVE && ks->authenticated == KS_AUTH_TRUE) + { + /* This will 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; + } + } + } + } + if (multi->multi_state == CAS_WAITING_AUTH && tas == TLS_AUTHENTICATION_SUCCEEDED) { multi->multi_state = CAS_PENDING; @@ -3246,11 +3234,10 @@ handle_data_channel_packet(struct tls_multi *multi, * passive side is the server which only listens for the connections, the * active side is the client which initiates connections). */ - if (TLS_AUTHENTICATED(multi, ks) - && key_id == ks->key_id - && (ks->authenticated == KS_AUTH_TRUE) + if (ks->state >= S_GENERATED_KEYS && key_id == ks->key_id && (floated || link_socket_actual_match(from, &ks->remote_addr))) { + ASSERT(ks->authenticated == KS_AUTH_TRUE); if (!ks->crypto_options.key_ctx_bi.initialized) { msg(D_MULTI_DROPPED, @@ -3572,8 +3559,7 @@ tls_pre_decrypt(struct tls_multi *multi, /* * Remote is requesting a key renegotiation */ - if (op == P_CONTROL_SOFT_RESET_V1 - && TLS_AUTHENTICATED(multi, ks)) + if (op == P_CONTROL_SOFT_RESET_V1 && TLS_AUTHENTICATED(multi, ks)) { if (!read_control_auth(buf, &session->tls_wrap, from, session->opt)) @@ -3834,10 +3820,11 @@ struct key_state *tls_select_encryption_key(struct tls_multi *multi) for (int i = 0; i < KEY_SCAN_SIZE; ++i) { struct key_state *ks = get_key_scan(multi, i); - if (ks->state >= S_ACTIVE - && ks->authenticated == KS_AUTH_TRUE - && ks->crypto_options.key_ctx_bi.initialized) + if (ks->state >= S_GENERATED_KEYS) { + ASSERT(ks->authenticated == KS_AUTH_TRUE); + ASSERT(ks->crypto_options.key_ctx_bi.initialized); + if (!ks_select) { ks_select = ks; diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 81cc22131..baf3560d2 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -612,4 +612,14 @@ show_available_tls_ciphers(const char *cipher_list, const char *cipher_list_tls13, const char *tls_cert_profile); + +/** + * Generate data channel keys for the supplied TLS session. + * + * This erases the source material used to generate the data channel keys, and + * can thus be called only once per session. + */ +bool +tls_session_generate_data_channel_keys(struct tls_session *session); + #endif /* ifndef OPENVPN_SSL_H */ diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 66700bf68..928e80929 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -64,7 +64,8 @@ * material. * -# \c S_GOT_KEY, have received remote part of \c key_source2 random * material. - * -# \c S_ACTIVE, normal operation + * -# \c S_ACTIVE, control channel successfully established + * -# \c S_GENERATED_KEYS, the * * Servers follow the same order, except for \c S_SENT_KEY and \c * S_GOT_KEY being reversed, because the server first receives the @@ -92,7 +93,10 @@ #define S_ACTIVE 6 /**< Operational \c key_state state * immediately after negotiation has * completed while still within the - * handshake window. */ + * handshake window, deferred auth, client + * connect and can still + * be pending. */ +#define S_GENERATED_KEYS 7 /**< The data channel keys have been generated */ /* 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 */ @@ -516,6 +520,7 @@ enum multi_status { CAS_PENDING_DEFERRED, CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/ CAS_FAILED, + CAS_WAITING_OPTIONS_IMPORT, /**< client with pull or p2p waiting for first time options import */ CAS_CONNECT_DONE, }; From patchwork Thu May 20 05:11:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1831 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director13.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id QGkbMXR8pmBVbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from proxy17.mail.ord1d.rsapps.net ([172.30.191.6]) by director13.mail.ord1d.rsapps.net with LMTP id IELWMHR8pmAPOQAA91zNiA (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from smtp28.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy17.mail.ord1d.rsapps.net with LMTPS id kFGEMHR8pmB7XwAAWC7mWg (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp28.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d88fe678-b97d-11eb-825d-a0369f1890f1-1-1 Received: from [216.105.38.7] ([216.105.38.7:47778] helo=lists.sourceforge.net) by smtp28.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id AE/35-28313-47C76A06; Thu, 20 May 2021 11:12:52 -0400 Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1ljkLF-00065Z-RF; Thu, 20 May 2021 15:12:05 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ljkLC-00064M-EH for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=bCzBIlOtxdjhS5uvg+57MYc/cEIsBdKr/7omydM3CBU=; b=Cr1TbQ3q3DxA9hLfxBH8jLrpMG UxsU2fLTXxPhLGNHJtxCF7TO2harZxTGFw5XjgImbB22qgGwk2odw0M4YT6tE2igsGlsKXVBIWSUL gfzTael/rqdj65U4slSQv629E+8aFu/RPzMOnaF1D1zfQudBQJNuu2LZ0WNT1hPdTVCo=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=bCzBIlOtxdjhS5uvg+57MYc/cEIsBdKr/7omydM3CBU=; b=IwJbayS7iZXtwjrjq3FlKatcvG w2PgEQ7AQyYF2ae4X1tAWa6RkvBh6akpZ4keaHPyzfDJlrSDkD8wBYA8nI4++l68n02NzNPsVpOsr JH+dFHdWHzdi5YVUIS+0q20J5jnw+2SzrMATaKQ2jgGu1iCj1WQTtehunOj3jBtq8wVo=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1ljkL5-0008Is-Ir for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:03 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2W-M8 for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565644 invoked by uid 10006); Thu, 20 May 2021 15:11:49 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:46 +0200 Message-Id: <20210520151148.2565578-7-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL5-0008Is-Ir Subject: [Openvpn-devel] [PATCH v2 7/9] Move auth_token_state_flags to tls_session and cleanup initial_token X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox The usage of the auth_token_state_flags is tied to the authentication. The other authentication related flags and status are in the tls_session struct instead of the tls_multi struct. Move auth_token_state_flags to the right place. This also changes that auth_token_initial is set when the token is initially generated instead when pushing the token. Even I don't know anymore why I did it in this way in the first place. Also use multi->auth_token_initial as source for the sesssion ID since it should now always be available. Signed-off-by: Arne Schwabe --- src/openvpn/auth_token.c | 28 +++++++++++++++------- src/openvpn/push.c | 8 ------- src/openvpn/ssl_verify.c | 5 +++- tests/unit_tests/openvpn/test_auth_token.c | 15 ++++++++---- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c index 0ea6d1832..60604e6e3 100644 --- a/src/openvpn/auth_token.c +++ b/src/openvpn/auth_token.c @@ -109,11 +109,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 +184,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 +192,18 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi) */ char old_tstamp_decode[9]; + /* Make a copy of the string to not modify multi->auth_token_initial */ + char* initial_token_copy = string_alloc(multi->auth_token_initial, &gc); + /* * 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); + char *old_sessid =initial_token_copy + strlen(SESSION_ID_PREFIX); char *old_tsamp_initial = old_sessid + AUTH_TOKEN_SESSION_ID_LEN*8/6; + + old_tsamp_initial[12] = '\0'; ASSERT(openvpn_base64_decode(old_tsamp_initial, old_tstamp_decode, 9) == 9); @@ -212,10 +217,6 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi) 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); } 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 for clients that ignore + * the updates to the token + */ + multi->auth_token_initial = strdup(multi->auth_token); + } + gc_free(&gc); } diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 2d621e472..98a9c3689 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_verify.c b/src/openvpn/ssl_verify.c index 455a5cd1b..dfb475017 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -1512,6 +1512,7 @@ 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 (session->opt->auth_token_call_auth) { @@ -1640,7 +1641,9 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, * Otherwise the auth-token get pushed out as part of the "normal" * push-reply */ - if (multi->auth_token_initial) + bool initial_connect = session->key[KS_PRIMARY].key_id == 0; + + if (multi->auth_token_initial && !initial_connect) { /* * We do not explicitly schedule the sending of the diff --git a/tests/unit_tests/openvpn/test_auth_token.c b/tests/unit_tests/openvpn/test_auth_token.c index 69fc1f8c9..161bc4499 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), @@ -244,10 +247,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); @@ -305,13 +308,16 @@ auth_token_test_env(void **state) { struct test_context *ctx = (struct test_context *) *state; + 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); assert_string_equal(lastsesion_statevalue, "Initial"); + ks->auth_token_state_flags = 0; strcpy(ctx->up.password, now0key0); add_session_token_env(ctx->session, &ctx->multi, &ctx->up); @@ -331,6 +337,7 @@ auth_token_test_env(void **state) 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"); } @@ -341,13 +348,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); From patchwork Thu May 20 05:11:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1829 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director14.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id WMD4G3R8pmASbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from proxy12.mail.ord1d.rsapps.net ([172.30.191.6]) by director14.mail.ord1d.rsapps.net with LMTP id sAK0G3R8pmB7HwAAeJ7fFg (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from smtp9.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.ord1d.rsapps.net with LMTPS id cNthG3R8pmCICAAA7PHxkg (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp9.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d7d9ba2e-b97d-11eb-8dbd-0026b95bddb7-1-1 Received: from [216.105.38.7] ([216.105.38.7:34366] helo=lists.sourceforge.net) by smtp9.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 7F/0A-18268-37C76A06; Thu, 20 May 2021 11:12:51 -0400 Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1ljkL9-0005kf-GV; Thu, 20 May 2021 15:11:59 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ljkL7-0005k1-Bj for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:57 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=LVZZbUlYJR2WrKM10Tmn2MUIBZ8VdsdV06DeGMok08Y=; b=jF7QGlOvg12tANrm5oyn1MSM/d yuvnLw3kbL8nDJWWdG2RO3fEGi9YrGC+nURDQ6/gHPVlUY/4gKzxvVUGXL8WyzknaH3EbNgPUk+60 gpOj8kx3yxTAOKS6FeBUvHzydoVExq00NO8RGsgudNTeC78jhbUHM7QqH7WHpKLPPtpU=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=LVZZbUlYJR2WrKM10Tmn2MUIBZ8VdsdV06DeGMok08Y=; b=mXAnUWRfutkZPUJz5TpXeNRChY BTM9hHVqV4HukrZYHiHR240ToLp6BL/HwWcQnd4vSs4ItwUYgJ7xfzR+V7prDh0oCIvU+YGyRVKub zJrgGKmrlwK/3O4DmpoqE//QnJ0LyKpZ6oYBFwoLP//lEyB1EFid9CtRq0Ny4u3F8KaY=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1ljkL5-00GeSo-PQ for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:57 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2Z-OZ for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565647 invoked by uid 10006); Thu, 20 May 2021 15:11:49 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:47 +0200 Message-Id: <20210520151148.2565578-8-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL5-00GeSo-PQ Subject: [Openvpn-devel] [PATCH v2 8/9] Remove --ncp-disable option X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox NCP has proven to be stable and apart from the one VPN Provider doing hacky things with homebrewed NCP we have not had any reports about ncp-disable being required. Remove ncp-disable to simplify code paths. Note: This patch breaks client without --pull. The follow up patch for P2P NCP will restore that. But to avoid all the NCP/non-NCP special cases to be implemented in P2P. P2P will directly switch from always non-NCP to always NCP. Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- Changes.rst | 4 +++ doc/man-sections/protocol-options.rst | 8 ++---- src/openvpn/init.c | 17 ++++--------- src/openvpn/multi.c | 4 --- src/openvpn/options.c | 36 +++------------------------ src/openvpn/options.h | 1 - src/openvpn/ssl.c | 3 +-- src/openvpn/ssl_common.h | 1 - src/openvpn/ssl_ncp.c | 4 --- 9 files changed, 16 insertions(+), 62 deletions(-) diff --git a/Changes.rst b/Changes.rst index 9185b55f7..e7ae6abed 100644 --- a/Changes.rst +++ b/Changes.rst @@ -57,6 +57,10 @@ Deprecated features is considered "too complicated", using ``--peer-fingerprint`` makes TLS mode about as easy as using ``--secret``. +``ncp-disable`` has been removed + This option mainly served a role as debug option when NCP was first + introduced. It should now no longer be necessary. + Overview of changes in 2.5 ========================== diff --git a/doc/man-sections/protocol-options.rst b/doc/man-sections/protocol-options.rst index 34d4255ee..5ae780e1f 100644 --- a/doc/man-sections/protocol-options.rst +++ b/doc/man-sections/protocol-options.rst @@ -65,8 +65,8 @@ configured in a compatible way between both the local and remote side. The default is :code:`BF-CBC`, an abbreviation for Blowfish in Cipher Block Chaining mode. When cipher negotiation (NCP) is allowed, OpenVPN 2.4 and newer on both client and server side will automatically - upgrade to :code:`AES-256-GCM`. See ``--data-ciphers`` and - ``--ncp-disable`` for more details on NCP. + upgrade to :code:`AES-256-GCM`. See ``--data-ciphers`` for more details + on NCP. Using :code:`BF-CBC` is no longer recommended, because of its 64-bit block size. This small block size allows attacks based on collisions, as @@ -235,10 +235,6 @@ configured in a compatible way between both the local and remote side. have been configured with `--enable-small` (typically used on routers or other embedded devices). ---ncp-disable - **DEPRECATED** Disable "Negotiable Crypto Parameters". This completely - disables cipher negotiation. - --secret args **DEPRECATED** Enable Static Key encryption mode (non-TLS). Use pre-shared secret ``file`` which was generated with ``--genkey``. diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 4335d4870..38abf9f3a 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2227,18 +2227,14 @@ pull_permission_mask(const struct context *c) | OPT_P_EXPLICIT_NOTIFY | OPT_P_ECHO | OPT_P_PULL_MODE - | OPT_P_PEER_ID; + | OPT_P_PEER_ID + | OPT_P_NCP; if (!c->options.route_nopull) { flags |= (OPT_P_ROUTE | OPT_P_IPWIN32); } - if (c->options.ncp_enabled) - { - flags |= OPT_P_NCP; - } - return flags; } @@ -2720,8 +2716,6 @@ do_init_crypto_tls_c1(struct context *c) * * Therefore, the key structure has to be initialized when: * - any non-BF-CBC cipher was selected; or - * - BF-CBC is selected and NCP is disabled (explicit request to - * use the BF-CBC cipher); or * - BF-CBC is selected, NCP is enabled and fallback is enabled * (BF-CBC will be the fallback). * - BF-CBC is in data-ciphers and we negotiate to use BF-CBC: @@ -2731,12 +2725,12 @@ do_init_crypto_tls_c1(struct context *c) * Note that BF-CBC will still be part of the OCC string to retain * backwards compatibility with older clients. */ - if (!streq(options->ciphername, "BF-CBC") || !options->ncp_enabled - || (options->ncp_enabled && tls_item_in_cipher_list("BF-CBC", options->ncp_ciphers)) + if (!streq(options->ciphername, "BF-CBC") + || tls_item_in_cipher_list("BF-CBC", options->ncp_ciphers) || options->enable_ncp_fallback) { /* Do not warn if the if the cipher is used only in OCC */ - bool warn = !options->ncp_enabled || options->enable_ncp_fallback; + bool warn = options->enable_ncp_fallback; init_key_type(&c->c1.ks.key_type, options->ciphername, options->authname, true, warn); } @@ -2838,7 +2832,6 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.tcp_mode = link_socket_proto_connection_oriented(options->ce.proto); to.config_ciphername = c->options.ciphername; to.config_ncp_ciphers = c->options.ncp_ciphers; - to.ncp_enabled = options->ncp_enabled; to.transition_window = options->transition_window; to.handshake_window = options->handshake_window; to.packet_timeout = options->tls_timeout; diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 7cb9e86aa..2507108e2 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1791,10 +1791,6 @@ multi_client_set_protocol_options(struct context *c) #endif /* Select cipher if client supports Negotiable Crypto Parameters */ - if (!o->ncp_enabled) - { - return true; - } /* if we have already created our key, we cannot *change* our own * cipher -> so log the fact and push the "what we have now" cipher diff --git a/src/openvpn/options.c b/src/openvpn/options.c index fa3ee50d6..2f4ccaa09 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -526,7 +526,6 @@ static const char usage_message[] = " (default=%s).\n" " Set alg=none to disable encryption.\n" "--data-ciphers list : List of ciphers that are allowed to be negotiated.\n" - "--ncp-disable : (DEPRECATED) Disable cipher negotiation.\n" "--prng alg [nsl] : For PRNG, use digest algorithm alg, and\n" " nonce_secret_len=nsl. Set alg=none to disable PRNG.\n" #ifndef ENABLE_CRYPTO_MBEDTLS @@ -843,7 +842,6 @@ init_options(struct options *o, const bool init_gc) o->stale_routes_check_interval = 0; o->ifconfig_pool_persist_refresh_freq = 600; o->scheduled_exit_interval = 5; - o->ncp_enabled = true; o->ncp_ciphers = "AES-256-GCM:AES-128-GCM"; o->authname = "SHA1"; o->prng_hash = "SHA1"; @@ -1719,7 +1717,6 @@ show_settings(const struct options *o) SHOW_STR_INLINE(shared_secret_file); SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false, true), "%s"); SHOW_STR(ciphername); - SHOW_BOOL(ncp_enabled); SHOW_STR(ncp_ciphers); SHOW_STR(authname); SHOW_STR(prng_hash); @@ -3082,7 +3079,6 @@ options_postprocess_cipher(struct options *o) if (!o->pull && !(o->mode == MODE_SERVER)) { /* we are in the classic P2P mode */ - o->ncp_enabled = false; msg( M_WARN, "Cipher negotiation is disabled since neither " "P2MP client nor server mode is enabled"); @@ -3099,18 +3095,6 @@ options_postprocess_cipher(struct options *o) /* pull or P2MP mode */ if (!o->ciphername) { - if (!o->ncp_enabled) - { - msg(M_USAGE, "--ncp-disable needs an explicit --cipher or " - "--data-ciphers-fallback config option"); - } - - msg(M_WARN, "--cipher is not set. Previous OpenVPN version defaulted to " - "BF-CBC as fallback when cipher negotiation failed in this case. " - "If you need this fallback please add '--data-ciphers-fallback " - "BF-CBC' to your configuration and/or add BF-CBC to " - "--data-ciphers."); - /* We still need to set the ciphername to BF-CBC since various other * parts of OpenVPN assert that the ciphername is set */ o->ciphername = "BF-CBC"; @@ -3152,13 +3136,10 @@ options_postprocess_mutate(struct options *o) options_postprocess_cipher(o); options_postprocess_mutate_invariant(o); - if (o->ncp_enabled) + o->ncp_ciphers = mutate_ncp_cipher_list(o->ncp_ciphers, &o->gc); + if (o->ncp_ciphers == NULL) { - o->ncp_ciphers = mutate_ncp_cipher_list(o->ncp_ciphers, &o->gc); - if (o->ncp_ciphers == NULL) - { - msg(M_USAGE, "NCP cipher list contains unsupported ciphers or is too long."); - } + msg(M_USAGE, "NCP cipher list contains unsupported ciphers or is too long."); } if (o->remote_list && !o->connection_list) @@ -3908,8 +3889,7 @@ options_string(const struct options *o, } /* Only announce the cipher to our peer if we are willing to * support it */ - if (p2p_nopull || !o->ncp_enabled - || tls_item_in_cipher_list(ciphername, o->ncp_ciphers)) + if (p2p_nopull || tls_item_in_cipher_list(ciphername, o->ncp_ciphers)) { buf_printf(&out, ",cipher %s", ciphername); } @@ -7994,14 +7974,6 @@ add_option(struct options *options, msg(msglevel, "Unknown key-derivation method %s", p[1]); } } - else if (streq(p[0], "ncp-disable") && !p[1]) - { - VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE); - options->ncp_enabled = false; - msg(M_WARN, "DEPRECATED OPTION: ncp-disable. Disabling " - "cipher negotiation is a deprecated debug feature that " - "will be removed in OpenVPN 2.6"); - } else if (streq(p[0], "prng") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_GENERAL); diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 41e84f7e1..69897c5a7 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -512,7 +512,6 @@ struct options const char *ciphername; bool enable_ncp_fallback; /**< If defined fall back to * ciphername if NCP fails */ - bool ncp_enabled; const char *ncp_ciphers; const char *authname; const char *prng_hash; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index b28d8edf8..dd6e462d0 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2187,8 +2187,7 @@ push_peer_info(struct buffer *buf, struct tls_session *session) } /* support for Negotiable Crypto Parameters */ - if (session->opt->ncp_enabled - && (session->opt->mode == MODE_SERVER || session->opt->pull)) + if (session->opt->mode == MODE_SERVER || session->opt->pull) { if (tls_item_in_cipher_list("AES-128-GCM", session->opt->config_ncp_ciphers) && tls_item_in_cipher_list("AES-256-GCM", session->opt->config_ncp_ciphers)) diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 928e80929..43af51ee9 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -324,7 +324,6 @@ struct tls_options const char *config_ciphername; const char *config_ncp_ciphers; - bool ncp_enabled; bool tls_crypt_v2; const char *tls_crypt_v2_verify_script; diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index f02a3103c..722256b42 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -289,10 +289,6 @@ check_pull_client_ncp(struct context *c, const int found) return true; } - if (!c->options.ncp_enabled) - { - return true; - } /* If the server did not push a --cipher, we will switch to the * remote cipher if it is in our ncp-ciphers list */ if(tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername)) From patchwork Thu May 20 05:11:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1824 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director13.mail.ord1d.rsapps.net ([172.28.255.1]) by backend30.mail.ord1d.rsapps.net with LMTP id 7SLEHG18pmAJbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:45 -0400 Received: from proxy7.mail.ord1c.rsapps.net ([172.28.255.1]) by director13.mail.ord1d.rsapps.net with LMTP id cLD6G218pmAPOQAA91zNiA (envelope-from ) for ; Thu, 20 May 2021 11:12:45 -0400 Received: from smtp15.gate.ord1c ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy7.mail.ord1c.rsapps.net with LMTPS id OCSIG218pmCuSAAAknS3pQ (envelope-from ) for ; Thu, 20 May 2021 11:12:45 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp15.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: d376ae60-b97d-11eb-b0ce-bc305bf03694-1-1 Received: from [216.105.38.7] ([216.105.38.7:43218] helo=lists.sourceforge.net) by smtp15.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 45/78-17546-B6C76A06; Thu, 20 May 2021 11:12:44 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.92.3) (envelope-from ) id 1ljkLF-000465-DD; Thu, 20 May 2021 15:12:05 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) (envelope-from ) id 1ljkLD-00045H-Oh for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:03 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=j3VJ0IYdRs7tgT2VvlTaT09Xs3b9kuEiarK8gzFwy68=; b=CpGmblD/aRD4K8qwhTdtz1FQ4S t+Z11TivEhaLV60HD1QV98ysnm/80bDVl8qaa3zOGpdlntR1RFZ8YrAVeU7SlWkzVy59fb8S0goDL rH9oLdMtnAmCn5Wibj0Qi43U6AgaKUzoCP7uqo5tHsmg2MnfDpgiBXTRxPmxgiIQhhdM=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=j3VJ0IYdRs7tgT2VvlTaT09Xs3b9kuEiarK8gzFwy68=; b=Dx6NlphiqgHHYS0k9K560THkGf 4F72oQwEkxff2yrFNAQUMxfOZg4/4DtRkGYGEqy1QJXVRCnrjTBPyMewUFoLDSBSnSlHMaBCoFv3u uIUMBN3egqBiJeaWWC1YUmiPyyQIMpvZbdC5JeopeSloyCOPwC2MoVAbvUmdl7WYFtzI=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1ljkL5-00GeSq-PG for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:12:04 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2c-Qx for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565650 invoked by uid 10006); Thu, 20 May 2021 15:11:49 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:48 +0200 Message-Id: <20210520151148.2565578-9-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1ljkL5-00GeSq-PG Subject: [Openvpn-devel] [PATCH v2 9/9] Support NCP in pure P2P VPN setups X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox Currently P2P mode of OpenVPN is on of the few places that cannot negotiate modern OpenVPN features. This becomes more and more problematic since P2P and P2MP code diverge more and more and also the lack of switching to more advanced features like Data v2 currently blocks P2P mode from working together with the upcoming ovpn-dco support. This NCP support is a lot simpler and works in the following way: - P2P peer announce an extremely limited IV_ variable set (IV_PROTO and IV_CIPHERS) - Both peers check if the IV_PROTO_NCP_P2P bit is present in IV_PROTO - if yes both sides deterministically determine according to IV_PROTO and IV_CIPHER what options can be used and start using these There are no poor man's NCP or other compatibility workaround like in the normal NCP, making this NCP leaner and more deterministic. Signed-off-by: Arne Schwabe --- src/openvpn/init.c | 100 ++++++++++++++++--- src/openvpn/options.c | 8 +- src/openvpn/ssl.c | 120 +++++++++++++++-------- src/openvpn/ssl.h | 5 + src/openvpn/ssl_backend.h | 1 + src/openvpn/ssl_common.h | 10 ++ src/openvpn/ssl_ncp.c | 145 ++++++++++++++++++++++++++++ src/openvpn/ssl_ncp.h | 25 +++++ tests/unit_tests/openvpn/test_ncp.c | 11 +++ 9 files changed, 366 insertions(+), 59 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 38abf9f3a..5d4852ae1 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -68,6 +68,7 @@ static const char *saved_pid_file_name; /* GLOBAL */ #define CF_INIT_TLS_AUTH_STANDALONE (1<<2) static void do_init_first_time(struct context *c); +static bool do_deferred_p2p_ncp(struct context *c); void context_clear(struct context *c) @@ -2151,6 +2152,14 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found) return false; } } + else if (c->mode == MODE_POINT_TO_POINT) + { + if (!do_deferred_p2p_ncp(c)) + { + msg(D_TLS_ERRORS, "ERROR: Failed to apply P2P negotiated protocol options"); + return false; + } + } /* if --up-delay specified, open tun, do ifconfig, and run up script now */ if (c->options.up_delay || PULL_DEFINED(&c->options)) @@ -2238,6 +2247,72 @@ pull_permission_mask(const struct context *c) return flags; } +static +void adjust_mtu_peerid(struct context *c) +{ + frame_add_to_extra_frame(&c->c2.frame, +3); /* peer-id overhead */ + if (!c->options.ce.link_mtu_defined) + { + frame_add_to_link_mtu(&c->c2.frame, +3); + msg(D_PUSH, "OPTIONS IMPORT: adjusting link_mtu to %d", + EXPANDED_SIZE(&c->c2.frame)); + } + else + { + msg(M_WARN, "OPTIONS IMPORT: WARNING: peer-id set, but link-mtu" + " fixed by config - reducing tun-mtu to %d, expect" + " MTU problems", TUN_MTU_SIZE(&c->c2.frame) ); + } +} + +static bool +do_deferred_p2p_ncp(struct context *c) +{ + + if (!c->c2.tls_multi) + { + return true; + } + + if (c->c2.tls_multi->use_peer_id) + { + adjust_mtu_peerid(c); + } + + struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE]; + + const char *ncp_cipher = get_p2p_ncp_cipher(session, c->c2.tls_multi->peer_info, + &c->options.gc); + + if (ncp_cipher) + { + c->options.ciphername = ncp_cipher; + } + else if (!c->options.enable_ncp_fallback) + { + msg(D_TLS_ERRORS, "ERROR: failed to negotiate cipher with peer and " + "--data-ciphers-fallback not enabled. No useable " + "data channel cipher"); + return false; + } + + struct frame *frame_fragment = NULL; +#ifdef ENABLE_FRAGMENT + if (c->options.ce.fragment) + { + frame_fragment = &c->c2.frame_fragment; + } +#endif + + if (!tls_session_update_crypto_params(session, &c->options, &c->c2.frame, + frame_fragment)) + { + msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher"); + return false; + } + return true; +} + /* * Handle non-tun-related pulled options. */ @@ -2325,19 +2400,7 @@ do_deferred_options(struct context *c, const unsigned int found) msg(D_PUSH, "OPTIONS IMPORT: peer-id set"); c->c2.tls_multi->use_peer_id = true; c->c2.tls_multi->peer_id = c->options.peer_id; - frame_add_to_extra_frame(&c->c2.frame, +3); /* peer-id overhead */ - if (!c->options.ce.link_mtu_defined) - { - frame_add_to_link_mtu(&c->c2.frame, +3); - msg(D_PUSH, "OPTIONS IMPORT: adjusting link_mtu to %d", - EXPANDED_SIZE(&c->c2.frame)); - } - else - { - msg(M_WARN, "OPTIONS IMPORT: WARNING: peer-id set, but link-mtu" - " fixed by config - reducing tun-mtu to %d, expect" - " MTU problems", TUN_MTU_SIZE(&c->c2.frame) ); - } + adjust_mtu_peerid(c); } /* process (potentially pushed) crypto options */ @@ -2856,16 +2919,21 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.pull = options->pull; if (options->push_peer_info) /* all there is */ { - to.push_peer_info_detail = 2; + to.push_peer_info_detail = 3; } else if (options->pull) /* pull clients send some details */ { - to.push_peer_info_detail = 1; + to.push_peer_info_detail = 2; } - else /* default: no peer-info at all */ + else if (options->mode == MODE_SERVER) /* server: no peer info at all */ { to.push_peer_info_detail = 0; } + else /* default: minimal info to allow NCP in P2P mode */ + { + to.push_peer_info_detail = 1; + } + /* should we not xmit any packets until we get an initial * response from client? */ diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 2f4ccaa09..9cd88a36e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3078,10 +3078,6 @@ options_postprocess_cipher(struct options *o) { if (!o->pull && !(o->mode == MODE_SERVER)) { - /* we are in the classic P2P mode */ - msg( M_WARN, "Cipher negotiation is disabled since neither " - "P2MP client nor server mode is enabled"); - /* If the cipher is not set, use the old default of BF-CBC. We will * warn that this is deprecated on cipher initialisation, no need * to warn here as well */ @@ -3089,6 +3085,10 @@ options_postprocess_cipher(struct options *o) { o->ciphername = "BF-CBC"; } + else + { + o->enable_ncp_fallback = true; + } return; } diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index dd6e462d0..8a02a2006 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1874,33 +1874,18 @@ cleanup: return ret; } + bool -tls_session_update_crypto_params(struct tls_session *session, - struct options *options, struct frame *frame, +tls_session_update_crypto_params_do_work(struct tls_session *session, + struct options* options, struct frame *frame, struct frame *frame_fragment) { if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized) { /* keys already generated, nothing to do */ return true; - } - bool cipher_allowed_as_fallback = options->enable_ncp_fallback - && streq(options->ciphername, session->opt->config_ciphername); - - if (!session->opt->server && !cipher_allowed_as_fallback - && !tls_item_in_cipher_list(options->ciphername, options->ncp_ciphers)) - { - msg(D_TLS_ERRORS, "Error: pushed cipher not allowed - %s not in %s", - options->ciphername, options->ncp_ciphers); - /* undo cipher push, abort connection setup */ - options->ciphername = session->opt->config_ciphername; - return false; } - - /* Import crypto settings that might be set by pull/push */ - session->opt->crypto_flags |= options->data_channel_crypto_flags; - if (strcmp(options->ciphername, session->opt->config_ciphername)) { msg(D_HANDSHAKE, "Data Channel: using negotiated cipher '%s'", @@ -1952,6 +1937,32 @@ tls_session_update_crypto_params(struct tls_session *session, return tls_session_generate_data_channel_keys(session); } +bool +tls_session_update_crypto_params(struct tls_session *session, + struct options *options, struct frame *frame, + struct frame *frame_fragment) +{ + + bool cipher_allowed_as_fallback = options->enable_ncp_fallback + && streq(options->ciphername, session->opt->config_ciphername); + + if (!session->opt->server && !cipher_allowed_as_fallback + && !tls_item_in_cipher_list(options->ciphername, options->ncp_ciphers)) + { + msg(D_TLS_ERRORS, "Error: negotiated cipher not allowed - %s not in %s", + options->ciphername, options->ncp_ciphers); + /* undo cipher push, abort connection setup */ + options->ciphername = session->opt->config_ciphername; + return false; + } + + /* Import crypto settings that might be set by pull/push */ + session->opt->crypto_flags |= options->data_channel_crypto_flags; + + return tls_session_update_crypto_params_do_work(session, options, frame, frame_fragment); +} + + static bool random_bytes_to_buf(struct buffer *buf, uint8_t *out, @@ -2145,12 +2156,10 @@ push_peer_info(struct buffer *buf, struct tls_session *session) { struct gc_arena gc = gc_new(); bool ret = false; + struct buffer out = alloc_buf_gc(512 * 3, &gc); - if (session->opt->push_peer_info_detail > 0) + if (session->opt->push_peer_info_detail > 1) { - struct env_set *es = session->opt->es; - struct buffer out = alloc_buf_gc(512*3, &gc); - /* push version */ buf_printf(&out, "IV_VER=%s\n", PACKAGE_VERSION); @@ -2172,7 +2181,11 @@ push_peer_info(struct buffer *buf, struct tls_session *session) #elif defined(_WIN32) buf_printf(&out, "IV_PLAT=win\n"); #endif + } + /* These are the IV variable that are sent to peers in p2p mode */ + if (session->opt->push_peer_info_detail > 0) + { /* support for P_DATA_V2 */ int iv_proto = IV_PROTO_DATA_V2; @@ -2195,19 +2208,28 @@ push_peer_info(struct buffer *buf, struct tls_session *session) buf_printf(&out, "IV_NCP=2\n"); } - buf_printf(&out, "IV_CIPHERS=%s\n", session->opt->config_ncp_ciphers); + } + else + { + /* We are not using pull or p2mp server, instead do P2P NCP */ + iv_proto |= IV_PROTO_NCP_P2P; + } + + buf_printf(&out, "IV_CIPHERS=%s\n", session->opt->config_ncp_ciphers); #ifdef HAVE_EXPORT_KEYING_MATERIAL - iv_proto |= IV_PROTO_TLS_KEY_EXPORT; + iv_proto |= IV_PROTO_TLS_KEY_EXPORT; #endif - } buf_printf(&out, "IV_PROTO=%d\n", iv_proto); - /* push compression status */ + if (session->opt->push_peer_info_detail > 1) + { + /* push compression status */ #ifdef USE_COMP - comp_generate_peer_info_string(&session->opt->comp_options, &out); + comp_generate_peer_info_string(&session->opt->comp_options, &out); #endif + } if (session->opt->push_peer_info_detail >= 2) { @@ -2224,24 +2246,29 @@ push_peer_info(struct buffer *buf, struct tls_session *session) #endif } - /* push env vars that begin with UV_, IV_PLAT_VER and IV_GUI_VER */ - for (struct env_item *e = es->list; e != NULL; e = e->next) + if (session->opt->push_peer_info_detail > 1) { - if (e->string) + struct env_set *es = session->opt->es; + /* push env vars that begin with UV_, IV_PLAT_VER and IV_GUI_VER */ + for (struct env_item *e = es->list; e != NULL; e = e->next) { - if ((((strncmp(e->string, "UV_", 3)==0 - || strncmp(e->string, "IV_PLAT_VER=", sizeof("IV_PLAT_VER=")-1)==0) - && session->opt->push_peer_info_detail >= 2) - || (strncmp(e->string,"IV_GUI_VER=",sizeof("IV_GUI_VER=")-1)==0) - || (strncmp(e->string,"IV_SSO=",sizeof("IV_SSO=")-1)==0) - ) - && buf_safe(&out, strlen(e->string)+1)) + if (e->string) { - buf_printf(&out, "%s\n", e->string); + if ((((strncmp(e->string, "UV_", 3) == 0 + || strncmp(e->string, "IV_PLAT_VER=", sizeof("IV_PLAT_VER=") - 1) == 0) + && session->opt->push_peer_info_detail >= 2) + || (strncmp(e->string, "IV_GUI_VER=", sizeof("IV_GUI_VER=") - 1) == 0) + || (strncmp(e->string, "IV_SSO=", sizeof("IV_SSO=") - 1) == 0) + ) + && buf_safe(&out, strlen(e->string) + 1)) + { + buf_printf(&out, "%s\n", e->string); + } } } } + if (!write_string(buf, BSTR(&out), -1)) { goto error; @@ -2380,6 +2407,14 @@ key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct tls_sessi goto error; } + if (session->opt->server &&session->opt->mode != MODE_SERVER + && ks->key_id == 0) + { + /* tls-server option set and not P2MP server, so we + * are a P2P client running in tls-server mode */ + p2p_mode_ncp(multi, session); + } + return true; error: @@ -2579,7 +2614,14 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio setenv_del(session->opt->es, "exported_keying_material"); } - + + if (!session->opt->server && !session->opt->pull && ks->key_id == 0) + { + /* We are a p2p tls-client without pull, enable common + * protocol options */ + p2p_mode_ncp(multi, session); + } + gc_free(&gc); return true; diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index baf3560d2..14fabcc26 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -119,6 +119,11 @@ /** Supports signaling keywords with AUTH_PENDING, e.g. timeout=xy */ #define IV_PROTO_AUTH_PENDING_KW (1<<4) +/** Support doing NCP in P2P mode. This mode works by both peers looking at + * each other's IV_ variables and deterministically deciding both on the same + * result. */ +#define IV_PROTO_NCP_P2P (1<<5) + /* Default field in X509 to be username */ #define X509_USERNAME_FIELD_DEFAULT "CN" diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h index c3d12e5be..d228c70d8 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -390,6 +390,7 @@ void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file, bool crl_inline); #define EXPORT_KEY_DATA_LABEL "EXPORTER-OpenVPN-datakeys" +#define EXPORT_P2P_PEERID_LABEL "EXPORTER-OpenVPN-p2p-peerid" /** * Keying Material Exporters [RFC 5705] allows additional keying material to be * derived from existing TLS channel. This exported keying material can then be diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 43af51ee9..bcb199176 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -287,6 +287,16 @@ struct tls_options bool disable_occ; int mode; bool pull; + /** + * The detail of info we push in peer info + * + * 0 - nothing at all, P2MP server only + * 1 - only the most basic information to negotiate cipher and features + * for P2P NCP + * 2 - normal setting for clients + * 3 - full information including "sensitive data" like IV_HWADDR + * enabled by --push-peer-info + */ int push_peer_info_detail; int transition_window; int handshake_window; diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index 722256b42..8850ba237 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -324,3 +324,148 @@ check_pull_client_ncp(struct context *c, const int found) return false; } } + +const char* +get_p2p_ncp_cipher(struct tls_session *session, const char *peer_info, + struct gc_arena *gc) +{ + /* we use a local gc arena to keep the temporary strings needed by strsep */ + struct gc_arena gc_local = gc_new(); + const char *peer_ciphers = extract_var_peer_info(peer_info, "IV_CIPHERS=", &gc_local); + + if (!peer_ciphers) + { + gc_free(&gc_local); + return NULL; + } + + const char* server_ciphers; + const char* client_ciphers; + + if (session->opt->server) + { + server_ciphers = session->opt->config_ncp_ciphers; + client_ciphers = peer_ciphers; + } + else + { + client_ciphers = session->opt->config_ncp_ciphers; + server_ciphers = peer_ciphers; + } + + /* Find the first common cipher from TLS server and TLS client. We + * use the preference of the server here to make it deterministic */ + char *tmp_ciphers = string_alloc(server_ciphers, &gc_local); + + const char *token; + while ((token = strsep(&tmp_ciphers, ":"))) + { + if (tls_item_in_cipher_list(token, client_ciphers)) + { + break; + } + } + + const char *ret = NULL; + if (token != NULL) + { + const char *chosen_cipher = string_alloc(token, gc); + ret = chosen_cipher; + } + gc_free(&gc_local); + + return ret; +} + +static void +p2p_ncp_set_options(struct tls_multi *multi, struct tls_session *session) +{ + /* will return 0 if peer_info is null */ + const unsigned int iv_proto_peer = extract_iv_proto(multi->peer_info); + + /* The other peer does not support P2P NCP */ + if (!(iv_proto_peer & IV_PROTO_NCP_P2P)) + { + return; + } + + if (iv_proto_peer & IV_PROTO_DATA_V2) + { + multi->use_peer_id = true; + multi->peer_id = 0x76706e; // 'v' 'p' 'n' + + } + +#if defined(HAVE_EXPORT_KEYING_MATERIAL) + if (iv_proto_peer & IV_PROTO_TLS_KEY_EXPORT) + { + session->opt->crypto_flags |= CO_USE_TLS_KEY_MATERIAL_EXPORT; + + if (multi->use_peer_id) + { + /* Using a non hardcoded peer-id makes a tiny bit harder to + * fingerprint packets and also gives each connection a unique + * peer-id that can be useful for NAT tracking etc. */ + + uint8_t peerid[3]; + if (!key_state_export_keying_material(session, EXPORT_P2P_PEERID_LABEL, + strlen(EXPORT_P2P_PEERID_LABEL), + &peerid, 3)) + { + /* Non DCO setup might still work but also this should never + * happen or very likely the TLS encryption key exporter will + * also fail */ + msg(M_NONFATAL, "TLS key export for P2P peer id failed. " + "Continuing anyway, expect problems"); + } + else + { + multi->peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2]; + } + + } + } +#endif +} + +void +p2p_mode_ncp(struct tls_multi *multi, struct tls_session *session) +{ + /* Set the common options */ + p2p_ncp_set_options(multi, session); + + struct gc_arena gc = gc_new(); + + /* Query the common cipher here to log it as part of our message. + * We postpone switching the cipher to do_up */ + const char* common_cipher = get_p2p_ncp_cipher(session, multi->peer_info, &gc); + + if (!common_cipher) + { + struct buffer out = alloc_buf_gc(128, &gc); + struct key_state *ks = get_key_scan(multi, KS_PRIMARY); + + const cipher_ctx_t *ctx = ks->crypto_options.key_ctx_bi.encrypt.cipher; + const cipher_kt_t *cipher = cipher_ctx_get_cipher_kt(ctx); + const char *fallback_name = cipher_kt_name(cipher); + + if (!cipher) + { + /* at this point we do not really know if our fallback is + * not enabled or if we use 'none' cipher as fallback, so + * keep this ambiguity here and print fallback-cipher: none + */ + fallback_name = "none"; + } + + buf_printf(&out, "(not negotiated, fallback-cipher: %s)", fallback_name); + common_cipher = BSTR(&out); + } + + msg(D_TLS_DEBUG_LOW, "P2P mode NCP negotiation result: " + "TLS_export=%d, DATA_v2=%d, peer-id %d, cipher=%s", + (bool)(session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT), + multi->use_peer_id, multi->peer_id, common_cipher); + + gc_free(&gc); +} \ No newline at end of file diff --git a/src/openvpn/ssl_ncp.h b/src/openvpn/ssl_ncp.h index 39158a568..c02be6939 100644 --- a/src/openvpn/ssl_ncp.h +++ b/src/openvpn/ssl_ncp.h @@ -32,6 +32,7 @@ #include "buffer.h" #include "options.h" +#include "ssl_common.h" /** * Returns whether the client supports NCP either by @@ -115,4 +116,28 @@ bool tls_item_in_cipher_list(const char *item, const char *list); */ #define MAX_NCP_CIPHERS_LENGTH 127 +/** + * Determines if there is common cipher of both peer by looking at the + * IV_CIPHER peer info. In contrast of the server mode NCP that tries to + * accomandate all kind of corner cases in P2P mode NCP only takes IV_CIPHER + * into account and falls back to previous behaviour if this fails. + */ +void p2p_mode_ncp(struct tls_multi *multi, struct tls_session *session); + +/** + * Determines the best common cipher from both peers IV_CIPHER lists. The + * first cipher from the tls-server that is also in the tls-client IV_CIPHER + * list will be returned. If no common cipher can be found, both peer + * will continue to use whatever cipher is their default and NULL will be + * returned. + * + * @param session tls_session + * @param peer_info peer info of the peer + * @param gc gc arena that will be used to allocate the returned cipher + * @return common cipher if one exist. + */ +const char * +get_p2p_ncp_cipher(struct tls_session *session, const char *peer_info, + struct gc_arena *gc); + #endif /* ifndef OPENVPN_SSL_NCP_H */ diff --git a/tests/unit_tests/openvpn/test_ncp.c b/tests/unit_tests/openvpn/test_ncp.c index 4077be5e8..9dcd6f0a7 100644 --- a/tests/unit_tests/openvpn/test_ncp.c +++ b/tests/unit_tests/openvpn/test_ncp.c @@ -44,6 +44,17 @@ const char *bf_chacha = "BF-CBC:CHACHA20-POLY1305"; const char *aes_ciphers = "AES-256-GCM:AES-128-GCM"; + +/* Define this function here as dummy since including the ssl_*.c files + * leads to having to include even more unrelated code */ +bool +key_state_export_keying_material(struct tls_session *session, + const char* label, size_t label_size, + void *ekm, size_t ekm_size) +{ + ASSERT(0); +} + static void test_check_ncp_ciphers_list(void **state) {