From patchwork Wed Apr 17 16:52:38 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: 3685 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:57c6:b0:571:36e9:c9de with SMTP id v6csp141710mau; Wed, 17 Apr 2024 09:53:30 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCWIqUxXOsOgbYCRX2Tzz1iToxrS/7EsG/h3omyB46ovwme9T3ZPRbCqQ8RQxyJgsJv+onRjMOzTFwAYO62Dj4SboJqhrE4= X-Google-Smtp-Source: AGHT+IEZ/HKE3SQX27P7+sOpBuhTSCKjdbjUWZ65tLKHCXkfCYwvT5XKWPoFYHaKghmd391IAITP X-Received: by 2002:a17:902:728a:b0:1e0:99b2:8a91 with SMTP id d10-20020a170902728a00b001e099b28a91mr11645pll.4.1713372810347; Wed, 17 Apr 2024 09:53:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1713372810; cv=none; d=google.com; s=arc-20160816; b=kRWcz9VxmqugAZ1CaKn2KyZRLk5CzKkWQhpRgp1HbpZVG7rNRd/eAIebSgx098EKQM dWaH4OAQJBC5cx2BTSOhhnv6W4isFaaaz17gyWyiGoZaUWkvB9FFLcM9j9RLrJrXPuQe D3jZLXts3uuYv/x+I8B0x9WYJOCzYyxQJVJUnk3sjaOaLu09B9vrb8Ixr8bAgFbXuP6I vB68ZsfGBwJ65Z6ms0jU/gna1mDxPxKG3M4e7lMrUi73dp1jxKliIsZ6h05vuWp/yuut 44ACOnxItqDrciiNsx3AM7wce3xSlq9yHYUZsHUF4dRHVE/q01N/OyETvtx5/rauwBrA DuxA== 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=SmHFCsXxytlyUlUbYqZizrw5Qm0Yma9BMKAtyDGa6PA=; fh=GFP4qDxgyJ2WEPo/oeLZg3Mj4NqvY1j2nTvTt7psNwg=; b=GfpZ3Cm3tGwNxR4/k6PPN9MuU9RCULeed4A7SQRfV+XlW2KaEWy97dhCTGNN3VcUWg 5H2VidsjbBk6QvD1y/TRrupsCzEMR6ohyTOs6lTiqK64GvLCWt77fkWxih1iAfg+MOvK RjPsRE+cPnKmrwi57Gf4GuDAhbDF/NHoQ+6Ge2UuebiPP3I8dpByb+vP4AmIhuINPG3e PPP1IhNxYKbH0qQbTy9BFN3nASw6N6xjo0ZU0we/YjWfR/pJdFm/Z+qH1zrTnzU5dHUY ukuZ3EZzqRDzoYqObouACJqXPRzzc6NWGE5J9v311heIMx/nrSJhWvs7JY9mSAHai8Zz /y4w==; 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=CBazIhAY; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=eWXcZCNE; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b="JnEAN/tL"; 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 e4-20020a170902ef4400b001e49bce914asi12130230plx.429.2024.04.17.09.53.30 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 17 Apr 2024 09:53:30 -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=CBazIhAY; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=eWXcZCNE; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b="JnEAN/tL"; 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-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1rx8Wk-0001bv-CJ; Wed, 17 Apr 2024 16:52:54 +0000 Received: from [172.30.20.202] (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 1rx8Wi-0001bn-Sa for openvpn-devel@lists.sourceforge.net; Wed, 17 Apr 2024 16:52:53 +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=DhEvxcjbQMaPWJwoYuOj2XJEQRZjM7+lpwxWSo5g9XY=; b=CBazIhAYhrSHHuUPHqCZ0UPWUY WEBKIXRF98AXyVJO2NC5ReyQSuUBq3jWlljIJSAs/q7O1+WvLFF5L4T0fTyItDUR9t2wwV1/LO4pg 66BJlxFyIIb7FLA/1xeqWfuCTYUdEucFqcKtfGI6PHvZh+8Nbr1CgKdZLuncRESDhc70=; 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=DhEvxcjbQMaPWJwoYuOj2XJEQRZjM7+lpwxWSo5g9XY=; b=e WXcZCNEK8z+LUgVr7+vyp4XILDjaQYSx0X+ldhfVyN8R3n10N4LL7i8nRMs2XosG0G4s0yYb0qr4n Zgh5qj0qb/woOtNb9D86+evAGtSIJtuGwMC55NiWHKqjJ3v13YU3n7Sg/UhrVobUyAVq02crxcxJ1 V7ZsCzTd6AA6ZEBE=; Received: from mail-lj1-f177.google.com ([209.85.208.177]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1rx8Wh-0004ks-Q4 for openvpn-devel@lists.sourceforge.net; Wed, 17 Apr 2024 16:52:53 +0000 Received: by mail-lj1-f177.google.com with SMTP id 38308e7fff4ca-2d895138ce6so70238411fa.0 for ; Wed, 17 Apr 2024 09:52:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1713372760; x=1713977560; 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=DhEvxcjbQMaPWJwoYuOj2XJEQRZjM7+lpwxWSo5g9XY=; b=JnEAN/tLjDFkarY9AVxWaOJQPD22nsE009FtA90VnHgNADBI8KvEwMVVolg1hGvEHg g8UWezX1UIGX+qB9LjhVYk1cp+y6XQHtsPtrl5a44CJiekdse75F/W/7d+m4yHbZQYb0 YCJwMDZrrmur15Es1p3z2oDC5JpJ1vuvIMDN7Db1BvU0L2yGDcpDWlMRjPttLE/Jx+/T waCb/r7ndrhja+DJpbFgjpi0WRocWxMfTWKrk2BpobNbX2wnhv7hGep+PCY8BW5LDKhQ h7wE5G8wUn+mdeOo68ciqaosW8TYUda5v1NsMinTlBNdjnbVjliaLJBUQdN0d7WMzh58 6xJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713372760; x=1713977560; 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=DhEvxcjbQMaPWJwoYuOj2XJEQRZjM7+lpwxWSo5g9XY=; b=V/65e8WYAjUMhNUQqSYbwbIe2bbmmXMda93/6diGcP2ftVegO7loDiOwNf4sd+Y48e 7Wm3pf0rDQR1c//cwYD+ev5JhNcsxrMWHlDb9TYTbkKJflEHZkYxSmXzdADmb+WjKkK5 RFWkeKS678ZPPZ8JTGfyfqVtPpCGsDDXuN5tVkNoAwdIx/kPHehLwm/yuLgm21oITbby MzzqHnKo+DfwIdfpDLHiPpbb9ZSyZpQknhi9DmYZ2V+464vjggorHUFz6XhRyhiBtMB4 jz8L1hsnP8JXog+QpO0y7bvDym0Fl+8ShSqco3ecRn42KZ55ttFGPy9cYSZVXVy6gwB+ GyvA== X-Gm-Message-State: AOJu0YwRCgm4ADcZVfXs73kR8w/zS2caYtaFYuA1lZapDEfrIRVxwbYr g/5D5jfx7mz+FUsmgkosh907Miug+BToQKMiGpk0lysUfWdp+dnx/vciO/doQACayhPgAbTUtBO 7 X-Received: by 2002:a05:651c:11:b0:2d9:fb50:5292 with SMTP id n17-20020a05651c001100b002d9fb505292mr10774578lja.18.1713372759499; Wed, 17 Apr 2024 09:52:39 -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 t13-20020a05600c198d00b00417da22df18sm3412951wmq.9.2024.04.17.09.52.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Apr 2024 09:52:39 -0700 (PDT) From: "flichtenheld (Code Review)" X-Google-Original-From: "flichtenheld (Code Review)" X-Gerrit-PatchSet: 1 Date: Wed, 17 Apr 2024 16:52:38 +0000 To: plaisthos Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: Iefd209c0856ef395ab74055496130de00b86ead0 X-Gerrit-Change-Number: 554 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 7abb1cfdf65a8daf95a541a30b30fce171d7b4fa References: Message-ID: MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.2 (/) 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: plaisthos. Hello plaisthos, I'd like you to do a code review. Please visit 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_MSPIKE_H2 RBL: Average reputation (+2) [209.85.208.177 listed in wl.mailspike.net] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches 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 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.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -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: 1rx8Wh-0004ks-Q4 Subject: [Openvpn-devel] [M] Change in openvpn[master]: Use topology default of "subnet" only for server mode 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: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1796601608366821851?= X-GMAIL-MSGID: =?utf-8?q?1796601608366821851?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: plaisthos. Hello plaisthos, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/554?usp=email to review the following change. Change subject: Use topology default of "subnet" only for server mode ...................................................................... Use topology default of "subnet" only for server mode The setting of --topology changes the syntax of --ifconfig. So changing the default of --topology breaks all existing configs that use --ifconfig but not --topology. For P2P setups that is probably a signification percentage. For server setups the percentage is hopefully lower since --ifconfig is implicitly set by --server. Also more people might have set their topology explicitly since it makes a much bigger difference. Clients will usually get the topology and the IP config pushed by the server. So we decided to not switch the default for everyone to not affect P2P setups. What we care about is to change the default for --mode server, so we only do that now. For people using --server this should be transparent except for a pool reset. Change-Id: Iefd209c0856ef395ab74055496130de00b86ead0 Signed-off-by: Frank Lichtenheld --- M Changes.rst M src/openvpn/helper.c M src/openvpn/helper.h M src/openvpn/options.c 4 files changed, 49 insertions(+), 16 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/54/554/1 diff --git a/Changes.rst b/Changes.rst index b2278ab..fa0fb22 100644 --- a/Changes.rst +++ b/Changes.rst @@ -23,11 +23,12 @@ ``persist-key`` option has been enabled by default. All the keys will be kept in memory across restart. -Default for ``--topology`` changed to ``subnet`` - Previous releases used ``net30`` as default. This only affects - configs with ``--dev tun`` and only IPv4. Note that this - changes the semantics of ``--ifconfig``, so if you have manual - settings for that in your config but not set ``--topology`` +Default for ``--topology`` changed to ``subnet`` for ``--mode server`` + Previous releases always used ``net30`` as default. This only affects + configs with ``--mode server`` or ``--server`` (the latter implies the + former), and ``--dev tun``, and only if IPv4 is enabled. + Note that this changes the semantics of ``--ifconfig``, so if you have + manual settings for that in your config but not set ``--topology`` your config might fail to parse with the new version. Just adding ``--topology net30`` to the config should fix the problem. By default ``--topology`` is pushed from server to client. diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c index 1bab84c..5681718 100644 --- a/src/openvpn/helper.c +++ b/src/openvpn/helper.c @@ -137,6 +137,32 @@ } +/** + * Set --topology default depending on --mode + */ +void +helper_setdefault_topology(struct options *o) +{ + if (o->topology != TOP_UNDEF) + { + return; + } + int dev = dev_type_enum(o->dev, o->dev_type); + if (dev != DEV_TYPE_TUN) + { + return; + } + if (o->mode == MODE_SERVER) + { + o->topology = TOP_SUBNET; + } + else + { + o->topology = TOP_NET30; + } +} + + /* * Process server, server-bridge, and client helper * directives after the parameters themselves have been @@ -151,7 +177,6 @@ * Get tun/tap/null device type */ const int dev = dev_type_enum(o->dev, o->dev_type); - const int topology = o->topology; /* * @@ -177,11 +202,11 @@ if (o->server_flags & SF_NOPOOL) { - msg( M_USAGE, "--server-ipv6 is incompatible with 'nopool' option" ); + msg(M_USAGE, "--server-ipv6 is incompatible with 'nopool' option"); } if (o->ifconfig_ipv6_pool_defined) { - msg( M_USAGE, "--server-ipv6 already defines an ifconfig-ipv6-pool, so you can't also specify --ifconfig-pool explicitly"); + msg(M_USAGE, "--server-ipv6 already defines an ifconfig-ipv6-pool, so you can't also specify --ifconfig-pool explicitly"); } o->mode = MODE_SERVER; @@ -207,7 +232,7 @@ o->server_netbits_ipv6 < 112 ? 0x1000 : 2); o->ifconfig_ipv6_pool_netbits = o->server_netbits_ipv6; - push_option( o, "tun-ipv6", M_USAGE ); + push_option(o, "tun-ipv6", M_USAGE); } /* @@ -305,8 +330,10 @@ o->mode = MODE_SERVER; o->tls_server = true; + /* Need to know topology now */ + helper_setdefault_topology(o); - if (topology == TOP_NET30 || topology == TOP_P2P) + if (o->topology == TOP_NET30 || o->topology == TOP_P2P) { o->ifconfig_local = print_in_addr_t(o->server_network + 1, 0, &o->gc); o->ifconfig_remote_netmask = print_in_addr_t(o->server_network + 2, 0, &o->gc); @@ -324,12 +351,12 @@ { push_option(o, print_opt_route(o->server_network, o->server_netmask, &o->gc), M_USAGE); } - else if (topology == TOP_NET30) + else if (o->topology == TOP_NET30) { push_option(o, print_opt_route(o->server_network + 1, 0, &o->gc), M_USAGE); } } - else if (topology == TOP_SUBNET) + else if (o->topology == TOP_SUBNET) { o->ifconfig_local = print_in_addr_t(o->server_network + 1, 0, &o->gc); o->ifconfig_remote_netmask = print_in_addr_t(o->server_netmask, 0, &o->gc); @@ -354,9 +381,9 @@ ASSERT(0); } - push_option(o, print_opt_topology(topology, &o->gc), M_USAGE); + push_option(o, print_opt_topology(o->topology, &o->gc), M_USAGE); - if (topology == TOP_NET30 && !(o->server_flags & SF_NOPOOL)) + if (o->topology == TOP_NET30 && !(o->server_flags & SF_NOPOOL)) { msg(M_WARN, "WARNING: --topology net30 support for server " "configs with IPv4 pools will be removed in a future " @@ -394,7 +421,7 @@ } /* set push-ifconfig-constraint directive */ - if ((dev == DEV_TYPE_TAP || topology == TOP_SUBNET)) + if ((dev == DEV_TYPE_TAP || o->topology == TOP_SUBNET)) { o->push_ifconfig_constraint_defined = true; o->push_ifconfig_constraint_network = o->server_network; diff --git a/src/openvpn/helper.h b/src/openvpn/helper.h index d0fd17d..6b42e13 100644 --- a/src/openvpn/helper.h +++ b/src/openvpn/helper.h @@ -30,6 +30,8 @@ #include "options.h" +void helper_setdefault_topology(struct options *o); + void helper_keepalive(struct options *o); void helper_client_server(struct options *o); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index e2bfe0e..07387cd 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -796,7 +796,7 @@ o->gc_owned = true; } o->mode = MODE_POINT_TO_POINT; - o->topology = TOP_SUBNET; + o->topology = TOP_UNDEF; o->ce.proto = PROTO_UDP; o->ce.af = AF_UNSPEC; o->ce.bind_ipv6_only = false; @@ -3478,6 +3478,7 @@ } } + /** * Checks for availibility of Chacha20-Poly1305 and sets * the ncp_cipher to either AES-256-GCM:AES-128-GCM or @@ -3680,6 +3681,8 @@ * sequences of options. */ helper_client_server(o); + /* must be called after helpers that might set --mode */ + helper_setdefault_topology(o); helper_keepalive(o); helper_tcp_nodelay(o);