Pass interface by LUID instead of alias to WinDns

3 files changed, 46 insertions, 60 deletions

diff --git a/talpid-core/src/dns/windows/mod.rs b/talpid-core/src/dns/windows/mod.rs
index 42ca632e92..d3e22111cd 100644
--- a/talpid-core/src/dns/windows/mod.rs
+++ b/talpid-core/src/dns/windows/mod.rs
@@ -1,10 +1,14 @@
-use crate::logging::windows::{log_sink, LogSink};
+use crate::{
+    logging::windows::{log_sink, LogSink},
+    windows::luid_from_alias,
+};
 
 use lazy_static::lazy_static;
 use log::{error, trace, warn};
 use std::{env, io, net::IpAddr, path::Path};
 use talpid_types::ErrorExt;
 use widestring::WideCString;
+use winapi::shared::ifdef::NET_LUID;
 use winreg::{
     enums::{HKEY_LOCAL_MACHINE, REG_MULTI_SZ},
     transaction::Transaction,
@@ -22,6 +26,7 @@ lazy_static! {
 
 /// Errors that can happen when configuring DNS on Windows.
 #[derive(err_derive::Error, Debug)]
+#[error(no_from)]
 pub enum Error {
     /// Failure to initialize WinDns.
     #[error(display = "Failed to initialize WinDns")]
@@ -35,6 +40,10 @@ pub enum Error {
     #[error(display = "Failed to set new DNS servers on interface")]
     Setting,
 
+    /// Failure to obtain an interface LUID given an alias.
+    #[error(display = "Failed to obtain LUID for the interface alias")]
+    InterfaceLuidError(#[error(source)] io::Error),
+
     /// Failure to set new DNS servers.
     #[error(display = "Failed to update dnscache policy config")]
     UpdateDnsCachePolicy(#[error(source)] io::Error),
@@ -78,9 +87,11 @@ impl super::DnsMonitorT for DnsMonitor {
         trace!("ipv4 ips - {:?} - {}", ipv4, ipv4.len());
         trace!("ipv6 ips - {:?} - {}", ipv6, ipv6.len());
 
+        let luid = luid_from_alias(interface).map_err(Error::InterfaceLuidError)?;
+
         unsafe {
             WinDns_Set(
-                WideCString::from_str(interface).unwrap().as_ptr(),
+                &luid,
                 ipv4_address_ptrs.as_mut_ptr(),
                 ipv4_address_ptrs.len() as u32,
                 ipv6_address_ptrs.as_mut_ptr(),
@@ -132,20 +143,15 @@ impl Drop for DnsMonitor {
 }
 
 fn set_dns_cache_policy(servers: &[IpAddr]) -> Result<(), Error> {
-    let transaction = Transaction::new()?;
-    match set_dns_cache_policy_inner(&transaction, servers) {
-        Ok(()) => {
-            transaction.commit()?;
-            Ok(())
-        }
-        Err(error) => {
-            transaction.rollback()?;
-            Err(error)
-        }
-    }
+    let transaction = Transaction::new().map_err(Error::UpdateDnsCachePolicy)?;
+    let result = match set_dns_cache_policy_inner(&transaction, servers) {
+        Ok(()) => transaction.commit(),
+        Err(error) => transaction.rollback().and_then(|_| Err(error)),
+    };
+    result.map_err(Error::UpdateDnsCachePolicy)
 }
 
-fn set_dns_cache_policy_inner(transaction: &Transaction, servers: &[IpAddr]) -> Result<(), Error> {
+fn set_dns_cache_policy_inner(transaction: &Transaction, servers: &[IpAddr]) -> io::Result<()> {
     let (dns_cache_parameters, _) = RegKey::predef(HKEY_LOCAL_MACHINE).create_subkey_transacted(
         r#"SYSTEM\CurrentControlSet\Services\DnsCache\Parameters"#,
         transaction,
@@ -178,7 +184,8 @@ fn set_dns_cache_policy_inner(transaction: &Transaction, servers: &[IpAddr]) ->
 
 fn reset_dns_cache_policy() -> Result<(), Error> {
     let (dns_cache_parameters, _) = RegKey::predef(HKEY_LOCAL_MACHINE)
-        .create_subkey(r#"SYSTEM\CurrentControlSet\Services\DnsCache\Parameters"#)?;
+        .create_subkey(r#"SYSTEM\CurrentControlSet\Services\DnsCache\Parameters"#)
+        .map_err(Error::UpdateDnsCachePolicy)?;
     match dns_cache_parameters.delete_value("DnsSecureNameQueryFallback") {
         Ok(()) => Ok(()),
         Err(error) => {
@@ -236,7 +243,7 @@ extern "stdcall" {
     // Configure which DNS servers should be used and start enforcing these settings.
     #[link_name = "WinDns_Set"]
     pub fn WinDns_Set(
-        interface_alias: *const u16,
+        interface_luid: *const NET_LUID,
         v4_ips: *mut *const u16,
         v4_n_ips: u32,
         v6_ips: *mut *const u16,
diff --git a/windows/windns/src/windns/windns.cpp b/windows/windns/src/windns/windns.cpp
index 26532f00e8..f7cc35f27f 100644
--- a/windows/windns/src/windns/windns.cpp
+++ b/windows/windns/src/windns/windns.cpp
@@ -45,26 +45,16 @@ std::vector<std::wstring> MakeStringArray(const wchar_t **strings, uint32_t numS
 	return v;
 }
 
-uint32_t ConvertInterfaceAliasToIndex(const std::wstring &interfaceAlias)
+uint32_t ConvertInterfaceLuidToIndex(const NET_LUID &luid)
 {
-	NET_LUID luid;
-
-	if (NO_ERROR != ConvertInterfaceAliasToLuid(interfaceAlias.c_str(), &luid))
-	{
-		const auto err = std::wstring(L"Could not resolve LUID of interface: \"")
-			.append(interfaceAlias).append(L"\"");
-
-		THROW_ERROR(common::string::ToAnsi(err).c_str());
-	}
-
 	NET_IFINDEX index;
 
 	if (NO_ERROR != ConvertInterfaceLuidToIndex(&luid, &index))
 	{
 		std::wstringstream ss;
 
-		ss << L"Could not resolve index of interface: \"" << interfaceAlias << L"\""
-			<< L"with LUID: 0x" << std::hex << luid.Value;
+		ss << L"Could not resolve index of interface with LUID: 0x"
+			<< std::hex << luid.Value;
 
 		THROW_ERROR(common::string::ToAnsi(ss.str()).c_str());
 	}
@@ -79,13 +69,12 @@ struct AdapterDnsAddresses
 };
 
 //
-// Use name when finding the adapter to be more resilient over time.
 // The adapter structure that is returned has two fields for interface index.
 // If IPv4 is enabled, 'IfIndex' will be set. Otherwise set to 0.
 // If IPv6 is enabled, 'Ipv6IfIndex' will be set. Otherwise set to 0.
 // If both IPv4 and IPv6 is enabled, then both fields will be set, and have the same value.
 //
-AdapterDnsAddresses GetAdapterDnsAddresses(const std::wstring &adapterAlias)
+AdapterDnsAddresses GetAdapterDnsAddresses(const NET_LUID &adapterLuid)
 {
 	using shared::network::InterfaceUtils;
 
@@ -94,17 +83,9 @@ AdapterDnsAddresses GetAdapterDnsAddresses(const std::wstring &adapterAlias)
 		GAA_FLAG_SKIP_UNICAST | GAA_FLAG_SKIP_ANYCAST | GAA_FLAG_SKIP_MULTICAST
 	);
 
-	NET_LUID luid;
-	if (NO_ERROR != ConvertInterfaceAliasToLuid(adapterAlias.c_str(), &luid))
-	{
-		const auto err = std::wstring(L"Could not resolve LUID of interface: \"")
-			.append(adapterAlias).append(L"\"");
-		THROW_ERROR(common::string::ToAnsi(err).c_str());
-	}
-
 	for (const auto adapter : adapters)
 	{
-		if (luid.Value != adapter.raw().Luid.Value)
+		if (adapterLuid.Value != adapter.raw().Luid.Value)
 		{
 			continue;
 		}
@@ -126,10 +107,9 @@ AdapterDnsAddresses GetAdapterDnsAddresses(const std::wstring &adapterAlias)
 		return out;
 	}
 
-	const auto msg = std::string("Could not find interface with alias: ")
-		.append(common::string::ToAnsi(adapterAlias));
-
-	THROW_ERROR(msg.c_str());
+	std::stringstream ss;
+	ss << "Could not find interface with LUID: 0x" << std::hex << adapterLuid.Value;
+	THROW_ERROR(ss.str().c_str());
 }
 
 AdapterDnsAddresses ConvertAddresses(
@@ -249,7 +229,7 @@ WINDNS_LINKAGE
 bool
 WINDNS_API
 WinDns_Set(
-	const wchar_t *interfaceAlias,
+	const NET_LUID *interfaceLuid,
 	const wchar_t **ipv4Servers,
 	uint32_t numIpv4Servers,
 	const wchar_t **ipv6Servers,
@@ -261,9 +241,9 @@ WinDns_Set(
 		return false;
 	}
 
-	if (nullptr == interfaceAlias)
+	if (nullptr == interfaceLuid)
 	{
-		g_LogSink->error("Invalid argument: interfaceAlias");
+		g_LogSink->error("Invalid argument: interfaceLuid");
 		return false;
 	}
 
@@ -271,18 +251,16 @@ WinDns_Set(
 	// Check the settings on the adapter.
 	// If it already has the exact same settings we need, we're done.
 	//
-
 	try
 	{
-		const auto activeSettings = GetAdapterDnsAddresses(interfaceAlias);
+		const auto activeSettings = GetAdapterDnsAddresses(*interfaceLuid);
 		const auto wantedSetting = ConvertAddresses(ipv4Servers, numIpv4Servers, ipv6Servers, numIpv6Servers);
 
 		if (Equal(activeSettings, wantedSetting))
 		{
 			std::stringstream ss;
 
-			ss << "DNS settings on adapter with alias \"" << common::string::ToAnsi(interfaceAlias)
-				<< "\" are up-to-date";
+			ss << "DNS settings on adapter with LUID 0x" << std::hex << interfaceLuid->Value << " are up-to-date";
 
 			g_LogSink->info(ss.str().c_str());
 
@@ -293,8 +271,8 @@ WinDns_Set(
 	{
 		std::stringstream ss;
 
-		ss << "Failed to evaluate DNS settings on adapter with alias \""
-			<< common::string::ToAnsi(interfaceAlias) << "\": " << err.what();
+		ss << "Failed to evaluate DNS settings on adapter with LUID 0x"
+			<< std::hex << interfaceLuid->Value << ": " << err.what();
 
 		g_LogSink->info(ss.str().c_str());
 	}
@@ -302,8 +280,8 @@ WinDns_Set(
 	{
 		std::stringstream ss;
 
-		ss << "Failed to evaluate DNS settings on adapter with alias \""
-			<< common::string::ToAnsi(interfaceAlias) << "\": Unspecified failure";
+		ss << "Failed to evaluate DNS settings on adapter with LUID 0x"
+			<< std::hex << interfaceLuid->Value << ": Unspecified failure";
 
 		g_LogSink->info(ss.str().c_str());
 	}
@@ -312,12 +290,13 @@ WinDns_Set(
 	// Apply specified settings.
 	//
 
-	const auto operation = std::string("Apply DNS settings on adapter with alias \"")
-		.append(common::string::ToAnsi(interfaceAlias)).append("\"");
+	std::stringstream operation;
+	operation << "Apply DNS settings on adapter with LUID 0x"
+		<< std::hex << interfaceLuid->Value;
 
-	return ConfineOperation(operation.c_str(), g_LogSink, [&]()
+	return ConfineOperation(operation.str().c_str(), g_LogSink, [&]()
 	{
-		const auto interfaceIndex = ConvertInterfaceAliasToIndex(interfaceAlias);
+		const auto interfaceIndex = ConvertInterfaceLuidToIndex(*interfaceLuid);
 
 		if (nullptr != ipv4Servers && 0 != numIpv4Servers)
 		{
diff --git a/windows/windns/src/windns/windns.h b/windows/windns/src/windns/windns.h
index dee2993681..a26d4a88d4 100644
--- a/windows/windns/src/windns/windns.h
+++ b/windows/windns/src/windns/windns.h
@@ -56,7 +56,7 @@ WINDNS_LINKAGE
 bool
 WINDNS_API
 WinDns_Set(
-	const wchar_t *interfaceAlias,
+	const NET_LUID *interfaceLuid,
 	const wchar_t **ipv4Servers,
 	uint32_t numIpv4Servers,
 	const wchar_t **ipv6Servers,