When handshake messages can't be reassembled because a fragment got lost, the ChangeCipherSpec included in the same flight was still processed. The new mastersecret has not been calculated yet, so random memory is used causing the connection to fail. This patch drops every ChangeCipherSpec which arrives before all previous messages are processed. It doesn't have to be buffered because always the entire flight is repeated, so it will be sent again anyway. This patch also fixes wrong socket timing causing retransmissions to be sent later than necessary. Finally this patch also fixes a problem with stale retransmits of Finished messages of a previous handshake when a new one is initiated. It will be dropped instead of buffered and processed after the ChangeCipherSpec, if it arrives before the ServerHello, when it obviously can't belong to the current handshake. Otherwise the connection fails with unexpected message.
Thanks to Daniel Mentz for providing hints!
--- crypto/bio/bss_dgram.c 18 May 2009 16:11:58 -0000 1.7.2.12
+++ crypto/bio/bss_dgram.c 4 Jun 2009 19:59:10 -0000
@@ -220,9 +220,10 @@
/* Adjust socket timeout if next handhake message timer
* will expire earlier.
*/
- if (data->socket_timeout.tv_sec < timeleft.tv_sec ||
+ if ((data->socket_timeout.tv_sec == 0 && data-
>socket_timeout.tv_usec == 0) ||
+ (data->socket_timeout.tv_sec > timeleft.tv_sec) ||
(data->socket_timeout.tv_sec == timeleft.tv_sec &&
- data->socket_timeout.tv_usec <= timeleft.tv_usec))
+ data->socket_timeout.tv_usec >= timeleft.tv_usec))
{
#ifdef OPENSSL_SYS_WINDOWS
timeout = timeleft.tv_sec * 1000 + timeleft.tv_usec /
1000;
--- ssl/d1_both.c 16 May 2009 16:22:11 -0000 1.14.2.9
+++ ssl/d1_both.c 4 Jun 2009 19:59:11 -0000
@@ -569,9 +569,13 @@
item = pqueue_find(s->d1->buffered_messages, seq64be);
/* Discard the message if sequence number was already there, is
- * too far in the future or the fragment is already in the queue */
+ * too far in the future, already in the queue or if we received
+ * a FINISHED before the SERVER_HELLO, which then must be a stale
+ * retransmit.
+ */
if (msg_hdr->seq <= s->d1->handshake_read_seq ||
- msg_hdr->seq > s->d1->handshake_read_seq + 10 || item != NULL)
+ msg_hdr->seq > s->d1->handshake_read_seq + 10 || item != NULL ||
+ s->d1->handshake_read_seq == 0 && msg_hdr->type ==
SSL3_MT_FINISHED)
{
unsigned char devnull [256];
--- ssl/d1_clnt.c 31 May 2009 17:11:24 -0000 1.16.2.8
+++ ssl/d1_clnt.c 4 Jun 2009 19:59:12 -0000
@@ -442,7 +442,7 @@
case SSL3_ST_CR_FINISHED_A:
case SSL3_ST_CR_FINISHED_B:
-
+ s->d1->change_cipher_spec_ok = 1;
ret=ssl3_get_finished(s,SSL3_ST_CR_FINISHED_A,
SSL3_ST_CR_FINISHED_B);
if (ret <= 0) goto end;
--- ssl/d1_pkt.c 16 May 2009 16:17:46 -0000 1.27.2.8
+++ ssl/d1_pkt.c 4 Jun 2009 19:59:12 -0000
@@ -1102,6 +1102,16 @@
s->msg_callback(0, s->version,
SSL3_RT_CHANGE_CIPHER_SPEC,
rr->data, 1, s, s->msg_callback_arg);
+ /* We can't process a CCS now, because previous handshake
+ * messages are still missing, so just drop it.
+ */
+ if (!s->d1->change_cipher_spec_ok)
+ {
+ goto start;
+ }
+
+ s->d1->change_cipher_spec_ok = 0;
+
s->s3->change_cipher_spec=1;
if (!ssl3_do_change_cipher_spec(s))
goto err;
--- ssl/d1_srvr.c 31 May 2009 17:11:24 -0000 1.20.2.5
+++ ssl/d1_srvr.c 4 Jun 2009 19:59:13 -0000
@@ -501,6 +501,7 @@
ret=ssl3_get_cert_verify(s);
if (ret <= 0) goto end;
dtls1_stop_timer(s);
+ s->d1->change_cipher_spec_ok = 1;
s->state=SSL3_ST_SR_FINISHED_A;
s->init_num=0;
@@ -508,6 +509,7 @@
case SSL3_ST_SR_FINISHED_A:
case SSL3_ST_SR_FINISHED_B:
+ s->d1->change_cipher_spec_ok = 1;
ret=ssl3_get_finished(s,SSL3_ST_SR_FINISHED_A,
SSL3_ST_SR_FINISHED_B);
if (ret <= 0) goto end;
--- ssl/dtls1.h 2 Jun 2009 11:23:30 -0000 1.12.2.9
+++ ssl/dtls1.h 4 Jun 2009 19:59:13 -0000
@@ -231,6 +231,7 @@
unsigned int handshake_fragment_len;
unsigned int retransmitting;
+ unsigned int change_cipher_spec_ok;
} DTLS1_STATE;
dtls-fragment-retransmission-bug-1.0.0.patch
Description: Binary data
dtls-fragment-retransmission-bug-0.9.8.patch
Description: Binary data
