tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit b82a54f19cf7a8d5164b8adac4fba4ae29d2e060
parent 5c048492bfaa82c71487eec14b702a74213d7472
Author: Anna <anna.weine@mozilla.com>
Date:   Mon,  6 Oct 2025 13:19:31 +0000

Bug 1932591 - WebRTC DTLS1.3 to enable sending a PQ key share and put the PQ share as the top priority in Nightly/Early Beta r=webrtc-reviewers,djackson,bwc

Differential Revision: https://phabricator.services.mozilla.com/D262568

Diffstat:
Mbrowser/components/enterprisepolicies/Policies.sys.mjs | 2+-
Mdom/media/webrtc/tests/mochitests/test_peerConnection_glean.html | 133+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
Mdom/media/webrtc/transport/transportlayerdtls.cpp | 103++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------
Mmodules/libpref/init/StaticPrefList.yaml | 6------
Mmodules/libpref/init/all.js | 6++++++
Mtoolkit/components/nimbus/FeatureManifest.yaml | 2+-
6 files changed, 202 insertions(+), 50 deletions(-)

diff --git a/browser/components/enterprisepolicies/Policies.sys.mjs b/browser/components/enterprisepolicies/Policies.sys.mjs @@ -2097,7 +2097,7 @@ export var Policies = { onBeforeAddons(manager, param) { setAndLockPref("network.http.http3.enable_kyber", param); setAndLockPref("security.tls.enable_kyber", param); - setAndLockPref("media.webrtc.enable_pq_dtls", param); + setAndLockPref("media.peerconnection.dtls.enable_pq_hybrid_kex", param); }, }, diff --git a/dom/media/webrtc/tests/mochitests/test_peerConnection_glean.html b/dom/media/webrtc/tests/mochitests/test_peerConnection_glean.html @@ -223,7 +223,41 @@ }, async function checkDtlsKEA() { - // Currently, ssl_grp_ec_curve25519 is the default option (ECDH) + // "media.peerconnection.dtls.enable_pq_hybrid_kex" is enabled by default + // "send_mlkem_keyshare" enabled in Nightly and Early Beta + // By default we send 2 key shares, PQ and X25519 + // PQ Key share (ECDH Hybrid) has a higher preference, so it will be chosen as KEA + const pc1 = new RTCPeerConnection(); + const pc2 = new RTCPeerConnection(); + await gleanResetTestValues(); + // SSL Handshake Key Exchange Algorithm (null=0, rsa=1, dh=2, ecdh=4, ecdh_hybrid=8) + let keyExchangeValue = await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue() || 0; + is(keyExchangeValue, 0, "Expected no keyExchange distribution defined"); + + const stream = await navigator.mediaDevices.getUserMedia({ video: true }); + pc1.addTrack(stream.getTracks()[0]); + + await connect(pc1, pc2, 32000, "DTLS connected", true, true); + // This telemetry happens on STS/socket process + await gleanFlushChildren(); + + let count1_0 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["0"] || 0; + let count1_1 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["1"] || 0; + let count1_2 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["2"] || 0; + let count1_4 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["4"] || 0; + let count1_8 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["8"] || 0; + is(count1_0, 0, "Expected 0 connections using NULL"); + is(count1_1, 0, "Expected 0 connections using RSA"); + is(count1_2, 0, "Expected 0 connections using DH"); + is(count1_4, 0, "Expected 0 connections using ECDH"); + is(count1_8, 2, "Expected 2 connections using ECDH Hybrid"); + }, + + async function checkDtlsKEA_DTLSBelow13() { + // DTLS1.2 does not use Kyber + // In this case, X25519 (ECDH) key share will be used + await withPrefs([["media.peerconnection.dtls.version.max", 771]], + async () => { const pc1 = new RTCPeerConnection(); const pc2 = new RTCPeerConnection(); await gleanResetTestValues(); @@ -248,7 +282,102 @@ is(count1_2, 0, "Expected 0 connections using DH"); is(count1_4, 2, "Expected 2 connections using ECDH"); is(count1_8, 0, "Expected 0 connections using ECDH Hybrid"); - }, + })}, + + async function checkDtlsKEA_DTLS13DisablePQ() { + await withPrefs([["media.peerconnection.dtls.enable_pq_hybrid_kex", false]], + async () => { + const pc1 = new RTCPeerConnection(); + const pc2 = new RTCPeerConnection(); + await gleanResetTestValues(); + // SSL Handshake Key Exchange Algorithm (null=0, rsa=1, dh=2, ecdh=4, ecdh_hybrid=8) + let keyExchangeValue = await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue() || 0; + is(keyExchangeValue, 0, "Expected no keyExchange distribution defined"); + + const stream = await navigator.mediaDevices.getUserMedia({ video: true }); + pc1.addTrack(stream.getTracks()[0]); + + await connect(pc1, pc2, 32000, "DTLS connected", true, true); + // This telemetry happens on STS/socket process + await gleanFlushChildren(); + + let count1_0 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["0"] || 0; + let count1_1 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["1"] || 0; + let count1_2 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["2"] || 0; + let count1_4 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["4"] || 0; + let count1_8 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["8"] || 0; + is(count1_0, 0, "Expected 0 connections using NULL"); + is(count1_1, 0, "Expected 0 connections using RSA"); + is(count1_2, 0, "Expected 0 connections using DH"); + is(count1_4, 2, "Expected 2 connections using ECDH"); + is(count1_8, 0, "Expected 0 connections using ECDH Hybrid"); + })}, + + async function checkDtlsKEA_DTLS13DisablePQEnablePQShare() { + // Safety measures, when PQ is disabled, even if the sending ml-kem share is enabled + // it should not be sent. + await withPrefs([["media.peerconnection.dtls.enable_pq_hybrid_kex", false], + ["media.peerconnection.dtls.send_mlkem_keyshare", true] + ], + async () => { + const pc1 = new RTCPeerConnection(); + const pc2 = new RTCPeerConnection(); + await gleanResetTestValues(); + // SSL Handshake Key Exchange Algorithm (null=0, rsa=1, dh=2, ecdh=4, ecdh_hybrid=8) + let keyExchangeValue = await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue() || 0; + is(keyExchangeValue, 0, "Expected no keyExchange distribution defined"); + + const stream = await navigator.mediaDevices.getUserMedia({ video: true }); + pc1.addTrack(stream.getTracks()[0]); + + await connect(pc1, pc2, 32000, "DTLS connected", true, true); + // This telemetry happens on STS/socket process + await gleanFlushChildren(); + + let count1_0 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["0"] || 0; + let count1_1 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["1"] || 0; + let count1_2 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["2"] || 0; + let count1_4 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["4"] || 0; + let count1_8 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["8"] || 0; + is(count1_0, 0, "Expected 0 connections using NULL"); + is(count1_1, 0, "Expected 0 connections using RSA"); + is(count1_2, 0, "Expected 0 connections using DH"); + is(count1_4, 2, "Expected 2 connections using ECDH"); + is(count1_8, 0, "Expected 0 connections using ECDH Hybrid"); + })}, + + async function checkDtlsKEA_DTLS13EnablePQDisablePQShare() { + // We will still advertise PQ, but we won't send a key share. + // See bug 1992457. + await withPrefs([["media.peerconnection.dtls.enable_pq_hybrid_kex", true], + ["media.peerconnection.dtls.send_mlkem_keyshare", false] + ], + async () => { + const pc1 = new RTCPeerConnection(); + const pc2 = new RTCPeerConnection(); + await gleanResetTestValues(); + // SSL Handshake Key Exchange Algorithm (null=0, rsa=1, dh=2, ecdh=4, ecdh_hybrid=8) + let keyExchangeValue = await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue() || 0; + is(keyExchangeValue, 0, "Expected no keyExchange distribution defined"); + + const stream = await navigator.mediaDevices.getUserMedia({ video: true }); + pc1.addTrack(stream.getTracks()[0]); + + await connect(pc1, pc2, 32000, "DTLS connected", true, true); + // This telemetry happens on STS/socket process + await gleanFlushChildren(); + + let count1_0 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["0"] || 0; + let count1_1 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["1"] || 0; + let count1_2 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["2"] || 0; + let count1_4 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["4"] || 0; + let count1_8 = (await GleanTest.webrtcdtls.keyExchangeAlgorithm.testGetValue()).values["8"] || 0; + is(count1_0, 0, "Expected 0 connections using NULL"); + is(count1_1, 0, "Expected 0 connections using RSA"); + is(count1_2, 0, "Expected 0 connections using DH"); + is(count1_4, 2, "Expected 2 connections using ECDH"); + is(count1_8, 0, "Expected 0 connections using ECDH Hybrid"); + })}, async function checkRTCRtpSenderCount() { const pc = new RTCPeerConnection(); diff --git a/dom/media/webrtc/transport/transportlayerdtls.cpp b/dom/media/webrtc/transport/transportlayerdtls.cpp @@ -593,39 +593,62 @@ bool TransportLayerDtls::Setup() { return false; } - unsigned int additional_shares = - StaticPrefs::security_tls_client_hello_send_p256_keyshare(); + // If the version is DTLS1.3 and pq in enabled, we will indicate support for + // ML-KEM + bool enable_mlkem = + maxVersion_ >= Version::DTLS_1_3 && + Preferences::GetBool("media.peerconnection.dtls.enable_pq_hybrid_kex"); - const SSLNamedGroup namedGroups[] = { - ssl_grp_ec_curve25519, ssl_grp_ec_secp256r1, ssl_grp_ec_secp384r1, - ssl_grp_ffdhe_2048, ssl_grp_ffdhe_3072, - // Advertise MLKEM support but do not pro-actively send a keyshare + bool send_mlkem = + Preferences::GetBool("media.peerconnection.dtls.send_mlkem_keyshare"); - // Mlkem must stay the last in the list because if we don't support it - // the amount of supported_groups will be sent without it. - ssl_grp_kem_mlkem768x25519}; + if (!enable_mlkem && send_mlkem) { + MOZ_MTLOG(ML_NOTICE, + "The PQ preferences are inconsistent. ML-KEM support will not be " + "advertised, nor will the ML-KEM key share be sent."); + } + + std::vector<SSLNamedGroup> namedGroups; - size_t numGroups = std::size(namedGroups); - if (!(StaticPrefs::security_tls_enable_kyber() && - StaticPrefs::media_webrtc_enable_pq_dtls() && - maxVersion_ >= Version::DTLS_1_3)) { - // Excluding the last group of the namedGroups. - numGroups -= 1; + if (enable_mlkem && send_mlkem) { + // RFC 8446: client_shares: A list of offered KeyShareEntry values in + // descending order of client preference. + // {ssl_grp_kem_mlkem768x25519}, {ssl_grp_ec_curve2551} key shared to be + // sent ML-KEM has the highest preference, so it's sent first. + namedGroups = {ssl_grp_kem_mlkem768x25519, ssl_grp_ec_curve25519, + ssl_grp_ec_secp256r1, ssl_grp_ec_secp384r1, + ssl_grp_ffdhe_2048, ssl_grp_ffdhe_3072}; + + if (SECSuccess != SSL_SendAdditionalKeyShares(ssl_fd.get(), 1)) { + MOZ_MTLOG(ML_ERROR, "Couldn't set up additional key shares"); + return false; + } + } + // Else we don't send any additional key share + else if (enable_mlkem && !send_mlkem) { + // Here the order of the namedGroups is different than in the first if + // {ssl_grp_ec_curve25519} is first because it's the key share + // that will be sent by default + namedGroups = {ssl_grp_ec_curve25519, ssl_grp_kem_mlkem768x25519, + ssl_grp_ec_secp256r1, ssl_grp_ec_secp384r1, + ssl_grp_ffdhe_2048, ssl_grp_ffdhe_3072}; + } + // ml_kem is disabled + // {ssl_grp_ec_curve25519} will be send as a default key_share + else { + namedGroups = {ssl_grp_ec_curve25519, ssl_grp_ec_secp256r1, + ssl_grp_ec_secp384r1, ssl_grp_ffdhe_2048, + ssl_grp_ffdhe_3072}; } - rv = SSL_NamedGroupConfig(ssl_fd.get(), namedGroups, numGroups); + rv = SSL_NamedGroupConfig(ssl_fd.get(), namedGroups.data(), + std::size(namedGroups)); if (rv != SECSuccess) { MOZ_MTLOG(ML_ERROR, "Couldn't set named groups"); return false; } - if (SECSuccess != - SSL_SendAdditionalKeyShares(ssl_fd.get(), additional_shares)) { - MOZ_MTLOG(ML_ERROR, "Couldn't set up additional key shares"); - return false; - } - // Certificate validation rv = SSL_AuthCertificateHook(ssl_fd.get(), AuthCertificateHook, reinterpret_cast<void*>(this)); @@ -699,16 +722,16 @@ bool TransportLayerDtls::SetupAlpn(UniquePRFileDesc& ssl_fd) const { // builds, but can be disabled with prefs and they aren't on in our unit tests // since that uses NSS default configuration. // -// Only override prefs to comply with MUST statements in the security-arch doc. -// Anything outside this list is governed by the usual combination of policy -// and user preferences. +// Only override prefs to comply with MUST statements in the security-arch +// doc. Anything outside this list is governed by the usual combination of +// policy and user preferences. static const uint32_t EnabledCiphers[] = { TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA}; -// Disable all NSS suites modes without PFS or with old and rusty ciphersuites. -// Anything outside this list is governed by the usual combination of policy -// and user preferences. +// Disable all NSS suites modes without PFS or with old and rusty +// ciphersuites. Anything outside this list is governed by the usual +// combination of policy and user preferences. static const uint32_t DisabledCiphers[] = { // Bug 1310061: disable all SHA384 ciphers until fixed TLS_AES_256_GCM_SHA384, @@ -840,8 +863,8 @@ std::vector<uint16_t> TransportLayerDtls::GetDefaultSrtpCiphers() { ciphers.push_back(kDtlsSrtpAeadAes256Gcm); ciphers.push_back(kDtlsSrtpAes128CmHmacSha1_80); #ifndef NIGHTLY_BUILD - // To support bug 1491583 lets try to find out if we get bug reports if we no - // longer offer this in Nightly builds. + // To support bug 1491583 lets try to find out if we get bug reports if we + // no longer offer this in Nightly builds. ciphers.push_back(kDtlsSrtpAes128CmHmacSha1_32); #endif @@ -870,8 +893,8 @@ void TransportLayerDtls::StateChange(TransportLayer* layer, State state) { LAYER_INFO << "Lower layer is now open; starting TLS"); timer_->Cancel(); timer_->SetTarget(target_); - // Async, since the ICE layer might need to send a STUN response, and we - // don't want the handshake to start until that is sent. + // Async, since the ICE layer might need to send a STUN response, and + // we don't want the handshake to start until that is sent. timer_->InitWithNamedFuncCallback( TimerCallback, this, 0, nsITimer::TYPE_ONE_SHOT, "TransportLayerDtls::TimerCallback"_ns); @@ -1002,8 +1025,8 @@ bool TransportLayerDtls::CheckAlpn() { return !alpn_.empty(); case SSL_NEXT_PROTO_NO_OVERLAP: - // This only happens if there is a custom NPN/ALPN callback installed and - // that callback doesn't properly handle ALPN. + // This only happens if there is a custom NPN/ALPN callback installed + // and that callback doesn't properly handle ALPN. MOZ_MTLOG(ML_ERROR, LAYER_INFO << "error in ALPN selection callback"); return false; @@ -1016,8 +1039,8 @@ bool TransportLayerDtls::CheckAlpn() { std::string chosen(chosenAlpn, chosenAlpnLen); MOZ_MTLOG(ML_NOTICE, LAYER_INFO << "Selected ALPN string: " << chosen); if (alpn_allowed_.find(chosen) == alpn_allowed_.end()) { - // Maybe our peer chose a protocol we didn't offer (when we are client), or - // something is seriously wrong. + // Maybe our peer chose a protocol we didn't offer (when we are client), + // or something is seriously wrong. std::ostringstream ss; for (auto i = alpn_allowed_.begin(); i != alpn_allowed_.end(); ++i) { ss << (i == alpn_allowed_.begin() ? " '" : ", '") << *i << "'"; @@ -1226,8 +1249,8 @@ PRBool TransportLayerDtls::WriteSrtpXtn(PRFileDesc* fd, if (message == ssl_hs_client_hello) { MOZ_ASSERT(self->role_ == CLIENT); MOZ_ASSERT(self->enabled_srtp_ciphers_.size(), "Haven't enabled SRTP"); - // We will take 2 octets for each cipher, plus a 2 octet length and 1 octet - // for the length of the empty MKI. + // We will take 2 octets for each cipher, plus a 2 octet length and 1 + // octet for the length of the empty MKI. if (max_len < self->enabled_srtp_ciphers_.size() * 2 + 3) { MOZ_ASSERT(false, "Not enough space to send SRTP extension"); return false; @@ -1357,8 +1380,8 @@ SECStatus TransportLayerDtls::HandleSrtpXtn( MOZ_ASSERT(self->role_ == SERVER); if (self->enabled_srtp_ciphers_.empty()) { // We don't have SRTP enabled, which is probably bad, but no sense in - // having the handshake fail at this point, let the client decide if this - // is a problem. + // having the handshake fail at this point, let the client decide if + // this is a problem. return SECSuccess; } diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml @@ -12161,12 +12161,6 @@ value: false mirror: always -# This pref disables using PQ crypto for WebRTC DTLS code. -- name: media.webrtc.enable_pq_dtls - type: RelaxedAtomicBool - value: @IS_EARLY_BETA_OR_EARLIER@ - mirror: always - # This pref controls whether dispatch testing-only events. - name: media.webvtt.testing.events type: bool diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js @@ -319,6 +319,12 @@ pref("media.videocontrols.keyboard-tab-to-all-controls", true); // 770 = DTLS 1.0, 771 = DTLS 1.2, 772 = DTLS 1.3 pref("media.peerconnection.dtls.version.min", 771); pref("media.peerconnection.dtls.version.max", 772); + pref("media.peerconnection.dtls.enable_pq_hybrid_kex", true); + #ifdef EARLY_BETA_OR_EARLIER + pref("media.peerconnection.dtls.send_mlkem_keyshare", true); + #else + pref("media.peerconnection.dtls.send_mlkem_keyshare", false); + #endif pref("media.peerconnection.sctp.default_max_streams", 2048); diff --git a/toolkit/components/nimbus/FeatureManifest.yaml b/toolkit/components/nimbus/FeatureManifest.yaml @@ -4843,7 +4843,7 @@ pqcrypto: type: boolean setPref: branch: default - pref: media.webrtc.enable_pq_dtls + pref: media.peerconnection.dtls.enable_pq_hybrid_kex description: >- Whether to enable mlkem768x25519 for DTLS in WebRTC.