runner: Remove redundant -enable-all-curves shim flag. We already know all the supported curves in runner.go. No sense in repeating this list in more places than needed. (I'm about to need a similar construct for -signing-prefs, so I figure it's worth being consistent.) This CL also adds a ShimConfig option because others don't support the same curves we do and will likely run into this quickly. Change-Id: Id79cea16891802af021b53a33ffd811a5d51c4ae Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46186 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index cc0fd06..6a2ef15 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -97,6 +97,11 @@ // HalfRTTTickets is the number of half-RTT tickets the client should // expect before half-RTT data when testing 0-RTT. HalfRTTTickets int + + // AllCurves is the list of all curve code points supported by the shim. + // This is currently used to control tests that enable all curves but may + // automatically disable tests in the future. + AllCurves []int } // Setup shimConfig defaults aligning with BoringSSL. @@ -253,6 +258,14 @@ garbageCertificate.PrivateKey = rsaCertificate.PrivateKey } +func flagInts(flagName string, vals []int) []string { + ret := make([]string, 0, 2*len(vals)) + for _, val := range vals { + ret = append(ret, flagName, strconv.Itoa(val)) + } + return ret +} + func useDebugger() bool { return *useGDB || *useLLDB || *useRR || *waitForDebugger } @@ -9982,11 +9995,13 @@ fakeSigAlg2, }, }, - flags: []string{ - "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)), - "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)), - "-enable-all-curves", - }, + flags: append( + []string{ + "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)), + "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)), + }, + flagInts("-curves", shimConfig.AllCurves)..., + ), shouldFail: shouldFail, expectedError: signError, expectedLocalError: signLocalError, @@ -10004,12 +10019,14 @@ MaxVersion: ver.version, VerifySignatureAlgorithms: allAlgorithms, }, - flags: []string{ - "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)), - "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)), - "-enable-all-curves", - "-signing-prefs", strconv.Itoa(int(alg.id)), - }, + flags: append( + []string{ + "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)), + "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)), + "-signing-prefs", strconv.Itoa(int(alg.id)), + }, + flagInts("-curves", shimConfig.AllCurves)..., + ), expectations: connectionExpectations{ peerSignatureAlgorithm: alg.id, }, @@ -10046,12 +10063,14 @@ IgnorePeerSignatureAlgorithmPreferences: shouldFail, }, }, - flags: []string{ - "-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)), - "-enable-all-curves", - // The algorithm may be disabled by default, so explicitly enable it. - "-verify-prefs", strconv.Itoa(int(alg.id)), - }, + flags: append( + []string{ + "-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)), + // The algorithm may be disabled by default, so explicitly enable it. + "-verify-prefs", strconv.Itoa(int(alg.id)), + }, + flagInts("-curves", shimConfig.AllCurves)..., + ), // Resume the session to assert the peer signature // algorithm is reported on both handshakes. resumeSession: !shouldFail, @@ -10077,10 +10096,10 @@ IgnorePeerSignatureAlgorithmPreferences: rejectByDefault, }, }, - flags: []string{ - "-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)), - "-enable-all-curves", - }, + flags: append( + []string{"-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id))}, + flagInts("-curves", shimConfig.AllCurves)..., + ), // Resume the session to assert the peer signature // algorithm is reported on both handshakes. resumeSession: !rejectByDefault, @@ -10103,11 +10122,11 @@ InvalidSignature: true, }, }, - flags: []string{ - "-enable-all-curves", + flags: append( // The algorithm may be disabled by default, so explicitly enable it. - "-verify-prefs", strconv.Itoa(int(alg.id)), - }, + []string{"-verify-prefs", strconv.Itoa(int(alg.id))}, + flagInts("-curves", shimConfig.AllCurves)..., + ), shouldFail: true, expectedError: ":BAD_SIGNATURE:", } @@ -11491,10 +11510,10 @@ }, CurvePreferences: []CurveID{curve.id}, }, - flags: []string{ - "-enable-all-curves", - "-expect-curve-id", strconv.Itoa(int(curve.id)), - }, + flags: append( + []string{"-expect-curve-id", strconv.Itoa(int(curve.id))}, + flagInts("-curves", shimConfig.AllCurves)..., + ), expectations: connectionExpectations{ curveID: curve.id, }, @@ -11511,10 +11530,10 @@ }, CurvePreferences: []CurveID{curve.id}, }, - flags: []string{ - "-enable-all-curves", - "-expect-curve-id", strconv.Itoa(int(curve.id)), - }, + flags: append( + []string{"-expect-curve-id", strconv.Itoa(int(curve.id))}, + flagInts("-curves", shimConfig.AllCurves)..., + ), expectations: connectionExpectations{ curveID: curve.id, }, @@ -11535,7 +11554,7 @@ SendCompressedCoordinates: true, }, }, - flags: []string{"-enable-all-curves"}, + flags: flagInts("-curves", shimConfig.AllCurves), shouldFail: true, expectedError: ":BAD_ECPOINT:", }) @@ -11554,7 +11573,7 @@ SendCompressedCoordinates: true, }, }, - flags: []string{"-enable-all-curves"}, + flags: flagInts("-curves", shimConfig.AllCurves), shouldFail: true, expectedError: ":BAD_ECPOINT:", }) @@ -16760,6 +16779,25 @@ *resourceDir = path.Clean(*resourceDir) initCertificates() + if len(*shimConfigFile) != 0 { + encoded, err := ioutil.ReadFile(*shimConfigFile) + if err != nil { + fmt.Fprintf(os.Stderr, "Couldn't read config file %q: %s\n", *shimConfigFile, err) + os.Exit(1) + } + + if err := json.Unmarshal(encoded, &shimConfig); err != nil { + fmt.Fprintf(os.Stderr, "Couldn't decode config file %q: %s\n", *shimConfigFile, err) + os.Exit(1) + } + } + + if shimConfig.AllCurves == nil { + for _, curve := range testCurves { + shimConfig.AllCurves = append(shimConfig.AllCurves, int(curve.id)) + } + } + addBasicTests() addCipherSuiteTests() addBadECDSASignatureTests() @@ -16817,19 +16855,6 @@ testChan := make(chan *testCase, numWorkers) doneChan := make(chan *testresult.Results) - if len(*shimConfigFile) != 0 { - encoded, err := ioutil.ReadFile(*shimConfigFile) - if err != nil { - fmt.Fprintf(os.Stderr, "Couldn't read config file %q: %s\n", *shimConfigFile, err) - os.Exit(1) - } - - if err := json.Unmarshal(encoded, &shimConfig); err != nil { - fmt.Fprintf(os.Stderr, "Couldn't decode config file %q: %s\n", *shimConfigFile, err) - os.Exit(1) - } - } - go statusPrinter(doneChan, statusChan, len(testCases)) for i := 0; i < numWorkers; i++ {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 59c5e39..d5a5357 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -109,7 +109,6 @@ {"-renegotiate-explicit", &TestConfig::renegotiate_explicit}, {"-forbid-renegotiation-after-handshake", &TestConfig::forbid_renegotiation_after_handshake}, - {"-enable-all-curves", &TestConfig::enable_all_curves}, {"-use-old-client-cert-callback", &TestConfig::use_old_client_cert_callback}, {"-send-alert", &TestConfig::send_alert}, @@ -1721,16 +1720,6 @@ } } } - if (enable_all_curves) { - static const int kAllCurves[] = { - NID_secp224r1, NID_X9_62_prime256v1, NID_secp384r1, - NID_secp521r1, NID_X25519, NID_CECPQ2, - }; - if (!SSL_set1_curves(ssl.get(), kAllCurves, - OPENSSL_ARRAY_SIZE(kAllCurves))) { - return nullptr; - } - } if (initial_timeout_duration_ms > 0) { DTLSv1_set_initial_timeout_duration(ssl.get(), initial_timeout_duration_ms); }
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index c24c376..1f3542b 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -131,7 +131,6 @@ bool renegotiate_explicit = false; bool forbid_renegotiation_after_handshake = false; int expect_peer_signature_algorithm = 0; - bool enable_all_curves = false; int expect_curve_id = 0; bool use_old_client_cert_callback = false; int initial_timeout_duration_ms = 0;