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

Fix macOS Access Groups #538

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rivera-ernesto
Copy link

When using Access Groups on macOS kSecUseDataProtectionKeychain needs to be set to true:

https://developer.apple.com/documentation/security/ksecusedataprotectionkeychain

Set the value for this key to true in the query dictionary when accessing a macOS keychain item that behaves like an iOS keychain item. For example, use the data protection key when adding, searching for, or deleting an item to which the kSecAttrAccessible or kSecAttrAccessGroup attributes apply.

The data protection key affects operations only in macOS. Other platforms automatically behave as if the key is set to true, and ignore the key in the query dictionary. You can safely use the key on all platforms.

Tip
It’s highly recommended that you set the value of this key to true for all keychain operations. This key helps to improve the portability of your code across platforms. Use it unless you specifically need access to items previously stored in a legacy keychain in macOS.

This pull request is a first fix for issues #438, #491 and #535

if #available(iOS 13.0, OSX 10.15, watchOS 6.0, tvOS 13.0, *) {
if accessGroup != nil {
query[UseDataProtectionKeychain] = true
}
Copy link
Author

Choose a reason for hiding this comment

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

Automatically set UseDataProtectionKeychain to true when an access group was set. This is required for macOS and doesn't affect other platforms

if #available(iOS 13.0, OSX 10.15, watchOS 6.0, tvOS 13.0, *) {
if accessGroup != nil {
attributes[UseDataProtectionKeychain] = true
}
Copy link
Author

Choose a reason for hiding this comment

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

Same thing here

if #available(iOS 13.0, OSX 10.15, watchOS 6.0, tvOS 13.0, *) {
if accessGroupsCompatible {
query[UseDataProtectionKeychain] = true
}
Copy link
Author

Choose a reason for hiding this comment

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

This code is from a class function, so we can't check if options has access groups set or not...

@@ -887,12 +887,18 @@ public final class Keychain {

// MARK:

public class func allKeys(_ itemClass: ItemClass) -> [(String, String)] {
public class func allKeys(_ itemClass: ItemClass, accessGroupsCompatible: Bool = true) -> [(String, String)] {
Copy link
Author

Choose a reason for hiding this comment

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

...so I added an optional accessGroupsCompatible that defaults to true (per Apple documentation Tip)

@rivera-ernesto
Copy link
Author

There may be other places that also require setting UseDataProtectionKeychain to true

papac25

This comment was marked as spam.

@rivera-ernesto
Copy link
Author

unapproved

Could you elaborate? 🙃

@kishikawakatsumi
Copy link
Owner

@rivera-ernesto That comment was written by an unrelated person. Please ignore it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants