From patchwork Tue Jul 21 23:30:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1323 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director9.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id 2PVVLIwHGF9nEgAAIUCqbw for ; Wed, 22 Jul 2020 05:31:56 -0400 Received: from proxy1.mail.ord1d.rsapps.net ([172.30.191.6]) by director9.mail.ord1d.rsapps.net with LMTP id WGU5LIwHGF87TwAAalYnBA ; Wed, 22 Jul 2020 05:31:56 -0400 Received: from smtp11.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy1.mail.ord1d.rsapps.net with LMTP id wNBHLIwHGF9waAAAasrz9Q ; Wed, 22 Jul 2020 05:31:56 -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: smtp11.gate.ord1d.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: 2ee75b28-cbfe-11ea-9744-5254005f837b-1-1 Received: from [216.105.38.7] ([216.105.38.7:58222] helo=lists.sourceforge.net) by smtp11.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 4B/A8-14468-C87081F5; Wed, 22 Jul 2020 05:31:56 -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 1jyB5a-0002zr-D2; Wed, 22 Jul 2020 09:31:02 +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 1jyB5X-0002zO-T0 for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 09:30:59 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=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:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=1WIZ+w1MQ17T18Zu28609LPa9EYTJphZr6hQouiRPtA=; b=dYEtOZSJY0bRl9kHMypk7/5zLt v/LywTywRUjH//QHbiatOZiD7ly13cYHiN7XqgjIA47yfmDhxuuIo9isuh+WZqopPc4n11uuQzM+Z aDLmlgiRrPD8C9so+1aGcNzK5DREPv0v5maP4sjbM6igRlENQbnBtaUp6LorAad8DcmE=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=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: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=1WIZ+w1MQ17T18Zu28609LPa9EYTJphZr6hQouiRPtA=; b=IvU8FiJvqkIZqLjRBnWdKPUvag nuuRS7t1xtAj1zguU4Acaz3B54Tr5aYz7r12y6PZ6t+/tcUQav0F/9NsreMyM7ew6sOELMTLwnwoK Ky79PeiBY6wRtuYxx9evowaVhoXFhMa1+rbIDZvU6MG+JeeVVJLq010o/QPm2tuBrvro=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-4.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1jyB5V-00GKGW-2e for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 09:30:59 +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 1jyB5O-0005Og-Jw for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 11:30:50 +0200 Received: (nullmailer pid 20519 invoked by uid 10006); Wed, 22 Jul 2020 09:30:50 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Wed, 22 Jul 2020 11:30:48 +0200 Message-Id: <20200722093050.20474-1-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 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.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 X-Headers-End: 1jyB5V-00GKGW-2e Subject: [Openvpn-devel] [PATCH 1/3] Refactor/Reformat tls_pre_decrypt 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 - Extract data packet handling to its own function - Replace two instances of if (x) { code } with if (!x) return; code - Remove extra curly braces that were used for pre C99 code style to be able to declare variables in the middle of a block This patch is easier to review with "ignore white space" as the diff is then a lot smaller in that case and the changes more obvious. Signed-off-by: Arne Schwabe --- src/openvpn/ssl.c | 791 ++++++++++++++++++++++++---------------------- 1 file changed, 410 insertions(+), 381 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 06dc9f8f..6d146a63 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3166,6 +3166,102 @@ nohard: * to implement a multiplexed TLS channel over the TCP/UDP port. */ +static inline void +handle_data_channel_paket(struct tls_multi *multi, + const struct link_socket_actual *from, + struct buffer *buf, + struct crypto_options **opt, + bool floated, + const uint8_t **ad_start) +{ + struct gc_arena gc = gc_new(); + + uint8_t c = *BPTR(buf); + int op = c >> P_OPCODE_SHIFT; + int key_id = c & P_KEY_ID_MASK; + + /* data channel packet */ + for (int i = 0; i < KEY_SCAN_SIZE; ++i) + { + struct key_state *ks = multi->key_scan[i]; + + /* + * This is the basic test of TLS state compatibility between a local OpenVPN + * instance and its remote peer. + * + * If the test fails, it tells us that we are getting a packet from a source + * which claims reference to a prior negotiated TLS session, but the local + * OpenVPN instance has no memory of such a negotiation. + * + * It almost always occurs on UDP sessions when the passive side of the + * connection is restarted without the active side restarting as well (the + * passive side is the server which only listens for the connections, the + * active side is the client which initiates connections). + */ + if (DECRYPT_KEY_ENABLED(multi, ks) + && key_id == ks->key_id + && (ks->authenticated == KS_AUTH_TRUE) + && (floated || link_socket_actual_match(from, &ks->remote_addr))) + { + if (!ks->crypto_options.key_ctx_bi.initialized) + { + msg(D_MULTI_DROPPED, + "Key %s [%d] not initialized (yet), dropping packet.", + print_link_socket_actual(from, &gc), key_id); + goto error_lite; + } + + /* return appropriate data channel decrypt key in opt */ + *opt = &ks->crypto_options; + if (op == P_DATA_V2) + { + *ad_start = BPTR(buf); + } + ASSERT(buf_advance(buf, 1)); + if (op == P_DATA_V1) + { + *ad_start = BPTR(buf); + } + else if (op == P_DATA_V2) + { + if (buf->len < 4) + { + msg(D_TLS_ERRORS, "Protocol error: received P_DATA_V2 from %s but length is < 4", + print_link_socket_actual(from, &gc)); + goto error; + } + ASSERT(buf_advance(buf, 3)); + } + + ++ks->n_packets; + ks->n_bytes += buf->len; + dmsg(D_TLS_KEYSELECT, + "TLS: tls_pre_decrypt, key_id=%d, IP=%s", + key_id, print_link_socket_actual(from, &gc)); + gc_free(&gc); + return; + } + } + + msg(D_TLS_ERRORS, + "TLS Error: local/remote TLS keys are out of sync: %s [%d]", + print_link_socket_actual(from, &gc), key_id); + goto error_lite; + + +done: + buf->len = 0; + *opt = NULL; + gc_free(&gc); + return; + +error: + ++multi->n_soft_errors; +error_lite: + tls_clear_error(); + goto done; +} + /* * * When we are in TLS mode, this is the first routine which sees @@ -3199,440 +3295,374 @@ tls_pre_decrypt(struct tls_multi *multi, bool floated, const uint8_t **ad_start) { + + if (buf->len <= 0) + { + buf->len = 0; + *opt = NULL; + return false; + } + struct gc_arena gc = gc_new(); bool ret = false; - if (buf->len > 0) + /* get opcode */ + uint8_t pkt_firstbyte = *BPTR(buf); + int op = pkt_firstbyte >> P_OPCODE_SHIFT; + + if ((op == P_DATA_V1) || (op == P_DATA_V2)) { - int i; - int op; - int key_id; + handle_data_channel_paket(multi, from, buf, opt, floated, ad_start); + return false; + } - /* get opcode and key ID */ + /* get key_id */ + int key_id = pkt_firstbyte & P_KEY_ID_MASK; + + /* control channel packet */ + bool do_burst = false; + bool new_link = false; + struct session_id sid; /* remote session ID */ + + /* verify legal opcode */ + if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE) + { + if (op == P_CONTROL_HARD_RESET_CLIENT_V1 + || op == P_CONTROL_HARD_RESET_SERVER_V1) { - uint8_t c = *BPTR(buf); - op = c >> P_OPCODE_SHIFT; - key_id = c & P_KEY_ID_MASK; + msg(D_TLS_ERRORS, "Peer tried unsupported key-method 1"); } + msg(D_TLS_ERRORS, + "TLS Error: unknown opcode received from %s op=%d", + print_link_socket_actual(from, &gc), op); + goto error; + } - if ((op == P_DATA_V1) || (op == P_DATA_V2)) + /* hard reset ? */ + if (is_hard_reset_method2(op)) + { + /* verify client -> server or server -> client connection */ + if (((op == P_CONTROL_HARD_RESET_CLIENT_V2 + || op == P_CONTROL_HARD_RESET_CLIENT_V3) && !multi->opt.server) + || ((op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server)) { - /* data channel packet */ - for (i = 0; i < KEY_SCAN_SIZE; ++i) - { - struct key_state *ks = multi->key_scan[i]; - - /* - * This is the basic test of TLS state compatibility between a local OpenVPN - * instance and its remote peer. - * - * If the test fails, it tells us that we are getting a packet from a source - * which claims reference to a prior negotiated TLS session, but the local - * OpenVPN instance has no memory of such a negotiation. - * - * It almost always occurs on UDP sessions when the passive side of the - * connection is restarted without the active side restarting as well (the - * passive side is the server which only listens for the connections, the - * active side is the client which initiates connections). - */ - if (DECRYPT_KEY_ENABLED(multi, ks) - && key_id == ks->key_id - && (ks->authenticated == KS_AUTH_TRUE) - && (floated || link_socket_actual_match(from, &ks->remote_addr))) - { - if (!ks->crypto_options.key_ctx_bi.initialized) - { - msg(D_MULTI_DROPPED, - "Key %s [%d] not initialized (yet), dropping packet.", - print_link_socket_actual(from, &gc), key_id); - goto error_lite; - } - - /* return appropriate data channel decrypt key in opt */ - *opt = &ks->crypto_options; - if (op == P_DATA_V2) - { - *ad_start = BPTR(buf); - } - ASSERT(buf_advance(buf, 1)); - if (op == P_DATA_V1) - { - *ad_start = BPTR(buf); - } - else if (op == P_DATA_V2) - { - if (buf->len < 4) - { - msg(D_TLS_ERRORS, "Protocol error: received P_DATA_V2 from %s but length is < 4", - print_link_socket_actual(from, &gc)); - goto error; - } - ASSERT(buf_advance(buf, 3)); - } + msg(D_TLS_ERRORS, + "TLS Error: client->client or server->server connection attempted from %s", + print_link_socket_actual(from, &gc)); + goto error; + } + } - ++ks->n_packets; - ks->n_bytes += buf->len; - dmsg(D_TLS_KEYSELECT, - "TLS: tls_pre_decrypt, key_id=%d, IP=%s", - key_id, print_link_socket_actual(from, &gc)); - gc_free(&gc); - return ret; - } - } + /* + * Authenticate Packet + */ + dmsg(D_TLS_DEBUG, "TLS: control channel, op=%s, IP=%s", + packet_opcode_name(op), print_link_socket_actual(from, &gc)); + /* get remote session-id */ + { + struct buffer tmp = *buf; + buf_advance(&tmp, 1); + if (!session_id_read(&sid, &tmp) || !session_id_defined(&sid)) + { msg(D_TLS_ERRORS, - "TLS Error: local/remote TLS keys are out of sync: %s [%d]", - print_link_socket_actual(from, &gc), key_id); - goto error_lite; + "TLS Error: session-id not found in packet from %s", + print_link_socket_actual(from, &gc)); + goto error; } - else /* control channel packet */ - { - bool do_burst = false; - bool new_link = false; - struct session_id sid; /* remote session ID */ + } + + int i; + /* use session ID to match up packet with appropriate tls_session object */ + for (i = 0; i < TM_SIZE; ++i) + { + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; + + dmsg(D_TLS_DEBUG, + "TLS: initial packet test, i=%d state=%s, mysid=%s, rec-sid=%s, rec-ip=%s, stored-sid=%s, stored-ip=%s", + i, + state_name(ks->state), + session_id_print(&session->session_id, &gc), + session_id_print(&sid, &gc), + print_link_socket_actual(from, &gc), + session_id_print(&ks->session_id_remote, &gc), + print_link_socket_actual(&ks->remote_addr, &gc)); - /* verify legal opcode */ - if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE) + if (session_id_equal(&ks->session_id_remote, &sid)) + /* found a match */ + { + if (i == TM_LAME_DUCK) { - if (op == P_CONTROL_HARD_RESET_CLIENT_V1 - || op == P_CONTROL_HARD_RESET_SERVER_V1) - { - msg(D_TLS_ERRORS, "Peer tried unsupported key-method 1"); - } msg(D_TLS_ERRORS, - "TLS Error: unknown opcode received from %s op=%d", - print_link_socket_actual(from, &gc), op); + "TLS ERROR: received control packet with stale session-id=%s", + session_id_print(&sid, &gc)); goto error; } + dmsg(D_TLS_DEBUG, + "TLS: found match, session[%d], sid=%s", + i, session_id_print(&sid, &gc)); + break; + } + } - /* hard reset ? */ - if (is_hard_reset_method2(op)) - { - /* verify client -> server or server -> client connection */ - if (((op == P_CONTROL_HARD_RESET_CLIENT_V2 - || op == P_CONTROL_HARD_RESET_CLIENT_V3) && !multi->opt.server) - || ((op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server)) - { - msg(D_TLS_ERRORS, - "TLS Error: client->client or server->server connection attempted from %s", - print_link_socket_actual(from, &gc)); - goto error; - } - } - - /* - * Authenticate Packet - */ - dmsg(D_TLS_DEBUG, "TLS: control channel, op=%s, IP=%s", - packet_opcode_name(op), print_link_socket_actual(from, &gc)); + /* + * Hard reset and session id does not match any session in + * multi->session: Possible initial packet + */ + if (i == TM_SIZE && is_hard_reset_method2(op)) + { + struct tls_session *session = &multi->session[TM_ACTIVE]; + struct key_state *ks = &session->key[KS_PRIMARY]; - /* get remote session-id */ + /* + * If we have no session currently in progress, the initial packet will + * open a new session in TM_ACTIVE rather than TM_UNTRUSTED. + */ + if (!session_id_defined(&ks->session_id_remote)) + { + if (multi->opt.single_session && multi->n_sessions) { - struct buffer tmp = *buf; - buf_advance(&tmp, 1); - if (!session_id_read(&sid, &tmp) || !session_id_defined(&sid)) - { - msg(D_TLS_ERRORS, - "TLS Error: session-id not found in packet from %s", - print_link_socket_actual(from, &gc)); - goto error; - } + msg(D_TLS_ERRORS, + "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [1]", + print_link_socket_actual(from, &gc)); + goto error; } - /* use session ID to match up packet with appropriate tls_session object */ - for (i = 0; i < TM_SIZE; ++i) +#ifdef ENABLE_MANAGEMENT + if (management) { - struct tls_session *session = &multi->session[i]; - struct key_state *ks = &session->key[KS_PRIMARY]; - - dmsg(D_TLS_DEBUG, - "TLS: initial packet test, i=%d state=%s, mysid=%s, rec-sid=%s, rec-ip=%s, stored-sid=%s, stored-ip=%s", - i, - state_name(ks->state), - session_id_print(&session->session_id, &gc), - session_id_print(&sid, &gc), - print_link_socket_actual(from, &gc), - session_id_print(&ks->session_id_remote, &gc), - print_link_socket_actual(&ks->remote_addr, &gc)); - - if (session_id_equal(&ks->session_id_remote, &sid)) - /* found a match */ - { - if (i == TM_LAME_DUCK) - { - msg(D_TLS_ERRORS, - "TLS ERROR: received control packet with stale session-id=%s", - session_id_print(&sid, &gc)); - goto error; - } - dmsg(D_TLS_DEBUG, - "TLS: found match, session[%d], sid=%s", - i, session_id_print(&sid, &gc)); - break; - } + management_set_state(management, + OPENVPN_STATE_AUTH, + NULL, + NULL, + NULL, + NULL, + NULL); } +#endif - /* - * Hard reset and session id does not match any session in - * multi->session: Possible initial packet - */ - if (i == TM_SIZE && is_hard_reset_method2(op)) - { - struct tls_session *session = &multi->session[TM_ACTIVE]; - struct key_state *ks = &session->key[KS_PRIMARY]; - - /* - * If we have no session currently in progress, the initial packet will - * open a new session in TM_ACTIVE rather than TM_UNTRUSTED. - */ - if (!session_id_defined(&ks->session_id_remote)) - { - if (multi->opt.single_session && multi->n_sessions) - { - msg(D_TLS_ERRORS, - "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [1]", - print_link_socket_actual(from, &gc)); - goto error; - } + msg(D_TLS_DEBUG_LOW, + "TLS: Initial packet from %s, sid=%s", + print_link_socket_actual(from, &gc), + session_id_print(&sid, &gc)); -#ifdef ENABLE_MANAGEMENT - if (management) - { - management_set_state(management, - OPENVPN_STATE_AUTH, - NULL, - NULL, - NULL, - NULL, - NULL); - } -#endif + do_burst = true; + new_link = true; + i = TM_ACTIVE; + session->untrusted_addr = *from; + } + } - msg(D_TLS_DEBUG_LOW, - "TLS: Initial packet from %s, sid=%s", - print_link_socket_actual(from, &gc), - session_id_print(&sid, &gc)); + /* + * If we detected new session in the last if block, i has + * changed to TM_ACTIVE, so check the condition again. + */ + if (i == TM_SIZE && is_hard_reset_method2(op)) + { + /* + * No match with existing sessions, + * probably a new session. + */ + struct tls_session *session = &multi->session[TM_UNTRUSTED]; - do_burst = true; - new_link = true; - i = TM_ACTIVE; - session->untrusted_addr = *from; - } - } + /* + * If --single-session, don't allow any hard-reset connection request + * unless it the the first packet of the session. + */ + if (multi->opt.single_session) + { + msg(D_TLS_ERRORS, + "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [2]", + print_link_socket_actual(from, &gc)); + goto error; + } - /* - * If we detected new session in the last if block, i has - * changed to TM_ACTIVE, so check the condition again. - */ - if (i == TM_SIZE && is_hard_reset_method2(op)) - { - /* - * No match with existing sessions, - * probably a new session. - */ - struct tls_session *session = &multi->session[TM_UNTRUSTED]; - - /* - * If --single-session, don't allow any hard-reset connection request - * unless it the the first packet of the session. - */ - if (multi->opt.single_session) - { - msg(D_TLS_ERRORS, - "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [2]", - print_link_socket_actual(from, &gc)); - goto error; - } + if (!read_control_auth(buf, &session->tls_wrap, from, + session->opt)) + { + goto error; + } - if (!read_control_auth(buf, &session->tls_wrap, from, - session->opt)) - { - goto error; - } + /* + * New session-initiating control packet is authenticated at this point, + * assuming that the --tls-auth command line option was used. + * + * Without --tls-auth, we leave authentication entirely up to TLS. + */ + msg(D_TLS_DEBUG_LOW, + "TLS: new session incoming connection from %s", + print_link_socket_actual(from, &gc)); - /* - * New session-initiating control packet is authenticated at this point, - * assuming that the --tls-auth command line option was used. - * - * Without --tls-auth, we leave authentication entirely up to TLS. - */ - msg(D_TLS_DEBUG_LOW, - "TLS: new session incoming connection from %s", - print_link_socket_actual(from, &gc)); + new_link = true; + i = TM_UNTRUSTED; + session->untrusted_addr = *from; + } + else + { + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; - new_link = true; - i = TM_UNTRUSTED; - session->untrusted_addr = *from; - } - else + /* + * Packet must belong to an existing session. + */ + if (i != TM_ACTIVE && i != TM_UNTRUSTED) + { + msg(D_TLS_ERRORS, + "TLS Error: Unroutable control packet received from %s (si=%d op=%s)", + print_link_socket_actual(from, &gc), + i, + packet_opcode_name(op)); + goto error; + } + + /* + * Verify remote IP address + */ + if (!new_link && !link_socket_actual_match(&ks->remote_addr, from)) + { + msg(D_TLS_ERRORS, "TLS Error: Received control packet from unexpected IP addr: %s", + print_link_socket_actual(from, &gc)); + goto error; + } + + /* + * Remote is requesting a key renegotiation + */ + if (op == P_CONTROL_SOFT_RESET_V1 + && DECRYPT_KEY_ENABLED(multi, ks)) + { + if (!read_control_auth(buf, &session->tls_wrap, from, + session->opt)) { - struct tls_session *session = &multi->session[i]; - struct key_state *ks = &session->key[KS_PRIMARY]; + goto error; + } - /* - * Packet must belong to an existing session. - */ - if (i != TM_ACTIVE && i != TM_UNTRUSTED) - { - msg(D_TLS_ERRORS, - "TLS Error: Unroutable control packet received from %s (si=%d op=%s)", - print_link_socket_actual(from, &gc), - i, - packet_opcode_name(op)); - goto error; - } + key_state_soft_reset(session); - /* - * Verify remote IP address - */ - if (!new_link && !link_socket_actual_match(&ks->remote_addr, from)) - { - msg(D_TLS_ERRORS, "TLS Error: Received control packet from unexpected IP addr: %s", - print_link_socket_actual(from, &gc)); - goto error; - } + dmsg(D_TLS_DEBUG, + "TLS: received P_CONTROL_SOFT_RESET_V1 s=%d sid=%s", + i, session_id_print(&sid, &gc)); + } + else + { + /* + * Remote responding to our key renegotiation request? + */ + if (op == P_CONTROL_SOFT_RESET_V1) + { + do_burst = true; + } - /* - * Remote is requesting a key renegotiation - */ - if (op == P_CONTROL_SOFT_RESET_V1 - && DECRYPT_KEY_ENABLED(multi, ks)) - { - if (!read_control_auth(buf, &session->tls_wrap, from, - session->opt)) - { - goto error; - } + if (!read_control_auth(buf, &session->tls_wrap, from, + session->opt)) + { + goto error; + } - key_state_soft_reset(session); + dmsg(D_TLS_DEBUG, + "TLS: received control channel packet s#=%d sid=%s", + i, session_id_print(&sid, &gc)); + } + } - dmsg(D_TLS_DEBUG, - "TLS: received P_CONTROL_SOFT_RESET_V1 s=%d sid=%s", - i, session_id_print(&sid, &gc)); - } - else - { - /* - * Remote responding to our key renegotiation request? - */ - if (op == P_CONTROL_SOFT_RESET_V1) - { - do_burst = true; - } + /* + * We have an authenticated control channel packet (if --tls-auth was set). + * Now pass to our reliability layer which deals with + * packet acknowledgements, retransmits, sequencing, etc. + */ + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; - if (!read_control_auth(buf, &session->tls_wrap, from, - session->opt)) - { - goto error; - } + /* Make sure we were initialized and that we're not in an error state */ + ASSERT(ks->state != S_UNDEF); + ASSERT(ks->state != S_ERROR); + ASSERT(session_id_defined(&session->session_id)); - dmsg(D_TLS_DEBUG, - "TLS: received control channel packet s#=%d sid=%s", - i, session_id_print(&sid, &gc)); - } - } + /* Let our caller know we processed a control channel packet */ + ret = true; - /* - * We have an authenticated control channel packet (if --tls-auth was set). - * Now pass to our reliability layer which deals with - * packet acknowledgements, retransmits, sequencing, etc. - */ - { - struct tls_session *session = &multi->session[i]; - struct key_state *ks = &session->key[KS_PRIMARY]; + /* + * Set our remote address and remote session_id + */ + if (new_link) + { + ks->session_id_remote = sid; + ks->remote_addr = *from; + ++multi->n_sessions; + } + else if (!link_socket_actual_match(&ks->remote_addr, from)) + { + msg(D_TLS_ERRORS, + "TLS Error: Existing session control channel packet from unknown IP address: %s", + print_link_socket_actual(from, &gc)); + goto error; + } - /* Make sure we were initialized and that we're not in an error state */ - ASSERT(ks->state != S_UNDEF); - ASSERT(ks->state != S_ERROR); - ASSERT(session_id_defined(&session->session_id)); + /* + * Should we do a retransmit of all unacknowledged packets in + * the send buffer? This improves the start-up efficiency of the + * initial key negotiation after the 2nd peer comes online. + */ + if (do_burst && !session->burst) + { + reliable_schedule_now(ks->send_reliable); + session->burst = true; + } - /* Let our caller know we processed a control channel packet */ - ret = true; + /* Check key_id */ + if (ks->key_id != key_id) + { + msg(D_TLS_ERRORS, + "TLS ERROR: local/remote key IDs out of sync (%d/%d) ID: %s", + ks->key_id, key_id, print_key_id(multi, &gc)); + goto error; + } - /* - * Set our remote address and remote session_id - */ - if (new_link) - { - ks->session_id_remote = sid; - ks->remote_addr = *from; - ++multi->n_sessions; - } - else if (!link_socket_actual_match(&ks->remote_addr, from)) - { - msg(D_TLS_ERRORS, - "TLS Error: Existing session control channel packet from unknown IP address: %s", - print_link_socket_actual(from, &gc)); - goto error; - } + /* + * Process incoming ACKs for packets we can now + * delete from reliable send buffer + */ + { + /* buffers all packet IDs to delete from send_reliable */ + struct reliable_ack send_ack; - /* - * Should we do a retransmit of all unacknowledged packets in - * the send buffer? This improves the start-up efficiency of the - * initial key negotiation after the 2nd peer comes online. - */ - if (do_burst && !session->burst) - { - reliable_schedule_now(ks->send_reliable); - session->burst = true; - } + send_ack.len = 0; + if (!reliable_ack_read(&send_ack, buf, &session->session_id)) + { + msg(D_TLS_ERRORS, + "TLS Error: reading acknowledgement record from packet"); + goto error; + } + reliable_send_purge(ks->send_reliable, &send_ack); + } - /* Check key_id */ - if (ks->key_id != key_id) - { - msg(D_TLS_ERRORS, - "TLS ERROR: local/remote key IDs out of sync (%d/%d) ID: %s", - ks->key_id, key_id, print_key_id(multi, &gc)); - goto error; - } + if (op != P_ACK_V1 && reliable_can_get(ks->rec_reliable)) + { + packet_id_type id; - /* - * Process incoming ACKs for packets we can now - * delete from reliable send buffer - */ + /* Extract the packet ID from the packet */ + if (reliable_ack_read_packet_id(buf, &id)) + { + /* Avoid deadlock by rejecting packet that would de-sequentialize receive buffer */ + if (reliable_wont_break_sequentiality(ks->rec_reliable, id)) + { + if (reliable_not_replay(ks->rec_reliable, id)) { - /* buffers all packet IDs to delete from send_reliable */ - struct reliable_ack send_ack; - - send_ack.len = 0; - if (!reliable_ack_read(&send_ack, buf, &session->session_id)) + /* Save incoming ciphertext packet to reliable buffer */ + struct buffer *in = reliable_get_buf(ks->rec_reliable); + ASSERT(in); + if (!buf_copy(in, buf)) { - msg(D_TLS_ERRORS, - "TLS Error: reading acknowledgement record from packet"); + msg(D_MULTI_DROPPED, + "Incoming control channel packet too big, dropping."); goto error; } - reliable_send_purge(ks->send_reliable, &send_ack); + reliable_mark_active_incoming(ks->rec_reliable, in, id, op); } - if (op != P_ACK_V1 && reliable_can_get(ks->rec_reliable)) - { - packet_id_type id; - - /* Extract the packet ID from the packet */ - if (reliable_ack_read_packet_id(buf, &id)) - { - /* Avoid deadlock by rejecting packet that would de-sequentialize receive buffer */ - if (reliable_wont_break_sequentiality(ks->rec_reliable, id)) - { - if (reliable_not_replay(ks->rec_reliable, id)) - { - /* Save incoming ciphertext packet to reliable buffer */ - struct buffer *in = reliable_get_buf(ks->rec_reliable); - ASSERT(in); - if (!buf_copy(in, buf)) - { - msg(D_MULTI_DROPPED, - "Incoming control channel packet too big, dropping."); - goto error; - } - reliable_mark_active_incoming(ks->rec_reliable, in, id, op); - } - - /* Process outgoing acknowledgment for packet just received, even if it's a replay */ - reliable_ack_acknowledge_packet_id(ks->rec_ack, id); - } - } - } + /* Process outgoing acknowledgment for packet just received, even if it's a replay */ + reliable_ack_acknowledge_packet_id(ks->rec_ack, id); } } } @@ -3645,7 +3675,6 @@ done: error: ++multi->n_soft_errors; -error_lite: tls_clear_error(); goto done; } From patchwork Tue Jul 21 23:30:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1322 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id aL2BMIIHGF/TRgAAIUCqbw for ; Wed, 22 Jul 2020 05:31:46 -0400 Received: from proxy8.mail.ord1d.rsapps.net ([172.30.191.6]) by director11.mail.ord1d.rsapps.net with LMTP id mIZ0MIIHGF+mAwAAvGGmqA ; Wed, 22 Jul 2020 05:31:46 -0400 Received: from smtp9.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy8.mail.ord1d.rsapps.net with LMTP id uB4/MIIHGF+zTgAAGdz6CA ; Wed, 22 Jul 2020 05:31: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: smtp9.gate.ord1d.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: 28f23436-cbfe-11ea-972c-525400bd3b1f-1-1 Received: from [216.105.38.7] ([216.105.38.7:39970] helo=lists.sourceforge.net) by smtp9.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 8D/AB-09877-287081F5; Wed, 22 Jul 2020 05:31:46 -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 1jyB5g-0002nX-0f; Wed, 22 Jul 2020 09:31:08 +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 1jyB5f-0002nP-DU for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 09:31:07 +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=mCfHMiQEOVSW1KlSOjZeTNAmhfmQw2TBjPULEmUqK64=; b=NJtEeTdI1uGwP0rglFq2RhGHDK juBsiOtViEGN93Z8TLsMOacmYcFmGA2gyjsW8ddbgfEpvsMoSCvNTUvfVkAsmHBwr4oC8PNWZ2lig H6bixsRV3xfnA6cCTRyOibUgqruJMHzUp9Elh1uduRxgC+HiIsPxxAZxpqON0bQp7rLw=; 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=mCfHMiQEOVSW1KlSOjZeTNAmhfmQw2TBjPULEmUqK64=; b=gAtLdtV4AmvJA+4gcbFu+0nmAr 3xkAz0dvZ3gO12EILab3bS4GUCk2KSwyQyW288v00c5hb6fWUZenscQ8ReuiZlNJHmPkKDbg/1g3M jU2xngFw4IXCHiL4horY9rPPXQ1PQAHrjy4aa+hDEOgtAi/UXZgwpRmNR6YpjfyhaS2g=; 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 1jyB5d-00DJLC-S1 for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 09:31:07 +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 1jyB5O-0005Oi-MY for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 11:30:50 +0200 Received: (nullmailer pid 20522 invoked by uid 10006); Wed, 22 Jul 2020 09:30:50 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Wed, 22 Jul 2020 11:30:49 +0200 Message-Id: <20200722093050.20474-2-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200722093050.20474-1-arne@rfc2549.org> References: <20200722093050.20474-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.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 X-Headers-End: 1jyB5d-00DJLC-S1 Subject: [Openvpn-devel] [PATCH 2/3] 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 --- src/openvpn/ssl.c | 219 ++++++++++++++++++++++------------------------ 1 file changed, 106 insertions(+), 113 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 6d146a63..916d2d37 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -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,50 @@ 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) { - int i; - struct key_state *ks_select = NULL; - for (i = 0; i < KEY_SCAN_SIZE; ++i) + buf->len = 0; + opt = NULL; + } + + struct key_state *ks_select = NULL; + for (int 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 From patchwork Tue Jul 21 23:30:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1321 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.27.255.51]) by backend30.mail.ord1d.rsapps.net with LMTP id 8NM9HIIHGF/TRgAAIUCqbw for ; Wed, 22 Jul 2020 05:31:46 -0400 Received: from proxy5.mail.iad3a.rsapps.net ([172.27.255.51]) by director11.mail.ord1d.rsapps.net with LMTP id SA+eGYIHGF/fAwAAvGGmqA ; Wed, 22 Jul 2020 05:31:46 -0400 Received: from smtp38.gate.iad3a ([172.27.255.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy5.mail.iad3a.rsapps.net with LMTP id KIC3E4IHGF8MZwAAhn5joQ ; Wed, 22 Jul 2020 05:31: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: smtp38.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: 289cc0d2-cbfe-11ea-a412-525400000c92-1-1 Received: from [216.105.38.7] ([216.105.38.7:40754] helo=lists.sourceforge.net) by smtp38.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 24/FB-12730-187081F5; Wed, 22 Jul 2020 05:31:45 -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 1jyB5Z-0002pU-FA; Wed, 22 Jul 2020 09:31:01 +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 1jyB5X-0002op-BB for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 09:30:59 +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=m8tslenaiu6icrBqgSXAse/g98w5g/DubDNNRoNgZu8=; b=fV31zGlsKZE9rAb0pKr4PJleLk dMipFGd80bU601bhvW/PaUBNEcOT5YXMfnYgNE9iP0kH+kW8MKR21UKK+Xr+D1ydROsiHavY1+qA9 MPA5gDXyoILUTkYpVXS1xbee3usLZ8pkbb7lCm9+ORST0GxbRpbKxUDqcIRhzTn9tfUs=; 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=m8tslenaiu6icrBqgSXAse/g98w5g/DubDNNRoNgZu8=; b=iDogCsAM3UhGjCXBQzBFJiisxh Sp9dS+AgH+jLFdga1zFjV9u2DYsjrDaLTQ7vxPJ55fnISjJVpKHk1bhh3mn7pRgY+VT2unZRUiSDT OPNa8mwZUnEsWodHnF/oX0b7V8o7U9d3RKAv8k4vF0WI/8uBfTPFDRQ3RRDnmav2cwAA=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-4.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1jyB5V-00GKGX-2e for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 09:30:59 +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 1jyB5O-0005Om-Ou for openvpn-devel@lists.sourceforge.net; Wed, 22 Jul 2020 11:30:50 +0200 Received: (nullmailer pid 20525 invoked by uid 10006); Wed, 22 Jul 2020 09:30:50 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Wed, 22 Jul 2020 11:30:50 +0200 Message-Id: <20200722093050.20474-3-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200722093050.20474-1-arne@rfc2549.org> References: <20200722093050.20474-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.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 -0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1jyB5V-00GKGX-2e Subject: [Openvpn-devel] [PATCH 3/3] Clean up a number of leftover C89 initialisations in ssl.c 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 Signed-off-by: Arne Schwabe --- src/openvpn/ssl.c | 56 +++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 916d2d37..7e5c0805 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -831,10 +831,9 @@ session_index_name(int index) static const char * print_key_id(struct tls_multi *multi, struct gc_arena *gc) { - int i; struct buffer out = alloc_buf_gc(256, gc); - for (i = 0; i < KEY_SCAN_SIZE; ++i) + for (int i = 0; i < KEY_SCAN_SIZE; ++i) { struct key_state *ks = multi->key_scan[i]; buf_printf(&out, " [key#%d state=%s id=%d sid=%s]", i, @@ -1315,8 +1314,6 @@ tls_multi_init_set_options(struct tls_multi *multi, void tls_multi_free(struct tls_multi *multi, bool clear) { - int i; - ASSERT(multi); auth_set_client_reason(multi, NULL); @@ -1339,7 +1336,7 @@ tls_multi_free(struct tls_multi *multi, bool clear) free(multi->remote_ciphername); - for (i = 0; i < TM_SIZE; ++i) + for (int i = 0; i < TM_SIZE; ++i) { tls_session_free(&multi->session[i], false); } @@ -1367,11 +1364,10 @@ tls_multi_free(struct tls_multi *multi, bool clear) static bool swap_hmac(struct buffer *buf, const struct crypto_options *co, bool incoming) { - const struct key_ctx *ctx; - ASSERT(co); - ctx = (incoming ? &co->key_ctx_bi.decrypt : &co->key_ctx_bi.encrypt); + const struct key_ctx *ctx = (incoming ? &co->key_ctx_bi.decrypt : + &co->key_ctx_bi.encrypt); ASSERT(ctx->hmac); { @@ -1623,25 +1619,21 @@ tls1_P_hash(const md_kt_t *md_kt, int olen) { struct gc_arena gc = gc_new(); - int chunk; - hmac_ctx_t *ctx; - hmac_ctx_t *ctx_tmp; uint8_t A1[MAX_HMAC_KEY_LENGTH]; - unsigned int A1_len; #ifdef ENABLE_DEBUG const int olen_orig = olen; const uint8_t *out_orig = out; #endif - ctx = hmac_ctx_new(); - ctx_tmp = hmac_ctx_new(); + hmac_ctx_t *ctx = hmac_ctx_new(); + hmac_ctx_t *ctx_tmp = hmac_ctx_new(); dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash sec: %s", format_hex(sec, sec_len, 0, &gc)); dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash seed: %s", format_hex(seed, seed_len, 0, &gc)); - chunk = md_kt_size(md_kt); - A1_len = md_kt_size(md_kt); + int chunk = md_kt_size(md_kt); + unsigned int A1_len = md_kt_size(md_kt); hmac_ctx_init(ctx, sec, sec_len, md_kt); hmac_ctx_init(ctx_tmp, sec, sec_len, md_kt); @@ -1711,21 +1703,18 @@ tls1_PRF(const uint8_t *label, struct gc_arena gc = gc_new(); const md_kt_t *md5 = md_kt_get("MD5"); const md_kt_t *sha1 = md_kt_get("SHA1"); - int len,i; - const uint8_t *S1,*S2; - uint8_t *out2; - out2 = (uint8_t *) gc_malloc(olen, false, &gc); + uint8_t *out2 = (uint8_t *) gc_malloc(olen, false, &gc); - len = slen/2; - S1 = sec; - S2 = &(sec[len]); + int len = slen/2; + const uint8_t *S1 = sec; + const uint8_t *S2 = &(sec[len]); len += (slen&1); /* add for odd, make longer */ tls1_P_hash(md5,S1,len,label,label_len,out1,olen); tls1_P_hash(sha1,S2,len,label,label_len,out2,olen); - for (i = 0; iopt->push_peer_info_detail > 0) { struct env_set *es = session->opt->es; - struct env_item *e; struct buffer out = alloc_buf_gc(512*3, &gc); /* push version */ @@ -2271,7 +2259,7 @@ push_peer_info(struct buffer *buf, struct tls_session *session) } /* push env vars that begin with UV_, IV_PLAT_VER and IV_GUI_VER */ - for (e = es->list; e != NULL; e = e->next) + for (struct env_item *e = es->list; e != NULL; e = e->next) { if (e->string) { @@ -3004,10 +2992,8 @@ tls_multi_process(struct tls_multi *multi, interval_t *wakeup) { struct gc_arena gc = gc_new(); - int i; int active = TLSMP_INACTIVE; bool error = false; - int tas; perf_push(PERF_TLS_MULTI_PROCESS); @@ -3018,7 +3004,7 @@ tls_multi_process(struct tls_multi *multi, * and which has a defined remote IP addr. */ - for (i = 0; i < TM_SIZE; ++i) + for (int i = 0; i < TM_SIZE; ++i) { struct tls_session *session = &multi->session[i]; struct key_state *ks = &session->key[KS_PRIMARY]; @@ -3093,7 +3079,7 @@ tls_multi_process(struct tls_multi *multi, update_time(); - tas = tls_authentication_status(multi, TLS_MULTI_AUTH_STATUS_INTERVAL); + int tas = tls_authentication_status(multi, TLS_MULTI_AUTH_STATUS_INTERVAL); /* * If lame duck session expires, kill it. @@ -3126,7 +3112,7 @@ tls_multi_process(struct tls_multi *multi, */ if (error) { - for (i = 0; i < (int) SIZE(multi->key_scan); ++i) + for (int i = 0; i < (int) SIZE(multi->key_scan); ++i) { if (multi->key_scan[i]->state >= S_ACTIVE) { @@ -3143,7 +3129,7 @@ nohard: const int throw_level = GREMLIN_CONNECTION_FLOOD_LEVEL(multi->opt.gremlin); if (throw_level) { - for (i = 0; i < (int) SIZE(multi->key_scan); ++i) + for (int i = 0; i < (int) SIZE(multi->key_scan); ++i) { if (multi->key_scan[i]->state >= throw_level) { @@ -3956,13 +3942,11 @@ void tls_update_remote_addr(struct tls_multi *multi, const struct link_socket_actual *addr) { struct gc_arena gc = gc_new(); - int i, j; - - for (i = 0; i < TM_SIZE; ++i) + for (int i = 0; i < TM_SIZE; ++i) { struct tls_session *session = &multi->session[i]; - for (j = 0; j < KS_SIZE; ++j) + for (int j = 0; j < KS_SIZE; ++j) { struct key_state *ks = &session->key[j];