Skip to content

Add versioning support for Connection resumption ticket #5114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

sm-msft
Copy link
Contributor

@sm-msft sm-msft commented May 22, 2025

Update: Given the various unrelated changes that have accumulated in this PR, I will split this into two PRs and abandon this one.

The crux of the changes are in this PR: #5131
Will post a link to the second PR when it is ready.

Addresses the issue #5115

Description

This change adds the ability to version the connection resumption ticket on the server and the ability to filter out desirable versions of the tickets on the client.

  • Min and max resumption ticket versions are hardcoded to 1 and 2 by default
  • Add several missing tests for existing settings and add storage platform API to support these tests
  • Check if the received resumption ticket is in a valid range on the client
  • V2 resumption ticket includes a 64 byte fixed size buffer for usage in the near future
  • NULL ptr checks in CxPlatStorageReadValue
  • Other minor test updates

Testing

Unit tests have been updated and expanded to test this change.

Documentation

TBD

- Min and max resumption ticket version is set to 1 by default
- Update API to read/set the new versions through QUIC_SETTINGS
- New settings can be read from storage(registry)
- Add tests for new version fields, as well as several missing tests for existing settings
- Use the max resumption ticket version when issuing the ticket on the server
- Check if the received resumption ticket is in a valid range on the client
- NULL ptr checks in CxPlatStorageReadValue
- Other minor test updates
Copy link

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 2523e0a..ad966ad 100644
--- a/src/rs/ffi/linux_bindings.rs
+++ b/src/rs/ffi/linux_bindings.rs
@@ -1925,6 +1925,8 @@ pub struct QUIC_SETTINGS {
     pub StreamRecvWindowBidiLocalDefault: u32,
     pub StreamRecvWindowBidiRemoteDefault: u32,
     pub StreamRecvWindowUnidiDefault: u32,
+    pub ResumptionTicketMinVersion: u8,
+    pub ResumptionTicketMaxVersion: u8,
 }
 #[repr(C)]
 #[derive(Copy, Clone)]
@@ -1936,7 +1938,7 @@ pub union QUIC_SETTINGS__bindgen_ty_1 {
 #[repr(align(8))]
 #[derive(Debug, Copy, Clone)]
 pub struct QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
-    pub _bitfield_align_1: [u32; 0],
+    pub _bitfield_align_1: [u16; 0],
     pub _bitfield_1: __BindgenBitfieldUnit<[u8; 8usize]>,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
@@ -3466,14 +3468,80 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         }
     }
     #[inline]
+    pub fn ResumptionTicketMinVersion(&self) -> u64 {
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 1u8) as u64) }
+    }
+    #[inline]
+    pub fn set_ResumptionTicketMinVersion(&mut self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            self._bitfield_1.set(46usize, 1u8, val as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn ResumptionTicketMinVersion_raw(this: *const Self) -> u64 {
+        unsafe {
+            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
+                ::std::ptr::addr_of!((*this)._bitfield_1),
+                46usize,
+                1u8,
+            ) as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn set_ResumptionTicketMinVersion_raw(this: *mut Self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
+                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
+                46usize,
+                1u8,
+                val as u64,
+            )
+        }
+    }
+    #[inline]
+    pub fn ResumptionTicketMaxVersion(&self) -> u64 {
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(47usize, 1u8) as u64) }
+    }
+    #[inline]
+    pub fn set_ResumptionTicketMaxVersion(&mut self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            self._bitfield_1.set(47usize, 1u8, val as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn ResumptionTicketMaxVersion_raw(this: *const Self) -> u64 {
+        unsafe {
+            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
+                ::std::ptr::addr_of!((*this)._bitfield_1),
+                47usize,
+                1u8,
+            ) as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn set_ResumptionTicketMaxVersion_raw(this: *mut Self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
+                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
+                47usize,
+                1u8,
+                val as u64,
+            )
+        }
+    }
+    #[inline]
     pub fn RESERVED(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 18u8) as u64) }
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(48usize, 16u8) as u64) }
     }
     #[inline]
     pub fn set_RESERVED(&mut self, val: u64) {
         unsafe {
             let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(46usize, 18u8, val as u64)
+            self._bitfield_1.set(48usize, 16u8, val as u64)
         }
     }
     #[inline]
