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: NAndy Polyakov <appro@openssl.org>

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;

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];

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));

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

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
@@ -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

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;

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 */

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;

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)