From bb5cd1e1ab2b569bd5fe75224b6b261fbdeeba74 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 23 Nov 2021 00:24:01 +0100 Subject: [PATCH] Return password reference at the time of setting Simplifies app/extension IPC. --- Demo/Demo/iOS/ViewController.swift | 11 +++------ Demo/Demo/macOS/ViewController.swift | 11 +++------ Sources/TunnelKitManager/Keychain.swift | 24 +++++++++---------- .../OpenVPNTunnelProvider.swift | 5 ++-- .../OpenVPNProvider+Configuration.swift | 10 ++++---- .../AppExtensionTests.swift | 2 +- 6 files changed, 27 insertions(+), 36 deletions(-) diff --git a/Demo/Demo/iOS/ViewController.swift b/Demo/Demo/iOS/ViewController.swift index 88cc676..56a29cb 100644 --- a/Demo/Demo/iOS/ViewController.swift +++ b/Demo/Demo/iOS/ViewController.swift @@ -98,12 +98,11 @@ class ViewController: UIViewController, URLSessionDataDelegate { let credentials = OpenVPN.Credentials(textUsername.text!, textPassword.text!) let cfg = Configuration.make(hostname: hostname, port: port, socketType: socketType) - try? keychain.set(password: credentials.password, for: credentials.username, context: tunnelIdentifier) let proto = try! cfg.generatedTunnelProtocol( withBundleIdentifier: tunnelIdentifier, appGroup: appGroup, context: tunnelIdentifier, - username: credentials.username + credentials: credentials ) let neCfg = NetworkExtensionVPNConfiguration(title: "BasicTunnel", protocolConfiguration: proto, onDemandRules: []) vpn.reconnect(configuration: neCfg) { (error) in @@ -147,15 +146,11 @@ class ViewController: UIViewController, URLSessionDataDelegate { let username = "foo" let password = "bar" - guard let _ = try? keychain.set(password: password, for: username, context: tunnelIdentifier) else { + guard let ref = try? keychain.set(password: password, for: username, context: tunnelIdentifier) else { print("Couldn't set password") return } - guard let passwordReference = try? keychain.passwordReference(for: username, context: tunnelIdentifier) else { - print("Couldn't get password reference") - return - } - guard let fetchedPassword = try? keychain.password(for: username, reference: passwordReference, context: tunnelIdentifier) else { + guard let fetchedPassword = try? Keychain.password(forReference: ref) else { print("Couldn't fetch password") return } diff --git a/Demo/Demo/macOS/ViewController.swift b/Demo/Demo/macOS/ViewController.swift index a77aed8..859d6b7 100644 --- a/Demo/Demo/macOS/ViewController.swift +++ b/Demo/Demo/macOS/ViewController.swift @@ -89,12 +89,11 @@ class ViewController: NSViewController { let credentials = OpenVPN.Credentials(textUsername.stringValue, textPassword.stringValue) let cfg = Configuration.make(hostname: hostname, port: port, socketType: .udp) - try? keychain.set(password: credentials.password, for: credentials.username, context: tunnelIdentifier) let proto = try! cfg.generatedTunnelProtocol( withBundleIdentifier: tunnelIdentifier, appGroup: appGroup, context: tunnelIdentifier, - username: credentials.username + credentials: credentials ) let neCfg = NetworkExtensionVPNConfiguration(title: "BasicTunnel", protocolConfiguration: proto, onDemandRules: []) vpn.reconnect(configuration: neCfg) { (error) in @@ -132,15 +131,11 @@ class ViewController: NSViewController { let username = "foo" let password = "bar" - guard let _ = try? keychain.set(password: password, for: username, context: tunnelIdentifier) else { + guard let ref = try? keychain.set(password: password, for: username, context: tunnelIdentifier) else { print("Couldn't set password") return } - guard let passwordReference = try? keychain.passwordReference(for: username, context: tunnelIdentifier) else { - print("Couldn't get password reference") - return - } - guard let fetchedPassword = try? keychain.password(for: username, reference: passwordReference, context: tunnelIdentifier) else { + guard let fetchedPassword = try? Keychain.password(forReference: ref) else { print("Couldn't fetch password") return } diff --git a/Sources/TunnelKitManager/Keychain.swift b/Sources/TunnelKitManager/Keychain.swift index 020d1ba..4cc323f 100644 --- a/Sources/TunnelKitManager/Keychain.swift +++ b/Sources/TunnelKitManager/Keychain.swift @@ -74,13 +74,15 @@ public class Keychain { - Parameter password: The password to set. - Parameter username: The username to set the password for. - Parameter context: An optional context. + - Returns: The reference to the password. - Throws: `KeychainError.add` if unable to add the password to the keychain. **/ - public func set(password: String, for username: String, context: String? = nil) throws { + @discardableResult + public func set(password: String, for username: String, context: String? = nil) throws -> Data { do { let currentPassword = try self.password(for: username, context: context) guard password != currentPassword else { - return + return try passwordReference(for: username, context: context) } removePassword(for: username, context: context) } catch let e as KeychainError { @@ -99,11 +101,14 @@ public class Keychain { query[kSecAttrAccount as String] = username query[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock query[kSecValueData as String] = password.data(using: .utf8) + query[kSecReturnPersistentRef as String] = true - let status = SecItemAdd(query as CFDictionary, nil) - guard status == errSecSuccess else { + var ref: CFTypeRef? + let status = SecItemAdd(query as CFDictionary, &ref) + guard status == errSecSuccess, let refData = ref as? Data else { throw KeychainError.add } + return refData } /** @@ -195,18 +200,13 @@ public class Keychain { /** Gets a password associated with a password reference. - - Parameter username: The username to get the password for. - Parameter reference: The password reference. - - Parameter context: An optional context. - - Returns: The password for the input username and reference. + - Returns: The password for the input reference. - Throws: `KeychainError.notFound` if unable to find the password in the keychain. **/ - public func password(for username: String, reference: Data, context: String? = nil) throws -> String { + public static func password(forReference reference: Data) throws -> String { var query = [String: Any]() - setScope(query: &query, context: context) - query[kSecClass as String] = kSecClassGenericPassword - query[kSecAttrAccount as String] = username - query[kSecMatchItemList as String] = [reference] + query[kSecValuePersistentRef as String] = reference query[kSecReturnData as String] = true var result: AnyObject? diff --git a/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift b/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift index 5cae71c..4928c02 100644 --- a/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift +++ b/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift @@ -217,9 +217,8 @@ open class OpenVPNTunnelProvider: NEPacketTunnelProvider { // optional credentials let credentials: OpenVPN.Credentials? if let username = protocolConfiguration.username, let passwordReference = protocolConfiguration.passwordReference { - let keychain = Keychain(group: appGroup) - guard let password = try? keychain.password(for: username, reference: passwordReference) else { - completionHandler(OpenVPNProviderConfigurationError.credentials(details: "keychain.password(for:, reference:)")) + guard let password = try? Keychain.password(forReference: passwordReference) else { + completionHandler(OpenVPNProviderConfigurationError.credentials(details: "Keychain.password(forReference:)")) return } credentials = OpenVPN.Credentials(username, password) diff --git a/Sources/TunnelKitOpenVPNManager/OpenVPNProvider+Configuration.swift b/Sources/TunnelKitOpenVPNManager/OpenVPNProvider+Configuration.swift index 5d03394..0a85fba 100644 --- a/Sources/TunnelKitOpenVPNManager/OpenVPNProvider+Configuration.swift +++ b/Sources/TunnelKitOpenVPNManager/OpenVPNProvider+Configuration.swift @@ -276,7 +276,7 @@ extension OpenVPNProvider { - Parameter bundleIdentifier: The provider bundle identifier required to locate the tunnel extension. - Parameter appGroup: The name of the app group in which the tunnel extension lives in. - Parameter context: The keychain context where to look for the password reference. - - Parameter username: The username to authenticate with. + - Parameter credentials: The credentials to authenticate with. - Returns: The generated `NETunnelProviderProtocol` object. - Throws: `OpenVPNProviderError.credentials` if unable to store `credentials.password` to the `appGroup` keychain. */ @@ -284,16 +284,18 @@ extension OpenVPNProvider { withBundleIdentifier bundleIdentifier: String, appGroup: String, context: String, - username: String?) throws -> NETunnelProviderProtocol { + credentials: OpenVPN.Credentials?) throws -> NETunnelProviderProtocol { let protocolConfiguration = NETunnelProviderProtocol() let keychain = Keychain(group: appGroup) protocolConfiguration.providerBundleIdentifier = bundleIdentifier protocolConfiguration.serverAddress = sessionConfiguration.hostname ?? resolvedAddresses?.first - if let username = username { + if let username = credentials?.username { protocolConfiguration.username = username - protocolConfiguration.passwordReference = try? keychain.passwordReference(for: username, context: context) + if let password = credentials?.password { + protocolConfiguration.passwordReference = try? keychain.set(password: password, for: username, context: context) + } } protocolConfiguration.providerConfiguration = generatedProviderConfiguration(appGroup: appGroup) diff --git a/Tests/TunnelKitOpenVPNTests/AppExtensionTests.swift b/Tests/TunnelKitOpenVPNTests/AppExtensionTests.swift index c265d4f..5517111 100644 --- a/Tests/TunnelKitOpenVPNTests/AppExtensionTests.swift +++ b/Tests/TunnelKitOpenVPNTests/AppExtensionTests.swift @@ -81,7 +81,7 @@ class AppExtensionTests: XCTestCase { withBundleIdentifier: identifier, appGroup: appGroup, context: context, - username: credentials.username + credentials: credentials ) XCTAssertNotNil(proto)