@@ -3481,8 +3549,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         unsafe {
             ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
                 ::std::ptr::addr_of!((*this)._bitfield_1),
-                46usize,
-                18u8,
+                48usize,
+                16u8,
             ) as u64)
         }
     }
@@ -3492,8 +3560,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let val: u64 = ::std::mem::transmute(val);
             <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
                 ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                46usize,
-                18u8,
+                48usize,
+                16u8,
                 val as u64,
             )
         }
@@ -3546,6 +3614,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         XdpEnabled: u64,
         QTIPEnabled: u64,
         RioEnabled: u64,
+        ResumptionTicketMinVersion: u64,
+        ResumptionTicketMaxVersion: u64,
         RESERVED: u64,
     ) -> __BindgenBitfieldUnit<[u8; 8usize]> {
         let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 8usize]> = Default::default();
@@ -3755,7 +3825,17 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let RioEnabled: u64 = unsafe { ::std::mem::transmute(RioEnabled) };
             RioEnabled as u64
         });
-        __bindgen_bitfield_unit.set(46usize, 18u8, {
+        __bindgen_bitfield_unit.set(46usize, 1u8, {
+            let ResumptionTicketMinVersion: u64 =
+                unsafe { ::std::mem::transmute(ResumptionTicketMinVersion) };
+            ResumptionTicketMinVersion as u64
+        });
+        __bindgen_bitfield_unit.set(47usize, 1u8, {
+            let ResumptionTicketMaxVersion: u64 =
+                unsafe { ::std::mem::transmute(ResumptionTicketMaxVersion) };
+            ResumptionTicketMaxVersion as u64
+        });
+        __bindgen_bitfield_unit.set(48usize, 16u8, {
             let RESERVED: u64 = unsafe { ::std::mem::transmute(RESERVED) };
             RESERVED as u64
         });
@@ -4255,6 +4335,10 @@ const _: () = {
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowBidiRemoteDefault) - 132usize];
     ["Offset of field: QUIC_SETTINGS::StreamRecvWindowUnidiDefault"]
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowUnidiDefault) - 136usize];
+    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMinVersion"]
+        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMinVersion) - 140usize];
+    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMaxVersion"]
+        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMaxVersion) - 141usize];
 };
 impl QUIC_SETTINGS {
     #[inline]

Copy link

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..beba39c 100644
--- a/src/rs/ffi/win_bindings.rs
+++ b/src/rs/ffi/win_bindings.rs
@@ -1919,6 +1919,8 @@ pub struct QUIC_SETTINGS {
     pub StreamRecvWindowBidiLocalDefault: u32,
     pub StreamRecvWindowBidiRemoteDefault: u32,
     pub StreamRecvWindowUnidiDefault: u32,
+    pub ResumptionTicketMinVersion: u8,
+    pub ResumptionTicketMaxVersion: u8,
 }
 #[repr(C)]
 #[derive(Copy, Clone)]
@@ -1930,7 +1932,7 @@ pub union QUIC_SETTINGS__bindgen_ty_1 {
 #[repr(align(8))]
 #[derive(Debug, Copy, Clone)]
 pub struct QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
-    pub _bitfield_align_1: [u32; 0],
+    pub _bitfield_align_1: [u16; 0],
     pub _bitfield_1: __BindgenBitfieldUnit<[u8; 8usize]>,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
@@ -3460,14 +3462,80 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         }
     }
     #[inline]
+    pub fn ResumptionTicketMinVersion(&self) -> u64 {
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 1u8) as u64) }
+    }
+    #[inline]
+    pub fn set_ResumptionTicketMinVersion(&mut self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            self._bitfield_1.set(46usize, 1u8, val as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn ResumptionTicketMinVersion_raw(this: *const Self) -> u64 {
+        unsafe {
+            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
+                ::std::ptr::addr_of!((*this)._bitfield_1),
+                46usize,
+                1u8,
+            ) as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn set_ResumptionTicketMinVersion_raw(this: *mut Self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
+                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
+                46usize,
+                1u8,
+                val as u64,
+            )
+        }
+    }
+    #[inline]
+    pub fn ResumptionTicketMaxVersion(&self) -> u64 {
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(47usize, 1u8) as u64) }
+    }
+    #[inline]
+    pub fn set_ResumptionTicketMaxVersion(&mut self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            self._bitfield_1.set(47usize, 1u8, val as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn ResumptionTicketMaxVersion_raw(this: *const Self) -> u64 {
+        unsafe {
+            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
+                ::std::ptr::addr_of!((*this)._bitfield_1),
+                47usize,
+                1u8,
+            ) as u64)
+        }
+    }
+    #[inline]
+    pub unsafe fn set_ResumptionTicketMaxVersion_raw(this: *mut Self, val: u64) {
+        unsafe {
+            let val: u64 = ::std::mem::transmute(val);
+            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
+                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
+                47usize,
+                1u8,
+                val as u64,
+            )
+        }
+    }
+    #[inline]
     pub fn RESERVED(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 18u8) as u64) }
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(48usize, 16u8) as u64) }
     }
     #[inline]
     pub fn set_RESERVED(&mut self, val: u64) {
         unsafe {
             let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(46usize, 18u8, val as u64)
+            self._bitfield_1.set(48usize, 16u8, val as u64)
         }
     }
     #[inline]
@@ -3475,8 +3543,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         unsafe {
             ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
                 ::std::ptr::addr_of!((*this)._bitfield_1),
-                46usize,
-                18u8,
+                48usize,
+                16u8,
             ) as u64)
         }
     }
@@ -3486,8 +3554,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let val: u64 = ::std::mem::transmute(val);
             <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
                 ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                46usize,
-                18u8,
+                48usize,
+                16u8,
                 val as u64,
             )
         }
@@ -3540,6 +3608,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         XdpEnabled: u64,
         QTIPEnabled: u64,
         RioEnabled: u64,
+        ResumptionTicketMinVersion: u64,
+        ResumptionTicketMaxVersion: u64,
         RESERVED: u64,
     ) -> __BindgenBitfieldUnit<[u8; 8usize]> {
         let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 8usize]> = Default::default();
@@ -3749,7 +3819,17 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let RioEnabled: u64 = unsafe { ::std::mem::transmute(RioEnabled) };
             RioEnabled as u64
         });
