search

henrinormak / Heimdall

Heimdall is a wrapper around the Security framework for simple encryption/decryption operations.
MIT License
401 stars 68 forks source link

Improve AES encryption logic #37

Closed henrinormak closed 8 years ago

henrinormak commented 8 years ago

Two big changes, making sure initialisation vector is not null, indeed using random bytes that are stored along with the encrypted AES key.

Second, make sure the AES key size is determined correctly, also leaving room for the included IV in the first block.

henrinormak commented 8 years ago

This is not backwards compatible, meaning it will require bumping version to 1.0.0 to signify the fact. Too bad, as the reason for it is a :bug:, but it is the correct thing to do.

soyersoyer commented 8 years ago

If you check decryptedMetadataLength, and it is less than the keysize+ivsize, you can use a null IV. It mades it partial backward compatible (it will work when the rsa key size is the default 2048bit or bigger)

The keySize function is not completely correct, because the available size depends on the padding too. But I don't think somebody ever want to use it with a key which is less than 2048 bit.

But if you don't want any backward compatibility please consider changing the rsa padding to OAEP, because it is recommended for new applications.

henrinormak commented 8 years ago

That sounds like a reasonable plan (I mean taking the chance and switching over to OAEP), I don't particularly like the idea of having special cases for backwards compatibility (especially if it doesn't cover all of them). I will look into changing the padding (as far as I managed to read into it, it should be just about switching two enums in encrypt/decrypt).

henrinormak commented 8 years ago

Crazy typo in the last commit message, mind is totally somewhere else. Anyhow, I will likely just merge this as is and then bump version to 1.0.0

EDIT: Related to keySize, I will look into what the padding would amount to and make sure that is taken into account, sounds like something that should be done.

henrinormak commented 8 years ago

I adjusted the keySize calculation to take the padding into account. Could you go over it once again @soyersoyer? Thanks a bunch!

soyersoyer commented 8 years ago

:thumbsup:

henrinormak commented 8 years ago

Submitted 1.0.0 to CocoaPods and released on GitHub. Thanks a bunch for your help!