From patchwork Tue Nov 28 10:37:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Lichtenheld X-Patchwork-Id: 3482 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:693c:368c:b0:fb:b703:d903 with SMTP id sp12csp2465588dyc; Tue, 28 Nov 2023 02:38:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IFpK6es5n60kcSgx+o3DMRla/Nbzf++FY8ACrAmBgggCL7cvQRtiUEe1qFfMYxnvtBb6oRx X-Received: by 2002:a17:90a:d996:b0:285:f76:9d6e with SMTP id d22-20020a17090ad99600b002850f769d6emr15213327pjv.3.1701167920412; Tue, 28 Nov 2023 02:38:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701167920; cv=none; d=google.com; s=arc-20160816; b=CCtXZA17xo8FeCxrcGiVW9HfEv5MIFa/gtsLwlOH4UmnWcX1EUPc6uqM99jnQk+gHM 7ijzAI9teSm+s0f9KkkWgZXO2yxEaabhZuhgte9K3yN353W9a0SmLuF6ZP31YE+fMaOt E+IAigLvwGmh0ZBxWLy3vtJzHZs14zfRcRxs/ikLSUdcDEYyXdQLQOPdJNoemOOjTXwg 2sYmwdsFINqYp26qdWdtzAVZN5rluddhR2rGbYB3OL4QqUYgFOGWrRcp5ptHkTKInv1O 9Cv7dmKv2dmBdLtJgwqtew6GOXUdXtPRb7yOH14sZWGqkqTggMEcySr5HHxq2ViL9WrS 8pJw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature; bh=S7oR2571QTWdtmE82k68FVAjDV7f+XcPwzD35k/M6bw=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=ycUYR19PpSom8MX7h4L4Wi+YHs3ucaap646UHPRRRBGOkrhBZKyXjebAPSAHvl8Tu4 kp1wCbO9NdgkOdMo5cuSgvHPMa7PTEq+IFRezst7Qz71XK7AKv50C+Z12oKLglHFBM3z Bv6z2ur84hd34Crdbd+iPkGj1hFMj0X30A5YTmVfBRvCjC7hwBFrWYcy9c0MDJENzyzV w/wsbjWR6Cq6RouxMEzoqtE8YvJMtMPLZmYcglURiiKz4HeQB8db8UdZAp/17Q9Tj16m JjAIsmkjk8JQg5v8raIcnABRgqujf3NupNXMVpA8HJUTrlLbcCEM+yJf+Apg9G9HuW2f EbSg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=GecpAlOV; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=aE4la8Sr; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b="zyASv/pK"; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id fs2-20020a17090af28200b002858ed80acfsi9473576pjb.131.2023.11.28.02.38.40 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 Nov 2023 02:38:40 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=GecpAlOV; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=aE4la8Sr; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b="zyASv/pK"; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net 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.95) (envelope-from ) id 1r7vTD-0006am-I3; Tue, 28 Nov 2023 10:37:35 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1r7vT9-0006ae-Rg for openvpn-devel@lists.sourceforge.net; Tue, 28 Nov 2023 10:37:33 +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:Cc:To:From:Sender:Reply-To: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=CC5XlfCHkFB0G6Vi3ZILfRfe3SjFroZ1Ig7W5VDLjYc=; b=GecpAlOVqTE+S81530a8W4AA2C WFP1+9X618AaQxNl1ArrAWMFnBOE7SIgmZaVGkGS3WcseP9jXnf6qYK9nkqSdKOTOkQ+mH0n3TOti n6kjB1H/jmX0Eeds/21fqRLsKy1e0akdP7oGu3lRGIOST9GQxCnsEcXlaQb2pdUwPpKE=; 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:Cc:To:From:Sender:Reply-To: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=CC5XlfCHkFB0G6Vi3ZILfRfe3SjFroZ1Ig7W5VDLjYc=; b=aE4la8Srm91lSY7j6F/SC8CxU0 kajOe04phDcC6jglDwBj8xJ07Amjbf6uW8tLUg3FSD6zOXF6QIpJtYIuE2/c91/0BdpbmPOTT+HCx QYAhMAYkVFXMME9oL6WN4bsdnT7KqTPEzM/pRHkmd9P5Asxr1qbXkJjLHeEQRrgRb4ZY=; Received: from mout-p-202.mailbox.org ([80.241.56.172]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1r7vT1-0004yj-QY for openvpn-devel@lists.sourceforge.net; Tue, 28 Nov 2023 10:37:29 +0000 Received: from smtp102.mailbox.org (smtp102.mailbox.org [IPv6:2001:67c:2050:b231:465::102]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-202.mailbox.org (Postfix) with ESMTPS id 4Sff5v02KSz9tLH; Tue, 28 Nov 2023 11:37:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lichtenheld.com; s=MBO0001; t=1701167827; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CC5XlfCHkFB0G6Vi3ZILfRfe3SjFroZ1Ig7W5VDLjYc=; b=zyASv/pKZ/7LEy2Dd8y9+7hNsmtFkFAWQEmZtgIAm3jSQ8oGsNX9yPr7f8Q8c8K7yyIbhA mgWec0xr00iF/m8NHLsyUCJqbMzw2QsHMp0P1lClNnOBw8giH7q9ODKD/41EGgapllugHb XlT0kVJX6oM5u9Uigqjs8IDH0JI95uY21QemFz/IFc/4nq4lsIT95btAc6Zb2d46G/+nI2 +Q9FA5XC2YWbEW7gtM4+S200a173BHMF3D6apqmWKPMHk6o6Xr7HDn2USdjRX9L0jQo2RH C+dYtVEa/g+GH3DRyoUntic1gYs2BUT5wFjWWXkH1oOeXGm7PwlMOE5Z/f8rmQ== From: Frank Lichtenheld To: openvpn-devel@lists.sourceforge.net Date: Tue, 28 Nov 2023 11:37:04 +0100 Message-Id: <20231128103704.61046-1-frank@lichtenheld.com> In-Reply-To: References: MIME-Version: 1.0 X-Rspamd-Queue-Id: 4Sff5v02KSz9tLH X-Spam-Score: -0.9 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Arne Schwabe The name state_change is more confusing than helpful as it not really indicates if there was a state change but rather if processing should be continued. There even some states that are definitively s [...] Content analysis details: (-0.9 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at https://www.dnswl.org/, low trust [80.241.56.172 listed in list.dnswl.org] 0.0 RCVD_IN_MSPIKE_H5 RBL: Excellent reputation (+5) [80.241.56.172 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.0 RCVD_IN_MSPIKE_WL Mailspike good senders -0.0 T_SCC_BODY_TEXT_LINE No description available. 0.0 T_FILL_THIS_FORM_SHORT Fill in a short form with personal information X-Headers-End: 1r7vT1-0004yj-QY Subject: [Openvpn-devel] [PATCH v3] Rename state_change to continue_tls_process 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 X-GMAIL-THRID: =?utf-8?q?1783803853185717503?= X-GMAIL-MSGID: =?utf-8?q?1783803853185717503?= From: Arne Schwabe The name state_change is more confusing than helpful as it not really indicates if there was a state change but rather if processing should be continued. There even some states that are definitively state changes (setting to_link buffer) that require continue_tls_process to be set to false. Change-Id: Ib6d713f2eb08a4c39d97de3e1a4a832cedc09585 Acked-by: Frank Lichtenheld Signed-off-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/452 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 400230c..20f4956 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2694,7 +2694,7 @@ */ static bool read_incoming_tls_ciphertext(struct buffer *buf, struct key_state *ks, - bool *state_change) + bool *continue_tls_process) { int status = 0; if (buf->len) @@ -2714,7 +2714,7 @@ if (status == 1) { reliable_mark_deleted(ks->rec_reliable, buf); - *state_change = true; + *continue_tls_process = true; dmsg(D_TLS_DEBUG, "Incoming Ciphertext -> TLS"); } return true; @@ -2730,7 +2730,7 @@ static bool read_incoming_tls_plaintext(struct key_state *ks, struct buffer *buf, - interval_t *wakeup, bool *state_change) + interval_t *wakeup, bool *continue_tls_process) { ASSERT(buf_init(buf, 0)); @@ -2744,7 +2744,7 @@ } if (status == 1) { - *state_change = true; + *continue_tls_process = true; dmsg(D_TLS_DEBUG, "TLS -> Incoming Plaintext"); /* More data may be available, wake up again asap to check. */ @@ -2754,7 +2754,7 @@ } static bool -write_outgoing_tls_ciphertext(struct tls_session *session, bool *state_change) +write_outgoing_tls_ciphertext(struct tls_session *session, bool *continue_tls_process) { struct key_state *ks = &session->key[KS_PRIMARY]; @@ -2830,7 +2830,7 @@ reliable_mark_active_outgoing(ks->send_reliable, buf, opcode); INCR_GENERATED; - *state_change = true; + *continue_tls_process = true; } dmsg(D_TLS_DEBUG, "Outgoing Ciphertext -> Reliable"); } @@ -2839,7 +2839,6 @@ return true; } - static bool tls_process_state(struct tls_multi *multi, struct tls_session *session, @@ -2848,13 +2847,19 @@ struct link_socket_info *to_link_socket_info, interval_t *wakeup) { - bool state_change = false; + /* This variable indicates if we should call this method + * again to process more incoming/outgoing TLS state/data + * We want to repeat this until we either determined that there + * is nothing more to process or that further processing + * should only be done after the outer loop (sending packets etc.) + * has run once more */ + bool continue_tls_process = false; struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ /* Initial handshake */ if (ks->state == S_INITIAL) { - state_change = session_move_pre_start(session, ks, false); + continue_tls_process = session_move_pre_start(session, ks, false); } /* Are we timed out on receive? */ @@ -2872,7 +2877,7 @@ if (ks->state == S_PRE_START && reliable_empty(ks->send_reliable)) { ks->state = S_START; - state_change = true; + continue_tls_process = true; /* New connection, remove any old X509 env variables */ tls_x509_clear_env(session->opt->es); @@ -2885,7 +2890,7 @@ && reliable_empty(ks->send_reliable)) { session_move_active(multi, session, to_link_socket_info, ks); - state_change = true; + continue_tls_process = true; } /* Reliable buffer to outgoing TCP/UDP (send up to CONTROL_SEND_ACK_MAX ACKs @@ -2927,7 +2932,7 @@ } else { - if (!read_incoming_tls_ciphertext(&entry->buf, ks, &state_change)) + if (!read_incoming_tls_ciphertext(&entry->buf, ks, &continue_tls_process)) { goto error; } @@ -2938,7 +2943,7 @@ struct buffer *buf = &ks->plaintext_read_buf; if (!buf->len) { - if (!read_incoming_tls_plaintext(ks, buf, wakeup, &state_change)) + if (!read_incoming_tls_plaintext(ks, buf, wakeup, &continue_tls_process)) { goto error; } @@ -2954,7 +2959,7 @@ goto error; } - state_change = true; + continue_tls_process = true; dmsg(D_TLS_DEBUG_MED, "STATE S_SENT_KEY"); ks->state = S_SENT_KEY; } @@ -2970,7 +2975,7 @@ goto error; } - state_change = true; + continue_tls_process = true; dmsg(D_TLS_DEBUG_MED, "STATE S_GOT_KEY"); ks->state = S_GOT_KEY; } @@ -2988,7 +2993,7 @@ } if (status == 1) { - state_change = true; + continue_tls_process = true; dmsg(D_TLS_DEBUG, "Outgoing Plaintext -> TLS"); } } @@ -2999,14 +3004,14 @@ buf = reliable_get_buf_output_sequenced(ks->send_reliable); if (buf) { - if (!write_outgoing_tls_ciphertext(session, &state_change)) + if (!write_outgoing_tls_ciphertext(session, &continue_tls_process)) { goto error; } } } - return state_change; + return continue_tls_process; error: tls_clear_error(); ks->state = S_ERROR; @@ -3065,19 +3070,19 @@ msg(D_TLS_DEBUG_LOW, "TLS: tls_process: killed expiring key"); } - bool state_change = true; - while (state_change) + bool continue_tls_process = true; + while (continue_tls_process) { update_time(); dmsg(D_TLS_DEBUG, "TLS: tls_process: chg=%d ks=%s lame=%s to_link->len=%d wakeup=%d", - state_change, + continue_tls_process, state_name(ks->state), state_name(ks_lame->state), to_link->len, *wakeup); - state_change = tls_process_state(multi, session, to_link, to_link_addr, - to_link_socket_info, wakeup); + continue_tls_process = tls_process_state(multi, session, to_link, to_link_addr, + to_link_socket_info, wakeup); if (ks->state == S_ERROR) {