Skip to content

Commit 6632ecb

Browse files
mrdeep1mrdeep1
committed
dtls.c: Tidy up fragment handling checking code
If a fragmented ClientHello is received with no peer, do not handle it. Check subsequent fragments have the same msg_type, length and message_seq. Check fragment length + offset is not greater than handshake length. Check provided data is not less than fragment length Simplify re-assembly code with better variable names. Signed-off-by: Jon Shallow <supjps-libcoap@jpshallow.com>
1 parent 94205ff commit 6632ecb

3 files changed

Lines changed: 89 additions & 57 deletions

File tree

‎crypto.c‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -152,17 +152,14 @@ dtls_handshake_parameters_t *dtls_handshake_new(void)
152152
}
153153

154154
memset(handshake, 0, sizeof(*handshake));
155-
/* Just to be sure */
156-
handshake->reassemble_buf = NULL;
157-
158-
if (handshake) {
159-
/* initialize the handshake hash wrt. the hard-coded DTLS version */
160-
dtls_debug("DTLSv12: initialize HASH_SHA256\n");
161-
/* TLS 1.2: PRF(secret, label, seed) = P_<hash>(secret, label + seed) */
162-
/* FIXME: we use the default SHA256 here, might need to support other
163-
hash functions as well */
164-
dtls_hash_init(&handshake->hs_state.hs_hash);
165-
}
155+
156+
/* initialize the handshake hash wrt. the hard-coded DTLS version */
157+
dtls_debug("DTLSv12: initialize HASH_SHA256\n");
158+
/* TLS 1.2: PRF(secret, label, seed) = P_<hash>(secret, label + seed) */
159+
/* FIXME: we use the default SHA256 here, might need to support other
160+
hash functions as well */
161+
dtls_hash_init(&handshake->hs_state.hs_hash);
162+
166163
return handshake;
167164
}
168165

