Skip to content

Commit

Permalink
Do DTLS13 and TLS13 connection version check in one macro
Browse files Browse the repository at this point in the history 
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #22366)
  • Loading branch information
@fwh-dc @mattcaswell
fwh-dc authored and mattcaswell committed Apr 23, 2024
1 parent 2496f91 commit 1bd689a
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 88 deletions.
6 changes: 5 additions & 1 deletion ssl/ssl_local.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,12 @@
&& SSL_CONNECTION_GET_SSL(s)->method->version >= TLS1_3_VERSION \
&& SSL_CONNECTION_GET_SSL(s)->method->version != TLS_ANY_VERSION)

/* Check if we are using (D)TLSv1.3 */
# define SSL_CONNECTION_IS_VERSION13(s) \
(SSL_CONNECTION_IS_DTLS13(s) || SSL_CONNECTION_IS_TLS13(s))

# define SSL_CONNECTION_TREAT_AS_TLS13(s) \
((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) \
(SSL_CONNECTION_IS_VERSION13(s) \
|| (s)->early_data_state == SSL_EARLY_DATA_CONNECTING \
|| (s)->early_data_state == SSL_EARLY_DATA_CONNECT_RETRY \
|| (s)->early_data_state == SSL_EARLY_DATA_WRITING \
Expand Down
2 changes: 1 addition & 1 deletion ssl/statem/statem.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ static int state_machine(SSL_CONNECTION *s, int server)

s->server = server;
if (cb != NULL) {
if (SSL_IS_FIRST_HANDSHAKE(s) || !(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
if (SSL_IS_FIRST_HANDSHAKE(s) || !SSL_CONNECTION_IS_VERSION13(s))
cb(ssl, SSL_CB_HANDSHAKE_START, 1);
}

Expand Down
58 changes: 29 additions & 29 deletions ssl/statem/statem_clnt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ int ossl_statem_client_read_transition(SSL_CONNECTION *s, int mt)
* Note that after writing the first ClientHello we don't know what version
* we are going to negotiate yet, so we don't take this branch until later.
*/
if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
if (!ossl_statem_client13_read_transition(s, mt))
goto err;
return 1;
Expand Down Expand Up @@ -547,7 +547,7 @@ WRITE_TRAN ossl_statem_client_write_transition(SSL_CONNECTION *s)
* version we are going to negotiate yet, so we don't take this branch until
* later
*/
if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))
if (SSL_CONNECTION_IS_VERSION13(s))
return ossl_statem_client13_write_transition(s);

switch (st->hand_state) {
Expand Down Expand Up @@ -836,7 +836,7 @@ WORK_STATE ossl_statem_client_post_work(SSL_CONNECTION *s, WORK_STATE wst)
break;

case TLS_ST_CW_CHANGE:
if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)
if (SSL_CONNECTION_IS_VERSION13(s)
|| s->hello_retry_request == SSL_HRR_PENDING)
break;
if (s->early_data_state == SSL_EARLY_DATA_CONNECTING
Expand Down Expand Up @@ -897,7 +897,7 @@ WORK_STATE ossl_statem_client_post_work(SSL_CONNECTION *s, WORK_STATE wst)
if (statem_flush(s) != 1)
return WORK_MORE_B;

if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
if (!tls13_save_handshake_digest_for_pha(s)) {
/* SSLfatal() already called */
return WORK_ERROR;
Expand Down Expand Up @@ -1058,7 +1058,7 @@ size_t ossl_statem_client_max_message_size(SSL_CONNECTION *s)
return CCS_MAX_LENGTH;

case TLS_ST_CR_SESSION_TICKET:
return (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) ? SESSION_TICKET_MAX_LENGTH_TLS13
return SSL_CONNECTION_IS_VERSION13(s) ? SESSION_TICKET_MAX_LENGTH_TLS13
: SESSION_TICKET_MAX_LENGTH_TLS12;

case TLS_ST_CR_FINISHED:
Expand Down Expand Up @@ -1408,7 +1408,7 @@ static int set_client_ciphersuite(SSL_CONNECTION *s,
return 0;
}

if ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) && s->s3.tmp.new_cipher != NULL
if (SSL_CONNECTION_IS_VERSION13(s) && s->s3.tmp.new_cipher != NULL
&& s->s3.tmp.new_cipher->id != c->id) {
/* ServerHello selected a different ciphersuite to that in the HRR */
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_WRONG_CIPHER_RETURNED);
Expand All @@ -1423,7 +1423,7 @@ static int set_client_ciphersuite(SSL_CONNECTION *s,
if (s->session->cipher != NULL)
s->session->cipher_id = s->session->cipher->id;
if (s->hit && (s->session->cipher_id != c->id)) {
if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
const EVP_MD *md = ssl_md(sctx, c->algorithm2);

if (!ossl_assert(s->session->cipher != NULL)) {
Expand Down Expand Up @@ -1548,7 +1548,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
}
}

if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s) || hrr) {
if (SSL_CONNECTION_IS_VERSION13(s) || hrr) {
if (compression != 0) {
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
SSL_R_INVALID_COMPRESSION_ALGORITHM);
Expand Down Expand Up @@ -1576,7 +1576,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
* Now we have chosen the version we need to check again that the extensions
* are appropriate for this version.
*/
context = (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) ? SSL_EXT_TLS1_3_SERVER_HELLO
context = SSL_CONNECTION_IS_VERSION13(s) ? SSL_EXT_TLS1_3_SERVER_HELLO
: SSL_EXT_TLS1_2_SERVER_HELLO;
if (!tls_validate_all_contexts(s, context, extensions)) {
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_EXTENSION);
Expand All @@ -1585,7 +1585,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)

s->hit = 0;

if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
/*
* In TLSv1.3 a ServerHello message signals a key change so the end of
* the message must be on a record boundary.
Expand Down Expand Up @@ -1678,7 +1678,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
* echo of what we originally sent in the ClientHello and should not be
* used for resumption.
*/
if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))) {
if (!SSL_CONNECTION_IS_VERSION13(s)) {
s->session->session_id_length = session_id_len;
/* session_id_len could be 0 */
if (session_id_len > 0)
Expand Down Expand Up @@ -1785,7 +1785,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
* In TLSv1.3 we have some post-processing to change cipher state, otherwise
* we're done with this message
*/
if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
if (!ssl->method->ssl3_enc->setup_key_block(s)
|| !ssl->method->ssl3_enc->change_cipher_state(s,
SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
Expand Down Expand Up @@ -1936,7 +1936,7 @@ static WORK_STATE tls_post_process_server_rpk(SSL_CONNECTION *sc,
* skip check since TLS 1.3 ciphersuites can be used with any certificate
* type.
*/
if (!(SSL_CONNECTION_IS_TLS13(sc) || SSL_CONNECTION_IS_DTLS13(sc))) {
if (!SSL_CONNECTION_IS_VERSION13(sc)) {
if ((clu->amask & sc->s3.tmp.new_cipher->algorithm_auth) == 0) {
SSLfatal(sc, SSL_AD_ILLEGAL_PARAMETER, SSL_R_WRONG_RPK_TYPE);
return WORK_ERROR;
Expand All @@ -1951,7 +1951,7 @@ static WORK_STATE tls_post_process_server_rpk(SSL_CONNECTION *sc,
sc->session->verify_result = sc->verify_result;

/* Save the current hash state for when we receive the CertificateVerify */
if ((SSL_CONNECTION_IS_TLS13(sc) || SSL_CONNECTION_IS_DTLS13(sc))
if (SSL_CONNECTION_IS_VERSION13(sc)
&& !ssl_handshake_hash(sc, sc->cert_verify_hash,
sizeof(sc->cert_verify_hash),
&sc->cert_verify_hash_len)) {
Expand Down Expand Up @@ -1986,7 +1986,7 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL_CONNECTION *s,
goto err;
}

if (((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) && !PACKET_get_1(pkt, &context))
if ((SSL_CONNECTION_IS_VERSION13(s) && !PACKET_get_1(pkt, &context))
|| context != 0
|| !PACKET_get_net_3(pkt, &cert_list_len)
|| PACKET_remaining(pkt) != cert_list_len
Expand Down Expand Up @@ -2018,7 +2018,7 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL_CONNECTION *s,
goto err;
}

if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
RAW_EXTENSION *rawexts = NULL;
PACKET extensions;

Expand Down Expand Up @@ -2122,7 +2122,7 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s,
* skip check since TLS 1.3 ciphersuites can be used with any certificate
* type.
*/
if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))) {
if (!SSL_CONNECTION_IS_VERSION13(s)) {
if ((clu->amask & s->s3.tmp.new_cipher->algorithm_auth) == 0) {
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_WRONG_CERTIFICATE_TYPE);
return WORK_ERROR;
Expand All @@ -2138,7 +2138,7 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s,
s->session->peer_rpk = NULL;

/* Save the current hash state for when we receive the CertificateVerify */
if ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))
if (SSL_CONNECTION_IS_VERSION13(s)
&& !ssl_handshake_hash(s, s->cert_verify_hash,
sizeof(s->cert_verify_hash),
&s->cert_verify_hash_len)) {
Expand Down Expand Up @@ -2572,7 +2572,7 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL_CONNECTION *s,
if (s->s3.tmp.valid_flags == NULL)
return 0;

if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
PACKET reqctx, extensions;
RAW_EXTENSION *rawexts = NULL;

Expand Down Expand Up @@ -2677,7 +2677,7 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL_CONNECTION *s,
* SSL_get1_peer_certificate() returns something sensible in
* client_cert_cb.
*/
if ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))
if (SSL_CONNECTION_IS_VERSION13(s)
&& s->post_handshake_auth != SSL_PHA_REQUESTED)
return MSG_PROCESS_CONTINUE_READING;

Expand All @@ -2698,11 +2698,11 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL_CONNECTION *s,
PACKET_null_init(&nonce);

if (!PACKET_get_net_4(pkt, &ticket_lifetime_hint)
|| ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))
|| (SSL_CONNECTION_IS_VERSION13(s)
&& (!PACKET_get_net_4(pkt, &age_add)
|| !PACKET_get_length_prefixed_1(pkt, &nonce)))
|| !PACKET_get_net_2(pkt, &ticklen)
|| ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) ? (ticklen == 0
|| (SSL_CONNECTION_IS_VERSION13(s) ? (ticklen == 0
|| PACKET_remaining(pkt) < ticklen)
: PACKET_remaining(pkt) != ticklen)) {
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_LENGTH_MISMATCH);
Expand All @@ -2725,7 +2725,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL_CONNECTION *s,
* post-handshake and the session may have already gone into the session
* cache.
*/
if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s) || s->session->session_id_length > 0) {
if (SSL_CONNECTION_IS_VERSION13(s) || s->session->session_id_length > 0) {
SSL_SESSION *new_sess;

/*
Expand All @@ -2738,7 +2738,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL_CONNECTION *s,
}

if ((s->session_ctx->session_cache_mode & SSL_SESS_CACHE_CLIENT) != 0
&& !(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))) {
&& !SSL_CONNECTION_IS_VERSION13(s)) {
/*
* In TLSv1.2 and below the arrival of a new tickets signals that
* any old ticket we were using is now out of date, so we remove the
Expand Down Expand Up @@ -2772,7 +2772,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL_CONNECTION *s,
s->session->ext.tick_age_add = age_add;
s->session->ext.ticklen = ticklen;

if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
PACKET extpkt;

if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
Expand Down Expand Up @@ -2825,7 +2825,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL_CONNECTION *s,
s->session->not_resumable = 0;

/* This is a standalone message in TLSv1.3, so there is no more to read */
if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
const EVP_MD *md = ssl_handshake_md(s);
int hashleni = EVP_MD_get_size(md);
size_t hashlen;
Expand Down Expand Up @@ -3741,7 +3741,7 @@ WORK_STATE tls_prepare_client_certificate(SSL_CONNECTION *s, WORK_STATE wst)
}
}

if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))
if (!SSL_CONNECTION_IS_VERSION13(s)
|| (s->options & SSL_OP_NO_TX_CERTIFICATE_COMPRESSION) != 0)
s->ext.compress_certificate_from_peer[0] = TLSEXT_comp_cert_none;

Expand All @@ -3761,7 +3761,7 @@ CON_FUNC_RETURN tls_construct_client_certificate(SSL_CONNECTION *s,
CERT_PKEY *cpk = NULL;
SSL *ssl = SSL_CONNECTION_GET_SSL(s);

if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
if (SSL_CONNECTION_IS_VERSION13(s)) {
if (s->pha_context == NULL) {
/* no context available, add 0-length context */
if (!WPACKET_put_bytes_u8(pkt, 0)) {
Expand Down Expand Up @@ -3798,7 +3798,7 @@ CON_FUNC_RETURN tls_construct_client_certificate(SSL_CONNECTION *s,
* then we deferred changing the handshake write keys to the last possible
* moment. We need to do it now.
*/
if ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))
if (SSL_CONNECTION_IS_VERSION13(s)
&& SSL_IS_FIRST_HANDSHAKE(s)
&& (s->early_data_state != SSL_EARLY_DATA_NONE
|| (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
Expand Down

0 comments on commit 1bd689a

Please sign in to comment.