diff --git a/include/openssl/dtls1.h b/include/openssl/dtls1.h index 18fd13685..5aef0c45d 100644 --- a/include/openssl/dtls1.h +++ b/include/openssl/dtls1.h @@ -171,7 +171,10 @@ typedef struct hm_fragment_st typedef struct dtls1_state_st { + /* send_cookie is true if we are resending the ClientHello + * with a cookie from a HelloVerifyRequest. */ unsigned int send_cookie; + uint8_t cookie[DTLS1_COOKIE_LENGTH]; size_t cookie_len; diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c index b34ed426c..282c427a4 100644 --- a/ssl/d1_clnt.c +++ b/ssl/d1_clnt.c @@ -232,8 +232,6 @@ int dtls1_connect(SSL *s) s->state=SSL3_ST_CW_CLNT_HELLO_A; s->ctx->stats.sess_connect++; s->init_num=0; - /* mark client_random uninitialized */ - memset(s->s3->client_random,0,sizeof(s->s3->client_random)); s->d1->send_cookie = 0; s->hit = 0; break; @@ -256,7 +254,7 @@ int dtls1_connect(SSL *s) s->s3->tmp.next_state=SSL3_ST_CR_SRVR_HELLO_A; } else - s->state=SSL3_ST_CR_SRVR_HELLO_A; + s->state=DTLS1_ST_CR_HELLO_VERIFY_REQUEST_A; s->init_num=0; /* turn on buffering for the next lot of output */ @@ -265,41 +263,46 @@ int dtls1_connect(SSL *s) break; - case SSL3_ST_CR_SRVR_HELLO_A: - case SSL3_ST_CR_SRVR_HELLO_B: - ret=ssl3_get_server_hello(s); - if (ret <= 0) goto end; - else - { - if (s->hit) - { - s->state=SSL3_ST_CR_FINISHED_A; - if (s->tlsext_ticket_expected) - { - /* receive renewed session ticket */ - s->state=SSL3_ST_CR_SESSION_TICKET_A; - } - } - else - s->state=DTLS1_ST_CR_HELLO_VERIFY_REQUEST_A; - } - s->init_num=0; - break; - case DTLS1_ST_CR_HELLO_VERIFY_REQUEST_A: case DTLS1_ST_CR_HELLO_VERIFY_REQUEST_B: ret = dtls1_get_hello_verify(s); if ( ret <= 0) goto end; - dtls1_stop_timer(s); - if ( s->d1->send_cookie) /* start again, with a cookie */ - s->state=SSL3_ST_CW_CLNT_HELLO_A; + if ( s->d1->send_cookie) + { + /* start again, with a cookie */ + dtls1_stop_timer(s); + s->state = SSL3_ST_CW_CLNT_HELLO_A; + } else - s->state = SSL3_ST_CR_CERT_A; + { + s->state = SSL3_ST_CR_SRVR_HELLO_A; + } s->init_num = 0; break; + case SSL3_ST_CR_SRVR_HELLO_A: + case SSL3_ST_CR_SRVR_HELLO_B: + ret=ssl3_get_server_hello(s); + if (ret <= 0) goto end; + + if (s->hit) + { + s->state=SSL3_ST_CR_FINISHED_A; + if (s->tlsext_ticket_expected) + { + /* receive renewed session ticket */ + s->state=SSL3_ST_CR_SESSION_TICKET_A; + } + } + else + { + s->state=SSL3_ST_CR_CERT_A; + } + s->init_num=0; + break; + case SSL3_ST_CR_CERT_A: case SSL3_ST_CR_CERT_B: if (ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher)) @@ -594,7 +597,8 @@ static int dtls1_get_hello_verify(SSL *s) DTLS1_ST_CR_HELLO_VERIFY_REQUEST_A, DTLS1_ST_CR_HELLO_VERIFY_REQUEST_B, -1, - s->max_cert_list, + /* Use the same maximum size as ssl3_get_server_hello. */ + 20000, &ok); s->first_packet = 0; @@ -603,7 +607,7 @@ static int dtls1_get_hello_verify(SSL *s) if (s->s3->tmp.message_type != DTLS1_MT_HELLO_VERIFY_REQUEST) { s->d1->send_cookie = 0; - s->s3->tmp.reuse_message=1; + s->s3->tmp.reuse_message = 1; return(1); } diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 631ed8bdd..45bf9b460 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -683,27 +683,14 @@ int ssl3_client_hello(SSL *s) p=s->s3->client_random; - /* for DTLS if client_random is initialized, reuse it, we are - * required to use same upon reply to HelloVerify */ - if (SSL_IS_DTLS(s)) + /* If resending the ClientHello in DTLS after a + * HelloVerifyRequest, don't renegerate the client_random. The + * random must be reused. */ + if (!SSL_IS_DTLS(s) || !s->d1->send_cookie) { - size_t idx; - i = 1; - for (idx=0; idx < sizeof(s->s3->client_random); idx++) - { - if (p[idx]) - { - i = 0; - break; - } - } - } - else - i = 1; - - if (i) ssl_fill_hello_random(s, 0, p, sizeof(s->s3->client_random)); + } /* Do the message type and length last. * Note: the final argument to ssl_add_clienthello_tlsext below @@ -829,47 +816,16 @@ int ssl3_get_server_hello(SSL *s) CBS server_hello, server_random, session_id; uint16_t server_version, cipher_suite; uint8_t compression_method; - /* Hello verify request and/or server hello version may not - * match so set first packet if we're negotiating version. - */ - if (SSL_IS_DTLS(s)) - s->first_packet = 1; n=s->method->ssl_get_message(s, SSL3_ST_CR_SRVR_HELLO_A, SSL3_ST_CR_SRVR_HELLO_B, - -1, + SSL3_MT_SERVER_HELLO, 20000, /* ?? */ &ok); if (!ok) return((int)n); - if (SSL_IS_DTLS(s)) - { - s->first_packet = 0; - if ( s->s3->tmp.message_type == DTLS1_MT_HELLO_VERIFY_REQUEST) - { - if ( s->d1->send_cookie == 0) - { - s->s3->tmp.reuse_message = 1; - return 1; - } - else /* already sent a cookie */ - { - al=SSL_AD_UNEXPECTED_MESSAGE; - OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_BAD_MESSAGE_TYPE); - goto f_err; - } - } - } - - if ( s->s3->tmp.message_type != SSL3_MT_SERVER_HELLO) - { - al=SSL_AD_UNEXPECTED_MESSAGE; - OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_BAD_MESSAGE_TYPE); - goto f_err; - } - CBS_init(&server_hello, s->init_msg, n); if (!CBS_get_u16(&server_hello, &server_version) || diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 3a54eb242..04f7c6ca2 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -142,9 +142,17 @@ func (hs *serverHandshakeState) readClientHello() (isResume bool, err error) { if !bytes.Equal(newClientHello.cookie, helloVerifyRequest.cookie) { return false, errors.New("dtls: invalid cookie") } - // Apart from the cookie, client hello must match. - hs.clientHello.cookie = newClientHello.cookie - if hs.clientHello.equal(newClientHello) { + + // Apart from the cookie, the two ClientHellos must + // match. Note that clientHello.equal compares the + // serialization, so we make a copy. + oldClientHelloCopy := *hs.clientHello + oldClientHelloCopy.raw = nil + oldClientHelloCopy.cookie = nil + newClientHelloCopy := *newClientHello + newClientHelloCopy.raw = nil + newClientHelloCopy.cookie = nil + if !oldClientHelloCopy.equal(&newClientHelloCopy) { return false, errors.New("dtls: retransmitted ClientHello does not match") } hs.clientHello = newClientHello