diff options
author | Emilia Kasper <emilia@openssl.org> | 2015-10-06 17:20:32 +0200 |
---|---|---|
committer | Emilia Kasper <emilia@openssl.org> | 2015-10-09 15:32:35 +0200 |
commit | 310115448188415e270bb0bef958c7c130939838 (patch) | |
tree | 4acce2a2cb0626327668858b21dc9f7811e803c5 /ssl | |
parent | 0f0cfbe24c07376a67b12048686baa318db2cd95 (diff) |
DTLS: remove unused cookie field
Note that this commit constifies a user callback parameter and therefore
will break compilation for applications using this callback. But unless
they are abusing write access to the buffer, the fix is trivial.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/d1_lib.c | 6 | ||||
-rw-r--r-- | ssl/packet_locl.h | 13 | ||||
-rw-r--r-- | ssl/s3_srvr.c | 39 | ||||
-rw-r--r-- | ssl/ssl_locl.h | 3 | ||||
-rw-r--r-- | ssl/ssl_sess.c | 2 |
5 files changed, 25 insertions, 38 deletions
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index 4bdf90a657..3a0a4cf443 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -723,9 +723,9 @@ int dtls1_listen(SSL *s, struct sockaddr *client) /* This is fatal */ return -1; } - if (PACKET_remaining(&cookiepkt) > sizeof(s->d1->rcvd_cookie) - || s->ctx->app_verify_cookie_cb(s, PACKET_data(&cookiepkt), - PACKET_remaining(&cookiepkt)) == 0) { + if (s->ctx->app_verify_cookie_cb(s, PACKET_data(&cookiepkt), + PACKET_remaining(&cookiepkt)) == + 0) { /* * We treat invalid cookies in the same was as no cookie as * per RFC6347 diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h index 9354e6c998..778ec774bc 100644 --- a/ssl/packet_locl.h +++ b/ssl/packet_locl.h @@ -62,6 +62,7 @@ # include <string.h> # include <openssl/bn.h> # include <openssl/buffer.h> +# include <openssl/crypto.h> # include "e_os.h" # ifdef __cplusplus @@ -125,6 +126,18 @@ static inline void PACKET_null_init(PACKET *pkt) } /* + * Returns 1 if the packet has length |num| and its contents equal the |num| + * bytes read from |ptr|. Returns 0 otherwise (lengths or contents not equal). + * If lengths are equal, performs the comparison in constant time. + */ +__owur static inline int PACKET_equal(const PACKET *pkt, const void *ptr, + size_t num) { + if (PACKET_remaining(pkt) != num) + return 0; + return CRYPTO_memcmp(pkt->curr, ptr, num) == 0; +} + +/* * Peek ahead and initialize |subpkt| with the next |len| bytes read from |pkt|. * Data is not copied: the |subpkt| packet will share its underlying buffer with * the original |pkt|, so data wrapped by |pkt| must outlive the |subpkt|. diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 5f05b9f21f..ca11c6e8b3 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -1137,45 +1137,20 @@ int ssl3_get_client_hello(SSL *s) } if (SSL_IS_DTLS(s)) { - size_t cookie_len = PACKET_remaining(&cookie); - /* - * The ClientHello may contain a cookie even if the - * HelloVerify message has not been sent--make sure that it - * does not cause an overflow. - */ - if (cookie_len > sizeof(s->d1->rcvd_cookie)) { - /* too much data */ - al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH); - goto f_err; - } - - /* verify the cookie if appropriate option is set. */ - if ((SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) && cookie_len > 0) { - /* Get cookie */ - /* - * TODO(openssl-team): rcvd_cookie appears unused outside this - * function. Remove the field? - */ - if (!PACKET_copy_bytes(&cookie, s->d1->rcvd_cookie, cookie_len)) { - al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); - goto f_err; - } - + /* Empty cookie was already handled above by returning early. */ + if (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) { if (s->ctx->app_verify_cookie_cb != NULL) { - if (s->ctx->app_verify_cookie_cb(s, s->d1->rcvd_cookie, - cookie_len) == 0) { + if (s->ctx->app_verify_cookie_cb(s, PACKET_data(&cookie), + PACKET_remaining(&cookie)) == 0) { al = SSL_AD_HANDSHAKE_FAILURE; SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH); goto f_err; + /* else cookie verification succeeded */ } - /* else cookie verification succeeded */ - } /* default verification */ - else if (memcmp(s->d1->rcvd_cookie, s->d1->cookie, - s->d1->cookie_len) != 0) { + } else if (!PACKET_equal(&cookie, s->d1->cookie, + s->d1->cookie_len)) { al = SSL_AD_HANDSHAKE_FAILURE; SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH); goto f_err; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 7c57509fbc..ad6ae0ebca 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -798,7 +798,7 @@ struct ssl_ctx_st { unsigned int *cookie_len); /* verify cookie callback */ - int (*app_verify_cookie_cb) (SSL *ssl, unsigned char *cookie, + int (*app_verify_cookie_cb) (SSL *ssl, const unsigned char *cookie, unsigned int cookie_len); CRYPTO_EX_DATA ex_data; @@ -1421,7 +1421,6 @@ typedef struct hm_fragment_st { typedef struct dtls1_state_st { unsigned int send_cookie; unsigned char cookie[DTLS1_COOKIE_LENGTH]; - unsigned char rcvd_cookie[DTLS1_COOKIE_LENGTH]; unsigned int cookie_len; /* handshake message numbers */ diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 7660292196..6f46b9f37e 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -1217,7 +1217,7 @@ void SSL_CTX_set_cookie_generate_cb(SSL_CTX *ctx, } void SSL_CTX_set_cookie_verify_cb(SSL_CTX *ctx, - int (*cb) (SSL *ssl, unsigned char *cookie, + int (*cb) (SSL *ssl, const unsigned char *cookie, unsigned int cookie_len)) { ctx->app_verify_cookie_cb = cb; |