-        __bindgen_bitfield_unit.set(46usize, 18u8, {
+        __bindgen_bitfield_unit.set(46usize, 1u8, {
+            let ResumptionTicketMinVersion: u64 =
+                unsafe { ::std::mem::transmute(ResumptionTicketMinVersion) };
+            ResumptionTicketMinVersion as u64
+        });
+        __bindgen_bitfield_unit.set(47usize, 1u8, {
+            let ResumptionTicketMaxVersion: u64 =
+                unsafe { ::std::mem::transmute(ResumptionTicketMaxVersion) };
+            ResumptionTicketMaxVersion as u64
+        });
+        __bindgen_bitfield_unit.set(48usize, 16u8, {
             let RESERVED: u64 = unsafe { ::std::mem::transmute(RESERVED) };
             RESERVED as u64
         });
@@ -4249,6 +4329,10 @@ const _: () = {
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowBidiRemoteDefault) - 132usize];
     ["Offset of field: QUIC_SETTINGS::StreamRecvWindowUnidiDefault"]
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowUnidiDefault) - 136usize];
+    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMinVersion"]
+        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMinVersion) - 140usize];
+    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMaxVersion"]
+        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMaxVersion) - 141usize];
 };
 impl QUIC_SETTINGS {
     #[inline]

Copy link

codecov bot commented May 22, 2025

Codecov Report

Attention: Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 86.87%. Comparing base (65d6f63) to head (f176b1b).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/core/crypto.c 76.92% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5114      +/-   ##
==========================================
+ Coverage   86.47%   86.87%   +0.40%     
==========================================
  Files          59       59              
  Lines       18035    18059      +24     
==========================================
+ Hits        15595    15688      +93     
+ Misses       2440     2371      -69     

☔ 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.

@anrossi
Copy link
Contributor

anrossi commented May 23, 2025

Why does the application need to set resumption ticket versions? The resumption ticket is opaque to the application, so the version of a ticket is for MsQuic's own benefit for parsing.

@anrossi
Copy link
Contributor

anrossi commented May 23, 2025

For careful resume, the application should control whether it wants to use careful resume, not which version of the ticket to send. If careful resume is enabled on a server, send a version 2 ticket. If a client receives a version 2 ticket, but doesn't have careful resume enabled, ignore the version 2 field, and just use the version 1 fields.
The intent is much clearer than controlling the version of the ticket being sent/received to determine careful resume.

@sm-msft
Copy link
Contributor Author

sm-msft commented May 23, 2025

Why does the application need to set resumption ticket versions? The resumption ticket is opaque to the application, so the version of a ticket is for MsQuic's own benefit for parsing.

Part of the resumption ticket is visible to the client application and this versioning will help the client filter out tickets it cannot parse.

@csujedihy
Copy link
Contributor

Why does the application need to set resumption ticket versions? The resumption ticket is opaque to the application, so the version of a ticket is for MsQuic's own benefit for parsing.

Part of the resumption ticket is visible to the client application and this versioning will help the client filter out tickets it cannot parse.

Looking at the code, it doesn't seem like this is the case. The ticket, where ticket version, TP and whatnot are encoded in QuicCryptoEncodeServerTicket and then passed to schannel/quictls to encrypt (QuicCryptoEncodeServerTicket->QuicCryptoProcessAppData->CxPlatTlsProcessData). I don't see any code that skips the first few bytes.

@nibanks
Copy link
Member

nibanks commented May 23, 2025

Why does the application need to set resumption ticket versions? The resumption ticket is opaque to the application, so the version of a ticket is for MsQuic's own benefit for parsing.

Part of the resumption ticket is visible to the client application and this versioning will help the client filter out tickets it cannot parse.

Looking at the code, it doesn't seem like this is the case. The ticket, where ticket version, TP and whatnot are encoded in QuicCryptoEncodeServerTicket and then passed to schannel/quictls to encrypt (QuicCryptoEncodeServerTicket->QuicCryptoProcessAppData->CxPlatTlsProcessData). I don't see any code that skips the first few bytes.

The format/version of the ticket is not exposed to the app. They can have their own blob as a part of it, but we only give them back their part. They should have their own version, which is completely independent of MsQuic's.

@anrossi
Copy link
Contributor

anrossi commented May 23, 2025

Why does the application need to set resumption ticket versions? The resumption ticket is opaque to the application, so the version of a ticket is for MsQuic's own benefit for parsing.

Part of the resumption ticket is visible to the client application and this versioning will help the client filter out tickets it cannot parse.

The client shouldn't need to think about things like this. The client application, when it gets the resumption token from MsQuic, is supposed to treat it like an opaque string of bytes and save the ticket without modification for use later.

If a client which supports careful resume receives a V1 resumption ticket from the server, that means it can't do careful resume, but it can still do resumption.
If a client which doesn't support careful resume (or has it turned off) receives a V2 resumption ticket from the server, it can't do careful resume, but it must still be able to do resumption.
We can't break resumption for careful resume.

@csujedihy
Copy link
Contributor

Why does the application need to set resumption ticket versions? The resumption ticket is opaque to the application, so the version of a ticket is for MsQuic's own benefit for parsing.

Part of the resumption ticket is visible to the client application and this versioning will help the client filter out tickets it cannot parse.

Looking at the code, it doesn't seem like this is the case. The ticket, where ticket version, TP and whatnot are encoded in QuicCryptoEncodeServerTicket and then passed to schannel/quictls to encrypt (QuicCryptoEncodeServerTicket->QuicCryptoProcessAppData->CxPlatTlsProcessData). I don't see any code that skips the first few bytes.

The format/version of the ticket is not exposed to the app. They can have their own blob as a part of it, but we only give them back their part. They should have their own version, which is completely independent of MsQuic's.

So, the metadata like ticket version, quic version and TP etc can actually be decrypted and be decoded if the client is also msquic? My bad. I thought it couldn't be even decrypted by the client.

@sm-msft
Copy link
Contributor Author

sm-msft commented May 23, 2025

Part of the resumption ticket is visible to the client application and this versioning will help the client filter out tickets it cannot parse.

Discussed this with Anthony - this is not the case here.

@sm-msft sm-msft closed this May 23, 2025
@sm-msft sm-msft reopened this May 23, 2025
sm-msft added 3 commits May 23, 2025 18:55
- Address PR comments, including coding bot comments
- Update settings test to use new API and minor code cleanup
Copy link

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 5eb87c1..283e538 100644
--- a/src/rs/ffi/linux_bindings.rs
+++ b/src/rs/ffi/linux_bindings.rs
@@ -1926,8 +1926,6 @@ pub struct QUIC_SETTINGS {
     pub StreamRecvWindowBidiLocalDefault: u32,
     pub StreamRecvWindowBidiRemoteDefault: u32,
     pub StreamRecvWindowUnidiDefault: u32,
-    pub ResumptionTicketMinVersion: u8,
-    pub ResumptionTicketMaxVersion: u8,
 }
 #[repr(C)]
 #[derive(Copy, Clone)]
@@ -1939,7 +1937,7 @@ pub union QUIC_SETTINGS__bindgen_ty_1 {
 #[repr(align(8))]
 #[derive(Debug, Copy, Clone)]
 pub struct QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
-    pub _bitfield_align_1: [u16; 0],
+    pub _bitfield_align_1: [u32; 0],
     pub _bitfield_1: __BindgenBitfieldUnit<[u8; 8usize]>,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
@@ -3469,80 +3467,14 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         }
     }
     #[inline]
