Merge OpenVPN 3 Core v3.8.2 into master

Signed-off-by: David Sommerseth <davids@openvpn.net>

deps/lib-versions

@@ -4,8 +4,8 @@ export ASIO_CSUM=cbcaaba0f66722787b1a7c33afe1befb3a012b5af3ad7da7ff0f6b8c9b7a8a5
 export LZ4_VERSION=lz4-1.8.3
 export LZ4_CSUM=33af5936ac06536805f9745e0b6d61da606a1f8b4cc5c04dd3cbaca3b9b4fc43
 
-export MBEDTLS_VERSION=mbedtls-2.28.2
+export MBEDTLS_VERSION=mbedtls-2.28.4
-export MBEDTLS_CSUM=bc55232bf71fd66045122ba9050a29ea7cb2e8f99b064a9e6334a82f715881a0
+export MBEDTLS_CSUM=578c4dcd15bbff3f5cd56aa07cd4f850fc733634e3d5947be4f7157d5bfd81ac
 
 export JSONCPP_VERSION=1.8.4
 export JSONCPP_CSUM=c49deac9e0933bcb7044f08516861a2d560988540b23de2ac1ad443b219afdb6

deps/mbedtls/build-mbedtls

@@ -50,8 +50,6 @@ else
 
     # enable MD4 (needed for NTLM auth)
     perl -pi -e 's/^\/\/// if /#define MBEDTLS_MD4_C/' include/mbedtls/config.h
-
-    apply_patches "mbedtls"
 fi
 
 if [ "x$NO_BUILD" == x1 ]; then

deps/mbedtls/patches/0001-relax-x509-date-format-check.patch

@@ -1,55 +0,0 @@
-From 0554efae4e27b6a764def80f600394519ef1addb Mon Sep 17 00:00:00 2001
-From: Antonio Quartulli <antonio@openvpn.net>
-Date: Tue, 20 Mar 2018 09:35:47 +0800
-Subject: [PATCH 1/2] relax x509 date format check
-
-Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
----
- library/x509.c | 18 +++++++++++++++++-
- 1 file changed, 17 insertions(+), 1 deletion(-)
-
-diff --git a/library/x509.c b/library/x509.c
-index 264c7fb0c..9372bcb92 100644
---- a/library/x509.c
-+++ b/library/x509.c
-@@ -556,13 +556,20 @@ static int x509_parse_time( unsigned char **p, size_t len, size_t yearlen,
-     /*
-      * Parse seconds if present
-      */
--    if ( len >= 2 )
-+    if ( len >= 2 && **p >= '0' && **p <= '9' )
-     {
-         CHECK( x509_parse_int( p, 2, &tm->sec ) );
-         len -= 2;
-     }
-     else
-+    {
-+#if defined(MBEDTLS_RELAXED_X509_DATE)
-+	/* if relaxed mode, allow seconds to be absent */
-+	tm->sec = 0;
-+#else
-         return ( MBEDTLS_ERR_X509_INVALID_DATE );
-+#endif
-+    }
- 
-     /*
-      * Parse trailing 'Z' if present
-@@ -572,6 +579,15 @@ static int x509_parse_time( unsigned char **p, size_t len, size_t yearlen,
-         (*p)++;
-         len--;
-     }
-+#if defined(MBEDTLS_RELAXED_X509_DATE)
-+    else if ( len == 5 && **p == '+' )
-+    {
-+	int tz; /* throwaway timezone */
-+	(*p)++;
-+	CHECK( x509_parse_int( p, 4, &tz ) );
-+	return 0;
-+    }
-+#endif
- 
-     /*
-      * We should have parsed all characters at this point
--- 
-2.18.0
-

deps/vcpkg-ports/mbedtls/CONTROL

@@ -1,4 +0,0 @@
-Source: mbedtls
-Version: 2.7.12-1
-Homepage: https://github.com/ARMmbed/mbedtls
-Description: An open source, portable, easy to use, readable and flexible SSL library

deps/vcpkg-ports/mbedtls/portfile.cmake

@@ -1,29 +0,0 @@
-include(vcpkg_common_functions)
-
-set(VCPKG_LIBRARY_LINKAGE static)
-
-vcpkg_from_github(
-    OUT_SOURCE_PATH SOURCE_PATH
-    REPO ARMmbed/mbedtls
-    REF mbedtls-2.7.12
-    SHA512 bfad5588804e52827ecba81ca030fe570c9772f633fbf470d71a781db4366541da69b85ee10941bf500a987c4da825caa049afc2c0e6ec0ecc55d50efd74e5a6
-    HEAD_REF master
-    PATCHES
-        ../../mbedtls/patches/0001-relax-x509-date-format-check.patch
-)
-
-vcpkg_configure_cmake(
-    SOURCE_PATH ${SOURCE_PATH}
-    PREFER_NINJA
-    OPTIONS
-        -DENABLE_TESTING=OFF
-        -DENABLE_PROGRAMS=OFF
-)
-
-vcpkg_install_cmake()
-
-file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/include)
-
-file(INSTALL ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/mbedtls RENAME copyright)
-
-vcpkg_copy_pdbs()

openvpn/client/cliopt.hpp

@@ -656,6 +656,9 @@ class ClientOptions : public RC<thread_unsafe_refcount>
         if (opt.exists("fragment"))
             throw option_error("sorry, 'fragment' directive is not supported, nor is connecting to a server that uses 'fragment' directive");
 
+        if (!opt.exists("client"))
+            throw option_error("Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode.");
+
         // Only p2p mode accept
         if (opt.exists("mode"))
         {
@@ -862,7 +865,6 @@ class ClientOptions : public RC<thread_unsafe_refcount>
         "replay-persist", /* Makes little sense in TLS mode */
         "script-security",
         "sndbuf",
