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

Jump to bottom

crypto/cipher: NewGCMWithNonceSize allows zero-length nonce #37118

Closed
katiehockman opened this issue Feb 7, 2020 · 10 comments
Closed

crypto/cipher: NewGCMWithNonceSize allows zero-length nonce #37118

katiehockman opened this issue Feb 7, 2020 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.15

Comments

@katiehockman
Copy link
Contributor

cipher.NewGCMWithNonceSize allows for any nonce size, including one that is zero length. This is not allowed by NIST SP 800-38D and encrypting with such an IV leaks the authentication key.

NIST SP 800-38D:

The bit lengths of the input strings to the authenticated encryption function shall meet the
following requirements:
...
1 ≤ len(IV) ≤ 264-1

Allowing a zero-length nonce opens the package up to misuse, and there is never a valid reason to do this. It could be argued that cipher.NewGCMWithNonceSize isn't meant to be as safe, and the recommended approach is to use cipher.NewGCM, however this is a hardening measure that has no negative side effects, in my opinion.

cipher.NewGCMWithNonceSize docs:

Only use this function if you require compatibility with an existing cryptosystem that uses non-standard nonce lengths. All other users should use NewGCM, which is faster and more resistant to misuse.

/cc @FiloSottile
@katiehockman katiehockman added this to the Go1.15 milestone Feb 7, 2020
@katiehockman
Copy link
Contributor Author

@FiloSottile suggested that we backport this fix as well.

@katiehockman katiehockman added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 7, 2020
@katiehockman katiehockman self-assigned this Feb 7, 2020
@katiehockman katiehockman added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Feb 7, 2020
@gopherbot
Copy link

Change https://golang.org/cl/218500 mentions this issue: crypto/cipher: require non-zero nonce size for NewGCMWithNonceSize

@networkimprov
Copy link

@gopherbot please open backport, at suggestion of @FiloSottile.

This was referenced Feb 24, 2020
Closed
Closed
@gopherbot
Copy link

gopherbot commented Feb 24, 2020 •
edited by dmitshur

Backport issue(s) opened: #37416 (for 1.14), #37417 (for 1.13), #37418 (for 1.12).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@networkimprov
Copy link

@dmitshur, also needs 1.14 backport...

@dmitshur
Copy link
Contributor

dmitshur commented Feb 24, 2020 •
edited

I'll repurpose the 1.12 one. It won't be getting this backport because 1.14 will be out by then, so it'll be too old.

@dmitshur dmitshur mentioned this issue Feb 24, 2020
Closed
@networkimprov
Copy link

Backport issue for 1.12 is now #37418

@gopherbot
Copy link

Change https://golang.org/cl/220651 mentions this issue: [release-branch.go1.14] crypto/cipher: require non-zero nonce size for AES-GCM

gopherbot pushed a commit that referenced this issue Feb 24, 2020
@katiehockman @dmitshur
[release-branch.go1.14] crypto/cipher: require non-zero nonce size fo…
f63e55b 
…r AES-GCM

Also fix typo in crypto/cipher/gcm_test.go.

Updates #37118
Fixes #37416

Change-Id: I8544d1eeeb1f0336cebb977b8c5bfa5e4c5ad8c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/218500
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit 4e8badb)
Reviewed-on: https://go-review.googlesource.com/c/go/+/220651
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/220653 mentions this issue: [release-branch.go1.13] crypto/cipher: require non-zero nonce size for AES-GCM

@dmitshur
Copy link
Contributor

/cc @dunhamsteve FYI.

gopherbot pushed a commit that referenced this issue Feb 26, 2020
@katiehockman
[release-branch.go1.13] crypto/cipher: require non-zero nonce size fo…
6f57b10 
…r AES-GCM

Also fix typo in crypto/cipher/gcm_test.go.

Updates #37118
Fixes #37417

Change-Id: I8544d1eeeb1f0336cebb977b8c5bfa5e4c5ad8c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/218500
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit 4e8badb)
Reviewed-on: https://go-review.googlesource.com/c/go/+/220653
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@amasover amasover mentioned this issue Mar 23, 2020
Closed
@golang golang locked and limited conversation to collaborators Feb 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.15
Development

No branches or pull requests
4 participants
@networkimprov @dmitshur @gopherbot @katiehockman