PR: 2028

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

Fix DTLS cookie management bugs.

diff --git a/apps/s_apps.h b/apps/s_apps.h
index be3b4da7402162145d22b9da87b94796789f79bb..820e5c58155b440c4cf1bd8972ae27027785a905 100644 (file)
--- 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 4d8a66298841e92484f438516a2d14581e1e0f5f..d92f43d52ae07b2b16954e7d50de4e7ee069bc1b 100644 (file)
--- 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 a5b1e9147179f4371986ebad08dae8a65a0b788f..6277c797fc2a21525f2a448f536755f605cffab5 100644 (file)
--- 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 7e94edd5ea2635e335303fe7bd55b770958b6875..152802fbdf2c9ba080ddc50ad640248ef65ac5c4 100644 (file)
--- 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 b2b4a4a993d6d121e9c0d63bba2b14b827e26a1e..89b9986a4b1351ad56d744d9fa38e633939c3ef6 100644 (file)
--- 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 ac09b45ef186d6b868cdeeca3dab09677cd29af4..00536ad3cd6a4913cf79355dabbbce9e2841c72c 100644 (file)
--- 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 f27362cad4d14b70f0b3745de9b908d82cdf7175..926af1c4c5737c4241a156ef6cb1db6df301781e 100644 (file)
--- 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 6c1751a9054505f75c08ae1825e263ac7118c3c6..35e826f35dae97d5e0015e12a3b1c7b5971880a5 100644 (file)
--- 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 bd849ac593a4344357729de3cb03f861ca02fbb3..4d6789264d381d7b16774d7014f19841ef2c112c 100644 (file)
--- 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;