[go] crypto/cipher: Support unusual GCM nonce lengths

[Carl Jackson (Gerrit)]

Carl Jackson uploaded a change:
https://go-review.googlesource.com/8946

crypto/cipher: Support unusual GCM nonce lengths

GCM is traditionally used with a 96-bit nonce, but the standard allows
for nonces of any size. Non-standard nonce sizes are required in some
protocols, so add support for them in crypto/cipher's GCM
implementation.

Change-Id: I7feca7e903eeba557dcce370412b6ffabf1207ab
---
M src/crypto/cipher/gcm.go
M src/crypto/cipher/gcm_test.go
2 files changed, 52 insertions(+), 14 deletions(-)



diff --git a/src/crypto/cipher/gcm.go b/src/crypto/cipher/gcm.go
index bdafd85..55d0f9d 100644
--- a/src/crypto/cipher/gcm.go
+++ b/src/crypto/cipher/gcm.go
@@ -103,16 +103,10 @@
}

func (g *gcm) Seal(dst, nonce, plaintext, data []byte) []byte {
- if len(nonce) != gcmNonceSize {
- panic("cipher: incorrect nonce length given to GCM")
- }
-
ret, out := sliceForAppend(dst, len(plaintext)+gcmTagSize)

- // See GCM spec, section 7.1.
var counter, tagMask [gcmBlockSize]byte
- copy(counter[:], nonce)
- counter[gcmBlockSize-1] = 1
+ g.deriveCounter(nonce, &counter)

g.cipher.Encrypt(tagMask[:], counter[:])
gcmInc32(&counter)
@@ -126,20 +120,14 @@
var errOpen = errors.New("cipher: message authentication failed")

func (g *gcm) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) {
- if len(nonce) != gcmNonceSize {
- panic("cipher: incorrect nonce length given to GCM")
- }
-
if len(ciphertext) < gcmTagSize {
return nil, errOpen
}
tag := ciphertext[len(ciphertext)-gcmTagSize:]
ciphertext = ciphertext[:len(ciphertext)-gcmTagSize]

- // See GCM spec, section 7.1.
var counter, tagMask [gcmBlockSize]byte
- copy(counter[:], nonce)
- counter[gcmBlockSize-1] = 1
+ g.deriveCounter(nonce, &counter)

g.cipher.Encrypt(tagMask[:], counter[:])
gcmInc32(&counter)
@@ -301,6 +289,28 @@
}
}

+// deriveCounter computes the initial GCM counter state from the given
nonce.
+// See NIST SP 800-38D, section 7.1.
+func (g *gcm) deriveCounter(nonce []byte, counter *[gcmBlockSize]byte) {
+ // GCM has two modes of operation with respect to the initial counter
+ // state: a "fast path" for 96-bit (12-byte) nonces, and a "slow path"
+ // for nonces of other lengths. For a 96-bit nonce, the nonce, along
+ // with a four-byte big-endian counter starting at one, is used
+ // directly as the starting counter. For other nonce sizes, the counter
+ // is computed by passing it through the GHASH function.
+ if len(nonce) == gcmNonceSize {
+ copy(counter[:], nonce)
+ counter[gcmBlockSize-1] = 1
+ } else {
+ var y gcmFieldElement
+ g.update(&y, nonce)
+ y.high ^= uint64(len(nonce)) * 8
+ g.mul(&y)
+ putUint64(counter[:8], y.low)
+ putUint64(counter[8:], y.high)
+ }
+}
+
// auth calculates GHASH(ciphertext, additionalData), masks the result with
// tagMask and writes the result to out.
func (g *gcm) auth(out, ciphertext, additionalData []byte, tagMask
*[gcmTagSize]byte) {
diff --git a/src/crypto/cipher/gcm_test.go b/src/crypto/cipher/gcm_test.go
index 0c502ce..8db87ef 100644
--- a/src/crypto/cipher/gcm_test.go
+++ b/src/crypto/cipher/gcm_test.go
@@ -101,6 +101,34 @@
"",
"b2051c80014f42f08735a7b0cd38e6bcd29962e5f2c13626b85a877101",
},
+ {
+ "1672c3537afa82004c6b8a46f6f0d026",
+ "05",
+ "",
+ "",
+ "8e2ad721f9455f74d8b53d3141f27e8e",
+ },
+ {
+ "9a4fea86a621a91ab371e492457796c0",
+ "75",
+ "ca6131faf0ff210e4e693d6c31c109fc5b6f54224eb120f37de31dc59ec669b6",
+ "4f6e2585c161f05a9ae1f2f894e9f0ab52b45d0f",
+
"5698c0a384241d30004290aac56bb3ece6fe8eacc5c4be98954deb9c3ff6aebf5d50e1af100509e1fba2a5e8a0af9670",
+ },
+ {
+ "d0f1f4defa1e8c08b4b26d576392027c",
+
"42b4f01eb9f5a1ea5b1eb73b0fb0baed54f387ecaa0393c7d7dffc6af50146ecc021abf7eb9038d4303d91f8d741a11743166c0860208bcc02c6258fd9511a2fa626f96d60b72fcff773af4e88e7a923506e4916ecbd814651e9f445adef4ad6a6b6c7290cc13b956130eef5b837c939fcac0cbbcc9656cd75b13823ee5acdac",
+ "",
+ "",
+ "7ab49b57ddf5f62c427950111c5c4f0d",
+ },
+ {
+ "4a0c00a3d284dea9d4bf8b8dde86685e",
+
"f8cbe82588e784bcacbe092cd9089b51e01527297f635bf294b3aa787d91057ef23869789698ac960707857f163ecb242135a228ad93964f5dc4a4d7f88fd7b3b07dd0a5b37f9768fb05a523639f108c34c661498a56879e501a2321c8a4a94d7e1b89db255ac1f685e185263368e99735ebe62a7f2931b47282be8eb165e4d7",
+ "6d4bf87640a6a48a50d28797b7",
+ "8d8c7ffc55086d539b5a8f0d1232654c",
+ "0d803ec309482f35b8e6226f2b56303239298e06b281c2d51aaba3c125",
+ },
}

