From 0386aad1ab472a4059da85131cceca15aab5ebae Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 10 Jan 2017 14:58:17 +0000 Subject: [PATCH] Remove use of the SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS flag This flag is never set by anything so remove it. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2259) --- include/openssl/ssl3.h | 1 + ssl/record/rec_layer_d1.c | 4 +- ssl/record/rec_layer_s3.c | 15 ++++--- ssl/s3_lib.c | 3 -- ssl/statem/statem.c | 84 +++++++++++++++++++++------------------ ssl/statem/statem.h | 2 + ssl/statem/statem_srvr.c | 15 ++++--- 7 files changed, 68 insertions(+), 56 deletions(-) diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index c005440f82..8d146be19b 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -256,6 +256,7 @@ extern "C" { */ # define SSL3_CT_NUMBER 9 +/* No longer used as of OpenSSL 1.1.1 */ # define SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS 0x0001 /* Removed from OpenSSL 1.1.0 */ diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c index 5d971fb31b..0a32f07c2a 100644 --- a/ssl/record/rec_layer_d1.c +++ b/ssl/record/rec_layer_d1.c @@ -657,7 +657,6 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, s->msg_callback_arg); if (SSL_is_init_finished(s) && - !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) && !s->s3->renegotiate) { s->d1->handshake_read_seq++; s->new_session = 1; @@ -838,8 +837,7 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, goto start; } - if (SSL_is_init_finished(s) && - !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS)) { + if (SSL_is_init_finished(s)) { ossl_statem_set_in_init(s, 1); s->renegotiate = 1; s->new_session = 1; diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 5f37b0fa66..b4dee6e21d 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1390,6 +1390,7 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, /* If we are a client, check for an incoming 'Hello Request': */ if ((!s->server) && (s->rlayer.handshake_fragment_len >= 4) && + !SSL_IS_TLS13(s) && (s->rlayer.handshake_fragment[0] == SSL3_MT_HELLO_REQUEST) && (s->session != NULL) && (s->session->cipher != NULL)) { s->rlayer.handshake_fragment_len = 0; @@ -1408,7 +1409,6 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, s->msg_callback_arg); if (SSL_is_init_finished(s) && - !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) && !s->s3->renegotiate) { ssl3_renegotiate(s); if (ssl3_renegotiate_check(s)) { @@ -1459,6 +1459,7 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, SSL_is_init_finished(s) && !s->s3->send_connection_binding && (s->version > SSL3_VERSION) && + !SSL_IS_TLS13(s) && (s->rlayer.handshake_fragment_len >= 4) && (s->rlayer.handshake_fragment[0] == SSL3_MT_CLIENT_HELLO) && (s->session != NULL) && (s->session->cipher != NULL) && @@ -1557,15 +1558,17 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, } /* - * Unexpected handshake message (Client Hello, or protocol violation) + * Unexpected handshake message (Client Hello, NewSessionTicket (TLS1.3) or + * protocol violation) */ if ((s->rlayer.handshake_fragment_len >= 4) && !ossl_statem_get_in_handshake(s)) { - if (SSL_is_init_finished(s) && - !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS)) { + if (SSL_is_init_finished(s)) { ossl_statem_set_in_init(s, 1); - s->renegotiate = 1; - s->new_session = 1; + if (!SSL_IS_TLS13(s)) { + s->renegotiate = 1; + s->new_session = 1; + } } i = s->handshake_func(s); if (i < 0) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 4010985f23..ff4a03b147 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3874,9 +3874,6 @@ int ssl3_renegotiate(SSL *s) if (s->handshake_func == NULL) return (1); - if (s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) - return (0); - s->s3->renegotiate = 1; return (1); } diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c index 51a9266e42..ac78d2dc24 100644 --- a/ssl/statem/statem.c +++ b/ssl/statem/statem.c @@ -107,6 +107,7 @@ void ossl_statem_set_renegotiate(SSL *s) { s->statem.state = MSG_FLOW_RENEGOTIATE; s->statem.in_init = 1; + s->statem.request_state = TLS_ST_SW_HELLO_REQ; } /* @@ -259,9 +260,12 @@ static int state_machine(SSL *s, int server) s->ctx->stats.sess_connect_renegotiate++; } - if (st->state == MSG_FLOW_UNINITED || st->state == MSG_FLOW_RENEGOTIATE) { + if (st->state == MSG_FLOW_UNINITED + || st->state == MSG_FLOW_RENEGOTIATE + || st->state == MSG_FLOW_FINISHED) { if (st->state == MSG_FLOW_UNINITED) { st->hand_state = TLS_ST_BEFORE; + st->request_state = TLS_ST_BEFORE; } s->server = server; @@ -318,54 +322,57 @@ static int state_machine(SSL *s, int server) goto end; } - if (!server || st->state != MSG_FLOW_RENEGOTIATE) { - if (!ssl3_init_finished_mac(s)) { - ossl_statem_set_error(s); - goto end; + if (!SSL_IS_TLS13(s)) { + if (!server || st->state != MSG_FLOW_RENEGOTIATE) { + if (!ssl3_init_finished_mac(s)) { + ossl_statem_set_error(s); + goto end; + } } - } - if (server) { - if (st->state != MSG_FLOW_RENEGOTIATE) { - s->ctx->stats.sess_accept++; - } else if (!s->s3->send_connection_binding && - !(s->options & - SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) { - /* - * Server attempting to renegotiate with client that doesn't - * support secure renegotiation. - */ - SSLerr(SSL_F_STATE_MACHINE, - SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED); - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); - ossl_statem_set_error(s); - goto end; + if (server) { + if (st->state != MSG_FLOW_RENEGOTIATE) { + s->ctx->stats.sess_accept++; + } else if (!s->s3->send_connection_binding && + !(s->options & + SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) { + /* + * Server attempting to renegotiate with client that doesn't + * support secure renegotiation. + */ + SSLerr(SSL_F_STATE_MACHINE, + SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + ossl_statem_set_error(s); + goto end; + } else { + /* + * st->state == MSG_FLOW_RENEGOTIATE, we will just send a + * HelloRequest + */ + s->ctx->stats.sess_accept_renegotiate++; + + s->s3->tmp.cert_request = 0; + } } else { - /* - * st->state == MSG_FLOW_RENEGOTIATE, we will just send a - * HelloRequest - */ - s->ctx->stats.sess_accept_renegotiate++; - } - - s->s3->tmp.cert_request = 0; - } else { - s->ctx->stats.sess_connect++; + s->ctx->stats.sess_connect++; - /* mark client_random uninitialized */ - memset(s->s3->client_random, 0, sizeof(s->s3->client_random)); - s->hit = 0; + /* mark client_random uninitialized */ + memset(s->s3->client_random, 0, sizeof(s->s3->client_random)); + s->hit = 0; - s->s3->tmp.cert_req = 0; + s->s3->tmp.cert_req = 0; - if (SSL_IS_DTLS(s)) { - st->use_timer = 1; + if (SSL_IS_DTLS(s)) { + st->use_timer = 1; + } } + + st->read_state_first_init = 1; } st->state = MSG_FLOW_WRITING; init_write_state_machine(s); - st->read_state_first_init = 1; } while (st->state != MSG_FLOW_FINISHED) { @@ -396,7 +403,6 @@ static int state_machine(SSL *s, int server) } } - st->state = MSG_FLOW_UNINITED; ret = 1; end: diff --git a/ssl/statem/statem.h b/ssl/statem/statem.h index 2fca39b0db..6765c304a9 100644 --- a/ssl/statem/statem.h +++ b/ssl/statem/statem.h @@ -86,6 +86,8 @@ struct ossl_statem_st { READ_STATE read_state; WORK_STATE read_state_work; OSSL_HANDSHAKE_STATE hand_state; + /* The handshake state requested by an API call (e.g. HelloRequest) */ + OSSL_HANDSHAKE_STATE request_state; int in_init; int read_state_first_init; /* true when we are actually in SSL_accept() or SSL_connect() */ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 2e76b80b86..0a72287059 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -162,6 +162,7 @@ int ossl_statem_server_read_transition(SSL *s, int mt) break; case TLS_ST_BEFORE: + case TLS_ST_OK: case DTLS_ST_SW_HELLO_VERIFY_REQUEST: if (mt == SSL3_MT_CLIENT_HELLO) { st->hand_state = TLS_ST_SR_CLNT_HELLO; @@ -465,15 +466,19 @@ WRITE_TRAN ossl_statem_server_write_transition(SSL *s) /* Shouldn't happen */ return WRITE_TRAN_ERROR; + case TLS_ST_OK: + if (st->request_state == TLS_ST_SW_HELLO_REQ) { + /* We must be trying to renegotiate */ + st->hand_state = TLS_ST_SW_HELLO_REQ; + st->request_state = TLS_ST_BEFORE; + return WRITE_TRAN_CONTINUE; + } + /* Fall through */ + case TLS_ST_BEFORE: /* Just go straight to trying to read from the client */ return WRITE_TRAN_FINISHED; - case TLS_ST_OK: - /* We must be trying to renegotiate */ - st->hand_state = TLS_ST_SW_HELLO_REQ; - return WRITE_TRAN_CONTINUE; - case TLS_ST_SW_HELLO_REQ: st->hand_state = TLS_ST_OK; ossl_statem_set_in_init(s, 0); -- 2.25.1