diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 96f00cfb..5c5fa612 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -366,6 +366,10 @@ typedef struct ssl3_state_st { * the version has not been negotiated yet. */ char have_version; + /* initial_handshake_complete is true if the initial handshake has + * completed. */ + char initial_handshake_complete; + /* sniff_buffer is used by the server in the initial handshake to read a * V2ClientHello before the record layer is initialized. */ BUF_MEM *sniff_buffer; diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c index 1827a675..a5a7b0c7 100644 --- a/ssl/d1_clnt.c +++ b/ssl/d1_clnt.c @@ -474,6 +474,7 @@ int dtls1_connect(SSL *s) { s->init_num = 0; s->renegotiate = 0; s->new_session = 0; + s->s3->initial_handshake_complete = 1; ssl_update_cache(s, SSL_SESS_CACHE_CLIENT); diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c index e314910f..d93b26ff 100644 --- a/ssl/d1_srvr.c +++ b/ssl/d1_srvr.c @@ -475,6 +475,7 @@ int dtls1_accept(SSL *s) { /* skipped if we just sent a HelloRequest */ s->renegotiate = 0; s->new_session = 0; + s->s3->initial_handshake_complete = 1; ssl_update_cache(s, SSL_SESS_CACHE_SERVER); diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index d01acaed..700c9386 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -457,7 +457,7 @@ int ssl3_connect(SSL *s) { ssl3_can_false_start(s) && /* No False Start on renegotiation (would complicate the state * machine). */ - s->s3->previous_server_finished_len == 0) { + !s->s3->initial_handshake_complete) { s->s3->tmp.next_state = SSL3_ST_FALSE_START; } else { /* Allow NewSessionTicket if ticket expected */ @@ -554,6 +554,7 @@ int ssl3_connect(SSL *s) { s->renegotiate = 0; s->new_session = 0; s->s3->tmp.in_false_start = 0; + s->s3->initial_handshake_complete = 1; ssl_update_cache(s, SSL_SESS_CACHE_CLIENT); @@ -778,6 +779,7 @@ int ssl3_get_server_hello(SSL *s) { goto f_err; } + assert(s->s3->have_version == s->s3->initial_handshake_complete); if (!s->s3->have_version) { if (!ssl3_is_version_enabled(s, server_version)) { OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_UNSUPPORTED_PROTOCOL); diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 3cc30328..3e1e7c2c 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -644,6 +644,7 @@ int ssl3_accept(SSL *s) { /* skipped if we just sent a HelloRequest */ s->renegotiate = 0; s->new_session = 0; + s->s3->initial_handshake_complete = 1; ssl_update_cache(s, SSL_SESS_CACHE_SERVER); @@ -1011,6 +1012,11 @@ int ssl3_get_client_hello(SSL *s) { } } + /* Note: This codepath may run twice if |ssl_get_prev_session| completes + * asynchronously. + * + * TODO(davidben): Clean up the order of events around ClientHello + * processing. */ if (!s->s3->have_version) { /* Select version to use */ uint16_t version = ssl3_get_mutual_version(s, client_version); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 433a6471..4830fd58 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -871,7 +871,7 @@ uint8_t *ssl_add_clienthello_tlsext(SSL *s, uint8_t *buf, uint8_t *limit, } /* Add RI if renegotiating */ - if (s->renegotiate) { + if (s->s3->initial_handshake_complete) { int el; if (!ssl_add_clienthello_renegotiate_ext(s, 0, &el, 0)) { @@ -954,7 +954,7 @@ uint8_t *ssl_add_clienthello_tlsext(SSL *s, uint8_t *buf, uint8_t *limit, s2n(0, ret); } - if (s->ctx->next_proto_select_cb && !s->s3->tmp.finish_md_len && + if (s->ctx->next_proto_select_cb && !s->s3->initial_handshake_complete && !SSL_IS_DTLS(s)) { /* The client advertises an emtpy extension to indicate its support for * Next Protocol Negotiation */ @@ -965,7 +965,7 @@ uint8_t *ssl_add_clienthello_tlsext(SSL *s, uint8_t *buf, uint8_t *limit, s2n(0, ret); } - if (s->signed_cert_timestamps_enabled && !s->s3->tmp.finish_md_len) { + if (s->signed_cert_timestamps_enabled) { /* The client advertises an empty extension to indicate its support for * certificate timestamps. */ if (limit - ret - 4 < 0) { @@ -975,7 +975,7 @@ uint8_t *ssl_add_clienthello_tlsext(SSL *s, uint8_t *buf, uint8_t *limit, s2n(0, ret); } - if (s->alpn_client_proto_list && !s->s3->tmp.finish_md_len) { + if (s->alpn_client_proto_list && !s->s3->initial_handshake_complete) { if ((size_t)(limit - ret) < 6 + s->alpn_client_proto_list_len) { return NULL; } @@ -1584,29 +1584,16 @@ static int ssl_scan_clienthello_tlsext(SSL *s, CBS *cbs, int *out_alert) { return 0; } } else if (type == TLSEXT_TYPE_next_proto_neg && - s->s3->tmp.finish_md_len == 0 && s->s3->alpn_selected == NULL && - !SSL_IS_DTLS(s)) { + !s->s3->initial_handshake_complete && + s->s3->alpn_selected == NULL && !SSL_IS_DTLS(s)) { /* The extension must be empty. */ if (CBS_len(&extension) != 0) { *out_alert = SSL_AD_DECODE_ERROR; return 0; } - - /* We shouldn't accept this extension on a renegotiation. - * - * s->new_session will be set on renegotiation, but we probably shouldn't - * rely that it couldn't be set on the initial renegotation too in - * certain cases (when there's some other reason to disallow resuming an - * earlier session -- the current code won't be doing anything like that, - * but this might change). - - * A valid sign that there's been a previous handshake in this connection - * is if s->s3->tmp.finish_md_len > 0. (We are talking about a check - * that will happen in the Hello protocol round, well before a new - * Finished message could have been computed.) */ s->s3->next_proto_neg_seen = 1; } else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation && - s->ctx->alpn_select_cb && s->s3->tmp.finish_md_len == 0) { + s->ctx->alpn_select_cb && !s->s3->initial_handshake_complete) { if (!tls1_alpn_handle_client_hello(s, &extension, out_alert)) { return 0; } @@ -1788,8 +1775,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, CBS *cbs, int *out_alert) { /* Set a flag to expect a CertificateStatus message */ s->s3->tmp.certificate_status_expected = 1; } else if (type == TLSEXT_TYPE_next_proto_neg && - s->s3->tmp.finish_md_len == 0 && - !SSL_IS_DTLS(s)) { + !s->s3->initial_handshake_complete && !SSL_IS_DTLS(s)) { uint8_t *selected; uint8_t selected_len; @@ -1821,7 +1807,8 @@ static int ssl_scan_serverhello_tlsext(SSL *s, CBS *cbs, int *out_alert) { s->next_proto_negotiated_len = selected_len; s->s3->next_proto_neg_seen = 1; - } else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation) { + } else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation && + !s->s3->initial_handshake_complete) { CBS protocol_name_list, protocol_name; /* We must have requested it. */