Allow Configuration of Stateless Retry Key #5112

anrossi · 2025-05-21T05:50:50Z

Description

Add a new Global SetParam to set the Stateless Retry key, algorithm, and key rotation interval.
Fixes #5005.

Testing

New tests added covering the invalid parameter cases.

Documentation

Settings.md was updated to describe the new parameter.

src/test/lib/ApiTest.cpp

src/core/library.c

src/core/library.h

src/inc/msquic.h

github-actions · 2025-05-22T23:51:00Z

Cargo - windows-latest

The rust bindings need to be updated. Please apply (git apply) this patch:

diff --git a/src/rs/ffi/win_bindings.rs b/src/rs/ffi/win_bindings.rs
index 5b5a00e..ff0bfe3 100644
--- a/src/rs/ffi/win_bindings.rs
+++ b/src/rs/ffi/win_bindings.rs
@@ -168,6 +168,7 @@ pub const QUIC_PARAM_GLOBAL_LIBRARY_GIT_HASH: u32 = 16777224;
 pub const QUIC_PARAM_GLOBAL_EXECUTION_CONFIG: u32 = 16777225;
 pub const QUIC_PARAM_GLOBAL_TLS_PROVIDER: u32 = 16777226;
 pub const QUIC_PARAM_GLOBAL_STATELESS_RESET_KEY: u32 = 16777227;
+pub const QUIC_PARAM_GLOBAL_STATELESS_RETRY_CONFIG: u32 = 16777228;
 pub const QUIC_PARAM_CONFIGURATION_SETTINGS: u32 = 50331648;
 pub const QUIC_PARAM_CONFIGURATION_TICKET_KEYS: u32 = 50331649;
 pub const QUIC_PARAM_CONFIGURATION_VERSION_SETTINGS: u32 = 50331650;
@@ -4848,6 +4849,34 @@ const _: () = {
     ["Offset of field: QUIC_STREAM_STATISTICS::StreamBlockedByAppUs"]
         [::std::mem::offset_of!(QUIC_STREAM_STATISTICS, StreamBlockedByAppUs) - 56usize];
 };
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_AES_128_GCM: QUIC_AEAD_ALGORITHM_TYPE = 0;
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_AES_256_GCM: QUIC_AEAD_ALGORITHM_TYPE = 1;
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_CHACHA20_POLY1305: QUIC_AEAD_ALGORITHM_TYPE =
+    2;
+pub type QUIC_AEAD_ALGORITHM_TYPE = ::std::os::raw::c_int;
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct QUIC_STATELESS_RETRY_CONFIG {
+    pub Algorithm: QUIC_AEAD_ALGORITHM_TYPE,
+    pub RotationMs: u32,
+    pub SecretLength: u32,
+    pub Secret: [u8; 1usize],
+}
+#[allow(clippy::unnecessary_operation, clippy::identity_op)]
+const _: () = {
+    ["Size of QUIC_STATELESS_RETRY_CONFIG"]
+        [::std::mem::size_of::<QUIC_STATELESS_RETRY_CONFIG>() - 16usize];
+    ["Alignment of QUIC_STATELESS_RETRY_CONFIG"]
+        [::std::mem::align_of::<QUIC_STATELESS_RETRY_CONFIG>() - 4usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::Algorithm"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, Algorithm) - 0usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::RotationMs"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, RotationMs) - 4usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::SecretLength"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, SecretLength) - 8usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::Secret"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, Secret) - 12usize];
+};
 pub type QUIC_SET_CONTEXT_FN = ::std::option::Option<
     unsafe extern "C" fn(Handle: HQUIC, Context: *mut ::std::os::raw::c_void),
 >;

github-actions · 2025-05-22T23:59:12Z

Cargo - windows-latest

The rust bindings need to be updated. Please apply (git apply) this patch:

diff --git a/src/rs/ffi/win_bindings.rs b/src/rs/ffi/win_bindings.rs
index 5b5a00e..ff0bfe3 100644
--- a/src/rs/ffi/win_bindings.rs
+++ b/src/rs/ffi/win_bindings.rs
@@ -168,6 +168,7 @@ pub const QUIC_PARAM_GLOBAL_LIBRARY_GIT_HASH: u32 = 16777224;
 pub const QUIC_PARAM_GLOBAL_EXECUTION_CONFIG: u32 = 16777225;
 pub const QUIC_PARAM_GLOBAL_TLS_PROVIDER: u32 = 16777226;
 pub const QUIC_PARAM_GLOBAL_STATELESS_RESET_KEY: u32 = 16777227;
+pub const QUIC_PARAM_GLOBAL_STATELESS_RETRY_CONFIG: u32 = 16777228;
 pub const QUIC_PARAM_CONFIGURATION_SETTINGS: u32 = 50331648;
 pub const QUIC_PARAM_CONFIGURATION_TICKET_KEYS: u32 = 50331649;
 pub const QUIC_PARAM_CONFIGURATION_VERSION_SETTINGS: u32 = 50331650;
