From patchwork Fri Oct 23 01:02:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1525 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director7.mail.ord1d.rsapps.net ([172.27.255.9]) by backend41.mail.ord1d.rsapps.net with LMTP id QFg6FMDGkl9gTgAAqwncew (envelope-from ) for ; Fri, 23 Oct 2020 08:04:16 -0400 Received: from proxy12.mail.iad3a.rsapps.net ([172.27.255.9]) by director7.mail.ord1d.rsapps.net with LMTP id KDMsFMDGkl9GNQAAovjBpQ (envelope-from ) for ; Fri, 23 Oct 2020 08:04:16 -0400 Received: from smtp24.gate.iad3a ([172.27.255.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.iad3a.rsapps.net with LMTPS id ACD6DcDGkl8XOQAAh9K5Vw (envelope-from ) for ; Fri, 23 Oct 2020 08:04:16 -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: smtp24.gate.iad3a.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: de50958a-1527-11eb-b29f-5254009f6f51-1-1 Received: from [216.105.38.7] ([216.105.38.7:40286] helo=lists.sourceforge.net) by smtp24.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id C0/D0-08455-EB6C29F5; Fri, 23 Oct 2020 08:04:15 -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.90_1) (envelope-from ) id 1kVvmx-0003jq-VS; Fri, 23 Oct 2020 12:03:19 +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.90_1) (envelope-from ) id 1kVvmr-0003j9-NE for openvpn-devel@lists.sourceforge.net; Fri, 23 Oct 2020 12:03:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:To: From:Sender:Reply-To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: 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=iG68BAXLGQK9fY3RgDM7KxHnRtQc5GMk3QV/ZpFiSsM=; b=TMGdSqP4R2JqI1c4UQkxYtB59b 4EYDuWG8YszndZf8Uq7/nEvsrqsoQfiCeowdB+ulvD1X0b/RsbDB1m0luy2hKJoFz6+IYgaU+etKe vFpsMhNjZBkAEBH3QiQSQXq2fFOgCoPULCHS9o/oYnjamVlfh0l3Xv8uATgLf5vLZrdg=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc :MIME-Version:Content-Type:Content-Transfer-Encoding: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=iG68BAXLGQK9fY3RgDM7KxHnRtQc5GMk3QV/ZpFiSsM=; b=hwcyh73jiksr8chFer3Zm73XH8 4+AXS+jy0mPcObGrB++++StwWDop9VS/UXPM4YiUy3Kk26JB5YgWyPc3ijY3Balf9oSX+EUuTprxd 3ehGSBcQrgX+8eqIv5eSSNXaq4JrMJdCkGgFtniThMncqfwufSp08RyDIF8eqMML+72Y=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1kVvmm-003qie-J5 for openvpn-devel@lists.sourceforge.net; Fri, 23 Oct 2020 12:03:13 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94 (FreeBSD)) (envelope-from ) id 1kVvmd-000JFu-VC for openvpn-devel@lists.sourceforge.net; Fri, 23 Oct 2020 14:02:59 +0200 Received: (nullmailer pid 29838 invoked by uid 10006); Fri, 23 Oct 2020 12:02:59 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Fri, 23 Oct 2020 14:02:56 +0200 Message-Id: <20201023120259.29783-4-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20201023120259.29783-1-arne@rfc2549.org> References: <20201023113244.26295-1-arne@rfc2549.org> <20201023120259.29783-1-arne@rfc2549.org> 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: 1kVvmm-003qie-J5 Subject: [Openvpn-devel] [PATCH 5/8] Clean up tls_authentication_status and document it 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: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox The gain of the used optimisation approach of using a array with a calculated index in favour of simple ifs is questionable with modern compilers and the readability of the function suffers. Also change the return type from simple int to an enum and add comments and doxygen documentation. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/push.c | 3 +- src/openvpn/ssl.c | 2 +- src/openvpn/ssl_verify.c | 108 +++++++++++++++------------------------ src/openvpn/ssl_verify.h | 35 +++++++++---- 4 files changed, 70 insertions(+), 78 deletions(-) diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 19004077..26a6201f 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -736,7 +736,8 @@ process_incoming_push_request(struct context *c) { int ret = PUSH_MSG_ERROR; - if (tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED || c->c2.context_auth == CAS_FAILED) + if ((c->c2.tls_multi && tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED) + || c->c2.context_auth == CAS_FAILED) { const char *client_reason = tls_client_reason(c->c2.tls_multi); send_auth_failed(c, client_reason); diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 79ad322a..e59dba31 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3196,7 +3196,7 @@ tls_multi_process(struct tls_multi *multi, update_time(); - int tas = tls_authentication_status(multi, TLS_MULTI_AUTH_STATUS_INTERVAL); + enum tls_auth_status tas = tls_authentication_status(multi, TLS_MULTI_AUTH_STATUS_INTERVAL); /* * If lame duck session expires, kill it. diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 862a6f56..4172e2fd 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -928,86 +928,56 @@ key_state_test_auth_control_file(struct key_state *ks) return ACF_DISABLED; } -/* - * Return current session authentication state. Return - * value is TLS_AUTHENTICATION_x. - */ - -int +enum tls_auth_status tls_authentication_status(struct tls_multi *multi, const int latency) { bool deferred = false; + + /* at least one valid key has successfully completed authentication */ bool success = false; - bool active = false; - - static const unsigned char acf_merge[] = - { - ACF_UNDEFINED, /* s1=ACF_UNDEFINED s2=ACF_UNDEFINED */ - ACF_UNDEFINED, /* s1=ACF_UNDEFINED s2=ACF_SUCCEEDED */ - ACF_UNDEFINED, /* s1=ACF_UNDEFINED s2=ACF_DISABLED */ - ACF_FAILED, /* s1=ACF_UNDEFINED s2=ACF_FAILED */ - ACF_UNDEFINED, /* s1=ACF_SUCCEEDED s2=ACF_UNDEFINED */ - ACF_SUCCEEDED, /* s1=ACF_SUCCEEDED s2=ACF_SUCCEEDED */ - ACF_SUCCEEDED, /* s1=ACF_SUCCEEDED s2=ACF_DISABLED */ - ACF_FAILED, /* s1=ACF_SUCCEEDED s2=ACF_FAILED */ - ACF_UNDEFINED, /* s1=ACF_DISABLED s2=ACF_UNDEFINED */ - ACF_SUCCEEDED, /* s1=ACF_DISABLED s2=ACF_SUCCEEDED */ - ACF_DISABLED, /* s1=ACF_DISABLED s2=ACF_DISABLED */ - ACF_FAILED, /* s1=ACF_DISABLED s2=ACF_FAILED */ - ACF_FAILED, /* s1=ACF_FAILED s2=ACF_UNDEFINED */ - ACF_FAILED, /* s1=ACF_FAILED s2=ACF_SUCCEEDED */ - ACF_FAILED, /* s1=ACF_FAILED s2=ACF_DISABLED */ - ACF_FAILED /* s1=ACF_FAILED s2=ACF_FAILED */ - }; - if (multi) - { - int i; + /* at least one key is enabled for decryption */ + int active = 0; - if (latency && multi->tas_last && multi->tas_last + latency >= now) - { - return TLS_AUTHENTICATION_UNDEFINED; - } - multi->tas_last = now; + if (latency && multi->tas_last + latency >= now) + { + return TLS_AUTHENTICATION_UNDEFINED; + } + multi->tas_last = now; - for (i = 0; i < KEY_SCAN_SIZE; ++i) + for (int i = 0; i < KEY_SCAN_SIZE; ++i) + { + struct key_state *ks = get_key_scan(multi, i); + if (DECRYPT_KEY_ENABLED(multi, ks)) { - struct key_state *ks = get_key_scan(multi, i); - if (DECRYPT_KEY_ENABLED(multi, ks)) + active++; + if (ks->authenticated > KS_AUTH_FALSE) { - active = true; - if (ks->authenticated > KS_AUTH_FALSE) - { - unsigned int s1 = ACF_DISABLED; - unsigned int s2 = ACF_DISABLED; - s1 = key_state_test_auth_control_file(ks); + unsigned int s1 = ACF_DISABLED; + unsigned int s2 = ACF_DISABLED; + s1 = key_state_test_auth_control_file(ks); #ifdef ENABLE_MANAGEMENT - s2 = man_def_auth_test(ks); + s2 = man_def_auth_test(ks); #endif - ASSERT(s1 < 4 && s2 < 4); - switch (acf_merge[(s1<<2) + s2]) + ASSERT(s1 < 4 && s2 < 4); + + if (s1 == ACF_FAILED || s2 == ACF_FAILED) + { + ks->authenticated = KS_AUTH_FALSE; + } + else if (s1 == ACF_UNDEFINED || s2 == ACF_UNDEFINED) + { + if (now < ks->auth_deferred_expire) { - case ACF_SUCCEEDED: - case ACF_DISABLED: - success = true; - ks->authenticated = KS_AUTH_TRUE; - break; - - case ACF_UNDEFINED: - if (now < ks->auth_deferred_expire) - { - deferred = true; - } - break; - - case ACF_FAILED: - ks->authenticated = KS_AUTH_FALSE; - break; - - default: - ASSERT(0); + deferred = true; } } + else + { + /* s1 and s2 are either ACF_DISABLED or ACF_SUCCEDED */ + success = true; + ks->authenticated = KS_AUTH_TRUE; + } } } } @@ -1020,12 +990,16 @@ tls_authentication_status(struct tls_multi *multi, const int latency) { return TLS_AUTHENTICATION_SUCCEEDED; } - else if (!active || deferred) + else if (active == 0 || deferred) { + /* We have a deferred authentication and no currently active key + * (first auth, no renegotiation) */ return TLS_AUTHENTICATION_DEFERRED; } else { + /* at least one key is active but none is fully authenticated (!success) + * and all active are either failed authed or expired deferred auth */ return TLS_AUTHENTICATION_FAILED; } } diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h index d913f102..b3fe25d2 100644 --- a/src/openvpn/ssl_verify.h +++ b/src/openvpn/ssl_verify.h @@ -65,18 +65,35 @@ struct cert_hash_set { #define VERIFY_X509_SUBJECT_RDN 2 #define VERIFY_X509_SUBJECT_RDN_PREFIX 3 -#define TLS_AUTHENTICATION_SUCCEEDED 0 -#define TLS_AUTHENTICATION_FAILED 1 -#define TLS_AUTHENTICATION_DEFERRED 2 -#define TLS_AUTHENTICATION_UNDEFINED 3 +enum tls_auth_status +{ + TLS_AUTHENTICATION_SUCCEEDED=0, + TLS_AUTHENTICATION_FAILED=1, + TLS_AUTHENTICATION_DEFERRED=2, + TLS_AUTHENTICATION_UNDEFINED=3 +}; -/* - * Return current session authentication state. Return - * value is TLS_AUTHENTICATION_x. +/** + * Return current session authentication state of the tls_multi structure + * This will return TLS_AUTHENTICATION_SUCCEEDED only if the session is + * fully authenicated, i.e. VPN traffic is allowed over it. + * + * Checks the status of all active keys and checks if the deferred + * authentication has succeeded. + * + * As a side effect this function will also transition ks->authenticated + * from KS_AUTH_DEFERRED to KS_AUTH_FALSE/KS_AUTH_TRUE if the deferred + * authentication has succeeded after last call. + * + * @param latency if not null, return TLS_AUTHENTICATION_UNDEFINED if + * the last call for this multi struct has been less + * than latency seconds ago + * @param multi the tls_multi struct to operate on * - * TODO: document this function + * @return Current authentication status of the tls_multi */ -int tls_authentication_status(struct tls_multi *multi, const int latency); +enum tls_auth_status +tls_authentication_status(struct tls_multi *multi, const int latency); /** Check whether the \a ks \c key_state is ready to receive data channel * packets.