openvpn-devel Mailing List for OpenVPN

On Mon, 2020-06-29 at 19:51 +0200, Gert Doering wrote: > The rules to generate $(builddir)/openssl.cnf from > $(srcdir)/openssl.cnf.in only worked for GNU Make. BSD make needs > the rules more explicit, and the target must not have a directory > specification (fixes commit 542c69c37). This definitely works for me on Linux. James 

hi, On 30/06/20 16:11, Gert Doering wrote: > Hi, > > On Tue, Jun 30, 2020 at 04:07:52PM +0200, Jan Just Keijser wrote: >> @@ -5697,6 +5740,11 @@ build_dhcp_options_string(struct buffer *buf, const struct tuntap_options *o) >> write_dhcp_u32_array(buf, 42, (uint32_t *)o->ntp, o->ntp_len, &error); >> write_dhcp_u32_array(buf, 45, (uint32_t *)o->nbdd, o->nbdd_len, &error); >> >> + for (int i=0; i < o->domain_search_list_len; i++) >> + { >> + write_dhcp_search_str(buf, 119, o->domain_search_list[i], &error); >> + } > I assume that this won't work. In the DHCP answer, it must be a single > option with the concatenated list, not multiple answers with a single > domain name each. > > gert > see https://tools.ietf.org/html/rfc3397 " In the above diagram, Searchstring is a string specifying the searchlist. If the length of the searchlist exceeds the maximum permissible within a single option (255 octets), then multiple options MAY be used, as described in "Encoding Long Options in the Dynamic Host Configuration Protocol (DHCPv4)" [RFC3396 <https://tools.ietf.org/html/rfc3396>]. " so you MAY use this option multiple times - and wireshark groks it fine. I don't have a Windows 10 client to test it against, however, so I am hoping that someone (Selva?) can do that for me. cheers, JJK 

Hi, On Tue, Jun 30, 2020 at 04:07:52PM +0200, Jan Just Keijser wrote: > @@ -5697,6 +5740,11 @@ build_dhcp_options_string(struct buffer *buf, const struct tuntap_options *o) > write_dhcp_u32_array(buf, 42, (uint32_t *)o->ntp, o->ntp_len, &error); > write_dhcp_u32_array(buf, 45, (uint32_t *)o->nbdd, o->nbdd_len, &error); > > + for (int i=0; i < o->domain_search_list_len; i++) > + { > + write_dhcp_search_str(buf, 119, o->domain_search_list[i], &error); > + } I assume that this won't work. In the DHCP answer, it must be a single option with the concatenated list, not multiple answers with a single domain name each. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... 

Hi, On 24/06/20 12:28, Gert Doering wrote: > Hi, > > On Tue, Jun 23, 2020 at 03:53:52PM -0400, Selva Nair wrote: >>> So what option do we want? >>> >>> --dhcp-option SEARCH >>> --dhcp-option DOMAIN-SEARCH >>> --dhcp-option SEARCH-DOMAIN >> RFC 3397 calls it "Domain Search" so it has to be DOMAIN-SEARCH, in my >> view. Platform scripts accepting other forms in foreign_option is up >> to them. We don't have to officially support that. > I like that argument. > > (I do not care too much which string it is, in the end, but if we have > an RFC which has a name for it, and that name maps directly to one of > the candidates, this is a strong argument :-) ) > > > On the "shall it be a single occurrance with multiple domains in it" or > "shall it be multiple occurances that are concatenated into a single DHCP > option which then has multiple domains in it", I do not have a truly > strong opinion. So I'd go with "what Tunnelblick has", which is > "multiple occurances, a single string each". > > He who goes first wins :-) here's V2: -  allow a user to specify   dhcp-option DOMAIN-SEARCH  multiple times - only a single FQDN per entry cheers, JJK 

that makes sense. I updated the patch. Christopher --- src/openvpn/tun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 5567c445..2a2df27f 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -5525,7 +5525,7 @@ windows_set_mtu(const int iface_index, const short family, } else { - msg(M_INFO, "Successfully set %s mtu on interface %d", family_name, iface_index); + msg(M_INFO, "%s MTU set to %d on interface %d using SetIpInterfaceEntry()", family_name, mtu, iface_index); } } -- 2.21.0.windows.1 

