From 197ab47bdd7e9d5b897091132dd6c05b0a06360a Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Fri, 4 Sep 2009 17:53:30 +0000 Subject: [PATCH] PR: 2028 Submitted by: Robin Seggelmann Approved by: steve@openssl.org Fix DTLS cookie management bugs. --- apps/s_apps.h | 3 ++ apps/s_cb.c | 88 ++++++++++++++++++++++++++++++++++++++++++ apps/s_server.c | 4 ++ crypto/bio/bio.h | 3 ++ crypto/bio/bss_dgram.c | 12 ++++-- ssl/d1_srvr.c | 16 ++------ ssl/dtls1.h | 2 +- ssl/s3_srvr.c | 35 ++++++++++------- ssl/t1_lib.c | 8 ++++ 9 files changed, 140 insertions(+), 31 deletions(-) diff --git a/apps/s_apps.h b/apps/s_apps.h index 08fbbc2229..f5a39bae66 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 974ed5d0a5..8c388afd3f 100644 --- a/apps/s_cb.c +++ b/apps/s_cb.c @@ -117,12 +117,17 @@ #undef NON_MAIN #undef USE_SOCKETS #include +#include #include #include #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 5c5168aad4..cedd64c43c 100644 --- 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 b576e347a0..ebb42781e6 100644 --- 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 cd9f497a25..97b9ec8a5f 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)) { @@ -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 992b6a742e..526cc9c745 100644 --- 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 3775b36eb4..8909dc9ee0 100644 --- 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 a9f0077968..535b61b83a 100644 --- 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 c813729f33..2fc5176dcf 100644 --- 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; -- 2.25.1