Re: [openssl.org #1950] [PATCH] DTLS fragment retransmission bug
Here is an updated version, the last didn't cover every variation of
the handshake message flow because the CertificateVerify state is
entered every time, not only when a CertificateVerify message is
really expected.
--- crypto/bio/bss_dgram.c 5 Jun 2009 08:35:54 - 1.7.2.13
+++ crypto/bio/bss_dgram.c 5 Jun 2009 12:05:47 -
@@ -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 - 1.14.2.9
+++ ssl/d1_both.c 5 Jun 2009 12:05:47 -
@@ -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 - 1.16.2.8
+++ ssl/d1_clnt.c 5 Jun 2009 12:05:47 -
@@ -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.c16 May 2009 16:17:46 - 1.27.2.8
+++ ssl/d1_pkt.c5 Jun 2009 12:05:47 -
@@ -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 - 1.20.2.5
+++ ssl/d1_srvr.c 5 Jun 2009 12:05:47 -
@@ -497,6 +497,7 @@
case SSL3_ST_SR_CERT_VRFY_A:
case SSL3_ST_SR_CERT_VRFY_B:
+ s-d1-change_cipher_spec_ok = 1;
/* we should decide if we expected this one */
ret=ssl3_get_cert_verify(s);
if (ret = 0) goto end;
@@ -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 - 1.12.2.9
+++ ssl/dtls1.h 5 Jun 2009 12:05:47 -
@@ -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
Re: [openssl.org #1950] [PATCH] DTLS fragment retransmission bug
I just found another timing bug...
--- crypto/bio/bss_dgram.c 5 Jun 2009 08:35:54 - 1.7.2.13
+++ crypto/bio/bss_dgram.c 5 Jun 2009 14:00:26 -
@@ -217,12 +217,19 @@
timeleft.tv_usec += 100;
}
+ if (timeleft.tv_sec 0)
+ {
+ timeleft.tv_sec = 0;
+ timeleft.tv_usec = 1;
+ }
+
/* 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 - 1.14.2.9
+++ ssl/d1_both.c 5 Jun 2009 14:00:31 -
@@ -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 - 1.16.2.8
+++ ssl/d1_clnt.c 5 Jun 2009 14:00:31 -
@@ -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.c16 May 2009 16:17:46 - 1.27.2.8
+++ ssl/d1_pkt.c5 Jun 2009 14:00:31 -
@@ -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 - 1.20.2.5
+++ ssl/d1_srvr.c 5 Jun 2009 14:00:31 -
@@ -497,6 +497,7 @@
case SSL3_ST_SR_CERT_VRFY_A:
case SSL3_ST_SR_CERT_VRFY_B:
+ s-d1-change_cipher_spec_ok = 1;
/* we should decide if we expected this one */
ret=ssl3_get_cert_verify(s);
if (ret = 0) goto end;
@@ -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 - 1.12.2.9
+++ ssl/dtls1.h 5 Jun 2009 14:00:31 -
@@ -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
