Merge branch 'main' into sam/vpn-snooze-initial-support

* main:
  Prevent endless rekey and tunnel update cycle (#897)
  Duck player support on RMF (#914)
  Update autofill to 12.1.0 (#916)
  Report Autofill failures (#893)
  Update GRDB to 2.4.0 (upstream 6.29.0, SQLCipher 4.6.0) (#915)

Package.resolved

@@ -23,17 +23,17 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/duckduckgo/duckduckgo-autofill.git",
       "state" : {
-        "revision" : "2b81745565db09eee8c1cd44d38eefa1011a9f0a",
-        "version" : "12.0.1"
+        "revision" : "9fea1c6762db726328b14bb9ebfd6508849eae28",
+        "version" : "12.1.0"
       }
     },
     {
       "identity" : "grdb.swift",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/duckduckgo/GRDB.swift.git",
       "state" : {
-        "revision" : "9f049d7b97b1e68ffd86744b500660d34a9e79b8",
-        "version" : "2.3.0"
+        "revision" : "4225b85c9a0c50544e413a1ea1e502c802b44b35",
+        "version" : "2.4.0"
       }
     },
     {

Package.swift

@@ -42,8 +42,8 @@ let package = Package(
         .library(name: "PixelKitTestingUtilities", targets: ["PixelKitTestingUtilities"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "12.0.1"),
-        .package(url: "https://github.com/duckduckgo/GRDB.swift.git", exact: "2.3.0"),
+        .package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "12.1.0"),
+        .package(url: "https://github.com/duckduckgo/GRDB.swift.git", exact: "2.4.0"),
         .package(url: "https://github.com/duckduckgo/TrackerRadarKit", exact: "2.1.2"),
         .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.2.0"),
         .package(url: "https://github.com/gumob/PunycodeSwift.git", exact: "2.1.0"),

Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift

@@ -30,6 +30,7 @@ public enum PrivacyFeature: String {
     case autoconsent
     case clickToLoad
     case autofill
+    case autofillBreakageReporter
     case ampLinks
     case trackingParameters
     case customUserAgent

Sources/Common/Concurrency/TaskExtension.swift

@@ -42,6 +42,29 @@ public extension Task where Success == Never, Failure == Error {
             }
         }
     }
+
+    static func periodic(delay: TimeInterval? = nil,
+                         interval: TimeInterval,
+                         operation: @escaping @Sendable () async throws -> Void,
+                         cancellationHandler: (@Sendable () async -> Void)? = nil) -> Task {
+
+        Task {
+            do {
+                if let delay {
+                    try await Task<Never, Never>.sleep(interval: delay)
+                }
+
+                repeat {
+                    try await operation()
+
+                    try await Task<Never, Never>.sleep(interval: interval)
+                } while true
+            } catch {
+                await cancellationHandler?()
+                throw error
+            }
+        }
+    }
 }
 
 public extension Task where Success == Never, Failure == Never {

Sources/NetworkProtection/Diagnostics/NetworkProtectionEntitlementMonitor.swift

@@ -52,7 +52,7 @@ public actor NetworkProtectionEntitlementMonitor {
 
     // MARK: - Start/Stop monitoring
 
-    public func start(entitlementCheck: @escaping () async -> Swift.Result<Bool, Error>, callback: @escaping (Result) -> Void) {
+    public func start(entitlementCheck: @escaping () async -> Swift.Result<Bool, Error>, callback: @escaping (Result) async -> Void) {
         os_log("⚫️ Starting entitlement monitor", log: .networkProtectionEntitlementMonitorLog)
 
         task = Task.periodic(interval: Self.monitoringInterval) {
@@ -61,14 +61,14 @@ public actor NetworkProtectionEntitlementMonitor {
             case .success(let hasEntitlement):
                 if hasEntitlement {
                     os_log("⚫️ Valid entitlement", log: .networkProtectionEntitlementMonitorLog)
-                    callback(.validEntitlement)
+                    await callback(.validEntitlement)
                 } else {
                     os_log("⚫️ Invalid entitlement", log: .networkProtectionEntitlementMonitorLog)
-                    callback(.invalidEntitlement)
+                    await callback(.invalidEntitlement)
                 }
             case .failure(let error):
                 os_log("⚫️ Error retrieving entitlement: %{public}@", log: .networkProtectionEntitlementMonitorLog, error.localizedDescription)
-                callback(.error(error))
+                await callback(.error(error))
             }
         }
     }

Sources/NetworkProtection/PacketTunnelProvider.swift

@@ -276,10 +276,20 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
             providerEvents.fire(.rekeyAttempt(.success))
         } catch {
             providerEvents.fire(.rekeyAttempt(.failure(error)))
+            await subscriptionAccessErrorHandler(error)
             throw error
         }
     }
 
+    private func subscriptionAccessErrorHandler(_ error: Error) async {
+        switch error {
+        case TunnelError.vpnAccessRevoked:
+            await handleAccessRevoked(attemptsShutdown: true)
+        default:
+            break
+        }
+    }
+
     private func setKeyValidity(_ interval: TimeInterval?) {
         if let interval {
             let firstExpirationDate = Date().addingTimeInterval(interval)
@@ -603,7 +613,18 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         settings.changePublisher
             .receive(on: DispatchQueue.main)
             .sink { [weak self] change in
-                self?.handleSettingsChange(change)
+                guard let self else { return }
+                let handleSettingsChange = handleSettingsChange
+                let subscriptionAccessErrorHandler = subscriptionAccessErrorHandler
+
+                Task { @MainActor in
+                    do {
+                        try await handleSettingsChange(change)
+                    } catch {
+                        await subscriptionAccessErrorHandler(error)
+                        throw error
+                    }
+                }
             }.store(in: &cancellables)
     }
 
@@ -670,7 +691,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
             }
 
             os_log("🔴 Stopping VPN due to no auth token: %{public}s", log: .networkProtection)
-            await attemptShutdown()
+            await attemptShutdownDueToRevokedAccess()
 
             throw error
         }
@@ -962,7 +983,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
             )
         } catch {
             if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error {
-                await handleInvalidEntitlement(attemptsShutdown: true)
                 throw TunnelError.vpnAccessRevoked
             }
 
@@ -1067,16 +1087,14 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         settings.apply(change: change)
     }
 
-    private func handleSettingsChange(_ change: VPNSettings.Change, completionHandler: ((Data?) -> Void)? = nil) {
+    @MainActor
+    private func handleSettingsChange(_ change: VPNSettings.Change) async throws {
         switch change {
         case .setExcludeLocalNetworks:
-            Task { @MainActor in
-                if case .connected = connectionStatus {
-                    try? await updateTunnelConfiguration(
-                        updateMethod: .selectServer(currentServerSelectionMethod),
-                        reassert: false)
-                }
-                completionHandler?(nil)
+            if case .connected = connectionStatus {
+                try await updateTunnelConfiguration(
+                    updateMethod: .selectServer(currentServerSelectionMethod),
+                    reassert: false)
             }
         case .setSelectedServer(let selectedServer):
             let serverSelectionMethod: NetworkProtectionServerSelectionMethod
@@ -1088,13 +1106,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
                 serverSelectionMethod = .preferredServer(serverName: serverName)
             }
 
-            Task { @MainActor in
-                if case .connected = connectionStatus {
-                    try? await updateTunnelConfiguration(
-                        updateMethod: .selectServer(serverSelectionMethod),
-                        reassert: true)
-                }
-                completionHandler?(nil)
+            if case .connected = connectionStatus {
+                try? await updateTunnelConfiguration(
+                    updateMethod: .selectServer(serverSelectionMethod),
+                    reassert: true)
             }
         case .setSelectedLocation(let selectedLocation):
             let serverSelectionMethod: NetworkProtectionServerSelectionMethod
@@ -1106,22 +1121,16 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
                 serverSelectionMethod = .preferredLocation(location)
             }
 
-            Task { @MainActor in
-                if case .connected = connectionStatus {
-                    try? await updateTunnelConfiguration(
-                        updateMethod: .selectServer(serverSelectionMethod),
-                        reassert: true)
-                }
-                completionHandler?(nil)
+            if case .connected = connectionStatus {
+                try? await updateTunnelConfiguration(
+                    updateMethod: .selectServer(serverSelectionMethod),
+                    reassert: true)
             }
         case .setDNSSettings:
-            Task { @MainActor in
-                if case .connected = connectionStatus {
-                    try? await updateTunnelConfiguration(
-                        updateMethod: .selectServer(currentServerSelectionMethod),
-                        reassert: true)
-                }
-                completionHandler?(nil)
+            if case .connected = connectionStatus {
+                try? await updateTunnelConfiguration(
+                    updateMethod: .selectServer(currentServerSelectionMethod),
+                    reassert: true)
             }
         case .setConnectOnLogin,
                 .setIncludeAllNetworks,
@@ -1132,7 +1141,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
                 .setShowInMenuBar,
                 .setDisableRekeying:
             // Intentional no-op, as some setting changes don't require any further operation
-            completionHandler?(nil)
+            break
         }
     }
 
@@ -1147,7 +1156,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
             handleSendTestNotification(completionHandler: completionHandler)
         case .disableConnectOnDemandAndShutDown:
             Task { [weak self] in
-                await self?.attemptShutdown()
+                await self?.attemptShutdownDueToRevokedAccess()
                 completionHandler?(nil)
             }
         case .removeVPNConfiguration:
@@ -1268,26 +1277,23 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
     }
 
     @available(iOS 17, *)
-    public func handleShutDown(completionHandler: ((Data?) -> Void)? = nil) {
-        Task { @MainActor in
-            let managers = try await NETunnelProviderManager.loadAllFromPreferences()
-
-            guard let manager = managers.first else {
-                completionHandler?(nil)
-                return
-            }
-
-            os_log("⚪️ Disabling Connect On Demand and shutting down the tunnel", log: .networkProtection)
+    @MainActor
+    public func handleShutDown() async throws {
+        os_log("🔴 Disabling Connect On Demand and shutting down the tunnel", log: .networkProtection)
+        let managers = try await NETunnelProviderManager.loadAllFromPreferences()
 
-            manager.isOnDemandEnabled = false
-            try await manager.saveToPreferences()
-            try await manager.loadFromPreferences()
+        guard let manager = managers.first else {
+            os_log("Could not find a viable manager, bailing out of shutdown", log: .networkProtection)
+            // Doesn't matter a lot what error we throw here, since we'll try cancelling the
+            // tunnel.
+            throw TunnelError.vpnAccessRevoked
+        }
 
-            let error = NSError(domain: "com.duckduckgo.vpn", code: 0)
-            await cancelTunnel(with: error)
+        manager.isOnDemandEnabled = false
+        try await manager.loadFromPreferences()
+        try await manager.saveToPreferences()
 
-            completionHandler?(nil)
-        }
+        await cancelTunnel(with: TunnelError.vpnAccessRevoked)
     }
 
     private func setIncludedRoutes(_ includedRoutes: [IPAddressRange], completionHandler: ((Data?) -> Void)? = nil) {
@@ -1475,9 +1481,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
             /// Ignore otherwise
             switch result {
             case .invalidEntitlement:
-                Task { [weak self] in
-                    await self?.handleInvalidEntitlement(attemptsShutdown: true)
-                }
+                await self?.handleAccessRevoked(attemptsShutdown: true)
             case .validEntitlement, .error:
                 break
             }
@@ -1516,7 +1520,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
     }
 
     @MainActor
-    private func handleInvalidEntitlement(attemptsShutdown: Bool) async {
+    private func handleAccessRevoked(attemptsShutdown: Bool) async {
         defaults.enableEntitlementMessaging()
         notificationsPresenter.showEntitlementNotification()
 
@@ -1526,18 +1530,32 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         try? await Task.sleep(interval: .seconds(5))
 
         if attemptsShutdown {
-            await attemptShutdown()
+            await attemptShutdownDueToRevokedAccess()
         }
     }
 
-    // Attempt to shut down the tunnel
-    // On iOS 16 and below, as a workaround, we rekey to force a 403 error so that the tunnel fails to restart
+    /// Tries to shut down the tunnel after access has been revoked.
+    ///
+    /// iOS 17+ supports disabling on-demand, but macOS does not... so we resort to removing the subscription token
+    /// which should prevent the VPN from even trying to start.
+    ///
     @MainActor
-    private func attemptShutdown() async {
+    private func attemptShutdownDueToRevokedAccess() async {
+        let cancelTunnel = {
+#if os(macOS)
+            try? self.tokenStore.deleteToken()
+#endif
+            self.cancelTunnelWithError(TunnelError.vpnAccessRevoked)
+        }
+
         if #available(iOS 17, *) {
-            handleShutDown()
+            do {
+                try await handleShutDown()
+            } catch {
+                cancelTunnel()
+            }
         } else {
-            try? await rekey()
+            cancelTunnel()
         }
     }
 

Sources/PrivacyDashboard/BrokenSiteReporting/BrokenSiteReportEntry.swift

@@ -29,10 +29,10 @@ public struct BrokenSiteReportEntry {
     /// Object Creation + 30 days
     let expiryDate: Date?
 
-    public init?(report: BrokenSiteReport, currentDate: Date) {
+    public init?(report: BrokenSiteReport, currentDate: Date, daysToExpiry: Int) {
 
         guard let domainIdentifier = report.siteUrl.privacySafeDomainIdentifier,
-              let expiryDate = Calendar.current.date(byAdding: .day, value: 30, to: currentDate) else {
+              let expiryDate = Calendar.current.date(byAdding: .day, value: daysToExpiry, to: currentDate) else {
             return nil
         }
         self.identifier = domainIdentifier
@@ -50,7 +50,7 @@ public struct BrokenSiteReportEntry {
 
 }
 
-fileprivate extension URL {
+public extension URL {
 
     /// A string containing the first 6 chars of the sha256 hash of the URL's domain part
     var privacySafeDomainIdentifier: String? {

Sources/PrivacyDashboard/BrokenSiteReporting/BrokenSiteReporter.swift

@@ -42,15 +42,17 @@ public class BrokenSiteReporter {
     public typealias PixelHandler = (_ parameters: [String: String]) -> Void
     /// Pixels are sent by the main apps not by BSK, this is the closure called by the class when a pixel need to be sent
     let pixelHandler: PixelHandler
-    let persistencyManager: ExpiryStorage
+    public let persistencyManager: ExpiryStorage
 
-    public init(pixelHandler: @escaping PixelHandler, keyValueStoring: KeyValueStoringDictionaryRepresentable) {
+    public init(pixelHandler: @escaping PixelHandler,
+                keyValueStoring: KeyValueStoringDictionaryRepresentable,
+                storageConfiguration: ExpiryStorageConfiguration = .defaultConfig) {
         self.pixelHandler = pixelHandler
-        self.persistencyManager = ExpiryStorage(keyValueStoring: keyValueStoring)
+        self.persistencyManager = ExpiryStorage(keyValueStoring: keyValueStoring, configuration: storageConfiguration)
     }
 
     /// Report the site breakage
-    public func report(_ report: BrokenSiteReport, reportMode: BrokenSiteReport.Mode) throws {
+    public func report(_ report: BrokenSiteReport, reportMode: BrokenSiteReport.Mode, daysToExpiry: Int = 30) throws {
 
         let now = Date()
         let removedCount = persistencyManager.removeExpiredItems(currentDate: now)
@@ -61,7 +63,7 @@ public class BrokenSiteReporter {
         var report = report
 
         // Create history entry
-        guard let entry = BrokenSiteReportEntry(report: report, currentDate: now) else {
+        guard let entry = BrokenSiteReportEntry(report: report, currentDate: now, daysToExpiry: daysToExpiry) else {
             os_log(.error, "Failed to create a history entry for broken site report")
             throw BrokenSiteReporterError.failedToGenerateHistoryEntry
         }