On 22-06-2020 19:59, David Sommerseth wrote: > On 22/06/2020 14:43, Steffan Karger wrote: >> On 22-06-2020 14:29, David Sommerseth wrote: >>> On 22/06/2020 14:21, Arne Schwabe wrote: >>>> >>>>> PrivateTmp=true >>>>> WorkingDirectory=/etc/openvpn/server >>>>> -ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf >>>>> +ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --cipher AES-256-GCM --ncp-ciphers AES-256-GCM:AES-128-GCM:AES-256-CBC:AES-128-CBC:BF-CBC --config %i.conf >>>>> CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE CAP_AUDIT_WRITE >>>>> LimitNPROC=10 >>>>> DeviceAllow=/dev/null rw >>>>> >>>> >>>> NACK. >>>> >>>> Setting ncp-cipher to include BF-CBC by default allows BF-CBC in configs >>>> that did not allow it before. Basically any config that had something >>>> other than cipher BF-CBC and no ncp-ciphers in it will now allow clients >>>> with BF-CBC to connect. I don't want force users to set ncp-cipher to a >>>> sane value since the systemd unit file doesn't. >>> >>> That will break existing configs on the next upgrade. Do we want do do that? >>> >>> I'm fine with removing BF-CBC, but it is scheduled for removal in OpenVPN 2.6. >> >> I think Arne has a very good point that it's kinda weird to "degrade" >> the NCP defaults. >> >> Making AES-256-GCM the default cipher for TLS-based connections (GCM >> won't work with static key configs) does not imply *removing* BF-CBC >> support. Maybe these should be the steps: >> >> 2.4: Use to AES-256-GCM when available (basically what NCP did) >> 2.5: Switch to AES-256-GCM as the default cipher (but allow overriding) >> 2.6: Remove support for small block ciphers all together >> >> Yes, this will probably break some (less secure) setups and make some >> people angry. But at some point people need to move on. We've been >> throwing warnings at them for a while now. > > Yes, I agree that Arne got a point. But I'm not completely convinced breaking > configs in OpenVPN 2.5 without a prior note that it will stop working. Our > warning only screams "you shouldn't use ciphers with block sizes < 128 bits", > it doesn't say "this will stop working in a future release". > > OpenVPN 2.4.9 gives this warning: > > WARNING: INSECURE cipher with block size less than 128 bit (64 bit). This > allows attacks like SWEET32. Mitigate by using a --cipher with a larger > block size (e.g. AES-256-CBC). > > The community has been informed it will stop working in 2.6, not before this > release. > > This must be documented properly and log warnings updated to indicate > short-term workarounds. > > I could be willing to consider breaking configs for ciphers in a later 2.5.x > release as long as users are properly warned *when* it will stop working - and > that users gets a real chance to fix configs before we do break their configs > - where a workaround approach could be considered and available from v2.5.0 > (on the other hand, if they change their configs, they should swap ciphers > instead of applying a workaround for a dying cipher - but for some it might be > a bit more complicated to do such a swap). > > We need to find a good middle-way for OpenVPN 2.5, where we clearly signal > "weak ciphers will be removed" and when we will do that. While also moving > forward and break as few configs as possible and not make configs weaker than > before. I agree the warning we log should be made even more scary. The DeprecatedOptions wiki however already says: Removal of insecure ciphers: Ciphers with cipher block-size less than 128 bits (most commonly BF, DES, CAST5, IDEA and RC2) Deprecated in: OpenVPN v2.4 To be removed in: OpenVPN v2.6 What I'm proposing is a step in between for something that is long overdue: update the *defaults* to no longer use insecure ciphers. But to cater for those that are unprepared, allow them to explicitly re-enable BF-CBC during a transition period. They've been ignoring our advice to migrate for a long time now, and even the wiki page says that these ciphers are already deprecated in 2.4. -Steffan 

Hi, On 22-06-2020 16:01, Arne Schwabe wrote: > Am 22.06.20 um 14:43 schrieb Steffan Karger: >> Maybe these should be the steps: >> >> 2.4: Use to AES-256-GCM when available (basically what NCP did) >> 2.5: Switch to AES-256-GCM as the default cipher (but allow overriding) >> 2.6: Remove support for small block ciphers all together >> >> Yes, this will probably break some (less secure) setups and make some >> people angry. But at some point people need to move on. We've been >> throwing warnings at them for a while now. > > I had a different suggestion in the channel: > > - Deprecate ncp-disable. Reason: Was a good debug switch when introduced > should not be necessary anymore. Deprecate in 2.5, remove in 2.6 makes sense to me. Should we do the same for --cipher in P2MP configs? > - Introduce ncp-fallback-cipher for compatibility with ncp-disable/old > versions > - needs to be a cipher from ncp-ciphers list > - Eventually default the first cipher from ncp-ciphers list > - in 2.5 default to --cipher if --cipher is set and automatically > add cipher to ncp-ciphers and set ncp-fallback-cipher. > > - If cipher is not set >	a) warn about that cipher will be ignored in p2mp mode and if BF-CBC is > still needed (e.g. peer 2.3 or older) that ncp-fallback-cipher should be set > b) same a) but do it automatically for the user+warn > > This allows us to eventually get rid of --cipher while providing still a > smooth transaction. That does sounds more friendly than my proposal. But do we really need an extra option? Does is really differ so much from changing the default cipher to AES-256-CBC? Both proposals require config changes to keep old clients working with 2.5+ if --cipher wasn't set explicitly. We already have a lot of things in place to cater for a smooth transition: - 2.4+ does NCP by default. - Any 2.4+ client always sends OCC strings, which makes "poor man's NCP" work for --ncp-disable clients as long as their cipher is in the server's --ncp-ciphers list. - pre-2.4 clients almost always send OCC strings too, except for ENABLE_SMALL builds. - If NCP and poor-man's NCP fail, 2.5 can (if --cipher is explicitly set) fall back to whatever is in --cipher and print a big fat warning that this connection will break in 2.6. - If someone *really* wants to have 2.3 ENABLE_SMALL clients connect to a 2.6 server, they can run a separate server for those clients with only the cipher of their choice in --ncp-ciphers ("Fall back to the first cipher", as you proposed). Basically: if we can agree to no longer guarantee interoperability between 2.6+ and 2.3- ("mostly just works, but special cases - in particular ENABLE_SMALL builds - might break"), I don't think we need to add yet another option. -Steffan 

Acked-by: Gert Doering <ge...@gr...> Stared-at-code, looks good. Test build on ubuntu/mingw, passes. I have not actually tested that it prints the warnings when expected, but the code looks good and the change is simple enough :-) Your patch has been applied to the master branch. commit 93439307e597007e0d60b904c0f3d9d85de26b49 Author: Christopher Schenk Date: Mon Jun 29 21:09:30 2020 +0200 Log a note if someone wants to set a MTU below 1280 on IPv6 Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@ma...> URL: https://www.mail-archive.com/ope...@li.../msg20161.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Hi, On Tue, Jun 30, 2020 at 12:08:39AM +0100, Richard Bonhomme wrote: > Signed-off-by: Richard Bonhomme <tin...@gm...> > --- > doc/man-sections/advanced-options.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/doc/man-sections/advanced-options.rst b/doc/man-sections/advanced-options.rst > index dbf7799c..9b96e406 100644 > --- a/doc/man-sections/advanced-options.rst > +++ b/doc/man-sections/advanced-options.rst > @@ -104,4 +104,4 @@ used when debugging or testing out special usage scenarios. > --txqueuelen n > *(Linux only)* Set the TX queue length on the TUN/TAP interface. > - Currently defaults to 100. > + Currently defaults to operating system default. Acked-by: ge...@gr... David, can you please merge into your tree? gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... 

