Expose an SSLNamedGroup for mlkem768x25519 #2102
Conversation
jschanck
requested review from
KershawChang,
martinthomson,
larseggert and
mxinden
as code owners
Failed Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2102 +/- ##
==========================================
- Coverage 95.35% 95.31% -0.04%
==========================================
Files 112 112
Lines 36325 36319 -6
==========================================
- Hits 34636 34619 -17
- Misses 1689 1700 +11 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
There's a difference here between what it takes to write the code and what it takes to land it. This patch is great and we should be taking it. The challenge is in managing the release arrangements.
It isn't great having this repository reliant on prerelease NSS. But we're going to end up in that situation at times.
Is this just a case where we have to quickly slot all the pieces in place in order (NSS, neqo, gecko)? Or can we make that a little less difficult with something like a feature flag?
Benchmark resultsPerformance differences relative to d4978de. coalesce_acked_from_zero 1+1 entries: Change within noise threshold. time: [99.740 ns 100.07 ns 100.41 ns]
change: [+0.1355% +0.6516% +1.2387%] (p = 0.02 < 0.05)
coalesce_acked_from_zero 3+1 entries: Change within noise threshold. time: [118.09 ns 118.35 ns 118.64 ns]
change: [+0.1370% +0.9424% +1.7933%] (p = 0.02 < 0.05)
coalesce_acked_from_zero 10+1 entries: No change in performance detected. time: [117.71 ns 118.28 ns 118.97 ns]
change: [-0.1790% +0.5341% +1.1364%] (p = 0.12 > 0.05)
coalesce_acked_from_zero 1000+1 entries: No change in performance detected. time: [98.171 ns 98.283 ns 98.421 ns]
change: [-0.1274% +0.6451% +1.3001%] (p = 0.08 > 0.05)
RxStreamOrderer::inbound_frame(): Change within noise threshold. time: [111.62 ms 111.67 ms 111.73 ms]
change: [-0.4380% -0.2334% -0.0996%] (p = 0.00 < 0.05)
transfer/pacing-false/varying-seeds: 💚 Performance has improved. time: [24.481 ms 25.377 ms 26.305 ms]
change: [-16.177% -11.450% -6.5520%] (p = 0.00 < 0.05)
transfer/pacing-true/varying-seeds: No change in performance detected. time: [34.646 ms 36.428 ms 38.200 ms]
change: [-10.008% -3.9628% +2.0557%] (p = 0.21 > 0.05)
transfer/pacing-false/same-seed: Change within noise threshold. time: [31.401 ms 32.210 ms 32.989 ms]
change: [-7.1453% -3.7301% -0.6615%] (p = 0.03 < 0.05)
transfer/pacing-true/same-seed: No change in performance detected. time: [39.272 ms 41.477 ms 43.736 ms]
change: [-5.0105% +5.0241% +15.767%] (p = 0.32 > 0.05)
1-conn/1-100mb-resp (aka. Download)/client: Change within noise threshold. time: [114.35 ms 114.85 ms 115.31 ms]
thrpt: [867.22 MiB/s 870.72 MiB/s 874.49 MiB/s]
change:
time: [+0.9550% +1.5223% +2.0774%] (p = 0.00 < 0.05)
thrpt: [-2.0352% -1.4995% -0.9460%]
1-conn/10_000-parallel-1b-resp (aka. RPS)/client: Change within noise threshold. time: [316.31 ms 319.52 ms 322.74 ms]
thrpt: [30.985 Kelem/s 31.297 Kelem/s 31.615 Kelem/s]
change:
time: [+0.0913% +1.5959% +3.2606%] (p = 0.04 < 0.05)
thrpt: [-3.1576% -1.5708% -0.0912%]
1-conn/1-1b-resp (aka. HPS)/client: Change within noise threshold. time: [34.160 ms 34.424 ms 34.708 ms]
thrpt: [28.812 elem/s 29.050 elem/s 29.274 elem/s]
change:
time: [+0.3613% +1.2635% +2.1923%] (p = 0.01 < 0.05)
thrpt: [-2.1453% -1.2477% -0.3600%]
Client/server transfer resultsTransfer of 33554432 bytes over loopback.
|
Not really. This isn't blocking anything except for enabling mlkem768x25519 in h3 (D222041). The NSS uplift (D222012) and the PSM integration (D222019) are both in autoland with clean try runs, so they'll be in Nightly shortly. The NSS release happens on the same day as the Firefox soft freeze, so waiting for the release just means that Firefox 132 will use mlkem768x25519 for TLS but not for h3 (which is fine).
Thought about this for a bit---probably more hassle than it's worth. |
|
I thought that I'd ask, but I reached the same conclusion. Thanks! |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
|
Landing this will need to wait until we can back out #2106, i.e., unpin NSS to a version that has the MLKEM stuff. |
* Replace xyber768 with mlkem768x25519 * rustfmt --------- Co-authored-by: Lars Eggert <[email protected]>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
NSS 3.105 will support ML-KEM-768 (the FIPS-203 version of Kyber768). An alpha release of 3.105 will land in M-C in D222012. I'm switching PSM from xyber768d00 to mlkem768x25519 in D222019. This patch exposes an SSLNamedGroup variant for
ssl_grp_kem_mlkem768x25519so that I can do the same for neqo_glue.This patch also changes the behavior of
init_once()so that xyber768d00 is disabled by policy (it's disabled by default in NSS and was being enabled ininit_once()). This will prevent servers from doing an HRR for xyber768d00 when they are presented with, say, {mlkem768x25519, x25519}. I hear that most servers that currently support xyber768d00 will support mlkem768x25519 by mid-October, slightly before the Firefox 132 release. So disabling xyber768d00 should not have a significant impact on the fraction of Firefox-initiated handshakes that use a post-quantum key exchange.If you're not comfortable merging a change that depends on pre-release NSS code, the plan is to cut NSS_3_105_BETA on Sept 19 and NSS_3_105_RTM on Sept 26. I'll have plenty of time to make the neqo_glue changes even if you wait until the 26th.