-    pub fn ResumptionTicketMinVersion(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 1u8) as u64) }
-    }
-    #[inline]
-    pub fn set_ResumptionTicketMinVersion(&mut self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(46usize, 1u8, val as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn ResumptionTicketMinVersion_raw(this: *const Self) -> u64 {
-        unsafe {
-            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
-                ::std::ptr::addr_of!((*this)._bitfield_1),
-                46usize,
-                1u8,
-            ) as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn set_ResumptionTicketMinVersion_raw(this: *mut Self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
-                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                46usize,
-                1u8,
-                val as u64,
-            )
-        }
-    }
-    #[inline]
-    pub fn ResumptionTicketMaxVersion(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(47usize, 1u8) as u64) }
-    }
-    #[inline]
-    pub fn set_ResumptionTicketMaxVersion(&mut self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(47usize, 1u8, val as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn ResumptionTicketMaxVersion_raw(this: *const Self) -> u64 {
-        unsafe {
-            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
-                ::std::ptr::addr_of!((*this)._bitfield_1),
-                47usize,
-                1u8,
-            ) as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn set_ResumptionTicketMaxVersion_raw(this: *mut Self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
-                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                47usize,
-                1u8,
-                val as u64,
-            )
-        }
-    }
-    #[inline]
     pub fn RESERVED(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(48usize, 16u8) as u64) }
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 18u8) as u64) }
     }
     #[inline]
     pub fn set_RESERVED(&mut self, val: u64) {
         unsafe {
             let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(48usize, 16u8, val as u64)
+            self._bitfield_1.set(46usize, 18u8, val as u64)
         }
     }
     #[inline]
@@ -3550,8 +3482,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         unsafe {
             ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
                 ::std::ptr::addr_of!((*this)._bitfield_1),
-                48usize,
-                16u8,
+                46usize,
+                18u8,
             ) as u64)
         }
     }
@@ -3561,8 +3493,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let val: u64 = ::std::mem::transmute(val);
             <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
                 ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                48usize,
-                16u8,
+                46usize,
+                18u8,
                 val as u64,
             )
         }
@@ -3615,8 +3547,6 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         XdpEnabled: u64,
         QTIPEnabled: u64,
         RioEnabled: u64,
-        ResumptionTicketMinVersion: u64,
-        ResumptionTicketMaxVersion: u64,
         RESERVED: u64,
     ) -> __BindgenBitfieldUnit<[u8; 8usize]> {
         let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 8usize]> = Default::default();
@@ -3826,17 +3756,7 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let RioEnabled: u64 = unsafe { ::std::mem::transmute(RioEnabled) };
             RioEnabled as u64
         });
-        __bindgen_bitfield_unit.set(46usize, 1u8, {
-            let ResumptionTicketMinVersion: u64 =
-                unsafe { ::std::mem::transmute(ResumptionTicketMinVersion) };
-            ResumptionTicketMinVersion as u64
-        });
-        __bindgen_bitfield_unit.set(47usize, 1u8, {
-            let ResumptionTicketMaxVersion: u64 =
-                unsafe { ::std::mem::transmute(ResumptionTicketMaxVersion) };
-            ResumptionTicketMaxVersion as u64
-        });
-        __bindgen_bitfield_unit.set(48usize, 16u8, {
+        __bindgen_bitfield_unit.set(46usize, 18u8, {
             let RESERVED: u64 = unsafe { ::std::mem::transmute(RESERVED) };
             RESERVED as u64
         });
@@ -4336,10 +4256,6 @@ const _: () = {
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowBidiRemoteDefault) - 132usize];
     ["Offset of field: QUIC_SETTINGS::StreamRecvWindowUnidiDefault"]
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowUnidiDefault) - 136usize];
-    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMinVersion"]
-        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMinVersion) - 140usize];
-    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMaxVersion"]
-        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMaxVersion) - 141usize];
 };
 impl QUIC_SETTINGS {
     #[inline]

Copy link

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 503f07b..c2af262 100644
--- a/src/rs/ffi/win_bindings.rs
+++ b/src/rs/ffi/win_bindings.rs
@@ -1920,8 +1920,6 @@ pub struct QUIC_SETTINGS {
     pub StreamRecvWindowBidiLocalDefault: u32,
     pub StreamRecvWindowBidiRemoteDefault: u32,
     pub StreamRecvWindowUnidiDefault: u32,
-    pub ResumptionTicketMinVersion: u8,
-    pub ResumptionTicketMaxVersion: u8,
 }
 #[repr(C)]
 #[derive(Copy, Clone)]
@@ -1933,7 +1931,7 @@ pub union QUIC_SETTINGS__bindgen_ty_1 {
 #[repr(align(8))]
 #[derive(Debug, Copy, Clone)]
 pub struct QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
-    pub _bitfield_align_1: [u16; 0],
+    pub _bitfield_align_1: [u32; 0],
     pub _bitfield_1: __BindgenBitfieldUnit<[u8; 8usize]>,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
@@ -3463,80 +3461,14 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         }
     }
     #[inline]
