From dae820bffc3d3d84d5caada9d3502301e3c6a00f Mon Sep 17 00:00:00 2001 From: Ruiqi Zhou Date: Wed, 30 Mar 2022 16:36:52 +0800 Subject: [PATCH] [!] Minor fixes (#124) --- CONTRIBUTING.md | 4 +++ README.md | 46 +++++++++++++++--------------- demo/xqc_hq_conn.c | 11 ++++--- include/xquic/xqc_http3.h | 7 +++++ include/xquic/xquic.h | 14 ++++++--- src/congestion_control/xqc_cubic.c | 20 +++++++++++++ src/congestion_control/xqc_cubic.h | 1 + src/http3/xqc_h3_conn.c | 20 +++++++------ src/http3/xqc_h3_request.c | 2 +- src/http3/xqc_h3_stream.c | 4 +-- src/tls/boringssl/xqc_ssl_if.c | 30 ++++--------------- src/transport/xqc_client.c | 2 +- src/transport/xqc_conn.c | 21 +++++++++++--- src/transport/xqc_conn.h | 3 +- src/transport/xqc_packet_parser.c | 10 ++++--- src/transport/xqc_send_ctl.c | 2 +- tests/test_client.c | 12 ++++---- tests/test_server.c | 8 +++--- 18 files changed, 129 insertions(+), 88 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5738b863e..dcfbdf11b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -208,5 +208,9 @@ In no particular order, thanks to these excellent individuals who contributed co * @flx413 * @robinhzp * @contrun +* @eltociear +* @happyomg +* @driventokill +* @chenzhanfeng This list will be continuously updated. Contributions are welcome! diff --git a/README.md b/README.md index 5aa326306..113d5b789 100644 --- a/README.md +++ b/README.md @@ -34,52 +34,52 @@ To run test cases, you need XQUIC supports both BabaSSL and BoringSSL. -### Build with BabaSSL +### Build with BoringSSL ```bash # get XQUIC source code git clone git@github.com:alibaba/xquic.git cd xquic -# get and build BabaSSL -git clone git@github.com:BabaSSL/BabaSSL.git ./third_party/babassl -cd ./third_party/babassl/ -./config --prefix=/usr/local/babassl -make -j -SSL_TYPE_STR="babassl" +# get and build BoringSSL +git clone git@github.com:google/boringssl.git ./third_party/boringssl +cd ./third_party/boringssl +mkdir -p build && cd build +cmake -DBUILD_SHARED_LIBS=0 -DCMAKE_C_FLAGS="-fPIC" -DCMAKE_CXX_FLAGS="-fPIC" .. +make ssl crypto +cd .. +SSL_TYPE_STR="boringssl" SSL_PATH_STR="${PWD}" SSL_INC_PATH_STR="${PWD}/include" -SSL_LIB_PATH_STR="${PWD}/libssl.a;${PWD}/libcrypto.a" -cd - +SSL_LIB_PATH_STR="${PWD}/build/ssl/libssl.a;${PWD}/build/crypto/libcrypto.a" +cd ../.. -# build XQUIC with BabaSSL +# build XQUIC with BoringSSL git submodule update --init --recursive mkdir -p build; cd build cmake -DGCOV=on -DCMAKE_BUILD_TYPE=Debug -DXQC_ENABLE_TESTING=1 -DXQC_SUPPORT_SENDMMSG_BUILD=1 -DXQC_ENABLE_EVENT_LOG=1 -DXQC_ENABLE_BBR2=1 -DXQC_DISABLE_RENO=0 -DSSL_TYPE=${SSL_TYPE_STR} -DSSL_PATH=${SSL_PATH_STR} -DSSL_INC_PATH=${SSL_INC_PATH_STR} -DSSL_LIB_PATH=${SSL_LIB_PATH_STR} .. make -j ``` -### Build with BoringSSL +### Build with BabaSSL ```bash # get XQUIC source code git clone git@github.com:alibaba/xquic.git cd xquic -# get and build BoringSSL -git clone git@github.com:google/boringssl.git ./third_party/boringssl -cd ./third_party/boringssl -mkdir -p build && cd build -cmake -DBUILD_SHARED_LIBS=0 -DCMAKE_C_FLAGS="-fPIC" -DCMAKE_CXX_FLAGS="-fPIC" .. -make ssl crypto -cd .. -SSL_TYPE_STR="boringssl" +# get and build BabaSSL +git clone git@github.com:BabaSSL/BabaSSL.git ./third_party/babassl +cd ./third_party/babassl/ +./config --prefix=/usr/local/babassl +make -j +SSL_TYPE_STR="babassl" SSL_PATH_STR="${PWD}" SSL_INC_PATH_STR="${PWD}/include" -SSL_LIB_PATH_STR="${PWD}/build/ssl/libssl.a;${PWD}/build/crypto/libcrypto.a" -cd ../.. +SSL_LIB_PATH_STR="${PWD}/libssl.a;${PWD}/libcrypto.a" +cd - -# build XQUIC with BoringSSL +# build XQUIC with BabaSSL git submodule update --init --recursive mkdir -p build; cd build cmake -DGCOV=on -DCMAKE_BUILD_TYPE=Debug -DXQC_ENABLE_TESTING=1 -DXQC_SUPPORT_SENDMMSG_BUILD=1 -DXQC_ENABLE_EVENT_LOG=1 -DXQC_ENABLE_BBR2=1 -DXQC_DISABLE_RENO=0 -DSSL_TYPE=${SSL_TYPE_STR} -DSSL_PATH=${SSL_PATH_STR} -DSSL_INC_PATH=${SSL_INC_PATH_STR} -DSSL_LIB_PATH=${SSL_LIB_PATH_STR} .. @@ -99,7 +99,7 @@ sh ../scripts/xquic_test.sh * For Chinese Simplified (zh-CN) translation of the IETF QUIC Protocol, see the Translation docs. - The following translation is based on draft-34 and RFC Translation is Working In Progress. - [draft-ietf-quic-invariants-13-zh](./docs/translation/draft-ietf-quic-invariants-13-zh.md) - - [draft-ietf-quic-transport-34-zh](./docs/translation/draft-ietf-quic-transport-34-zh.md) + - [RFC9000-transport-zh](./docs/translation/rfc9000-transport-zh.md) - [draft-ietf-quic-recovery-34-zh](./docs/translation/draft-ietf-quic-recovery-34-zh.md) - [draft-ietf-quic-tls-34-zh](./docs/translation/draft-ietf-quic-tls-34-zh.md) - [draft-ietf-quic-http-34-zh](./docs/translation/draft-ietf-quic-http-34-zh.md) diff --git a/demo/xqc_hq_conn.c b/demo/xqc_hq_conn.c index ef49ddf3a..035cfc231 100644 --- a/demo/xqc_hq_conn.c +++ b/demo/xqc_hq_conn.c @@ -115,7 +115,8 @@ xqc_hq_conn_get_peer_addr(xqc_hq_conn_t *hqc, struct sockaddr *addr, socklen_t a } xqc_int_t -xqc_hq_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *conn_user_data) +xqc_hq_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, + void *conn_user_data, void *conn_proto_data) { /* here conn_user_data is the app-layer user_data */ xqc_hq_conn_t *hqc = xqc_hq_conn_create(conn, cid, conn_user_data); @@ -136,11 +137,12 @@ xqc_hq_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *co } xqc_int_t -xqc_hq_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *conn_user_data) +xqc_hq_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, + void *conn_user_data, void *conn_proto_data) { xqc_int_t ret = XQC_OK; - xqc_hq_conn_t *hqc = (xqc_hq_conn_t *)conn_user_data; + xqc_hq_conn_t *hqc = (xqc_hq_conn_t *)conn_proto_data; if (hqc->hqc_cbs->conn_close_notify) { ret = hqc->hqc_cbs->conn_close_notify(hqc, cid, hqc->user_data); if (ret != XQC_OK) { @@ -155,7 +157,8 @@ xqc_hq_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *con void -xqc_hq_conn_handshake_finished(xqc_connection_t *conn, void *conn_user_data) +xqc_hq_conn_handshake_finished(xqc_connection_t *conn, void *conn_user_data, + void *conn_proto_data) { return; } diff --git a/include/xquic/xqc_http3.h b/include/xquic/xqc_http3.h index 82613a017..bea8cf6bc 100644 --- a/include/xquic/xqc_http3.h +++ b/include/xquic/xqc_http3.h @@ -499,6 +499,13 @@ void *xqc_h3_get_conn_user_data_by_request(xqc_h3_request_t *h3_request); XQC_EXPORT_PUBLIC_API xqc_stream_id_t xqc_h3_stream_id(xqc_h3_request_t *h3_request); +/** + * @brief Get QUIC Transport connection handler + */ +XQC_EXPORT_PUBLIC_API +xqc_connection_t *xqc_h3_conn_get_xqc_conn(xqc_h3_conn_t *h3_conn); + + #ifdef __cplusplus } #endif diff --git a/include/xquic/xquic.h b/include/xquic/xquic.h index 5038e8eae..235588df9 100644 --- a/include/xquic/xquic.h +++ b/include/xquic/xquic.h @@ -164,9 +164,14 @@ typedef ssize_t (*xqc_stateless_reset_pt)(const unsigned char *buf, size_t size, /** * @brief general callback function definition for connection create and close + * + * @param conn_user_data the user_data which will be used in callback functions + * between xquic transport connection and application + * @param conn_proto_data the user_data which will be used in callback functions + * between xquic transport connection and application-layer-protocol */ typedef int (*xqc_conn_notify_pt)(xqc_connection_t *conn, const xqc_cid_t *cid, - void *conn_user_data); + void *conn_user_data, void *conn_proto_data); /** * @brief QUIC connection token callback. REQUIRED for client. @@ -209,7 +214,8 @@ typedef xqc_save_string_pt xqc_save_trans_param_pt; * this will be trigger when the QUIC connection handshake is completed, that is, when the TLS * stack has both sent a Finished message and verified the peer's Finished message */ -typedef void (*xqc_handshake_finished_pt)(xqc_connection_t *conn, void *conn_user_data); +typedef void (*xqc_handshake_finished_pt)(xqc_connection_t *conn, void *conn_user_data, + void *conn_proto_data); /** * @brief PING acked callback function. @@ -220,7 +226,7 @@ typedef void (*xqc_handshake_finished_pt)(xqc_connection_t *conn, void *conn_use * xquic might send PING frames will not trigger this callback */ typedef void (*xqc_conn_ping_ack_notify_pt)(xqc_connection_t *conn, const xqc_cid_t *cid, - void *ping_user_data, void *conn_user_data); + void *ping_user_data, void *conn_user_data, void *conn_proto_data); /** * @brief cid update callback function. @@ -984,7 +990,7 @@ void xqc_conn_set_transport_user_data(xqc_connection_t *conn, void *user_data); * xqc_conn_callbacks_t */ XQC_EXPORT_PUBLIC_API -void xqc_conn_set_alp_user_data(xqc_connection_t *conn, void *app_proto_user_data); +void xqc_conn_set_alp_user_data(xqc_connection_t *conn, void *proto_data); /** diff --git a/src/congestion_control/xqc_cubic.c b/src/congestion_control/xqc_cubic.c index 8d0793b9a..2e1d11b2c 100644 --- a/src/congestion_control/xqc_cubic.c +++ b/src/congestion_control/xqc_cubic.c @@ -100,6 +100,14 @@ xqc_cubic_update(void *cong_ctl, uint32_t acked_bytes, xqc_usec_t now) cubic->cwnd = bic_target; } +/* https://datatracker.ietf.org/doc/html/rfc9002#appendix-B.5 */ +static int +xqc_cubic_in_congestion_recovery(void *cong_ctl, xqc_usec_t sent_time) +{ + xqc_cubic_t *cubic = (xqc_cubic_t*)(cong_ctl); + return sent_time <= cubic->congestion_recovery_start_time; +} + size_t xqc_cubic_size() { @@ -115,6 +123,7 @@ xqc_cubic_init(void *cong_ctl, xqc_send_ctl_t *ctl_ctx, xqc_cc_params_t cc_param cubic->tcp_cwnd = XQC_CUBIC_INIT_WIN; cubic->last_max_cwnd = XQC_CUBIC_INIT_WIN; cubic->ssthresh = XQC_CUBIC_MAX_SSTHRESH; + cubic->congestion_recovery_start_time = 0; if (cc_params.customize_on) { cc_params.init_cwnd *= XQC_CUBIC_MSS; @@ -130,6 +139,12 @@ xqc_cubic_on_lost(void *cong_ctl, xqc_usec_t lost_sent_time) { xqc_cubic_t *cubic = (xqc_cubic_t*)(cong_ctl); + /* No reaction if already in a recovery period. */ + if (xqc_cubic_in_congestion_recovery(cong_ctl, lost_sent_time)) { + return; + } + + cubic->congestion_recovery_start_time = xqc_monotonic_timestamp(); cubic->epoch_start = 0; /* should we make room for others */ @@ -162,6 +177,11 @@ xqc_cubic_on_ack(void *cong_ctl, xqc_packet_out_t *po, xqc_usec_t now) cubic->min_rtt = rtt; } + /* Do not increase congestion window in recovery period. */ + if (xqc_cubic_in_congestion_recovery(cong_ctl, po->po_sent_time)) { + return; + } + if (cubic->cwnd < cubic->ssthresh) { /* slow start */ cubic->tcp_cwnd += acked_bytes; diff --git a/src/congestion_control/xqc_cubic.h b/src/congestion_control/xqc_cubic.h index 0a54afdeb..cb24d7ca2 100644 --- a/src/congestion_control/xqc_cubic.h +++ b/src/congestion_control/xqc_cubic.h @@ -20,6 +20,7 @@ typedef struct { uint64_t bic_K; /* time period from W growth to Wmax */ xqc_usec_t epoch_start; /* the moment when congestion switchover begins, in microseconds */ xqc_usec_t min_rtt; + xqc_usec_t congestion_recovery_start_time; } xqc_cubic_t; extern const xqc_cong_ctrl_callback_t xqc_cubic_cb; diff --git a/src/http3/xqc_h3_conn.c b/src/http3/xqc_h3_conn.c index c349d2fab..9a608f182 100644 --- a/src/http3/xqc_h3_conn.c +++ b/src/http3/xqc_h3_conn.c @@ -603,13 +603,13 @@ xqc_h3_conn_process_blocked_stream(xqc_h3_conn_t *h3c) return XQC_OK; } - xqc_int_t -xqc_h3_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data) +xqc_h3_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, + void *conn_user_data, void *conn_proto_data) { xqc_int_t ret; xqc_h3_conn_t *h3c; - h3c = xqc_h3_conn_create(conn, user_data); + h3c = xqc_h3_conn_create(conn, conn_user_data); if (!h3c) { xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_h3_conn_create error|"); return -XQC_H3_ECREATE_CONN; @@ -649,9 +649,10 @@ xqc_h3_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *us xqc_int_t -xqc_h3_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data) +xqc_h3_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, + void *conn_user_data, void *conn_proto_data) { - xqc_h3_conn_t *h3c = (xqc_h3_conn_t*)user_data; + xqc_h3_conn_t *h3c = (xqc_h3_conn_t*)conn_proto_data; xqc_h3_conn_destroy(h3c); xqc_log(conn->log, XQC_LOG_DEBUG, "|destroy h3 conn success|"); return XQC_OK; @@ -659,9 +660,10 @@ xqc_h3_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *use void -xqc_h3_conn_handshake_finished(xqc_connection_t *conn, void *user_data) +xqc_h3_conn_handshake_finished(xqc_connection_t *conn, + void *conn_user_data, void *conn_proto_data) { - xqc_h3_conn_t *h3c = (xqc_h3_conn_t*)user_data; + xqc_h3_conn_t *h3c = (xqc_h3_conn_t *)conn_proto_data; if (h3c->h3_conn_callbacks.h3_conn_handshake_finished) { xqc_log(conn->log, XQC_LOG_DEBUG, "|HANDSHAKE_COMPLETED notify|"); @@ -671,9 +673,9 @@ xqc_h3_conn_handshake_finished(xqc_connection_t *conn, void *user_data) void xqc_h3_conn_ping_acked_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *ping_user_data, - void *user_data) + void *conn_user_data, void *conn_proto_data) { - xqc_h3_conn_t *h3c = (xqc_h3_conn_t*)user_data; + xqc_h3_conn_t *h3c = (xqc_h3_conn_t*)conn_proto_data; if (h3c->h3_conn_callbacks.h3_conn_ping_acked) { xqc_log(conn->log, XQC_LOG_DEBUG, "|Ping acked notify|"); diff --git a/src/http3/xqc_h3_request.c b/src/http3/xqc_h3_request.c index 97cc1f15a..786349b08 100644 --- a/src/http3/xqc_h3_request.c +++ b/src/http3/xqc_h3_request.c @@ -24,7 +24,7 @@ xqc_h3_request_create(xqc_engine_t *engine, const xqc_cid_t *cid, void *user_dat return NULL; } - h3_conn = (xqc_h3_conn_t*)stream->stream_conn->app_proto_user_data; + h3_conn = (xqc_h3_conn_t*)stream->stream_conn->proto_data; h3_stream = xqc_h3_stream_create(h3_conn, stream, XQC_H3_STREAM_TYPE_REQUEST, user_data); if (!h3_stream) { diff --git a/src/http3/xqc_h3_stream.c b/src/http3/xqc_h3_stream.c index 374cfc9f3..daca1d0d4 100644 --- a/src/http3/xqc_h3_stream.c +++ b/src/http3/xqc_h3_stream.c @@ -1162,7 +1162,7 @@ xqc_h3_stream_process_data(xqc_stream_t *stream, xqc_h3_stream_t *h3s, xqc_bool_ { xqc_int_t ret; ssize_t read; - xqc_h3_conn_t *h3c = (xqc_h3_conn_t *)stream->stream_conn->app_proto_user_data; + xqc_h3_conn_t *h3c = (xqc_h3_conn_t *)stream->stream_conn->proto_data; unsigned char buff[XQC_DATA_BUF_SIZE_4K]; size_t buff_size = XQC_DATA_BUF_SIZE_4K; uint64_t insert_cnt = xqc_qpack_get_dec_insert_count(h3s->qpack); @@ -1290,7 +1290,7 @@ xqc_h3_stream_read_notify(xqc_stream_t *stream, void *user_data) xqc_int_t ret; xqc_h3_stream_t *h3s; - xqc_h3_conn_t *h3c = (xqc_h3_conn_t *)stream->stream_conn->app_proto_user_data; + xqc_h3_conn_t *h3c = (xqc_h3_conn_t *)stream->stream_conn->proto_data; /* server h3_stream might not be created yet */ if (!user_data) { diff --git a/src/tls/boringssl/xqc_ssl_if.c b/src/tls/boringssl/xqc_ssl_if.c index 8a377b13f..fa29bfc7f 100644 --- a/src/tls/boringssl/xqc_ssl_if.c +++ b/src/tls/boringssl/xqc_ssl_if.c @@ -52,36 +52,18 @@ xqc_int_t xqc_ssl_get_certs_array(SSL *ssl, X509_STORE_CTX *store_ctx, unsigned char **certs_array, size_t array_cap, size_t *certs_array_len, size_t *certs_len) { - unsigned char *cert_buf = NULL; - X509 *cert = NULL; - int cert_size = 0; + const STACK_OF(CRYPTO_BUFFER) *chain = SSL_get0_peer_certificates(ssl); - const STACK_OF(X509) *chain = X509_STORE_CTX_get0_chain(store_ctx); - *certs_array_len = sk_X509_num(chain); - if (*certs_array_len > XQC_MAX_VERIFY_DEPTH) { /* impossible */ + *certs_array_len = sk_CRYPTO_BUFFER_num(chain); + if (*certs_array_len > XQC_MAX_VERIFY_DEPTH) { X509_STORE_CTX_set_error(store_ctx, X509_V_ERR_CERT_CHAIN_TOO_LONG); return -XQC_TLS_INTERNAL; } for (int i = 0; i < *certs_array_len; i++) { - /* get the size of cert */ - cert = sk_X509_value(chain, i); - cert_size = i2d_X509(cert, NULL); - if (cert_size <= 0) { - return -XQC_TLS_INTERNAL; - } - - /* malloc memory for copy cert */ - certs_array[i] = xqc_malloc(cert_size); - if (certs_array[i] == NULL) { - return -XQC_TLS_NOMEM; - } - - /* copy cert */ - certs_len[i] = i2d_X509(cert, &certs_array[i]); - if (certs_len[i] <= 0) { - return -XQC_TLS_INTERNAL; - } + CRYPTO_BUFFER * buffer = sk_CRYPTO_BUFFER_value(chain, i); + certs_array[i] = (unsigned char *)CRYPTO_BUFFER_data(buffer); + certs_len[i] = (size_t)CRYPTO_BUFFER_len(buffer); } return XQC_OK; diff --git a/src/transport/xqc_client.c b/src/transport/xqc_client.c index 22e32989b..0fe610c7b 100644 --- a/src/transport/xqc_client.c +++ b/src/transport/xqc_client.c @@ -67,7 +67,7 @@ xqc_client_connect(xqc_engine_t *engine, const xqc_conn_settings_t *conn_setting /* conn_create callback */ if (xc->app_proto_cbs.conn_cbs.conn_create_notify) { - if (xc->app_proto_cbs.conn_cbs.conn_create_notify(xc, &xc->scid_set.user_scid, user_data)) { + if (xc->app_proto_cbs.conn_cbs.conn_create_notify(xc, &xc->scid_set.user_scid, user_data, NULL)) { xqc_conn_destroy(xc); return NULL; } diff --git a/src/transport/xqc_conn.c b/src/transport/xqc_conn.c index a16c35e04..b89f2e507 100644 --- a/src/transport/xqc_conn.c +++ b/src/transport/xqc_conn.c @@ -228,6 +228,8 @@ xqc_conn_init_key_update_ctx(xqc_connection_t *conn) ctx->first_sent_pktno = 0; ctx->first_recv_pktno = 0; ctx->enc_pkt_cnt = 0; + + ctx->initiate_time_guard = 0; } xqc_connection_t * @@ -548,7 +550,7 @@ xqc_conn_server_on_alpn(xqc_connection_t *conn, const unsigned char *alpn, size_ /* do callback */ if (conn->app_proto_cbs.conn_cbs.conn_create_notify) { if (conn->app_proto_cbs.conn_cbs.conn_create_notify(conn, &conn->scid_set.user_scid, - conn->app_proto_user_data)) + conn->user_data, conn->proto_data)) { XQC_CONN_ERR(conn, TRA_INTERNAL_ERROR); return -TRA_INTERNAL_ERROR; @@ -608,7 +610,8 @@ xqc_conn_destroy(xqc_connection_t *xc) /* ALPN negotiated, notify close through application layer protocol callback function */ if (xc->app_proto_cbs.conn_cbs.conn_close_notify) { xc->app_proto_cbs.conn_cbs.conn_close_notify(xc, &xc->scid_set.user_scid, - xc->app_proto_user_data); + xc->user_data, + xc->proto_data); } else if (xc->transport_cbs.server_refuse) { /* ALPN context is not initialized, ClientHello has not been received */ @@ -673,7 +676,7 @@ xqc_conn_set_transport_user_data(xqc_connection_t *conn, void *user_data) void xqc_conn_set_alp_user_data(xqc_connection_t *conn, void *user_data) { - conn->app_proto_user_data = user_data; + conn->proto_data = user_data; } xqc_int_t @@ -1123,6 +1126,7 @@ xqc_conn_enc_packet(xqc_connection_t *conn, xqc_packet_out_t *packet_out, packet_out->po_pkt.pkt_num = conn->conn_send_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns]++; xqc_write_packet_number(packet_out->po_ppktno, packet_out->po_pkt.pkt_num, XQC_PKTNO_BITS); xqc_long_packet_update_length(packet_out); + xqc_short_packet_update_key_phase(packet_out, conn->key_update_ctx.cur_out_key_phase); /* encrypt */ xqc_int_t ret = xqc_packet_encrypt_buf(conn, packet_out, enc_pkt, enc_pkt_cap, enc_pkt_len); @@ -2621,7 +2625,7 @@ xqc_conn_check_handshake_complete(xqc_connection_t *conn) xqc_conn_handshake_complete(conn); if (conn->app_proto_cbs.conn_cbs.conn_handshake_finished) { - conn->app_proto_cbs.conn_cbs.conn_handshake_finished(conn, conn->app_proto_user_data); + conn->app_proto_cbs.conn_cbs.conn_handshake_finished(conn, conn->user_data, conn->proto_data); } } @@ -2729,6 +2733,15 @@ xqc_conn_confirm_key_update(xqc_connection_t *conn) xqc_send_ctl_timer_set(conn->conn_send_ctl, XQC_TIMER_KEY_UPDATE, now, 3 * pto); } + /* + * Endpoints need to allow for the possibility that a peer might not be able to decrypt + * packets that initiate a key update during the period when the peer retains old keys. + * Endpoints SHOULD wait three times the PTO before initiating a key update after receiving + * an acknowledgment that confirms that the previous key update was received. + * Failing to allow sufficient time could lead to packets being discarded. + */ + ctx->initiate_time_guard = now + 3 * pto; + return XQC_OK; } diff --git a/src/transport/xqc_conn.h b/src/transport/xqc_conn.h index da4db6831..612ff64cf 100644 --- a/src/transport/xqc_conn.h +++ b/src/transport/xqc_conn.h @@ -210,6 +210,7 @@ typedef struct { xqc_packet_number_t first_sent_pktno; /* lowest packet number sent with each key phase */ xqc_packet_number_t first_recv_pktno; /* lowest packet number recv with each key phase */ uint64_t enc_pkt_cnt; /* number of packet encrypt with each key phase */ + xqc_usec_t initiate_time_guard; /* time limit for initiating next key update */ } xqc_key_update_ctx_t; @@ -277,7 +278,7 @@ struct xqc_connection_s { /* callback function and user_data to application-layer-protocol layer */ xqc_app_proto_callbacks_t app_proto_cbs; - void *app_proto_user_data; + void *proto_data; xqc_list_head_t undecrypt_packet_in[XQC_ENC_LEV_MAX]; /* buffer for reordered packets */ uint32_t undecrypt_count[XQC_ENC_LEV_MAX]; diff --git a/src/transport/xqc_packet_parser.c b/src/transport/xqc_packet_parser.c index 3c64a2027..98937bfdb 100644 --- a/src/transport/xqc_packet_parser.c +++ b/src/transport/xqc_packet_parser.c @@ -604,13 +604,15 @@ xqc_packet_encrypt_buf(xqc_connection_t *conn, xqc_packet_out_t *packet_out, } /* update enc_pkt_cnt for current 1-rtt key & maybe initiate a key update */ - if (packet_out->po_pkt.pkt_type == XQC_PTYPE_SHORT_HEADER && level == XQC_ENC_LEV_1RTT) { + if (conn->conn_settings.keyupdate_pkt_threshold > 0 + && packet_out->po_pkt.pkt_type == XQC_PTYPE_SHORT_HEADER && level == XQC_ENC_LEV_1RTT) + { conn->key_update_ctx.enc_pkt_cnt++; - if (conn->conn_settings.keyupdate_pkt_threshold > 0 - && conn->key_update_ctx.enc_pkt_cnt > conn->conn_settings.keyupdate_pkt_threshold + if (conn->key_update_ctx.enc_pkt_cnt > conn->conn_settings.keyupdate_pkt_threshold && conn->key_update_ctx.first_sent_pktno <= - conn->conn_send_ctl->ctl_largest_acked[XQC_PNS_APP_DATA]) + conn->conn_send_ctl->ctl_largest_acked[XQC_PNS_APP_DATA] + && xqc_monotonic_timestamp() > conn->key_update_ctx.initiate_time_guard) { ret = xqc_tls_update_1rtt_keys(conn->tls, XQC_KEY_TYPE_RX_READ); if (ret != XQC_OK) { diff --git a/src/transport/xqc_send_ctl.c b/src/transport/xqc_send_ctl.c index 0f343f450..8c27795bc 100644 --- a/src/transport/xqc_send_ctl.c +++ b/src/transport/xqc_send_ctl.c @@ -1550,7 +1550,7 @@ xqc_send_ctl_on_packet_acked(xqc_send_ctl_t *ctl, && (packet_out->po_flag & XQC_POF_NOTIFY)) { conn->app_proto_cbs.conn_cbs.conn_ping_acked(conn, &conn->scid_set.user_scid, - packet_out->po_user_data, conn->app_proto_user_data); + packet_out->po_user_data, conn->user_data, conn->proto_data); } } diff --git a/tests/test_client.c b/tests/test_client.c index 02a04a55e..a6bec5ac3 100644 --- a/tests/test_client.c +++ b/tests/test_client.c @@ -615,11 +615,11 @@ xqc_client_user_conn_create(const char *server_addr, int server_port, int -xqc_client_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data) +xqc_client_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data, void *conn_proto_data) { DEBUG; - user_conn_t *user_conn = (user_conn_t *) user_data; + user_conn_t *user_conn = (user_conn_t *)user_data; xqc_conn_set_alp_user_data(conn, user_conn); printf("xqc_conn_is_ready_to_send_early_data:%d\n", xqc_conn_is_ready_to_send_early_data(conn)); @@ -627,11 +627,11 @@ xqc_client_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void } int -xqc_client_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data) +xqc_client_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data, void *conn_proto_data) { DEBUG; - user_conn_t *user_conn = (user_conn_t *) user_data; + user_conn_t *user_conn = (user_conn_t *)conn_proto_data; xqc_conn_stats_t stats = xqc_conn_get_stats(ctx.engine, cid); printf("send_count:%u, lost_count:%u, tlp_count:%u, recv_count:%u, srtt:%"PRIu64" early_data_flag:%d, conn_err:%d, ack_info:%s\n", @@ -642,7 +642,7 @@ xqc_client_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void } void -xqc_client_conn_ping_acked_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *ping_user_data, void *user_data) +xqc_client_conn_ping_acked_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *ping_user_data, void *user_data, void *conn_proto_data) { DEBUG; if (ping_user_data) { @@ -669,7 +669,7 @@ xqc_client_conn_update_cid_notify(xqc_connection_t *conn, const xqc_cid_t *retir } void -xqc_client_conn_handshake_finished(xqc_connection_t *conn, void *user_data) +xqc_client_conn_handshake_finished(xqc_connection_t *conn, void *user_data, void *conn_proto_data) { DEBUG; user_conn_t *user_conn = (user_conn_t *) user_data; diff --git a/tests/test_server.c b/tests/test_server.c index 8bdc6c346..58ebbd683 100644 --- a/tests/test_server.c +++ b/tests/test_server.c @@ -163,17 +163,17 @@ read_file_data( char * data, size_t data_len, char *filename) } int -xqc_server_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data) +xqc_server_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data, void *conn_proto_data) { DEBUG; return 0; } int -xqc_server_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data) +xqc_server_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void *user_data, void *conn_proto_data) { DEBUG; - user_conn_t *user_conn = (user_conn_t*)user_data; + user_conn_t *user_conn = (user_conn_t *)conn_proto_data; xqc_conn_stats_t stats = xqc_conn_get_stats(ctx.engine, cid); printf("send_count:%u, lost_count:%u, tlp_count:%u, recv_count:%u, srtt:%"PRIu64" early_data_flag:%d, conn_err:%d, ack_info:%s\n", stats.send_count, stats.lost_count, stats.tlp_count, stats.recv_count, stats.srtt, stats.early_data_flag, stats.conn_err, stats.ack_info); @@ -183,7 +183,7 @@ xqc_server_conn_close_notify(xqc_connection_t *conn, const xqc_cid_t *cid, void } void -xqc_server_conn_handshake_finished(xqc_connection_t *conn, void *user_data) +xqc_server_conn_handshake_finished(xqc_connection_t *conn, void *user_data, void *conn_proto_data) { DEBUG; user_conn_t *user_conn = (user_conn_t *) user_data;