PR: 2028

Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Approved by: steve@openssl.org

Fix DTLS cookie management bugs.

9 files changed, 140 insertions, 31 deletions

diff --git a/apps/s_apps.h b/apps/s_apps.h
index be3b4da740..820e5c5815 100644
--- a/apps/s_apps.h
+++ b/apps/s_apps.h
@@ -171,3 +171,6 @@ void MS_CALLBACK tlsext_cb(SSL *s, int client_server, int type,
 					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);
diff --git a/apps/s_cb.c b/apps/s_cb.c
index 4d8a662988..d92f43d52a 100644
--- a/apps/s_cb.c
+++ b/apps/s_cb.c
@@ -117,13 +117,18 @@
 #undef NON_MAIN
 #undef USE_SOCKETS
 #include <openssl/err.h>
+#include <openssl/rand.h>
 #include <openssl/x509.h>
 #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)
 	{
@@ -682,3 +687,86 @@ void MS_CALLBACK tlsext_cb(SSL *s, int client_server, int type,
 	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(cookie_secret, COOKIE_SECRET_LENGTH))
+			{
+			BIO_printf(bio_err,"error setting random cookie secret\n");
+			return 0;
+			}
+		cookie_initialized = 1;
+		}
+
+	/* Read peer information */
+	(void)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, 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;
+	
+	/* If secret isn't initialized yet, the cookie can't be valid */
+	if (!cookie_initialized)
+		return 0;
+
+	/* Read peer information */
+	(void)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, result, &resultlength);
+	OPENSSL_free(buffer);
+	
+	if (cookie_len == resultlength && memcmp(result, cookie, resultlength) == 0)
+		return 1;
+
+	return 0;
+	}
diff --git a/apps/s_server.c b/apps/s_server.c
index a5b1e91471..6277c797fc 100644
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -1654,6 +1654,10 @@ bad:
 	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)
 		{
diff --git a/crypto/bio/bio.h b/crypto/bio/bio.h
index 7e94edd5ea..152802fbdf 100644
--- a/crypto/bio/bio.h
+++ b/crypto/bio/bio.h
@@ -157,6 +157,7 @@ extern "C" {
 					      * previous write
 					      * operation */
 
+#define BIO_CTRL_DGRAM_GET_PEER           46
 #define BIO_CTRL_DGRAM_SET_PEER           44 /* Destination for the data */
 
 #define BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT   45 /* Next DTLS handshake timeout to
@@ -538,6 +539,8 @@ int BIO_ctrl_reset_read_request(BIO *b);
          (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)
 
diff --git a/crypto/bio/bss_dgram.c b/crypto/bio/bss_dgram.c
index b2b4a4a993..89b9986a4b 100644
--- a/crypto/bio/bss_dgram.c
+++ b/crypto/bio/bss_dgram.c
@@ -290,11 +290,11 @@ static int dgram_read(BIO *b, char *out, int outl)
 		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 @@ static long dgram_ctrl(BIO *b, int cmd, long num, void *ptr)
 			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;
 
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index ac09b45ef1..00536ad3cd 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -238,11 +238,6 @@ int dtls1_accept(SSL *s)
 				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 @@ int dtls1_accept(SSL *s)
 			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 @@ int dtls1_accept(SSL *s)
 			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,13 @@ int dtls1_send_hello_verify_request(SSL *s)
 		*(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 (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);
diff --git a/ssl/dtls1.h b/ssl/dtls1.h
index f27362cad4..926af1c4c5 100644
--- a/ssl/dtls1.h
+++ b/ssl/dtls1.h
@@ -84,7 +84,7 @@ extern "C" {
 #endif
 
 /* lengths of messages */
-#define DTLS1_COOKIE_LENGTH                     32
+#define DTLS1_COOKIE_LENGTH                     256
 
 #define DTLS1_RT_HEADER_LENGTH                  13
 
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 6c1751a905..35e826f35d 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -816,6 +816,21 @@ int ssl3_get_client_hello(SSL *s)
 		goto f_err;
 		}
 
+	/* If we require cookies and this ClientHello doesn't
+	 * contain one, just return since we do not want to
+	 * allocate any memory yet. So check cookie length...
+	 */
+	if (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE)
+		{
+		unsigned int session_length, cookie_length;
+		
+		session_length = *(p + SSL3_RANDOM_SIZE);
+		cookie_length = *(p + SSL3_RANDOM_SIZE + session_length + 1);
+
+		if (cookie_len == 0)
+			return 1;
+		}
+
 	/* load the client random */
 	memcpy(s->s3->client_random,p,SSL3_RANDOM_SIZE);
 	p+=SSL3_RANDOM_SIZE;
@@ -855,23 +870,11 @@ int ssl3_get_client_hello(SSL *s)
 
 	p+=j;
 
-	if (s->version == DTLS1_VERSION)
+	if (s->version == DTLS1_VERSION || s->version == DTLS1_BAD_VER)
 		{
 		/* 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
@@ -886,7 +889,7 @@ int ssl3_get_client_hello(SSL *s)
 			}
 
 		/* verify the cookie if appropriate option is set. */
-		if ( (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) &&
+		if ((SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) &&
 			cookie_len > 0)
 			{
 			memcpy(s->d1->rcvd_cookie, p, cookie_len);
@@ -911,6 +914,8 @@ int ssl3_get_client_hello(SSL *s)
 						SSL_R_COOKIE_MISMATCH);
 					goto f_err;
 				}
+
+			ret = 2;
 			}
 
 		p += cookie_len;
@@ -1185,7 +1190,7 @@ int ssl3_get_client_hello(SSL *s)
 	 * s->tmp.new_cipher	- the new cipher to use.
 	 */
 
-	ret=1;
+	if (ret < 0) ret=1;
 	if (0)
 		{
 f_err:
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index bd849ac593..4d6789264d 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1444,6 +1444,14 @@ int tls1_process_ticket(SSL *s, unsigned char *session_id, int len,
 		return 1;
 	if (p >= limit)
 		return -1;
+	/* Skip past DTLS cookie */
+	if (s->version == DTLS1_VERSION || s->version == DTLS1_BAD_VER)
+		{
+		i = *(p++);
+		p+= i;
+		if (p >= limit)
+			return -1;
+		}
 	/* Skip past cipher list */
 	n2s(p, i);
 	p+= i;