If a ticket key callback returns 0 in TLSv1.3 don't send a ticket

If we can't construct the ticket don't send one. This requires a change
to the TLS state machine to be able to a handle a construction function
deciding not to send a message after all.

Fixes #18977

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18990)
This commit is contained in:
Matt Caswell 2022-08-12 13:24:19 +01:00 committed by Hugo Landau
parent 3b7a3241c2
commit 3e93c5fe1e
2 changed files with 64 additions and 22 deletions

View File

@ -888,10 +888,24 @@ static SUB_STATE_RETURN write_state_machine(SSL_CONNECTION *s)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return SUB_STATE_ERROR;
}
if (confunc != NULL && !confunc(s, &pkt)) {
WPACKET_cleanup(&pkt);
check_fatal(s);
return SUB_STATE_ERROR;
if (confunc != NULL) {
int tmpret;
tmpret = confunc(s, &pkt);
if (tmpret <= 0) {
WPACKET_cleanup(&pkt);
check_fatal(s);
return SUB_STATE_ERROR;
} else if (tmpret == 2) {
/*
* The construction function decided not to construct the
* message after all and continue. Skip sending.
*/
WPACKET_cleanup(&pkt);
st->write_state = WRITE_STATE_POST_WORK;
st->write_state_work = WORK_MORE_A;
break;
} /* else success */
}
if (!ssl_close_construct_packet(s, &pkt, mt)
|| !WPACKET_finish(&pkt)) {

View File

@ -3696,6 +3696,10 @@ static int create_ticket_prequel(SSL_CONNECTION *s, WPACKET *pkt,
return 1;
}
/*
* Returns 1 on success, 0 to abort construction of the ticket (non-fatal), or
* -1 on fatal error
*/
static int construct_stateless_ticket(SSL_CONNECTION *s, WPACKET *pkt,
uint32_t age_add,
unsigned char *tick_nonce)
@ -3711,7 +3715,7 @@ static int construct_stateless_ticket(SSL_CONNECTION *s, WPACKET *pkt,
SSL_CTX *tctx = s->session_ctx;
unsigned char iv[EVP_MAX_IV_LENGTH];
unsigned char key_name[TLSEXT_KEYNAME_LENGTH];
int iv_len, ok = 0;
int iv_len, ok = -1;
size_t macoffset, macendoffset;
SSL *ssl = SSL_CONNECTION_GET_SSL(s);
SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s);
@ -3794,7 +3798,15 @@ static int construct_stateless_ticket(SSL_CONNECTION *s, WPACKET *pkt,
#endif
if (ret == 0) {
/*
* In TLSv1.2 we construct a 0 length ticket. In TLSv1.3 a 0
* length ticket is not allowed so we abort construction of the
* ticket
*/
if (SSL_CONNECTION_IS_TLS13(s)) {
ok = 0;
goto err;
}
/* Put timeout and length */
if (!WPACKET_put_bytes_u32(pkt, 0)
|| !WPACKET_put_bytes_u16(pkt, 0)) {
@ -3908,6 +3920,20 @@ static int construct_stateful_ticket(SSL_CONNECTION *s, WPACKET *pkt,
return 1;
}
static void tls_update_ticket_counts(SSL_CONNECTION *s)
{
/*
* Increment both |sent_tickets| and |next_ticket_nonce|. |sent_tickets|
* gets reset to 0 if we send more tickets following a post-handshake
* auth, but |next_ticket_nonce| does not. If we're sending extra
* tickets, decrement the count of pending extra tickets.
*/
s->sent_tickets++;
s->next_ticket_nonce++;
if (s->ext.extra_tickets_expected > 0)
s->ext.extra_tickets_expected--;
}
int tls_construct_new_session_ticket(SSL_CONNECTION *s, WPACKET *pkt)
{
SSL_CTX *tctx = s->session_ctx;
@ -3916,6 +3942,7 @@ int tls_construct_new_session_ticket(SSL_CONNECTION *s, WPACKET *pkt)
unsigned char age_add_c[sizeof(uint32_t)];
uint32_t age_add;
} age_add_u;
int ret = 0;
age_add_u.age_add = 0;
@ -4014,10 +4041,20 @@ int tls_construct_new_session_ticket(SSL_CONNECTION *s, WPACKET *pkt)
/* SSLfatal() already called */
goto err;
}
} else if (!construct_stateless_ticket(s, pkt, age_add_u.age_add,
tick_nonce)) {
/* SSLfatal() already called */
goto err;
} else {
int tmpret;
tmpret = construct_stateless_ticket(s, pkt, age_add_u.age_add,
tick_nonce);
if (tmpret != 1) {
if (tmpret == 0) {
ret = 2; /* Non-fatal. Abort construction but continue */
/* We count this as a success so update the counts anwyay */
tls_update_ticket_counts(s);
}
/* else SSLfatal() already called */
goto err;
}
}
if (SSL_CONNECTION_IS_TLS13(s)) {
@ -4027,22 +4064,13 @@ int tls_construct_new_session_ticket(SSL_CONNECTION *s, WPACKET *pkt)
/* SSLfatal() already called */
goto err;
}
/*
* Increment both |sent_tickets| and |next_ticket_nonce|. |sent_tickets|
* gets reset to 0 if we send more tickets following a post-handshake
* auth, but |next_ticket_nonce| does not. If we're sending extra
* tickets, decrement the count of pending extra tickets.
*/
s->sent_tickets++;
s->next_ticket_nonce++;
if (s->ext.extra_tickets_expected > 0)
s->ext.extra_tickets_expected--;
tls_update_ticket_counts(s);
ssl_update_cache(s, SSL_SESS_CACHE_SERVER);
}
return 1;
ret = 1;
err:
return 0;
return ret;
}
/*