Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCM: Allow nonces of any length #62

Closed
oberien opened this issue Dec 6, 2019 · 12 comments
Closed

GCM: Allow nonces of any length #62

oberien opened this issue Dec 6, 2019 · 12 comments

Comments

@oberien
Copy link

oberien commented Dec 6, 2019

Currently, AesGcm only allows nonces with a length of 12 bytes. However, the the AES-GCM specification allows nonces of any length:

screenshot20191206010437

Is there a way to use AesGcm with a nonce of a length ≠ 12 bytes?

@tarcieri
Copy link
Member

tarcieri commented Dec 6, 2019

The Aead trait presently doesn't support variable-size nonces. That's an issue I raised here:

That said, you almost certainly want to use a 96-bit nonce with AES-GCM. Notice what it actually does if the nonce isn't 96-bits: much like something like HMAC, it tolerates an "unusual sized" nonce via a special computation for this case. My understanding is there are some issues with it which make it somewhat undesirable versus specifying your own 96-bit nonce, but I don't have a citation offhand.

This post covers some of the tradeoffs:

https://crypto.stackexchange.com/posts/66500/revisions

If you're really concerned with nonces, may I perhaps suggest AES-GCM-SIV?

Otherwise, the only real argument for other nonce sizes for AES-GCM is protocols which have (mis)specified a different nonce size, e.g. Apple Pay's use of 128-bit nonces. I think it would be ok to add some specific nonce sizes used in protocols like this for interoperability in weird cases like this, with a note that a 96-bit nonce should be preferred.

@oberien
Copy link
Author

oberien commented Dec 6, 2019

My use-case is interoperability with a Java implementation using BouncyCastle. The Java implementation uses variable-length nonces (for whatever weird reason). As such having preconfigured fixed lengths for some protocols doesn't solve my use-case directly. I could imagine a special trait or just method for GCM and similar algorithms allowing variable-length nonces, which takes the nonce as &[u8]. That somewhat breaks the whole idea of using GenericArray and the type-level constants. However, it only affects that single parameter. All other type-level constants stay the same, because no other length of internal state or the produced result changes (for GCM at least). Only the initization of the internal state differs.

For now I'll use openssl.

@tarcieri
Copy link
Member

tarcieri commented Dec 6, 2019

I'd suggest opening an issue on https://github.com/RustCrypto/traits requesting variable length nonce support as part of the AEAD trait. You can reference my comment here:

https://github.com/RustCrypto/traits/pull/40/files/a5db8f2d8e07593883e7e6aba27eb4e9d54b9460#r307931303

@tarcieri
Copy link
Member

So it seems I may have been a bit hasty to suggest that we need to modify the Aead trait.

Non-standard-sized GCM nonces work a bit like XChaCha20/XSalsa20 vs ChaCha20/Salsa20: you compute a standard 96-bit sized nonce from a longer one.

So rather than dealing with this in the trait, it can have separate encrypt/decrypt methods that take non-standard-sized nonces. I'm not sure what to call those exactly... encrypt_nonstandard_nonce or something but that sounds bad.

Anyway, it should be possible to implement those methods in terms of encrypt/decrypt, which avoids having to change the Aead trait.

That said, RustCrypto/traits#65 (i.e. fixing this upstream in the trait) is still worth considering.

@tarcieri
Copy link
Member

FYI, prospective PR to add support for non-96-bit nonces in #126.

I ended up making AesGcm generic around Aead::NonceSize, which seems to work great IMO.

@tarcieri
Copy link
Member

Fixed in #126

@tarcieri
Copy link
Member

Now released in aes-gcm v0.5.0:

https://docs.rs/aes-gcm/0.5.0/aes_gcm/

@kartonrad
Copy link

How does this work?
Looking though the documentation there doesn't seem to be a way to use a &[u8] or even &[u8; 16] for en/decryption

@tarcieri
Copy link
Member

@drkslv the AesGcm type is generic over a nonce size:

https://docs.rs/aes-gcm/latest/aes_gcm/struct.AesGcm.html

You'll still need to specify the nonce size at compile time, currently using a typenum-based parameter

@kartonrad
Copy link

@tarcieri Thank you very much! I had already figured it out myself by looking at your tests!
Before i asked the question i was hopelessly down the wrong path and looking at the wrong things, and ofc the moment i ask i find the solution xD

what i did to make it compile, for reference:

type Aes256GcmWith16BitNonce = AesGcm<Aes256, typenum::U16>;
    
let key = GenericArray::clone_from_slice(&master_key);
let cipher = Aes256GcmWith16BitNonce::new(&key);
cipher.decrypt_in_place( GenericArray::from_slice(&iv), &[],  &mut ciphertext)
    .expect("decrytion_failure");

also, at some point it might be worth the time to convert the code to actual const-generics, as it would allow people to just pass in the fixed-size array and have it automatically detect what constant to use
(but its not urgent by anyy means, this works fine once you figure it out)

@tarcieri
Copy link
Member

We'd need to change the aead crate to use const generics to make that work, and we can't do that yet because the nonce size is an associated type/constant of the AeadCore trait, which isn't yet supported by const generics.

See the tracking issue here: RustCrypto/traits#970

@kartonrad
Copy link

yeah const-generics are a little fresh out of the oven right now -
you quickly run into their limitations, in my case its often generic_const_exprs

really looking forward to when they're fully stable and adopted tho- it's just so much cleaner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants