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>
This commit is contained in:
parent
0f0cfbe24c
commit
3101154481
@ -195,7 +195,7 @@ void tlsext_cb(SSL *s, int client_server, int type, unsigned char *data,
|
||||
|
||||
int generate_cookie_callback(SSL *ssl, unsigned char *cookie,
|
||||
unsigned int *cookie_len);
|
||||
int verify_cookie_callback(SSL *ssl, unsigned char *cookie,
|
||||
int verify_cookie_callback(SSL *ssl, const unsigned char *cookie,
|
||||
unsigned int cookie_len);
|
||||
|
||||
typedef struct ssl_excert_st SSL_EXCERT;
|
||||
|
@ -806,7 +806,7 @@ int generate_cookie_callback(SSL *ssl, unsigned char *cookie,
|
||||
return 1;
|
||||
}
|
||||
|
||||
int verify_cookie_callback(SSL *ssl, unsigned char *cookie,
|
||||
int verify_cookie_callback(SSL *ssl, const unsigned char *cookie,
|
||||
unsigned int cookie_len)
|
||||
{
|
||||
unsigned char *buffer, result[EVP_MAX_MD_SIZE];
|
||||
|
@ -750,7 +750,7 @@ void SSL_CTX_set_cookie_generate_cb(SSL_CTX *ctx,
|
||||
*cookie_len));
|
||||
void SSL_CTX_set_cookie_verify_cb(SSL_CTX *ctx,
|
||||
int (*app_verify_cookie_cb) (SSL *ssl,
|
||||
unsigned char
|
||||
const unsigned char
|
||||
*cookie,
|
||||
unsigned int
|
||||
cookie_len));
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
@ -124,6 +125,18 @@ static inline void PACKET_null_init(PACKET *pkt)
|
||||
pkt->remaining = 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* 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
|
||||
|
@ -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;
|
||||
|
@ -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 */
|
||||
|
@ -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;
|
||||
|
@ -360,6 +360,25 @@ static int test_PACKET_null_init()
|
||||
return 1;
|
||||
}
|
||||
|
||||
static int test_PACKET_equal(unsigned char buf[BUF_LEN])
|
||||
{
|
||||
PACKET pkt;
|
||||
|
||||
if ( !PACKET_buf_init(&pkt, buf, 4)
|
||||
|| !PACKET_equal(&pkt, buf, 4)
|
||||
|| PACKET_equal(&pkt, buf + 1, 4)
|
||||
|| !PACKET_buf_init(&pkt, buf, BUF_LEN)
|
||||
|| !PACKET_equal(&pkt, buf, BUF_LEN)
|
||||
|| PACKET_equal(&pkt, buf, BUF_LEN - 1)
|
||||
|| PACKET_equal(&pkt, buf, BUF_LEN + 1)
|
||||
|| PACKET_equal(&pkt, buf, 0)) {
|
||||
fprintf(stderr, "test_PACKET_equal() failed\n");
|
||||
return 0;
|
||||
}
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
static int test_PACKET_get_length_prefixed_1()
|
||||
{
|
||||
unsigned char buf[BUF_LEN];
|
||||
@ -452,6 +471,7 @@ int main(int argc, char **argv)
|
||||
if ( !test_PACKET_buf_init()
|
||||
|| !test_PACKET_null_init()
|
||||
|| !test_PACKET_remaining(buf)
|
||||
|| !test_PACKET_equal(buf)
|
||||
|| !test_PACKET_get_1(buf)
|
||||
|| !test_PACKET_get_4(buf)
|
||||
|| !test_PACKET_get_net_2(buf)
|
||||
|
Loading…
x
Reference in New Issue
Block a user