search

sfackler / rust-openssl

OpenSSL bindings for Rust
1.37k stars 739 forks source link

Why is AES using GCM not supported? #326

Closed jimmycuadra closed 7 years ago

jimmycuadra commented 8 years ago

In the openssl crate, crypto::symm::Type has AES_128_GCM and AES_256_GCM commented out. The openssl-sys crate has the supporting functions (e.g. EVP_aes_128_gcm) commented out as well. Is there a reason these modes and functions were not included when the others were?

sfackler commented 8 years ago

The crypto side of the crate was inherited. I do not know why the GCM types would be disabled.

kcking commented 8 years ago

One reason could be that Crypter::finalize currently does not handle the case of a bad decryption (the return value of EVP_CipherFinal needs to be checked according to https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption#Authenticated_Decryption_using_GCM_mode). The question is what should be returned in the event of a failed decryption. The return type could be changed to a Result, but then we might want to make a new Authenticated Cryptor type to avoid breaking the api of Cryptor.

I would find GCM being enabled to be useful. Do you have thoughts @sfackler ?

sfackler commented 8 years ago

Yeah, I would definitely be on board with supporting it. We could panic if decryption fails for now, I suppose, though that sounds a bit dubious. Alternatively we could build out the functionality on the breaks branch and have it land in the next major version of rust-openssl. I'm planning on releasing one shortly after Rust 1.9 lands to enable panic propagation logic for the SSL implementation so it's not all that far off.

DemiMarie commented 8 years ago

ChaCha20-Poly1305 also needs to be supported.

orthecreedence commented 8 years ago

Hey, everyone. Fairly new to Rust, but thought I'd weigh in. After evaluating all the crypto libraries out there, their portability to different platforms, and which ones actually have Rust wrappers, I'm fairly convinced that adding AES-GCM support would make this lib the most complete (and yes, ChaCha20-Poly1305 too, but let's stay on topic).

I dove into the internals a bit last night (had to write my own Ansix923 padding so ended up using the Crypter directly w/ .pad(false)). After poking around, I think having an AuthenticatedCrypter that returns a Result would be an excellent way of handling this, as opposed to panicking or breaking the Crypter API.

Has there been any progress here, or would anyone mind if I took a stab at it? To be very up front, I'm not an expert in implementing or coding with crypto, nor am I very well-versed in Rust. However, I have worked with AES-GCM in several different libraries so I have a decent understanding of how it works, and I could map that understanding to the OpenSSL API while following rust-openssl's conventions.

orthecreedence commented 8 years ago

Oh, just saw this: https://github.com/sfackler/rust-openssl/pull/173. Did any of this get merged anywhere I can check out? Or are there any plans to merge?