Remove server-side HelloVerifyRequest support.
I found no users of this. We can restore it if needbe, but I don't expect
anyone to find it useful in its current form. The API is suspect for the same
reasons DTLSv1_listen was. An SSL object is stateful and assumes you already
have the endpoint separated out.
If we ever need it, server-side HelloVerifyRequest and DTLSv1_listen should be
implemented by a separate stateless listener that statelessly handles
cookieless ClientHello + HelloVerifyRequest. Once a ClientHello with a valid
cookie comes in, it sets up a stateful SSL object and passes control along to
that.
Change-Id: I86adc1dfb6a81bebe987784c36ad6634a9a1b120
Reviewed-on: https://boringssl-review.googlesource.com/3480
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 42a4e42..b1b1b7f 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -391,8 +391,6 @@
/* DTLS options */
#define SSL_OP_NO_QUERY_MTU 0x00001000L
-/* Turn on Cookie Exchange (on relevant for servers) */
-#define SSL_OP_COOKIE_EXCHANGE 0x00002000L
/* Don't use RFC4507 ticket extension */
#define SSL_OP_NO_TICKET 0x00004000L
@@ -748,13 +746,6 @@
/* get channel id callback */
void (*channel_id_cb)(SSL *ssl, EVP_PKEY **pkey);
- /* cookie generate callback */
- int (*app_gen_cookie_cb)(SSL *ssl, uint8_t *cookie, size_t *cookie_len);
-
- /* verify cookie callback */
- int (*app_verify_cookie_cb)(SSL *ssl, const uint8_t *cookie,
- size_t cookie_len);
-
CRYPTO_EX_DATA ex_data;
STACK_OF(X509) *extra_certs;
@@ -987,13 +978,6 @@
SSL_CTX *ctx, void (*channel_id_cb)(SSL *ssl, EVP_PKEY **pkey));
OPENSSL_EXPORT void (*SSL_CTX_get_channel_id_cb(SSL_CTX *ctx))(SSL *ssl,
EVP_PKEY **pkey);
-OPENSSL_EXPORT void SSL_CTX_set_cookie_generate_cb(
- SSL_CTX *ctx,
- int (*app_gen_cookie_cb)(SSL *ssl, uint8_t *cookie, size_t *cookie_len));
-OPENSSL_EXPORT void SSL_CTX_set_cookie_verify_cb(
- SSL_CTX *ctx, int (*app_verify_cookie_cb)(SSL *ssl, const uint8_t *cookie,
- size_t cookie_len));
-
/* SSL_enable_signed_cert_timestamps causes |ssl| (which must be the client end
* of a connection) to request SCTs from the server. See
diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index 8dcc2c2..de3b96c 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -585,8 +585,6 @@
#define SSL3_ST_SR_CLNT_HELLO_C (0x112 | SSL_ST_ACCEPT)
#define SSL3_ST_SR_CLNT_HELLO_D (0x115 | SSL_ST_ACCEPT)
/* write to client */
-#define DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A (0x113 | SSL_ST_ACCEPT)
-#define DTLS1_ST_SW_HELLO_VERIFY_REQUEST_B (0x114 | SSL_ST_ACCEPT)
#define SSL3_ST_SW_HELLO_REQ_A (0x120 | SSL_ST_ACCEPT)
#define SSL3_ST_SW_HELLO_REQ_B (0x121 | SSL_ST_ACCEPT)
#define SSL3_ST_SW_HELLO_REQ_C (0x122 | SSL_ST_ACCEPT)
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 3919e44..4031dd1 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -127,8 +127,6 @@
#include "ssl_locl.h"
-static int dtls1_send_hello_verify_request(SSL *s);
-
int dtls1_accept(SSL *s) {
BUF_MEM *buf = NULL;
void (*cb)(const SSL *ssl, int type, int val) = NULL;
@@ -244,33 +242,10 @@
goto end;
}
dtls1_stop_timer(s);
-
- if (ret == 1 && (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE)) {
- s->state = DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A;
- } else {
- s->state = SSL3_ST_SW_SRVR_HELLO_A;
- }
-
+ s->state = SSL3_ST_SW_SRVR_HELLO_A;
s->init_num = 0;
break;
- case DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A:
- case DTLS1_ST_SW_HELLO_VERIFY_REQUEST_B:
- ret = dtls1_send_hello_verify_request(s);
- if (ret <= 0) {
- goto end;
- }
- s->state = SSL3_ST_SW_FLUSH;
- s->s3->tmp.next_state = SSL3_ST_SR_CLNT_HELLO_A;
-
- /* HelloVerifyRequest resets Finished MAC */
- if (!ssl3_init_finished_mac(s)) {
- OPENSSL_PUT_ERROR(SSL, dtls1_accept, ERR_R_INTERNAL_ERROR);
- ret = -1;
- goto end;
- }
- break;
-
case SSL3_ST_SW_SRVR_HELLO_A:
case SSL3_ST_SW_SRVR_HELLO_B:
s->renegotiate = 2;
@@ -565,36 +540,3 @@
}
return ret;
}
-
-int dtls1_send_hello_verify_request(SSL *s) {
- uint8_t *msg, *p;
-
- if (s->state == DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A) {
- msg = p = ssl_handshake_start(s);
- /* Always use DTLS 1.0 version: see RFC 6347 */
- *(p++) = DTLS1_VERSION >> 8;
- *(p++) = DTLS1_VERSION & 0xFF;
-
- /* Inform the callback how much space is in the
- * cookie's buffer. */
- s->d1->cookie_len = sizeof(s->d1->cookie);
-
- if (s->ctx->app_gen_cookie_cb == NULL ||
- s->ctx->app_gen_cookie_cb(s, s->d1->cookie, &(s->d1->cookie_len)) ==
- 0) {
- OPENSSL_PUT_ERROR(SSL, dtls1_send_hello_verify_request,
- ERR_R_INTERNAL_ERROR);
- return 0;
- }
-
- *(p++) = (uint8_t)s->d1->cookie_len;
- memcpy(p, s->d1->cookie, s->d1->cookie_len);
- p += s->d1->cookie_len;
-
- ssl_set_handshake_header(s, DTLS1_MT_HELLO_VERIFY_REQUEST, p - msg);
- s->state = DTLS1_ST_SW_HELLO_VERIFY_REQUEST_B;
- }
-
- /* s->state = DTLS1_ST_SW_HELLO_VERIFY_REQUEST_B */
- return ssl_do_write(s);
-}
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 14e471d..3f89558 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -946,25 +946,6 @@
return n;
}
- /* If we require cookies and this ClientHello doesn't contain one, just
- * return since we do not want to allocate any memory yet. So check
- * cookie length... */
- if (SSL_IS_DTLS(s) && (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE)) {
- uint8_t cookie_length;
-
- CBS_init(&client_hello, s->init_msg, n);
- if (!CBS_skip(&client_hello, 2 + SSL3_RANDOM_SIZE) ||
- !CBS_get_u8_length_prefixed(&client_hello, &session_id) ||
- !CBS_get_u8(&client_hello, &cookie_length)) {
- al = SSL_AD_DECODE_ERROR;
- OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_DECODE_ERROR);
- goto f_err;
- }
-
- if (cookie_length == 0) {
- return 1;
- }
- }
s->state = SSL3_ST_SR_CLNT_HELLO_C;
/* fallthrough */
case SSL3_ST_SR_CLNT_HELLO_C:
@@ -1038,27 +1019,6 @@
OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_DECODE_ERROR);
goto f_err;
}
-
- /* Verify the cookie if appropriate option is set. */
- if ((SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) && CBS_len(&cookie) > 0) {
- if (s->ctx->app_verify_cookie_cb != NULL) {
- if (s->ctx->app_verify_cookie_cb(s, CBS_data(&cookie),
- CBS_len(&cookie)) == 0) {
- al = SSL_AD_HANDSHAKE_FAILURE;
- OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_COOKIE_MISMATCH);
- goto f_err;
- }
- /* else cookie verification succeeded */
- } else if (!CBS_mem_equal(&cookie, s->d1->cookie, s->d1->cookie_len)) {
- /* default verification */
- al = SSL_AD_HANDSHAKE_FAILURE;
- OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_COOKIE_MISMATCH);
- goto f_err;
- }
- /* Set to -2 so if successful we return 2 and don't send
- * HelloVerifyRequest. */
- ret = -2;
- }
}
if (!s->s3->have_version) {
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 87f4d7b..a4c94dc 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1844,8 +1844,6 @@
ret->default_passwd_callback = 0;
ret->default_passwd_callback_userdata = NULL;
ret->client_cert_cb = 0;
- ret->app_gen_cookie_cb = 0;
- ret->app_verify_cookie_cb = 0;
ret->sessions = lh_SSL_SESSION_new(ssl_session_hash, ssl_session_cmp);
if (ret->sessions == NULL) {
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index c6913fc..aadc84c 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -889,18 +889,6 @@
return ctx->client_cert_cb;
}
-void SSL_CTX_set_cookie_generate_cb(SSL_CTX *ctx,
- int (*cb)(SSL *ssl, uint8_t *cookie,
- size_t *cookie_len)) {
- ctx->app_gen_cookie_cb = cb;
-}
-
-void SSL_CTX_set_cookie_verify_cb(SSL_CTX *ctx,
- int (*cb)(SSL *ssl, const uint8_t *cookie,
- size_t cookie_len)) {
- ctx->app_verify_cookie_cb = cb;
-}
-
void SSL_CTX_set_channel_id_cb(SSL_CTX *ctx,
void (*cb)(SSL *ssl, EVP_PKEY **pkey)) {
ctx->channel_id_cb = cb;
diff --git a/ssl/ssl_stat.c b/ssl/ssl_stat.c
index 450ed7c..4703af4 100644
--- a/ssl/ssl_stat.c
+++ b/ssl/ssl_stat.c
@@ -382,14 +382,6 @@
str = "DTLS1 read hello verify request B";
break;
- case DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A:
- str = "DTLS1 write hello verify request A";
- break;
-
- case DTLS1_ST_SW_HELLO_VERIFY_REQUEST_B:
- str = "DTLS1 write hello verify request B";
- break;
-
default:
str = "unknown state";
break;
@@ -691,14 +683,6 @@
str = "DRCHVB";
break;
- case DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A:
- str = "DWCHVA";
- break;
-
- case DTLS1_ST_SW_HELLO_VERIFY_REQUEST_B:
- str = "DWCHVB";
- break;
-
default:
str = "UNKWN ";
break;
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 0b1ebc5..6ec3aff 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -199,32 +199,6 @@
return SSL_TLSEXT_ERR_OK;
}
-static int CookieGenerateCallback(SSL *ssl, uint8_t *cookie,
- size_t *cookie_len) {
- if (*cookie_len < 32) {
- fprintf(stderr, "Insufficient space for cookie\n");
- return 0;
- }
- *cookie_len = 32;
- memset(cookie, 42, *cookie_len);
- return 1;
-}
-
-static int CookieVerifyCallback(SSL *ssl, const uint8_t *cookie,
- size_t cookie_len) {
- if (cookie_len != 32) {
- fprintf(stderr, "Cookie length mismatch.\n");
- return 0;
- }
- for (size_t i = 0; i < cookie_len; i++) {
- if (cookie[i] != 42) {
- fprintf(stderr, "Cookie mismatch.\n");
- return 0;
- }
- }
- return 1;
-}
-
static unsigned PskClientCallback(SSL *ssl, const char *hint,
char *out_identity,
unsigned max_identity_len,
@@ -349,9 +323,6 @@
SSL_CTX_set_alpn_select_cb(ssl_ctx.get(), AlpnSelectCallback, NULL);
}
- SSL_CTX_set_cookie_generate_cb(ssl_ctx.get(), CookieGenerateCallback);
- SSL_CTX_set_cookie_verify_cb(ssl_ctx.get(), CookieVerifyCallback);
-
ssl_ctx->tlsext_channel_id_enabled_new = 1;
SSL_CTX_set_channel_id_cb(ssl_ctx.get(), ChannelIdCallback);
@@ -464,9 +435,6 @@
if (config->no_ssl3) {
SSL_set_options(ssl.get(), SSL_OP_NO_SSLv3);
}
- if (config->cookie_exchange) {
- SSL_set_options(ssl.get(), SSL_OP_COOKIE_EXCHANGE);
- }
if (config->tls_d5_bug) {
SSL_set_options(ssl.get(), SSL_OP_TLS_D5_BUG);
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 9b07a68..99c66a4 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1880,18 +1880,6 @@
},
flags: flags,
})
-
- testCases = append(testCases, testCase{
- testType: serverTest,
- protocol: protocol,
- name: "CookieExchange" + suffix,
- config: Config{
- Bugs: ProtocolBugs{
- MaxHandshakeRecordLength: maxHandshakeRecordLength,
- },
- },
- flags: append(flags, "-cookie-exchange"),
- })
}
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 5d4b787..78dcb01 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -60,7 +60,6 @@
{ "-no-tls11", &TestConfig::no_tls11 },
{ "-no-tls1", &TestConfig::no_tls1 },
{ "-no-ssl3", &TestConfig::no_ssl3 },
- { "-cookie-exchange", &TestConfig::cookie_exchange },
{ "-shim-writes-first", &TestConfig::shim_writes_first },
{ "-tls-d5-bug", &TestConfig::tls_d5_bug },
{ "-expect-session-miss", &TestConfig::expect_session_miss },
@@ -125,7 +124,6 @@
no_tls11(false),
no_tls1(false),
no_ssl3(false),
- cookie_exchange(false),
shim_writes_first(false),
tls_d5_bug(false),
expect_session_miss(false),
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 73ea08c..3bd65e6 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -42,7 +42,6 @@
bool no_tls11;
bool no_tls1;
bool no_ssl3;
- bool cookie_exchange;
std::string expected_channel_id;
std::string send_channel_id;
bool shim_writes_first;
