From: Hugo Landau Date: Fri, 28 Jul 2023 16:48:14 +0000 (+0100) Subject: QUIC CHANNEL: Improve error reporting X-Git-Tag: openssl-3.2.0-alpha1~274 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=741170bef340b31a32a94a4ea86cc0d7744c01b2;p=thirdparty%2Fopenssl.git QUIC CHANNEL: Improve error reporting Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/21547) --- diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index baa86b622af..0be223b974f 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1498,6 +1498,7 @@ SSL_R_PROTOCOL_IS_SHUTDOWN:207:protocol is shutdown SSL_R_PSK_IDENTITY_NOT_FOUND:223:psk identity not found SSL_R_PSK_NO_CLIENT_CB:224:psk no client cb SSL_R_PSK_NO_SERVER_CB:225:psk no server cb +SSL_R_QUIC_NETWORK_ERROR:387:quic network error SSL_R_QUIC_PROTOCOL_ERROR:382:quic protocol error SSL_R_READ_BIO_NOT_SET:211:read bio not set SSL_R_READ_TIMEOUT_EXPIRED:312:read timeout expired diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h index abeeb05b964..f9e654fd937 100644 --- a/include/internal/quic_channel.h +++ b/include/internal/quic_channel.h @@ -211,11 +211,28 @@ int ossl_quic_channel_on_handshake_confirmed(QUIC_CHANNEL *ch); * reason string is not currently handled, but should be a string of static * storage duration. If the connection has already terminated due to a previous * protocol error, this is a no-op; first error wins. + * + * Usually the ossl_quic_channel_raise_protocol_error() function should be used. + * The ossl_quic_channel_raise_protocol_error_loc() function can be used + * directly for passing through existing call site information from an existing + * error. */ -void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch, - uint64_t error_code, - uint64_t frame_type, - const char *reason); +void ossl_quic_channel_raise_protocol_error_loc(QUIC_CHANNEL *ch, + uint64_t error_code, + uint64_t frame_type, + const char *reason, + const char *src_file, + int src_line, + const char *src_func); + +#define ossl_quic_channel_raise_protocol_error(ch, error_code, frame_type, reason) \ + ossl_quic_channel_raise_protocol_error_loc((ch), (error_code), \ + (frame_type), \ + (reason), \ + OPENSSL_FILE, \ + OPENSSL_LINE, \ + OPENSSL_FUNC) + /* * Returns 1 if permanent net error was detected on the QUIC_CHANNEL, * 0 otherwise. diff --git a/include/internal/quic_error.h b/include/internal/quic_error.h index e32bdf03f8f..9495c3e67d9 100644 --- a/include/internal/quic_error.h +++ b/include/internal/quic_error.h @@ -46,6 +46,34 @@ # define QUIC_ERR_CRYPTO_NO_APP_PROTO \ QUIC_ERR_CRYPTO_ERR(TLS1_AD_NO_APPLICATION_PROTOCOL) +static ossl_inline ossl_unused const char * +ossl_quic_err_to_string(uint64_t error_code) +{ + switch (error_code) { +#define X(name) case QUIC_ERR_##name: return #name; + X(NO_ERROR) + X(INTERNAL_ERROR) + X(CONNECTION_REFUSED) + X(FLOW_CONTROL_ERROR) + X(STREAM_LIMIT_ERROR) + X(STREAM_STATE_ERROR) + X(FINAL_SIZE_ERROR) + X(FRAME_ENCODING_ERROR) + X(TRANSPORT_PARAMETER_ERROR) + X(CONNECTION_ID_LIMIT_ERROR) + X(PROTOCOL_VIOLATION) + X(INVALID_TOKEN) + X(APPLICATION_ERROR) + X(CRYPTO_BUFFER_EXCEEDED) + X(KEY_UPDATE_ERROR) + X(AEAD_LIMIT_REACHED) + X(NO_VIABLE_PATH) +#undef X + default: + return NULL; + } +} + # endif #endif diff --git a/include/internal/quic_tls.h b/include/internal/quic_tls.h index 9c5fa9cd5ac..13da5882c96 100644 --- a/include/internal/quic_tls.h +++ b/include/internal/quic_tls.h @@ -97,6 +97,9 @@ int ossl_quic_tls_set_transport_params(QUIC_TLS *qtls, int ossl_quic_tls_get_error(QUIC_TLS *qtls, uint64_t *error_code, - const char **error_msg); + const char **error_msg, + const char **error_src_file, + int *error_src_line, + const char **error_src_func); #endif diff --git a/include/internal/quic_wire.h b/include/internal/quic_wire.h index ee8f7e76cb6..4f059120b36 100644 --- a/include/internal/quic_wire.h +++ b/include/internal/quic_wire.h @@ -87,6 +87,48 @@ # define OSSL_QUIC_FRAME_TYPE_IS_CONN_CLOSE(x) \ (((x) & ~(uint64_t)1) == OSSL_QUIC_FRAME_TYPE_CONN_CLOSE_TRANSPORT) +static ossl_unused ossl_inline const char * +ossl_quic_frame_type_to_string(uint64_t frame_type) +{ + switch (frame_type) { +#define X(name) case OSSL_QUIC_FRAME_TYPE_##name: return #name; + X(PADDING) + X(PING) + X(ACK_WITHOUT_ECN) + X(ACK_WITH_ECN) + X(RESET_STREAM) + X(STOP_SENDING) + X(CRYPTO) + X(NEW_TOKEN) + X(MAX_DATA) + X(MAX_STREAM_DATA) + X(MAX_STREAMS_BIDI) + X(MAX_STREAMS_UNI) + X(DATA_BLOCKED) + X(STREAM_DATA_BLOCKED) + X(STREAMS_BLOCKED_BIDI) + X(STREAMS_BLOCKED_UNI) + X(NEW_CONN_ID) + X(RETIRE_CONN_ID) + X(PATH_CHALLENGE) + X(PATH_RESPONSE) + X(CONN_CLOSE_TRANSPORT) + X(CONN_CLOSE_APP) + X(HANDSHAKE_DONE) + X(STREAM) + X(STREAM_FIN) + X(STREAM_LEN) + X(STREAM_LEN_FIN) + X(STREAM_OFF) + X(STREAM_OFF_FIN) + X(STREAM_OFF_LEN) + X(STREAM_OFF_LEN_FIN) +#undef X + default: + return NULL; + } +} + static ossl_unused ossl_inline int ossl_quic_frame_type_is_ack_eliciting(uint64_t frame_type) { diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index 4a05f6636f6..b330b90be4a 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -230,6 +230,7 @@ # define SSL_R_PSK_IDENTITY_NOT_FOUND 223 # define SSL_R_PSK_NO_CLIENT_CB 224 # define SSL_R_PSK_NO_SERVER_CB 225 +# define SSL_R_QUIC_NETWORK_ERROR 387 # define SSL_R_QUIC_PROTOCOL_ERROR 382 # define SSL_R_READ_BIO_NOT_SET 211 # define SSL_R_READ_TIMEOUT_EXPIRED 312 diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index c7fbb9f4a19..5fb49a90dfc 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -1616,7 +1616,8 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags) QUIC_CHANNEL *ch = arg; int channel_only = (flags & QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY) != 0; uint64_t error_code; - const char *error_msg; + const char *error_msg, *error_src_file, *error_src_func; + int error_src_line; /* * When we tick the QUIC connection, we do everything we need to do @@ -1672,9 +1673,16 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags) if (!channel_only) { ossl_quic_tls_tick(ch->qtls); - if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg)) - ossl_quic_channel_raise_protocol_error(ch, error_code, 0, - error_msg); + if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg, + &error_src_file, + &error_src_line, + &error_src_func)) { + ossl_quic_channel_raise_protocol_error_loc(ch, error_code, 0, + error_msg, + error_src_file, + error_src_line, + error_src_func); + } } /* @@ -2877,6 +2885,9 @@ static void ch_raise_net_error(QUIC_CHANNEL *ch) QUIC_TERMINATE_CAUSE tcause = {0}; ch->net_error = 1; + + ERR_raise_data(ERR_LIB_SSL, SSL_R_QUIC_NETWORK_ERROR, + "connection terminated due to network error"); ch_save_err_state(ch); tcause.error_code = QUIC_ERR_INTERNAL_ERROR; @@ -2901,19 +2912,55 @@ void ossl_quic_channel_restore_err_state(QUIC_CHANNEL *ch) OSSL_ERR_STATE_restore(ch->err_state); } -void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch, - uint64_t error_code, - uint64_t frame_type, - const char *reason) +void ossl_quic_channel_raise_protocol_error_loc(QUIC_CHANNEL *ch, + uint64_t error_code, + uint64_t frame_type, + const char *reason, + const char *src_file, + int src_line, + const char *src_func) { QUIC_TERMINATE_CAUSE tcause = {0}; int err_reason = error_code == QUIC_ERR_INTERNAL_ERROR ? ERR_R_INTERNAL_ERROR : SSL_R_QUIC_PROTOCOL_ERROR; + const char *err_str = ossl_quic_err_to_string(error_code); + const char *err_str_pfx = " (", *err_str_sfx = ")"; + const char *ft_str = NULL; + const char *ft_str_pfx = " (", *ft_str_sfx = ")"; + + if (err_str == NULL) { + err_str = ""; + err_str_pfx = ""; + err_str_sfx = ""; + } + + if (frame_type != 0) { + ft_str = ossl_quic_frame_type_to_string(frame_type); + if (ft_str == NULL) { + ft_str = ""; + ft_str_pfx = ""; + ft_str_sfx = ""; + } + + ERR_raise_data(ERR_LIB_SSL, err_reason, + "QUIC error code: 0x%llx%s%s%s " + "(triggered by frame type: 0x%llx%s%s%s), reason: \"%s\"", + (unsigned long long) error_code, + err_str_pfx, err_str, err_str_sfx, + (unsigned long long) frame_type, + ft_str_pfx, ft_str, ft_str_sfx, + reason); + } else { + ERR_raise_data(ERR_LIB_SSL, err_reason, + "QUIC error code: 0x%llx%s%s%s, reason: \"%s\"", + (unsigned long long) error_code, + err_str_pfx, err_str, err_str_sfx, + reason); + } + + if (src_file != NULL) + ERR_set_debug(src_file, src_line, src_func); - ERR_raise_data(ERR_LIB_SSL, err_reason, - "Error code: %llu Frame type: %llu Reason: %s", - (unsigned long long) error_code, - (unsigned long long) frame_type, reason); ch_save_err_state(ch); tcause.error_code = error_code; diff --git a/ssl/quic/quic_tls.c b/ssl/quic/quic_tls.c index 05baa32b41b..d8770bf9f7d 100644 --- a/ssl/quic/quic_tls.c +++ b/ssl/quic/quic_tls.c @@ -36,11 +36,15 @@ struct quic_tls_st { uint64_t error_code; /* - * Error message with static storage duration. Valid only if inerr is 1. + * Error message with static storage duration. Valid only if inerror is 1. * Should be suitable for encapsulation in a CONNECTION_CLOSE frame. */ const char *error_msg; + const char *error_src_file; + const char *error_src_func; + int error_src_line; + /* Whether our SSL object for TLS has been configured for use in QUIC */ unsigned int configured : 1; @@ -642,18 +646,26 @@ void ossl_quic_tls_free(QUIC_TLS *qtls) } static int raise_error(QUIC_TLS *qtls, uint64_t error_code, - const char *error_msg) -{ - qtls->error_code = error_code; - qtls->error_msg = error_msg; - qtls->inerror = 1; + const char *error_msg, + const char *src_file, + int src_line, + const char *src_func) +{ + qtls->error_code = error_code; + qtls->error_msg = error_msg; + qtls->error_src_file = src_file; + qtls->error_src_line = src_line; + qtls->error_src_func = src_func; + qtls->inerror = 1; return 0; } -static int raise_internal_error(QUIC_TLS *qtls) -{ - return raise_error(qtls, QUIC_ERR_INTERNAL_ERROR, "internal error"); -} +#define RAISE_ERROR(qtls, error_code, error_msg) \ + raise_error((qtls), (error_code), (error_msg), \ + OPENSSL_FILE, OPENSSL_LINE, OPENSSL_FUNC) + +#define RAISE_INTERNAL_ERROR(qtls) \ + RAISE_ERROR((qtls), QUIC_ERR_INTERNAL_ERROR, "internal error") int ossl_quic_tls_tick(QUIC_TLS *qtls) { @@ -684,13 +696,13 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls) /* ALPN is a requirement for QUIC and must be set */ if (qtls->args.is_server) { if (sctx->ext.alpn_select_cb == NULL) - return raise_internal_error(qtls); + return RAISE_INTERNAL_ERROR(qtls); } else { if (sc->ext.alpn == NULL || sc->ext.alpn_len == 0) - return raise_internal_error(qtls); + return RAISE_INTERNAL_ERROR(qtls); } if (!SSL_set_min_proto_version(qtls->args.s, TLS1_3_VERSION)) - return raise_internal_error(qtls); + return RAISE_INTERNAL_ERROR(qtls); SSL_clear_options(qtls->args.s, SSL_OP_ENABLE_MIDDLEBOX_COMPAT); ossl_ssl_set_custom_record_layer(sc, &quic_tls_record_method, qtls); @@ -705,11 +717,11 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls) add_transport_params_cb, free_transport_params_cb, qtls, parse_transport_params_cb, qtls)) - return raise_internal_error(qtls); + return RAISE_INTERNAL_ERROR(qtls); nullbio = BIO_new(BIO_s_null()); if (nullbio == NULL) - return raise_internal_error(qtls); + return RAISE_INTERNAL_ERROR(qtls); /* * Our custom record layer doesn't use the BIO - but libssl generally @@ -739,7 +751,7 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls) case SSL_ERROR_WANT_WRITE: return 1; default: - return raise_internal_error(qtls); + return RAISE_INTERNAL_ERROR(qtls); } } @@ -747,7 +759,7 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls) /* Validate that we have ALPN */ SSL_get0_alpn_selected(qtls->args.s, &alpn, &alpnlen); if (alpn == NULL || alpnlen == 0) - return raise_error(qtls, QUIC_ERR_CRYPTO_NO_APP_PROTO, + return RAISE_ERROR(qtls, QUIC_ERR_CRYPTO_NO_APP_PROTO, "no application protocol negotiated"); qtls->complete = 1; @@ -768,11 +780,17 @@ int ossl_quic_tls_set_transport_params(QUIC_TLS *qtls, int ossl_quic_tls_get_error(QUIC_TLS *qtls, uint64_t *error_code, - const char **error_msg) + const char **error_msg, + const char **error_src_file, + int *error_src_line, + const char **error_src_func) { if (qtls->inerror) { - *error_code = qtls->error_code; - *error_msg = qtls->error_msg; + *error_code = qtls->error_code; + *error_msg = qtls->error_msg; + *error_src_file = qtls->error_src_file; + *error_src_line = qtls->error_src_line; + *error_src_func = qtls->error_src_func; } return qtls->inerror; diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c index a8584658993..4768cd856b0 100644 --- a/ssl/quic/quic_txp.c +++ b/ssl/quic/quic_txp.c @@ -844,7 +844,6 @@ out: return pkts_done > 0 ? TX_PACKETISER_RES_SENT_PKT : res; } - static const struct archetype_data archetypes[QUIC_ENC_LEVEL_NUM][TX_PACKETISER_ARCHETYPE_NUM] = { /* EL 0(INITIAL) */ { diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index d18cbf9bca9..9d24bc8014b 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -355,6 +355,7 @@ static const ERR_STRING_DATA SSL_str_reasons[] = { "psk identity not found"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_PSK_NO_CLIENT_CB), "psk no client cb"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_PSK_NO_SERVER_CB), "psk no server cb"}, + {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_QUIC_NETWORK_ERROR), "quic network error"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_QUIC_PROTOCOL_ERROR), "quic protocol error"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_READ_BIO_NOT_SET), "read bio not set"}, diff --git a/test/quic_multistream_test.c b/test/quic_multistream_test.c index 5a923947263..c14339975fa 100644 --- a/test/quic_multistream_test.c +++ b/test/quic_multistream_test.c @@ -2203,7 +2203,7 @@ static const struct script_op script_20_child[] = { OP_C_READ_FAIL_WAIT (a) OP_C_EXPECT_SSL_ERR (a, SSL_ERROR_SYSCALL) OP_EXPECT_ERR_LIB (ERR_LIB_SYS) - OP_EXPECT_ERR_REASON (SSL_R_PROTOCOL_IS_SHUTDOWN) + OP_EXPECT_ERR_REASON (SSL_R_QUIC_NETWORK_ERROR) OP_C_FREE_STREAM (a) OP_END