Update state machine to send CCS based on whether we did an HRR

The CCS may be sent at different times based on whether or not we
sent an HRR earlier. In order to make that decision this commit
also updates things to make sure we remember whether an HRR was
used or not.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
This commit is contained in:
Matt Caswell 2017-11-13 11:24:51 +00:00
parent 6f40214f68
commit fc7129dc37
8 changed files with 51 additions and 36 deletions

View File

@ -276,7 +276,7 @@ int ssl3_get_record(SSL *s)
* that explicitly
*/
if (!s->first_packet && !SSL_IS_TLS13(s)
&& !s->hello_retry_request
&& s->hello_retry_request != SSL_HRR_PENDING
&& version != (unsigned int)s->version) {
if ((s->version & 0xFF00) == (version & 0xFF00)
&& !s->enc_write_ctx && !s->write_hash) {
@ -449,7 +449,7 @@ int ssl3_get_record(SSL *s)
if (num_recs == 1
&& thisrr->type == SSL3_RT_CHANGE_CIPHER_SPEC
&& SSL_IS_TLS13(s)
&& (SSL_IS_TLS13(s) || s->hello_retry_request != SSL_HRR_NONE)
&& SSL_IS_FIRST_HANDSHAKE(s)) {
/*
* CCS messages must be exactly 1 byte long, containing the value 0x01

View File

@ -1117,7 +1117,8 @@ struct ssl_st {
size_t cert_verify_hash_len;
/* Flag to indicate whether we should send a HelloRetryRequest or not */
int hello_retry_request;
enum {SSL_HRR_NONE = 0, SSL_HRR_PENDING, SSL_HRR_COMPLETE}
hello_retry_request;
/*
* the session_id_context is used to ensure sessions are only reused in

View File

@ -1235,7 +1235,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent)
*/
if (s->server && s->s3->peer_tmp == NULL) {
/* No suitable share */
if (s->hello_retry_request == 0 && sent
if (s->hello_retry_request == SSL_HRR_NONE && sent
&& (!s->hit
|| (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE)
!= 0)) {
@ -1260,7 +1260,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent)
if (i < num_groups) {
/* A shared group exists so send a HelloRetryRequest */
s->s3->group_id = group_id;
s->hello_retry_request = 1;
s->hello_retry_request = SSL_HRR_PENDING;
return 1;
}
}
@ -1275,8 +1275,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent)
}
/* We have a key_share so don't send any more HelloRetryRequest messages */
if (s->server)
s->hello_retry_request = 0;
if (s->server && s->hello_retry_request == SSL_HRR_PENDING)
s->hello_retry_request = SSL_HRR_COMPLETE;
/*
* For a client side resumption with no key_share we need to generate
@ -1405,7 +1405,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
* following a HelloRetryRequest then this includes the hash of the first
* ClientHello and the HelloRetryRequest itself.
*/
if (s->hello_retry_request) {
if (s->hello_retry_request == SSL_HRR_PENDING) {
size_t hdatalen;
void *hdata;
@ -1516,7 +1516,7 @@ static int final_early_data(SSL *s, unsigned int context, int sent)
|| s->session->ext.tick_identity != 0
|| s->early_data_state != SSL_EARLY_DATA_ACCEPTING
|| !s->ext.early_data_ok
|| s->hello_retry_request) {
|| s->hello_retry_request != SSL_HRR_NONE) {
s->ext.early_data = SSL_EARLY_DATA_REJECTED;
} else {
s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;

View File

@ -599,7 +599,7 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
size_t encodedlen;
if (s->s3->tmp.pkey != NULL) {
if (!ossl_assert(s->hello_retry_request)) {
if (!ossl_assert(s->hello_retry_request == SSL_HRR_PENDING)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_ADD_KEY_SHARE,
ERR_R_INTERNAL_ERROR);
return 0;
@ -749,7 +749,7 @@ EXT_RETURN tls_construct_ctos_early_data(SSL *s, WPACKET *pkt,
SSL_SESSION *edsess = NULL;
const EVP_MD *handmd = NULL;
if (s->hello_retry_request)
if (s->hello_retry_request == SSL_HRR_PENDING)
handmd = ssl_handshake_md(s);
if (s->psk_use_session_cb != NULL
@ -961,7 +961,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|| (s->session->ext.ticklen == 0 && s->psksession == NULL))
return EXT_RETURN_NOT_SENT;
if (s->hello_retry_request)
if (s->hello_retry_request == SSL_HRR_PENDING)
handmd = ssl_handshake_md(s);
if (s->session->ext.ticklen != 0) {
@ -980,7 +980,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
goto dopsksess;
}
if (s->hello_retry_request && mdres != handmd) {
if (s->hello_retry_request == SSL_HRR_PENDING && mdres != handmd) {
/*
* Selected ciphersuite hash does not match the hash for the session
* so we can't use it.
@ -1044,7 +1044,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
return EXT_RETURN_FAIL;
}
if (s->hello_retry_request && mdpsk != handmd) {
if (s->hello_retry_request == SSL_HRR_PENDING && mdpsk != handmd) {
/*
* Selected ciphersuite hash does not match the hash for the PSK
* session. This is an application bug.

View File

@ -704,7 +704,7 @@ int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context,
return 0;
}
if (s->hello_retry_request) {
if (s->hello_retry_request != SSL_HRR_NONE) {
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
SSL_F_TLS_PARSE_CTOS_EARLY_DATA, SSL_R_BAD_EXTENSION);
return 0;
@ -1245,7 +1245,7 @@ EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt,
if (ckey == NULL) {
/* No key_share received from client */
if (s->hello_retry_request) {
if (s->hello_retry_request == SSL_HRR_PENDING) {
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
|| !WPACKET_start_sub_packet_u16(pkt)
|| !WPACKET_put_bytes_u16(pkt, s->s3->group_id)

View File

@ -387,7 +387,7 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
|| s->early_data_state == SSL_EARLY_DATA_FINISHED_WRITING)
st->hand_state = TLS_ST_PENDING_EARLY_DATA_END;
else if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0
&& !s->hello_retry_request)
&& s->hello_retry_request == SSL_HRR_NONE)
st->hand_state = TLS_ST_CW_CHANGE;
else
st->hand_state = (s->s3->tmp.cert_req != 0) ? TLS_ST_CW_CERT
@ -1055,7 +1055,8 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
if (sess == NULL
|| !ssl_version_supported(s, sess->ssl_version)
|| !SSL_SESSION_is_resumable(sess)) {
if (!s->hello_retry_request && !ssl_get_new_session(s, 0)) {
if (s->hello_retry_request == SSL_HRR_NONE
&& !ssl_get_new_session(s, 0)) {
/* SSLfatal() already called */
return 0;
}
@ -1078,7 +1079,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
}
}
} else {
i = s->hello_retry_request == 0;
i = (s->hello_retry_request == SSL_HRR_NONE);
}
if (i && ssl_fill_hello_random(s, 0, p, sizeof(s->s3->client_random),
@ -1136,7 +1137,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
sess_id_len = sizeof(s->tmp_session_id);
s->tmp_session_id_len = sess_id_len;
session_id = s->tmp_session_id;
if (!s->hello_retry_request
if (s->hello_retry_request == SSL_HRR_NONE
&& ssl_randbytes(s, s->tmp_session_id,
sess_id_len) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR,
@ -1360,7 +1361,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
&& sversion == TLS1_2_VERSION
&& PACKET_remaining(pkt) >= SSL3_RANDOM_SIZE
&& memcmp(hrrrandom, PACKET_data(pkt), SSL3_RANDOM_SIZE) == 0) {
s->hello_retry_request = hrr = 1;
s->hello_retry_request = SSL_HRR_PENDING;
hrr = 1;
if (!PACKET_forward(pkt, SSL3_RANDOM_SIZE)) {
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
SSL_R_LENGTH_MISMATCH);

View File

@ -1656,7 +1656,7 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
suppversions = &hello->pre_proc_exts[TLSEXT_IDX_supported_versions];
/* If we did an HRR then supported versions is mandatory */
if (!suppversions->present && s->hello_retry_request)
if (!suppversions->present && s->hello_retry_request != SSL_HRR_NONE)
return SSL_R_UNSUPPORTED_PROTOCOL;
if (suppversions->present && !SSL_IS_DTLS(s)) {
@ -1703,7 +1703,7 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
}
if (best_vers > 0) {
if (s->hello_retry_request) {
if (s->hello_retry_request != SSL_HRR_NONE) {
/*
* This is after a HelloRetryRequest so we better check that we
* negotiated TLSv1.3
@ -1779,7 +1779,8 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
return 0;
}
if (s->hello_retry_request && s->version != TLS1_3_VERSION) {
if (s->hello_retry_request != SSL_HRR_NONE
&& s->version != TLS1_3_VERSION) {
s->version = origv;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_WRONG_SSL_VERSION);

View File

@ -48,7 +48,7 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
break;
case TLS_ST_EARLY_DATA:
if (s->hello_retry_request) {
if (s->hello_retry_request == SSL_HRR_PENDING) {
if (mt == SSL3_MT_CLIENT_HELLO) {
st->hand_state = TLS_ST_SR_CLNT_HELLO;
return 1;
@ -395,16 +395,20 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
return WRITE_TRAN_CONTINUE;
case TLS_ST_SW_SRVR_HELLO:
if (s->hello_retry_request)
st->hand_state = TLS_ST_EARLY_DATA;
else if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0
&& s->hello_retry_request != SSL_HRR_COMPLETE)
st->hand_state = TLS_ST_SW_CHANGE;
else if (s->hello_retry_request == SSL_HRR_PENDING)
st->hand_state = TLS_ST_EARLY_DATA;
else
st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
return WRITE_TRAN_CONTINUE;
case TLS_ST_SW_CHANGE:
st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
if (s->hello_retry_request == SSL_HRR_PENDING)
st->hand_state = TLS_ST_EARLY_DATA;
else
st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
return WRITE_TRAN_CONTINUE;
case TLS_ST_SW_ENCRYPTED_EXTENSIONS:
@ -664,6 +668,8 @@ WORK_STATE ossl_statem_server_pre_work(SSL *s, WORK_STATE wst)
break;
case TLS_ST_SW_CHANGE:
if (SSL_IS_TLS13(s))
break;
s->session->cipher = s->s3->tmp.new_cipher;
if (!s->method->ssl3_enc->setup_key_block(s)) {
/* SSLfatal() already called */
@ -733,7 +739,7 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
break;
case TLS_ST_SW_SRVR_HELLO:
if (SSL_IS_TLS13(s) && s->hello_retry_request) {
if (SSL_IS_TLS13(s) && s->hello_retry_request == SSL_HRR_PENDING) {
if (statem_flush(s) != 1)
return WORK_MORE_A;
break;
@ -765,11 +771,14 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
}
#endif
if (!SSL_IS_TLS13(s)
|| (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
|| ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0
&& s->hello_retry_request != SSL_HRR_COMPLETE))
break;
/* Fall through */
case TLS_ST_SW_CHANGE:
if (s->hello_retry_request == SSL_HRR_PENDING)
break;
/*
* TODO(TLS1.3): This actually causes a problem. We don't yet know
* whether the next record we are going to receive is an unencrypted
@ -1267,7 +1276,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
if (clienthello->isv2) {
unsigned int mt;
if (!SSL_IS_FIRST_HANDSHAKE(s) || s->hello_retry_request) {
if (!SSL_IS_FIRST_HANDSHAKE(s)
|| s->hello_retry_request != SSL_HRR_NONE) {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_UNEXPECTED_MESSAGE);
goto err;
@ -1624,7 +1634,7 @@ static int tls_early_post_process_client_hello(SSL *s)
SSL_R_NO_SHARED_CIPHER);
goto err;
}
if (s->hello_retry_request
if (s->hello_retry_request == SSL_HRR_PENDING
&& (s->s3->tmp.new_cipher == NULL
|| s->s3->tmp.new_cipher->id != cipher->id)) {
/*
@ -2200,7 +2210,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
size_t sl, len;
int version;
unsigned char *session_id;
int usetls13 = SSL_IS_TLS13(s) || s->hello_retry_request;
int usetls13 = SSL_IS_TLS13(s) || s->hello_retry_request == SSL_HRR_PENDING;
version = usetls13 ? TLS1_2_VERSION : s->version;
if (!WPACKET_put_bytes_u16(pkt, version)
@ -2209,7 +2219,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
* tls_process_client_hello()
*/
|| !WPACKET_memcpy(pkt,
s->hello_retry_request
s->hello_retry_request == SSL_HRR_PENDING
? hrrrandom : s->s3->server_random,
SSL3_RANDOM_SIZE)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_SERVER_HELLO,
@ -2269,6 +2279,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
|| !WPACKET_put_bytes_u8(pkt, compm)
|| !tls_construct_extensions(s, pkt,
s->hello_retry_request
== SSL_HRR_PENDING
? SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
: (SSL_IS_TLS13(s)
? SSL_EXT_TLS1_3_SERVER_HELLO
@ -2278,7 +2289,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
return 0;
}
if (s->hello_retry_request) {
if (s->hello_retry_request == SSL_HRR_PENDING) {
/* Ditch the session. We'll create a new one next time around */
SSL_SESSION_free(s->session);
s->session = NULL;