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 08fbbc2229640f3eb6d4e15af90f7e64926a4970..f5a39bae6636328ef9ec294ff39d14e274758771 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 974ed5d0a5986620bf2248a911445729b224d220..8c388afd3fa6e112d0190d3d4f6704d15fc77027 100644 (file)
--- a/apps/s_cb.c
+++ b/apps/s_cb.c
@@ -117,12 +117,17 @@
 #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;
+unsigned char cookie_secret[COOKIE_SECRET_LENGTH];
+int cookie_initialized=0;
 
 int MS_CALLBACK verify_callback(int ok, X509_STORE_CTX *ctx)
        {
@@ -646,3 +651,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 = (unsigned char*) 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 5c5168aad45e3ea8419e047c5eabe730d333ca37..cedd64c43c0633abbad0e806be04e36f2e4a1555 100644 (file)
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -1497,6 +1497,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 b576e347a08e6c0052b2afbb4ff99bed27900775..ebb42781e6ef9f657ea55fd3dca63ad63108b468 100644 (file)
--- a/crypto/bio/bio.h
+++ b/crypto/bio/bio.h
@@ -156,6 +156,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
@@ -543,6 +544,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 cd9f497a25031991c7475c7b8c74388c3e3a44f4..97b9ec8a5f564391bdc6ce89615c22ae7bb11228 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))
                                {
@@ -514,6 +514,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 992b6a742e2c2dc16fd26202e86b123fc3afd7e1..526cc9c74513134993d94f39579ce437b8df2ab6 100644 (file)
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -236,11 +236,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:
@@ -271,7 +266,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;
@@ -285,7 +280,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;
 
@@ -647,15 +641,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 3775b36eb4f7efc30a4e60a67334281c0cb0e11a..8909dc9ee00407fba03b20466d61f055f38f8c9e 100644 (file)
--- a/ssl/dtls1.h
+++ b/ssl/dtls1.h
@@ -88,7 +88,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 a9f00779684664e08083944bcd8b0116ac664256..535b61b83a1c6d61c29de7458a73c81261709ba7 100644 (file)
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -758,6 +758,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;
@@ -797,23 +812,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
@@ -828,7 +831,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);
@@ -853,6 +856,8 @@ int ssl3_get_client_hello(SSL *s)
                                                SSL_R_COOKIE_MISMATCH);
                                        goto f_err;
                                }
+
+                       ret = 2;
                        }
 
                p += cookie_len;
@@ -1087,7 +1092,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 c813729f3348c63eb54e6fdaeb34be0a80b38713..2fc5176dcf58631352d3b497d5a24565df8d74f3 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -753,6 +753,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;