Signed-off-by: Richard Bonhomme <tin...@gm...> --- doc/man-sections/advanced-options.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/man-sections/advanced-options.rst b/doc/man-sections/advanced-options.rst index dbf7799c..9b96e406 100644 --- a/doc/man-sections/advanced-options.rst +++ b/doc/man-sections/advanced-options.rst @@ -104,4 +104,4 @@ used when debugging or testing out special usage scenarios. --txqueuelen n *(Linux only)* Set the TX queue length on the TUN/TAP interface. - Currently defaults to 100. + Currently defaults to operating system default. -- 2.17.1 

Hi, On Mon, Jun 29, 2020 at 09:09:29PM +0200, Christopher Schenk wrote: > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index 18cdf38d..3ef79b2f 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -251,7 +251,7 @@ do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu) > } > else > { > - msg(M_INFO, "%s MTU set to %d on interface %d using service", family_name, mtu, mtu_msg.iface.index); > + msg(M_INFO, "%s MTU set to %d on interface %d using SetIpInterfaceEntry()", family_name, mtu, mtu_msg.iface.index); > ret = true; > } While I do like unified messages, this is not exactly what I had in mind - the "using service" bit should stay, if using the service :-) - too much unification. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... 

Patch has been applied to the master branch. (I claim to have tested this before sending to the list :-) ) commit 3ef858b3d63c61be2f473a8dc5f1f79fa09a85d8 Author: Gert Doering Date: Mon Jun 29 20:04:05 2020 +0200 Linux: do not change --txqueuelen OS default if not configured. Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Arne Schwabe <ar...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg20160.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Am 29.06.20 um 20:04 schrieb Gert Doering: > Remove default setting of "set txqueuelen to 100". This default dates > back to the "pre git" times (before 2005) and might have been beneficial > back then - nowadays, the Linux default is 500, and thus reducing(!) > txqueuelen by-default can cause TX packet drops on the tun interface, > and that's bad for throughput. > > This is a similar change to commit f0b64e5dc (remove setting of the > socket send/receive buffers by default) - similar vintage of the > existing code, similar motivation. > > Note: buffer length can be checked with "ip link show" (qlen NNN) > Acked-By: Arne Schwabe <ar...@rf...> 

