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>
chromium-stable
Adam Langley 1 year ago committed by Boringssl LUCI CQ
parent 8e7025e3df
commit be84aeed7d
  1. 3
      util/fipstools/acvp/acvptool/subprocess/aead.go
  2. 3
      util/fipstools/acvp/acvptool/subprocess/block.go
  3. 3
      util/fipstools/acvp/acvptool/subprocess/drbg.go
  4. 4
      util/fipstools/acvp/acvptool/subprocess/ecdsa.go
  5. 3
      util/fipstools/acvp/acvptool/subprocess/hash.go
  6. 2
      util/fipstools/acvp/acvptool/subprocess/hkdf.go
  7. 3
      util/fipstools/acvp/acvptool/subprocess/hmac.go
  8. 3
      util/fipstools/acvp/acvptool/subprocess/kas.go
  9. 3
      util/fipstools/acvp/acvptool/subprocess/kasdh.go
  10. 2
      util/fipstools/acvp/acvptool/subprocess/kdf.go
  11. 2
      util/fipstools/acvp/acvptool/subprocess/keyedMac.go
  12. 11
      util/fipstools/acvp/acvptool/subprocess/rsa.go
  13. 2
      util/fipstools/acvp/acvptool/subprocess/tls13.go
  14. 2
      util/fipstools/acvp/acvptool/subprocess/tlskdf.go
  15. 2
      util/fipstools/acvp/acvptool/subprocess/xts.go
  16. BIN
      util/fipstools/acvp/acvptool/test/expected/TLS12.bz2
  17. BIN
      util/fipstools/acvp/acvptool/test/vectors/TLS12.bz2

@ -72,6 +72,7 @@ func (a *aead) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (a *aead) Process(vectorSet []byte, m Transactable) (any, error) {
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)
}

@ -299,6 +299,7 @@ func (b *blockCipher) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (b *blockCipher) Process(vectorSet []byte, m Transactable) (any, error) {
}
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

@ -84,6 +84,7 @@ func (d *drbg) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (d *drbg) Process(vectorSet []byte, m Transactable) (any, error) {
}
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)

@ -83,6 +83,8 @@ func (e *ecdsa) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (e *ecdsa) Process(vectorSet []byte, m Transactable) (any, error) {
var sigGenPrivateKey []byte
for _, test := range group.Tests {
test := test
var testResp ecdsaTestResponse
testResp.ID = test.ID

@ -73,11 +73,14 @@ func (h *hashPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
// 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)
}

@ -124,6 +124,7 @@ func (k *hkdf) Process(vectorSet []byte, m Transactable) (any, error) {
var respGroups []hkdfTestGroupResponse
for _, group := range parsed.Groups {
group := group
groupResp := hkdfTestGroupResponse{ID: group.ID}
var isValidationTest bool
@ -142,6 +143,7 @@ func (k *hkdf) Process(vectorSet []byte, m Transactable) (any, error) {
}
for _, test := range group.Tests {
test := test
testResp := hkdfTestResponse{ID: test.ID}
key, salt, err := test.Params.extract()

@ -87,6 +87,7 @@ func (h *hmacPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (h *hmacPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
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)
}

@ -77,6 +77,7 @@ func (k *kas) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (k *kas) Process(vectorSet []byte, m Transactable) (any, error) {
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

@ -68,6 +68,7 @@ func (k *kasDH) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (k *kasDH) Process(vectorSet []byte, m Transactable) (any, error) {
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)
}

@ -68,6 +68,7 @@ func (k *kdfPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
var respGroups []kdfTestGroupResponse
for _, group := range parsed.Groups {
group := group
groupResp := kdfTestGroupResponse{ID: group.ID}
if group.OutputBits%8 != 0 {
@ -91,6 +92,7 @@ func (k *kdfPrimitive) Process(vectorSet []byte, m Transactable) (any, error) {
outputBytes := uint32le(group.OutputBits / 8)
for _, test := range group.Tests {
test := test
testResp := kdfTestResponse{ID: test.ID}
var key []byte

@ -65,6 +65,7 @@ func (k *keyedMACPrimitive) Process(vectorSet []byte, m Transactable) (any, erro
var respGroups []keyedMACTestGroupResponse
for _, group := range vs.Groups {
group := group
respGroup := keyedMACTestGroupResponse{ID: group.ID}
if group.KeyBits%8 != 0 {
@ -90,6 +91,7 @@ func (k *keyedMACPrimitive) Process(vectorSet []byte, m Transactable) (any, erro
outputBytes := uint32le(group.MACBits / 8)
for _, test := range group.Tests {
test := test
respTest := keyedMACTestResponse{ID: test.ID}
// Validate input.

@ -126,6 +126,8 @@ func processKeyGen(vectorSet []byte, m Transactable) (any, error) {
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 @@ func processKeyGen(vectorSet []byte, m Transactable) (any, error) {
}
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 @@ func processSigGen(vectorSet []byte, m Transactable) (any, error) {
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 @@ func processSigGen(vectorSet []byte, m Transactable) (any, error) {
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 @@ func processSigVer(vectorSet []byte, m Transactable) (any, error) {
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 @@ func processSigVer(vectorSet []byte, m Transactable) (any, error) {
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)

@ -77,9 +77,11 @@ func (k *tls13) Process(vectorSet []byte, m Transactable) (any, error) {
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)

@ -64,6 +64,7 @@ func (k *tlsKDF) Process(vectorSet []byte, m Transactable) (any, error) {
// 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 @@ func (k *tlsKDF) Process(vectorSet []byte, m Transactable) (any, error) {
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

@ -67,6 +67,7 @@ func (h *xts) Process(vectorSet []byte, m Transactable) (any, error) {
var ret []xtsTestGroupResponse
for _, group := range parsed.Groups {
group := group
response := xtsTestGroupResponse{
ID: group.ID,
}
@ -88,6 +89,7 @@ func (h *xts) Process(vectorSet []byte, m Transactable) (any, error) {
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)
}

Loading…
Cancel
Save