From cf7ea85b5bf36f55ab478c421dfe24884d55a004 Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Tue, 11 Feb 2025 23:58:42 +0100
Subject: [PATCH 1/8] first implementation draft

---
 .../NetworkProtectionDeviceManager.swift      | 49 ++++++++++++++++---
 .../PacketTunnelProvider.swift                |  2 +-
 .../Extensions/UserDefaults+dnsSettings.swift | 28 ++++++++---
 .../Settings/VPNSettings.swift                |  8 +++
 4 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
index 003a045b2..010d02124 100644
--- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
+++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
@@ -45,7 +45,7 @@ public enum NetworkProtectionServerSelectionMethod: CustomDebugStringConvertible
 }
 
 public enum NetworkProtectionDNSSettings: Codable, Equatable, CustomStringConvertible {
-    case `default`
+    case ddg(maliciousSiteProtection: Bool)
     case custom([String])
 
     public var usesCustomDNS: Bool {
@@ -55,7 +55,7 @@ public enum NetworkProtectionDNSSettings: Codable, Equatable, CustomStringConver
 
     public var description: String {
         switch self {
-        case .default: return "DuckDuckGo"
+        case .ddg: return "DuckDuckGo"
         case .custom(let servers): return servers.joined(separator: ", ")
         }
     }
@@ -277,9 +277,20 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
         }
 
         let dns: [DNSServer]
+        Logger.networkProtection.log("🐩 DNS settings: \(dnsSettings, privacy: .public)")
+        Logger.networkProtection.error("🐩 Test ERROR: DNS settings = \(dnsSettings)")
+        Logger.networkProtection.log("🐩 DEBUG: Before logging DNS settings")
+        Logger.networkProtection.log("🐩 DNS settings: \(dnsSettings, privacy: .public)")
+        Logger.networkProtection.log("🐩 DEBUG: After logging DNS settings")
         switch dnsSettings {
-        case .default:
-            dns = [DNSServer(address: server.serverInfo.internalIP.ipAddress)]
+        case .ddg(let protectionActive):
+            var ipAddress: IPAddress = server.serverInfo.internalIP.ipAddress
+            if protectionActive {
+                ipAddress = ipAddress.computeBlockMaliciousSitesDnsOrSame()
+            }
+            dns = [DNSServer(address: ipAddress)]
+            print("ipAddress: \(ipAddress)")
+            Logger.networkProtection.log("🐩 DNS settings: \(dns, privacy: .public)")
         case .custom(let servers):
             dns = servers
                 .compactMap { IPv4Address($0) }
@@ -289,8 +300,8 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
         let routingTableResolver = VPNRoutingTableResolver(
             dnsServers: dns,
             excludeLocalNetworks: excludeLocalNetworks)
-
-        Logger.networkProtection.log("Routing table information:\nL Included Routes: \(routingTableResolver.includedRoutes, privacy: .public)\nL Excluded Routes: \(routingTableResolver.excludedRoutes, privacy: .public)")
+        Logger.networkProtection.log("🔵 DEBUG: After logging DNS settings")
+        Logger.networkProtection.log("🐩 Routing table information:\nL Included Routes: \(routingTableResolver.includedRoutes, privacy: .public)\nL Excluded Routes: \(routingTableResolver.excludedRoutes, privacy: .public)")
 
         let interface = InterfaceConfiguration(privateKey: interfacePrivateKey,
                                                addresses: [interfaceAddressRange],
@@ -333,3 +344,29 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
         }
     }
 }
+
+extension IPAddress {
+    /// Returns a new IP address by left-shifting the last octet of the IPv4 address.
+    ///
+    /// if the new address cannot be created, the original address is returned.
+    func computeBlockMaliciousSitesDnsOrSame() -> Self {
+        // Extracts the last byte
+        let data = self.rawValue
+        var bytes = [UInt8](data)
+        guard let lastOctet = bytes.last else { return self }
+
+        // Perform a left-shift on the last octet.
+        // We cast to UInt16 to avoid overflow, then mask with 0xFF to ensure the result fits in 8 bits.
+        let shiftedOctet = UInt8((UInt16(lastOctet) << 1) & 0xFF)
+
+        // Update the last element with the shifted value.
+        bytes[bytes.count - 1] = shiftedOctet
+
+        // Recreate the Data object from the modified bytes.
+        let newData = Data(bytes)
+
+        // Attempt to create a new IPAddress with the updated raw data, preserving the interface.
+        // If creation fails, return the original address.
+        return Self(newData, self.interface) ?? self
+    }
+}
diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift
index ee68a40bd..352ec0a2c 100644
--- a/Sources/NetworkProtection/PacketTunnelProvider.swift
+++ b/Sources/NetworkProtection/PacketTunnelProvider.swift
@@ -558,7 +558,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         case .useExisting:
             break
         case .reset:
-            settings.dnsSettings = .default
+            settings.dnsSettings = .ddg(maliciousSiteProtection: settings.isProtectionEnabled)
         }
     }
 
diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
index 7c1432ac2..02fccbf94 100644
--- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
+++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
@@ -23,10 +23,12 @@ extension UserDefaults {
     final class StorableDNSSettings: NSObject, Codable {
         let usesCustomDNS: Bool
         let dnsServers: [String]
+        let isMaliciousSiteProtectionEnabled: Bool?
 
-        init(usesCustomDNS: Bool = false, dnsServers: [String] = []) {
+        init(usesCustomDNS: Bool = false, dnsServers: [String] = [], isMaliciousSiteProtectionEnabled: Bool = false) {
             self.usesCustomDNS = usesCustomDNS
             self.dnsServers = dnsServers
+            self.isMaliciousSiteProtectionEnabled = isMaliciousSiteProtectionEnabled
         }
     }
 
@@ -35,7 +37,7 @@ extension UserDefaults {
     }
 
     private static func dnsSettingsFromStorageValue(_ value: StorableDNSSettings) -> NetworkProtectionDNSSettings {
-        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .default }
+        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .ddg(maliciousSiteProtection: value.isMaliciousSiteProtectionEnabled ?? false) }
         return .custom(value.dnsServers)
     }
 
@@ -53,6 +55,10 @@ extension UserDefaults {
         }
     }
 
+    var isProtectionEnabled: Bool {
+        dnsSettingStorageValue.isMaliciousSiteProtectionEnabled ?? false
+    }
+
     var dnsSettings: NetworkProtectionDNSSettings {
         get {
             Self.dnsSettingsFromStorageValue(dnsSettingStorageValue)
@@ -60,14 +66,15 @@ extension UserDefaults {
 
         set {
             switch newValue {
-            case .default:
-                dnsSettingStorageValue = StorableDNSSettings()
+            case .ddg(let isMaliciousSiteProtectionEnabled):
+                dnsSettingStorageValue = StorableDNSSettings(isMaliciousSiteProtectionEnabled: isMaliciousSiteProtectionEnabled)
             case .custom(let dnsServers):
                 let hosts = dnsServers.compactMap(\.toIPv4Host)
+                let isMaliciousSiteProtectionEnabled = dnsSettingStorageValue.isMaliciousSiteProtectionEnabled ?? false
                 if hosts.isEmpty {
-                    dnsSettingStorageValue = StorableDNSSettings()
+                    dnsSettingStorageValue = StorableDNSSettings(isMaliciousSiteProtectionEnabled: isMaliciousSiteProtectionEnabled)
                 } else {
-                    dnsSettingStorageValue = StorableDNSSettings(usesCustomDNS: true, dnsServers: hosts)
+                    dnsSettingStorageValue = StorableDNSSettings(usesCustomDNS: true, dnsServers: hosts, isMaliciousSiteProtectionEnabled: isMaliciousSiteProtectionEnabled)
                 }
             }
         }
@@ -79,7 +86,14 @@ extension UserDefaults {
             .eraseToAnyPublisher()
     }
 
+    var isProtectionEnabledPublisher: AnyPublisher<Bool, Never> {
+        publisher(for: \.dnsSettingStorageValue)
+            .map { $0.isMaliciousSiteProtectionEnabled ?? false }
+            .eraseToAnyPublisher()
+    }
+
     func resetDNSSettings() {
-        dnsSettings = .default
+        let isMaliciousSiteProtectionEnabled = dnsSettingStorageValue.isMaliciousSiteProtectionEnabled ?? false
+        dnsSettings = .ddg(maliciousSiteProtection: isMaliciousSiteProtectionEnabled)
     }
 }
diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift
index a5ccbbb37..c1a71469c 100644
--- a/Sources/NetworkProtection/Settings/VPNSettings.swift
+++ b/Sources/NetworkProtection/Settings/VPNSettings.swift
@@ -376,6 +376,14 @@ public final class VPNSettings {
         defaults.dnsSettingsPublisher
     }
 
+    public var isProtectionEnabledPublisher: AnyPublisher<Bool, Never> {
+        defaults.isProtectionEnabledPublisher
+    }
+
+    public var isProtectionEnabled: Bool {
+        defaults.isProtectionEnabled
+    }
+
     public var dnsSettings: NetworkProtectionDNSSettings {
         get {
             defaults.dnsSettings

From 93e6b5b006bb61a0dc97936bc8004de598c8e767 Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Wed, 12 Feb 2025 11:38:06 +0100
Subject: [PATCH 2/8] push default to a single place

---
 .../Settings/Extensions/UserDefaults+dnsSettings.swift      | 6 +++---
 Sources/NetworkProtection/Settings/VPNSettings.swift        | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
index 02fccbf94..7978658f0 100644
--- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
+++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
@@ -25,7 +25,7 @@ extension UserDefaults {
         let dnsServers: [String]
         let isMaliciousSiteProtectionEnabled: Bool?
 
-        init(usesCustomDNS: Bool = false, dnsServers: [String] = [], isMaliciousSiteProtectionEnabled: Bool = false) {
+        init(usesCustomDNS: Bool = false, dnsServers: [String] = [], isMaliciousSiteProtectionEnabled: Bool? = nil) {
             self.usesCustomDNS = usesCustomDNS
             self.dnsServers = dnsServers
             self.isMaliciousSiteProtectionEnabled = isMaliciousSiteProtectionEnabled
@@ -55,8 +55,8 @@ extension UserDefaults {
         }
     }
 
-    var isProtectionEnabled: Bool {
-        dnsSettingStorageValue.isMaliciousSiteProtectionEnabled ?? false
+    var isProtectionEnabled: Bool? {
+        dnsSettingStorageValue.isMaliciousSiteProtectionEnabled
     }
 
     var dnsSettings: NetworkProtectionDNSSettings {
diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift
index c1a71469c..a09a71c57 100644
--- a/Sources/NetworkProtection/Settings/VPNSettings.swift
+++ b/Sources/NetworkProtection/Settings/VPNSettings.swift
@@ -381,7 +381,7 @@ public final class VPNSettings {
     }
 
     public var isProtectionEnabled: Bool {
-        defaults.isProtectionEnabled
+        defaults.isProtectionEnabled ?? true
     }
 
     public var dnsSettings: NetworkProtectionDNSSettings {

From b46b557a3c647360be0ae59487850ef1735324f3 Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Wed, 12 Feb 2025 12:10:25 +0100
Subject: [PATCH 3/8] add and fix tests

---
 .../NetworkProtectionDeviceManager.swift      | 19 +++-----
 .../PacketTunnelProvider.swift                |  2 +-
 .../Extensions/UserDefaults+dnsSettings.swift |  4 +-
 .../NetworkProtectionDeviceManagerTests.swift | 45 ++++++++++++++++++-
 .../FailureRecoveryHandlerTests.swift         | 10 ++---
 5 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
index 010d02124..df230d94f 100644
--- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
+++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
@@ -45,7 +45,7 @@ public enum NetworkProtectionServerSelectionMethod: CustomDebugStringConvertible
 }
 
 public enum NetworkProtectionDNSSettings: Codable, Equatable, CustomStringConvertible {
-    case ddg(maliciousSiteProtection: Bool)
+    case ddg(blockRiskyDomains: Bool)
     case custom([String])
 
     public var usesCustomDNS: Bool {
@@ -278,19 +278,14 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
 
         let dns: [DNSServer]
         Logger.networkProtection.log("🐩 DNS settings: \(dnsSettings, privacy: .public)")
-        Logger.networkProtection.error("🐩 Test ERROR: DNS settings = \(dnsSettings)")
-        Logger.networkProtection.log("🐩 DEBUG: Before logging DNS settings")
-        Logger.networkProtection.log("🐩 DNS settings: \(dnsSettings, privacy: .public)")
-        Logger.networkProtection.log("🐩 DEBUG: After logging DNS settings")
         switch dnsSettings {
-        case .ddg(let protectionActive):
+        case .ddg(let blockRiskyDomains):
             var ipAddress: IPAddress = server.serverInfo.internalIP.ipAddress
-            if protectionActive {
-                ipAddress = ipAddress.computeBlockMaliciousSitesDnsOrSame()
+            if blockRiskyDomains {
+                ipAddress = ipAddress.computeBlockRiskyDomainsDnsOrSame()
             }
             dns = [DNSServer(address: ipAddress)]
-            print("ipAddress: \(ipAddress)")
-            Logger.networkProtection.log("🐩 DNS settings: \(dns, privacy: .public)")
+            Logger.networkProtection.log("🐩 DNS: \(dns, privacy: .public)")
         case .custom(let servers):
             dns = servers
                 .compactMap { IPv4Address($0) }
@@ -300,8 +295,6 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
         let routingTableResolver = VPNRoutingTableResolver(
             dnsServers: dns,
             excludeLocalNetworks: excludeLocalNetworks)
-        Logger.networkProtection.log("🔵 DEBUG: After logging DNS settings")
-        Logger.networkProtection.log("🐩 Routing table information:\nL Included Routes: \(routingTableResolver.includedRoutes, privacy: .public)\nL Excluded Routes: \(routingTableResolver.excludedRoutes, privacy: .public)")
 
         let interface = InterfaceConfiguration(privateKey: interfacePrivateKey,
                                                addresses: [interfaceAddressRange],
@@ -349,7 +342,7 @@ extension IPAddress {
     /// Returns a new IP address by left-shifting the last octet of the IPv4 address.
     ///
     /// if the new address cannot be created, the original address is returned.
-    func computeBlockMaliciousSitesDnsOrSame() -> Self {
+    func computeBlockRiskyDomainsDnsOrSame() -> Self {
         // Extracts the last byte
         let data = self.rawValue
         var bytes = [UInt8](data)
diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift
index 352ec0a2c..2c70d4966 100644
--- a/Sources/NetworkProtection/PacketTunnelProvider.swift
+++ b/Sources/NetworkProtection/PacketTunnelProvider.swift
@@ -558,7 +558,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         case .useExisting:
             break
         case .reset:
-            settings.dnsSettings = .ddg(maliciousSiteProtection: settings.isProtectionEnabled)
+            settings.dnsSettings = .ddg(blockRiskyDomains: settings.isProtectionEnabled)
         }
     }
 
diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
index 7978658f0..ed769eedc 100644
--- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
+++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
@@ -37,7 +37,7 @@ extension UserDefaults {
     }
 
     private static func dnsSettingsFromStorageValue(_ value: StorableDNSSettings) -> NetworkProtectionDNSSettings {
-        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .ddg(maliciousSiteProtection: value.isMaliciousSiteProtectionEnabled ?? false) }
+        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .ddg(blockRiskyDomains: value.isMaliciousSiteProtectionEnabled ?? false) }
         return .custom(value.dnsServers)
     }
 
@@ -94,6 +94,6 @@ extension UserDefaults {
 
     func resetDNSSettings() {
         let isMaliciousSiteProtectionEnabled = dnsSettingStorageValue.isMaliciousSiteProtectionEnabled ?? false
-        dnsSettings = .ddg(maliciousSiteProtection: isMaliciousSiteProtectionEnabled)
+        dnsSettings = .ddg(blockRiskyDomains: isMaliciousSiteProtectionEnabled)
     }
 }
diff --git a/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift b/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift
index 23a43f58c..ba662af78 100644
--- a/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift
+++ b/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift
@@ -20,6 +20,7 @@ import Foundation
 import XCTest
 @testable import NetworkProtection
 @testable import NetworkProtectionTestUtils
+import Network
 
 final class NetworkProtectionDeviceManagerTests: XCTestCase {
     var tokenHandler: SubscriptionTokenHandlingMock!
@@ -204,16 +205,56 @@ final class NetworkProtectionDeviceManagerTests: XCTestCase {
         XCTAssertEqual(firstKey, secondKey) // Check that the key did NOT change, even though we tried to regenerate it
         XCTAssertNotNil(networkClient.spyRegister)
     }
+
+    func testDNSConfigurationWhenProtectionIsActive() async {
+        // GIVEN
+        let server = NetworkProtectionServer.mockRegisteredServer
+        networkClient.stubRegister = .success([server])
+        let protectionActive = true
+        let configuration: NetworkProtectionDeviceManager.GenerateTunnelConfigurationResult
+        let expectedIPAddress = IPv4Address("10.11.12.2")!
+
+        // WHEN
+        do {
+            configuration = try await manager.generateTunnelConfiguration(selectionMethod: .automatic, regenerateKey: false, protectionActive: protectionActive)
+        } catch {
+            XCTFail("Unexpected error \(error.localizedDescription)")
+            return
+        }
+
+        // THEN
+        XCTAssertEqual(configuration.0.interface.dns.first?.address.rawValue, expectedIPAddress.rawValue)
+    }
+
+    func testDNSConfigurationWhenProtectionIsNotActive() async {
+        // GIVEN
+        let server = NetworkProtectionServer.mockRegisteredServer
+        networkClient.stubRegister = .success([server])
+        let protectionActive = false
+        let configuration: NetworkProtectionDeviceManager.GenerateTunnelConfigurationResult
+        let expectedIPAddress = IPv4Address("10.11.12.1")!
+
+        // WHEN
+        do {
+            configuration = try await manager.generateTunnelConfiguration(selectionMethod: .automatic, regenerateKey: false, protectionActive: protectionActive)
+        } catch {
+            XCTFail("Unexpected error \(error.localizedDescription)")
+            return
+        }
+
+        // THEN
+        XCTAssertEqual(configuration.0.interface.dns.first?.address.rawValue, expectedIPAddress.rawValue)
+    }
 }
 
 extension NetworkProtectionDeviceManager {
 
     func generateTunnelConfiguration(selectionMethod: NetworkProtectionServerSelectionMethod,
-                                     regenerateKey: Bool) async throws -> NetworkProtectionDeviceManager.GenerateTunnelConfigurationResult {
+                                     regenerateKey: Bool, protectionActive: Bool = false) async throws -> NetworkProtectionDeviceManager.GenerateTunnelConfigurationResult {
         try await generateTunnelConfiguration(
             resolvedSelectionMethod: selectionMethod,
             excludeLocalNetworks: false,
-            dnsSettings: .default,
+            dnsSettings: .ddg(blockRiskyDomains: protectionActive),
             regenerateKey: regenerateKey
         )
     }
diff --git a/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift b/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift
index 6ec6711d9..d1ba21f76 100644
--- a/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift
+++ b/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift
@@ -58,7 +58,7 @@ final class FailureRecoveryHandlerTests: XCTestCase {
         await failureRecoveryHandler.attemptRecovery(
             to: server,
             excludeLocalNetworks: expectedExcludeLocalNetworks,
-            dnsSettings: .default
+            dnsSettings: .ddg(blockRiskyDomains: false)
         ) {_ in }
         guard let spyGenerateTunnelConfiguration = deviceManager.spyGenerateTunnelConfiguration else {
             XCTFail("attemptRecovery not called")
@@ -122,7 +122,7 @@ final class FailureRecoveryHandlerTests: XCTestCase {
         await failureRecoveryHandler.attemptRecovery(
             to: .mockRegisteredServer,
             excludeLocalNetworks: false,
-            dnsSettings: .default
+            dnsSettings: .ddg(blockRiskyDomains: false)
         ) {_ in }
 
         XCTAssertEqual(startedCount, 1)
@@ -307,7 +307,7 @@ final class FailureRecoveryHandlerTests: XCTestCase {
         await failureRecoveryHandler.attemptRecovery(
             to: .mockRegisteredServer,
             excludeLocalNetworks: false,
-            dnsSettings: .default
+            dnsSettings: .ddg(blockRiskyDomains: false)
         ) {_ in }
     }
 
@@ -323,7 +323,7 @@ final class FailureRecoveryHandlerTests: XCTestCase {
         await failureRecoveryHandler.attemptRecovery(
             to: .mockRegisteredServer,
             excludeLocalNetworks: false,
-            dnsSettings: .default
+            dnsSettings: .ddg(blockRiskyDomains: false)
         ) { _ in
             let underlyingError = NSError(domain: "test", code: 1)
             throw WireGuardAdapterError.startWireGuardBackend(underlyingError)
@@ -342,7 +342,7 @@ final class FailureRecoveryHandlerTests: XCTestCase {
 
         var newConfigResult: NetworkProtectionDeviceManagement.GenerateTunnelConfigurationResult?
 
-        await failureRecoveryHandler.attemptRecovery(to: lastServer, excludeLocalNetworks: false, dnsSettings: .default) { configResult in
+        await failureRecoveryHandler.attemptRecovery(to: lastServer, excludeLocalNetworks: false, dnsSettings: .ddg(blockRiskyDomains: false)) { configResult in
             newConfigResult = configResult
         }
         return newConfigResult

From af0e405b22c2b750a99bdc208f1beb0efb764b0d Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Wed, 12 Feb 2025 12:34:49 +0100
Subject: [PATCH 4/8] tidy up

---
 .../PacketTunnelProvider.swift                |  2 +-
 .../Extensions/UserDefaults+dnsSettings.swift | 30 +++++++++----------
 .../Settings/VPNSettings.swift                |  8 ++---
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift
index 2c70d4966..fd12491b3 100644
--- a/Sources/NetworkProtection/PacketTunnelProvider.swift
+++ b/Sources/NetworkProtection/PacketTunnelProvider.swift
@@ -558,7 +558,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         case .useExisting:
             break
         case .reset:
-            settings.dnsSettings = .ddg(blockRiskyDomains: settings.isProtectionEnabled)
+            settings.dnsSettings = .ddg(blockRiskyDomains: settings.isBlockRiskyDomainsOn)
         }
     }
 
diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
index ed769eedc..67ba73b16 100644
--- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
+++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
@@ -23,12 +23,12 @@ extension UserDefaults {
     final class StorableDNSSettings: NSObject, Codable {
         let usesCustomDNS: Bool
         let dnsServers: [String]
-        let isMaliciousSiteProtectionEnabled: Bool?
+        let isBlockRiskyDomainsOn: Bool?
 
-        init(usesCustomDNS: Bool = false, dnsServers: [String] = [], isMaliciousSiteProtectionEnabled: Bool? = nil) {
+        init(usesCustomDNS: Bool = false, dnsServers: [String] = [], isBlockRiskyDomainsOn: Bool? = nil) {
             self.usesCustomDNS = usesCustomDNS
             self.dnsServers = dnsServers
-            self.isMaliciousSiteProtectionEnabled = isMaliciousSiteProtectionEnabled
+            self.isBlockRiskyDomainsOn = isBlockRiskyDomainsOn
         }
     }
 
@@ -37,7 +37,7 @@ extension UserDefaults {
     }
 
     private static func dnsSettingsFromStorageValue(_ value: StorableDNSSettings) -> NetworkProtectionDNSSettings {
-        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .ddg(blockRiskyDomains: value.isMaliciousSiteProtectionEnabled ?? false) }
+        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .ddg(blockRiskyDomains: value.isBlockRiskyDomainsOn ?? false) }
         return .custom(value.dnsServers)
     }
 
@@ -55,8 +55,8 @@ extension UserDefaults {
         }
     }
 
-    var isProtectionEnabled: Bool? {
-        dnsSettingStorageValue.isMaliciousSiteProtectionEnabled
+    var isBlockRiskyDomainsOn: Bool? {
+        dnsSettingStorageValue.isBlockRiskyDomainsOn
     }
 
     var dnsSettings: NetworkProtectionDNSSettings {
@@ -66,15 +66,15 @@ extension UserDefaults {
 
         set {
             switch newValue {
-            case .ddg(let isMaliciousSiteProtectionEnabled):
-                dnsSettingStorageValue = StorableDNSSettings(isMaliciousSiteProtectionEnabled: isMaliciousSiteProtectionEnabled)
+            case .ddg(let isBlockRiskyDomainsOn):
+                dnsSettingStorageValue = StorableDNSSettings(isBlockRiskyDomainsOn: isBlockRiskyDomainsOn)
             case .custom(let dnsServers):
                 let hosts = dnsServers.compactMap(\.toIPv4Host)
-                let isMaliciousSiteProtectionEnabled = dnsSettingStorageValue.isMaliciousSiteProtectionEnabled ?? false
+                let isBlockRiskyDomainsOn = dnsSettingStorageValue.isBlockRiskyDomainsOn ?? true
                 if hosts.isEmpty {
-                    dnsSettingStorageValue = StorableDNSSettings(isMaliciousSiteProtectionEnabled: isMaliciousSiteProtectionEnabled)
+                    dnsSettingStorageValue = StorableDNSSettings(isBlockRiskyDomainsOn: isBlockRiskyDomainsOn)
                 } else {
-                    dnsSettingStorageValue = StorableDNSSettings(usesCustomDNS: true, dnsServers: hosts, isMaliciousSiteProtectionEnabled: isMaliciousSiteProtectionEnabled)
+                    dnsSettingStorageValue = StorableDNSSettings(usesCustomDNS: true, dnsServers: hosts, isBlockRiskyDomainsOn: isBlockRiskyDomainsOn)
                 }
             }
         }
@@ -86,14 +86,14 @@ extension UserDefaults {
             .eraseToAnyPublisher()
     }
 
-    var isProtectionEnabledPublisher: AnyPublisher<Bool, Never> {
+    var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool?, Never> {
         publisher(for: \.dnsSettingStorageValue)
-            .map { $0.isMaliciousSiteProtectionEnabled ?? false }
+            .map { $0.isBlockRiskyDomainsOn }
             .eraseToAnyPublisher()
     }
 
     func resetDNSSettings() {
-        let isMaliciousSiteProtectionEnabled = dnsSettingStorageValue.isMaliciousSiteProtectionEnabled ?? false
-        dnsSettings = .ddg(blockRiskyDomains: isMaliciousSiteProtectionEnabled)
+        let isBlockRiskyDomainsOn = dnsSettingStorageValue.isBlockRiskyDomainsOn ?? true
+        dnsSettings = .ddg(blockRiskyDomains: isBlockRiskyDomainsOn)
     }
 }
diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift
index a09a71c57..594f3a126 100644
--- a/Sources/NetworkProtection/Settings/VPNSettings.swift
+++ b/Sources/NetworkProtection/Settings/VPNSettings.swift
@@ -376,12 +376,12 @@ public final class VPNSettings {
         defaults.dnsSettingsPublisher
     }
 
-    public var isProtectionEnabledPublisher: AnyPublisher<Bool, Never> {
-        defaults.isProtectionEnabledPublisher
+    public var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool?, Never> {
+        defaults.isBlockRiskyDomainsOnPublisher
     }
 
-    public var isProtectionEnabled: Bool {
-        defaults.isProtectionEnabled ?? true
+    public var isBlockRiskyDomainsOn: Bool {
+        defaults.isBlockRiskyDomainsOn ?? true
     }
 
     public var dnsSettings: NetworkProtectionDNSSettings {

From de3b2508eebb84777a922bc8142b0488a1a4f18d Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Thu, 13 Feb 2025 14:11:35 +0100
Subject: [PATCH 5/8] set default as true

---
 .../Extensions/UserDefaults+dnsSettings.swift | 32 +++++++++++++++----
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
index 67ba73b16..653392995 100644
--- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
+++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
@@ -23,13 +23,32 @@ extension UserDefaults {
     final class StorableDNSSettings: NSObject, Codable {
         let usesCustomDNS: Bool
         let dnsServers: [String]
-        let isBlockRiskyDomainsOn: Bool?
+        let isBlockRiskyDomainsOn: Bool
 
-        init(usesCustomDNS: Bool = false, dnsServers: [String] = [], isBlockRiskyDomainsOn: Bool? = nil) {
+        init(usesCustomDNS: Bool = false, dnsServers: [String] = [], isBlockRiskyDomainsOn: Bool = true) {
             self.usesCustomDNS = usesCustomDNS
             self.dnsServers = dnsServers
             self.isBlockRiskyDomainsOn = isBlockRiskyDomainsOn
         }
+
+        private enum CodingKeys: String, CodingKey {
+            case usesCustomDNS, dnsServers, isBlockRiskyDomainsOn
+        }
+
+        required init(from decoder: Decoder) throws {
+            let container = try decoder.container(keyedBy: CodingKeys.self)
+            self.usesCustomDNS = try container.decodeIfPresent(Bool.self, forKey: .usesCustomDNS) ?? false
+            self.dnsServers = try container.decodeIfPresent([String].self, forKey: .dnsServers) ?? []
+            // Provide a default value if key is missing (e.g., true)
+            self.isBlockRiskyDomainsOn = try container.decodeIfPresent(Bool.self, forKey: .isBlockRiskyDomainsOn) ?? true
+        }
+
+        func encode(to encoder: Encoder) throws {
+            var container = encoder.container(keyedBy: CodingKeys.self)
+            try container.encode(usesCustomDNS, forKey: .usesCustomDNS)
+            try container.encode(dnsServers, forKey: .dnsServers)
+            try container.encode(isBlockRiskyDomainsOn, forKey: .isBlockRiskyDomainsOn)
+        }
     }
 
     private var dnsSettingKey: String {
@@ -37,7 +56,7 @@ extension UserDefaults {
     }
 
     private static func dnsSettingsFromStorageValue(_ value: StorableDNSSettings) -> NetworkProtectionDNSSettings {
-        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .ddg(blockRiskyDomains: value.isBlockRiskyDomainsOn ?? false) }
+        guard value.usesCustomDNS, !value.dnsServers.isEmpty else { return .ddg(blockRiskyDomains: value.isBlockRiskyDomainsOn) }
         return .custom(value.dnsServers)
     }
 
@@ -55,7 +74,7 @@ extension UserDefaults {
         }
     }
 
-    var isBlockRiskyDomainsOn: Bool? {
+    var isBlockRiskyDomainsOn: Bool {
         dnsSettingStorageValue.isBlockRiskyDomainsOn
     }
 
@@ -70,7 +89,7 @@ extension UserDefaults {
                 dnsSettingStorageValue = StorableDNSSettings(isBlockRiskyDomainsOn: isBlockRiskyDomainsOn)
             case .custom(let dnsServers):
                 let hosts = dnsServers.compactMap(\.toIPv4Host)
-                let isBlockRiskyDomainsOn = dnsSettingStorageValue.isBlockRiskyDomainsOn ?? true
+                let isBlockRiskyDomainsOn = dnsSettingStorageValue.isBlockRiskyDomainsOn
                 if hosts.isEmpty {
                     dnsSettingStorageValue = StorableDNSSettings(isBlockRiskyDomainsOn: isBlockRiskyDomainsOn)
                 } else {
@@ -86,14 +105,13 @@ extension UserDefaults {
             .eraseToAnyPublisher()
     }
 
-    var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool?, Never> {
+    var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool, Never> {
         publisher(for: \.dnsSettingStorageValue)
             .map { $0.isBlockRiskyDomainsOn }
             .eraseToAnyPublisher()
     }
 
     func resetDNSSettings() {
-        let isBlockRiskyDomainsOn = dnsSettingStorageValue.isBlockRiskyDomainsOn ?? true
         dnsSettings = .ddg(blockRiskyDomains: isBlockRiskyDomainsOn)
     }
 }

From f5b5750e3b05894c6430db2593adbf61121f5994 Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Thu, 13 Feb 2025 14:14:57 +0100
Subject: [PATCH 6/8] fix

---
 Sources/NetworkProtection/Settings/VPNSettings.swift | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift
index 594f3a126..da9824a77 100644
--- a/Sources/NetworkProtection/Settings/VPNSettings.swift
+++ b/Sources/NetworkProtection/Settings/VPNSettings.swift
@@ -376,12 +376,12 @@ public final class VPNSettings {
         defaults.dnsSettingsPublisher
     }
 
-    public var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool?, Never> {
+    public var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool, Never> {
         defaults.isBlockRiskyDomainsOnPublisher
     }
 
     public var isBlockRiskyDomainsOn: Bool {
-        defaults.isBlockRiskyDomainsOn ?? true
+        defaults.isBlockRiskyDomainsOn
     }
 
     public var dnsSettings: NetworkProtectionDNSSettings {

From 9a6d1f4f0220da9639e72cf81bd40554341f106c Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Fri, 14 Feb 2025 01:41:38 +0100
Subject: [PATCH 7/8] add subfeature

---
 .../PrivacyConfig/Features/PrivacyFeature.swift     |  3 +++
 .../Extensions/UserDefaults+dnsSettings.swift       | 13 ++++++-------
 .../NetworkProtection/Settings/VPNSettings.swift    |  8 ++++----
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift
index db897d0a1..d34524b1f 100644
--- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift
+++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift
@@ -151,6 +151,9 @@ public enum NetworkProtectionSubfeature: String, Equatable, PrivacySubfeature {
     /// Enforce routes for the VPN to fix TunnelVision
     /// https://app.asana.com/0/72649045549333/1208617860225199/f
     case enforceRoutes
+
+    /// Risky Domain Protection for VPN
+    case riskyDomainsProtection
 }
 
 public enum SyncSubfeature: String, PrivacySubfeature {
diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
index 653392995..51738dee3 100644
--- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
+++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
@@ -78,6 +78,10 @@ extension UserDefaults {
         dnsSettingStorageValue.isBlockRiskyDomainsOn
     }
 
+    var customDnsServers: [String] {
+        dnsSettingStorageValue.dnsServers
+    }
+
     var dnsSettings: NetworkProtectionDNSSettings {
         get {
             Self.dnsSettingsFromStorageValue(dnsSettingStorageValue)
@@ -86,7 +90,8 @@ extension UserDefaults {
         set {
             switch newValue {
             case .ddg(let isBlockRiskyDomainsOn):
-                dnsSettingStorageValue = StorableDNSSettings(isBlockRiskyDomainsOn: isBlockRiskyDomainsOn)
+                let dnsServers = dnsSettingStorageValue.dnsServers
+                dnsSettingStorageValue = StorableDNSSettings(dnsServers: dnsServers, isBlockRiskyDomainsOn: isBlockRiskyDomainsOn)
             case .custom(let dnsServers):
                 let hosts = dnsServers.compactMap(\.toIPv4Host)
                 let isBlockRiskyDomainsOn = dnsSettingStorageValue.isBlockRiskyDomainsOn
@@ -105,12 +110,6 @@ extension UserDefaults {
             .eraseToAnyPublisher()
     }
 
-    var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool, Never> {
-        publisher(for: \.dnsSettingStorageValue)
-            .map { $0.isBlockRiskyDomainsOn }
-            .eraseToAnyPublisher()
-    }
-
     func resetDNSSettings() {
         dnsSettings = .ddg(blockRiskyDomains: isBlockRiskyDomainsOn)
     }
diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift
index da9824a77..bf976b75e 100644
--- a/Sources/NetworkProtection/Settings/VPNSettings.swift
+++ b/Sources/NetworkProtection/Settings/VPNSettings.swift
@@ -376,14 +376,14 @@ public final class VPNSettings {
         defaults.dnsSettingsPublisher
     }
 
-    public var isBlockRiskyDomainsOnPublisher: AnyPublisher<Bool, Never> {
-        defaults.isBlockRiskyDomainsOnPublisher
-    }
-
     public var isBlockRiskyDomainsOn: Bool {
         defaults.isBlockRiskyDomainsOn
     }
 
+    public var customDnsServers: [String] {
+        defaults.customDnsServers
+    }
+
     public var dnsSettings: NetworkProtectionDNSSettings {
         get {
             defaults.dnsSettings

From 0f5645beb2b1c39d578d94b5ea04d445462319dc Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <stardio@duckduckgo.com>
Date: Fri, 14 Feb 2025 11:16:38 +0100
Subject: [PATCH 8/8] tidy up

---
 .../PrivacyConfig/Features/PrivacyFeature.swift             | 1 +
 .../NetworkProtection/NetworkProtectionDeviceManager.swift  | 6 +-----
 .../Settings/Extensions/UserDefaults+dnsSettings.swift      | 1 -
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift
index d34524b1f..63eb19667 100644
--- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift
+++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift
@@ -153,6 +153,7 @@ public enum NetworkProtectionSubfeature: String, Equatable, PrivacySubfeature {
     case enforceRoutes
 
     /// Risky Domain Protection for VPN
+    /// https://app.asana.com/0/1204186595873227/1206489252288889
     case riskyDomainsProtection
 }
 
diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
index df230d94f..0dc398ade 100644
--- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
+++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
@@ -277,7 +277,6 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
         }
 
         let dns: [DNSServer]
-        Logger.networkProtection.log("🐩 DNS settings: \(dnsSettings, privacy: .public)")
         switch dnsSettings {
         case .ddg(let blockRiskyDomains):
             var ipAddress: IPAddress = server.serverInfo.internalIP.ipAddress
@@ -285,7 +284,6 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
                 ipAddress = ipAddress.computeBlockRiskyDomainsDnsOrSame()
             }
             dns = [DNSServer(address: ipAddress)]
-            Logger.networkProtection.log("🐩 DNS: \(dns, privacy: .public)")
         case .custom(let servers):
             dns = servers
                 .compactMap { IPv4Address($0) }
@@ -355,11 +353,9 @@ extension IPAddress {
         // Update the last element with the shifted value.
         bytes[bytes.count - 1] = shiftedOctet
 
-        // Recreate the Data object from the modified bytes.
-        let newData = Data(bytes)
-
         // Attempt to create a new IPAddress with the updated raw data, preserving the interface.
         // If creation fails, return the original address.
+        let newData = Data(bytes)
         return Self(newData, self.interface) ?? self
     }
 }
diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
index 51738dee3..08042df6b 100644
--- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
+++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+dnsSettings.swift
@@ -39,7 +39,6 @@ extension UserDefaults {
             let container = try decoder.container(keyedBy: CodingKeys.self)
             self.usesCustomDNS = try container.decodeIfPresent(Bool.self, forKey: .usesCustomDNS) ?? false
             self.dnsServers = try container.decodeIfPresent([String].self, forKey: .dnsServers) ?? []
-            // Provide a default value if key is missing (e.g., true)
             self.isBlockRiskyDomainsOn = try container.decodeIfPresent(Bool.self, forKey: .isBlockRiskyDomainsOn) ?? true
         }