Don't send two post-quantum initial key shares.
More than one post-quantum group is now defined so it would be possible
for two PQ groups to be 1st and 2nd preferences. In that case, we
probably don't want to send two PQ initial key shares.
(Only one PQ group is _implemented_ currently, so we can't write a test
for this.)
Change-Id: I51ff118f224153e09a0c3ee8b142aebb6b340dcb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56226
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/ssl/extensions.cc b/ssl/extensions.cc
index abc7e38..7ac8212 100644
--- a/ssl/extensions.cc
+++ b/ssl/extensions.cc
@@ -2307,11 +2307,13 @@
group_id = groups[0];
- if (is_post_quantum_group(group_id) && groups.size() >= 2) {
- // CECPQ2(b) is not sent as the only initial key share. We'll include the
- // 2nd preference group too to avoid round-trips.
- second_group_id = groups[1];
- assert(second_group_id != group_id);
+ // We'll try to include one post-quantum and one classical initial key
+ // share.
+ for (size_t i = 1; i < groups.size() && second_group_id == 0; i++) {
+ if (is_post_quantum_group(group_id) != is_post_quantum_group(groups[i])) {
+ second_group_id = groups[i];
+ assert(second_group_id != group_id);
+ }
}
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index de64a9a..d33fbc5 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -11901,13 +11901,13 @@
},
})
- // ... but only if CECPQ2 is listed first.
+ // ... and the other way around
testCases = append(testCases, testCase{
- name: "CECPQ2KeyShareNotIncludedSecond",
+ name: "CECPQ2KeyShareIncludedSecond",
config: Config{
MinVersion: VersionTLS13,
Bugs: ProtocolBugs{
- ExpectedKeyShares: []CurveID{CurveX25519},
+ ExpectedKeyShares: []CurveID{CurveX25519, CurveCECPQ2},
},
},
flags: []string{
@@ -11917,6 +11917,25 @@
},
})
+ // ... and even if there's another curve in the middle because it's the
+ // first classical and first post-quantum "curves" that get key shares
+ // included.
+ testCases = append(testCases, testCase{
+ name: "CECPQ2KeyShareIncludedThird",
+ config: Config{
+ MinVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ ExpectedKeyShares: []CurveID{CurveX25519, CurveCECPQ2},
+ },
+ },
+ flags: []string{
+ "-curves", strconv.Itoa(int(CurveX25519)),
+ "-curves", strconv.Itoa(int(CurveP256)),
+ "-curves", strconv.Itoa(int(CurveCECPQ2)),
+ "-expect-curve-id", strconv.Itoa(int(CurveX25519)),
+ },
+ })
+
// If CECPQ2 is the only configured curve, the key share is sent.
testCases = append(testCases, testCase{
name: "JustConfiguringCECPQ2Works",
