From patchwork Mon Jul 8 10:09:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3753 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:298e:b0:59e:d24b:d55c with SMTP id f14csp1753383max; Mon, 8 Jul 2024 03:10:13 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVkjDEdVYplWkbTH4tnm8xtnSaPoZcL2ANfjCeB5XTPJFm6rncQsAOYeLfhXoo+Xd55GjHHzOe0dt/5tzb1KIm3fwx6S5g= X-Google-Smtp-Source: AGHT+IGv6K6wcZnN0WKLJQniLx0sM10jM26EArFbUVtpg2ykK2hSZjvZw3Df4Nwe2YlwtvmOhVxU X-Received: by 2002:aa7:8b8c:0:b0:70a:f603:99a6 with SMTP id d2e1a72fcca58-70b00a93b6dmr11654388b3a.2.1720433413243; Mon, 08 Jul 2024 03:10:13 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1720433413; cv=none; d=google.com; s=arc-20160816; b=mpR+SrYthGUuRIqk+B0G/xcTund/sAH+mUWxe/i9fXu+cSc9/Hi9yEMYSYrQre+J7s I8VsOdSVr3hjDqFnFVNCmPdUQD6dlSwgR1KsCR+3Kt+AZURa/3FJ/rzw1u74tfX8yx8v f94NXTBd8q3NGkpGtPc7PQMt7W91qiu/9M//qUHairzRjkIvEOAqH5zM+22dXoSn6QLF vAkeHA+6+Os64BHq5arOWmfDySqvoHTNnJrD3JvetQm/VlYBUGAg4IdmyuZL58FPSfJW TBS+vzrq5PbmE9acm/ZtDgo5YDhUdTgQCvaIO5b/dbe5JiasyLvJm2h46YBIKiY0e4Ei xb6w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=GFItng0X9w95afY3GtQ7cIlNgg0fhyloy97wOgs9uis=; fh=lm0MLPW7DntlrDqRECIiC9JlE1uPxhepE0URYHIf+eE=; b=son+ClKQsGTH/5zeEbdVoU7PqWbZNx6705qMjimcRYuWgq5yM3AYQzXDVEUvpsqbvr VQ1HKNhPwXhEMUEw7hoXfqMP+tAsg3yQOK8JsQQQbq4vCfJThzQgzh+aEeU2qJOA1gpR aNGFZ6LqcklLepCq2uRX+cbLqno16JE4PVdJTfh25pfsayvKB84NtPMvzrhgiqXFmRV/ NlpUyrrK5U9pwixMCskZPpd7lFg+G/V9SZUUXL16Wb4nHonoELNPIJrSlcMfAJwMaWOd 3B4Ud3U3jGupQYbI7uuOgaEaj9UlgcC161jjrvAd3zVwueKq1nGunLpZKkdPyduKi0mN xkiw==; 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=hOv2WKli; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="Asjpjcv/"; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=FRmss5NF; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id d2e1a72fcca58-70af5316239si10971761b3a.94.2024.07.08.03.10.12 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Jul 2024 03:10:13 -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=hOv2WKli; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="Asjpjcv/"; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=FRmss5NF; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.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 1sQlJP-0007wa-Lj; Mon, 08 Jul 2024 10:09:36 +0000 Received: from [172.30.29.66] (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 1sQlJO-0007wS-Fl for openvpn-devel@lists.sourceforge.net; Mon, 08 Jul 2024 10:09:34 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=tV3AzC171lfrnShWRVZSc7T4j2gbcGywgZsXFAcjWAs=; b=hOv2WKli3kYd1tbVuligTGo/PT zPB9bi3TRvDJ/Um/zy1DGUyFsq/x7JRf5TF21z6nQgZn0Bj96Sc+gqlqOvKEW+oFN+GVflU1O+Zv/ 0nAP78f0XpRwbe6Zqkfwu35J0YiCppzH7fKcO7/FHxo+p5GhvxBVEydsSNNcYEIVMbY4=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=tV3AzC171lfrnShWRVZSc7T4j2gbcGywgZsXFAcjWAs=; b=A sjpjcv/zS6siGH2onJZNcaN00J1XscniupSXU1Xd+GjNtJLblsFDY9agzxhb8Mk5iguEFt8XQkmBi mS0EARuiqprIX1eB95m2qS/9Cv9Nny4ln8283/ca1SEJsZx1e76uUjuBLS9FeLydIYIurtSit0i0g fWuFUbEIjZKOTfcM=; Received: from mail-wm1-f46.google.com ([209.85.128.46]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1sQlJO-0006fS-Ly for openvpn-devel@lists.sourceforge.net; Mon, 08 Jul 2024 10:09:34 +0000 Received: by mail-wm1-f46.google.com with SMTP id 5b1f17b1804b1-4266bb98b4aso4448725e9.3 for ; Mon, 08 Jul 2024 03:09:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1720433367; x=1721038167; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=tV3AzC171lfrnShWRVZSc7T4j2gbcGywgZsXFAcjWAs=; b=FRmss5NFHuuG2Rbi9scggSRAiPyNKtafDS00p3T24YvPuH+qW5lWinBZxUJHkPdHso miYvmevUyFG+3s5txlLbdTEnDzUG/1FRXP7xKp0tf2mF/il46BTAjeWcseol247bfM7j ThOi2DMpKHB8aWIo+SQQ6UzWWj3v3kY3K0fckhUW+zcQKRxNnZTdwxYlk/qH7CwhEaAe WRyCZg1Ba0dPKcPWu2hdGjUfsAdCY8QZa7U0sTvVyzv70RFI+Q7GQluU2VJOzcS17GK+ o12dFHKrR7Qsy6DL5ZWgkf7P9NbsUS0L7rrkaIaXboZz5aEeanSRHchro0fip62Uzmy2 sJ2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720433367; x=1721038167; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=tV3AzC171lfrnShWRVZSc7T4j2gbcGywgZsXFAcjWAs=; b=rWSJ271Nk0xBLzb97oyqV8C/XW/IeBC8sDuwApD2g+3mwXuDk9zSorBwBVmz/g3LLL 84vcaWl+0KCf1bjV5ozQMgtumydgo5yYeGySbl9DLW4H3zIFPWs/HvkvldZ/SZje1R3S o5z7gv0ia5K/PI3wLlluR1xtxmA80IPqEVVCZl92l/yulSKnx/uT02n56bqGlk8asqwj d9+mTo5pM7D9TCIaa2i/MQxuecPOmgCyWPnUBU8SDHDFK9/VWyJpD+Pgg/ewcZGDcctP 6MOWHhth2y14twuJ5Fq2T0c7SiCIsmfWRyRrABczfZ+uiL2DX3Z9xdmqHkfpAQcNqpP4 nIVA== X-Gm-Message-State: AOJu0YzW8EZ8IMEPbdYuDOOkoS63k8/PcErEdOnIYVlSv1pr3Dm9suuJ RAwouujbbv+PP/tvv1OMhpr1FeKlZm59pemRYQo7IM3luIFXGm2j+9sqZohXAaM6YmkfAUlSWWc o X-Received: by 2002:a05:600c:3215:b0:426:545b:ec00 with SMTP id 5b1f17b1804b1-426545bed56mr65698585e9.19.1720433366886; Mon, 08 Jul 2024 03:09:26 -0700 (PDT) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4264a1dd952sm156355975e9.13.2024.07.08.03.09.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Jul 2024 03:09:26 -0700 (PDT) From: "plaisthos (Code Review)" X-Google-Original-From: "plaisthos (Code Review)" X-Gerrit-PatchSet: 1 Date: Mon, 8 Jul 2024 10:09:26 +0000 To: flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I47c992b6b73b1475cbff8a28f720cf50dc1fbe3e X-Gerrit-Change-Number: 672 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 48703d64a7234b7cf7a1319d404f9702a7a7a4e6 References: Message-ID: MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -5.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: Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-5.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at https://www.dnswl.org/, high trust [209.85.128.46 listed in list.dnswl.org] 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_CERTIFIED_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.128.46 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. [209.85.128.46 listed in bl.score.senderscore.com] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.128.46 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.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's 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_EF Message has a valid DKIM or DK signature from envelope-from domain 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1sQlJO-0006fS-Ly Subject: [Openvpn-devel] [S] Change in openvpn[master]: 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: , Reply-To: arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1804005187051343348?= X-GMAIL-MSGID: =?utf-8?q?1804005187051343348?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/672?usp=email to review the following change. Change subject: Allow trailing \r and \n in control channel message ...................................................................... Allow trailing \r and \n in control channel message 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 --- M src/openvpn/forward.c 1 file changed, 14 insertions(+), 6 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/72/672/1 diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 71b7167..0f89876 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -290,7 +290,6 @@ 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. @@ -300,19 +299,28 @@ if (cmdlen < BLEN(&buf)) { /* include the NUL byte and ensure NUL termination */ - int cmdlen = (int)strlen(BSTR(&buf)) + 1; + int buflen = (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); + struct buffer cmdbuf = alloc_buf_gc(buflen, &gc); + buf_write(&cmdbuf, BPTR(&buf), buflen); + + /* Remove \r and \n at the end of the buffer to avoid + * problems with scripts and other that add extra \r and \n */ + while (BLEN(&cmdbuf) + && (*(BEND(&cmdbuf) - 2) == '\r' || *(BEND(&cmdbuf) - 2)== '\n')) + { + *(BEND(&cmdbuf) - 2) = '\0'; + buf_inc_len(&cmdbuf, -1); + } /* 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)) + 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(&buf), BLEN(&buf), 256, &gc)); + format_hex(BPTR(&buf), BLEN(&cmdbuf), 256, &gc)); } else {