func TestAESGCM(t *testing.T) {

--
https://go-review.googlesource.com/8946

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 1: Code-Review+1

R=agl

The code looks fine; adding agl for the semantics.

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>
Gerrit-HasComments: No

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 1: Code-Review-1

The variable-length nonce mode of GCM breaks the usual security model for
AEADs. The usual rule is that a (key, nonce) pair must not be repeated for
all time, but if the nonce is not 96 bits then this change will essentially
randomise it. This means that a completely innocuous (or even mistaken)
change to the IV length means that the analysis must switch to a
birthday-attack model. That's a very sharp corner to leave for people.

I'm not aware of anything common that needs this and we certainly wouldn't
want to promote it. What's your motivation for this change?

[Carl Jackson (Gerrit)]

Carl Jackson has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 1:

(Oops--replied to the email on list, which does not appear to have done the
right thing. X-posting here)

My specific use-case is Apple's Payment Token (the object underlying Apple
Pay), which somewhat bizarrely specifies GCM with a 16-byte all-zeroes
nonce:
https://developer.apple.com/library/ios/documentation/PassKit/Reference/PaymentTokenJSON/PaymentTokenJSON.html

Perhaps a better approach would be to add a nonceSize field to the gcm
struct and a companion function to NewGCM (say, NewGCMWithNonceSize) that
sets it to something non-standard. Each AEAD instance would then only
accept a single nonce size, and it gives us a venue in the docs to warn
people that this is probably not what they want.

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>

Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>

[Carl Jackson]

My specific use-case is Apple's Payment Token (the object underlying Apple Pay), which somewhat bizarrely specifies GCM with a 16-byte all-zeroes nonce:

https://developer.apple.com/library/ios/documentation/PassKit/Reference/PaymentTokenJSON/PaymentTokenJSON.html

Perhaps a better approach would be to add a nonceSize field to the gcm struct and a companion function to NewGCM (say, NewGCMWithNonceSize) that sets it to something non-standard. Each AEAD instance would then only accept a single nonce size, and it gives us a venue in the docs to warn people that this is probably not what they want.

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 1: -Code-Review

> My specific use-case is Apple's Payment Token (the object
> underlying Apple Pay), which somewhat bizarrely specifies GCM with
> a 16-byte all-zeroes nonce:

I think we can guess what happened here: someone at Apple tried to look up
the nonce size for AES-GCM, found that an AES IV is 16 bytes (which it is
for CBC, CTR etc), tried it and, because of this NIST misfeature, it worked
and shipped.

I hope you can understand that I'm not keen on making that mistake easy to
do in Go too!

> Perhaps a better approach would be to add a nonceSize field to the
> gcm struct and a companion function to NewGCM (say,
> NewGCMWithNonceSize) that sets it to something non-standard. Each
> AEAD instance would then only accept a single nonce size, and it
> gives us a venue in the docs to warn people that this is probably
> not what they want.

Sadly the non-standard nonce mode also randomises the block counter so I
think that's probably the best design.

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>

Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>

