From patchwork Mon Aug 10 04:36:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1367 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 iKHWCdVbMV85DQAAIUCqbw for ; Mon, 10 Aug 2020 10:38:13 -0400 Received: from proxy9.mail.ord1c.rsapps.net ([172.28.255.1]) by director11.mail.ord1d.rsapps.net with LMTP id iHB2CdVbMV+ESwAAvGGmqA (envelope-from ) for ; Mon, 10 Aug 2020 10:38:13 -0400 Received: from smtp5.gate.ord1c ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy9.mail.ord1c.rsapps.net with LMTP id WIggCdVbMV8MSgAAgxtkuw ; Mon, 10 Aug 2020 10:38:13 -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: smtp5.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: 1e1afe30-db17-11ea-8980-a4badb0b200d-1-1 Received: from [216.105.38.7] ([216.105.38.7:44874] helo=lists.sourceforge.net) by smtp5.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 2B/47-09162-4DB513F5; Mon, 10 Aug 2020 10:38:12 -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 1k58vi-0004lv-Db; Mon, 10 Aug 2020 14:37:38 +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 1k58vV-0004kC-Nd for openvpn-devel@lists.sourceforge.net; Mon, 10 Aug 2020 14:37:25 +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=/dGKzIoAAXtw7QY9IlnbLXgNIq6wWD7JWioT0XM0j2U=; b=l2FRbsyxU3/Q773+uSgA6dGGgN BLLqFYuVFsyo9oTXd6sh1nRGFgBKxIH4dNcJMNhZdEzyhWPxnZXzPuzRP2H/1G+Ut82N5YdZCYJdg TNJSvT0HEdcS32GFLTFiuLc7zdj7Gru8oxpBQx5L2zgU2Uk8lwXK+WlU46CBcBdoTxuA=; 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=/dGKzIoAAXtw7QY9IlnbLXgNIq6wWD7JWioT0XM0j2U=; b=h2lm4GgHhCqjHUjxHX47PHjwPX G0ccxotaOcO+cjQVWextnBxfkmQyKprxD5NouGjGbpP20DVXvr7Hj7+gzXuugnOdulz9+V/kp7eqO Irf+GZBTy/k1pSzh5jEfVFGGnpzNTt6NMLsaatyIF0ekomjqgebAiNiX/g4IC24BjGvc=; Received: from [192.26.174.232] (helo=mail.blinkt.de) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1k58vU-00FaL7-B5 for openvpn-devel@lists.sourceforge.net; Mon, 10 Aug 2020 14:37:25 +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 1k58vE-000ORu-43 for openvpn-devel@lists.sourceforge.net; Mon, 10 Aug 2020 16:37:08 +0200 Received: (nullmailer pid 5886 invoked by uid 10006); Mon, 10 Aug 2020 14:37:07 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:52 +0200 Message-Id: <20200810143707.5834-3-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200810143707.5834-1-arne@rfc2549.org> References: <20200810143707.5834-1-arne@rfc2549.org> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 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 1.0 RDNS_NONE Delivered to internal network by a host with no rDNS -0.5 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1k58vU-00FaL7-B5 Subject: [Openvpn-devel] [PATCH 02/17] Cleanup tls_pre_decrypt_lite and tls_pre_encrypt 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 Mostly C90 -> C99 cleanups and again immediately instead wrapping function body into if. (Review with ignore whitespace) Signed-off-by: Arne Schwabe Tested-By: Vladislav Grishenko Acked-by: Gert Doering --- src/openvpn/ssl.c | 224 ++++++++++++++++++++++------------------------ 1 file changed, 109 insertions(+), 115 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 6d146a63..2354a017 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3455,7 +3455,7 @@ tls_pre_decrypt(struct tls_multi *multi, } /* - * If we detected new session in the last if block, i has + * If we detected new session in the last if block, variable i has * changed to TM_ACTIVE, so check the condition again. */ if (i == TM_SIZE && is_hard_reset_method2(op)) @@ -3468,7 +3468,7 @@ tls_pre_decrypt(struct tls_multi *multi, /* * If --single-session, don't allow any hard-reset connection request - * unless it the the first packet of the session. + * unless it the first packet of the session. */ if (multi->opt.single_session) { @@ -3696,100 +3696,91 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas, const struct buffer *buf) { - struct gc_arena gc = gc_new(); - bool ret = false; - - if (buf->len > 0) + if (buf->len <= 0) { - int op; - int key_id; + return false; + } + struct gc_arena gc = gc_new(); - /* get opcode and key ID */ - { - uint8_t c = *BPTR(buf); - op = c >> P_OPCODE_SHIFT; - key_id = c & P_KEY_ID_MASK; - } + /* get opcode and key ID */ + uint8_t pkt_firstbyte = *BPTR(buf); + int op = pkt_firstbyte >> P_OPCODE_SHIFT; + int key_id = pkt_firstbyte & P_KEY_ID_MASK; - /* this packet is from an as-yet untrusted source, so - * scrutinize carefully */ + /* this packet is from an as-yet untrusted source, so + * scrutinize carefully */ - if (op != P_CONTROL_HARD_RESET_CLIENT_V2 - && op != P_CONTROL_HARD_RESET_CLIENT_V3) - { - /* - * This can occur due to bogus data or DoS packets. - */ - dmsg(D_TLS_STATE_ERRORS, - "TLS State Error: No TLS state for client %s, opcode=%d", - print_link_socket_actual(from, &gc), - op); - goto error; - } + if (op != P_CONTROL_HARD_RESET_CLIENT_V2 + && op != P_CONTROL_HARD_RESET_CLIENT_V3) + { + /* + * This can occur due to bogus data or DoS packets. + */ + dmsg(D_TLS_STATE_ERRORS, + "TLS State Error: No TLS state for client %s, opcode=%d", + print_link_socket_actual(from, &gc), + op); + goto error; + } - if (key_id != 0) - { - dmsg(D_TLS_STATE_ERRORS, - "TLS State Error: Unknown key ID (%d) received from %s -- 0 was expected", - key_id, - print_link_socket_actual(from, &gc)); - goto error; - } + if (key_id != 0) + { + dmsg(D_TLS_STATE_ERRORS, + "TLS State Error: Unknown key ID (%d) received from %s -- 0 was expected", + key_id, + print_link_socket_actual(from, &gc)); + goto error; + } - if (buf->len > EXPANDED_SIZE_DYNAMIC(&tas->frame)) - { - dmsg(D_TLS_STATE_ERRORS, - "TLS State Error: Large packet (size %d) received from %s -- a packet no larger than %d bytes was expected", - buf->len, - print_link_socket_actual(from, &gc), - EXPANDED_SIZE_DYNAMIC(&tas->frame)); - goto error; - } + if (buf->len > EXPANDED_SIZE_DYNAMIC(&tas->frame)) + { + dmsg(D_TLS_STATE_ERRORS, + "TLS State Error: Large packet (size %d) received from %s -- a packet no larger than %d bytes was expected", + buf->len, + print_link_socket_actual(from, &gc), + EXPANDED_SIZE_DYNAMIC(&tas->frame)); + goto error; + } - { - struct buffer newbuf = clone_buf(buf); - struct tls_wrap_ctx tls_wrap_tmp = tas->tls_wrap; - bool status; - - /* HMAC test, if --tls-auth was specified */ - status = read_control_auth(&newbuf, &tls_wrap_tmp, from, NULL); - free_buf(&newbuf); - free_buf(&tls_wrap_tmp.tls_crypt_v2_metadata); - if (tls_wrap_tmp.cleanup_key_ctx) - { - free_key_ctx_bi(&tls_wrap_tmp.opt.key_ctx_bi); - } - if (!status) - { - goto error; - } - /* - * At this point, if --tls-auth is being used, we know that - * the packet has passed the HMAC test, but we don't know if - * it is a replay yet. We will attempt to defeat replays - * by not advancing to the S_START state until we - * receive an ACK from our first reply to the client - * that includes an HMAC of our randomly generated 64 bit - * session ID. - * - * On the other hand if --tls-auth is not being used, we - * will proceed to begin the TLS authentication - * handshake with only cursory integrity checks having - * been performed, since we will be leaving the task - * of authentication solely up to TLS. - */ + struct buffer newbuf = clone_buf(buf); + struct tls_wrap_ctx tls_wrap_tmp = tas->tls_wrap; - ret = true; - } + /* HMAC test, if --tls-auth was specified */ + bool status = read_control_auth(&newbuf, &tls_wrap_tmp, from, NULL); + free_buf(&newbuf); + free_buf(&tls_wrap_tmp.tls_crypt_v2_metadata); + if (tls_wrap_tmp.cleanup_key_ctx) + { + free_key_ctx_bi(&tls_wrap_tmp.opt.key_ctx_bi); + } + if (!status) + { + goto error; } + + /* + * At this point, if --tls-auth is being used, we know that + * the packet has passed the HMAC test, but we don't know if + * it is a replay yet. We will attempt to defeat replays + * by not advancing to the S_START state until we + * receive an ACK from our first reply to the client + * that includes an HMAC of our randomly generated 64 bit + * session ID. + * + * On the other hand if --tls-auth is not being used, we + * will proceed to begin the TLS authentication + * handshake with only cursory integrity checks having + * been performed, since we will be leaving the task + * of authentication solely up to TLS. + */ gc_free(&gc); - return ret; + return true; error: tls_clear_error(); gc_free(&gc); - return ret; + return false; } /* Choose the key with which to encrypt a data packet */ @@ -3798,48 +3789,51 @@ tls_pre_encrypt(struct tls_multi *multi, struct buffer *buf, struct crypto_options **opt) { multi->save_ks = NULL; - if (buf->len > 0) + if (buf->len <= 0) + { + buf->len = 0; + *opt = NULL; + return; + } + + struct key_state *ks_select = NULL; + for (int i = 0; i < KEY_SCAN_SIZE; ++i) { - int i; - struct key_state *ks_select = NULL; - for (i = 0; i < KEY_SCAN_SIZE; ++i) + struct key_state *ks = multi->key_scan[i]; + if (ks->state >= S_ACTIVE + && (ks->authenticated == KS_AUTH_TRUE) + && ks->crypto_options.key_ctx_bi.initialized + ) { - struct key_state *ks = multi->key_scan[i]; - if (ks->state >= S_ACTIVE - && (ks->authenticated == KS_AUTH_TRUE) - && ks->crypto_options.key_ctx_bi.initialized - ) + if (!ks_select) { - if (!ks_select) - { - ks_select = ks; - } - if (now >= ks->auth_deferred_expire) - { - ks_select = ks; - break; - } + ks_select = ks; + } + if (now >= ks->auth_deferred_expire) + { + ks_select = ks; + break; } } + } - if (ks_select) - { - *opt = &ks_select->crypto_options; - multi->save_ks = ks_select; - dmsg(D_TLS_KEYSELECT, "TLS: tls_pre_encrypt: key_id=%d", ks_select->key_id); - return; - } - else - { - struct gc_arena gc = gc_new(); - dmsg(D_TLS_KEYSELECT, "TLS Warning: no data channel send key available: %s", - print_key_id(multi, &gc)); - gc_free(&gc); - } + if (ks_select) + { + *opt = &ks_select->crypto_options; + multi->save_ks = ks_select; + dmsg(D_TLS_KEYSELECT, "TLS: tls_pre_encrypt: key_id=%d", ks_select->key_id); + return; } + else + { + struct gc_arena gc = gc_new(); + dmsg(D_TLS_KEYSELECT, "TLS Warning: no data channel send key available: %s", + print_key_id(multi, &gc)); + gc_free(&gc); - buf->len = 0; - *opt = NULL; + *opt = NULL; + buf->len = 0; + } } void