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

Application has issues with line 537 let status = SecItemCopyMatching(query as CFDictionary, &result) #373

Closed
dkalinai opened this issue Mar 5, 2018 · 12 comments
Assignees

Comments

@dkalinai
Copy link

dkalinai commented Mar 5, 2018

Each time my application opens which has a breakpoint for any errors this line stops execution for some reason:

    let status = SecItemCopyMatching(query as CFDictionary, &result)

Any ideas what may be going wrong?

@kishikawakatsumi
Copy link
Owner

@dkalinai I cannot understand what you mean and what problem occurs. Can you explain more for me?

@dkalinai
Copy link
Author

dkalinai commented Mar 5, 2018

screenshot 2018-03-05 12 53 19

I have a breakpoint on all exceptions and my app doesnt have any, but seems like when the app is initialized or started the KeychainAccess encounters a problem. Image attached. I dont quite know how to get you more details on this. I can explain only that i am trying to get two keys from keychain when app starts.

@kishikawakatsumi
Copy link
Owner

@dkalinai Can you share a reproducible sample project?

@dkalinai
Copy link
Author

Sorry for the big delay. https://www.dropbox.com/s/spr7i5zrp2glufg/KeychainBug.zip?dl=0

I believe the issue is that i need to save multiple items at once or fetch multiple items at once and it just doesnt like that.

@kishikawakatsumi
Copy link
Owner

@dkalinai Thank you for the sample project. You are misunderstanding how to set a value to the keychain.

do {
    try AuthService.shared.keychain.set(AuthService.shared.kWoocommerceConsumerSecret,
                                    key: "cs_stufff078340t74589769")
    sleep(2)
    try AuthService.shared.keychain.set(AuthService.shared.kWoocommerceConsumerKey, key: "ck_consumerKey134353453")

^ You attempt to set the value AuthService.shared.kWoocommerceConsumerSecret with the key "cs_stufff 078340 t 74589769". The follows as same.

As result, ConsumerSecret is set as a password with cs_stufff078340t74589769 key like the following.

screen shot 2018-03-29 at 5 19 21

That's why you cannot retrieve the value with kWoocommerceConsumerKey.

@dkalinai
Copy link
Author

Sorry, this was a typo on my behalf, since i was typing this code by hand in reality in my app it looks like this for saving:

do {
if newValue.isNotEmpty {
try self.keychain
.label("Consumer Secret")
.comment("Woocommerce Consumer Secret")
.set(newValue, key: kWoocommerceConsumerSecret)
}

@dkalinai
Copy link
Author

So coude for sample project should be: do {
try AuthService.shared.keychain.set("ck_consumerKey134353453", key: AuthService.shared.kWoocommerceConsumerKey)

            try AuthService.shared.keychain.set("cs_stufff078340t74589769", key: AuthService.shared.kWoocommerceConsumerSecret)
            
        } catch let error {
            print(error.localizedDescription)
        }

but even that doesnt save normally. IS there a way to pass array as value and key and save that in bulk? Maybe i am doign it wrong for several items at once?

@kishikawakatsumi kishikawakatsumi self-assigned this Mar 28, 2018
@kishikawakatsumi
Copy link
Owner

@dkalinai OK, I swap the key and the value, then both values are saved correctly. Can you make sure again following steps?

  1. Remove all breakpoints.
  2. Launch the app
  3. Launch the KeychainAccess.app
  4. Search "keychainbug"

You probably can see the saved values like the following,

screen shot 2018-03-29 at 6 28 56

@dkalinai
Copy link
Author

THe values will be saved, there is no issues with that. The issue i am having is that if i set a breakpoint for all exceptions it hits saving, or in my case on launch of the app loading keychain data. So thats what i need to understand, why does an exception break point hit the code. I keep exception breakpoints usually on, to debug issues right away. Can you see any eay i can keep them on and not hit the keychainaccess code please?

@kishikawakatsumi
Copy link
Owner

kishikawakatsumi commented Mar 29, 2018

@dkalinai

The issue i am having is that if i set a breakpoint for all exceptions it hits saving

That is an expected behavior. The exception thrown SecItemAdd(). We cannot stop that it throws exceptions internally. And it was handled within SecItemAdd(). It won't leak out. So you don't mind it.

The SecItemAdd function seems to use exceptions to control the logic (of course it is a bad practice though).

You can see a same behavior more simple case without my library, like the following (from Apple's documents https://developer.apple.com/documentation/security/keychain_services/keychain_items/adding_a_password_to_the_keychain)

        let account = "username"
        let password = "password".data(using: .utf8)!
        let query: [String: Any] = [kSecClass as String: kSecClassInternetPassword,
                                    kSecAttrAccount as String: account,
                                    kSecAttrService as String: "com.kishikawakatsumi.KeychainTest",
                                    kSecValueData as String: password]

        let status = SecItemAdd(query as CFDictionary, nil)
        print(status)

@dkalinai
Copy link
Author

Hi again,

I see what you mean. Well that's quite annoying that Apple let this thing through then :S So sorry that I originally thought it was your framework, I did actually try another one yesterday and it had the same problem supporting your idea. I am just wondering if maybe there is a workaround like make the calls not one after another when fetching or saving items via some function that takes a dictionary as parameter?

Anyways thank you for taking the time to look into this, seems like nothing can be done here... just tad annoying that when you want to debug something on startup you have to go over the Keychain 6-7 breakpoints for 2 fetches...

Thanks again for the info.

@kishikawakatsumi
Copy link
Owner

Good, thank you for reporting!

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

No branches or pull requests

2 participants