From patchwork Wed Jul 6 04:29:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Quartulli X-Patchwork-Id: 2576 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director14.mail.ord1d.rsapps.net ([172.31.255.6]) by backend30.mail.ord1d.rsapps.net with LMTP id MMqbCn6cxWKPcQAAIUCqbw (envelope-from ) for ; Wed, 06 Jul 2022 10:30:22 -0400 Received: from proxy14.mail.iad3b.rsapps.net ([172.31.255.6]) by director14.mail.ord1d.rsapps.net with LMTP id MD10Cn6cxWL+SgAAeJ7fFg (envelope-from ) for ; Wed, 06 Jul 2022 10:30:22 -0400 Received: from smtp16.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy14.mail.iad3b.rsapps.net with LMTPS id QPdNAn6cxWICGAAA+7ETDg (envelope-from ) for ; Wed, 06 Jul 2022 10:30:22 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp16.gate.iad3b.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=unstable.cc X-Suspicious-Flag: YES X-Classification-ID: 29631dbe-fd38-11ec-a465-5254004ed364-1-1 Received: from [216.105.38.7] ([216.105.38.7:46020] helo=lists.sourceforge.net) by smtp16.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 23/3D-32148-C7C95C26; Wed, 06 Jul 2022 10:30:21 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.94.2) (envelope-from ) id 1o961U-00039I-Td; Wed, 06 Jul 2022 14:29:01 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1o961T-000392-AH for openvpn-devel@lists.sourceforge.net; Wed, 06 Jul 2022 14:29:00 +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=JvkMxRgwCgPoNvxPK4cZ7XyCdZV33n+JXAlXPgi2wnY=; b=GF+TzWrULA8u8k/GmA0NCBAWmK v55WsiDR09D/mZY58s4/qiZAARsC4mm/2eKIwSge22O91VhpzgUSuzIIVrCN7XRvj+kgr/gpDLZE5 AlDbb4T1Ls5PhhPxJU878bXAqUVlL9AeUrMOjfHVfB+T1V4JrmriZkTO93giR4oCFkFE=; 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=JvkMxRgwCgPoNvxPK4cZ7XyCdZV33n+JXAlXPgi2wnY=; b=aWl1NqbBSCFWpUOGKvY5C1Yq0B SwsezUnuwuba8rpIAsvnGPTUcZP0Ys/Lr58GUg/UB9irurJmiPy+K7sf2BYu3VO2EmUT5cjSKKhVb 7uFkKC5dz0P7meRU6Nh8dkChaKTKA51dPeilvDN4y/mAXyxFzXU22SEAC4m1b7ZAGsdk=; Received: from s2.neomailbox.net ([5.148.176.60]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.94.2) id 1o961S-0003ko-Mn for openvpn-devel@lists.sourceforge.net; Wed, 06 Jul 2022 14:28:59 +0000 From: Antonio Quartulli To: openvpn-devel@lists.sourceforge.net Date: Wed, 6 Jul 2022 16:29:07 +0200 Message-Id: <20220706142907.3962-2-a@unstable.cc> In-Reply-To: <20220706142907.3962-1-a@unstable.cc> References: <20220630140804.8277-1-a@unstable.cc> <20220706142907.3962-1-a@unstable.cc> MIME-Version: 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: open_tun_generic already contains the logic required to find a device name when not specified b the user. For this reason the DCO case can easily leverage on function and avoid code duplication. Signed-off-by: Antonio Quartulli --- Content analysis details: (-0.0 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 T_SCC_BODY_TEXT_LINE No description available. X-Headers-End: 1o961S-0003ko-Mn Subject: [Openvpn-devel] [PATCH v6 05/25] dco: let open_tun_generic handle the DCO case 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: , Cc: Antonio Quartulli Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox open_tun_generic already contains the logic required to find a device name when not specified b the user. For this reason the DCO case can easily leverage on function and avoid code duplication. Signed-off-by: Antonio Quartulli --- Changes from v5: * create TUN device when invoking --mktun with DCO enabled. Current code segfaults. * fix message printing "..device created" for DCO cases * print error string (and not just error code) when a DCO iface cannot be created * when creating DCO device, if a TUN device with the same name is found, disable DCO and use the existing device * don't use tunname as it contains the wrong string. Use dev or dynamic_name instead Changes from v4: * in open_tun_generic() use sizeof(tunname) when copying to tunname Changes from v3: * explicitly mention "DCO" in message when DCO device is successfully opened, as per Arne's request Changes from v2: * do not abuse the dynamic_name variable. Rather use 'dev' when no dynamic name is requested * ignore dev-node when using DCO, to avoid messing up naming logic in open_tun_generic. dev-node has sense when using DCO * add comment as to why we need to fill tunname Changes from v1: * improved INFO message when device already exists as per Arne's request src/openvpn/init.c | 30 +++++++- src/openvpn/init.h | 2 +- src/openvpn/options.c | 7 ++ src/openvpn/tun.c | 159 ++++++++++++++++++++++++++++++++---------- src/openvpn/tun.h | 2 +- 5 files changed, 161 insertions(+), 39 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 03221cbb..99fcda04 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1054,7 +1054,7 @@ do_genkey(const struct options *options) * Persistent TUN/TAP device management mode? */ bool -do_persist_tuntap(const struct options *options, openvpn_net_ctx_t *ctx) +do_persist_tuntap(struct options *options, openvpn_net_ctx_t *ctx) { if (options->persist_config) { @@ -1069,6 +1069,26 @@ do_persist_tuntap(const struct options *options, openvpn_net_ctx_t *ctx) msg(M_FATAL|M_OPTERR, "options --mktun or --rmtun should only be used together with --dev"); } + +#if defined(ENABLE_DCO) + if (dco_enabled(options)) + { + /* creating a DCO interface via --mktun is not supported as it does not + * make much sense. Since DCO is enabled by default, people may run into + * this without knowing, therefore this case should be properly handled. + * + * Disable DCO if --mktun was provided and print a message to let + * user know. + */ + if (dev_type_enum(options->dev, options->dev_type) == DEV_TYPE_TUN) + { + msg(M_WARN, "Note: --mktun does not support DCO. Creating TUN interface."); + } + + options->tuntap_options.disable_dco = true; + } +#endif + #ifdef ENABLE_FEATURE_TUN_PERSIST tuncfg(options->dev, options->dev_type, options->dev_node, options->persist_mode, @@ -1763,7 +1783,13 @@ do_open_tun(struct context *c) #endif /* open the tun device */ open_tun(c->options.dev, c->options.dev_type, c->options.dev_node, - c->c1.tuntap); + c->c1.tuntap, &c->net_ctx); + + /* DCO may have been disabled by open_tun(). propagate change */ + c->options.tuntap_options = c->c1.tuntap->options; +#if defined(ENABLE_DCO) + c->c2.tls_multi->opt.disable_dco = !dco_enabled(&c->options); +#endif /* set the hardware address */ if (c->options.lladdr) diff --git a/src/openvpn/init.h b/src/openvpn/init.h index 2b8c2dcc..5f412a33 100644 --- a/src/openvpn/init.h +++ b/src/openvpn/init.h @@ -56,7 +56,7 @@ bool print_openssl_info(const struct options *options); bool do_genkey(const struct options *options); -bool do_persist_tuntap(const struct options *options, openvpn_net_ctx_t *ctx); +bool do_persist_tuntap(struct options *options, openvpn_net_ctx_t *ctx); bool possibly_become_daemon(const struct options *options); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 233c02e0..705bb79a 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3528,6 +3528,13 @@ options_postprocess_mutate(struct options *o, struct env_set *es) o->verify_hash_no_ca = true; } + if (dco_enabled(o) && o->dev_node) + { + msg(M_WARN, "Note: ignoring --dev-node as it has no effect when using " + "data channel offload"); + o->dev_node = NULL; + } + /* * Save certain parms before modifying options during connect, especially * when using --pull diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index e12f0369..d75d75e3 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -37,6 +37,7 @@ #include "syshead.h" +#include "openvpn.h" #include "tun.h" #include "fdmisc.h" #include "common.h" @@ -1718,10 +1719,10 @@ read_tun_header(struct tuntap *tt, uint8_t *buf, int len) #endif /* if defined (TARGET_OPENBSD) || (defined(TARGET_DARWIN) && HAVE_NET_IF_UTUN_H) */ -#if !(defined(_WIN32) || defined(TARGET_LINUX)) +#if !defined(_WIN32) static void open_tun_generic(const char *dev, const char *dev_type, const char *dev_node, - bool dynamic, struct tuntap *tt) + bool dynamic, struct tuntap *tt, openvpn_net_ctx_t *ctx) { char tunname[256]; char dynamic_name[256]; @@ -1780,6 +1781,18 @@ open_tun_generic(const char *dev, const char *dev_type, const char *dev_node, "/dev/%s%d", dev, i); openvpn_snprintf(dynamic_name, sizeof(dynamic_name), "%s%d", dev, i); +#if defined(TARGET_LINUX) + if (!tt->options.disable_dco) + { + if (open_tun_dco(tt, ctx, dynamic_name) == 0) + { + dynamic_opened = true; + msg(M_INFO, "DCO device %s opened", dynamic_name); + break; + } + } + else +#endif /* if defined(TARGET_LINUX) */ if ((tt->fd = open(tunname, O_RDWR)) > 0) { dynamic_opened = true; @@ -1801,30 +1814,81 @@ open_tun_generic(const char *dev, const char *dev_type, const char *dev_node, } } - if (!dynamic_opened) +#if defined(TARGET_LINUX) + if (!tt->options.disable_dco) { - /* has named device existed before? if so, don't destroy at end */ - if (if_nametoindex( dev ) > 0) + if (!dynamic_opened) { - msg(M_INFO, "TUN/TAP device %s exists previously, keep at program end", dev ); - tt->persistent_if = true; - } + /* if dynamic_opened was true, then we already created the + * interface named 'dynamic_name', otherwise we have to create + * now the interface 'dev'. + * + * The variable 'dev_node' is totally ignored in the DCO case + * because it is unset by options post-processing as it makes no + * sense in this context. + */ + int ret = open_tun_dco(tt, ctx, dev); + if (ret == -EEXIST) + { + char type[IFACE_TYPE_LEN_MAX]; - if ((tt->fd = open(tunname, O_RDWR)) < 0) - { - msg(M_ERR, "Cannot open TUN/TAP dev %s", tunname); + ret = net_iface_type(ctx, dev, type); + if (ret < 0 || strcmp(type, "ovpn-dco") != 0) + { + msg(M_WARN, "Existing non-DCO device %s found. Disabling DCO", + dev); + tt->options.disable_dco = true; + + open_tun(dev, dev_type, dev_node, tt, ctx); + return; + } + else + { + msg(M_INFO, "DCO device %s already exists, won't be destroyed at shutdown", + dev); + } + tt->persistent_if = true; + } + else if (ret < 0) + { + msg(M_ERR, "Cannot open DCO device %s: %s (%d)", dev, + strerror(-ret), ret); + } + else + { + msg(M_INFO, "DCO device %s opened", dev); + } } } + else +#endif /* if defined(TARGET_LINUX) */ + { + if (!dynamic_opened) + { + /* has named device existed before? if so, don't destroy at end */ + if (if_nametoindex( dev ) > 0) + { + msg(M_INFO, "TUN/TAP device %s exists previously, keep at program end", dev ); + tt->persistent_if = true; + } - set_nonblock(tt->fd); - set_cloexec(tt->fd); /* don't pass fd to scripts */ - msg(M_INFO, "TUN/TAP device %s opened", tunname); + if ((tt->fd = open(tunname, O_RDWR)) < 0) + { + msg(M_ERR, "Cannot open TUN/TAP dev %s", tunname); + } + } + + set_nonblock(tt->fd); + set_cloexec(tt->fd); /* don't pass fd to scripts */ + + msg(M_INFO, "TUN/TAP device %s opened", tunname); + } /* tt->actual_name is passed to up and down scripts and used as the ifconfig dev name */ tt->actual_name = string_alloc(dynamic_opened ? dynamic_name : dev, NULL); } } -#endif /* !_WIN32 && !TARGET_LINUX */ +#endif /* !_WIN32 */ #if !defined(_WIN32) static void @@ -1842,7 +1906,8 @@ close_tun_generic(struct tuntap *tt) #if defined (TARGET_ANDROID) void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { #define ANDROID_TUNNAME "vpnservice-tun" struct user_pass up; @@ -1939,7 +2004,8 @@ read_tun(struct tuntap *tt, uint8_t *buf, int len) #if !PEDANTIC void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { struct ifreq ifr; @@ -1950,6 +2016,12 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun { open_null(tt); } +#if defined(TARGET_LINUX) + else if (!tt->options.disable_dco) + { + open_tun_generic(dev, dev_type, dev_node, true, tt, ctx); + } +#endif else { /* @@ -2056,7 +2128,8 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun #else /* if !PEDANTIC */ void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { ASSERT(0); } @@ -2081,7 +2154,8 @@ tuncfg(const char *dev, const char *dev_type, const char *dev_node, clear_tuntap(tt); tt->type = dev_type_enum(dev, dev_type); tt->options = *options; - open_tun(dev, dev_type, dev_node, tt); + + open_tun(dev, dev_type, dev_node, tt, ctx); if (ioctl(tt->fd, TUNSETPERSIST, persist_mode) < 0) { msg(M_ERR, "Cannot ioctl TUNSETPERSIST(%d) %s", persist_mode, dev); @@ -2199,6 +2273,12 @@ close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx) net_ctx_reset(ctx); } +#ifdef TARGET_LINUX + if (!tt->options.disable_dco) + { + close_tun_dco(tt, ctx); + } +#endif close_tun_generic(tt); free(tt); } @@ -2222,7 +2302,8 @@ read_tun(struct tuntap *tt, uint8_t *buf, int len) #endif void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { int if_fd, ip_muxid, arp_muxid, arp_fd, ppa = -1; struct lifreq ifr; @@ -2574,9 +2655,10 @@ read_tun(struct tuntap *tt, uint8_t *buf, int len) #elif defined(TARGET_OPENBSD) void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { - open_tun_generic(dev, dev_type, dev_node, true, tt); + open_tun_generic(dev, dev_type, dev_node, true, tt, ctx); /* Enable multicast on the interface */ if (tt->fd >= 0) @@ -2668,9 +2750,10 @@ read_tun(struct tuntap *tt, uint8_t *buf, int len) */ void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { - open_tun_generic(dev, dev_type, dev_node, true, tt); + open_tun_generic(dev, dev_type, dev_node, true, tt, ctx); if (tt->fd >= 0) { @@ -2808,9 +2891,10 @@ freebsd_modify_read_write_return(int len) } void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { - open_tun_generic(dev, dev_type, dev_node, true, tt); + open_tun_generic(dev, dev_type, dev_node, true, tt, ctx); if (tt->fd >= 0 && tt->type == DEV_TYPE_TUN) { @@ -2936,9 +3020,10 @@ dragonfly_modify_read_write_return(int len) } void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { - open_tun_generic(dev, dev_type, dev_node, true, tt); + open_tun_generic(dev, dev_type, dev_node, true, tt, ctx); if (tt->fd >= 0) { @@ -3164,7 +3249,8 @@ open_darwin_utun(const char *dev, const char *dev_type, const char *dev_node, st #endif /* ifdef HAVE_NET_IF_UTUN_H */ void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { #ifdef HAVE_NET_IF_UTUN_H /* If dev_node does not start start with utun assume regular tun/tap */ @@ -3190,7 +3276,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun { /* No explicit utun and utun failed, try the generic way) */ msg(M_INFO, "Failed to open utun device. Falling back to /dev/tun device"); - open_tun_generic(dev, dev_type, NULL, true, tt); + open_tun_generic(dev, dev_type, NULL, true, tt, ctx); } else { @@ -3213,7 +3299,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun dev_node = NULL; } - open_tun_generic(dev, dev_type, dev_node, true, tt); + open_tun_generic(dev, dev_type, dev_node, true, tt, ctx); } } @@ -3271,7 +3357,8 @@ read_tun(struct tuntap *tt, uint8_t *buf, int len) #elif defined(TARGET_AIX) void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { char tunname[256]; char dynamic_name[20]; @@ -6580,7 +6667,8 @@ tuntap_post_open(struct tuntap *tt, const char *device_guid) } void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { const char *device_guid = NULL; @@ -6881,9 +6969,10 @@ ipset2ascii_all(struct gc_arena *gc) #else /* generic */ void -open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt) +open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tuntap *tt, + openvpn_net_ctx_t *ctx) { - open_tun_generic(dev, dev_type, dev_node, true, tt); + open_tun_generic(dev, dev_type, dev_node, true, tt, ctx); } void diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h index 5fcea590..cf02bf43 100644 --- a/src/openvpn/tun.h +++ b/src/openvpn/tun.h @@ -249,7 +249,7 @@ tuntap_ring_empty(struct tuntap *tt) */ void open_tun(const char *dev, const char *dev_type, const char *dev_node, - struct tuntap *tt); + struct tuntap *tt, openvpn_net_ctx_t *ctx); void close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx);