acvptool: create fresh variables in loops.
Referencing a variable in a closure captures it by _address_. So
referencing a loop variable can go horribly wrong:
https://go.dev/play/p/f2ivPAIN_bG
This is accepted as essentially a bug by Go and will be fixed in a
future release (https://github.com/golang/go/wiki/LoopvarExperiment).
But, for now at least, work around it.
Our tests trim the ACVP inputs to only have a single test case per group
in many cases, which hides most of this issue from tests. When we run
run full ACVP sets, our modulewrapper is seemingly fast enough not to
notice there either. But I've updated one of the tests here by
duplicating a test case enough that it catches this a meaningful amount
of the time.
Change-Id: I8216c00f67636ab7dad927eae4b49ae45ae3cf31
Bug: 646
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62965
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/util/fipstools/acvp/acvptool/subprocess/aead.go b/util/fipstools/acvp/acvptool/subprocess/aead.go
index ba0eee9..c38b170 100644
--- a/util/fipstools/acvp/acvptool/subprocess/aead.go
+++ b/util/fipstools/acvp/acvptool/subprocess/aead.go
@@ -72,6 +72,7 @@
// versions of the ACVP documents. You can find fragments in
// https://github.com/usnistgov/ACVP.)
for _, group := range parsed.Groups {
+ group := group
response := aeadTestGroupResponse{
ID: group.ID,
}
@@ -102,6 +103,8 @@
tagBytes := group.TagBits / 8
for _, test := range group.Tests {
+ test := test
+
if len(test.KeyHex) != keyBytes*2 {
return nil, fmt.Errorf("test case %d/%d contains key %q of length %d, but expected %d-bit key", group.ID, test.ID, test.KeyHex, len(test.KeyHex), group.KeyBits)
}
diff --git a/util/fipstools/acvp/acvptool/subprocess/block.go b/util/fipstools/acvp/acvptool/subprocess/block.go
index 2f05802..bcc6613 100644
--- a/util/fipstools/acvp/acvptool/subprocess/block.go
+++ b/util/fipstools/acvp/acvptool/subprocess/block.go
@@ -299,6 +299,7 @@
// http://usnistgov.github.io/ACVP/artifacts/draft-celi-acvp-block-ciph-00.html#rfc.section.5.2
// for details about the tests.
for _, group := range parsed.Groups {
+ group := group
response := blockCipherTestGroupResponse{
ID: group.ID,
}
@@ -346,6 +347,8 @@
}
for _, test := range group.Tests {
+ test := test
+
if len(test.KeyHex) == 0 && len(test.Key1Hex) > 0 {
// 3DES encodes the key differently.
test.KeyHex = test.Key1Hex + test.Key2Hex + test.Key3Hex
diff --git a/util/fipstools/acvp/acvptool/subprocess/drbg.go b/util/fipstools/acvp/acvptool/subprocess/drbg.go
index b403f04..87584d6 100644
--- a/util/fipstools/acvp/acvptool/subprocess/drbg.go
+++ b/util/fipstools/acvp/acvptool/subprocess/drbg.go
@@ -84,6 +84,7 @@
// https://pages.nist.gov/ACVP/draft-vassilev-acvp-drbg.html#name-test-vectors
// for details about the tests.
for _, group := range parsed.Groups {
+ group := group
response := drbgTestGroupResponse{
ID: group.ID,
}
@@ -97,6 +98,8 @@
}
for _, test := range group.Tests {
+ test := test
+
ent, err := extractField(test.EntropyHex, group.EntropyBits)
if err != nil {
return nil, fmt.Errorf("failed to extract entropy hex from test case %d/%d: %s", group.ID, test.ID, err)
diff --git a/util/fipstools/acvp/acvptool/subprocess/ecdsa.go b/util/fipstools/acvp/acvptool/subprocess/ecdsa.go
index 16d3a83..69706bd 100644
--- a/util/fipstools/acvp/acvptool/subprocess/ecdsa.go
+++ b/util/fipstools/acvp/acvptool/subprocess/ecdsa.go
@@ -83,6 +83,8 @@
// https://pages.nist.gov/ACVP/draft-fussell-acvp-ecdsa.html#name-test-vectors
// for details about the tests.
for _, group := range parsed.Groups {
+ group := group
+
if _, ok := e.curves[group.Curve]; !ok {
return nil, fmt.Errorf("curve %q in test group %d not supported", group.Curve, group.ID)
}
@@ -93,6 +95,8 @@
var sigGenPrivateKey []byte
for _, test := range group.Tests {
+ test := test
+
var testResp ecdsaTestResponse
testResp.ID = test.ID
diff --git a/util/fipstools/acvp/acvptool/subprocess/hash.go b/util/fipstools/acvp/acvptool/subprocess/hash.go
index 1f34d1a..aeac6d6 100644
--- a/util/fipstools/acvp/acvptool/subprocess/hash.go
+++ b/util/fipstools/acvp/acvptool/subprocess/hash.go
@@ -73,11 +73,14 @@
// https://pages.nist.gov/ACVP/draft-celi-acvp-sha.html#name-test-vectors
// for details about the tests.
for _, group := range parsed.Groups {
+ group := group
response := hashTestGroupResponse{
ID: group.ID,
}
for _, test := range group.Tests {
+ test := test
+
if uint64(len(test.MsgHex))*4 != test.BitLength {
return nil, fmt.Errorf("test case %d/%d contains hex message of length %d but specifies a bit length of %d", group.ID, test.ID, len(test.MsgHex), test.BitLength)
}
diff --git a/util/fipstools/acvp/acvptool/subprocess/hkdf.go b/util/fipstools/acvp/acvptool/subprocess/hkdf.go
index 3a6ba04..c64e2b8 100644
--- a/util/fipstools/acvp/acvptool/subprocess/hkdf.go
+++ b/util/fipstools/acvp/acvptool/subprocess/hkdf.go
@@ -124,6 +124,7 @@
var respGroups []hkdfTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
groupResp := hkdfTestGroupResponse{ID: group.ID}
var isValidationTest bool
@@ -142,6 +143,7 @@
}
for _, test := range group.Tests {
+ test := test
testResp := hkdfTestResponse{ID: test.ID}
key, salt, err := test.Params.extract()
diff --git a/util/fipstools/acvp/acvptool/subprocess/hmac.go b/util/fipstools/acvp/acvptool/subprocess/hmac.go
index 8fc7695..6b8a3cf 100644
--- a/util/fipstools/acvp/acvptool/subprocess/hmac.go
+++ b/util/fipstools/acvp/acvptool/subprocess/hmac.go
@@ -87,6 +87,7 @@
// https://pages.nist.gov/ACVP/draft-fussell-acvp-mac.html#name-test-vectors
// for details about the tests.
for _, group := range parsed.Groups {
+ group := group
response := hmacTestGroupResponse{
ID: group.ID,
}
@@ -99,6 +100,8 @@
outBytes := group.MACBits / 8
for _, test := range group.Tests {
+ test := test
+
if len(test.MsgHex)*4 != group.MsgBits {
return nil, fmt.Errorf("test case %d/%d contains hex message of length %d but specifies a bit length of %d", group.ID, test.ID, len(test.MsgHex), group.MsgBits)
}
diff --git a/util/fipstools/acvp/acvptool/subprocess/kas.go b/util/fipstools/acvp/acvptool/subprocess/kas.go
index cbc99ed..4c99f8a 100644
--- a/util/fipstools/acvp/acvptool/subprocess/kas.go
+++ b/util/fipstools/acvp/acvptool/subprocess/kas.go
@@ -77,6 +77,7 @@
// See https://pages.nist.gov/ACVP/draft-fussell-acvp-kas-ecc.html#name-test-vectors
var ret []kasTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
response := kasTestGroupResponse{
ID: group.ID,
}
@@ -119,6 +120,8 @@
method := "ECDH/" + group.Curve
for _, test := range group.Tests {
+ test := test
+
var xHex, yHex, privateKeyHex string
if useStaticNamedFields {
xHex, yHex, privateKeyHex = test.StaticXHex, test.StaticYHex, test.StaticPrivateKeyHex
diff --git a/util/fipstools/acvp/acvptool/subprocess/kasdh.go b/util/fipstools/acvp/acvptool/subprocess/kasdh.go
index f262b82..212dd31 100644
--- a/util/fipstools/acvp/acvptool/subprocess/kasdh.go
+++ b/util/fipstools/acvp/acvptool/subprocess/kasdh.go
@@ -68,6 +68,7 @@
// See https://pages.nist.gov/ACVP/draft-hammett-acvp-kas-ffc-sp800-56ar3.html
var ret []kasDHTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
response := kasDHTestGroupResponse{
ID: group.ID,
}
@@ -110,6 +111,8 @@
const method = "FFDH"
for _, test := range group.Tests {
+ test := test
+
if len(test.PeerPublicHex) == 0 {
return nil, fmt.Errorf("%d/%d is missing peer's key", group.ID, test.ID)
}
diff --git a/util/fipstools/acvp/acvptool/subprocess/kdf.go b/util/fipstools/acvp/acvptool/subprocess/kdf.go
index e27fcaa..6e41458 100644
--- a/util/fipstools/acvp/acvptool/subprocess/kdf.go
+++ b/util/fipstools/acvp/acvptool/subprocess/kdf.go
@@ -68,6 +68,7 @@
var respGroups []kdfTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
groupResp := kdfTestGroupResponse{ID: group.ID}
if group.OutputBits%8 != 0 {
@@ -91,6 +92,7 @@
outputBytes := uint32le(group.OutputBits / 8)
for _, test := range group.Tests {
+ test := test
testResp := kdfTestResponse{ID: test.ID}
var key []byte
diff --git a/util/fipstools/acvp/acvptool/subprocess/keyedMac.go b/util/fipstools/acvp/acvptool/subprocess/keyedMac.go
index e43ab5d..c91bb41 100644
--- a/util/fipstools/acvp/acvptool/subprocess/keyedMac.go
+++ b/util/fipstools/acvp/acvptool/subprocess/keyedMac.go
@@ -65,6 +65,7 @@
var respGroups []keyedMACTestGroupResponse
for _, group := range vs.Groups {
+ group := group
respGroup := keyedMACTestGroupResponse{ID: group.ID}
if group.KeyBits%8 != 0 {
@@ -90,6 +91,7 @@
outputBytes := uint32le(group.MACBits / 8)
for _, test := range group.Tests {
+ test := test
respTest := keyedMACTestResponse{ID: test.ID}
// Validate input.
diff --git a/util/fipstools/acvp/acvptool/subprocess/rsa.go b/util/fipstools/acvp/acvptool/subprocess/rsa.go
index d975026..923cdad 100644
--- a/util/fipstools/acvp/acvptool/subprocess/rsa.go
+++ b/util/fipstools/acvp/acvptool/subprocess/rsa.go
@@ -126,6 +126,8 @@
var ret []rsaKeyGenTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
+
// GDT means "Generated data test", i.e. "please generate an RSA key".
const expectedType = "GDT"
if group.Type != expectedType {
@@ -137,6 +139,8 @@
}
for _, test := range group.Tests {
+ test := test
+
m.TransactAsync("RSA/keyGen", 5, [][]byte{uint32le(group.ModulusBits)}, func(result [][]byte) error {
response.Tests = append(response.Tests, rsaKeyGenTestResponse{
ID: test.ID,
@@ -171,6 +175,8 @@
var ret []rsaSigGenTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
+
// GDT means "Generated data test", i.e. "please generate an RSA signature".
const expectedType = "GDT"
if group.Type != expectedType {
@@ -184,6 +190,8 @@
operation := "RSA/sigGen/" + group.Hash + "/" + group.SigType
for _, test := range group.Tests {
+ test := test
+
msg, err := hex.DecodeString(test.MessageHex)
if err != nil {
return nil, fmt.Errorf("test case %d/%d contains invalid hex: %s", group.ID, test.ID, err)
@@ -226,6 +234,8 @@
var ret []rsaSigVerTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
+
// GDT means "Generated data test", which makes no sense in this context.
const expectedType = "GDT"
if group.Type != expectedType {
@@ -248,6 +258,7 @@
operation := "RSA/sigVer/" + group.Hash + "/" + group.SigType
for _, test := range group.Tests {
+ test := test
msg, err := hex.DecodeString(test.MessageHex)
if err != nil {
return nil, fmt.Errorf("test case %d/%d contains invalid hex: %s", group.ID, test.ID, err)
diff --git a/util/fipstools/acvp/acvptool/subprocess/tls13.go b/util/fipstools/acvp/acvptool/subprocess/tls13.go
index af2aae8..bd12142 100644
--- a/util/fipstools/acvp/acvptool/subprocess/tls13.go
+++ b/util/fipstools/acvp/acvptool/subprocess/tls13.go
@@ -77,9 +77,11 @@
var respGroups []tls13TestGroupResponse
for _, group := range parsed.Groups {
+ group := group
groupResp := tls13TestGroupResponse{ID: group.ID}
for _, test := range group.Tests {
+ test := test
testResp := tls13TestResponse{ID: test.ID}
clientHello, err := hex.DecodeString(test.ClientHelloHex)
diff --git a/util/fipstools/acvp/acvptool/subprocess/tlskdf.go b/util/fipstools/acvp/acvptool/subprocess/tlskdf.go
index 3a0d7ce..251b53e 100644
--- a/util/fipstools/acvp/acvptool/subprocess/tlskdf.go
+++ b/util/fipstools/acvp/acvptool/subprocess/tlskdf.go
@@ -64,6 +64,7 @@
// See https://pages.nist.gov/ACVP/draft-celi-acvp-kdf-tls.html
var ret []tlsKDFTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
response := tlsKDFTestGroupResponse{
ID: group.ID,
}
@@ -82,6 +83,7 @@
method := "TLSKDF/1.2/" + group.Hash
for _, test := range group.Tests {
+ test := test
pms, err := hex.DecodeString(test.PMSHex)
if err != nil {
return nil, err
diff --git a/util/fipstools/acvp/acvptool/subprocess/xts.go b/util/fipstools/acvp/acvptool/subprocess/xts.go
index e813409..5a9e740 100644
--- a/util/fipstools/acvp/acvptool/subprocess/xts.go
+++ b/util/fipstools/acvp/acvptool/subprocess/xts.go
@@ -67,6 +67,7 @@
var ret []xtsTestGroupResponse
for _, group := range parsed.Groups {
+ group := group
response := xtsTestGroupResponse{
ID: group.ID,
}
@@ -88,6 +89,7 @@
funcName := "AES-XTS/" + group.Direction
for _, test := range group.Tests {
+ test := test
if group.KeyLen != len(test.KeyHex)*4/2 {
return nil, fmt.Errorf("test case %d/%d contains hex message of length %d but specifies a key length of %d (remember that XTS keys are twice the length of the underlying key size)", group.ID, test.ID, len(test.KeyHex), group.KeyLen)
}
diff --git a/util/fipstools/acvp/acvptool/test/expected/TLS12.bz2 b/util/fipstools/acvp/acvptool/test/expected/TLS12.bz2
index d83b691..ff4ded0 100644
--- a/util/fipstools/acvp/acvptool/test/expected/TLS12.bz2
+++ b/util/fipstools/acvp/acvptool/test/expected/TLS12.bz2
Binary files differ
diff --git a/util/fipstools/acvp/acvptool/test/vectors/TLS12.bz2 b/util/fipstools/acvp/acvptool/test/vectors/TLS12.bz2
index d1911ab..00d9bbb 100644
--- a/util/fipstools/acvp/acvptool/test/vectors/TLS12.bz2
+++ b/util/fipstools/acvp/acvptool/test/vectors/TLS12.bz2
Binary files differ
