From 0407d41005972acf498b92ac53c2faf7b92fcc54 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 4 Feb 2019 07:37:26 +0100 Subject: [PATCH] Keychain: store configurations in keychain instead of providerConfig --- WireGuard/Shared/FileManager+Extension.swift | 7 +- WireGuard/Shared/Keychain.swift | 117 ++++++++++++++++++ .../Shared/Model/LegacyConfigMigration.swift | 32 +++-- .../NETunnelProviderProtocol+Extension.swift | 38 ++++-- WireGuard/WireGuard.xcodeproj/project.pbxproj | 10 ++ .../WireGuard/Tunnel/TunnelsManager.swift | 20 ++- .../WireGuard/UI/iOS/WireGuard.entitlements | 4 +- ...WireGuardNetworkExtension_iOS.entitlements | 4 +- 8 files changed, 201 insertions(+), 31 deletions(-) create mode 100644 WireGuard/Shared/Keychain.swift diff --git a/WireGuard/Shared/FileManager+Extension.swift b/WireGuard/Shared/FileManager+Extension.swift index 2155683..edd764f 100644 --- a/WireGuard/Shared/FileManager+Extension.swift +++ b/WireGuard/Shared/FileManager+Extension.swift @@ -5,7 +5,7 @@ import Foundation import os.log extension FileManager { - private static var sharedFolderURL: URL? { + static var appGroupId: String? { #if os(iOS) let appGroupIdInfoDictionaryKey = "com.wireguard.ios.app_group_id" #elseif os(macOS) @@ -13,7 +13,10 @@ extension FileManager { #else #error("Unimplemented") #endif - guard let appGroupId = Bundle.main.object(forInfoDictionaryKey: appGroupIdInfoDictionaryKey) as? String else { + return Bundle.main.object(forInfoDictionaryKey: appGroupIdInfoDictionaryKey) as? String + } + private static var sharedFolderURL: URL? { + guard let appGroupId = FileManager.appGroupId else { os_log("Cannot obtain app group ID from bundle", log: OSLog.default, type: .error) return nil } diff --git a/WireGuard/Shared/Keychain.swift b/WireGuard/Shared/Keychain.swift new file mode 100644 index 0000000..edc546d --- /dev/null +++ b/WireGuard/Shared/Keychain.swift @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: MIT +// Copyright © 2018-2019 WireGuard LLC. All Rights Reserved. + +import Foundation +import Security + +class Keychain { + static func openReference(called ref: Data) -> String? { + var result: CFTypeRef? + let ret = SecItemCopyMatching([kSecClass as String: kSecClassGenericPassword, + kSecValuePersistentRef as String: ref, + kSecReturnData as String: true] as CFDictionary, + &result) + if ret != errSecSuccess || result == nil { + wg_log(.error, message: "Unable to open config from keychain: \(ret)") + return nil + } + guard let data = result as? Data else { return nil } + return String(data: data, encoding: String.Encoding.utf8) + } + + static func makeReference(containing value: String, called name: String, previouslyReferencedBy oldRef: Data? = nil) -> Data? { + var ret: OSStatus + guard var id = Bundle.main.bundleIdentifier else { + wg_log(.error, staticMessage: "Unable to determine bundle identifier") + return nil + } + if id.hasSuffix(".network-extension") { + id.removeLast(".network-extension".count) + } + var items: [String: Any] = [kSecClass as String: kSecClassGenericPassword, + kSecAttrLabel as String: "WireGuard Tunnel: " + name, + kSecAttrAccount as String: name + ": " + UUID().uuidString, + kSecAttrDescription as String: "wg-quick(8) config", + kSecAttrService as String: id, + kSecValueData as String: value.data(using: .utf8) as Any, + kSecReturnPersistentRef as String: true] + + #if os(iOS) + items[kSecAttrAccessGroup as String] = FileManager.appGroupId + items[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock + #elseif os(macOS) + items[kSecAttrSynchronizable as String] = false + items[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly + + guard let extensionPath = Bundle.main.builtInPlugInsURL?.appendingPathComponent("WireGuardNetworkExtension.appex").path else { + wg_log(.error, staticMessage: "Unable to determine app extension path") + return nil + } + var extensionApp: SecTrustedApplication? + var mainApp: SecTrustedApplication? + ret = SecTrustedApplicationCreateFromPath(extensionPath, &extensionApp) + if ret != kOSReturnSuccess || extensionApp == nil { + wg_log(.error, message: "Unable to create keychain extension trusted application object: \(ret)") + return nil + } + ret = SecTrustedApplicationCreateFromPath(nil, &mainApp) + if ret != errSecSuccess || mainApp == nil { + wg_log(.error, message: "Unable to create keychain local trusted application object: \(ret)") + return nil + } + var access: SecAccess? + ret = SecAccessCreate((items[kSecAttrLabel as String] as? String)! as CFString, + [extensionApp!, mainApp!] as CFArray, + &access) + if ret != errSecSuccess || access == nil { + wg_log(.error, message: "Unable to create keychain ACL object: \(ret)") + return nil + } + items[kSecAttrAccess as String] = access! + #else + #error("Unimplemented") + #endif + + var ref: CFTypeRef? + ret = SecItemAdd(items as CFDictionary, &ref) + if ret != errSecSuccess || ref == nil { + wg_log(.error, message: "Unable to add config to keychain: \(ret)") + return nil + } + if let oldRef = oldRef { + deleteReference(called: oldRef) + } + return ref as? Data + } + + static func deleteReference(called ref: Data) { + let ret = SecItemDelete([kSecValuePersistentRef as String: ref] as CFDictionary) + if ret != errSecSuccess { + wg_log(.error, message: "Unable to delete config from keychain: \(ret)") + } + } + + static func deleteReferences(except whitelist: Set) { + var result: CFTypeRef? + let ret = SecItemCopyMatching([kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: Bundle.main.bundleIdentifier as Any, + kSecMatchLimit as String: kSecMatchLimitAll, + kSecReturnPersistentRef as String: true] as CFDictionary, + &result) + if ret != errSecSuccess || result == nil { + return + } + guard let items = result as? [Data] else { return } + for item in items { + if !whitelist.contains(item) { + deleteReference(called: item) + } + } + } + + static func verifyReference(called ref: Data) -> Bool { + return SecItemCopyMatching([kSecClass as String: kSecClassGenericPassword, + kSecValuePersistentRef as String: ref] as CFDictionary, + nil) == errSecSuccess + } +} diff --git a/WireGuard/Shared/Model/LegacyConfigMigration.swift b/WireGuard/Shared/Model/LegacyConfigMigration.swift index 16792fa..583e914 100644 --- a/WireGuard/Shared/Model/LegacyConfigMigration.swift +++ b/WireGuard/Shared/Model/LegacyConfigMigration.swift @@ -174,20 +174,32 @@ final class LegacyTunnelConfiguration: LegacyModel { extension NETunnelProviderProtocol { @discardableResult - func migrateConfigurationIfNeeded() -> Bool { - guard let configurationVersion = providerConfiguration?["tunnelConfigurationVersion"] as? Int else { return false } - if configurationVersion == 1 { - migrateFromConfigurationV1() - } else { - fatalError("No migration from configuration version \(configurationVersion) exists.") + func migrateConfigurationIfNeeded(called name: String) -> Bool { + var ret = false + if migrateFromConfigurationV1() { + ret = true } + if migrateFromConfigurationV2(called: name) { + ret = true + } + return ret + } + + private func migrateFromConfigurationV1() -> Bool { + guard let configurationVersion = providerConfiguration?["tunnelConfigurationVersion"] as? Int else { return false } + guard configurationVersion == 1 else { return false } + guard let serializedTunnelConfiguration = providerConfiguration?["tunnelConfiguration"] as? Data else { return false } + guard let configuration = try? JSONDecoder().decode(LegacyTunnelConfiguration.self, from: serializedTunnelConfiguration) else { return false } + providerConfiguration = ["WgQuickConfig": configuration.migrated.asWgQuickConfig()] return true } - private func migrateFromConfigurationV1() { - guard let serializedTunnelConfiguration = providerConfiguration?["tunnelConfiguration"] as? Data else { return } - guard let configuration = try? JSONDecoder().decode(LegacyTunnelConfiguration.self, from: serializedTunnelConfiguration) else { return } - providerConfiguration = [Keys.wgQuickConfig.rawValue: configuration.migrated.asWgQuickConfig()] + private func migrateFromConfigurationV2(called name: String) -> Bool { + guard let oldConfig = providerConfiguration?["WgQuickConfig"] as? String else { return false } + providerConfiguration = nil + guard passwordReference == nil else { return true } + passwordReference = Keychain.makeReference(containing: oldConfig, called: name) + return true } } diff --git a/WireGuard/Shared/Model/NETunnelProviderProtocol+Extension.swift b/WireGuard/Shared/Model/NETunnelProviderProtocol+Extension.swift index 7b3142e..3b7cd1e 100644 --- a/WireGuard/Shared/Model/NETunnelProviderProtocol+Extension.swift +++ b/WireGuard/Shared/Model/NETunnelProviderProtocol+Extension.swift @@ -12,17 +12,16 @@ enum PacketTunnelProviderError: String, Error { } extension NETunnelProviderProtocol { - - enum Keys: String { - case wgQuickConfig = "WgQuickConfig" - } - - convenience init?(tunnelConfiguration: TunnelConfiguration) { + convenience init?(tunnelConfiguration: TunnelConfiguration, previouslyFrom old: NEVPNProtocol? = nil) { self.init() - let appId = Bundle.main.bundleIdentifier! + guard let name = tunnelConfiguration.name else { return nil } + guard let appId = Bundle.main.bundleIdentifier else { return nil } providerBundleIdentifier = "\(appId).network-extension" - providerConfiguration = [Keys.wgQuickConfig.rawValue: tunnelConfiguration.asWgQuickConfig()] + passwordReference = Keychain.makeReference(containing: tunnelConfiguration.asWgQuickConfig(), called: name, previouslyReferencedBy: old?.passwordReference) + if passwordReference == nil { + return nil + } let endpoints = tunnelConfiguration.peers.compactMap { $0.endpoint } if endpoints.count == 1 { @@ -35,9 +34,26 @@ extension NETunnelProviderProtocol { } func asTunnelConfiguration(called name: String? = nil) -> TunnelConfiguration? { - migrateConfigurationIfNeeded() - guard let serializedConfig = providerConfiguration?[Keys.wgQuickConfig.rawValue] as? String else { return nil } - return try? TunnelConfiguration(fromWgQuickConfig: serializedConfig, called: name) + migrateConfigurationIfNeeded(called: name ?? "unknown") + //TODO: in the case where migrateConfigurationIfNeeded is called by the network extension, + // before the app has started, and when there is, in fact, configuration that needs to be + // put into the keychain, this will generate one new keychain item every time it is started, + // until finally the app is open. Would it be possible to call saveToPreferences here? Or is + // that generally not available to network extensions? In which case, what should our + // behavior be? + + guard let passwordReference = passwordReference else { return nil } + guard let config = Keychain.openReference(called: passwordReference) else { return nil } + return try? TunnelConfiguration(fromWgQuickConfig: config, called: name) } + func destroyConfigurationReference() { + guard let ref = passwordReference else { return } + Keychain.deleteReference(called: ref) + } + + func verifyConfigurationReference() -> Data? { + guard let ref = passwordReference else { return nil } + return Keychain.verifyReference(called: ref) ? ref : nil + } } diff --git a/WireGuard/WireGuard.xcodeproj/project.pbxproj b/WireGuard/WireGuard.xcodeproj/project.pbxproj index 473ae0b..7ef5dd0 100644 --- a/WireGuard/WireGuard.xcodeproj/project.pbxproj +++ b/WireGuard/WireGuard.xcodeproj/project.pbxproj @@ -30,6 +30,10 @@ 5FF7B96321CC95DE00A7DD74 /* InterfaceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5FF7B96121CC95DE00A7DD74 /* InterfaceConfiguration.swift */; }; 5FF7B96521CC95FA00A7DD74 /* PeerConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5FF7B96421CC95FA00A7DD74 /* PeerConfiguration.swift */; }; 5FF7B96621CC95FA00A7DD74 /* PeerConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5FF7B96421CC95FA00A7DD74 /* PeerConfiguration.swift */; }; + 6B5C5E27220A48D30024272E /* Keychain.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B5C5E26220A48D30024272E /* Keychain.swift */; }; + 6B5C5E28220A48D30024272E /* Keychain.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B5C5E26220A48D30024272E /* Keychain.swift */; }; + 6B5C5E29220A48D30024272E /* Keychain.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B5C5E26220A48D30024272E /* Keychain.swift */; }; + 6B5C5E2A220A48D30024272E /* Keychain.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B5C5E26220A48D30024272E /* Keychain.swift */; }; 6B707D8421F918D4000A8F73 /* TunnelConfiguration+UapiConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B707D8321F918D4000A8F73 /* TunnelConfiguration+UapiConfig.swift */; }; 6B707D8621F918D4000A8F73 /* TunnelConfiguration+UapiConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B707D8321F918D4000A8F73 /* TunnelConfiguration+UapiConfig.swift */; }; 6F4DD16B21DA558800690EAE /* TunnelListRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F4DD16A21DA558800690EAE /* TunnelListRow.swift */; }; @@ -238,6 +242,7 @@ 5F9696AF21CD7128008063FE /* TunnelConfiguration+WgQuickConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TunnelConfiguration+WgQuickConfig.swift"; sourceTree = ""; }; 5FF7B96121CC95DE00A7DD74 /* InterfaceConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InterfaceConfiguration.swift; sourceTree = ""; }; 5FF7B96421CC95FA00A7DD74 /* PeerConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PeerConfiguration.swift; sourceTree = ""; }; + 6B5C5E26220A48D30024272E /* Keychain.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Keychain.swift; sourceTree = ""; }; 6B707D8321F918D4000A8F73 /* TunnelConfiguration+UapiConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TunnelConfiguration+UapiConfig.swift"; sourceTree = ""; }; 6F4DD16721DA552B00690EAE /* NSTableView+Reuse.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NSTableView+Reuse.swift"; sourceTree = ""; }; 6F4DD16A21DA558800690EAE /* TunnelListRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelListRow.swift; sourceTree = ""; }; @@ -427,6 +432,7 @@ 6FF3526A21C23F720008484E /* Logging */, 6F7774E6217201E0006A79B3 /* Model */, 6F5A2B4421AFDE020081EDD8 /* FileManager+Extension.swift */, + 6B5C5E26220A48D30024272E /* Keychain.swift */, ); path = Shared; sourceTree = ""; @@ -1075,6 +1081,7 @@ 6F5A2B4621AFDED40081EDD8 /* FileManager+Extension.swift in Sources */, 6FFA5DA021958ECC0001E2F7 /* ErrorNotifier.swift in Sources */, 5F9696B121CD7128008063FE /* TunnelConfiguration+WgQuickConfig.swift in Sources */, + 6B5C5E28220A48D30024272E /* Keychain.swift in Sources */, 6FFA5D96219446380001E2F7 /* NETunnelProviderProtocol+Extension.swift in Sources */, 6FFA5D8E2194370D0001E2F7 /* TunnelConfiguration.swift in Sources */, 5FF7B96621CC95FA00A7DD74 /* PeerConfiguration.swift in Sources */, @@ -1105,6 +1112,7 @@ 6FB1BDD321D50F5300A991BF /* ZipArchive.swift in Sources */, 6FB1BDD421D50F5300A991BF /* ioapi.c in Sources */, 6FDB3C3C21DCF6BB00A0C0BF /* TunnelViewModel.swift in Sources */, + 6B5C5E29220A48D30024272E /* Keychain.swift in Sources */, 6FCD99AF21E0EA1700BA4C82 /* ImportPanelPresenter.swift in Sources */, 6FB1BDD521D50F5300A991BF /* unzip.c in Sources */, 6FB1BDD621D50F5300A991BF /* zip.c in Sources */, @@ -1162,6 +1170,7 @@ 6FB1BDB221D4F55700A991BF /* DNSResolver.swift in Sources */, 6FB1BDB321D4F55700A991BF /* ErrorNotifier.swift in Sources */, 6FB1BDA221D4F53300A991BF /* ringlogger.c in Sources */, + 6B5C5E2A220A48D30024272E /* Keychain.swift in Sources */, 6FB1BDA421D4F53300A991BF /* Logger.swift in Sources */, 6FB1BDA521D4F53300A991BF /* TunnelConfiguration+WgQuickConfig.swift in Sources */, 6FB1BDA621D4F53300A991BF /* NETunnelProviderProtocol+Extension.swift in Sources */, @@ -1200,6 +1209,7 @@ 6FDEF80021863C0100D8FBF6 /* ioapi.c in Sources */, 6F7F7E5F21C7D74B00527607 /* TunnelErrors.swift in Sources */, 6FDEF7FC21863B6100D8FBF6 /* zip.c in Sources */, + 6B5C5E27220A48D30024272E /* Keychain.swift in Sources */, 6F628C3F217F3413003482A3 /* DNSServer.swift in Sources */, 6F628C3D217F09E9003482A3 /* TunnelViewModel.swift in Sources */, 5F4541A621C4449E00994C13 /* ButtonCell.swift in Sources */, diff --git a/WireGuard/WireGuard/Tunnel/TunnelsManager.swift b/WireGuard/WireGuard/Tunnel/TunnelsManager.swift index 6bcf6f7..e10ba77 100644 --- a/WireGuard/WireGuard/Tunnel/TunnelsManager.swift +++ b/WireGuard/WireGuard/Tunnel/TunnelsManager.swift @@ -44,12 +44,21 @@ class TunnelsManager { return } - let tunnelManagers = managers ?? [] - tunnelManagers.forEach { tunnelManager in - if (tunnelManager.protocolConfiguration as? NETunnelProviderProtocol)?.migrateConfigurationIfNeeded() == true { + var tunnelManagers = managers ?? [] + var refs: Set = [] + for (index, tunnelManager) in tunnelManagers.enumerated().reversed() { + let proto = tunnelManager.protocolConfiguration as? NETunnelProviderProtocol + if proto?.migrateConfigurationIfNeeded(called: tunnelManager.localizedDescription ?? "unknown") ?? false { tunnelManager.saveToPreferences { _ in } } + if let ref = proto?.verifyConfigurationReference() { + refs.insert(ref) + } else { + tunnelManager.removeFromPreferences { _ in } + tunnelManagers.remove(at: index) + } } + Keychain.deleteReferences(except: refs) completionHandler(.success(TunnelsManager(tunnelProviders: tunnelManagers))) } #endif @@ -105,6 +114,7 @@ class TunnelsManager { tunnelProviderManager.saveToPreferences { [weak self] error in guard error == nil else { wg_log(.error, message: "Add: Saving configuration failed: \(error!)") + (tunnelProviderManager.protocolConfiguration as? NETunnelProviderProtocol)?.destroyConfigurationReference() completionHandler(.failure(TunnelsManagerError.systemErrorOnAddTunnel(systemError: error!))) return } @@ -153,7 +163,7 @@ class TunnelsManager { tunnel.name = tunnelName } - tunnelProviderManager.protocolConfiguration = NETunnelProviderProtocol(tunnelConfiguration: tunnelConfiguration) + tunnelProviderManager.protocolConfiguration = NETunnelProviderProtocol(tunnelConfiguration: tunnelConfiguration, previouslyFrom: tunnelProviderManager.protocolConfiguration) tunnelProviderManager.localizedDescription = tunnelConfiguration.name tunnelProviderManager.isEnabled = true @@ -162,6 +172,7 @@ class TunnelsManager { tunnelProviderManager.saveToPreferences { [weak self] error in guard error == nil else { + //TODO: the passwordReference for the old one has already been removed at this point and we can't easily roll back! wg_log(.error, message: "Modify: Saving configuration failed: \(error!)") completionHandler(TunnelsManagerError.systemErrorOnModifyTunnel(systemError: error!)) return @@ -202,6 +213,7 @@ class TunnelsManager { func remove(tunnel: TunnelContainer, completionHandler: @escaping (TunnelsManagerError?) -> Void) { let tunnelProviderManager = tunnel.tunnelProvider + (tunnelProviderManager.protocolConfiguration as? NETunnelProviderProtocol)?.destroyConfigurationReference() tunnelProviderManager.removeFromPreferences { [weak self] error in guard error == nil else { diff --git a/WireGuard/WireGuard/UI/iOS/WireGuard.entitlements b/WireGuard/WireGuard/UI/iOS/WireGuard.entitlements index b5bbc16..33ce9fc 100644 --- a/WireGuard/WireGuard/UI/iOS/WireGuard.entitlements +++ b/WireGuard/WireGuard/UI/iOS/WireGuard.entitlements @@ -8,7 +8,7 @@ com.apple.security.application-groups - group.$(APP_ID_IOS) - + group.$(APP_ID_IOS) + diff --git a/WireGuard/WireGuardNetworkExtension/WireGuardNetworkExtension_iOS.entitlements b/WireGuard/WireGuardNetworkExtension/WireGuardNetworkExtension_iOS.entitlements index b5bbc16..33ce9fc 100644 --- a/WireGuard/WireGuardNetworkExtension/WireGuardNetworkExtension_iOS.entitlements +++ b/WireGuard/WireGuardNetworkExtension/WireGuardNetworkExtension_iOS.entitlements @@ -8,7 +8,7 @@ com.apple.security.application-groups - group.$(APP_ID_IOS) - + group.$(APP_ID_IOS) +