From 754d2282cd50fef14971605d7151623bb11e3fd6 Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Tue, 23 May 2023 12:23:06 +0100 Subject: [PATCH] QUIC RX: Support reporting the key epoch a packet was received with This is needed to support key update validation on the receive side. Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/21029) --- include/internal/quic_record_rx.h | 6 +++ ssl/quic/quic_record_rx.c | 86 ++++++++++++++++++++++++++++--- test/quic_record_test.c | 22 ++++++++ 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/include/internal/quic_record_rx.h b/include/internal/quic_record_rx.h index f8527eaae6574..29755e2df1476 100644 --- a/include/internal/quic_record_rx.h +++ b/include/internal/quic_record_rx.h @@ -246,6 +246,12 @@ typedef struct ossl_qrx_pkt_st { /* The QRX which was used to receive the packet. */ OSSL_QRX *qrx; + + /* + * The key epoch the packet was received with. Always 0 for non-1-RTT + * packets. + */ + uint64_t key_epoch; } OSSL_QRX_PKT; /* diff --git a/ssl/quic/quic_record_rx.c b/ssl/quic/quic_record_rx.c index 037399701d697..774cc844597ec 100644 --- a/ssl/quic/quic_record_rx.c +++ b/ssl/quic/quic_record_rx.c @@ -61,6 +61,12 @@ struct rxe_st { /* Total length of the datagram which contained this packet. */ size_t datagram_len; + /* + * The key epoch the packet was received with. Always 0 for non-1-RTT + * packets. + */ + uint64_t key_epoch; + /* * alloc_len allocated bytes (of which data_len bytes are valid) follow this * structure. @@ -550,13 +556,21 @@ static int qrx_validate_hdr(OSSL_QRX *qrx, RXE *rxe) return 1; } -/* Retrieves the correct cipher context for an EL and key phase. */ +/* + * Retrieves the correct cipher context for an EL and key phase. Writes the key + * epoch number actually used for packet decryption to *rx_key_epoch. + */ static size_t qrx_get_cipher_ctx_idx(OSSL_QRX *qrx, OSSL_QRL_ENC_LEVEL *el, uint32_t enc_level, - unsigned char key_phase_bit) + unsigned char key_phase_bit, + uint64_t *rx_key_epoch) { - if (enc_level != QUIC_ENC_LEVEL_1RTT) + size_t idx; + + if (enc_level != QUIC_ENC_LEVEL_1RTT) { + *rx_key_epoch = 0; return 0; + } if (!ossl_assert(key_phase_bit <= 1)) return SIZE_MAX; @@ -584,8 +598,54 @@ static size_t qrx_get_cipher_ctx_idx(OSSL_QRX *qrx, OSSL_QRL_ENC_LEVEL *el, * the best we can reasonably do and appears to be directly suggested by the * RFC. */ - return el->state == QRL_EL_STATE_PROV_COOLDOWN ? el->key_epoch & 1 - : key_phase_bit; + idx = (el->state == QRL_EL_STATE_PROV_COOLDOWN ? el->key_epoch & 1 + : key_phase_bit); + + /* + * We also need to determine the key epoch number which this index + * corresponds to. This is so we can report the key epoch number in the + * OSSL_QRX_PKT structure, which callers need to validate whether it was OK + * for a packet to be sent using a given key epoch's keys. + */ + switch (el->state) { + case QRL_EL_STATE_PROV_NORMAL: + /* + * If we are in the NORMAL state, usually the KP bit will match the LSB + * of our key epoch, meaning no new key update is being signalled. If it + * does not match, this means the packet (purports to) belong to + * the next key epoch. + * + * IMPORTANT: The AEAD tag has not been verified yet when this function + * is called, so this code must be timing-channel safe, hence use of + * XOR. Moreover, the value output below is not yet authenticated. + */ + *rx_key_epoch + = el->key_epoch + ((el->key_epoch & 1) ^ (uint64_t)key_phase_bit); + break; + + case QRL_EL_STATE_PROV_UPDATING: + /* + * If we are in the UPDATING state, usually the KP bit will match the + * LSB of our key epoch. If it does not match, this means that the + * packet (purports to) belong to the previous key epoch. + * + * As above, must be timing-channel safe. + */ + *rx_key_epoch + = el->key_epoch - ((el->key_epoch & 1) ^ (uint64_t)key_phase_bit); + break; + + case QRL_EL_STATE_PROV_COOLDOWN: + /* + * If we are in COOLDOWN, there is only one key epoch we can possibly + * decrypt with, so just try that. If AEAD decryption fails, the + * value we output here isn't used anyway. + */ + *rx_key_epoch = el->key_epoch; + break; + } + + return idx; } /* @@ -602,7 +662,8 @@ static int qrx_decrypt_pkt_body(OSSL_QRX *qrx, unsigned char *dst, size_t src_len, size_t *dec_len, const unsigned char *aad, size_t aad_len, QUIC_PN pn, uint32_t enc_level, - unsigned char key_phase_bit) + unsigned char key_phase_bit, + uint64_t *rx_key_epoch) { int l = 0, l2 = 0; unsigned char nonce[EVP_MAX_IV_LENGTH]; @@ -628,7 +689,8 @@ static int qrx_decrypt_pkt_body(OSSL_QRX *qrx, unsigned char *dst, if (qrx->forged_pkt_count >= ossl_qrl_get_suite_max_forged_pkt(el->suite_id)) return 0; - cctx_idx = qrx_get_cipher_ctx_idx(qrx, el, enc_level, key_phase_bit); + cctx_idx = qrx_get_cipher_ctx_idx(qrx, el, enc_level, key_phase_bit, + rx_key_epoch); if (!ossl_assert(cctx_idx < OSSL_NELEM(el->cctx))) return 0; @@ -704,6 +766,7 @@ static int qrx_process_pkt(OSSL_QRX *qrx, QUIC_URXE *urxe, QUIC_PKT_HDR_PTRS ptrs; uint32_t pn_space, enc_level; OSSL_QRL_ENC_LEVEL *el = NULL; + uint64_t rx_key_epoch = UINT64_MAX; /* * Get a free RXE. If we need to allocate a new one, use the packet length @@ -884,10 +947,15 @@ static int qrx_process_pkt(OSSL_QRX *qrx, QUIC_URXE *urxe, dst = (unsigned char *)rxe_data(rxe) + i; if (!qrx_decrypt_pkt_body(qrx, dst, rxe->hdr.data, rxe->hdr.len, &dec_len, sop, aad_len, rxe->pn, enc_level, - rxe->hdr.key_phase)) + rxe->hdr.key_phase, &rx_key_epoch)) goto malformed; /* + * ----------------------------------------------------- + * IMPORTANT: ANYTHING ABOVE THIS LINE IS UNVERIFIED + * AND MUST BE TIMING-CHANNEL SAFE. + * ----------------------------------------------------- + * * At this point, we have successfully authenticated the AEAD tag and no * longer need to worry about exposing the Key Phase bit in timing channels. * Check for a Key Phase bit differing from our expectation. @@ -921,6 +989,7 @@ static int qrx_process_pkt(OSSL_QRX *qrx, QUIC_URXE *urxe, rxe->hdr.len = dec_len; rxe->data_len = dec_len; rxe->datagram_len = datagram_len; + rxe->key_epoch = rx_key_epoch; /* We processed the PN successfully, so update largest processed PN. */ pn_space = rxe_determine_pn_space(rxe); @@ -1108,6 +1177,7 @@ int ossl_qrx_read_pkt(OSSL_QRX *qrx, OSSL_QRX_PKT **ppkt) = BIO_ADDR_family(&rxe->peer) != AF_UNSPEC ? &rxe->peer : NULL; rxe->pkt.local = BIO_ADDR_family(&rxe->local) != AF_UNSPEC ? &rxe->local : NULL; + rxe->pkt.key_epoch = rxe->key_epoch; rxe->pkt.qrx = qrx; *ppkt = &rxe->pkt; diff --git a/test/quic_record_test.c b/test/quic_record_test.c index 088e730af1fb9..b4826a7730098 100644 --- a/test/quic_record_test.c +++ b/test/quic_record_test.c @@ -31,6 +31,7 @@ static const QUIC_CONN_ID empty_conn_id = {0, {0}}; #define RX_TEST_OP_CHECK_KEY_EPOCH 10 /* check key epoch value matches */ #define RX_TEST_OP_KEY_UPDATE_TIMEOUT 11 /* complete key update process */ #define RX_TEST_OP_SET_INIT_KEY_PHASE 12 /* initial Key Phase bit value */ +#define RX_TEST_OP_CHECK_PKT_EPOCH 13 /* check read key epoch matches */ struct rx_test_op { unsigned char op; @@ -78,6 +79,8 @@ struct rx_test_op { { RX_TEST_OP_KEY_UPDATE_TIMEOUT, 0, NULL, 0, NULL, (normal), 0, 0, NULL }, #define RX_OP_SET_INIT_KEY_PHASE(kp_bit) \ { RX_TEST_OP_SET_INIT_KEY_PHASE, 0, NULL, 0, NULL, (kp_bit), 0, 0, NULL }, +#define RX_OP_CHECK_PKT_EPOCH(expected) \ + { RX_TEST_OP_CHECK_PKT_EPOCH, 0, NULL, 0, NULL, 0, 0, (expected), NULL }, #define RX_OP_INJECT_N(n) \ RX_OP_INJECT(rx_script_##n##_in) @@ -1567,6 +1570,7 @@ static const struct rx_test_op rx_script_8[] = { RX_OP_CHECK_PKT_N(8a) RX_OP_CHECK_NO_PKT() RX_OP_CHECK_KEY_EPOCH(0) + RX_OP_CHECK_PKT_EPOCH(0) /* Packet with new key phase */ RX_OP_INJECT_N(8b) @@ -1575,6 +1579,7 @@ static const struct rx_test_op rx_script_8[] = { RX_OP_CHECK_NO_PKT() /* Key epoch has increased */ RX_OP_CHECK_KEY_EPOCH(1) + RX_OP_CHECK_PKT_EPOCH(1) /* * Now inject an old packet with the old keys (perhaps reordered in @@ -1586,18 +1591,21 @@ static const struct rx_test_op rx_script_8[] = { RX_OP_CHECK_NO_PKT() /* Epoch has not changed */ RX_OP_CHECK_KEY_EPOCH(1) + RX_OP_CHECK_PKT_EPOCH(0) /* Another packet with the new keys. */ RX_OP_INJECT_N(8d) RX_OP_CHECK_PKT_N(8d) RX_OP_CHECK_NO_PKT() RX_OP_CHECK_KEY_EPOCH(1) + RX_OP_CHECK_PKT_EPOCH(1) /* We can inject the old packet multiple times and it still works */ RX_OP_INJECT_N(8c) RX_OP_CHECK_PKT_N(8c) RX_OP_CHECK_NO_PKT() RX_OP_CHECK_KEY_EPOCH(1) + RX_OP_CHECK_PKT_EPOCH(0) /* Until we move from UPDATING to COOLDOWN */ RX_OP_KEY_UPDATE_TIMEOUT(0) @@ -1619,12 +1627,14 @@ static const struct rx_test_op rx_script_8[] = { RX_OP_CHECK_PKT_N(8e) RX_OP_CHECK_NO_PKT() RX_OP_CHECK_KEY_EPOCH(2) + RX_OP_CHECK_PKT_EPOCH(2) /* Can still receive old packet */ RX_OP_INJECT_N(8d) RX_OP_CHECK_PKT_N(8d) RX_OP_CHECK_NO_PKT() RX_OP_CHECK_KEY_EPOCH(2) + RX_OP_CHECK_PKT_EPOCH(1) /* Move straight from UPDATING to NORMAL */ RX_OP_KEY_UPDATE_TIMEOUT(1) @@ -1634,6 +1644,7 @@ static const struct rx_test_op rx_script_8[] = { RX_OP_CHECK_PKT_N(8f) RX_OP_CHECK_NO_PKT() RX_OP_CHECK_KEY_EPOCH(3) + RX_OP_CHECK_PKT_EPOCH(3) RX_OP_END }; @@ -1726,6 +1737,7 @@ static int rx_run_script(const struct rx_test_op *script) size_t i; OSSL_QRX_PKT *pkt = NULL; const struct rx_test_op *op = script; + uint64_t last_key_epoch = UINT64_MAX; for (; op->op != RX_TEST_OP_END; ++op) switch (op->op) { @@ -1793,6 +1805,8 @@ static int rx_run_script(const struct rx_test_op *script) op->buf, op->buf_len, 1))) goto err; + last_key_epoch = pkt->key_epoch; + ossl_qrx_pkt_release(pkt); pkt = NULL; break; @@ -1812,6 +1826,14 @@ static int rx_run_script(const struct rx_test_op *script) op->largest_pn)) goto err; + break; + case RX_TEST_OP_CHECK_PKT_EPOCH: + if (!TEST_true(rx_state_ensure(&s))) + goto err; + + if (!TEST_uint64_t_eq(last_key_epoch, op->largest_pn)) + goto err; + break; case RX_TEST_OP_KEY_UPDATE_TIMEOUT: if (!TEST_true(rx_state_ensure(&s)))