[openssl-commits] [openssl] master update
Emilia Kasper
emilia at openssl.org
Fri Oct 9 13:38:36 UTC 2015
The branch master has been updated
via 310115448188415e270bb0bef958c7c130939838 (commit)
from 0f0cfbe24c07376a67b12048686baa318db2cd95 (commit)
- Log -----------------------------------------------------------------
commit 310115448188415e270bb0bef958c7c130939838
Author: Emilia Kasper <emilia at openssl.org>
Date: Tue Oct 6 17:20:32 2015 +0200
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 at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
apps/s_apps.h | 2 +-
apps/s_cb.c | 2 +-
include/openssl/ssl.h | 2 +-
ssl/d1_lib.c | 6 +++---
ssl/packet_locl.h | 13 +++++++++++++
ssl/s3_srvr.c | 39 +++++++--------------------------------
ssl/ssl_locl.h | 3 +--
ssl/ssl_sess.c | 2 +-
test/packettest.c | 20 ++++++++++++++++++++
9 files changed, 48 insertions(+), 41 deletions(-)
diff --git a/apps/s_apps.h b/apps/s_apps.h
index c8069a0..55dc9f1 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 643d91a..884b5e1 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 0727e7f..25ceca8 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 4bdf90a..3a0a4cf 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 9354e6c..778ec77 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 5f05b9f..ca11c6e 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 7c57509..ad6ae0e 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 7660292..6f46b9f 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 edaa282..ac360f59 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)
More information about the openssl-commits
mailing list