From 7321d7944e56e3cf7f5cf80679e6c88a130167f2 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 13 Mar 2017 10:30:49 +0000 Subject: [PATCH] Fix DTLSv1_listen() sequence numbers DTLSv1_listen() is stateless. We never increment the record read sequence while listening, and we reflect the incoming record's sequence number in our write sequence. The logic for doing the write sequence reflection was *after* we had finished processing the incoming ClientHello and before we write the ServerHello. In the normal course of events this is fine. However if we need to write an early alert during ClientHello processing (e.g. no shared cipher), then we haven't done the write sequence reflection yet. This means the alert gets written with the wrong sequence number (it will just be set to whatever value we left it in the last time we wrote something). If the sequence number is less than expected then the client will believe that the incoming alert is a retransmit and will therefore drop it, causing the client to hang waiting for a response from the server. Fixes #2886 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2915) --- ssl/d1_both.c | 15 +++++++++++---- ssl/d1_srvr.c | 9 --------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/ssl/d1_both.c b/ssl/d1_both.c index 9bc6153610..232a6d4bf4 100644 --- a/ssl/d1_both.c +++ b/ssl/d1_both.c @@ -517,6 +517,17 @@ long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok) return i; } + /* + * Don't change the *message* read sequence number while listening. For + * the *record* write sequence we reflect the ClientHello sequence number + * when listening. + */ + if (s->d1->listen) + memcpy(s->s3->write_sequence, s->s3->read_sequence, + sizeof(s->s3->write_sequence)); + else + s->d1->handshake_read_seq++; + if (mt >= 0 && s->s3->tmp.message_type != mt) { al = SSL_AD_UNEXPECTED_MESSAGE; SSLerr(SSL_F_DTLS1_GET_MESSAGE, SSL_R_UNEXPECTED_MESSAGE); @@ -544,10 +555,6 @@ long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok) memset(msg_hdr, 0x00, sizeof(struct hm_header_st)); - /* Don't change sequence numbers while listening */ - if (!s->d1->listen) - s->d1->handshake_read_seq++; - s->init_msg = s->init_buf->data + DTLS1_HM_HEADER_LENGTH; return s->init_num; diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c index 32eae6bab4..8502b242e5 100644 --- a/ssl/d1_srvr.c +++ b/ssl/d1_srvr.c @@ -355,15 +355,6 @@ int dtls1_accept(SSL *s) s->init_num = 0; - /* - * Reflect ClientHello sequence to remain stateless while - * listening - */ - if (listen) { - memcpy(s->s3->write_sequence, s->s3->read_sequence, - sizeof(s->s3->write_sequence)); - } - /* If we're just listening, stop here */ if (listen && s->state == SSL3_ST_SW_SRVR_HELLO_A) { ret = 2; -- 2.25.1