diff options
author | Matt Caswell <matt@openssl.org> | 2022-06-23 11:39:38 +0100 |
---|---|---|
committer | Hugo Landau <hlandau@openssl.org> | 2022-09-22 12:22:09 +0100 |
commit | 81926c91567cd5d11eec38b9980438f45b276d72 (patch) | |
tree | 1037701655e5e516f2f7840d94d0bb012e1c8930 /ssl/statem | |
parent | 0ff98137445ec63249eed3c1e40cf01dc5190c65 (diff) |
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)
Diffstat (limited to 'ssl/statem')
-rw-r--r-- | ssl/statem/statem_dtls.c | 95 |
1 files changed, 85 insertions, 10 deletions
diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index 4fa70be723..d83e7404cb 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -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 |