@@ -4848,6 +4849,34 @@ const _: () = {
     ["Offset of field: QUIC_STREAM_STATISTICS::StreamBlockedByAppUs"]
         [::std::mem::offset_of!(QUIC_STREAM_STATISTICS, StreamBlockedByAppUs) - 56usize];
 };
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_AES_128_GCM: QUIC_AEAD_ALGORITHM_TYPE = 0;
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_AES_256_GCM: QUIC_AEAD_ALGORITHM_TYPE = 1;
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_CHACHA20_POLY1305: QUIC_AEAD_ALGORITHM_TYPE =
+    2;
+pub type QUIC_AEAD_ALGORITHM_TYPE = ::std::os::raw::c_int;
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct QUIC_STATELESS_RETRY_CONFIG {
+    pub Algorithm: QUIC_AEAD_ALGORITHM_TYPE,
+    pub RotationMs: u32,
+    pub SecretLength: u32,
+    pub Secret: [u8; 1usize],
+}
+#[allow(clippy::unnecessary_operation, clippy::identity_op)]
+const _: () = {
+    ["Size of QUIC_STATELESS_RETRY_CONFIG"]
+        [::std::mem::size_of::<QUIC_STATELESS_RETRY_CONFIG>() - 16usize];
+    ["Alignment of QUIC_STATELESS_RETRY_CONFIG"]
+        [::std::mem::align_of::<QUIC_STATELESS_RETRY_CONFIG>() - 4usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::Algorithm"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, Algorithm) - 0usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::RotationMs"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, RotationMs) - 4usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::SecretLength"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, SecretLength) - 8usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::Secret"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, Secret) - 12usize];
+};
 pub type QUIC_SET_CONTEXT_FN = ::std::option::Option<
     unsafe extern "C" fn(Handle: HQUIC, Context: *mut ::std::os::raw::c_void),
 >;

codecov · 2025-05-23T00:26:25Z

Codecov Report

Attention: Patch coverage is 96.15385% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.96%. Comparing base (31d2b73) to head (747da5a).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/core/library.c	96.73%	3 Missing ⚠️
src/core/partition.c	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5112      +/-   ##
==========================================
- Coverage   87.19%   86.96%   -0.24%     
==========================================
  Files          59       59              
  Lines       18086    18183      +97     
==========================================
+ Hits        15770    15812      +42     
- Misses       2316     2371      +55

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/core/library.c

github-actions · 2025-06-14T06:59:40Z

Cargo - ubuntu-latest

The rust bindings need to be updated. Please apply (git apply) this patch:

diff --git a/src/rs/ffi/linux_bindings.rs b/src/rs/ffi/linux_bindings.rs
index cf4039b..01b44d4 100644
--- a/src/rs/ffi/linux_bindings.rs
+++ b/src/rs/ffi/linux_bindings.rs
@@ -175,6 +175,7 @@ pub const QUIC_PARAM_GLOBAL_EXECUTION_CONFIG: u32 = 16777225;
 pub const QUIC_PARAM_GLOBAL_TLS_PROVIDER: u32 = 16777226;
 pub const QUIC_PARAM_GLOBAL_STATELESS_RESET_KEY: u32 = 16777227;
 pub const QUIC_PARAM_GLOBAL_STATISTICS_V2_SIZES: u32 = 16777228;
+pub const QUIC_PARAM_GLOBAL_STATELESS_RETRY_CONFIG: u32 = 16777229;
 pub const QUIC_PARAM_CONFIGURATION_SETTINGS: u32 = 50331648;
 pub const QUIC_PARAM_CONFIGURATION_TICKET_KEYS: u32 = 50331649;
 pub const QUIC_PARAM_CONFIGURATION_VERSION_SETTINGS: u32 = 50331650;
@@ -4884,6 +4885,34 @@ const _: () = {
     ["Offset of field: QUIC_STREAM_STATISTICS::StreamBlockedByAppUs"]
         [::std::mem::offset_of!(QUIC_STREAM_STATISTICS, StreamBlockedByAppUs) - 56usize];
 };
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_AES_128_GCM: QUIC_AEAD_ALGORITHM_TYPE = 0;
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_AES_256_GCM: QUIC_AEAD_ALGORITHM_TYPE = 1;
+pub const QUIC_AEAD_ALGORITHM_TYPE_QUIC_AEAD_ALGORITHM_CHACHA20_POLY1305: QUIC_AEAD_ALGORITHM_TYPE =
+    2;
+pub type QUIC_AEAD_ALGORITHM_TYPE = ::std::os::raw::c_uint;
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct QUIC_STATELESS_RETRY_CONFIG {
+    pub Algorithm: QUIC_AEAD_ALGORITHM_TYPE,
+    pub RotationMs: u32,
+    pub SecretLength: u32,
+    pub Secret: *const u8,
+}
+#[allow(clippy::unnecessary_operation, clippy::identity_op)]
+const _: () = {
+    ["Size of QUIC_STATELESS_RETRY_CONFIG"]
+        [::std::mem::size_of::<QUIC_STATELESS_RETRY_CONFIG>() - 24usize];
+    ["Alignment of QUIC_STATELESS_RETRY_CONFIG"]
+        [::std::mem::align_of::<QUIC_STATELESS_RETRY_CONFIG>() - 8usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::Algorithm"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, Algorithm) - 0usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::RotationMs"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, RotationMs) - 4usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::SecretLength"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, SecretLength) - 8usize];
+    ["Offset of field: QUIC_STATELESS_RETRY_CONFIG::Secret"]
+        [::std::mem::offset_of!(QUIC_STATELESS_RETRY_CONFIG, Secret) - 16usize];
+};
 pub type QUIC_SET_CONTEXT_FN = ::std::option::Option<
     unsafe extern "C" fn(Handle: HQUIC, Context: *mut ::std::os::raw::c_void),
 >;