--- src/openvpn/tun.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 3ef79b2f..67d7664e 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -238,6 +238,10 @@ do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu) .family = family }; strncpynt(mtu_msg.iface.name, tt->actual_name, sizeof(mtu_msg.iface.name)); + if (family == AF_INET6 && mtu < 1280) + { + msg(M_INFO, "NOTE: IPv6 interface MTU < 1280 conflicts with IETF standards and might not work"); + } if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack, "Set_mtu")) { @@ -5498,6 +5502,11 @@ windows_set_mtu(const int iface_index, const short family, const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4"; ipiface.Family = family; ipiface.InterfaceIndex = iface_index; + if (family == AF_INET6 && mtu < 1280) + { + msg(M_INFO, "NOTE: IPv6 interface MTU < 1280 conflicts with IETF standards and might not work"); + } + err = GetIpInterfaceEntry(&ipiface); if (err == NO_ERROR) { -- 2.21.0.windows.1 

--- src/openvpn/tun.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 18cdf38d..3ef79b2f 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -251,7 +251,7 @@ do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu) } else { - msg(M_INFO, "%s MTU set to %d on interface %d using service", family_name, mtu, mtu_msg.iface.index); + msg(M_INFO, "%s MTU set to %d on interface %d using SetIpInterfaceEntry()", family_name, mtu, mtu_msg.iface.index); ret = true; } @@ -5516,7 +5516,7 @@ windows_set_mtu(const int iface_index, const short family, } else { - msg(M_INFO, "Successfully set %s mtu on interface %d", family_name, iface_index); + msg(M_INFO, "%s MTU set to %d on interface %d using SetIpInterfaceEntry()", family_name, mtu, iface_index); } } -- 2.21.0.windows.1 

Remove default setting of "set txqueuelen to 100". This default dates back to the "pre git" times (before 2005) and might have been beneficial back then - nowadays, the Linux default is 500, and thus reducing(!) txqueuelen by-default can cause TX packet drops on the tun interface, and that's bad for throughput. This is a similar change to commit f0b64e5dc (remove setting of the socket send/receive buffers by default) - similar vintage of the existing code, similar motivation. Note: buffer length can be checked with "ip link show" (qlen NNN) See also: https://ivanvari.com/solving-openvpn-poor-throughput-and-packet-loss/ Signed-off-by: Gert Doering <ge...@gr...> --- src/openvpn/options.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index c21ef56d..a40efd68 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -840,9 +840,6 @@ init_options(struct options *o, const bool init_gc) #ifdef ENABLE_FEATURE_TUN_PERSIST o->persist_mode = 1; #endif -#ifdef TARGET_LINUX - o->tuntap_options.txqueuelen = 100; -#endif #ifdef _WIN32 #if 0 o->tuntap_options.ip_win32_type = IPW32_SET_ADAPTIVE; -- 2.26.2 

The rules to generate $(builddir)/openssl.cnf from $(srcdir)/openssl.cnf.in only worked for GNU Make. BSD make needs the rules more explicit, and the target must not have a directory specification (fixes commit 542c69c37). Signed-off-by: Gert Doering <ge...@gr...> --- tests/unit_tests/engine-key/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/engine-key/Makefile.am b/tests/unit_tests/engine-key/Makefile.am index 0bfdfcd4..24622251 100644 --- a/tests/unit_tests/engine-key/Makefile.am +++ b/tests/unit_tests/engine-key/Makefile.am @@ -21,8 +21,8 @@ CLEANFILES = \	log.txt \	$(conffiles) -$(builddir)/openssl.cnf: $(srcdir)/openssl.cnf.in -	sed "s|ABSBUILDDIR|$(abs_builddir)|" < $< > $@ +openssl.cnf: $(srcdir)/openssl.cnf.in +	sed "s|ABSBUILDDIR|$(abs_builddir)|" < $(srcdir)/openssl.cnf.in > $@ libtestengine_la_SOURCES = libtestengine.c libtestengine_la_LDFLAGS = @TEST_LDFLAGS@ -rpath /lib -shrext .so -- 2.27.0 

Acked-by: Gert Doering <ge...@gr...> Your patch has been applied to the master branch. I have stared at the code (looks reasonable) and run t_client tests on Linux and FreeBSD (pass, no major surprise). I have not actually tested the functionality, because I do not have a test rig with VRF (or multiple ethernet links) around. From what I found so far, what SO_BINDTODEVICE does is "ensure that packets sent via that socket go out via the specified interface, and only that interface" and "packets coming in have been received on that interface". Besides "a network interface" you can also specify the name of a "VRF device" (which seems to be a Linux abstraction to group together "Interfaces and Routes" under one umbrella). So you can use this for VRFs ("--bind-dev outer-vrf") or to select one particular interface (*and* routes) inside "global" or "VRF". I have tested the latter ("--bind-to lo") which makes connection setup *fail* - obviously. Trying to bind to nonexistant devices gives a proper error message in v2 now 2020-06-29 13:27:03 WARN: setsockopt SO_BINDTODEVICE=MyTunnelIsLongerThanYours failed: No such device (errno=19) Googling for SO_BINDTODEVICE shows lots of confusion on how this is used and what it does, but I consider the kernel documentation to be authoritative (Documentation/networking/vrf.txt) and that says "what you do is correct": Applications ------------ Applications that are to work within a VRF need to bind their socket to the VRF device: setsockopt(sd, SOL_SOCKET, SO_BINDTODEVICE, dev, strlen(dev)+1); (note the "strlen(dev)+1" part, and "not a struct ifreq") Your patch has been applied to the master branch. commit 19d3c602e7a3881cf7c2244b7c40b9958c0b7ebc Author: Maximilian Wilhelm Date: Mon Jun 29 12:49:07 2020 +0200 Add --bind-dev option. Signed-off-by: Maximilian Wilhelm <ma...@sd...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <159...@rf...> URL: https://www.mail-archive.com/search?l=mid&q=1...@rf... Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

