Remove a place where SSL_clear cleans up after client/server confusion.

SSL_clear sets s->state and dtls1_clear sets cookie_len on the server. Setting
cookie_len on the server seems to serve no purpose but to let the callback know
how large the buffer is. This can be done just before calling the callback.

It also avoids a bug where the cookie check can be bypassed, should the server
not specify an app_verify_cookie_cb, by supplying a cookie of all zeros of the
maximum size. (Zero is fine because an empty cookie is rejected.)

The goal here is to avoid needing the SSL_clear calls in the handshake
functions. They are currently needed to fix the cookie_len setting when using
the generic method. (They get set wrong and then flipped back.)

Change-Id: I5095891bc0f7df62d83a9c84312fcf0b84826faa
Reviewed-on: https://boringssl-review.googlesource.com/2435
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2014-11-26 00:29:45 -05:00 committed by Adam Langley
parent ff42cc1eac
commit 8c88153465
3 changed files with 8 additions and 5 deletions

View File

@ -255,11 +255,6 @@ void dtls1_clear(SSL *s)
memset(s->d1, 0, sizeof(*(s->d1)));
if (s->server)
{
s->d1->cookie_len = sizeof(s->d1->cookie);
}
if (SSL_get_options(s) & SSL_OP_NO_QUERY_MTU)
{
s->d1->mtu = mtu;

View File

@ -640,6 +640,10 @@ int dtls1_send_hello_verify_request(SSL *s)
*(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)
{

View File

@ -162,6 +162,10 @@ static int alpn_select_callback(SSL* ssl,
}
static int cookie_generate_callback(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;