diff options
author | David Benjamin <davidben@chromium.org> | 2015-02-17 00:31:43 +0300 |
---|---|---|
committer | Adam Langley <agl@google.com> | 2015-02-17 23:50:08 +0300 |
commit | a54e2e85ee64a4192c85c8c6250e3879c81248e4 (patch) | |
tree | 66bece29c8ceab91d99549230aeabdf32743a093 | |
parent | 26c2b929ba8d298950c502d0cf3af7f08a94d853 (diff) |
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>
-rw-r--r-- | include/openssl/ssl.h | 16 | ||||
-rw-r--r-- | include/openssl/ssl3.h | 2 | ||||
-rw-r--r-- | ssl/d1_srvr.c | 60 | ||||
-rw-r--r-- | ssl/s3_srvr.c | 40 | ||||
-rw-r--r-- | ssl/ssl_lib.c | 2 | ||||
-rw-r--r-- | ssl/ssl_sess.c | 12 | ||||
-rw-r--r-- | ssl/ssl_stat.c | 16 | ||||
-rw-r--r-- | ssl/test/bssl_shim.cc | 32 | ||||
-rw-r--r-- | ssl/test/runner/runner.go | 12 | ||||
-rw-r--r-- | ssl/test/test_config.cc | 2 | ||||
-rw-r--r-- | ssl/test/test_config.h | 1 |
11 files changed, 1 insertions, 194 deletions
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 42a4e42d..b1b1b7fc 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -391,8 +391,6 @@ typedef struct timeval OPENSSL_timeval; /* 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 @@ struct ssl_ctx_st { /* 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 @@ OPENSSL_EXPORT void SSL_CTX_set_channel_id_cb( 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 8dcc2c2a..de3b96cf 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -585,8 +585,6 @@ typedef struct ssl3_state_st { #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 3919e443..4031dd1c 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 @@ int dtls1_accept(SSL *s) { 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 @@ end: } 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 14e471de..3f89558b 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -946,25 +946,6 @@ int ssl3_get_client_hello(SSL *s) { 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 @@ int ssl3_get_client_hello(SSL *s) { 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 87f4d7bf..a4c94dc2 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1844,8 +1844,6 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) { 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 c6913fc5..aadc84cc 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -889,18 +889,6 @@ int (*SSL_CTX_get_client_cert_cb(SSL_CTX *ctx))(SSL *ssl, X509 **x509, 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 450ed7c1..4703af48 100644 --- a/ssl/ssl_stat.c +++ b/ssl/ssl_stat.c @@ -382,14 +382,6 @@ const char *SSL_state_string_long(const SSL *s) { 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 @@ const char *SSL_state_string(const SSL *s) { 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 0b1ebc56..6ec3aff9 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -199,32 +199,6 @@ static int AlpnSelectCallback(SSL* ssl, const uint8_t** out, uint8_t* outlen, 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 @@ static ScopedSSL_CTX SetupCtx(const TestConfig *config) { 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 @@ static int DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx, 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 9b07a689..99c66a45 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -1880,18 +1880,6 @@ func addStateMachineCoverageTests(async, splitHandshake bool, protocol protocol) }, 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 5d4b7876..78dcb012 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -60,7 +60,6 @@ const Flag<bool> kBoolFlags[] = { { "-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 @@ TestConfig::TestConfig() 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 73ea08ca..3bd65e66 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -42,7 +42,6 @@ struct TestConfig { 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; |