Random crash while storing a string in keychain

[gianfilippocisternino]

• Library setup method: CocoaPods
• Version of the library: 8.0.
• Xcode version: 10.1.
• OS version. iOS 11-12

Hello, some users of my app are facing a random crash while setting a string in the keychain.

I found only this few lines inside the crashlytics log:

#9. Crashed: com.apple.root.default-qos
[...]
25 KeychainSwift                  0x10513e408 $S13KeychainSwiftAAC6deleteySbSSF + 636
26 KeychainSwift                  0x10513d664 $S13KeychainSwiftAAC3set_6forKey10withAccessSb10Foundation4DataV_SSAA0abG7OptionsOSgtF + 108
27 KeychainSwift                  0x10513db20 $S13KeychainSwiftAAC3set_6forKey10withAccessS2b_SSAA0abG7OptionsOSgtF + 220

This crash affected only 0,1% of my app users, so i can't figure it out which could be the cause.

Help me to investigate, I'd really appreciate it, Thanks

[evgenyneu]

Thanks for reporting. When is the key chain accessed (at app launch, button click, network response, background process)? Is it done on the main thread?

[ahmadsallam]

@evgenyneu hello, i read the issue and i have some consideration about it, you mention above in question that if we access the key chain on the main thread, so is that wrong that if we access the key chain in background thread isn't safe and may make crashes ? I recently use this pod to encrypt some info in App and i wanna make sure my implementation is ok. for my case i access the keyChainSwift after network response using Alamofire library so is that ok or i just put the accessing code in main thread for avoiding any crash ?

Sorry for bad english Many Thanks :)

[evgenyneu]

so is that wrong that if we access the key chain in background thread isn't safe and may make crashes ?

I think it's fine to use Keychain from a background thread. There could be potential issues if one is accessing Keychain in parallel form two threads. We had people reporting crashes related to concurrency in the past.

[gianfilippocisternino]

@evgenyneu The keychain is accessed after a network call on the default-qos thread. As i can see from the craslytics log, the keychain is accessed once at time on that thread.

[evgenyneu]

@gianfilippocisternino, that's good to know. Honestly, I have no idea what's causing the crashes. If you have not done it already, I would try a different Keychain library or use the Keychain API directly. Sorry.

[gianfilippocisternino]

@evgenyneu i noticed in another crashlytics log that keychain was accessed in two threads at the same time. The code that caused the crash was this: KeychainHelper.shared.set(aValue, forKey: KeychainHelper.Keys.valueA.rawValue) KeychainHelper.shared.set(bValue, forKey: KeychainHelper.Keys.valueB.rawValue)

KeychainHelper is just a wrapper class that shares a single instance of KeychainSwift. Is the set function asynchronous and may cause crash if used too quickly in consecutive instructions? In that case i could use a semaphore to deny parallel access to the keychain.

Thank you for the support.

[evgenyneu]

Is the set function asynchronous and may cause crash if used too quickly in consecutive instructions?

The set methods have no locking that prevents them from being executed in parallel.

In that case i could use a semaphore to deny parallel access to the keychain.

Yes, I think it would be a good thing to try.

[gonzula]

I got a (probably) related issue, my app crashed only in the hands of the AppStore review team,

The relevant part of the crashlog is this:

Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed:
0   libsystem_kernel.dylib        	0x00000001c469e0f4 mach_msg_trap + 8
1   libsystem_kernel.dylib        	0x00000001c469d5a0 mach_msg + 72
2   libdispatch.dylib             	0x00000001c4503880 _dispatch_mach_send_and_wait_for_reply + 500
3   libdispatch.dylib             	0x00000001c4503d10 dispatch_mach_send_with_result_and_wait_for_reply$VARIANT$mp + 52
4   libxpc.dylib                  	0x00000001c4762a04 xpc_connection_send_message_with_reply_sync + 204
5   Security                      	0x00000001c5773edc securityd_message_with_reply_sync + 96
6   Security                      	0x00000001c577445c securityd_send_sync_and_do + 80
7   Security                      	0x00000001c57c9b90 __SecItemDelete_block_invoke_2 + 248
8   Security                      	0x00000001c57c92f4 __SecItemAuthDoQuery_block_invoke + 312
9   Security                      	0x00000001c57c7c60 SecItemAuthDo + 124
10  Security                      	0x00000001c57c85f4 SecItemAuthDoQuery + 504
11  Security                      	0x00000001c57c6284 SecOSStatusWith + 48
12  Security                      	0x00000001c57c8be0 SecItemDelete + 448
13  KeychainSwift                 	0x0000000100ba6cd4 KeychainSwift.delete(_:) + 27860 (KeychainSwift.swift:232)

This occurred twice in a row (both crashlogs are identical), and was called in the viewDidLoad of the first ViewController, so basically at app launch. The key that the app was trying to delete probably didn't exist.

I've tested in all of my devices and simulators and got not crash whatsoever.

Not sure if is the same issue, if you prefer I can create another one

[evgenyneu]

@gonzula thanks for letting us know. I don't know why it happens, sorry.

The key that the app was trying to delete probably didn't exist.

Yes, this call seems to be causing the crash. Very strange.

[ALexanderLonsky]

@gonzula did you happen to manage the issue? We have the same, only AppStore review team gets crashes #13 0x0000000101c9f42c in KeychainSwift.delete(_:) at /Work/PartyPoker/Sources/iOS/iOS-app/Pods/KeychainSwift/Sources/KeychainSwift.swift:217

[gonzula]

Yeah, I solved it with a wrapper around the delete method

private static func delete(_ key: String) {
    guard KeychainSwift().get(key) != nil else {return}

    KeychainSwift().delete(key)
}

Since then I had no problems

[gonzula]

@evgenyneu What do you think about adding this approach to the repo?

Something like

  @discardableResult
  open func delete(_ key: String) -> Bool {
    // The lock prevents the code to be run simlultaneously
    // from multiple threads which may result in crashing
    lock.lock()
    defer { lock.unlock() }

    guard get(key) != nil else {return true}    

    return deleteNoLock(key)
  }

Do you know something about timing overhead? Or if more people got this issue?

[ALexanderLonsky]

@gonzula thanks for the answer! I did the same and my app is already in the store :)

[evgenyneu]

@gonzula, thanks for the info. Inside the set method, we call delete to remove the key first. This means that it is safe to call delete if the key does not exist. I would like to understand why it crashes on delete before adding the code that checks if the item already exists.

[gonzula]

Yeah, makes sense. One way or another the app will always end up deleting non existing keys.

[evgenyneu]

@gonzula, maybe deleting an item during app startup causes a crash because Keychain is not ready for that. So at this stage, it allows to read items but not delete them? I'm just guessing.