From: Maximilian Wilhelm <ma...@sd...> This options allows the user to specify a network device the OpenVPN process should use when making a connection or binding to an address. This translates in setting the SO_BINDTODEVICE option to the corresponding socket (on Linux). When for example using VRFs on Linux [0] this allows making connections using the non-default VRF and having the tun/tap interface in the default VRF. Thanks to David Ahern (Cumulus Networks) for insights on this. [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/vrf.txt Signed-off-by: Maximilian Wilhelm <ma...@sd...> --- src/openvpn/init.c | 1 + src/openvpn/options.c | 10 ++++++++++ src/openvpn/options.h | 1 + src/openvpn/socket.c | 14 ++++++++++++++ src/openvpn/socket.h | 2 ++ 5 files changed, 28 insertions(+) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 2af25c1..dd1747f 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -3445,6 +3445,7 @@ do_init_socket_1(struct context *c, const int mode) c->options.rcvbuf, c->options.sndbuf, c->options.mark, + c->options.bind_dev, &c->c2.server_poll_interval, sockflags); } diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 9580d1d..34e2bfd 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -294,6 +294,9 @@ static const char usage_message[] = #if defined(TARGET_LINUX) && HAVE_DECL_SO_MARK "--mark value : Mark encrypted packets being sent with value. The mark value\n" " can be matched in policy routing and packetfilter rules.\n" + "--bind-dev dev : Bind to the given device when making connection to a peer or\n" + " listening for connections. This allows sending encrypted packets\n" + " via a VRF present on the system.\n" #endif "--txqueuelen n : Set the tun/tap TX queue length to n (Linux only).\n" #ifdef ENABLE_MEMSTATS @@ -6084,6 +6087,13 @@ add_option(struct options *options, } } } +#ifdef TARGET_LINUX + else if (streq (p[0], "bind-dev") && p[1]) + { + VERIFY_PERMISSION (OPT_P_SOCKFLAGS); + options->bind_dev = p[1]; + } +#endif else if (streq(p[0], "txqueuelen") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 375a4fc..c83a46a 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -352,6 +352,7 @@ struct options /* mark value */ int mark; + char *bind_dev; /* socket flags */ unsigned int sockflags; diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index f2fb54d..61463bc 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -1138,6 +1138,18 @@ create_socket(struct link_socket *sock, struct addrinfo *addr) /* set socket to --mark packets with given value */ socket_set_mark(sock->sd, sock->mark); +#if defined(TARGET_LINUX) + if (sock->bind_dev) + { + msg (M_INFO, "Using bind-dev %s", sock->bind_dev); + if (setsockopt (sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, strlen (sock->bind_dev) + 1) != 0) + { + msg(M_WARN|M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s failed", sock->bind_dev); + } + + } +#endif + bind_local(sock, addr->ai_family); } @@ -1880,6 +1892,7 @@ link_socket_init_phase1(struct link_socket *sock, int rcvbuf, int sndbuf, int mark, + const char *bind_dev, struct event_timeout *server_poll_timeout, unsigned int sockflags) { @@ -1906,6 +1919,7 @@ link_socket_init_phase1(struct link_socket *sock, sock->sockflags = sockflags; sock->mark = mark; + sock->bind_dev = bind_dev; sock->info.proto = proto; sock->info.af = af; diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h index 38e5138..8db9e8b 100644 --- a/src/openvpn/socket.h +++ b/src/openvpn/socket.h @@ -212,6 +212,7 @@ struct link_socket #define SF_GETADDRINFO_DGRAM (1<<4) unsigned int sockflags; int mark; + const char *bind_dev; /* for stream sockets */ struct stream_buf stream_buf; @@ -326,6 +327,7 @@ link_socket_init_phase1(struct link_socket *sock, int rcvbuf, int sndbuf, int mark, + const char *bind_dev, struct event_timeout *server_poll_timeout, unsigned int sockflags); /* Reenable uncrustify *INDENT-ON* */ -- 2.1.4 

Hi, once again, this time with added error handling in the setsockopt(). Sorry for missing this before. Best Max 

From: Maximilian Wilhelm <ma...@sd...> Signed-off-by: Maximilian Wilhelm <ma...@sd...> --- doc/man-sections/vrf.rst | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 doc/man-sections/vrf.rst diff --git a/doc/man-sections/vrf.rst b/doc/man-sections/vrf.rst new file mode 100644 index 0000000..f395db1 --- /dev/null +++ b/doc/man-sections/vrf.rst @@ -0,0 +1,75 @@ +Virtual Routing and Forwarding +--------------------------------------- + +Options in this section relates to configuration of virtual routing and +forwarding in combination with the underlying operating system. + +As of today this is only supported on Linux, a kernel >= 4.9 is recommended. + +This could come in handy when for example the external network should be only +used as a means to connect to some VPN endpoints and all regular traffic +should only be routed through any tunnel(s). +This could be achieved by setting up a VRF and configuring the interface +connected to the external network to be part of the VRF. The examples below +will cover this setup. + +Another option would be to put the tun/tap interface into a VRF. This could +be done by an up-script which uses the `ip link set` command shown below. + + +**VRF setup with iproute2** + +Create VRF `vrf_external` and map it to routing table `1023` + +:: + + ip link add vrf_external type vrf table 1023 + +Move `eth0` into `vrf_external` + +:: + + ip link set master vrf_external dev eth0 + +Any prefixes configured on `eth0` will be moved from the `main` routing +table into routing table `1023` + + +**VRF setup with ifupdown** + +For Debian based Distributions `ifupdown2` provides an almost drop-in +replacement for `ifupdown` including VRFs and other features. +A configuration for an interface `eth0` being part of VRF `vrf_external` +could look like this: + +:: + + auto eth0 + iface eth0 + address 192.0.2.42/24 + address 2001:db8:08:15::42/64 + gateway 192.0.2.1 + gateway 2001:db8:08:15::1 + vrf vrf_external + + auto vrf_external + iface vrf_external + vrf-table 1023 + + +**OpenVPN config** + +--bind-dev device + Set the device to bind the server socket to to the VRF + + --bind-dev vrf_external + + +**Further reading** + +Wikipedia has nice page one VRFs: https://en.wikipedia.org/wiki/Virtual_routing_and_forwarding + +This talk from the Network Track of FrOSCon 2018 provides an overview about +advanced layer 2 and layer 3 features of Linux + - Slides: https://www.slideshare.net/BarbarossaTM/l2l3-fr-fortgeschrittene-helle-und-dunkle-magie-im-linuxnetzwerkstack + - Video (german): https://media.ccc.de/v/froscon2018-2247-l2_l3_fur_fortgeschrittene_-_helle_und_dunkle_magie_im_linux-netzwerkstack -- 2.1.4 

Anno domini 2020 Gert Doering scripsit: Hi, > reading this more closely at merging/testing time, I do have a change > request... > > On Fri, Jun 26, 2020 at 08:49:44PM +0200, Maximilian Wilhelm wrote: > > +#ifdef TARGET_LINUX > > + else if (streq (p[0], "bind-dev") && p[1]) > > + { > > + VERIFY_PERMISSION (OPT_P_SOCKFLAGS); > > + options->bind_dev = p[1]; > > + } > > +#endif > > One could argue whether the argument should be changed for IFNAMSIZ here > (so we can error-out right away if it's too long). But this is just > something to consider. That might be a bad idea as of upcoming altnames, see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7a56493f0620cc1b4cffc9bc59289fdefe76b5f3 > > --- a/src/openvpn/socket.c > > +++ b/src/openvpn/socket.c > > @@ -1138,6 +1138,14 @@ create_socket(struct link_socket *sock, struct addrinfo *addr) > > /* set socket to --mark packets with given value */ > > socket_set_mark(sock->sd, sock->mark); > > > > +#if defined(TARGET_LINUX) > > + if (sock->bind_dev) > > + { > > + msg (M_INFO, "Using bind-dev %s", sock->bind_dev); > > + setsockopt (sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, strlen (sock->bind_dev) + 1); > > + } > > +#endif > > Here, we *must* have a return code check, and logging of an error message > if setsocktopt() fails. > > Imagine someone calling "openvpn --bind-dev eht0" (because he has fat > fingers). The current code will silently fail the setsockopt() - because > there is no such interface name - but nothing in the logs will show a hint > *why* openvpn is just not doing what requested. I'll look into that and add a patch for that. Best Max -- "I have to admit I've always suspected that MTBWTF would be a more useful metric of real-world performance." -- Valdis Kletnieks on NANOG 

Hi, reading this more closely at merging/testing time, I do have a change request... On Fri, Jun 26, 2020 at 08:49:44PM +0200, Maximilian Wilhelm wrote: > +#ifdef TARGET_LINUX > + else if (streq (p[0], "bind-dev") && p[1]) > + { > + VERIFY_PERMISSION (OPT_P_SOCKFLAGS); > + options->bind_dev = p[1]; > + } > +#endif One could argue whether the argument should be changed for IFNAMSIZ here (so we can error-out right away if it's too long). But this is just something to consider. > --- a/src/openvpn/socket.c > +++ b/src/openvpn/socket.c > @@ -1138,6 +1138,14 @@ create_socket(struct link_socket *sock, struct addrinfo *addr) > /* set socket to --mark packets with given value */ > socket_set_mark(sock->sd, sock->mark); > > +#if defined(TARGET_LINUX) > + if (sock->bind_dev) > + { > + msg (M_INFO, "Using bind-dev %s", sock->bind_dev); > + setsockopt (sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, strlen (sock->bind_dev) + 1); > + } > +#endif Here, we *must* have a return code check, and logging of an error message if setsocktopt() fails. Imagine someone calling "openvpn --bind-dev eht0" (because he has fat fingers). The current code will silently fail the setsockopt() - because there is no such interface name - but nothing in the logs will show a hint *why* openvpn is just not doing what requested. Sorry for not spotting this in the first review. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... 

 This options allows the user to specify a network device the OpenVPN process should use when making a connection or binding to an address. This translates in setting the SO_BINDTODEVICE option to the corresponding socket (on Linux). When for example using VRFs on Linux [0] this allows making connections using the non-default VRF and having the tun/tap interface in the default VRF. Thanks to David Ahern (Cumulus Networks) for insights on this. [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/vrf.txt Signed-off-by: Maximilian Wilhelm <ma...@sd...> --- src/openvpn/init.c | 1 + src/openvpn/options.c | 10 ++++++++++ src/openvpn/options.h | 1 + src/openvpn/socket.c | 10 ++++++++++ src/openvpn/socket.h | 2 ++ 5 files changed, 24 insertions(+) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 2af25c1..dd1747f 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -3445,6 +3445,7 @@ do_init_socket_1(struct context *c, const int mode) c->options.rcvbuf, c->options.sndbuf, c->options.mark, + c->options.bind_dev, &c->c2.server_poll_interval, sockflags); } diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 3484f7d..6b36269 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -294,6 +294,9 @@ static const char usage_message[] = #if defined(TARGET_LINUX) && HAVE_DECL_SO_MARK "--mark value : Mark encrypted packets being sent with value. The mark value\n" " can be matched in policy routing and packetfilter rules.\n" + "--bind-dev dev : Bind to the given device when making connection to a peer or\n" + " listening for connections. This allows sending encrypted packets\n" + " via a VRF present on the system.\n" #endif "--txqueuelen n : Set the tun/tap TX queue length to n (Linux only).\n" #ifdef ENABLE_MEMSTATS @@ -6064,6 +6067,13 @@ add_option(struct options *options, } } } +#ifdef TARGET_LINUX + else if (streq (p[0], "bind-dev") && p[1]) + { + VERIFY_PERMISSION (OPT_P_SOCKFLAGS); + options->bind_dev = p[1]; + } +#endif else if (streq(p[0], "txqueuelen") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 375a4fc..c83a46a 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -352,6 +352,7 @@ struct options /* mark value */ int mark; + char *bind_dev; /* socket flags */ unsigned int sockflags; diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index f2fb54d..fd92d52 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -1138,6 +1138,14 @@ create_socket(struct link_socket *sock, struct addrinfo *addr) /* set socket to --mark packets with given value */ socket_set_mark(sock->sd, sock->mark); +#if defined(TARGET_LINUX) + if (sock->bind_dev) + { + msg (M_INFO, "Using bind-dev %s", sock->bind_dev); + setsockopt (sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, strlen (sock->bind_dev) + 1); + } +#endif + bind_local(sock, addr->ai_family); } @@ -1880,6 +1888,7 @@ link_socket_init_phase1(struct link_socket *sock, int rcvbuf, int sndbuf, int mark, + const char *bind_dev, struct event_timeout *server_poll_timeout, unsigned int sockflags) { @@ -1906,6 +1915,7 @@ link_socket_init_phase1(struct link_socket *sock, sock->sockflags = sockflags; sock->mark = mark; + sock->bind_dev = bind_dev; sock->info.proto = proto; sock->info.af = af; diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h index 38e5138..8db9e8b 100644 --- a/src/openvpn/socket.h +++ b/src/openvpn/socket.h @@ -212,6 +212,7 @@ struct link_socket #define SF_GETADDRINFO_DGRAM (1<<4) unsigned int sockflags; int mark; + const char *bind_dev; /* for stream sockets */ struct stream_buf stream_buf; @@ -326,6 +327,7 @@ link_socket_init_phase1(struct link_socket *sock, int rcvbuf, int sndbuf, int mark, + const char *bind_dev, struct event_timeout *server_poll_timeout, unsigned int sockflags); /* Reenable uncrustify *INDENT-ON* */ -- 2.1.4 

Hi, this set reintroduces the patch for "VRF support on Linux" by implementing --bind-dev <dev> option from 2016 as well as some real live documentation as requested by Gert :) I wrote a .rst file in the style of the "man-page overhaul project" in the hope that this format is useful and can easily be included. Best Max