[Carl Jackson (Gerrit)]

Carl Jackson has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 1:

I think we're in agreement about how the 16-byte nonce came about :)

I guess to be explicit: should I change this diff to add a
NewGCMWithNonceSize so we can discuss that?

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 1:

> I guess to be explicit: should I change this diff to add a
> NewGCMWithNonceSize so we can discuss that?

Yes please.

[Carl Jackson (Gerrit)]

Reviewers: Russ Cox

Carl Jackson uploaded a new patch set:

https://go-review.googlesource.com/8946

crypto/cipher: Support unusual GCM nonce lengths

GCM is traditionally used with a 96-bit nonce, but the standard allows
for nonces of any size. Non-standard nonce sizes are required in some
protocols, so add support for them in crypto/cipher's GCM
implementation.

Change-Id: I7feca7e903eeba557dcce370412b6ffabf1207ab
---
M src/crypto/cipher/gcm.go
M src/crypto/cipher/gcm_test.go

2 files changed, 75 insertions(+), 18 deletions(-)

[Carl Jackson (Gerrit)]

Reviewers: Russ Cox

Carl Jackson uploaded a new patch set:

https://go-review.googlesource.com/8946

crypto/cipher: Support unusual GCM nonce lengths

GCM is traditionally used with a 96-bit nonce, but the standard allows
for nonces of any size. Non-standard nonce sizes are required in some
protocols, so add support for them in crypto/cipher's GCM
implementation.

Change-Id: I7feca7e903eeba557dcce370412b6ffabf1207ab
---
M src/crypto/cipher/gcm.go
M src/crypto/cipher/gcm_test.go

2 files changed, 78 insertions(+), 21 deletions(-)

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 3:

Mostly LGTM. I have some edits locally but Gerrit isn't letting me upload
them at the moment. Will do and land once that's fixed.

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 4:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=07b15839

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>

Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 4: Code-Review+2 Run-TryBot+1

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 4:

This change failed on linux-amd64:
See
https://storage.googleapis.com/go-build-log/07b15839/linux-amd64_1517e620.log

Consult https://build.golang.org/ to see whether it's a new failure. Other
builds still in progress; subsequent failure notices suppressed until final
report.

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>

Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

[Adam Langley (Gerrit)]

Reviewers: Russ Cox

Adam Langley uploaded a new patch set:

https://go-review.googlesource.com/8946

crypto/cipher: Support unusual GCM nonce lengths

GCM is traditionally used with a 96-bit nonce, but the standard allows
for nonces of any size. Non-standard nonce sizes are required in some
protocols, so add support for them in crypto/cipher's GCM
implementation.

Change-Id: I7feca7e903eeba557dcce370412b6ffabf1207ab
---
M src/crypto/cipher/gcm.go
M src/crypto/cipher/gcm_test.go

2 files changed, 80 insertions(+), 21 deletions(-)

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>

Gerrit-Reviewer: Russ Cox <r...@golang.org>

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

crypto/cipher: Support unusual GCM nonce lengths

Patch Set 4: TryBot-Result-1

3 of 20 TryBots failed:
Failed on linux-amd64:
https://storage.googleapis.com/go-build-log/07b15839/linux-amd64_1517e620.log
Failed on openbsd-amd64-gce56:
https://storage.googleapis.com/go-build-log/07b15839/openbsd-amd64-gce56_aa7c18b9.log
Failed on openbsd-386-gce56:
https://storage.googleapis.com/go-build-log/07b15839/openbsd-386-gce56_c7d84e14.log

Consult https://build.golang.org/ to see whether they are new failures.

--
https://go-review.googlesource.com/8946
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Carl Jackson <ca...@stripe.com>

[Adam Langley (Gerrit)]

Adam Langley has submitted this change and it was merged.

crypto/cipher: Support unusual GCM nonce lengths

GCM is traditionally used with a 96-bit nonce, but the standard allows
for nonces of any size. Non-standard nonce sizes are required in some
protocols, so add support for them in crypto/cipher's GCM
implementation.

Change-Id: I7feca7e903eeba557dcce370412b6ffabf1207ab

Reviewed-on: https://go-review.googlesource.com/8946
Reviewed-by: Adam Langley <a...@golang.org>
Run-TryBot: Adam Langley <a...@golang.org>

---
M src/crypto/cipher/gcm.go
M src/crypto/cipher/gcm_test.go

2 files changed, 80 insertions(+), 21 deletions(-)

Approvals:
Adam Langley: Looks good to me, approved; Run TryBots

Objections:
Gobot Gobot: TryBots failed