Skip to content
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

dtls.c: Handle DTLS1.3 ClientHello when calculating cookie #223

Closed
wants to merge 1 commit into from
Closed

dtls.c: Handle DTLS1.3 ClientHello when calculating cookie #223

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 14 additions & 5 deletions dtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ dtls_create_cookie(dtls_context_t *ctx,
uint8 *msg, size_t msglen,
uint8 *cookie, int *clen) {
unsigned char buf[DTLS_HMAC_MAX];
size_t e, fragment_length;
size_t e, compatability_length;
int len;

/* create cookie with HMAC-SHA256 over:
Expand Down Expand Up @@ -504,14 +504,23 @@ dtls_create_cookie(dtls_context_t *ctx,
e += dtls_uint8_to_int(msg + DTLS_HS_LENGTH + e);
e += sizeof(uint8_t);

/* read fragment length and check for consistency */
fragment_length = dtls_get_fragment_length(DTLS_HANDSHAKE_HEADER(msg));
if ((fragment_length < e) || (e + DTLS_HS_LENGTH) > msglen)
/*
* Read in compatablility_length length and check for consistency
* The compatability_length is because a DTLS1.3 Client Hello may come in,
* and the re-transmit of the Client Hello with cookie may be different.
*/
/* Get the Cipher Suites length + value */
compatability_length = dtls_uint16_to_int(msg + DTLS_HS_LENGTH + e) + 2;
/* Add in the Compression length byte + value */
compatability_length += dtls_uint8_to_int(msg + DTLS_HS_LENGTH + e
+ compatability_length) + 1;

Copy link
Contributor

@boaks boaks Feb 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The other places, where the length of the field is read, a length check is done before reading it.

  if (e + DTLS_HS_LENGTH + sizeof(uint8_t) > msglen)
    return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE);
  /* skip cookie bytes and length byte */
  e += dtls_uint8_to_int(msg + DTLS_HS_LENGTH + e);

In order to "standardize" that, we have two macros, SKIP_VAR_FIELD and GET_VAR_FIELD. In this case I guess, using SKIP_VAR_FIELD (together with other modifications) will do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

See PR #230

if (e + DTLS_HS_LENGTH + compatability_length > msglen)
return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE);

dtls_hmac_update(&hmac_context,
msg + DTLS_HS_LENGTH + e,
fragment_length - e);
compatability_length);

len = dtls_hmac_finalize(&hmac_context, buf);

Expand Down
Loading