-        "tls-client", /* Always enabled */
         "tmp-dir",
         "tun-ipv6",   /* ignored in v2 as well */
         "txqueuelen", /* so platforms evaluate that in tun, some do not, do not warn about that */
@@ -1248,8 +1250,6 @@ class ClientOptions : public RC<thread_unsafe_refcount>
         cc->set_tls_cert_profile_override(config.clientconf.tlsCertProfileOverride);
         cc->set_tls_cipher_list(config.clientconf.tlsCipherList);
         cc->set_tls_ciphersuite_list(config.clientconf.tlsCiphersuitesList);
-        if (!cc->get_mode().is_client())
-            throw option_error("only client configuration supported");
 
         // client ProtoContext config
         Client::ProtoConfig::Ptr cp(new Client::ProtoConfig());

openvpn/client/cliopthelper.hpp

@@ -358,7 +358,7 @@ class ParseClientConfig
             if (content_list)
             {
                 content_list->preprocess();
-                options.parse_from_key_value_list(*content_list, &limits);
+                options.parse_from_key_value_list(*content_list, "OVPN_ACCESS_SERVER", &limits);
             }
             process_setenv_opt(options);
             options.update_map();
@@ -367,7 +367,7 @@ class ParseClientConfig
             bool added = false;
 
             // client
-            if (!options.exists("client"))
+            if (options.exists("tls-client") && options.exists("pull"))
             {
                 Option opt;
                 opt.push_back("client");

openvpn/common/options.hpp

@@ -710,12 +710,22 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
             return key.length() + value.length();
         }
 
