From patchwork Wed Jul 10 14:06:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Lichtenheld X-Patchwork-Id: 3762 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:518c:b0:59e:d24b:d55c with SMTP id g12csp367829mae; Wed, 10 Jul 2024 07:07:02 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVGmHQC29koLBFnpqWXVFIAuZaelqyGhLvm6xCu0hPMiiEYnts98GYSbsV3jL5Rd5o1mAyh473gIMBRkPm/Et/gylcNDGY= X-Google-Smtp-Source: AGHT+IGeuji2sWi5HXNKlRu5AGaWdSGQfUdRSm28uQUOJfJS3TzUBkeSXfhlRiKbiZroOXRTlSU1 X-Received: by 2002:a17:902:f546:b0:1f3:10e8:1e0 with SMTP id d9443c01a7336-1fbb6d16ce2mr58888155ad.2.1720620422331; Wed, 10 Jul 2024 07:07:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1720620422; cv=none; d=google.com; s=arc-20160816; b=qxJs5unzpkgGAt392jk1CF3VIvIJMUI1UXh3vDHJ/cSdp+LWSLOoofH1OnYKjeqxw1 nmh2ziQ1xlBbv777l3efHoAaLUn5c4F9QAtEFtjVJQikdCsMLNJG0ahpsWYQekyb2eUd uAN6wKpwZsPV8U+SjnhAGHqz0ZE8y/MwzWyMhmVfkcrwI/3tuOk/DBahkt0z2VhvCoG+ +0P4R7nF9sg9XkEzVCL50pWDCILgQmEoBvFWNivYaLQuIjWgoK6Wgd/Um5ZpANFvAXv0 2rLwne1/JcxE556e3qduZBeEnfel7U4gjaM5+z8op665NAdkXHogl+SfvTOL2wkqx6Wj vwow== 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=ZXyTm6WGByzHZMpUivrqr98nEL8If5x1haxzb/JQc5Y=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=z6LVdsJFK1uNkMvCJRo8CXXl8qI1qUW67rbboRqgmH1F5HBSBItvXo397ZdG8dU1E2 6fsDBTUo2mbsr2n/4/W7GqTCDLaUsVV2ZaZsfdvea+O70nnmgpKGEq0cyopC6e95J1yo +i/h5l9GYN3xjUNlG82qBc2LfiBAnosExJvecU0pbgTgLYQJxxljAUrwtg2wKy/2emzq YrgLveO9+wXfcIHoLUWcBApnzcJ4IsZ76cXLIcYdmKXF7pXSoyFPuVCTyH10NF/pK66F M/qwMBsuaJnifqmlzkYcXy+zOxwU3hXtIPpuDRD59RUD/eWx9j19/Q6YO1dezxaMLHHf xyxQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=dza4BiHs; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=YAQO6Jbq; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b=ZVLuhwrg; 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 d9443c01a7336-1fbb6abb88asi5068455ad.337.2024.07.10.07.07.02 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jul 2024 07:07:02 -0700 (PDT) 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=dza4BiHs; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=YAQO6Jbq; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b=ZVLuhwrg; 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-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1sRXy0-0004jR-4S; Wed, 10 Jul 2024 14:06:43 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-3.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1sRXxx-0004jF-7E for openvpn-devel@lists.sourceforge.net; Wed, 10 Jul 2024 14:06:41 +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=SBU0na0HVA6rlfVif/p6mZizDDl/ApzNiX2wdSix0Ek=; b=dza4BiHsWb9Uy0OmqsB+FUrQX0 PATcTXbmTr+DbTITwfr80i1othWuW1/igiSLEGdgUpcRbEmDt13GHOPWju3hCJiDXoxXqfeymSCFq unCmk33X+u8OYLiegRFO+jFictvnylYYt+RhxYoJe+FBn2bW9rSBO4WF1miBwz8RTxYE=; 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=SBU0na0HVA6rlfVif/p6mZizDDl/ApzNiX2wdSix0Ek=; b=YAQO6JbqWpI/50ZZGjlwKa4eSR RMBga69HHSiAd3ZswregLrodWo/gxPQPpuVSkJ6fwimATbceJGJl7A1H70F//RqgFXfC3OWcWbadM NragtFyKanvJPZf8S0t0BA4FxHdMm7YmhOC75LEolqfWtYFUv0ucsoL3elPhD6dm683c=; Received: from mout-p-103.mailbox.org ([80.241.56.161]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1sRXxv-00035q-DK for openvpn-devel@lists.sourceforge.net; Wed, 10 Jul 2024 14:06:40 +0000 Received: from smtp202.mailbox.org (smtp202.mailbox.org [IPv6:2001:67c:2050:b231:465::202]) (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-103.mailbox.org (Postfix) with ESMTPS id 4WK05Y0cMzz9sd6; Wed, 10 Jul 2024 16:06:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lichtenheld.com; s=MBO0001; t=1720620385; 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=SBU0na0HVA6rlfVif/p6mZizDDl/ApzNiX2wdSix0Ek=; b=ZVLuhwrg9dADfk3E3ZNjOdJL+x+z4xX/IVO98O6EDw7TCbiW8eF8u6aQIl/NjfemoVd2ve eB/uPFF9mrPxK1YdZ1WNdy27w1ZGNz+Os7Cq+sYuTK8xn8LvQKCNzd1HNTiTUXuyPb7kPU Vk2KltlkWs9YREd1mz7ek7hW/Z6xa6i27bPr8dfzyX4cbWbXfV/6LcechL5sNRcsKWRZyu zBwEcna+VoCiTp0Pb5AAvzncoC44GE9QuWyTlYWBOiDPD2wU6uGyVBW98htqwFUm8FvQR3 J7gYFnI29jk2mFpdYJdc7Rxq1McPwthmUN0k3Lfym0M9C10wpX+s+bsSdjM2JA== From: Frank Lichtenheld To: openvpn-devel@lists.sourceforge.net Date: Wed, 10 Jul 2024 16:06:23 +0200 Message-Id: <20240710140623.172829-1-frank@lichtenheld.com> In-Reply-To: References: MIME-Version: 1.0 X-Rspamd-Queue-Id: 4WK05Y0cMzz9sd6 X-Spam-Score: -0.2 (/) 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 Writing a reason from a script will easily end up adding extra \r\n characters at the end of the reason. Our current code pushes this to the peer. So be more liberal in accepting these message. Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 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: openvpn.net] 0.0 RCVD_IN_VALIDITY_SAFE_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [80.241.56.161 listed in sa-trusted.bondedsender.org] 0.0 RCVD_IN_VALIDITY_RPBL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [80.241.56.161 listed in bl.score.senderscore.com] 0.0 RCVD_IN_DNSWL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to DNSWL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [80.241.56.161 listed in list.dnswl.org] -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 Message has at least one valid DKIM or DK signature -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_AU Message has a valid DKIM or DK signature from author's domain X-Headers-End: 1sRXxv-00035q-DK Subject: [Openvpn-devel] [PATCH v4] Allow trailing \r and \n in control channel message 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?1804201279825101054?= X-GMAIL-MSGID: =?utf-8?q?1804201279825101054?= From: Arne Schwabe Writing a reason from a script will easily end up adding extra \r\n characters at the end of the reason. Our current code pushes this to the peer. So be more liberal in accepting these message. Closes openvpn/openvpn#568 Change-Id: I47c992b6b73b1475cbff8a28f720cf50dc1fbe3e Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld --- 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/+/672 This mail reflects revision 4 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 71b7167..40b7cc4 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -290,41 +290,14 @@ struct buffer buf = alloc_buf_gc(len, &gc); if (tls_rec_payload(c->c2.tls_multi, &buf)) { - while (BLEN(&buf) > 1) { - /* commands on the control channel are seperated by 0x00 bytes. - * cmdlen does not include the 0 byte of the string */ - int cmdlen = (int)strnlen(BSTR(&buf), BLEN(&buf)); + struct buffer cmdbuf = extract_command_buffer(&buf, &gc); - if (cmdlen < BLEN(&buf)) + if (cmdbuf.len > 0) { - /* include the NUL byte and ensure NUL termination */ - int cmdlen = (int)strlen(BSTR(&buf)) + 1; - - /* Construct a buffer that only holds the current command and - * its closing NUL byte */ - struct buffer cmdbuf = alloc_buf_gc(cmdlen, &gc); - buf_write(&cmdbuf, BPTR(&buf), cmdlen); - - /* check we have only printable characters or null byte in the - * command string and no newlines */ - if (!string_check_buf(&buf, CC_PRINT | CC_NULL, CC_CRLF)) - { - msg(D_PUSH_ERRORS, "WARNING: Received control with invalid characters: %s", - format_hex(BPTR(&buf), BLEN(&buf), 256, &gc)); - } - else - { - parse_incoming_control_channel_command(c, &cmdbuf); - } + parse_incoming_control_channel_command(c, &cmdbuf); } - else - { - msg(D_PUSH_ERRORS, "WARNING: Ignoring control channel " - "message command without NUL termination"); - } - buf_advance(&buf, cmdlen); } } else diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c index 2ec0b2f..689cd7f 100644 --- a/src/openvpn/ssl_pkt.c +++ b/src/openvpn/ssl_pkt.c @@ -557,3 +557,43 @@ } return false; } + +struct buffer +extract_command_buffer(struct buffer *buf, struct gc_arena *gc) +{ + /* commands on the control channel are seperated by 0x00 bytes. + * cmdlen does not include the 0 byte of the string */ + int cmdlen = (int)strnlen(BSTR(buf), BLEN(buf)); + + if (cmdlen >= BLEN(buf)) + { + buf_advance(buf, cmdlen); + /* Return empty buffer */ + struct buffer empty = { 0 }; + return empty; + } + + /* include the NUL byte and ensure NUL termination */ + cmdlen += 1; + + /* Construct a buffer that only holds the current command and + * its closing NUL byte */ + struct buffer cmdbuf = alloc_buf_gc(cmdlen, gc); + buf_write(&cmdbuf, BPTR(buf), cmdlen); + + /* Remove \r and \n at the end of the buffer to avoid + * problems with scripts and other that add extra \r and \n */ + buf_chomp(&cmdbuf); + + /* check we have only printable characters or null byte in the + * command string and no newlines */ + if (!string_check_buf(&cmdbuf, CC_PRINT | CC_NULL, CC_CRLF)) + { + msg(D_PUSH_ERRORS, "WARNING: Received control with invalid characters: %s", + format_hex(BPTR(&cmdbuf), BLEN(&cmdbuf), 256, gc)); + cmdbuf.len = 0; + } + + buf_advance(buf, cmdlen); + return cmdbuf; +} diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h index 88b9e8c..c8a27fb 100644 --- a/src/openvpn/ssl_pkt.h +++ b/src/openvpn/ssl_pkt.h @@ -230,6 +230,20 @@ uint8_t header, bool request_resend_wkc); + +/** + * Extracts a control channel message from buf and adjusts the size of + * buf after the message has been extracted + * @param buf The buffer the message should be extracted from + * @param gc gc_arena to be used for the returned buffer and displaying + * diagnostic messages + * @return A buffer with a control channel message or a buffer with + * with length 0 if there is no message or the message has + * invalid characters. + */ +struct buffer +extract_command_buffer(struct buffer *buf, struct gc_arena *gc); + static inline const char * packet_opcode_name(int op) { diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c index 1084d66..741c982 100644 --- a/tests/unit_tests/openvpn/test_pkt.c +++ b/tests/unit_tests/openvpn/test_pkt.c @@ -626,6 +626,40 @@ free_tas(&tas_server); } +static void +test_extract_control_message(void **ut_state) +{ + struct gc_arena gc = gc_new(); + struct buffer input_buf = alloc_buf_gc(1024, &gc); + + /* This message will have a \0x00 at the end since it is a C string */ + const char input[] = "valid control message\r\n\0\0Invalid\r\none\0valid one again"; + + buf_write(&input_buf, input, sizeof(input)); + struct buffer cmd1 = extract_command_buffer(&input_buf, &gc); + struct buffer cmd2 = extract_command_buffer(&input_buf, &gc); + struct buffer cmd3 = extract_command_buffer(&input_buf, &gc); + struct buffer cmd4 = extract_command_buffer(&input_buf, &gc); + struct buffer cmd5 = extract_command_buffer(&input_buf, &gc); + + assert_string_equal(BSTR(&cmd1), "valid control message"); + /* empty message with just a \0x00 */ + assert_int_equal(cmd2.len, 1); + assert_string_equal(BSTR(&cmd2), ""); + assert_int_equal(cmd3.len, 0); + assert_string_equal(BSTR(&cmd4), "valid one again"); + assert_int_equal(cmd5.len, 0); + + const uint8_t nonull[6] = { 'n', 'o', ' ', 'N', 'U', 'L'}; + struct buffer nonull_buf = alloc_buf_gc(1024, &gc); + + buf_write(&nonull_buf, nonull, sizeof(nonull)); + struct buffer nonullcmd = extract_command_buffer(&nonull_buf, &gc); + assert_int_equal(nonullcmd.len, 0); + + gc_free(&gc); +} + int main(void) { @@ -640,6 +674,7 @@ cmocka_unit_test(test_verify_hmac_tls_auth), cmocka_unit_test(test_generate_reset_packet_plain), cmocka_unit_test(test_generate_reset_packet_tls_auth), + cmocka_unit_test(test_extract_control_message) }; #if defined(ENABLE_CRYPTO_OPENSSL)