From 5d8c43804ffd9d071a0e1fd6be83d20fd289a3f0 Mon Sep 17 00:00:00 2001 From: Heiko Hund Date: Tue, 4 Feb 2025 03:34:07 +0100 Subject: [PATCH] nrpt: match search domains when creating exclude rules Compare local domains for exclude rules to search domains and skip matching ones. This prevents the creation of exclude rules when the server indicates that the domain should be resolved via the VPN, by pushing the search domain. Jira: OVPN3-1324 Signed-off-by: Heiko Hund --- openvpn/tun/win/client/tunsetup.hpp | 18 +++++++++---- openvpn/tun/win/nrpt.hpp | 39 ++++++++++++++++++----------- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/openvpn/tun/win/client/tunsetup.hpp b/openvpn/tun/win/client/tunsetup.hpp index e2020598d..0ce9155b6 100644 --- a/openvpn/tun/win/client/tunsetup.hpp +++ b/openvpn/tun/win/client/tunsetup.hpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -609,7 +610,8 @@ class Setup : public SetupBase { // apply DNS settings from --dns options std::vector addresses; - std::vector domains; + std::vector split_domains; + std::vector wide_search_domains; std::string search_domains; bool dnssec = false; @@ -635,12 +637,13 @@ class Setup : public SetupBase // DNS server split domain(s) for (const auto &dom : server.domains) { - domains.push_back("." + dom.domain); + split_domains.push_back("." + dom.domain); } std::string delimiter; for (const auto &domain : pull.dns_options.search_domains) { + wide_search_domains.emplace_back(wstring::from_utf8(domain.to_string())); search_domains.append(delimiter + domain.to_string()); delimiter = ","; } @@ -656,9 +659,9 @@ class Setup : public SetupBase } // To keep local resolvers working, only split rules must be created - if (!allow_local_dns_resolvers || !domains.empty()) + if (!allow_local_dns_resolvers || !split_domains.empty()) { - create.add(new NRPT::ActionCreate(pid, domains, addresses, dnssec)); + create.add(new NRPT::ActionCreate(pid, split_domains, addresses, wide_search_domains, dnssec)); destroy.add(new NRPT::ActionDelete(pid)); } @@ -775,7 +778,12 @@ class Setup : public SetupBase // To keep local resolvers working, only split rules must be created if (!allow_local_dns_resolvers || !split_domains.empty()) { - create.add(new NRPT::ActionCreate(pid, split_domains, dserv, false)); + std::vector wide_search_domains; + for (const auto &domain : pull.dns_options.search_domains) + { + wide_search_domains.emplace_back(wstring::from_utf8(domain.to_string())); + } + create.add(new NRPT::ActionCreate(pid, split_domains, dserv, wide_search_domains, false)); destroy.add(new NRPT::ActionDelete(pid)); // Apply changes to DNS settings diff --git a/openvpn/tun/win/nrpt.hpp b/openvpn/tun/win/nrpt.hpp index bfaa3da32..dd5a2c0b7 100644 --- a/openvpn/tun/win/nrpt.hpp +++ b/openvpn/tun/win/nrpt.hpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -125,15 +126,17 @@ class Nrpt /** * Set NRPT exclude rules to accompany a catch all rule. This is done so that * local resolution of names is not interfered with in case the VPN resolves - * all names. Exclude rules are only installed if the DNS settings came in via - * --dns options, to keep backwards compatibility. + * all names. Exclude rules are only created when no search domain matches to + * local domain to deal with situations where look-ups should go via VPN, but + * exclude rules prevent this. * - * @param process_id the process id used for the rules + * @param process_id the process id used for the rules + * @param search_domains search domains to compare local domains to */ - static void create_exclude_rules(DWORD process_id) + static void create_exclude_rules(DWORD process_id, const std::vector &search_domains) { std::uint32_t n = 0; - const auto data = collect_exclude_rule_data(); + const auto data = collect_exclude_rule_data(search_domains); for (const auto &exclude : data) { const auto id = exclude_rule_id(process_id, n++); @@ -266,11 +269,14 @@ class Nrpt * * This data is only necessary if all the domains are to be resolved through * the VPN. To not break resolving local DNS names, we add so called exclude rules - * to the NRPT for as long as the tunnel persists. + * to the NRPT for as long as the tunnel persists. If a local domain matches one of + * the pushed search domains, skip it, so that look-ups are performed via VPN. + * + * @param sd search domains to compare local domains to * * @return std::vector The data collected to create exclude rules from. */ - static std::vector collect_exclude_rule_data() + static std::vector collect_exclude_rule_data(const std::vector &sd) { std::vector data; typename REG::Key itfs(REG::subkey_ipv4_itfs); @@ -284,7 +290,7 @@ class Nrpt } std::wstring domain = interface_dns_domain(itf_guid); - if (domain.empty()) + if (domain.empty() || std::find(sd.begin(), sd.end(), domain) != sd.end()) { continue; } @@ -387,12 +393,14 @@ class Nrpt { public: ActionCreate(DWORD process_id, - const std::vector &domains, + const std::vector &split_domains, const std::vector &dns_servers, + const std::vector &search_domains, bool dnssec) : process_id_(process_id), - domains_(domains), + split_domains_(split_domains), dns_servers_(dns_servers), + search_domains_(search_domains), dnssec_(dnssec) { } @@ -410,17 +418,17 @@ class Nrpt { // Convert domains into a wide MULTI_SZ string std::wstring domains; - if (domains_.empty()) + if (split_domains_.empty()) { // --dns options did not specify any domains to resolve. domains = L"."; domains.push_back(L'\0'); domains.push_back(L'\0'); - create_exclude_rules(process_id_); + create_exclude_rules(process_id_, search_domains_); } else { - domains = wstring::pack_string_vector(domains_); + domains = wstring::pack_string_vector(split_domains_); } const std::string id = rule_id(process_id_); @@ -439,7 +447,7 @@ class Nrpt std::ostringstream os; os << "NRPT::ActionCreate" << " pid=[" << process_id_ << "]" - << " domains=[" << string::join(domains_, ",") << "]" + << " domains=[" << string::join(split_domains_, ",") << "]" << " dns_servers=[" << string::join(dns_servers_, ",") << "]" << " dnssec=[" << dnssec_ << "]"; return os.str(); @@ -447,8 +455,9 @@ class Nrpt private: DWORD process_id_; - const std::vector domains_; + const std::vector split_domains_; const std::vector dns_servers_; + const std::vector search_domains_; const bool dnssec_; };