Add support for authenticated encryption/decryption AES-GCM

[MrMage]

Also see #59. This is mostly supported already; we'd just need to add a means of having our encrypt/decrypt method take/return additional authenticated data and the authentication tag.

[bmhatfield]

Hey there! After reading #59, and noticing that AES256GCM exists in the library, I wanted to quickly check - is this currently expected to fail?

let key = "12345678901234567890123456789012"
let iv = "123456789012"

let encrypted = try AES256GCM.encrypt("forks", key: key, iv: iv)
print(encrypted.base64EncodedString())

let decrypted = try AES256GCM.decrypt(encrypted, key: key, iv: iv)
print(decrypted.base64EncodedString())

It currently fails on the .decrypt() line with:

CryptoError.EVP_CipherFinal_ex: Failed finishing cipher.: unknown.

which seems plausible that it could be related to the discussion in #59, but may be due to some other problem on my machine.

[MrMage]

I think it's expected to fail at the moment, as decryption expects an authentication "tag" to.be provided. However, Vapor doesn't have an option to pass that tag right now, causing decryption to fail at the authentication stage. Not 100% sure about this, though. It should be fairly easy to add methods that return/take the authentication tag in addition to the ciphertext, though.

…

On 24. Jul 2018, at 16:32, Brian Hatfield ***@***.***> wrote: Hey there! After reading #59, and noticing that AES256GCM exists in the library, I wanted to quickly check - is this currently expected to fail? let key = "12345678901234567890123456789012" let iv = "123456789012" let encrypted = try AES256GCM.encrypt("forks", key: key, iv: iv) print(encrypted.base64EncodedString()) let decrypted = try AES256GCM.decrypt(encrypted, key: key, iv: iv) print(decrypted.base64EncodedString()) It currently fails on the .decrypt() line with: CryptoError.EVP_CipherFinal_ex: Failed finishing cipher.: unknown. which seems plausible that it could be related to the discussion in #59, but may be due to some other problem on my machine. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[vzsg]

The package could also prepend the tag to the ciphertext when encrypting, and split it off when decrypting.

I've heard of other libraries doing this, and even seen this in RFC 5116:

When choosing the format of application data, an application SHOULD position the ciphertext C so that it appears after any other data that is needed to construct the other inputs to the authenticated decryption operation. For instance, if the nonce and ciphertext both appear in a packet, the former value should precede the latter. This rule facilitates efficient and simple hardware implementations of AEAD algorithms.

[MrMage]

Note that nonce and tag are different things:

• nonce/initialization vector: random data to ensure that encrypting the same data twice yields different outputs. passed into the encryption and decryption algorithms
• tag: essentially a checksum computed by the encryption algorithm, letting the decryption algorithm verify the integrity of the plaintext. returned by the encryption algorithm and passed into the decryption algorithm
• additional authenticated data: extra data passed into both algorithms to ensure "that the right entity is decrypted". (See confused-deputy problem.) passed into the encryption and decryption algorithms

If we were to put the tag into the ciphertext, that should be well-documented; otherwise the encrypted data will hardly be portable to other consumers (e.g. when Vapor encrypts the data but another framework is used for decryption).

[bmhatfield]

We've got a working prototype of this locally that we're planning to put up as a PR shortly.

However, we've got a little bit of a design question for you - encrypt when working on an AEAD cipher must also return a tag, which is currently not supported by Cipher. Swift won't disambiguate a second function purely by the return type, so would you prefer:

1. A subclass of Cipher like AEADCipher that overrides encrypt (by removing the final designation of Cipher)
2. A second encrypt method that takes a tag as an inout (rather than as a second return value)
3. A second encrypt method with a different name that returns a tuple (like gcmEncrypt)?
4. Other

Thoughts?

[bmhatfield]

Regarding prepending the tag into the ciphertext: For our purposes, we'd prefer to have the tag separate, but if folks feel strongly (especially given the not-awesome tradeoffs above) that we do it that way, that's OK too.

[vzsg]

@MrMage In my understanding, the tag being passed into the decryption algorithm qualifies it to be used as a prefix – the nonce was just an example in the RFC text.

@bmhatfield I think the separate class (not subclass!) would make the most sense and the most failsafe API. A bad abstraction hurts more than a bit of code duplication.

[MrMage]

I also would prefer to have the tag separate. I also agree that a separate class would be best. My suggestion for the name would be AuthenticatedCipher.

[bmhatfield]

Okay! Thanks for the feedback! We've updated #71 with a new class, and a pair of protocols that help reduce some of the code duplication.

[bmhatfield]

@MrMage Just checking in here - #71 has been in a good state for a few days; is there anything else I should do here to see it merged? Thanks!

[MrMage]

@bmhatfield this indeed looks good from my side. Unfortunately, I can't merge PRs in this repo and was hoping for @tanner0101 to take a look and merge it. (I mostly reviewed it because I was the first to suggest the change and am invested in seeing it succeed :-) )