Skip to content

Commit be84aee

Browse files
aglBoringssl LUCI CQ
authored andcommitted
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>
1 parent 8e7025e commit be84aee

File tree

17 files changed

+48
-0
lines changed

17 files changed

+48
-0
lines changed

util/fipstools/acvp/acvptool/subprocess/aead.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ func (a *aead) Process(vectorSet []byte, m Transactable) (any, error) {
7272
// versions of the ACVP documents. You can find fragments in
7373
// https://github.com/usnistgov/ACVP.)
7474
for _, group := range parsed.Groups {
75+
group := group
7576
response := aeadTestGroupResponse{
7677
ID: group.ID,
7778
}
@@ -102,6 +103,8 @@ func (a *aead) Process(vectorSet []byte, m Transactable) (any, error) {
102103
tagBytes := group.TagBits / 8
103104

104105
for _, test := range group.Tests {
106+
test := test
107+
105108
if len(test.KeyHex) != keyBytes*2 {
106109
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)
107110
}

util/fipstools/acvp/acvptool/subprocess/block.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ func (b *blockCipher) Process(vectorSet []byte, m Transactable) (any, error) {
299299
// http://usnistgov.github.io/ACVP/artifacts/draft-celi-acvp-block-ciph-00.html#rfc.section.5.2
300300
// for details about the tests.
301301
for _, group := range parsed.Groups {
302+
group := group
302303
response := blockCipherTestGroupResponse{
303304
ID: group.ID,
304305
}
@@ -346,6 +347,8 @@ func (b *blockCipher) Process(vectorSet []byte, m Transactable) (any, error) {
346347
}
347348

348349
for _, test := range group.Tests {
350+
test := test
351+
349352
if len(test.KeyHex) == 0 && len(test.Key1Hex) > 0 {
350353
// 3DES encodes the key differently.
351354
test.KeyHex = test.Key1Hex + test.Key2Hex + test.Key3Hex

util/fipstools/acvp/acvptool/subprocess/drbg.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ func (d *drbg) Process(vectorSet []byte, m Transactable) (any, error) {
8484
// https://pages.nist.gov/ACVP/draft-vassilev-acvp-drbg.html#name-test-vectors
8585
// for details about the tests.
8686
for _, group := range parsed.Groups {
87+
group := group
8788
response := drbgTestGroupResponse{
8889
ID: group.ID,
8990
}
@@ -97,6 +98,8 @@ func (d *drbg) Process(vectorSet []byte, m Transactable) (any, error) {
9798
}
9899

99100
for _, test := range group.Tests {
101+
test := test
102+
100103
ent, err := extractField(test.EntropyHex, group.EntropyBits)
101104
if err != nil {
102105
return nil, fmt.Errorf("failed to extract entropy hex from test case %d/%d: %s", group.ID, test.ID, err)

util/fipstools/acvp/acvptool/subprocess/ecdsa.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ func (e *ecdsa) Process(vectorSet []byte, m Transactable) (any, error) {
8383
// https://pages.nist.gov/ACVP/draft-fussell-acvp-ecdsa.html#name-test-vectors
8484
// for details about the tests.
8585
for _, group := range parsed.Groups {
86+
group := group
87+
8688
if _, ok := e.curves[group.Curve]; !ok {
8789
return nil, fmt.Errorf("curve %q in test group %d not supported", group.Curve, group.ID)
8890
}
@@ -93,6 +95,8 @@ func (e *ecdsa) Process(vectorSet []byte, m Transactable) (any, error) {
9395
var sigGenPrivateKey []byte
9496

9597
for _, test := range group.Tests {
98+
test := test
99+
96100
var testResp ecdsaTestResponse
97101
testResp.ID = test.ID
98102

util/fipstools/acvp/acvptool/subprocess/hash.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,14 @@ func (h *hashPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
7373
// https://pages.nist.gov/ACVP/draft-celi-acvp-sha.html#name-test-vectors
7474
// for details about the tests.
7575
for _, group := range parsed.Groups {
76+
group := group
7677
response := hashTestGroupResponse{
7778
ID: group.ID,
7879
}
7980

8081
for _, test := range group.Tests {
82+
test := test
83+
8184
if uint64(len(test.MsgHex))*4 != test.BitLength {
8285
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)
8386
}

util/fipstools/acvp/acvptool/subprocess/hkdf.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ func (k *hkdf) Process(vectorSet []byte, m Transactable) (any, error) {
124124

125125
var respGroups []hkdfTestGroupResponse
126126
for _, group := range parsed.Groups {
127+
group := group
127128
groupResp := hkdfTestGroupResponse{ID: group.ID}
128129

129130
var isValidationTest bool
@@ -142,6 +143,7 @@ func (k *hkdf) Process(vectorSet []byte, m Transactable) (any, error) {
142143
}
143144

144145
for _, test := range group.Tests {
146+
test := test
145147
testResp := hkdfTestResponse{ID: test.ID}
146148

147149
key, salt, err := test.Params.extract()

util/fipstools/acvp/acvptool/subprocess/hmac.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ func (h *hmacPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
8787
// https://pages.nist.gov/ACVP/draft-fussell-acvp-mac.html#name-test-vectors
8888
// for details about the tests.
8989
for _, group := range parsed.Groups {
90+
group := group
9091
response := hmacTestGroupResponse{
9192
ID: group.ID,
9293
}
@@ -99,6 +100,8 @@ func (h *hmacPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
99100
outBytes := group.MACBits / 8
100101

101102
for _, test := range group.Tests {
103+
test := test
104+
102105
if len(test.MsgHex)*4 != group.MsgBits {
103106
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)
104107
}

util/fipstools/acvp/acvptool/subprocess/kas.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ func (k *kas) Process(vectorSet []byte, m Transactable) (any, error) {
7777
// See https://pages.nist.gov/ACVP/draft-fussell-acvp-kas-ecc.html#name-test-vectors
7878
var ret []kasTestGroupResponse
7979
for _, group := range parsed.Groups {
80+
group := group
8081
response := kasTestGroupResponse{
8182
ID: group.ID,
8283
}
@@ -119,6 +120,8 @@ func (k *kas) Process(vectorSet []byte, m Transactable) (any, error) {
119120
method := "ECDH/" + group.Curve
120121

121122
for _, test := range group.Tests {
123+
test := test
124+
122125
var xHex, yHex, privateKeyHex string
123126
if useStaticNamedFields {
124127
xHex, yHex, privateKeyHex = test.StaticXHex, test.StaticYHex, test.StaticPrivateKeyHex

util/fipstools/acvp/acvptool/subprocess/kasdh.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ func (k *kasDH) Process(vectorSet []byte, m Transactable) (any, error) {
6868
// See https://pages.nist.gov/ACVP/draft-hammett-acvp-kas-ffc-sp800-56ar3.html
6969
var ret []kasDHTestGroupResponse
7070
for _, group := range parsed.Groups {
71+
group := group
7172
response := kasDHTestGroupResponse{
7273
ID: group.ID,
7374
}
@@ -110,6 +111,8 @@ func (k *kasDH) Process(vectorSet []byte, m Transactable) (any, error) {
110111

111112
const method = "FFDH"
112113
for _, test := range group.Tests {
114+
test := test
115+
113116
if len(test.PeerPublicHex) == 0 {
114117
return nil, fmt.Errorf("%d/%d is missing peer's key", group.ID, test.ID)
115118
}

util/fipstools/acvp/acvptool/subprocess/kdf.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ func (k *kdfPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
6868

6969
var respGroups []kdfTestGroupResponse
7070
for _, group := range parsed.Groups {
71+
group := group
7172
groupResp := kdfTestGroupResponse{ID: group.ID}
7273

7374
if group.OutputBits%8 != 0 {
@@ -91,6 +92,7 @@ func (k *kdfPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
9192
outputBytes := uint32le(group.OutputBits / 8)
9293

9394
for _, test := range group.Tests {
95+
test := test
9496
testResp := kdfTestResponse{ID: test.ID}
9597

9698
var key []byte

0 commit comments

Comments
 (0)