From 59d6828848665bfd7a1191933a57597974076137 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 15 May 2022 11:57:00 +0900 Subject: [PATCH 1/2] Slightly simplified error handling for ngtcp2_conn_read_pkt --- src/h2load.h | 1 + src/h2load_quic.cc | 14 ++++++++------ src/shrpx_http3_upstream.cc | 13 +++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/h2load.h b/src/h2load.h index 4d79e0d7..b67dc018 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -342,6 +342,7 @@ struct Client { ev_timer pkt_timer; ngtcp2_conn *conn; ngtcp2_connection_close_error last_error; + uint8_t tls_alert; bool close_requested; FILE *qlog_file; diff --git a/src/h2load_quic.cc b/src/h2load_quic.cc index 4e234e8b..8795a456 100644 --- a/src/h2load_quic.cc +++ b/src/h2load_quic.cc @@ -574,10 +574,7 @@ int Client::quic_on_tx_secret(ngtcp2_crypto_level level, const uint8_t *secret, return 0; } -void Client::quic_set_tls_alert(uint8_t alert) { - ngtcp2_connection_close_error_set_transport_error_tls_alert( - &quic.last_error, alert, nullptr, 0); -} +void Client::quic_set_tls_alert(uint8_t alert) { quic.tls_alert = alert; } void Client::quic_write_client_handshake(ngtcp2_crypto_level level, const uint8_t *data, size_t datalen) { @@ -656,8 +653,13 @@ int Client::read_quic() { std::cerr << "ngtcp2_conn_read_pkt: " << ngtcp2_strerror(rv) << std::endl; if (!quic.last_error.error_code) { - ngtcp2_connection_close_error_set_transport_error_liberr( - &quic.last_error, rv, nullptr, 0); + if (rv == NGTCP2_ERR_CRYPTO) { + ngtcp2_connection_close_error_set_transport_error_tls_alert( + &quic.last_error, quic.tls_alert, nullptr, 0); + } else { + ngtcp2_connection_close_error_set_transport_error_liberr( + &quic.last_error, rv, nullptr, 0); + } } return -1; diff --git a/src/shrpx_http3_upstream.cc b/src/shrpx_http3_upstream.cc index 02255d63..f8e6048f 100644 --- a/src/shrpx_http3_upstream.cc +++ b/src/shrpx_http3_upstream.cc @@ -1786,14 +1786,11 @@ int Http3Upstream::on_read(const UpstreamAddr *faddr, return -1; } - case NGTCP2_ERR_REQUIRED_TRANSPORT_PARAM: - case NGTCP2_ERR_MALFORMED_TRANSPORT_PARAM: - case NGTCP2_ERR_TRANSPORT_PARAM: - // If rv indicates transport_parameters related error, we should - // send TRANSPORT_PARAMETER_ERROR even if last_error_.code is - // already set. This is because OpenSSL might set Alert. - ngtcp2_connection_close_error_set_transport_error_liberr(&last_error_, rv, - nullptr, 0); + case NGTCP2_ERR_CRYPTO: + if (!last_error_.error_code) { + ngtcp2_connection_close_error_set_transport_error_tls_alert( + &last_error_, tls_alert_, nullptr, 0); + } break; case NGTCP2_ERR_DROP_CONN: return -1; From 516cf851c38aa26bff058176002f56914698766e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 15 May 2022 12:01:07 +0900 Subject: [PATCH 2/2] h2load: Deal with error from ngtcp2_conn_submit_crypto_data --- src/h2load.h | 4 ++-- src/h2load_quic.cc | 29 ++++++++++++++++++++++------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/h2load.h b/src/h2load.h index b67dc018..087ee8cd 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -501,8 +501,8 @@ struct Client { size_t secretlen); void quic_set_tls_alert(uint8_t alert); - void quic_write_client_handshake(ngtcp2_crypto_level level, - const uint8_t *data, size_t datalen); + int quic_write_client_handshake(ngtcp2_crypto_level level, + const uint8_t *data, size_t datalen); int quic_pkt_timeout(); void quic_restart_pkt_timer(); void quic_write_qlog(const void *data, size_t datalen); diff --git a/src/h2load_quic.cc b/src/h2load_quic.cc index 8795a456..437f5754 100644 --- a/src/h2load_quic.cc +++ b/src/h2load_quic.cc @@ -267,8 +267,11 @@ namespace { int add_handshake_data(SSL *ssl, OSSL_ENCRYPTION_LEVEL ossl_level, const uint8_t *data, size_t len) { auto c = static_cast(SSL_get_app_data(ssl)); - c->quic_write_client_handshake( - ngtcp2_crypto_openssl_from_ossl_encryption_level(ossl_level), data, len); + if (c->quic_write_client_handshake( + ngtcp2_crypto_openssl_from_ossl_encryption_level(ossl_level), data, + len) != 0) { + return 0; + } return 1; } } // namespace @@ -332,8 +335,11 @@ namespace { int add_handshake_data(SSL *ssl, ssl_encryption_level_t ssl_level, const uint8_t *data, size_t len) { auto c = static_cast(SSL_get_app_data(ssl)); - c->quic_write_client_handshake( - ngtcp2_crypto_boringssl_from_ssl_encryption_level(ssl_level), data, len); + if (c->quic_write_client_handshake( + ngtcp2_crypto_boringssl_from_ssl_encryption_level(ssl_level), data, + len) != 0) { + return 0; + } return 1; } } // namespace @@ -576,11 +582,20 @@ int Client::quic_on_tx_secret(ngtcp2_crypto_level level, const uint8_t *secret, void Client::quic_set_tls_alert(uint8_t alert) { quic.tls_alert = alert; } -void Client::quic_write_client_handshake(ngtcp2_crypto_level level, - const uint8_t *data, size_t datalen) { +int Client::quic_write_client_handshake(ngtcp2_crypto_level level, + const uint8_t *data, size_t datalen) { + int rv; + assert(level < 2); - ngtcp2_conn_submit_crypto_data(quic.conn, level, data, datalen); + rv = ngtcp2_conn_submit_crypto_data(quic.conn, level, data, datalen); + if (rv != 0) { + std::cerr << "ngtcp2_conn_submit_crypto_data: " << ngtcp2_strerror(rv) + << std::endl; + return -1; + } + + return 0; } void quic_pkt_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) {