Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android alignment #25

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
6 changes: 3 additions & 3 deletions Sources/SpeziLocalStorage/LocalStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import SpeziSecureStorage
/// - ``delete(storageKey:)``
public final class LocalStorage: Module, DefaultInitializable, EnvironmentAccessible, @unchecked Sendable {
private let encryptionAlgorithm: SecKeyAlgorithm = .eciesEncryptionCofactorX963SHA256AESGCM
@Dependency private var secureStorage = SecureStorage()
@Dependency private var keyStorage = KeyStorage()


private var localStorageDirectory: URL {
Expand Down Expand Up @@ -145,7 +145,7 @@ public final class LocalStorage: Module, DefaultInitializable, EnvironmentAccess


// Determine if the data should be encrypted or not:
guard let keys = try settings.keys(from: secureStorage) else {
guard let keys = try settings.keys(from: keyStorage) else {
// No encryption:
try data.write(to: fileURL)
try setResourceValues()
Expand Down Expand Up @@ -225,7 +225,7 @@ public final class LocalStorage: Module, DefaultInitializable, EnvironmentAccess
let data = try Data(contentsOf: fileURL)

// Determine if the data should be decrypted or not:
guard let keys = try settings.keys(from: secureStorage) else {
guard let keys = try settings.keys(from: keyStorage) else {
return try decoding(data)
}

Expand Down
33 changes: 15 additions & 18 deletions Sources/SpeziLocalStorage/LocalStorageSetting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,16 @@ public enum LocalStorageSetting {
/// Unencrypted
case unencrypted(excludedFromBackup: Bool = true)
/// Encrypted using a `eciesEncryptionCofactorX963SHA256AESGCM` key: private key for encryption and a public key for decryption.
case encrypted(privateKey: SecKey, publicKey: SecKey, excludedFromBackup: Bool = true)
case encrypted(keys: SecKeyPair, excludedFromBackup: Bool = true)
/// Encrypted using a `eciesEncryptionCofactorX963SHA256AESGCM` key stored in the Secure Enclave.
case encryptedUsingSecureEnclave(userPresence: Bool = false)
/// Encrypted using a `eciesEncryptionCofactorX963SHA256AESGCM` key stored in the Keychain.
case encryptedUsingKeyChain(userPresence: Bool = false, excludedFromBackup: Bool = true)


var excludedFromBackup: Bool {
switch self {
case let .unencrypted(excludedFromBackup),
let .encrypted(_, _, excludedFromBackup),
let .encrypted(_, excludedFromBackup),
let .encryptedUsingKeyChain(_, excludedFromBackup):
return excludedFromBackup
case .encryptedUsingSecureEnclave:
Expand All @@ -35,31 +34,29 @@ public enum LocalStorageSetting {
}


func keys(from secureStorage: SecureStorage) throws -> (privateKey: SecKey, publicKey: SecKey)? {
func keys(from keyStorage: KeyStorage) throws -> SecKeyPair? {
let secureStorageScope: SecureStorageScope
switch self {
case .unencrypted:
return nil
case let .encrypted(privateKey, publicKey, _):
return (privateKey, publicKey)
case let .encrypted(keys, _):
return keys
case let .encryptedUsingSecureEnclave(userPresence):
secureStorageScope = .secureEnclave(userPresence: userPresence)
case let .encryptedUsingKeyChain(userPresence, _):
secureStorageScope = .keychain(userPresence: userPresence)
}

let tag = "LocalStorage.\(secureStorageScope.id)"

if let privateKey = try? secureStorage.retrievePrivateKey(forTag: tag),
let publicKey = try? secureStorage.retrievePublicKey(forTag: tag) {
return (privateKey, publicKey)
}

let privateKey = try secureStorage.createKey(tag)
guard let publicKey = try secureStorage.retrievePublicKey(forTag: tag) else {
throw LocalStorageError.encryptionNotPossible
}

return (privateKey, publicKey)
return try (try? keyStorage.retrieveKeyPair(forTag: tag))
?? keyStorage.create(tag)
}
}

extension LocalStorageSetting {
/// Encrypted using a `eciesEncryptionCofactorX963SHA256AESGCM` key: private key for encryption and a public key for decryption.
@available(*, deprecated, renamed: "encrypted(keys:excludedFromBackup:)")
public static func encrypted(privateKey: SecKey, publicKey: SecKey, excludedFromBackup: Bool = true) -> LocalStorageSetting {
.encrypted(keys: (privateKey, publicKey), excludedFromBackup: excludedFromBackup)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
// SPDX-License-Identifier: MIT
//

/// A user's credential containing username and password.
@available(*, deprecated, renamed: "Credential")
public typealias Credentials = Credential

/// A pair of username and password credentials.
public struct Credentials: Equatable, Identifiable {
/// A user's credential containing username and password.
public struct Credential: Equatable, Identifiable {
/// The username.
public var username: String
/// The password.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider adding server parameter as well? It is not a breaking change if you initialize it with null

Copy link
Author

@pauljohanneskraft pauljohanneskraft Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the only change that would kind of force us to make a major-version update... Is it really worth it?

Alternatively, we could have those deprecation functions with the server-property being explicit and then we override in the deprecated function just to keep the minor version update. But what behavior should we choose there? Keeping the old struct with the slightly different name but without the server-property, always overriding the struct-property with the function parameter or doing something like server ?? credential.server or the other way around?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind, changed it and deprecated SecureStorage as a whole. We now don't even have SecureStorage on Android anymore, since it isn't needed and just builds a Façade that isn't really necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would make sense to ensure that we don't have to take a breaking change for now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But soft-deprecation is fine, right? Like I still kept the implementation of SecureStorage on iOS, but marked the whole class as deprecated. So, one can still use it as-is, but internally we are pretty much just calling the new functions instead.

Credential now has a server property, but if you use the deprecated functions, we internally only use the function parameter and ignore what was set in the Credential struct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly; that would be ideal. Once we tag a new major version we can remove all these deprecated function implementations.

Sounds like a good plan!

Expand All @@ -21,7 +24,7 @@ public struct Credentials: Equatable, Identifiable {
}


/// Create new credentials.
/// Create new credential.
/// - Parameters:
/// - username: The username.
/// - password: The password.
Expand All @@ -32,4 +35,4 @@ public struct Credentials: Equatable, Identifiable {
}


extension Credentials: Sendable {}
extension Credential: Sendable {}
255 changes: 255 additions & 0 deletions Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,255 @@
//
// This source file is part of the Stanford Spezi open-source project
//
// SPDX-FileCopyrightText: 2022 Stanford University and the project authors (see CONTRIBUTORS.md)
//
// SPDX-License-Identifier: MIT
//

import Foundation
import Spezi

public final class CredentialStorage: Module, DefaultInitializable, EnvironmentAccessible, Sendable {
public required init() {}

/// Stores credentials in the Keychain.
///
/// ```swift
/// do {
/// let serverCredentials = Credentials(
/// username: "user",
/// password: "password"
/// )
/// try secureStorage.store(
/// credentials: serverCredentials,
/// server: "stanford.edu",
/// storageScope: .keychainSynchronizable
/// )
///
/// // ...
///
/// } catch {
/// // Handle creation error here.
/// // ...
/// }
/// ```
///
/// - Parameters:
/// - credentials: The ``Credentials`` stored in the Keychain.
/// - server: The server associated with the credentials.
/// - removeDuplicate: A flag indicating if any existing key for the `username` and `server`
/// combination should be overwritten when storing the credentials.
/// - storageScope: The ``SecureStorageScope`` of the stored credentials.
/// The ``SecureStorageScope/secureEnclave(userPresence:)`` option is not supported for credentials.
public func store(
_ credential: Credential,
server: String? = nil,
removeDuplicate: Bool = true,
storageScope: SecureStorageScope = .keychain
) throws {
// This method uses code provided by the Apple Developer documentation at
// https://developer.apple.com/documentation/security/keychain_services/keychain_items/adding_a_password_to_the_keychain.

assert(!(.secureEnclave ~= storageScope), "Storing of keys in the secure enclave is not supported by Apple.")

var query = queryFor(credential.username, server: server, accessGroup: storageScope.accessGroup)
query[kSecValueData as String] = Data(credential.password.utf8)

if case .keychainSynchronizable = storageScope {
query[kSecAttrSynchronizable as String] = true
} else if let accessControl = try storageScope.accessControl {
query[kSecAttrAccessControl as String] = accessControl
}

Check warning on line 62 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L61-L62

Added lines #L61 - L62 were not covered by tests

do {
try SecureStorageError.execute(SecItemAdd(query as CFDictionary, nil))
} catch let SecureStorageError.keychainError(status) where status == -25299 && removeDuplicate {
try delete(credential.username, server: server)
try store(credential, server: server, removeDuplicate: false)
}
}

/// Delete existing credentials stored in the Keychain.
///
/// ```swift
/// do {
/// try secureStorage.deleteCredentials(
/// "user",
/// server: "spezi.stanford.edu"
/// )
/// } catch {
/// // Handle deletion error here.
/// // ...
/// }
/// ```
///
/// Use to ``deleteAllCredentials(itemTypes:accessGroup:)`` delete all existing credentials stored in the Keychain.
///
/// - Parameters:
/// - username: The username associated with the credentials.
/// - server: The server associated with the credentials.
/// - accessGroup: The access group associated with the credentials.
public func delete(_ username: String, server: String? = nil, accessGroup: String? = nil) throws {
let query = queryFor(username, server: server, accessGroup: accessGroup)

try SecureStorageError.execute(SecItemDelete(query as CFDictionary))
}

/// Delete all existing credentials stored in the Keychain.
/// - Parameters:
/// - itemTypes: The types of items.
/// - accessGroup: The access group associated with the credentials.
public func deleteAll(types itemTypes: SecureStorageItemTypes = .all, accessGroup: String? = nil) throws {
for kSecClassType in itemTypes.kSecClass {
do {
var query: [String: Any] = [kSecClass as String: kSecClassType]
// Only append the accessGroup attribute if the `CredentialsStore` is configured to use KeyChain access groups
if let accessGroup {
query[kSecAttrAccessGroup as String] = accessGroup
}

Check warning on line 109 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L108-L109

Added lines #L108 - L109 were not covered by tests

// Use Data protection keychain on macOS
#if os(macOS)
query[kSecUseDataProtectionKeychain as String] = true

Check warning on line 113 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L113

Added line #L113 was not covered by tests
#endif

try SecureStorageError.execute(SecItemDelete(query as CFDictionary))
} catch SecureStorageError.notFound {
// We are fine it no keychain items have been found and therefore non had been deleted.
continue
} catch {
print(error)

Check warning on line 121 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L121

Added line #L121 was not covered by tests
}
}
}

/// Update existing credentials found in the Keychain.
///
/// ```swift
/// do {
/// let newCredentials = Credentials(
/// username: "user",
/// password: "newPassword"
/// )
/// try secureStorage.updateCredentials(
/// "user",
/// server: "stanford.edu",
/// newCredentials: newCredentials,
/// newServer: "spezi.stanford.edu"
/// )
/// } catch {
/// // Handle update error here.
/// // ...
/// }
/// ```
///
/// - Parameters:
/// - username: The username associated with the old credentials.
/// - server: The server associated with the old credentials.
/// - newCredentials: The new ``Credentials`` that should be stored in the Keychain.
/// - newServer: The server associated with the new credentials.
/// - removeDuplicate: A flag indicating if any existing key for the `username` of the new credentials and `newServer`
/// combination should be overwritten when storing the credentials.
/// - storageScope: The ``SecureStorageScope`` of the newly stored credentials.
public func update( // swiftlint:disable:this function_default_parameter_at_end
// The server parameter belongs to the `username` and therefore should be located next to the `username`.
_ username: String,
server: String? = nil,
newCredential: Credential,
newServer: String? = nil,
removeDuplicate: Bool = true,
storageScope: SecureStorageScope = .keychain
) throws {
try delete(username, server: server)
try store(newCredential, server: newServer, removeDuplicate: removeDuplicate, storageScope: storageScope)
}

/// Retrieve existing credentials stored in the Keychain.
///
/// ```swift
/// guard let serverCredentials = secureStorage.retrieveCredentials("user", server: "stanford.edu") else {
/// // Handle errors here.
/// }
///
/// // Use the credentials
/// ```
///
/// Use ``retrieveAllCredentials(forServer:accessGroup:)`` to retrieve all existing credentials stored in the Keychain for a specific server.
///
/// - Parameters:
/// - username: The username associated with the credentials.
/// - server: The server associated with the credentials.
/// - accessGroup: The access group associated with the credentials.
/// - Returns: Returns the credentials stored in the Keychain identified by the `username`, `server`, and `accessGroup`.
public func retrieve(_ username: String, server: String? = nil, accessGroup: String? = nil) throws -> Credential? {
try retrieveAll(forServer: server, accessGroup: accessGroup)
.first { $0.username == username }
}

/// Retrieve all existing credentials stored in the Keychain for a specific server.
/// - Parameters:
/// - server: The server associated with the credentials.
/// - accessGroup: The access group associated with the credentials.
/// - Returns: Returns all existing credentials stored in the Keychain identified by the `server` and `accessGroup`.
public func retrieveAll(forServer server: String? = nil, accessGroup: String? = nil) throws -> [Credential] {
// This method uses code provided by the Apple Developer documentation at
// https://developer.apple.com/documentation/security/keychain_services/keychain_items/searching_for_keychain_items

var query: [String: Any] = queryFor(nil, server: server, accessGroup: accessGroup)
query[kSecMatchLimit as String] = kSecMatchLimitAll
query[kSecReturnAttributes as String] = true
query[kSecReturnData as String] = true

var item: CFTypeRef?
do {
try SecureStorageError.execute(SecItemCopyMatching(query as CFDictionary, &item))
} catch SecureStorageError.notFound {
return []
}

guard let existingItems = item as? [[String: Any]] else {
throw SecureStorageError.unexpectedCredentialsData

Check warning on line 211 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L211

Added line #L211 was not covered by tests
}

return existingItems.compactMap { existingItem in
guard let passwordData = existingItem[kSecValueData as String] as? Data,
let password = String(data: passwordData, encoding: String.Encoding.utf8),
let account = existingItem[kSecAttrAccount as String] as? String else {
return nil

Check warning on line 218 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L218

Added line #L218 was not covered by tests
}

return Credential(username: account, password: password)
}
}

private func queryFor(_ account: String?, server: String?, accessGroup: String?) -> [String: Any] {
// This method uses code provided by the Apple Developer documentation at
// https://developer.apple.com/documentation/security/keychain_services/keychain_items/using_the_keychain_to_manage_user_secrets

var query: [String: Any] = [:]
if let account {
query[kSecAttrAccount as String] = account
}

// Only append the accessGroup attribute if the `CredentialsStore` is configured to use KeyChain access groups
if let accessGroup {
query[kSecAttrAccessGroup as String] = accessGroup
}

Check warning on line 237 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L236-L237

Added lines #L236 - L237 were not covered by tests

// Use Data protection keychain on macOS
#if os(macOS)
query[kSecUseDataProtectionKeychain as String] = true

Check warning on line 241 in Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziSecureStorage/Credentials/CredentialStorage.swift#L241

Added line #L241 was not covered by tests
#endif

// If the user provided us with a server associated with the credentials we assume it is an internet password.
if server == nil {
query[kSecClass as String] = kSecClassGenericPassword
} else {
query[kSecClass as String] = kSecClassInternetPassword
// Only append the server attribute if we assume the credentials to be an internet password.
query[kSecAttrServer as String] = server
}

return query
}
}
Loading
Loading