src/inc/msquic.h

docs/Settings.md

src/core/library.c

src/core/partition.c

src/core/library.c

src/test/bin/quic_gtest.cpp

src/test/lib/ApiTest.cpp

docs/Settings.md

src/inc/quic_crypt.h

src/core/partition.c

src/core/quicdef.h

src/core/library.c

…pdate Csharp API

sm-msft · 2025-06-20T20:56:18Z

docs/api/QUIC_STATELESS_RETRY_CONFIG.md

+
+The AEAD algorithm used for protecting the retry token. Must be one of the following constants:
+
+Constant |  Key Length


Should this be "key size" to match the description of "SecretLength" below?

sm-msft · 2025-06-20T20:59:21Z

docs/api/QUIC_STATELESS_RETRY_CONFIG.md

+The interval to rotate the retry key. 30,000ms is the default.
+
+`SecretLength`
+


SecretLengthBytes to disambiguate between the bits used in algorithm name and the bytes required to be passed here?

sm-msft · 2025-06-20T21:03:30Z

docs/api/QUIC_STATELESS_RETRY_CONFIG.md

+
+`RotationMs`
+
+The interval to rotate the retry key. 30,000ms is the default.


I am just curious about the default value here - 30 second key rotation seems too frequent. Does this stem from requirements or something we do in practice?

sm-msft · 2025-06-20T21:13:58Z

src/core/partition.c

@@ -144,15 +157,19 @@ QuicPartitionGetStatelessRetryKeyForTimestamp(
    )
 {
    const int64_t Now = CxPlatTimeEpochMs64();


Unrelated to your PR, but this code is susceptible to issues arising from system clock rollback. This should use CxPlatTimeUs64() instead of CxPlatTimeEpochMs64.

This applies to all functions that access QuicPartitionGetCurrentStatelessRetryKey() function.

This time is required to be synchronized across all load balanced machines to allow for distributed encryption and decryption.

sm-msft · 2025-06-20T21:45:07Z

src/core/library.c

+            AlgSecretLen);
+        return QUIC_STATUS_INVALID_PARAMETER;
+    }
+    CXPLAT_DBG_ASSERT(AlgSecretLen <= sizeof(MsQuicLib.StatelessRetry.BaseSecret));


We could make this FRE assert due to potential implication of the condition not being met.

sm-msft · 2025-06-20T21:53:28Z

src/core/library.c

+    CXPLAT_DBG_ASSERT(AlgSecretLen <= sizeof(MsQuicLib.StatelessRetry.BaseSecret));
+
+    CxPlatDispatchRwLockAcquireExclusive(&MsQuicLib.StatelessRetry.Lock, PrevIrql);
+    if (Config->Secret != MsQuicLib.StatelessRetry.BaseSecret) {


Can we return an error under this condition?

sm-msft · 2025-06-20T22:03:19Z

src/core/partition.c

-    const int64_t CurrentKeyIndex = Now / QUIC_STATELESS_RETRY_KEY_LIFETIME_MS;
-    const int64_t KeyIndex = Timestamp / QUIC_STATELESS_RETRY_KEY_LIFETIME_MS;
+    CxPlatDispatchRwLockAcquireShared(&MsQuicLib.StatelessRetry.Lock, PrevIrql);
+    const int64_t CurrentKeyIndex = Now / MsQuicLib.StatelessRetry.KeyRotationMs;


How would this indexing be impacted if the settings change a small key rotation interval to a large key rotation interval?

src/core/unittest/main.h

sm-msft · 2025-06-20T23:23:17Z

src/core/library.c

@@ -334,6 +345,78 @@ QuicPerfCounterSnapShot(
        sizeof(PerfCounterSamples));
 }

+_IRQL_requires_max_(PASSIVE_LEVEL)
+void
+QuicLibraryLoadRetryConfig(


Msg'd Anthony separately about this function.

For the sake of the review, it'd be awesome to have a quick summary of what the discussion was about and what the conclusion was :)

anrossi added 4 commits May 21, 2025 05:00

Internal key rotation logic

8ba591a

Add SetParam interface

00962ac

Add API test

a06eea8

Add documentation

4a81a65

anrossi added this to DPT Iteration Tracker May 21, 2025

anrossi requested a review from a team as a code owner May 21, 2025 05:50

anrossi added Area: API Area: Core Related to the shared, core protocol logic labels May 21, 2025