-    pub fn ResumptionTicketMinVersion(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 1u8) as u64) }
-    }
-    #[inline]
-    pub fn set_ResumptionTicketMinVersion(&mut self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(46usize, 1u8, val as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn ResumptionTicketMinVersion_raw(this: *const Self) -> u64 {
-        unsafe {
-            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
-                ::std::ptr::addr_of!((*this)._bitfield_1),
-                46usize,
-                1u8,
-            ) as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn set_ResumptionTicketMinVersion_raw(this: *mut Self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
-                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                46usize,
-                1u8,
-                val as u64,
-            )
-        }
-    }
-    #[inline]
-    pub fn ResumptionTicketMaxVersion(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(47usize, 1u8) as u64) }
-    }
-    #[inline]
-    pub fn set_ResumptionTicketMaxVersion(&mut self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(47usize, 1u8, val as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn ResumptionTicketMaxVersion_raw(this: *const Self) -> u64 {
-        unsafe {
-            ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
-                ::std::ptr::addr_of!((*this)._bitfield_1),
-                47usize,
-                1u8,
-            ) as u64)
-        }
-    }
-    #[inline]
-    pub unsafe fn set_ResumptionTicketMaxVersion_raw(this: *mut Self, val: u64) {
-        unsafe {
-            let val: u64 = ::std::mem::transmute(val);
-            <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
-                ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                47usize,
-                1u8,
-                val as u64,
-            )
-        }
-    }
-    #[inline]
     pub fn RESERVED(&self) -> u64 {
-        unsafe { ::std::mem::transmute(self._bitfield_1.get(48usize, 16u8) as u64) }
+        unsafe { ::std::mem::transmute(self._bitfield_1.get(46usize, 18u8) as u64) }
     }
     #[inline]
     pub fn set_RESERVED(&mut self, val: u64) {
         unsafe {
             let val: u64 = ::std::mem::transmute(val);
-            self._bitfield_1.set(48usize, 16u8, val as u64)
+            self._bitfield_1.set(46usize, 18u8, val as u64)
         }
     }
     #[inline]
@@ -3544,8 +3476,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         unsafe {
             ::std::mem::transmute(<__BindgenBitfieldUnit<[u8; 8usize]>>::raw_get(
                 ::std::ptr::addr_of!((*this)._bitfield_1),
-                48usize,
-                16u8,
+                46usize,
+                18u8,
             ) as u64)
         }
     }
@@ -3555,8 +3487,8 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let val: u64 = ::std::mem::transmute(val);
             <__BindgenBitfieldUnit<[u8; 8usize]>>::raw_set(
                 ::std::ptr::addr_of_mut!((*this)._bitfield_1),
-                48usize,
-                16u8,
+                46usize,
+                18u8,
                 val as u64,
             )
         }
@@ -3609,8 +3541,6 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
         XdpEnabled: u64,
         QTIPEnabled: u64,
         RioEnabled: u64,
-        ResumptionTicketMinVersion: u64,
-        ResumptionTicketMaxVersion: u64,
         RESERVED: u64,
     ) -> __BindgenBitfieldUnit<[u8; 8usize]> {
         let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 8usize]> = Default::default();
@@ -3820,17 +3750,7 @@ impl QUIC_SETTINGS__bindgen_ty_1__bindgen_ty_1 {
             let RioEnabled: u64 = unsafe { ::std::mem::transmute(RioEnabled) };
             RioEnabled as u64
         });
