summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apps/s_apps.h2
-rw-r--r--apps/s_cb.c2
-rw-r--r--include/openssl/ssl.h2
-rw-r--r--ssl/d1_lib.c6
-rw-r--r--ssl/packet_locl.h13
-rw-r--r--ssl/s3_srvr.c39
-rw-r--r--ssl/ssl_locl.h3
-rw-r--r--ssl/ssl_sess.c2
-rw-r--r--test/packettest.c20
9 files changed, 48 insertions, 41 deletions
diff --git a/apps/s_apps.h b/apps/s_apps.h
index c8069a0f6e..55dc9f1ffc 100644
--- a/apps/s_apps.h
+++ b/apps/s_apps.h
@@ -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;
diff --git a/apps/s_cb.c b/apps/s_cb.c
index 643d91a160..884b5e1cf3 100644
--- a/apps/s_cb.c
+++ b/apps/s_cb.c
@@ -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];
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 0727e7f78d..25ceca88f1 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -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));
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;
diff --git a/test/packettest.c b/test/packettest.c
index edaa2824ba..ac360f59bd 100644
--- a/test/packettest.c
+++ b/test/packettest.c
@@ -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)
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-20 14:43:44 +0000