From patchwork Tue Sep 12 10:19:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3343 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:390:b0:d7:3b0f:3938 with SMTP id 16csp305064dyq; Tue, 12 Sep 2023 03:19:48 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGjGwde+8//U3jai0/zTJS4fAC1kX10zB7q2BFFzqfpUFsmCVNtZBaPDqN8Er3On9E/cguL X-Received: by 2002:a05:6a20:4422:b0:134:d4d3:f0a5 with SMTP id ce34-20020a056a20442200b00134d4d3f0a5mr16959007pzb.2.1694513988076; Tue, 12 Sep 2023 03:19:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694513988; cv=none; d=google.com; s=arc-20160816; b=nNn675CwyIOKcjlJrBwms+OlghOEj6zwXh0aKZgnWMxBRkjm3Lph+eZzcu1ALDgfUZ yjIVKr+sGwgligIVJdQW0qK7igmC7w9lddW2ddb4jSdQcDaPTCaAcQLRMfJrE7UDq/Bn VaSkKlKhLeSbe1woGJqznOZ0Mx6ULMmtZ8rW0KC9Wszql/aWqOm2bVPQe6ImHB3UWMhj gko6/IQPgCkrhZ7SFO4b4UAuMoSq2BtRKegz48bfspUQ43U2pcq9/3CE47hTxsSFOBr0 +E6zvL0PGoglRiQLucyvhLa6MSo22qFHb3bi67IoSh0TRNsVis31asPSlQtI8h+U1MYp OM/w== 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:date:from :dkim-signature:dkim-signature:dkim-signature; bh=Lb3l7aqnkEQFQOSpt1bNjtXhCtp7KT1AAYm6gQt3IZc=; fh=65XNDMZDtUAVPEs22K9ZMElrrCiP292uHQHDlBUdKcw=; b=SLESxk3+1YipOBj6pz4e6VPNi+rFWyPw4l22kUWqlZp29AS5ZnnB6sOIgfVqCMc9Q0 vnu7ff35nz81w2MaNZj/69qFv4HbNpTb8vYHwHwlKE9CdO5KLL1v3TDcq+CubAN87k2e o6+5UfCMy35aXMpsTE0MAhuwfKCaD/Mvad+HdI5VpZ0Ye4ShakkHMH0YOhOvDM755YI6 xxRRzmx8zYmc6e5eJDzkq4Isws+mP3Wh441+LGpe0Q/iQiehPWOoBh53lfcVNT3yNN1p WNaoRT9RTS07LZLAuCv7EukUKOoUJ27bbhiGS6pXSmKpWSSnD3H7yEyMgX+5yAR8y1V+ 2T2w== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b="T9A/lQ36"; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=hu7gPlAU; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=X1AdOXTZ; 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 Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id k73-20020a636f4c000000b0056c4d97e152si7629080pgc.68.2023.09.12.03.19.47 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Sep 2023 03:19:48 -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="T9A/lQ36"; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=hu7gPlAU; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=X1AdOXTZ; 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 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 1qg0UN-0004aj-IM; Tue, 12 Sep 2023 10:19:24 +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 1qg0UI-0004aH-3M for openvpn-devel@lists.sourceforge.net; Tue, 12 Sep 2023 10:19:19 +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:Date:From :Sender:To: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=nZMVA7G9JZ6MA7Kc4McHt3Gn1yPNgW4CaZLAFwvYYSc=; b=T9A/lQ36tb7w8g8pzNxlEqfw7S FslJuQc1CV+uz5jiINV2H8QMUSDGInCw49pRZY5i+qf1DYoY9t7L8FeYgJRIWOZvs/gLoYWJFFWJZ pNGEgv1821AB3eSFTbQ25FonECF9EJXh+2E2vp5v9b8vHr0pk8gfPuvwsq8hc0yElAEY=; 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:Date:From:Sender:To: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=nZMVA7G9JZ6MA7Kc4McHt3Gn1yPNgW4CaZLAFwvYYSc=; b=h u7gPlAUxFyjEZCyvSTg3PNIar2AeycWJdfPdELWitWIu5bL1lY1qjfxfJeDGoI+V78YOIqhjuI7Hk mwqy2hh6Dmb4RdglnHxQqFvR4i/CheNc8gU/UjMOM3m+b6UqgawKXfDLoeJsrSfCT077lH133GfEn xvSIwK8NvNzeQqcY=; Received: from mail-wr1-f49.google.com ([209.85.221.49]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1qg0UH-001VvY-1H for openvpn-devel@lists.sourceforge.net; Tue, 12 Sep 2023 10:19:19 +0000 Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-31c4d5bd69cso5590454f8f.3 for ; Tue, 12 Sep 2023 03:19:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1694513950; x=1695118750; 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:date:from:from:to:cc :subject:date:message-id:reply-to; bh=nZMVA7G9JZ6MA7Kc4McHt3Gn1yPNgW4CaZLAFwvYYSc=; b=X1AdOXTZpv8dYUcgN15OqOHaJxQ+DmoTZqttLyMhOpbf6JYYoOGZ6mSePXx4Clg5Gu qqAG8fwH1KJ4YaoG+dt0A3bKaAwA8i4jcm8YxXJoUnm4btuySVoayHZFWpwcuJLY3Vp8 oc/bAjlTKQ+nwGeLlfJlSIodl5zAMwY5Wt0b+uTDRLDp5Ja+GLXKaqZCjiAfQGngCp6a BUvFfEAwA3W/+0h3P3LxnjUCkYYt7ZrkJORPBTYpmO8XpNWE0leerLCZzVT/HD0jGtve ODbKixYT+V2yEusLmCk/Q/44+/3vs4YoXioUgeiCLUccxf6i+qnvbM/hSLMXiKIFEXuj Qvug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694513950; x=1695118750; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=nZMVA7G9JZ6MA7Kc4McHt3Gn1yPNgW4CaZLAFwvYYSc=; b=bsKGKT6+T08BleNCL51hUUAzBuHFbKXxH56UqgzYkelm+dzblPbmsPJFPW7gbdfwkq UbdkdUehnwBTVdU8sefL/7xvMdcYwg5NrsuLctg1pv9MxrjV3wS3UQwG4YCRGdvnge1u gh57hw5syDNYmIWGf8xGpqE5OLgF3knls08NeWMT+la0Cy52ZpXsIywP8fKm53mlW6na rwsGNKRWaEmyi4R4uwbCe81KYKggr8jJBqIxgAauisSqlelj7cBgpVW9ab7AWMwpiCkt M83vo05Jkftau7q2JoquISCq9iLmviqfiHbo7RxZqBAp/dkoaPwSHx2HKjYMv00WYWoS alVg== X-Gm-Message-State: AOJu0YxirD6L3l6Lb3DzCBty1Ieolsg3cvz5iqPy+jeKX8VrYxkAaoa2 Hf1cOFgfKmXj0B6jAEtcGkcuHZ7z9C/s74Y24es= X-Received: by 2002:a5d:6a03:0:b0:314:1e47:8bc2 with SMTP id m3-20020a5d6a03000000b003141e478bc2mr9736296wru.0.1694513950431; Tue, 12 Sep 2023 03:19:10 -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 z6-20020a7bc7c6000000b003fa96fe2bd9sm15672512wmk.22.2023.09.12.03.19.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Sep 2023 03:19:09 -0700 (PDT) From: "flichtenheld (Code Review)" X-Google-Original-From: "flichtenheld (Code Review)" X-Gerrit-PatchSet: 3 Date: Tue, 12 Sep 2023 10:19:09 +0000 Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: Id8321dfbb8ad8d79f4bb2a9da61f8cd6b6c6ee26 X-Gerrit-Change-Number: 268 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 1a808abac9932ac5aada88beecd6e96758c0dfdb References: Message-ID: <37155e5f782173d96d1de2117e4545024e3aaafb-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.0 X-Spam-Score: 1.0 (+) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.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: cron2, plaisthos. Hello cron2, I'd like you to do a code review. Please visit Content analysis details: (1.0 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.49 listed in wl.mailspike.net] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.221.49 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 1.2 MISSING_HEADERS Missing To: header 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_EF Message has a valid DKIM or DK signature from envelope-from domain -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.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1qg0UH-001VvY-1H Subject: [Openvpn-devel] [S] Change in openvpn[master]: Change type of frame.mss_fix to uint16_t 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: frank@lichtenheld.com, arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net Cc: plaisthos , openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1776826699220580211?= X-GMAIL-MSGID: =?utf-8?q?1776826699220580211?= Attention is currently required from: cron2, plaisthos. Hello cron2, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/268?usp=email to review the following change. Change subject: Change type of frame.mss_fix to uint16_t ...................................................................... Change type of frame.mss_fix to uint16_t Since in the end this always ends up as an uint16_t anyway, just make the conversion much earlier. Cleans up the code and removes some -Wconversion warnings. v2: - proper error handling in options.c Change-Id: Id8321dfbb8ad8d79f4bb2a9da61f8cd6b6c6ee26 Signed-off-by: Frank Lichtenheld --- M src/openvpn/mss.c M src/openvpn/mss.h M src/openvpn/mtu.c M src/openvpn/mtu.h M src/openvpn/options.c 5 files changed, 23 insertions(+), 15 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/68/268/3 diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c index dbd3681..d7ee4c2 100644 --- a/src/openvpn/mss.c +++ b/src/openvpn/mss.c @@ -44,7 +44,7 @@ * if yes, hand to mss_fixup_dowork() */ void -mss_fixup_ipv4(struct buffer *buf, int maxmss) +mss_fixup_ipv4(struct buffer *buf, uint16_t maxmss) { const struct openvpn_iphdr *pip; int hlen; @@ -72,7 +72,7 @@ struct openvpn_tcphdr *tc = (struct openvpn_tcphdr *) BPTR(&newbuf); if (tc->flags & OPENVPN_TCPH_SYN_MASK) { - mss_fixup_dowork(&newbuf, (uint16_t) maxmss); + mss_fixup_dowork(&newbuf, maxmss); } } } @@ -84,7 +84,7 @@ * (IPv6 header structure is sufficiently different from IPv4...) */ void -mss_fixup_ipv6(struct buffer *buf, int maxmss) +mss_fixup_ipv6(struct buffer *buf, uint16_t maxmss) { const struct openvpn_ipv6hdr *pip6; struct buffer newbuf; @@ -130,7 +130,7 @@ struct openvpn_tcphdr *tc = (struct openvpn_tcphdr *) BPTR(&newbuf); if (tc->flags & OPENVPN_TCPH_SYN_MASK) { - mss_fixup_dowork(&newbuf, (uint16_t) maxmss-20); + mss_fixup_dowork(&newbuf, maxmss-20); } } } @@ -191,13 +191,14 @@ { continue; } - mssval = (opt[2]<<8)+opt[3]; + mssval = opt[2] << 8; + mssval += opt[3]; if (mssval > maxmss) { - dmsg(D_MSS, "MSS: %d -> %d", (int) mssval, (int) maxmss); + dmsg(D_MSS, "MSS: %" PRIu16 " -> %" PRIu16, mssval, maxmss); accumulate = htons(mssval); - opt[2] = (maxmss>>8)&0xff; - opt[3] = maxmss&0xff; + opt[2] = (uint8_t)((maxmss>>8)&0xff); + opt[3] = (uint8_t)(maxmss&0xff); accumulate -= htons(maxmss); ADJUST_CHECKSUM(accumulate, tc->check); } @@ -291,7 +292,7 @@ { /* we subtract IPv4 and TCP overhead here, mssfix method will add the * extra 20 for IPv6 */ - frame->mss_fix = options->ce.mssfix - (20 + 20); + frame->mss_fix = (uint16_t)(options->ce.mssfix - (20 + 20)); return; } @@ -325,7 +326,7 @@ /* This is the target value our payload needs to be smaller */ unsigned int target = options->ce.mssfix - overhead; - frame->mss_fix = adjust_payload_max_cbc(kt, target) - payload_overhead; + frame->mss_fix = (uint16_t)(adjust_payload_max_cbc(kt, target) - payload_overhead); } diff --git a/src/openvpn/mss.h b/src/openvpn/mss.h index 1c4704b..b2a68cf 100644 --- a/src/openvpn/mss.h +++ b/src/openvpn/mss.h @@ -29,9 +29,9 @@ #include "mtu.h" #include "ssl_common.h" -void mss_fixup_ipv4(struct buffer *buf, int maxmss); +void mss_fixup_ipv4(struct buffer *buf, uint16_t maxmss); -void mss_fixup_ipv6(struct buffer *buf, int maxmss); +void mss_fixup_ipv6(struct buffer *buf, uint16_t maxmss); void mss_fixup_dowork(struct buffer *buf, uint16_t maxmss); diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c index 132f93c..389d140 100644 --- a/src/openvpn/mtu.c +++ b/src/openvpn/mtu.c @@ -210,7 +210,7 @@ buf_printf(&out, "%s ", prefix); } buf_printf(&out, "["); - buf_printf(&out, " mss_fix:%d", frame->mss_fix); + buf_printf(&out, " mss_fix:%" PRIu16, frame->mss_fix); #ifdef ENABLE_FRAGMENT buf_printf(&out, " max_frag:%d", frame->max_fragment_size); #endif diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h index b602b86..c64398d 100644 --- a/src/openvpn/mtu.h +++ b/src/openvpn/mtu.h @@ -115,7 +115,7 @@ * decryption/encryption or compression. */ } buf; - unsigned int mss_fix; /**< The actual MSS value that should be + uint16_t mss_fix; /**< The actual MSS value that should be * written to the payload packets. This * is the value for IPv4 TCP packets. For * IPv6 packets another 20 bytes must diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 90d85be..bdef703 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -7253,9 +7253,16 @@ VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); if (p[1]) { + int mssfix = positive_atoi(p[1]); + if (mssfix > UINT16_MAX) + { + msg(msglevel, "--mssfix value '%s' is invalid", p[1]); + goto err; + } + /* value specified, assume encapsulation is not * included unless "mtu" follows later */ - options->ce.mssfix = positive_atoi(p[1]); + options->ce.mssfix = mssfix; options->ce.mssfix_encap = false; options->ce.mssfix_default = false; }