-        Option convert_to_option(Limits *lim) const
+        Option convert_to_option(Limits *lim, const std::string &meta_prefix) const
         {
             bool newline_present = false;
             Option opt;
             const std::string unesc_value = unescape(value, newline_present);
-            opt.push_back(key);
+
+            if (string::starts_with(key, meta_prefix))
+            {
+                opt.push_back(std::string(key, meta_prefix.length()));
+                opt.set_meta();
+            }
+            else
+            {
+                opt.push_back(key);
+            }
+
             if (newline_present || singular_arg(key))
                 opt.push_back(unesc_value);
             else if (unesc_value != "NOARGS")
@@ -966,14 +976,17 @@ class OptionList : public std::vector<Option>, public RCCopyable<thread_unsafe_r
 
     // caller may want to call list.preprocess() before this function
     // caller should call update_map() after this function
-    void parse_from_key_value_list(const KeyValueList &list, Limits *lim)
+    void parse_from_key_value_list(const KeyValueList &list, const std::string &meta_tag, Limits *lim)
     {
+        const std::string meta_prefix = meta_tag + "_";
+
         for (KeyValueList::const_iterator i = list.begin(); i != list.end(); ++i)
         {
             const KeyValue &kv = **i;
             if (lim)
                 lim->add_bytes(kv.combined_length());
-            const Option opt = kv.convert_to_option(lim);
+
+            Option opt = kv.convert_to_option(lim, meta_prefix);
             if (lim)
             {
                 lim->add_opt();

openvpn/mbedtls/ssl/sslctx.hpp

@@ -533,7 +533,7 @@ class MbedTLSContext : public SSLFactoryAPI
             }
 
             // DH
-            if (mode.is_server())
+            if (mode.is_server() && opt.exists("dh"))
             {
                 const std::string &dh_txt = opt.get("dh", 1, Option::MULTILINE);
                 load_dh(dh_txt);

openvpn/openssl/ssl/sslctx.hpp

@@ -435,7 +435,7 @@ class OpenSSLContext : public SSLFactoryAPI
             }
 
             // DH
-            if (mode.is_server())
+            if (mode.is_server() && opt.exists("dh"))
             {
                 const std::string &dh_txt = opt.get("dh", 1, Option::MULTILINE);
                 load_dh(dh_txt);
@@ -1195,15 +1195,16 @@ class OpenSSLContext : public SSLFactoryAPI
                 throw OpenSSLException("OpenSSLContext: SSL_CTX_new_ex failed for server method");
 
             // Set DH object
-            if (!config->dh.defined())
+            if (config->dh.defined())
-                OPENVPN_THROW(ssl_context_error, "OpenSSLContext: DH not defined");
+            {
 #if OPENSSL_VERSION_NUMBER >= 0x30000000L
-            if (!SSL_CTX_set0_tmp_dh_pkey(ctx.get(), config->dh.obj_release()))
+                if (!SSL_CTX_set0_tmp_dh_pkey(ctx.get(), config->dh.obj_release()))
-                throw OpenSSLException("OpenSSLContext: SSL_CTX_set0_tmp_dh_pkey failed");
+                    throw OpenSSLException("OpenSSLContext: SSL_CTX_set0_tmp_dh_pkey failed");
 #else
-            if (!SSL_CTX_set_tmp_dh(ctx.get(), config->dh.obj()))
+                if (!SSL_CTX_set_tmp_dh(ctx.get(), config->dh.obj()))
-                throw OpenSSLException("OpenSSLContext: SSL_CTX_set_tmp_dh failed");
+                    throw OpenSSLException("OpenSSLContext: SSL_CTX_set_tmp_dh failed");
 #endif
+            }
             if (config->flags & SSLConst::SERVER_TO_SERVER)
                 SSL_CTX_set_purpose(ctx.get(), X509_PURPOSE_SSL_SERVER);
 

openvpn/ssl/proto.hpp

@@ -1017,8 +1017,8 @@ class ProtoContext
             if (relay_mode)
                 out << "IV_RELAY=1\n";
             const std::string ret = out.str();
-            OPENVPN_LOG_PROTO("Peer Info:" << std::endl
+            OPENVPN_LOG_PROTO("Sending Peer Info:" << std::endl
-                                           << ret);
+                                                   << ret);
             return ret;
         }
 

openvpn/tun/client/tunprop.hpp

@@ -546,8 +546,9 @@ class TunProp
         DnsOptions dns_options(opt);
         for (const auto &domain : dns_options.search_domains)
         {
-            if (!tb->tun_builder_add_search_domain(domain))
+            if (!tb->tun_builder_set_adapter_domain_suffix(domain))
-                throw tun_prop_dhcp_option_error("tun_builder_add_search_domain failed");
+                throw tun_prop_dhcp_option_error("tun_builder_set_adapter_domain_suffix");
+            break; // use only the first domain for now
         }
         for (const auto &keyval : dns_options.servers)
         {
@@ -564,6 +565,11 @@ class TunProp
                     throw tun_prop_dhcp_option_error("tun_builder_add_dns_server failed");
                 flags |= F_ADD_DNS;
             }
+            for (const auto &domain : server.domains)
+            {
+                if (!tb->tun_builder_add_search_domain(domain))
+                    throw tun_prop_dhcp_option_error("tun_builder_add_search_domain failed");
+            }
         }
 
         OptionList::IndexMap::const_iterator dopt = opt.map().find("dhcp-option"); // DIRECTIVE
@@ -589,7 +595,7 @@ class TunProp
                             throw tun_prop_dhcp_option_error("tun_builder_add_dns_server failed");
                         flags |= F_ADD_DNS;
                     }
-                    else if ((type == "DOMAIN" || type == "DOMAIN-SEARCH") && dns_options.search_domains.empty())
+                    else if ((type == "DOMAIN" || type == "DOMAIN-SEARCH") && dns_options.servers.empty())
                     {
                         o.min_args(3);
                         for (size_t j = 2; j < o.size(); ++j)
@@ -603,7 +609,7 @@ class TunProp
                             }
                         }
                     }
