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

Closed
wants to merge 13 commits into from
Closed
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/BUILD.md
Original file line number Diff line number Diff line change
@@ -155,6 +155,13 @@ For more info, take a look at the [build.ps1](../scripts/build.ps1) script.

By default the build output will go in the `build` folder and the final build binaries in the `artifacts` folder. Under that it will create per-platform folders with subfolders for architecture/tls combinations. This allows for building different platforms and configurations at the same time.

## Updating Clog Sidecar
Some code changes such as adding/updating new Quic traces require updating the Clog sidecar for successful Linux builds. This is done by running the following command:
```PowerShell
./scripts/update-sidecar.ps1
```
This makes any necessary updates to the clog sidecar manifest and the generated files under `src/generated/` folder. The modified files must be committed along with the rest of the code changes for addressing the Linux build failures.

# Building with CMake

The following section details how to build MsQuic purely with CMake commands.
38 changes: 36 additions & 2 deletions src/core/crypto.c
Original file line number Diff line number Diff line change
@@ -2144,6 +2144,34 @@
PacketSpace->CurrentKeyPhaseBytesSent = 0;
}

uint8_t
QuicGetOutgoingResumptionTicketVersion(
_In_opt_ QUIC_CONNECTION* Connection
)
{

CXPLAT_DBG_ASSERT(QuicVarIntSize(CXPLAT_TLS_RESUMPTION_TICKET_VERSION) == sizeof(Connection->Settings.ResumptionTicketMaxVersion));
CXPLAT_STATIC_ASSERT(sizeof(uint8_t) == sizeof(Connection->Settings.ResumptionTicketMaxVersion), "Resumption ticket version setting field must be uint8_t");

if (Connection == NULL) {
return (uint8_t)CXPLAT_TLS_RESUMPTION_TICKET_VERSION;

Check warning on line 2157 in src/core/crypto.c

Codecov / codecov/patch

src/core/crypto.c#L2157

Added line #L2157 was not covered by tests
}

return Connection->Settings.ResumptionTicketMaxVersion;
}

BOOLEAN
IsQuicIncomingResumptionTicketSupported(_In_ QUIC_CONNECTION* Connection, QUIC_VAR_INT TicketVersion)
{
if (TicketVersion >= Connection->Settings.ResumptionTicketMinVersion &&
TicketVersion <= Connection->Settings.ResumptionTicketMaxVersion) {
return TRUE;
}

return FALSE;

Check warning on line 2171 in src/core/crypto.c

Codecov / codecov/patch

src/core/crypto.c#L2171

Added line #L2171 was not covered by tests
}

// Server calls this function to generate the resumption ticket for a specific client
QUIC_STATUS
QuicCryptoEncodeServerTicket(
_In_opt_ QUIC_CONNECTION* Connection,
@@ -2233,7 +2261,7 @@
//

_Analysis_assume_(sizeof(*TicketBuffer) >= 8);
uint8_t* TicketCursor = QuicVarIntEncode(CXPLAT_TLS_RESUMPTION_TICKET_VERSION, TicketBuffer);
uint8_t* TicketCursor = QuicVarIntEncode(QuicGetOutgoingResumptionTicketVersion(Connection), TicketBuffer);
CxPlatCopyMemory(TicketCursor, &QuicVersion, sizeof(QuicVersion));
TicketCursor += sizeof(QuicVersion);
TicketCursor = QuicVarIntEncode(AlpnLength, TicketCursor);
@@ -2263,6 +2291,7 @@
return Status;
}

// Server uses this function to decode the resumption ticket presented by the client
QUIC_STATUS
QuicCryptoDecodeServerTicket(
_In_ QUIC_CONNECTION* Connection,
@@ -2292,7 +2321,8 @@
"Resumption Ticket version failed to decode");
goto Error;
}
if (TicketVersion != CXPLAT_TLS_RESUMPTION_TICKET_VERSION) {

if (!IsQuicIncomingResumptionTicketSupported(Connection, TicketVersion)) {
QuicTraceEvent(
ConnError,
"[conn][%p] ERROR, %s.",
@@ -2301,6 +2331,10 @@
goto Error;
}

if (TicketVersion == CXPLAT_TLS_RESUMPTION_TICKET_VERSION_V2) {
// Handle V2 ticket specific extensions
}

if (TicketLength < Offset + sizeof(uint32_t)) {
QuicTraceEvent(
ConnError,
19 changes: 16 additions & 3 deletions src/core/quicdef.h
Original file line number Diff line number Diff line change
@@ -444,11 +444,22 @@ CXPLAT_STATIC_ASSERT(
//
#define QUIC_DEFAULT_SERVER_RESUMPTION_LEVEL QUIC_SERVER_NO_RESUME


//
// Version of the wire-format for resumption tickets.
// This needs to be incremented for each change in order or count of fields.
// Valid Resumption Ticket Versions - these must be contiguous
//
#define CXPLAT_TLS_RESUMPTION_TICKET_VERSION_V1 1
#define CXPLAT_TLS_RESUMPTION_TICKET_VERSION_V2 2

//
// Min version of the wire-format for resumption tickets.
//
#define CXPLAT_TLS_RESUMPTION_TICKET_VERSION CXPLAT_TLS_RESUMPTION_TICKET_VERSION_V1

//
// Max version of the wire-format for resumption tickets.
//
#define CXPLAT_TLS_RESUMPTION_TICKET_VERSION 1
#define CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION CXPLAT_TLS_RESUMPTION_TICKET_VERSION_V2

//
// Version of the blob for client resumption tickets.
@@ -700,3 +711,5 @@ CXPLAT_STATIC_ASSERT(
#define QUIC_SETTING_MTU_MISSING_PROBE_COUNT "MtuDiscoveryMissingProbeCount"

#define QUIC_SETTING_CONGESTION_CONTROL_ALGORITHM "CongestionControlAlgorithm"
#define QUIC_SETTING_RESUMPTION_TICKET_MIN_VERSION "ResumptionTicketMinVersion"
#define QUIC_SETTING_RESUMPTION_TICKET_MAX_VERSION "ResumptionTicketMaxVersion"
82 changes: 80 additions & 2 deletions src/core/settings.c
Original file line number Diff line number Diff line change
@@ -177,6 +177,13 @@
if (!Settings->IsSet.StreamMultiReceiveEnabled) {
Settings->StreamMultiReceiveEnabled = QUIC_DEFAULT_STREAM_MULTI_RECEIVE_ENABLED;
}
if (!Settings->IsSet.ResumptionTicketMinVersion) {
Settings->ResumptionTicketMinVersion = CXPLAT_TLS_RESUMPTION_TICKET_VERSION;
}
if (!Settings->IsSet.ResumptionTicketMaxVersion) {
// Default to the min version if not explicitly set
Settings->ResumptionTicketMaxVersion = CXPLAT_TLS_RESUMPTION_TICKET_VERSION;
}
}

_IRQL_requires_max_(PASSIVE_LEVEL)
@@ -354,6 +361,12 @@
if (!Destination->IsSet.StreamMultiReceiveEnabled) {
Destination->StreamMultiReceiveEnabled = Source->StreamMultiReceiveEnabled;
}
if (!Destination->IsSet.ResumptionTicketMinVersion) {
Destination->ResumptionTicketMinVersion = Source->ResumptionTicketMinVersion;
}
if (!Destination->IsSet.ResumptionTicketMaxVersion) {
Destination->ResumptionTicketMaxVersion = Source->ResumptionTicketMaxVersion;
}
}

_IRQL_requires_max_(PASSIVE_LEVEL)
@@ -747,6 +760,29 @@
Destination->StreamMultiReceiveEnabled = Source->StreamMultiReceiveEnabled;
Destination->IsSet.StreamMultiReceiveEnabled = TRUE;
}

if (Source->IsSet.ResumptionTicketMinVersion && (!Destination->IsSet.ResumptionTicketMinVersion || OverWrite)) {
Destination->ResumptionTicketMinVersion = Source->ResumptionTicketMinVersion;
if (Destination->ResumptionTicketMinVersion > CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION) {
Destination->ResumptionTicketMinVersion = CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION;
}
else if (Destination->ResumptionTicketMinVersion < CXPLAT_TLS_RESUMPTION_TICKET_VERSION) {
Destination->ResumptionTicketMinVersion = CXPLAT_TLS_RESUMPTION_TICKET_VERSION;

Check warning on line 770 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L765-L770

Added lines #L765 - L770 were not covered by tests
}
Destination->IsSet.ResumptionTicketMinVersion = TRUE;

Check warning on line 772 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L772

Added line #L772 was not covered by tests
}

if (Source->IsSet.ResumptionTicketMaxVersion && (!Destination->IsSet.ResumptionTicketMaxVersion || OverWrite)) {
Destination->ResumptionTicketMaxVersion = Source->ResumptionTicketMaxVersion;
if (Destination->ResumptionTicketMaxVersion > CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION) {
Destination->ResumptionTicketMaxVersion = CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION;
}
else if (Destination->ResumptionTicketMaxVersion < Destination->ResumptionTicketMinVersion) {
Destination->ResumptionTicketMaxVersion = Destination->ResumptionTicketMinVersion;

Check warning on line 781 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L776-L781

Added lines #L776 - L781 were not covered by tests
}
Destination->IsSet.ResumptionTicketMaxVersion = TRUE;

Check warning on line 783 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L783

Added line #L783 was not covered by tests
}

return TRUE;
}

@@ -1445,6 +1481,40 @@
&ValueLen);
Settings->StreamMultiReceiveEnabled = !!Value;
}
if (!Settings->IsSet.ResumptionTicketMinVersion) {
ValueLen = sizeof(Value);
if (QUIC_FAILED(CxPlatStorageReadValue(
Storage,
QUIC_SETTING_RESUMPTION_TICKET_MIN_VERSION,
(uint8_t*)&Value,
&ValueLen))) {
Value = CXPLAT_TLS_RESUMPTION_TICKET_VERSION; // Default value on failure
}
Settings->ResumptionTicketMinVersion = (uint8_t)Value;

if (Settings->ResumptionTicketMinVersion > CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION) {
Settings->ResumptionTicketMinVersion = CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION;

Check warning on line 1496 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L1496

Added line #L1496 was not covered by tests
} else if (Settings->ResumptionTicketMinVersion < CXPLAT_TLS_RESUMPTION_TICKET_VERSION) {
Settings->ResumptionTicketMinVersion = CXPLAT_TLS_RESUMPTION_TICKET_VERSION;

Check warning on line 1498 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L1498

Added line #L1498 was not covered by tests
}
}
if (!Settings->IsSet.ResumptionTicketMaxVersion) {
ValueLen = sizeof(Value);
if (QUIC_FAILED(CxPlatStorageReadValue(
Storage,
QUIC_SETTING_RESUMPTION_TICKET_MAX_VERSION,
(uint8_t*)&Value,
&ValueLen))) {
Value = CXPLAT_TLS_RESUMPTION_TICKET_VERSION; // Default value on failure
}
Settings->ResumptionTicketMaxVersion = (uint8_t)Value;
if (Settings->ResumptionTicketMaxVersion > CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION) {
Settings->ResumptionTicketMaxVersion = CXPLAT_TLS_RESUMPTION_TICKET_MAX_VERSION;
}

Check warning on line 1513 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L1512-L1513

Added lines #L1512 - L1513 were not covered by tests
else if (Settings->ResumptionTicketMaxVersion < Settings->ResumptionTicketMinVersion) {
Settings->ResumptionTicketMaxVersion = Settings->ResumptionTicketMinVersion;

Check warning on line 1515 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L1515

Added line #L1515 was not covered by tests
}
}
}

_IRQL_requires_max_(PASSIVE_LEVEL)
@@ -1517,6 +1587,8 @@
QuicTraceLogVerbose(SettingOneWayDelayEnabled, "[sett] OneWayDelayEnabled = %hhu", Settings->OneWayDelayEnabled);
QuicTraceLogVerbose(SettingNetStatsEventEnabled, "[sett] NetStatsEventEnabled = %hhu", Settings->NetStatsEventEnabled);
QuicTraceLogVerbose(SettingsStreamMultiReceiveEnabled, "[sett] StreamMultiReceiveEnabled= %hhu", Settings->StreamMultiReceiveEnabled);
QuicTraceLogVerbose(SettingsDumpResumptionTicketMinVersion, "[sett] ResumptionTicketMinVersion= %hhu", Settings->ResumptionTicketMinVersion);
QuicTraceLogVerbose(SettingsDumpResumptionTicketMaxVersion, "[sett] ResumptionTicketMaxVersion= %hhu", Settings->ResumptionTicketMaxVersion);
}

_IRQL_requires_max_(PASSIVE_LEVEL)
@@ -1690,6 +1762,12 @@
if (Settings->IsSet.StreamMultiReceiveEnabled) {
QuicTraceLogVerbose(SettingStreamMultiReceiveEnabled, "[sett] StreamMultiReceiveEnabled = %hhu", Settings->StreamMultiReceiveEnabled);
}
if (Settings->IsSet.ResumptionTicketMinVersion) {
QuicTraceLogVerbose(SettingResumptionTicketMinVersion, "[sett] ResumptionTicketMinVersion= %hhu", Settings->ResumptionTicketMinVersion);

Check warning on line 1766 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L1766

Added line #L1766 was not covered by tests
}
if (Settings->IsSet.ResumptionTicketMaxVersion) {
QuicTraceLogVerbose(SettingResumptionTicketMaxVersion, "[sett] ResumptionTicketMaxVersion= %hhu", Settings->ResumptionTicketMaxVersion);

Check warning on line 1769 in src/core/settings.c

Codecov / codecov/patch

src/core/settings.c#L1769

Added line #L1769 was not covered by tests
}
}

#define SETTING_COPY_TO_INTERNAL(Field, Settings, InternalSettings) \
@@ -1819,7 +1897,7 @@
_Out_ QUIC_SETTINGS_INTERNAL* InternalSettings
)
{
if (!CXPLAT_STRUCT_HAS_FIELD(QUIC_SETTINGS, SettingsSize, MtuDiscoveryMissingProbeCount)) {
if (!CXPLAT_STRUCT_HAS_FIELD(QUIC_SETTINGS, SettingsSize, StreamRecvWindowUnidiDefault)) {
Comment on lines -1822 to +1823
Copy link
Collaborator

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;
)

return QUIC_STATUS_INVALID_PARAMETER;
}

@@ -2000,7 +2078,7 @@
QUIC_SETTINGS* Settings
)
{
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);
Comment on lines -2003 to +2004
Copy link
Collaborator

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?


if (*SettingsLength == 0) {
*SettingsLength = sizeof(QUIC_SETTINGS);
6 changes: 5 additions & 1 deletion src/core/settings.h
Original file line number Diff line number Diff line change
@@ -65,7 +65,9 @@ typedef struct QUIC_SETTINGS_INTERNAL {
uint64_t XdpEnabled : 1;
uint64_t QTIPEnabled : 1;
uint64_t RioEnabled : 1;
uint64_t RESERVED : 13;
uint64_t ResumptionTicketMinVersion : 1;
uint64_t ResumptionTicketMaxVersion : 1;
uint64_t RESERVED : 11;
} IsSet;
};

@@ -120,6 +122,8 @@ typedef struct QUIC_SETTINGS_INTERNAL {
uint8_t QTIPEnabled : 1;
uint8_t RioEnabled : 1;
uint8_t MtuDiscoveryMissingProbeCount;
uint8_t ResumptionTicketMinVersion;
uint8_t ResumptionTicketMaxVersion;
} QUIC_SETTINGS_INTERNAL;

//
Loading
Oops, something went wrong.
Loading
Oops, something went wrong.