verify secure enclave keys exist in secure enclave #2116
Conversation
| // https://developer.apple.com/documentation/coreservices/1559994-anonymous/errkcitemnotfound/ | ||
| // apple site where you can search for error codes, just enter error code | ||
| // https://developer.apple.com/bugreporter/ | ||
| if strings.Contains(err.Error(), "-25300") { |
There was a problem hiding this comment.
Dont really like the string contains .... but I don't think there is a way to pull "proper go errors" out of the apple C apis. At some point in the chain I think there would have to be a string or int compare and then creation of the go error.
| @@ -215,4 +247,116 @@ func Test_secureEnclaveRunner(t *testing.T) { | |||
| // and public not called | |||
| require.Len(t, ser.uidPubKeyMap, 0) | |||
| }) | |||
|
|
|||
| t.Run("creates new key when existing not found in secure enclave", func(t *testing.T) { | |||
There was a problem hiding this comment.
Just curious -- I noticed these tests use t.Run instead of separate test functions. Is that a pattern you think we might want to adopt more widely across launcher? I like how specific the test names are without having to be like func TestFunc_someSpecificHappyOrUnhappyPath(t *testing.T).
There was a problem hiding this comment.
I just kind of tried it out and I really like it for a few reasons:
- The table driven tests can get really complicated as the the code it's testing grows and more edge cases are found
- Using VS Code I can click run on a single test, for the table driven tests I have to comment out the ones I don't want to run
- Feels easier to add or remove things from the tests, with the table driven tests, you change one field in the struct and your updating n tests
I still think table driven tests have their place and are a better choice for smaller scoped areas of code with simple inputs, outputs and few error / success paths.
Adds required plumbing and updates secure enclave runner to verify that the key exists in secure enclave when it loaded from db.
While testing I learned that you'll get a errKCItemNotFound = -25300 error if the secure enclave is unavailable for some reason, so it can't be relied on to determine if a key exists.
To test if the secure enclave is available, we can create a key, then delete it. Corresponding krypto PR to add ability to create delete key kolide/krypto#51