Correctly handle a retransmitted ClientHello

If we receive a ClientHello and send back a HelloVerifyRequest, we need
to be able to handle the scenario where the HelloVerifyRequest gets lost
and we receive another ClientHello with the message sequence number set to
0.

Fixes #18635

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18654)
This commit is contained in:
Matt Caswell 2022-06-23 11:39:38 +01:00 committed by Hugo Landau
parent 0ff9813744
commit 81926c9156
1 changed files with 85 additions and 10 deletions

View File

@ -496,23 +496,64 @@ static int dtls1_retrieve_buffered_fragment(SSL_CONNECTION *s, size_t *len)
* (2) update s->init_num
*/
pitem *item;
piterator iter;
hm_fragment *frag;
int ret;
int chretran = 0;
iter = pqueue_iterator(s->d1->buffered_messages);
do {
item = pqueue_peek(s->d1->buffered_messages);
item = pqueue_next(&iter);
if (item == NULL)
return 0;
frag = (hm_fragment *)item->data;
if (frag->msg_header.seq < s->d1->handshake_read_seq) {
/* This is a stale message that has been buffered so clear it */
pqueue_pop(s->d1->buffered_messages);
dtls1_hm_fragment_free(frag);
pitem_free(item);
item = NULL;
frag = NULL;
pitem *next;
hm_fragment *nextfrag;
if (!s->server
|| frag->msg_header.seq != 0
|| s->d1->handshake_read_seq != 1
|| s->statem.hand_state != DTLS_ST_SW_HELLO_VERIFY_REQUEST) {
/*
* This is a stale message that has been buffered so clear it.
* It is safe to pop this message from the queue even though
* we have an active iterator
*/
pqueue_pop(s->d1->buffered_messages);
dtls1_hm_fragment_free(frag);
pitem_free(item);
item = NULL;
frag = NULL;
} else {
/*
* We have fragments for a ClientHello without a cookie,
* even though we have sent a HelloVerifyRequest. It is possible
* that the HelloVerifyRequest got lost and this is a
* retransmission of the original ClientHello
*/
next = pqueue_next(&iter);
if (next != NULL) {
nextfrag = (hm_fragment *)next->data;
if (nextfrag->msg_header.seq == s->d1->handshake_read_seq) {
/*
* We have fragments for both a ClientHello without
* cookie and one with. Ditch the one without.
*/
pqueue_pop(s->d1->buffered_messages);
dtls1_hm_fragment_free(frag);
pitem_free(item);
item = next;
frag = nextfrag;
} else {
chretran = 1;
}
} else {
chretran = 1;
}
}
}
} while (item == NULL);
@ -520,7 +561,7 @@ static int dtls1_retrieve_buffered_fragment(SSL_CONNECTION *s, size_t *len)
if (frag->reassembly != NULL)
return 0;
if (s->d1->handshake_read_seq == frag->msg_header.seq) {
if (s->d1->handshake_read_seq == frag->msg_header.seq || chretran) {
size_t frag_len = frag->msg_header.frag_len;
pqueue_pop(s->d1->buffered_messages);
@ -538,6 +579,16 @@ static int dtls1_retrieve_buffered_fragment(SSL_CONNECTION *s, size_t *len)
pitem_free(item);
if (ret) {
if (chretran) {
/*
* We got a new ClientHello with a message sequence of 0.
* Reset the read/write sequences back to the beginning.
* We process it like this is the first time we've seen a
* ClientHello from the client.
*/
s->d1->handshake_read_seq = 0;
s->d1->next_handshake_write_seq = 0;
}
*len = frag_len;
return 1;
}
@ -768,6 +819,7 @@ static int dtls_get_reassembled_message(SSL_CONNECTION *s, int *errtype,
struct hm_header_st msg_hdr;
size_t readbytes;
SSL *ssl = SSL_CONNECTION_GET_SSL(s);
int chretran = 0;
*errtype = 0;
@ -837,8 +889,20 @@ static int dtls_get_reassembled_message(SSL_CONNECTION *s, int *errtype,
* although we're still expecting seq 0 (ClientHello)
*/
if (msg_hdr.seq != s->d1->handshake_read_seq) {
*errtype = dtls1_process_out_of_seq_message(s, &msg_hdr);
return 0;
if (!s->server
|| msg_hdr.seq != 0
|| s->d1->handshake_read_seq != 1
|| wire[0] != SSL3_MT_CLIENT_HELLO
|| s->statem.hand_state != DTLS_ST_SW_HELLO_VERIFY_REQUEST) {
*errtype = dtls1_process_out_of_seq_message(s, &msg_hdr);
return 0;
}
/*
* We received a ClientHello and sent back a HelloVerifyRequest. We
* now seem to have received a retransmitted initial ClientHello. That
* is allowed (possibly our HelloVerifyRequest got lost).
*/
chretran = 1;
}
if (frag_len && frag_len < mlen) {
@ -904,6 +968,17 @@ static int dtls_get_reassembled_message(SSL_CONNECTION *s, int *errtype,
goto f_err;
}
if (chretran) {
/*
* We got a new ClientHello with a message sequence of 0.
* Reset the read/write sequences back to the beginning.
* We process it like this is the first time we've seen a ClientHello
* from the client.
*/
s->d1->handshake_read_seq = 0;
s->d1->next_handshake_write_seq = 0;
}
/*
* Note that s->init_num is *not* used as current offset in
* s->init_buf->data, but as a counter summing up fragments' lengths: as