Corrected mismatch between .256/.384 cases and returned algorithm values

agens-no / EllipticCurveKeyPair

Sign, verify, encrypt and decrypt using the Secure Enclave

Other

709 stars 115 forks source link

Corrected mismatch between .256/.384 cases and returned algorithm values #29

Closed dschuetz closed 6 years ago

dschuetz commented 6 years ago

The .sha256 case had returned the algorithm for SHA384AESGCM, while .sha384 returned SHA256. Switched them back.

ma-pe commented 6 years ago

Just wanted to point out, that this is a breaking change. Just stumbled upon (since I linked master instead of a release or unique commit).

hfossli commented 6 years ago

Correct. This library should have proper versioning. Are you using this library for encryption and decryption?

ma-pe commented 6 years ago

Yes, I do. Meaning that decryption stopped working after upgrading my pods.

hfossli commented 6 years ago

Cool. Interesting to know how people use this library. Are you doing encryption serverside and decryption client side or are you both decrypting and encrypting client side?

ma-pe commented 6 years ago

Both client side. I use it to securely save a local database encryption key.

The key is generated on the device, encrypted via your library (backed up by fingerprint / pincode) and stored within the app. On database decryption I decrypt the key with your library (prompting the fingerprint / pincode).

hfossli commented 6 years ago

Nice. That's awesome.

I am sorry for the inconvenience it may have caused you that I promptly just changed the default values of those enum cases on master. What would have been the proper way to fix a typo like this?

ma-pe commented 6 years ago

I don't think that you did something wrong. My comment was more: "Hey just bear in mind that this will be breaking, when releasing the next version" (though the next version will already be a 2.0 - so I guess it's just fine).

My personal issue was, that I linked the master (which of course I can't be sure of to be stable). I use the master since I need changes of one pull requests (https://github.com/agens-no/EllipticCurveKeyPair/pull/24), that isn't in a version yet.

But now I switched from linking master to link a certain commit in my Podfile. This should fix things for me in the future :)

hfossli commented 6 years ago

Yes, thanks!

I think of the following tools to changes like this could be used

Proper versioning and releases
It should probably at least be stated in release notes
Deprecate the enum and create a new one stating that the old is deprecated because of misconfiguration

ma-pe commented 6 years ago

sounds good!