‎crypto.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,9 @@ typedef struct {
117117
struct netq_t;
118118

119119
typedef struct {
120-
uint16 mseq_r;
121-
size_t last_offset;
122-
size_t packet_length;
123-
uint8 *data;
120+
size_t last_offset;
121+
size_t data_length;
122+
uint8 *data;
124123
} dtls_hs_reassemble_t;
125124

126125
typedef struct {

‎dtls.c‎

Lines changed: 78 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,7 +1474,6 @@ dtls_prepare_record(dtls_peer_t *peer, dtls_security_parameters_t *security,
14741474
return 0;
14751475
}
14761476

1477-
14781477
static int
14791478
dtls_send_handshake_msg_hash(dtls_context_t *ctx,
14801479
dtls_peer_t *peer,
@@ -3636,7 +3635,8 @@ dtls_free_reassemble_buffer(dtls_handshake_parameters_t *handshake_params){
36363635
}
36373636

36383637
static int
3639-
dtls_init_reassemble_buffer(dtls_handshake_parameters_t *handshake_params, size_t packet_length){
3638+
dtls_init_reassemble_buffer(dtls_handshake_parameters_t *handshake_params,
3639+
size_t hs_size){
36403640
/* Allocate buffer for full packet */
36413641
void *ret;
36423642
ret = malloc(sizeof(dtls_hs_reassemble_t));
@@ -3645,14 +3645,14 @@ dtls_init_reassemble_buffer(dtls_handshake_parameters_t *handshake_params, size_
36453645
} else {
36463646
handshake_params->reassemble_buf = (dtls_hs_reassemble_t *)ret;
36473647
}
3648-
ret = malloc(packet_length + sizeof(dtls_handshake_header_t));
3648+
ret = malloc(hs_size + DTLS_HS_LENGTH);
36493649
if(ret == NULL){
36503650
free(handshake_params->reassemble_buf);
36513651
return 0;
36523652
} else {
36533653
handshake_params->reassemble_buf->data = (uint8 *)ret;
36543654
}
3655-
handshake_params->reassemble_buf->packet_length = packet_length;
3655+
handshake_params->reassemble_buf->data_length = hs_size + DTLS_HS_LENGTH;
36563656
handshake_params->reassemble_buf->last_offset = 0;
36573657
return 1;
36583658
}
@@ -3664,60 +3664,94 @@ handle_handshake(dtls_context_t *ctx, dtls_peer_t *peer, session_t *session,
36643664
{
36653665
dtls_handshake_header_t *hs_header;
36663666
int res;
3667+
size_t hs_size;
3668+
size_t fragment_length;
3669+
size_t fragment_offset;
36673670

36683671
if (data_length < DTLS_HS_LENGTH) {
36693672
dtls_warn("handshake message too short\n");
36703673
return dtls_alert_fatal_create(DTLS_ALERT_DECODE_ERROR);
36713674
}
36723675
hs_header = DTLS_HANDSHAKE_HEADER(data);
36733676

3674-
size_t packet_length = dtls_uint24_to_int(hs_header->length);
3675-
size_t fragment_length = dtls_uint24_to_int(hs_header->fragment_length);
3676-
size_t fragment_offset = dtls_uint24_to_int(hs_header->fragment_offset);
3677+
hs_size = dtls_uint24_to_int(hs_header->length);
3678+
fragment_length = dtls_uint24_to_int(hs_header->fragment_length);
3679+
fragment_offset = dtls_uint24_to_int(hs_header->fragment_offset);
36773680

3678-
if (packet_length > fragment_length){
3679-
dtls_debug("received fragmented handshake packet: length %zu, fragment length %zu.\n",
3680-
packet_length, fragment_length);
3681-
/* If (reassembled) packet is larger than our buffer, drop with error */
3682-
if(packet_length > DTLS_MAX_BUF){
3683-
dtls_warn("reassembled packet (%zu) would be larger than buffer (%i)\n", packet_length, DTLS_MAX_BUF);
3684-
return dtls_alert_fatal_create(DTLS_ALERT_RECORD_OVERFLOW); // TODO: Is this the correct alert?
3681+
if (fragment_length + DTLS_HS_LENGTH != data_length) {
3682+
dtls_warn("Fragment size does not match packet size\n");
3683+
return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE);
3684+
}
3685+
3686+
if (fragment_offset + fragment_length > hs_size) {
3687+
/* Fragment extension larger than handshake */
3688+
dtls_warn("fragment_offset (%zu) + fragment_length (%zu) >"
3689+
" handshake length (%zu)\n", fragment_offset,
3690+
fragment_length, hs_size);
3691+
return dtls_alert_fatal_create(DTLS_ALERT_RECORD_OVERFLOW); // TODO: Is this the correct alert?
3692+
}
3693+
else if (hs_size > fragment_length) {
3694+
3695+
/* Handle fragmented packet */
3696+
3697+
if (!peer || !peer->handshake_params) {
3698+
/* This is the initial ClientHello - can't handle fragmented */
3699+
dtls_alert("Cannot handle initial fragmented ClientHello\n");
3700+
return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE);
36853701
}
3686-
if((fragment_offset + fragment_length) > packet_length){
3687-
dtls_warn("fragment_offset (%zu) + fragment_length (%zu) > packet length (%zu)\n",
3688-
fragment_offset, fragment_length, packet_length);
3689-
return dtls_alert_fatal_create(DTLS_ALERT_RECORD_OVERFLOW); // TODO: Is this the correct alert?
3702+
dtls_debug("received fragmented handshake packet: handshake length %zu,"
3703+
" fragment length %zu.\n", hs_size, fragment_length);
3704+
3705+
if (DTLS_HS_LENGTH + hs_size > DTLS_MAX_BUF) {
3706+
/* Reassembled handshake is larger than our buffer, drop with error */
3707+
dtls_warn("reassembled packet (%zu) would be larger than buffer (%i)\n",
3708+
DTLS_HS_LENGTH + hs_size, DTLS_MAX_BUF);
3709+
return dtls_alert_fatal_create(DTLS_ALERT_RECORD_OVERFLOW); // TODO: Is this the correct alert?
36903710
}
3691-
/* Handle fragmented packet */
36923711

3693-
/* First fragment */
3694-
if(fragment_offset == 0 && packet_length <= DTLS_MAX_BUF){
3712+
if (fragment_offset == 0) {
3713+
/* First fragment */
36953714
dtls_debug("received first packet of fragmented.\n");
36963715
dtls_free_reassemble_buffer(peer->handshake_params);
36973716

36983717
/* Allocate buffer for full packet */
3699-
if(!dtls_init_reassemble_buffer(peer->handshake_params, packet_length)){
3700-
return dtls_alert_fatal_create(DTLS_ALERT_RECORD_OVERFLOW); // TODO: Is this the correct alert?
3718+
if (!dtls_init_reassemble_buffer(peer->handshake_params, hs_size)) {
3719+
return dtls_alert_fatal_create(DTLS_ALERT_RECORD_OVERFLOW); // TODO: Is this the correct alert?
37013720
}
3721+
/* Save handshake at the start */
3722+
memcpy(peer->handshake_params->reassemble_buf->data, data,
3723+
DTLS_HS_LENGTH);
37023724
} else {
3703-
/* Not first fragment, skip over handshake header */
3704-
data += sizeof(dtls_handshake_header_t);
3725+
/* Subsequent fragment */
3726+
if (memcmp(peer->handshake_params->reassemble_buf->data, data,
3727+
offsetof(dtls_handshake_header_t, fragment_offset)) != 0) {
3728+
/* Not a continuation fragment - msg_type+length+message_seq mismatch */
3729+
dtls_warn("fragment does not match fragment #0\n");
3730+
return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE); // TODO: Is this the correct alert?
3731+
}
3732+
if (peer->handshake_params->reassemble_buf->last_offset !=
3733+
fragment_offset) {
3734+
dtls_warn("Received fragment out of order\n");
3735+
return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE); // TODO: Is this the correct alert?
3736+
}
37053737
}
37063738

3707-
/* Check if we have fragment buffer, (possibly earlier fragments) and offset is consecutive */
3708-
if(peer->handshake_params->reassemble_buf == NULL ||
3709-
peer->handshake_params->reassemble_buf->last_offset != fragment_offset ){
3710-
dtls_warn("Received fragment out of order\n");
3711-
return dtls_alert_fatal_create(DTLS_ALERT_HANDSHAKE_FAILURE); // TODO: Is this the correct alert?
3739+
if (fragment_length > data_length - DTLS_HS_LENGTH) {
3740+
dtls_warn("insufficient data provided for fragment\n");
3741+
return dtls_alert_fatal_create(DTLS_ALERT_RECORD_OVERFLOW); // TODO: Is this the correct alert?
37123742
}
3743+
37133744
/* Looks good: copy fragment in buffer */
3714-
dtls_debug("copying fragment to buffer: offset (%zu), length (%zu).\n", fragment_offset,
3715-
fragment_length);
3716-
memcpy(peer->handshake_params->reassemble_buf->data + fragment_offset + (fragment_offset != 0 ? sizeof(dtls_handshake_header_t) : 0),
3717-
data, (size_t)fragment_length + (fragment_offset == 0 ? sizeof(dtls_handshake_header_t) : 0));
3745+
dtls_debug("copying fragment to buffer: offset (%zu), length (%zu),"
3746+
" handshake length (%zu).\n", fragment_offset, fragment_length,
3747+
hs_size);
3748+
3749+
memcpy(peer->handshake_params->reassemble_buf->data +
3750+
DTLS_HS_LENGTH + fragment_offset,
3751+
data + DTLS_HS_LENGTH, fragment_length);
37183752
peer->handshake_params->reassemble_buf->last_offset = fragment_offset + fragment_length;
37193753

3720-
if(peer->handshake_params->reassemble_buf->last_offset < packet_length){
3754+
if(peer->handshake_params->reassemble_buf->last_offset < hs_size){
37213755
dtls_debug("waiting for next fragment\n");
37223756
return DTLS_HS_FRAGMENTED;
37233757
}
@@ -3726,16 +3760,18 @@ handle_handshake(dtls_context_t *ctx, dtls_peer_t *peer, session_t *session,
37263760
dtls_debug("last hs fragment received.\n");
37273761
/* Last fragment received, packet reassembled */
37283762
data = peer->handshake_params->reassemble_buf->data;
3729-
/* packet_length is handshake payload size, data_length is record length (including header) */
3730-
data_length = peer->handshake_params->reassemble_buf->packet_length + DTLS_HS_LENGTH;
3763+
data_length = peer->handshake_params->reassemble_buf->data_length;
37313764

3732-
/* Rewrite header as if this package was received unfragmented. Needed for the hs_hash, see rfc6347#section-4.2.6 */
3765+
/*
3766+
* Rewrite header as if this package was received unfragmented.
3767+
* Needed for the hs_hash, see rfc6347#section-4.2.6
3768+
*/
37333769
hs_header = DTLS_HANDSHAKE_HEADER(data);
3734-
dtls_int_to_uint24(hs_header->fragment_offset, 0); //Should already be this way
3735-
dtls_int_to_uint24(hs_header->length, peer->handshake_params->reassemble_buf->packet_length);
3736-
dtls_int_to_uint24(hs_header->fragment_length, peer->handshake_params->reassemble_buf->packet_length);
3770+
dtls_int_to_uint24(hs_header->fragment_length, hs_size);
37373771

3738-
dtls_debug_dump("reassembled fragment header", (const unsigned char*)hs_header, sizeof(dtls_handshake_header_t));
3772+
dtls_debug_dump("reassembled fragment header",
3773+
(const unsigned char*)hs_header,
3774+
sizeof(dtls_handshake_header_t));
37393775
}
37403776

37413777
dtls_debug("received handshake packet of type: %s (%i)\n",

0 commit comments

Comments
 (0)