Skip to content

Commit

Permalink
nrpt: match search domains when creating exclude rules
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@d12fk
d12fk authored and Jenkins-dev committed Feb 7, 2025
1 parent b130db4 commit 5d8c438
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 20 deletions.
18 changes: 13 additions & 5 deletions openvpn/tun/win/client/tunsetup.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <openvpn/common/exception.hpp>
#include <openvpn/common/rc.hpp>
#include <openvpn/common/string.hpp>
#include <openvpn/common/wstring.hpp>
#include <openvpn/common/size.hpp>
#include <openvpn/common/arraysize.hpp>
#include <openvpn/error/excode.hpp>
Expand Down Expand Up @@ -609,7 +610,8 @@ class Setup : public SetupBase
{
// apply DNS settings from --dns options
std::vector<std::string> addresses;
std::vector<std::string> domains;
std::vector<std::string> split_domains;
std::vector<std::wstring> wide_search_domains;
std::string search_domains;
bool dnssec = false;

Expand All @@ -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 = ",";
}
Expand All @@ -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));
}

Expand Down Expand Up @@ -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<std::wstring> 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
Expand Down
39 changes: 24 additions & 15 deletions openvpn/tun/win/nrpt.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <sstream>
#include <vector>
#include <array>
#include <algorithm>

#include <openvpn/common/exception.hpp>
#include <openvpn/common/string.hpp>
Expand Down Expand Up @@ -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<std::wstring> &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++);
Expand Down Expand Up @@ -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<ExcludeRuleData> The data collected to create exclude rules from.
*/
static std::vector<ExcludeRuleData> collect_exclude_rule_data()
static std::vector<ExcludeRuleData> collect_exclude_rule_data(const std::vector<std::wstring> &sd)
{
std::vector<ExcludeRuleData> data;
typename REG::Key itfs(REG::subkey_ipv4_itfs);
Expand All @@ -284,7 +290,7 @@ class Nrpt
}

std::wstring domain = interface_dns_domain<REG>(itf_guid);
if (domain.empty())
if (domain.empty() || std::find(sd.begin(), sd.end(), domain) != sd.end())
{
continue;
}
Expand Down Expand Up @@ -387,12 +393,14 @@ class Nrpt
{
public:
ActionCreate(DWORD process_id,
const std::vector<std::string> &domains,
const std::vector<std::string> &split_domains,
const std::vector<std::string> &dns_servers,
const std::vector<std::wstring> &search_domains,
bool dnssec)
: process_id_(process_id),
domains_(domains),
split_domains_(split_domains),
dns_servers_(dns_servers),
search_domains_(search_domains),
dnssec_(dnssec)
{
}
Expand All @@ -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_);
Expand All @@ -439,16 +447,17 @@ 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();
}

private:
DWORD process_id_;
const std::vector<std::string> domains_;
const std::vector<std::string> split_domains_;
const std::vector<std::string> dns_servers_;
const std::vector<std::wstring> search_domains_;
const bool dnssec_;
};

Expand Down

0 comments on commit 5d8c438

Please sign in to comment.