Skip to content

Limit reorder buffer #286

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.

Sign up for GitHub

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

Merged
boaks merged 2 commits into from
Oct 16, 2026
Merged

Limit reorder buffer #286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c11c4ec
dtls.c: limit maximum size of reorder buffer.
boaks Sep 13, 2026
991cdc9
dtls.c: sort reorder buffer.
boaks Sep 13, 2026
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
1 change: 1 addition & 0 deletions crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ typedef struct {
unsigned int do_client_auth:1;
unsigned int extended_master_secret:1;
unsigned int renegotiation_info:1;
unsigned int reported_reorder:1;
union {
#ifdef DTLS_ECC
dtls_handshake_parameters_ecdsa_t ecdsa;
Expand Down
52 changes: 25 additions & 27 deletions dtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4404,16 +4404,27 @@ handle_handshake(dtls_context_t *ctx, dtls_peer_t *peer, uint8 *data, size_t dat
return 0;
}

size_t buffered_length = data_length;
netq_t *node = netq_head(&peer->handshake_params->reorder_queue);
while (node) {
dtls_handshake_header_t *node_header = DTLS_HANDSHAKE_HEADER(node->data);
if (dtls_uint16_to_int(node_header->message_seq) == mseq) {
uint16_t hmseq = dtls_uint16_to_int(node_header->message_seq);
if (hmseq == mseq) {
dtls_warn("a packet with this sequence number is already stored\n");
return 0;
}
buffered_length += node->length;
node = netq_next(node);
}

if (buffered_length > DTLS_MAX_REORDER_BUF) {
if (!peer->handshake_params->reported_reorder) {
peer->handshake_params->reported_reorder = 1;
dtls_warn("the packet exceeds the maximum reorder buffer size\n");
}
return 0;
}

n = netq_node_new(data_length);
if (!n) {
dtls_warn("no space in reorder buffer\n");
Expand All @@ -4422,6 +4433,7 @@ handle_handshake(dtls_context_t *ctx, dtls_peer_t *peer, uint8 *data, size_t dat

n->peer = peer;
n->length = data_length;
n->t = mseq; /* use handshake message sequence number for time to sort */
memcpy(n->data, data, data_length);

if (!netq_insert_node(&peer->handshake_params->reorder_queue, n)) {
Expand All @@ -4432,35 +4444,21 @@ handle_handshake(dtls_context_t *ctx, dtls_peer_t *peer, uint8 *data, size_t dat
return 0;
} else if (mseq == peer->handshake_params->hs_state.mseq_r) {
/* Found the expected packet, use this and all the buffered packet */
int next = 1;

netq_t *node =NULL;
res = handle_handshake_msg(ctx, peer, data, data_length);
if (res < 0)
return res;

/* We do not know in which order the packet are in the list just search the list for every packet. */
while (next && peer->handshake_params) {
next = 0;
netq_t *node = netq_head(&peer->handshake_params->reorder_queue);
while (node) {
dtls_handshake_header_t *node_header = DTLS_HANDSHAKE_HEADER(node->data);

if (dtls_uint16_to_int(node_header->message_seq) == peer->handshake_params->hs_state.mseq_r) {
netq_remove(&peer->handshake_params->reorder_queue, node);
next = 1;
res = handle_handshake_msg(ctx, peer, node->data, node->length);

/* free message data */
netq_node_free(node);
while (res >= 0 && peer->handshake_params &&
(node = netq_head(&peer->handshake_params->reorder_queue))) {
dtls_handshake_header_t *node_header = DTLS_HANDSHAKE_HEADER(node->data);

if (res < 0) {
return res;
}
/* Using netq_t.time for the handshake message sequence number */
if (dtls_uint16_to_int(node_header->message_seq) == peer->handshake_params->hs_state.mseq_r) {
netq_remove(&peer->handshake_params->reorder_queue, node);
res = handle_handshake_msg(ctx, peer, node->data, node->length);

break;
} else {
node = netq_next(node);
}
/* free message data */
netq_node_free(node);
} else {
break;
}
}
return res;
Expand Down
9 changes: 9 additions & 0 deletions global.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ typedef unsigned char uint48[6];
#endif /* WITH_CONTIKI || RIOT_VERSION */
#endif

#ifndef DTLS_MAX_REORDER_BUF
/**
* Limit size of reordered handshake messages.
* Using RPK the largest server flight has about 450 bytes
* and the largest server flight has about 400 bytes
*/
#define DTLS_MAX_REORDER_BUF 512

@mrdeep1 mrdeep1 Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to be generous and set the default limit to 1024?

@mrdeep1 mrdeep1 Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On further thinking, this needs to be at least the MTU size, otherwise a single large PDU will not get processed.

@boaks boaks Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The question, I tried to answer:
How much data is required to be queued for an valid handshake (PSK/RPK)?
If someone want to queue more, the value may be just provided from outside.

@mrdeep1 mrdeep1 Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need a sensible limit to prevent a DoS. I have found

Handshake Data Size (Length): A typical DTLS 1.3 handshake utilizing RPKs transmits approximately 894 bytes of total data.

I know we are not DTLS 1.3, but...

@boaks boaks Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

approximately 894 bytes of total data

The point is not DTLS 1.3, the point is rather, it's not the "total data".

See RFC 6347 - Flights:

A peer is not waiting for the "total data", the peer waits for a flight.

And so the maximum size to keep in the reorder buffer is the one of the largest flight.
Therefore the comment:

 * Using RPK the largest server flight has about 450 bytes
 * and the largest server flight has about 400 bytes

I guess, there will then be something: "but under attack".
There may happen much more under attack and everyone may just chose 1 or 2 k, if wanted.

And also just to mention, the most peers with PSK&RPK will be able to send the flights in one message and reordering will not happen at all.

@mrdeep1 mrdeep1 Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK

#endif

/*
* DTLS send buf is alloctaed on the stack by default
*/
Expand Down
Loading