From patchwork Thu Dec 15 19:01:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 2919 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:c95:b0:82:e4b3:40a0 with SMTP id p21csp631681dyk; Thu, 15 Dec 2022 11:02:21 -0800 (PST) X-Google-Smtp-Source: AA0mqf51KVuPLY2N/+2Vas+HNohG6lSxlqJ/VrEXE9ZrWymR45v4XjNFsm+dfiN3BioF8Uxdiqt/ X-Received: by 2002:a05:6102:3128:b0:3b1:3719:f2cf with SMTP id f8-20020a056102312800b003b13719f2cfmr18477208vsh.24.1671130937496; Thu, 15 Dec 2022 11:02:17 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671130937; cv=none; d=google.com; s=arc-20160816; b=ERQy8tAkLqPouHyhQEFw3IbhimVq02ubgmIz+XVUURC27VawnCGl1JS5ClG1buvWo5 yNOq8hfu3mS7UcerB+Wulsh0+1l0Q4h0LJ0hmKQtYLo9RaaoncBeocffFdbW3hXIbWGO E6irf8piPRZDV1FQtGdE0dYstmeMwSHvlN3bTjW+7b+4+k9nIQpfs10uD58Spc16cBBQ PGIqhG4Hr5oQQj3c4Ca8wEFm/T9LJ2y0eFnnkXJYG+ySUcaTcm6qoEPTl025/9yWoJKD Y93GH4tLQ320kaDQqDQNjbRoKw8GfMiLGuqdrOm3CA04YxiQPt4kp9HVa1gzOAwLId/B gTaA== 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; bh=ffK+TD/B52Ei/WV2g8wXI3FYKqDgfzWj+MdGohje+uQ=; b=ybB8YZh2TXQEXxpiNBTN7hguCQmlQVneo/DhyGyr6I6+bl5lRSWo+yTKsvwV+FvW8C OHS84sbWtTTM+oHtQK6VkNqxBqBs8ZwC6R2KrTBeAmPiUiPQ4u5eYEy5DBTZxQDAozMw 8JKl5WhL7DruLqAmuq0ylib6f6VGANKtH31/aBszYoQApPr7kTUOWlkmNwBXBReQlVk1 L9aGJL7riVUAETXMVgqMdUYwGbeij8BcYjYks/Rruv0+vzugIcFVBM2q+cqF23r5T1/A LD9Jcb4hoovPxb1zUhu0y79XpxAvb5P6I5oF5k/5TFBg6TNdULSZWa93GREWe3sHvDVQ Rdxg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b="ibf/l7uF"; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=lQmSbALl; 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 c19-20020a67e013000000b003aa236621efsi2002270vsl.499.2022.12.15.11.02.16 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Dec 2022 11:02:17 -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="ibf/l7uF"; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=lQmSbALl; 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-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1p5tUR-00010m-1P; Thu, 15 Dec 2022 19:01:55 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1p5tUP-00010e-G0 for openvpn-devel@lists.sourceforge.net; Thu, 15 Dec 2022 19:01:53 +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=xPgk6Ey0syzEm9M58VHUVKS5LFciu8DEx3T3k9c7d44=; b=ibf/l7uFbSAF0SL3Yc99dG8E5r NMlfawLCcGUSB/0ESzSe9nHs8BlmWGYllkWhBkXDY7p47u0tfh4Yl48ZR3DOWPJvd3x8amuikU420 26ix21SQOvgozH9G5TJeXxxSW45jBfku67Ldwxl6yg1L0TqQw7tL7hthgOwOzT6eeeSI=; 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=xPgk6Ey0syzEm9M58VHUVKS5LFciu8DEx3T3k9c7d44=; b=lQmSbALlaLrP57RA5aekzOXzVC sW8WUY9AyL9BtyHhSi2wNhpDqSLIuOpJlVmaBKCx44NNfsgwour315ZzPIiJZWwPu7ePZV6cK0DFo Xd9CoiJI/EHnRkihXjAH/LyXEwZRL5cPoSggI5OYk8EvHEuf4IXk81LlmhtkPMb1CP4E=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1p5tUN-0001Gi-If for openvpn-devel@lists.sourceforge.net; Thu, 15 Dec 2022 19:01:53 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.95 (FreeBSD)) (envelope-from ) id 1p5tUF-000Kw2-B8 for openvpn-devel@lists.sourceforge.net; Thu, 15 Dec 2022 20:01:43 +0100 Received: (nullmailer pid 2107956 invoked by uid 10006); Thu, 15 Dec 2022 19:01:43 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 15 Dec 2022 20:01:40 +0100 Message-Id: <20221215190143.2107896-6-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221215190143.2107896-1-arne@rfc2549.org> References: <20221215190143.2107896-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Score: 0.3 (/) 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: These empty blocks are intentional but trigger code checkers and were pointed out by Trail of Bits in the security audits. Add comments to them or eliminate them whatever makes more sense. For fallthrough C23 [1] has a standard way to signal that but we not adding a C23 feature to our codebase, so use a comment for now. Content analysis details: (0.3 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 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 X-Headers-End: 1p5tUN-0001Gi-If Subject: [Openvpn-devel] [PATCH 5/8] Eliminate or comment empty blocks and switch fallthrough 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?1752307793754970408?= X-GMAIL-MSGID: =?utf-8?q?1752307793754970408?= These empty blocks are intentional but trigger code checkers and were pointed out by Trail of Bits in the security audits. Add comments to them or eliminate them whatever makes more sense. For fallthrough C23 [1] has a standard way to signal that but we not adding a C23 feature to our codebase, so use a comment for now. [1] https://en.cppreference.com/w/c/language/attributes/fallthrough Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/comp-lz4.c | 1 + src/openvpn/crypto.c | 1 + src/openvpn/init.c | 1 + src/openvpn/lzo.c | 1 + src/openvpn/options.c | 5 +-- src/openvpn/ssl_openssl.c | 68 ++++++++++++++++++--------------------- 6 files changed, 36 insertions(+), 41 deletions(-) diff --git a/src/openvpn/comp-lz4.c b/src/openvpn/comp-lz4.c index b456182e7..b54775b7e 100644 --- a/src/openvpn/comp-lz4.c +++ b/src/openvpn/comp-lz4.c @@ -237,6 +237,7 @@ lz4_decompress(struct buffer *buf, struct buffer work, } else if (c == NO_COMPRESS_BYTE_SWAP) /* packet was not compressed */ { + /* nothing to do */ } else { diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index d266716c7..d735d7160 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -1300,6 +1300,7 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags) } else if (isspace(c)) { + /* ignore white space characters */ } else { diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 74b380327..219bff84c 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2921,6 +2921,7 @@ do_init_crypto_tls_c1(struct context *c) case AR_INTERACT: ssl_purge_auth(false); + /* Intentional [[fallthrough]]; */ case AR_NOINTERACT: c->sig->signal_received = SIGUSR1; /* SOFT-SIGUSR1 -- Password failure error */ diff --git a/src/openvpn/lzo.c b/src/openvpn/lzo.c index 39e833cb3..ef6c4c8d7 100644 --- a/src/openvpn/lzo.c +++ b/src/openvpn/lzo.c @@ -250,6 +250,7 @@ lzo_decompress(struct buffer *buf, struct buffer work, } else if (c == NO_COMPRESS_BYTE) /* packet was not compressed */ { + /* nothing to do */ } else { diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 1d6c0572c..4383c953e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -2973,10 +2973,7 @@ options_postprocess_verify_ce(const struct options *options, "--auth-user-pass"); } } - else if (sum == 2) - { - } - else + else if (sum != 2) { msg(M_USAGE, "If you use one of --cert or --key, you must use them both"); } diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index cd6d84246..dbf909269 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1826,10 +1826,7 @@ bio_write(BIO *bio, const uint8_t *data, int size, const char *desc) if (i < 0) { - if (BIO_should_retry(bio)) - { - } - else + if (!BIO_should_retry(bio)) { crypto_msg(D_TLS_ERRORS, "TLS ERROR: BIO write %s error", desc); ret = -1; @@ -1873,51 +1870,48 @@ bio_write_post(const int status, struct buffer *buf) static int bio_read(BIO *bio, struct buffer *buf, const char *desc) { - int i; - int ret = 0; ASSERT(buf->len >= 0); if (buf->len) { + /* we only want to write empty buffers, ignore read request + * if the buffer is not empty */ + return 0; } - else - { - int len = buf_forward_capacity(buf); + int len = buf_forward_capacity(buf); - /* - * BIO_read brackets most of the serious RSA - * key negotiation number crunching. - */ - i = BIO_read(bio, BPTR(buf), len); + /* + * BIO_read brackets most of the serious RSA + * key negotiation number crunching. + */ + int i = BIO_read(bio, BPTR(buf), len); - VALGRIND_MAKE_READABLE((void *) &i, sizeof(i)); + VALGRIND_MAKE_READABLE((void *) &i, sizeof(i)); #ifdef BIO_DEBUG - bio_debug_data("read", bio, BPTR(buf), i, desc); + bio_debug_data("read", bio, BPTR(buf), i, desc); #endif - if (i < 0) - { - if (BIO_should_retry(bio)) - { - } - else - { - crypto_msg(D_TLS_ERRORS, "TLS_ERROR: BIO read %s error", desc); - buf->len = 0; - ret = -1; - ERR_clear_error(); - } - } - else if (!i) + + int ret = 0; + if (i < 0) + { + if (!BIO_should_retry(bio)) { + crypto_msg(D_TLS_ERRORS, "TLS_ERROR: BIO read %s error", desc); buf->len = 0; + ret = -1; + ERR_clear_error(); } - else - { /* successful read */ - dmsg(D_HANDSHAKE_VERBOSE, "BIO read %s %d bytes", desc, i); - buf->len = i; - ret = 1; - VALGRIND_MAKE_READABLE((void *) BPTR(buf), BLEN(buf)); - } + } + else if (!i) + { + buf->len = 0; + } + else + { /* successful read */ + dmsg(D_HANDSHAKE_VERBOSE, "BIO read %s %d bytes", desc, i); + buf->len = i; + ret = 1; + VALGRIND_MAKE_READABLE((void *) BPTR(buf), BLEN(buf)); } return ret; }