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;
