This patch fixes several issues with DTLS cookies. At first the maximum cookie length was defined as 32 bytes, while the specification states 256 bytes. Then there was code in the wrong order which prevented the use of cookies larger than 0 bytes in ssl3_get_client_hello(), it was tried to process extensions before the cookie. The present implementation always expects a ClientHello, sends a HelloVerifyRequest and then expects a ClientHello with a cookie attached. Everything else is considered as an error, which is not correct because the first ClientHello could be repeated. Now every ClientHello is answered with a HelloVerifyRequest until a ClientHello with cookie appears. Then the cookie is verified and an error returned if the verification failed. If SSL_OP_COOKIE_EXCHANGE is set, in dtls1_send_hello_verify_request() the cookie is sent. It was assumed that the cookie was specified "somehow" else if the user did not register a callback function for cookie generation. This doesn't make any sense because if the user doesn't use the designated function to generate cookies, how else should they be generated? The result is a cookie with length 0, which is almost as bad as no HelloVerifyRequest at all. Now an internal error occurs if the user requests cookie exchange but did not register the required callback functions. The application s_server activated cookies but didn't supply any generation or verification functions. They are added now and use a 16 byte random number as a secret to calculate an HMAC over the peer's address and port as suggested in the DTLS specification.
--- apps/s_apps.h 22 Dec 2008 14:05:42 -0000 1.21
+++ apps/s_apps.h 31 Aug 2009 13:24:35 -0000
@@ -171,3 +171,6 @@
unsigned char *data, int len,
void *arg);
#endif
+
+int MS_CALLBACK generate_cookie_callback(SSL *ssl, unsigned char
*cookie, unsigned int *cookie_len);
+int MS_CALLBACK verify_cookie_callback(SSL *ssl, unsigned char
*cookie, unsigned int cookie_len);
--- apps/s_cb.c 24 Jul 2009 11:17:10 -0000 1.28
+++ apps/s_cb.c 31 Aug 2009 13:24:35 -0000
@@ -121,9 +121,13 @@
#include <openssl/ssl.h>
#include "s_apps.h"
+#define COOKIE_SECRET_LENGTH 16
+
int verify_depth=0;
int verify_error=X509_V_OK;
int verify_return_error=0;
+unsigned char cookie_secret[COOKIE_SECRET_LENGTH];
+int cookie_initialized=0;
int MS_CALLBACK verify_callback(int ok, X509_STORE_CTX *ctx)
{
@@ -668,3 +690,93 @@
BIO_dump(bio, (char *)data, len);
(void)BIO_flush(bio);
}
+
+int MS_CALLBACK generate_cookie_callback(SSL *ssl, unsigned char
*cookie, unsigned int *cookie_len)
+ {
+ unsigned char *buffer, result[EVP_MAX_MD_SIZE];
+ unsigned int length, resultlength;
+ struct sockaddr_in peer;
+
+ /* Initialize a random secret */
+ if (!cookie_initialized)
+ {
+ if (!RAND_bytes((unsigned char*) &cookie_secret,
COOKIE_SECRET_LENGTH))
+ {
+ BIO_printf(bio_err,"error setting random cookie
secret\n");
+ return 0;
+ }
+ cookie_initialized = 1;
+ }
+
+ /* Read peer information */
+ BIO_dgram_get_peer(SSL_get_rbio(ssl), &peer);
+
+ /* Create buffer with peer's address and port */
+ length = sizeof(peer.sin_addr);
+ length += sizeof(peer.sin_port);
+ buffer = OPENSSL_malloc(length);
+
+ if (buffer == NULL)
+ {
+ BIO_printf(bio_err,"out of memory\n");
+ return 0;
+ }
+
+ memcpy(buffer, &peer.sin_addr, sizeof(peer.sin_addr));
+ memcpy(buffer + sizeof(peer.sin_addr), &peer.sin_port,
sizeof(peer.sin_port));
+
+ /* Calculate HMAC of buffer using the secret */
+ HMAC(EVP_sha1(), &cookie_secret, COOKIE_SECRET_LENGTH, buffer,
+ length, (unsigned char*) &result, &resultlength);
+ OPENSSL_free(buffer);
+
+ memcpy(cookie, result, resultlength);
+ *cookie_len = resultlength;
+
+ return 1;
+ }
+
+int MS_CALLBACK verify_cookie_callback(SSL *ssl, unsigned char
*cookie, unsigned int cookie_len)
+ {
+ unsigned char *buffer, result[EVP_MAX_MD_SIZE];
+ unsigned int length, resultlength;
+ struct sockaddr_in peer;
+
+ /* Initialize a random secret */
+ if (!cookie_initialized)
+ {
+ if (!RAND_bytes((unsigned char*) &cookie_secret,
COOKIE_SECRET_LENGTH))
+ {
+ BIO_printf(bio_err,"error setting random cookie
secret\n");
+ return 0;
+ }
+ cookie_initialized = 1;
+ }
+
+ /* Read peer information */
+ BIO_dgram_get_peer(SSL_get_rbio(ssl), &peer);
+
+ /* Create buffer with peer's address and port */
+ length = sizeof(peer.sin_addr);
+ length += sizeof(peer.sin_port);
+ buffer = OPENSSL_malloc(length);
+
+ if (buffer == NULL)
+ {
+ BIO_printf(bio_err,"out of memory\n");
+ return 0;
+ }
+
+ memcpy(buffer, &peer.sin_addr, sizeof(peer.sin_addr));
+ memcpy(buffer + sizeof(peer.sin_addr), &peer.sin_port,
sizeof(peer.sin_port));
+
+ /* Calculate HMAC of buffer using the secret */
+ HMAC(EVP_sha1(), &cookie_secret, COOKIE_SECRET_LENGTH, buffer,
+ length, (unsigned char*) &result, &resultlength);
+ OPENSSL_free(buffer);
+
+ if (cookie_len == resultlength && memcmp(&result, cookie,
resultlength) == 0)
+ return 1;
+
+ return 0;
+ }
--- apps/s_server.c 18 Aug 2009 11:15:33 -0000 1.143
+++ apps/s_server.c 31 Aug 2009 13:24:35 -0000
@@ -1654,6 +1654,10 @@
SSL_CTX_set_session_id_context(ctx,
(void*)&s_server_session_id_context,
sizeof s_server_session_id_context);
+ /* Set DTLS cookie generation and verification callbacks */
+ SSL_CTX_set_cookie_generate_cb(ctx, generate_cookie_callback);
+ SSL_CTX_set_cookie_verify_cb(ctx, verify_cookie_callback);
+
#ifndef OPENSSL_NO_TLSEXT
if (ctx2)
{
--- crypto/bio/bio.h 24 Jul 2009 11:25:13 -0000 1.80
+++ crypto/bio/bio.h 31 Aug 2009 13:24:35 -0000
@@ -157,9 +157,10 @@
* previous write
* operation */
-#define BIO_CTRL_DGRAM_SET_PEER 44 /* Destination for the
data */
+#define BIO_CTRL_DGRAM_GET_PEER 44
+#define BIO_CTRL_DGRAM_SET_PEER 45 /* Destination for the
data */
-#define BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT 45 /* Next DTLS handshake
timeout to
+#define BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT 46 /* Next DTLS handshake
timeout to
* adjust socket timeouts */
/* modifiers */
@@ -538,6 +539,8 @@
(int)BIO_ctrl(b, BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP, 0, NULL)
#define BIO_dgram_send_timedout(b) \
(int)BIO_ctrl(b, BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP, 0, NULL)
+#define BIO_dgram_get_peer(b,peer) \
+ (int)BIO_ctrl(b, BIO_CTRL_DGRAM_GET_PEER, 0, (char *)peer)
#define BIO_dgram_set_peer(b,peer) \
(int)BIO_ctrl(b, BIO_CTRL_DGRAM_SET_PEER, 0, (char *)peer)
--- crypto/bio/bss_dgram.c 26 Aug 2009 15:15:15 -0000 1.21
+++ crypto/bio/bss_dgram.c 31 Aug 2009 13:24:35 -0000
@@ -290,11 +290,11 @@
ret=recvfrom(b->num,out,outl,0,&peer,(void *)&peerlen);
dgram_reset_rcv_timeout(b);
- if ( ! data->connected && ret > 0)
- BIO_ctrl(b, BIO_CTRL_DGRAM_CONNECT, 0, &peer);
+ if ( ! data->connected && ret >= 0)
+ BIO_ctrl(b, BIO_CTRL_DGRAM_SET_PEER, 0, &peer);
BIO_clear_retry_flags(b);
- if (ret <= 0)
+ if (ret < 0)
{
if (BIO_dgram_should_retry(ret))
{
@@ -518,6 +518,12 @@
memset(&(data->peer), 0x00, sizeof(struct sockaddr));
}
break;
+ case BIO_CTRL_DGRAM_GET_PEER:
+ to = (struct sockaddr *) ptr;
+
+ memcpy(to, &(data->peer), sizeof(struct sockaddr));
+ ret = sizeof(struct sockaddr);
+ break;
case BIO_CTRL_DGRAM_SET_PEER:
to = (struct sockaddr *) ptr;
--- ssl/d1_srvr.c 5 Jun 2009 14:59:26 -0000 1.25
+++ ssl/d1_srvr.c 31 Aug 2009 13:24:35 -0000
@@ -238,11 +238,6 @@
s->state=SSL3_ST_SW_HELLO_REQ_A;
}
- if ( (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE))
- s->d1->send_cookie = 1;
- else
- s->d1->send_cookie = 0;
-
break;
case SSL3_ST_SW_HELLO_REQ_A:
@@ -273,7 +268,7 @@
dtls1_stop_timer(s);
s->new_session = 2;
- if (s->d1->send_cookie)
+ if (ret == 1 && (SSL_get_options(s) &
SSL_OP_COOKIE_EXCHANGE))
s->state = DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A;
else
s->state = SSL3_ST_SW_SRVR_HELLO_A;
@@ -287,7 +282,6 @@
dtls1_start_timer(s);
ret = dtls1_send_hello_verify_request(s);
if ( ret <= 0) goto end;
- s->d1->send_cookie = 0;
s->state=SSL3_ST_SW_FLUSH;
s->s3->tmp.next_state=SSL3_ST_SR_CLNT_HELLO_A;
@@ -670,15 +664,14 @@
*(p++) = s->version >> 8;
*(p++) = s->version & 0xFF;
- if (s->ctx->app_gen_cookie_cb != NULL &&
- s->ctx->app_gen_cookie_cb(s, s->d1->cookie,
- &(s->d1->cookie_len)) == 0)
+ if ((SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) &&
+ (s->ctx->app_gen_cookie_cb == NULL ||
+ s->ctx->app_gen_cookie_cb(s, s->d1->cookie,
+ &(s->d1->cookie_len)) == 0))
{
SSLerr(SSL_F_DTLS1_SEND_HELLO_VERIFY_REQUEST,ERR_R_INTERNAL_ERROR);
return 0;
}
- /* else the cookie is assumed to have
- * been initialized by the application */
*(p++) = (unsigned char) s->d1->cookie_len;
memcpy(p, s->d1->cookie, s->d1->cookie_len);
--- ssl/dtls1.h 17 Jun 2009 11:37:44 -0000 1.21
+++ ssl/dtls1.h 31 Aug 2009 13:24:35 -0000
@@ -84,7 +84,7 @@
#endif
/* lengths of messages */
-#define DTLS1_COOKIE_LENGTH 32
+#define DTLS1_COOKIE_LENGTH 256
#define DTLS1_RT_HEADER_LENGTH 13
--- ssl/s3_srvr.c 26 Jun 2009 15:03:35 -0000 1.177
+++ ssl/s3_srvr.c 31 Aug 2009 13:24:35 -0000
@@ -823,55 +823,11 @@
/* get the session-id */
j= *(p++);
- s->hit=0;
- /* Versions before 0.9.7 always allow session reuse during
renegotiation
- * (i.e. when s->new_session is true), option
- * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION is new with 0.9.7.
- * Maybe this optional behaviour should always have been the default,
- * but we cannot safely change the default behaviour (or new
applications
- * might be written that become totally unsecure when compiled with
- * an earlier library version)
- */
- if ((s->new_session && (s->options &
SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION)))
- {
- if (!ssl_get_new_session(s,1))
- goto err;
- }
- else
- {
- i=ssl_get_prev_session(s, p, j, d + n);
- if (i == 1)
- { /* previous session */
- s->hit=1;
- }
- else if (i == -1)
- goto err;
- else /* i == 0 */
- {
- if (!ssl_get_new_session(s,1))
- goto err;
- }
- }
-
- p+=j;
-
if (s->version == DTLS1_VERSION)
{
/* cookie stuff */
cookie_len = *(p++);
- if ( (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) &&
- s->d1->send_cookie == 0)
- {
- /* HelloVerifyMessage has already been sent */
- if ( cookie_len != s->d1->cookie_len)
- {
- al = SSL_AD_HANDSHAKE_FAILURE;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
SSL_R_COOKIE_MISMATCH);
- goto f_err;
- }
- }
-
/*
* The ClientHello may contain a cookie even if the
* HelloVerify message has not been sent--make sure that it
@@ -911,11 +867,45 @@
SSL_R_COOKIE_MISMATCH);
goto f_err;
}
+
+ ret = 2;
}
p += cookie_len;
}
+ s->hit=0;
+ /* Versions before 0.9.7 always allow session reuse during
renegotiation
+ * (i.e. when s->new_session is true), option
+ * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION is new with 0.9.7.
+ * Maybe this optional behaviour should always have been the default,
+ * but we cannot safely change the default behaviour (or new
applications
+ * might be written that become totally unsecure when compiled with
+ * an earlier library version)
+ */
+ if ((s->new_session && (s->options &
SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION)))
+ {
+ if (!ssl_get_new_session(s,1))
+ goto err;
+ }
+ else
+ {
+ i=ssl_get_prev_session(s, p, j, d + n);
+ if (i == 1)
+ { /* previous session */
+ s->hit=1;
+ }
+ else if (i == -1)
+ goto err;
+ else /* i == 0 */
+ {
+ if (!ssl_get_new_session(s,1))
+ goto err;
+ }
+ }
+
+ p+=j;
+
n2s(p,i);
if ((i == 0) && (j != 0))
{
@@ -1185,7 +1175,7 @@
* s->tmp.new_cipher - the new cipher to use.
*/
- ret=1;
+ if (ret < 0) ret=1;
if (0)
{
f_err:
dtls-cookie-management-bugs.patch
Description: Binary data
