Skip to content

Commit

Permalink
QUIC TLS: Ensure QUIC_TLS is ticked between each processed RX packet
Browse files Browse the repository at this point in the history 
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#22476)
  • Loading branch information
@hlandau @mattcaswell
hlandau authored and mattcaswell committed Oct 25, 2023
1 parent 3860ef2 commit 82b7a0e
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 36 deletions.
67 changes: 38 additions & 29 deletions ssl/quic/quic_channel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@

static void ch_save_err_state(QUIC_CHANNEL *ch);
static void ch_rx_pre(QUIC_CHANNEL *ch);
static int ch_rx(QUIC_CHANNEL *ch);
static int ch_rx(QUIC_CHANNEL *ch, int channel_only);
static int ch_tx(QUIC_CHANNEL *ch);
static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags);
static void ch_rx_handle_packet(QUIC_CHANNEL *ch);
static int ch_tick_tls(QUIC_CHANNEL *ch, int channel_only);
static void ch_rx_handle_packet(QUIC_CHANNEL *ch, int channel_only);
static OSSL_TIME ch_determine_next_tick_deadline(QUIC_CHANNEL *ch);
static int ch_retry(QUIC_CHANNEL *ch,
const unsigned char *retry_token,
Expand Down Expand Up @@ -1850,9 +1851,6 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
OSSL_TIME now, deadline;
QUIC_CHANNEL *ch = arg;
int channel_only = (flags & QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY) != 0;
uint64_t error_code;
const char *error_msg;
ERR_STATE *error_state = NULL;

/*
* When we tick the QUIC connection, we do everything we need to do
Expand Down Expand Up @@ -1898,21 +1896,16 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)

do {
/* Process queued incoming packets. */
ch_rx(ch);
ch->did_tls_tick = 0;
ch->have_new_rx_secret = 0;
ch_rx(ch, channel_only);

/*
* Allow the handshake layer to check for any new incoming data and
* generate new outgoing data.
*/
ch->have_new_rx_secret = 0;
if (!channel_only) {
ossl_quic_tls_tick(ch->qtls);

if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg,
&error_state))
ossl_quic_channel_raise_protocol_error_state(ch, error_code, 0,
error_msg, error_state);
}
if (!ch->did_tls_tick)
ch_tick_tls(ch, channel_only);

/*
* If the handshake layer gave us a new secret, we need to do RX
Expand Down Expand Up @@ -1983,6 +1976,28 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
&& ossl_qtx_get_queue_len_datagrams(ch->qtx) > 0);
}

static int ch_tick_tls(QUIC_CHANNEL *ch, int channel_only)
{
uint64_t error_code;
const char *error_msg;
ERR_STATE *error_state = NULL;

if (channel_only)
return 1;

ch->did_tls_tick = 1;
ossl_quic_tls_tick(ch->qtls);

if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg,
&error_state)) {
ossl_quic_channel_raise_protocol_error_state(ch, error_code, 0,
error_msg, error_state);
return 0;
}

return 1;
}

/* Process incoming datagrams, if any. */
static void ch_rx_pre(QUIC_CHANNEL *ch)
{
Expand Down Expand Up @@ -2042,7 +2057,7 @@ static void ch_rx_check_forged_pkt_limit(QUIC_CHANNEL *ch)
}

/* Process queued incoming packets and handle frames, if any. */
static int ch_rx(QUIC_CHANNEL *ch)
static int ch_rx(QUIC_CHANNEL *ch, int channel_only)
{
int handled_any = 0;
const int closing = ossl_quic_channel_is_closing(ch);
Expand Down Expand Up @@ -2070,7 +2085,7 @@ static int ch_rx(QUIC_CHANNEL *ch)
ch_update_ping_deadline(ch);
}

ch_rx_handle_packet(ch); /* best effort */
ch_rx_handle_packet(ch, channel_only); /* best effort */

/*
* Regardless of the outcome of frame handling, unref the packet.
Expand Down Expand Up @@ -2122,7 +2137,7 @@ static int bio_addr_eq(const BIO_ADDR *a, const BIO_ADDR *b)
}

/* Handles the packet currently in ch->qrx_pkt->hdr. */
static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
static void ch_rx_handle_packet(QUIC_CHANNEL *ch, int channel_only)
{
uint32_t enc_level;
int old_have_processed_any_pkt = ch->have_processed_any_pkt;
Expand Down Expand Up @@ -2323,6 +2338,10 @@ static void ch_rx_handle_packet(QUIC_CHANNEL *ch)

/* This packet contains frames, pass to the RXDP. */
ossl_quic_handle_frames(ch, ch->qrx_pkt); /* best effort */

if (ch->did_crypto_frame)
ch_tick_tls(ch, channel_only);

break;

case QUIC_PKT_TYPE_VERSION_NEG:
Expand Down Expand Up @@ -2726,10 +2745,6 @@ int ossl_quic_channel_set_net_wbio(QUIC_CHANNEL *ch, BIO *net_wbio)
*/
int ossl_quic_channel_start(QUIC_CHANNEL *ch)
{
uint64_t error_code;
const char *error_msg;
ERR_STATE *error_state = NULL;

if (ch->is_server)
/*
* This is not used by the server. The server moves to active
Expand Down Expand Up @@ -2758,14 +2773,8 @@ int ossl_quic_channel_start(QUIC_CHANNEL *ch)
ch->doing_proactive_ver_neg = 0; /* not currently supported */

/* Handshake layer: start (e.g. send CH). */
ossl_quic_tls_tick(ch->qtls);

if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg,
&error_state)) {
ossl_quic_channel_raise_protocol_error_state(ch, error_code, 0,
error_msg, error_state);
if (!ch_tick_tls(ch, /*channel_only=*/0))
return 0;
}

ossl_quic_reactor_tick(&ch->rtor, 0); /* best effort */
return 1;
Expand Down
5 changes: 5 additions & 0 deletions ssl/quic/quic_channel_local.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,11 @@ struct quic_channel_st {
*/
unsigned int have_new_rx_secret : 1;

/* Have we ever called QUIC_TLS yet during RX processing? */
unsigned int did_tls_tick : 1;
/* Has any CRYPTO frame been processed during this tick? */
unsigned int did_crypto_frame : 1;

/*
* Have we sent an ack-eliciting packet since the last successful packet
* reception? Used to determine when to bump idle timer (see RFC 9000 s.
Expand Down
3 changes: 3 additions & 0 deletions ssl/quic/quic_rx_depack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ static int depack_do_frame_crypto(PACKET *pkt, QUIC_CHANNEL *ch,
return 0;
}

ch->did_crypto_frame = 1;
*datalen = f.len;

return 1;
Expand Down Expand Up @@ -1422,6 +1423,8 @@ int ossl_quic_handle_frames(QUIC_CHANNEL *ch, OSSL_QRX_PKT *qpacket)
if (ch == NULL)
goto end;

ch->did_crypto_frame = 0;

/* Initialize |ackm_data| (and reinitialize |ok|)*/
memset(&ackm_data, 0, sizeof(ackm_data));
/*
Expand Down
17 changes: 10 additions & 7 deletions test/quicapitest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1057,15 +1057,15 @@ static int non_io_retry_cert_verify_cb(X509_STORE_CTX *ctx, void *arg)
{
int idx = SSL_get_ex_data_X509_STORE_CTX_idx();
SSL *ssl;
int *ctr = (int *)arg;
const int *allow = (int *)arg;

/* this should not happen but check anyway */
if (idx < 0
|| (ssl = X509_STORE_CTX_get_ex_data(ctx, idx)) == NULL)
return 0;

/* If this is the first time we've been called then retry */
if (((*ctr)++) == 0)
/* If this is our first attempt then retry */
if (*allow == 0)
return SSL_set_retry_verify(ssl);

/* Otherwise do nothing - verification succeeds. Continue as normal */
Expand All @@ -1082,7 +1082,7 @@ static int test_non_io_retry(int idx)
SSL *clientquic = NULL;
QUIC_TSERVER *qtserv = NULL;
int testresult = 0;
int flags = 0, ctr = 0;
int flags = 0, allow = 0;

if (idx >= 1 && !qtest_supports_blocking())
return TEST_skip("Blocking tests not supported in this build");
Expand All @@ -1091,16 +1091,19 @@ static int test_non_io_retry(int idx)
if (!TEST_ptr(cctx))
goto err;

SSL_CTX_set_cert_verify_callback(cctx, non_io_retry_cert_verify_cb, &ctr);
SSL_CTX_set_cert_verify_callback(cctx, non_io_retry_cert_verify_cb, &allow);

flags = (idx >= 1) ? QTEST_FLAG_BLOCK : 0;
if (!TEST_true(qtest_create_quic_objects(libctx, cctx, NULL, cert, privkey,
flags, &qtserv, &clientquic, NULL,
NULL))
|| !TEST_true(qtest_create_quic_connection_ex(qtserv, clientquic,
SSL_ERROR_WANT_RETRY_VERIFY))
|| !TEST_int_eq(SSL_want(clientquic), SSL_RETRY_VERIFY)
|| !TEST_true(qtest_create_quic_connection(qtserv, clientquic)))
|| !TEST_int_eq(SSL_want(clientquic), SSL_RETRY_VERIFY))
goto err;

allow = 1;
if (!TEST_true(qtest_create_quic_connection(qtserv, clientquic)))
goto err;

testresult = 1;
Expand Down

0 comments on commit 82b7a0e

Please sign in to comment.