agens-no / EllipticCurveKeyPair

Sign, verify, encrypt and decrypt using the Secure Enclave
Other
708 stars 114 forks source link

Better support for `operationPrompt` #18

Open hfossli opened 6 years ago

hfossli commented 6 years ago

Continuing the discussion from #17.

I think it is a good idea, but I am still conflicted. I don't want to make the API and code more complicated than it is.

I would love to just use the LAContext under the hood, but sadly the localizedReason property was introduced on iOS 11... Thus forcing us to instead send operationPrompt parameter all the way down to the query.

Example of what's needed to change.

Current code

public final class Manager {

    ...1

    public func privateKey(context: LAContext? = nil) throws -> PrivateKey {
        ...2
    }

    enum Prompt {
        case context(LAContext)
        case string(String)
    }

    public final class Manager {

        ...1

        public func privateKey(context: LAContext? = nil) throws -> PrivateKey {    
            return privateKey(prompt: .context(context))
        }

        public func privateKey(operationPrompt: String) throws -> PrivateKey {    
            return privateKey(prompt: .string(operationPrompt))
        }

        private func privateKey(prompt: Prompt) throws -> PrivateKey {    
            ...2 
            - let key = try helper.getPrivateKey(context: context)
            + let key = try helper.getPrivateKey(prompt: prompt)
        }
wuf810 commented 6 years ago

I've had to do the same in my own implementation i.e. use operationPrompt pre iOS 11.x

I don't think you currently have any other choice and although it's not so nice, it is a necessary evil for the moment.

hfossli commented 6 years ago

Yep. Thanks for chiming in.

One additional constraint is that we don’t want to query the private key excessively for devices that doesnt support the access control flag «privateKeyUsage».

alkalim commented 6 years ago

Looks like a reasonable solution. I'm not sure if you implied this or not but this enum could also be utilized at the API level?

let config = EllipticCurveKeyPair.Config(
...
                operationPrompt: EllipticCurveKeyPair.Prompt.string("Message"),
                // or EllipticCurveKeyPair.Prompt.context(context)
...)
hfossli commented 6 years ago

Yes, it could. Users could also write the shorter version (omitting EllipticCurveKeyPair.Prompt)

let config = EllipticCurveKeyPair.Config(
...
                operationPrompt: .string("Message"),
...)

since the type is known.

I was looking for non-breaking changes, but it seems like a good idea to do a breaking change in order to get this right.