-        __bindgen_bitfield_unit.set(46usize, 1u8, {
-            let ResumptionTicketMinVersion: u64 =
-                unsafe { ::std::mem::transmute(ResumptionTicketMinVersion) };
-            ResumptionTicketMinVersion as u64
-        });
-        __bindgen_bitfield_unit.set(47usize, 1u8, {
-            let ResumptionTicketMaxVersion: u64 =
-                unsafe { ::std::mem::transmute(ResumptionTicketMaxVersion) };
-            ResumptionTicketMaxVersion as u64
-        });
-        __bindgen_bitfield_unit.set(48usize, 16u8, {
+        __bindgen_bitfield_unit.set(46usize, 18u8, {
             let RESERVED: u64 = unsafe { ::std::mem::transmute(RESERVED) };
             RESERVED as u64
         });
@@ -4330,10 +4250,6 @@ const _: () = {
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowBidiRemoteDefault) - 132usize];
     ["Offset of field: QUIC_SETTINGS::StreamRecvWindowUnidiDefault"]
         [::std::mem::offset_of!(QUIC_SETTINGS, StreamRecvWindowUnidiDefault) - 136usize];
-    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMinVersion"]
-        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMinVersion) - 140usize];
-    ["Offset of field: QUIC_SETTINGS::ResumptionTicketMaxVersion"]
-        [::std::mem::offset_of!(QUIC_SETTINGS, ResumptionTicketMaxVersion) - 141usize];
 };
 impl QUIC_SETTINGS {
     #[inline]

sm-msft and others added 4 commits May 28, 2025 16:48
Co-authored-by: Nick Banks <[email protected]>
- Remove settings product and test code around resumption ticket versioning
- Update CLOG files after settings update
- Included a fixed size buffer for V2 ticket extension
- Update resumption ticket tests around encoding v2 and decoding v1, v2 tickets
- Address code review comments
@@ -11353,6 +11353,30 @@
],
"macroName": "QuicTraceLogVerbose"
},
"SettingResumptionTicketMaxVersion": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this didnt get updated....

Comment on lines -1822 to +1823
if (!CXPLAT_STRUCT_HAS_FIELD(QUIC_SETTINGS, SettingsSize, MtuDiscoveryMissingProbeCount)) {
if (!CXPLAT_STRUCT_HAS_FIELD(QUIC_SETTINGS, SettingsSize, StreamRecvWindowUnidiDefault)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing code seems to be measuring the size of the struct incorrectly. I have updated this to point to the last member of QUIC_SETTINGS (

uint32_t StreamRecvWindowUnidiDefault;
)

Comment on lines -2003 to +2004
uint32_t MinimumSettingsSize = (uint32_t)CXPLAT_STRUCT_SIZE_THRU_FIELD(QUIC_SETTINGS, MtuDiscoveryMissingProbeCount);
uint32_t MinimumSettingsSize = (uint32_t)CXPLAT_STRUCT_SIZE_THRU_FIELD(QUIC_SETTINGS, StreamRecvWindowUnidiDefault);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downlevel tests are failing... got to revert this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be correct to say that MtuDiscoveryMissingProbeCount is part of the earliest released QUIC_SETTINGS struct and our API contract requires us to accept these smaller sized structs?

Is there reference documentation around this?

Comment on lines -345 to +614
uint32_t MinimumSettingsSize = (uint32_t)SETTINGS_SIZE_THRU_FIELD(QUIC_SETTINGS, MtuDiscoveryMissingProbeCount);
uint32_t MinimumSettingsSize = (uint32_t)SETTINGS_SIZE_THRU_FIELD(QUIC_SETTINGS, StreamRecvWindowUnidiDefault);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint32_t StreamRecvWindowUnidiDefault;

Should the minimum size not be the last member in the QUIC_SETTINGS struct?

Comment on lines -397 to +666
uint32_t MinimumSettingsSize = (uint32_t)SETTINGS_SIZE_THRU_FIELD(QUIC_SETTINGS, MtuDiscoveryMissingProbeCount);
uint32_t MinimumSettingsSize = (uint32_t)SETTINGS_SIZE_THRU_FIELD(QUIC_SETTINGS, StreamRecvWindowUnidiDefault);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert

- Fix up few settings tests to hit relevant code paths and other minor changes
- Remove unneeded settings load in ticket tests
@sm-msft
Copy link
Contributor Author

sm-msft commented May 29, 2025

Moved the resumption ticket related code into a separate PR: #5131
Please continue the review there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants