Skip to content

Commit

Permalink
Update SpinQuic to Validate Bytes in Stream Receive aren't Corrupted (#…
Browse files Browse the repository at this point in the history 
…4237)
  • Loading branch information
@nibanks
nibanks authored Apr 15, 2024
1 parent ff17a2e commit b449e9c
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 25 deletions.
10 changes: 6 additions & 4 deletions src/core/api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,12 @@ MsQuicStreamSend(
goto Exit;
}

//
// From here on, we cannot fail the call because the stream has been queued
// and possibly already started to be processed.
//
Status = QUIC_STATUS_PENDING;

if (SendInline) {

CXPLAT_PASSIVE_CODE();
Expand All @@ -1140,8 +1146,6 @@ MsQuicStreamSend(
"STRM_SEND operation",
0);

Status = QUIC_STATUS_OUT_OF_MEMORY;

//
// We failed to alloc the operation we needed to queue, so make sure
// to release the ref we took above.
Expand Down Expand Up @@ -1179,8 +1183,6 @@ MsQuicStreamSend(
QuicConnQueueOper(Connection, Oper);
}

Status = QUIC_STATUS_PENDING;

Exit:

QuicTraceEvent(
Expand Down
12 changes: 9 additions & 3 deletions src/core/datagram.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,18 +363,26 @@ QuicDatagramQueueSend(
goto Exit;
}

//
// From here on, we cannot fail the call because the stream has been queued
// and possibly already started to be processed.
//
Status = QUIC_STATUS_PENDING;

if (QueueOper) {
QUIC_OPERATION* Oper =
QuicOperationAlloc(Connection->Worker, QUIC_OPER_TYPE_API_CALL);
if (Oper == NULL) {
Status = QUIC_STATUS_OUT_OF_MEMORY;
QuicTraceEvent(
AllocFailure,
"Allocation of '%s' failed. (%llu bytes)",
"DATAGRAM_SEND operation",
0);
goto Exit;
}

// TODO - Kill the connection?

Oper->API_CALL.Context->Type = QUIC_API_TYPE_DATAGRAM_SEND;

//
Expand All @@ -383,8 +391,6 @@ QuicDatagramQueueSend(
QuicConnQueueOper(Connection, Oper);
}

Status = QUIC_STATUS_PENDING;

Exit:

return Status;
Expand Down
69 changes: 51 additions & 18 deletions src/tools/spin/spinquic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,14 @@ struct SpinQuicGlobals {
std::vector<HQUIC> ClientConfigurations;
QUIC_BUFFER* Alpns {nullptr};
uint32_t AlpnCount {0};
QUIC_BUFFER Buffers[BufferCount];
SpinQuicGlobals() { CxPlatZeroMemory(Buffers, sizeof(Buffers)); }
const size_t SendBufferSize { MaxBufferSizes[BufferCount - 1] + UINT8_MAX };
uint8_t* SendBuffer;
SpinQuicGlobals() {
SendBuffer = new uint8_t[SendBufferSize];
for (size_t i = 0; i < SendBufferSize; i++) {
SendBuffer[i] = (uint8_t)i;
}
}
~SpinQuicGlobals() {
while (ClientConfigurations.size() > 0) {
auto Configuration = ClientConfigurations.back();
Expand All @@ -239,9 +245,7 @@ struct SpinQuicGlobals {
#endif
MsQuicClose(MsQuic);
}
for (size_t j = 0; j < BufferCount; ++j) {
free(Buffers[j].Buffer);
}
delete [] SendBuffer;
}
};

Expand Down Expand Up @@ -272,6 +276,7 @@ typedef enum {
struct SpinQuicStream {
struct SpinQuicConnection& Connection;
HQUIC Handle;
uint8_t SendOffset {0};
bool Deleting {false};
uint64_t PendingRecvLength {UINT64_MAX}; // UINT64_MAX means no pending receive
SpinQuicStream(SpinQuicConnection& Connection, HQUIC Handle = nullptr) :
Expand Down Expand Up @@ -388,6 +393,9 @@ QUIC_STATUS QUIC_API SpinQuicHandleStreamEvent(HQUIC Stream, void* , QUIC_STREAM
}

switch (Event->Type) {
case QUIC_STREAM_EVENT_SEND_COMPLETE:
delete (QUIC_BUFFER*)Event->SEND_COMPLETE.ClientContext;
break;
case QUIC_STREAM_EVENT_PEER_SEND_SHUTDOWN:
MsQuic.StreamShutdown(Stream, (QUIC_STREAM_SHUTDOWN_FLAGS)GetRandom(16), 0);
break;
Expand All @@ -401,6 +409,15 @@ QUIC_STATUS QUIC_API SpinQuicHandleStreamEvent(HQUIC Stream, void* , QUIC_STREAM
ctx->PendingRecvLength = UINT64_MAX; // TODO - Add more complex handling
break;
}
auto Offset = Event->RECEIVE.AbsoluteOffset;
for (uint32_t i = 0; i < Event->RECEIVE.BufferCount; ++i) {
for (uint32_t j = 0; j < Event->RECEIVE.Buffers[i].Length; ++j) {
if (Event->RECEIVE.Buffers[i].Buffer[j] != (uint8_t)(Offset + j)) {
CXPLAT_FRE_ASSERT(FALSE); // Value is corrupt!
}
}
Offset += Event->RECEIVE.Buffers[i].Length;
}
int Random = GetRandom(5);
std::lock_guard<std::mutex> Lock(ctx->Connection.Lock);
CXPLAT_DBG_ASSERT(ctx->PendingRecvLength == UINT64_MAX);
Expand Down Expand Up @@ -477,6 +494,11 @@ QUIC_STATUS QUIC_API SpinQuicHandleConnectionEvent(HQUIC Connection, void* , QUI
ctx->AddStream(Event->PEER_STREAM_STARTED.Stream);
break;
}
case QUIC_CONNECTION_EVENT_DATAGRAM_SEND_STATE_CHANGED:
if (QUIC_DATAGRAM_SEND_STATE_IS_FINAL(Event->DATAGRAM_SEND_STATE_CHANGED.State)) {
delete (QUIC_BUFFER*)Event->DATAGRAM_SEND_STATE_CHANGED.ClientContext;
}
break;
default:
break;
}
Expand Down Expand Up @@ -769,9 +791,9 @@ void SpinQuicSetRandomConnectionParam(HQUIC Connection, uint16_t ThreadID)
break;
case QUIC_PARAM_CONN_DATAGRAM_SEND_ENABLED: // uint8_t (BOOLEAN)
break; // Get Only
case QUIC_PARAM_CONN_DISABLE_1RTT_ENCRYPTION: // uint8_t (BOOLEAN)
Helper.SetUint8(QUIC_PARAM_CONN_DISABLE_1RTT_ENCRYPTION, (uint8_t)GetRandom(2));
break;
//case QUIC_PARAM_CONN_DISABLE_1RTT_ENCRYPTION: // uint8_t (BOOLEAN)
// Helper.SetUint8(QUIC_PARAM_CONN_DISABLE_1RTT_ENCRYPTION, (uint8_t)GetRandom(2));
// break;
case QUIC_PARAM_CONN_RESUMPTION_TICKET: // uint8_t[]
// TODO
break;
Expand Down Expand Up @@ -978,8 +1000,19 @@ void Spin(Gbs& Gb, LockableVector<HQUIC>& Connections, std::vector<HQUIC>* Liste
std::lock_guard<std::mutex> Lock(ctx->Lock);
auto Stream = ctx->TryGetStream();
if (Stream == nullptr) continue;
auto Buffer = &Gb.Buffers[GetRandom(BufferCount)];
MsQuic.StreamSend(Stream, Buffer, 1, (QUIC_SEND_FLAGS)GetRandom(16), nullptr);
auto StreamCtx = SpinQuicStream::Get(Stream);
auto Buffer = new(std::nothrow) QUIC_BUFFER;
if (Buffer) {
const uint32_t Length = MaxBufferSizes[GetRandom(BufferCount)];
Buffer->Buffer = Gb.SendBuffer + StreamCtx->SendOffset;
Buffer->Length = Length;
if (QUIC_SUCCEEDED(
MsQuic.StreamSend(Stream, Buffer, 1, (QUIC_SEND_FLAGS)GetRandom(16), Buffer))) {
StreamCtx->SendOffset = (uint8_t)(StreamCtx->SendOffset + Length);
} else {
delete Buffer;
}
}
}
break;
}
Expand Down Expand Up @@ -1105,8 +1138,14 @@ void Spin(Gbs& Gb, LockableVector<HQUIC>& Connections, std::vector<HQUIC>* Liste
case SpinQuicAPICallDatagramSend: {
auto Connection = Connections.TryGetRandom();
BAIL_ON_NULL_CONNECTION(Connection);
auto Buffer = &Gb.Buffers[GetRandom(BufferCount)];
MsQuic.DatagramSend(Connection, Buffer, 1, (QUIC_SEND_FLAGS)GetRandom(8), nullptr);
auto Buffer = new(std::nothrow) QUIC_BUFFER;
if (Buffer) {
Buffer->Buffer = Gb.SendBuffer;
Buffer->Length = MaxBufferSizes[GetRandom(BufferCount)];
if (QUIC_FAILED(MsQuic.DatagramSend(Connection, Buffer, 1, (QUIC_SEND_FLAGS)GetRandom(8), Buffer))) {
delete Buffer;
}
}
break;
}
case SpinQuicAPICallCompleteTicketValidation: {
Expand Down Expand Up @@ -1322,12 +1361,6 @@ CXPLAT_THREAD_CALLBACK(RunThread, Context)
do {
Gbs Gb;

for (size_t j = 0; j < BufferCount; ++j) {
Gb.Buffers[j].Length = MaxBufferSizes[j]; // TODO - Randomize?
Gb.Buffers[j].Buffer = (uint8_t*)malloc(Gb.Buffers[j].Length);
ASSERT_ON_NOT(Gb.Buffers[j].Buffer);
}

#ifdef QUIC_BUILD_STATIC
CxPlatLockAcquire(&RunThreadLock);
QUIC_STATUS Status = MsQuicOpen2(&Gb.MsQuic);
Expand Down

0 comments on commit b449e9c

Please sign in to comment.