-                    else if (type == "ADAPTER_DOMAIN_SUFFIX")
+                    else if (type == "ADAPTER_DOMAIN_SUFFIX" && dns_options.search_domains.empty())
                     {
                         o.exact_args(3);
                         const std::string &adapter_domain_suffix = o.get(2, 256);

openvpn/tun/mac/gwv4.hpp

@@ -201,7 +201,7 @@ class MacGatewayInfoV4
             struct ifconf ifc;
             const int bufsize = 4096;
 
-            std::unique_ptr<char[]> buffer(new char[bufsize]);
+            const std::unique_ptr<char[]> buffer(new char[bufsize]);
             std::memset(buffer.get(), 0, bufsize);
             sockfd.reset(socket(AF_INET, SOCK_DGRAM, 0));
             if (!sockfd.defined())
@@ -218,26 +218,41 @@ class MacGatewayInfoV4
             {
                 ifreq ifr = {};
                 std::memcpy(&ifr, cp, sizeof(ifr));
-                const size_t len = sizeof(ifr.ifr_name) + std::max(sizeof(ifr.ifr_addr), size_t(ifr.ifr_addr.sa_len));
+                const size_t len = sizeof(ifr.ifr_name) + std::max(sizeof(ifr.ifr_addr), size_t{ifr.ifr_addr.sa_len});
+
                 if (!ifr.ifr_addr.sa_family)
                     break;
                 if (!::strncmp(ifr.ifr_name, iface_, IFNAMSIZ))
                 {
                     if (ifr.ifr_addr.sa_family == AF_LINK)
                     {
-                        /* This is a broken member access. struct sockaddr_dl has
+                        /* This is a confusing member access on multiple levels.
-                         * 20 bytes while if_addr has only 16 bytes. But sockaddr_dl
-                         * has 12 bytes space for the hw address and Ethernet only uses
-                         * 6 bytes. So the last 4 that are truncated can be ignored here
                          *
-                         * So we use a memcpy here to avoid the warnings with ASAN that we
+                         * struct sockaddr_dl is 20 bytes in size and has
-                         * are doing a very nasty cast here
+                         * 12 bytes space for the hw address (6 bytes)
+                         * and Ethernet interface name (max 16 bytes)
+                         *
+                         * So if the interface name is more than 6 byte, it
+                         * extends beyond the struct.
+                         *
+                         * This struct is embedded into ifreq that has
+                         * 16 bytes for a sockaddr and also expects this
+                         * struct to potentially extend beyond the bounds of
+                         * the struct.
+                         *
+                         * Since we only copied 32 bytes from cp to ifr but sdl
+                         * might extend after ifr's end, we  need to copy from
+                         * cp directly to avoid missing out on extra bytes
+                         * behind the struct
                          */
-                        static_assert(sizeof(ifr.ifr_addr) >= 12, "size of if_addr too small to contain MAC");
+                        const size_t sock_dl_len = std::max(sizeof(sockaddr_dl), size_t{ifr.ifr_addr.sa_len});
-                        static_assert(sizeof(sockaddr_dl) >= sizeof(ifr.ifr_addr), "dest struct needs to be larger than source struct");
+
-                        sockaddr_dl sdl{};
+                        const std::unique_ptr<char[]> sock_dl_buf(new char[sock_dl_len]);
-                        std::memcpy(&sdl, &ifr.ifr_addr, sizeof(ifr.ifr_addr));
+                        std::memcpy(sock_dl_buf.get(), cp + offsetof(struct ifreq, ifr_addr), sock_dl_len);
-                        hwaddr_.reset((const unsigned char *)LLADDR(&sdl));
+
+                        const struct sockaddr_dl *sockaddr_dl = reinterpret_cast<struct sockaddr_dl *>(sock_dl_buf.get());
+
+                        hwaddr_.reset(reinterpret_cast<unsigned char *>(LLADDR(sockaddr_dl)));
                         flags_ |= HWADDR_DEFINED;
                     }
                 }

openvpn/tun/win/client/tunsetup.hpp

@@ -548,7 +548,8 @@ class Setup : public SetupBase
         if (pull.reroute_gw.ipv4)
         {
             // get default gateway
-            const Util::BestGateway gw{AF_INET, pull.remote_address.address, tap.index};
+            ADDRESS_FAMILY af = pull.remote_address.ipv6 ? AF_INET6 : AF_INET;
+            const Util::BestGateway gw{af, pull.remote_address.address, tap.index};
 
             if (!gw.local_route())
             {

scripts/mingw/build

@@ -55,9 +55,9 @@ download_deps()
 
     rm -rf xxHash
     git clone https://github.com/Cyan4973/xxHash.git
-    portfile_url=https://raw.githubusercontent.com/microsoft/vcpkg/master/ports/xxhash/portfile.cmake
+    portfile_url=https://raw.githubusercontent.com/microsoft/vcpkg/master/ports/xxhash/vcpkg.json
-    gitref=$(wget -q -O- "$portfile_url" | grep -oP '\bREF\s+\S+' | cut -d' ' -f2)
+    gitref=$(wget -q -O- "$portfile_url" | grep -oP '\"version": \"\S+' | cut -d' ' -f2 | tr -d "\",")
-    git -C xxHash checkout "${gitref}"
+    git -C xxHash checkout "v${gitref}"
 
     popd
 }

test/unittests/test_cliopt.cpp

@@ -27,13 +27,16 @@ std::string dummysecp256key = "-----BEGIN PRIVATE KEY-----\n"
                               "3oP+eSyQewIqu8XJECJhmt8NoXqNNYRUF0P+Jit8LC2a+2WZeyAwuIYT\n"
                               "-----END PRIVATE KEY-----\n";
 
-std::string minimalConfig = "remote wooden.box\n"
+std::string certconfig = "<ca>\n"
-                            "<ca>\n"
+                         + dummysecp256cert + "</ca>\n"
-                            + dummysecp256cert + "</ca>\n"
+                         + "<cert>\n"
-                            + "<cert>\n"
+                         + dummysecp256cert + "</cert>\n"
-                            + dummysecp256cert + "</cert>\n"
+                         + "<key>\n" + dummysecp256key
-                            + "<key>\n" + dummysecp256key
+                         + "</key>\n";
-                            + "</key>\n";
+
+std::string minimalConfig = certconfig + "\n"
+                            + "client\n"
+                              "remote wooden.box\n";
 
 void load_client_config(const std::string &config_content, bool dco = false)
 {
@@ -187,3 +190,64 @@ TEST(config, dco_compatibility)
             "option_error: dco_compatibility: config/options are not compatible with dco");
     }
 }
+
+TEST(config, client_missing_in_config)
+{
+    std::string configNoClient = certconfig + "\nremote 1.2.3.4\n";
+    OVPN_EXPECT_THROW(
+        load_client_config(configNoClient),
+        option_error,
+        "option_error: Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode.");
+}
+
+TEST(config, pull_and_tlsclient_in_config)
+{
+    std::string configNoClient = certconfig + "\nremote 1.2.3.4\ntls-client\npull\n";
+    /* Should not trigger an error, even without --client in place */
+    load_client_config(configNoClient);
+}
+
+TEST(config, pull_and_client_and_tlsclient_in_config)
+{
+    std::string configNoClient = certconfig + "\nremote 1.2.3.4\ntls-client\npull\nclient\n";
+    /* Should not trigger an error. Redundant options are no problem */
+    load_client_config(configNoClient);
+}
+
+TEST(config, onlypullortlsclient)
+{
+    std::array<std::string, 2> options{"tls-client", "pull"};
+
+    for (const auto &opt : options)
+    {
+        std::string configNoClient = certconfig + "\nremote 1.2.3.4\n" + opt + "\n";
+        OVPN_EXPECT_THROW(
+            load_client_config(configNoClient),
+            option_error,
+            "option_error: Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode.");
+    }
+}
+
+TEST(config, meta_option_in_content)
+{
+    OptionList options;
+    auto cfg = minimalConfig + "\n# OVPN_ACCESS_SERVER_AAA=BBB";
+
+    OptionList::KeyValueList kvl;
+    kvl.push_back(new OptionList::KeyValue("OVPN_ACCESS_SERVER_CCC", "DDD"));
+
+    auto parsed_config = ParseClientConfig::parse(cfg, &kvl, options);
+
+    ClientOptions::Config config;
+    config.clientconf.dco = true;
+    config.proto_context_options.reset(new ProtoContextCompressionOptions());
+    ClientOptions cliopt(options, config);
+
+    auto opt = options.get("AAA");
+    ASSERT_TRUE(opt.meta());
+    ASSERT_EQ(opt.get(1, 256), "BBB");
+
+    opt = options.get("CCC");
+    ASSERT_TRUE(opt.meta());
+    ASSERT_EQ(opt.get(1, 256), "DDD");
+}

test/unittests/test_sitnl.cpp

@@ -251,21 +251,27 @@ TEST_F(SitnlTest, TestAddRoute6)
     ASSERT_EQ(SITNL::net_route_add(IP::Route6(route6), IPv6::Addr::from_string(gw6), dev, 0, 0), 0);
 
     std::string dst{"fe80:20c3:cccc:dddd:cccc:dddd:eeee:ffff"};
+    /* Bug in iproute 6.1.0 with glibc 2.37+ that truncates
+       long ipv6 addresses due to strcpy on overlapping buffers.
+       See also https://bugzilla.redhat.com/show_bug.cgi?id=2209701 */
+    std::string dst_trunc{dst};
+    dst_trunc.resize(31);
 
-    ip_route_get(dst, [this, &dst](std::vector<std::string> &v1, const std::string &out, bool &called)
+    ip_route_get(dst, [this, &dst, &dst_trunc](std::vector<std::string> &v1, const std::string &out, bool &called)
                  {
-	  if (v1[0] == dst)
+	  if (v1[0] == dst || v1[0] == dst_trunc)
 	  {
+	    std::string dst_out = (v1[0] == dst) ? dst : dst_trunc;
 	    called = true;
 	    v1.resize(7);
-	    // iptools 4.15 (Ubuntu 18)
+	    // iproute 4.15 (Ubuntu 18)
-	    auto expected1 = std::vector<std::string>{dst, "from", "::", "via", gw6, "dev", dev};
+	    auto expected1 = std::vector<std::string>{dst_out, "from", "::", "via", gw6, "dev", dev};
 	    auto ok1 = (v1 == expected1);
 
 	    auto v2 = v1;
 	    v2.resize(5);
-	    // iptools 4.11 (CentOS 7)
+	    // iproute 4.11 (CentOS 7)
-	    auto expected2 = std::vector<std::string>{dst, "via", gw6, "dev", dev};
+	    auto expected2 = std::vector<std::string>{dst_out, "via", gw6, "dev", dev};
 	    auto ok2 = (v2 == expected2);
 
 	    if (!ok1 && !ok2)