[PATCH] QUIC: removed use of SSL_quic_read_level and SSL_quic_write_level
Roman Arutyunyan
arut at nginx.com
Fri Sep 1 09:52:25 UTC 2023
Hi,
On Thu, Aug 31, 2023 at 01:51:24PM +0400, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1693475261 -14400
> # Thu Aug 31 13:47:41 2023 +0400
> # Node ID 015353ca1be7acc176f6369ed92ec6c49975ee6a
> # Parent 8f7e6d8c061e1f4b7656141d2fac85ce6846ac23
> QUIC: removed use of SSL_quic_read_level and SSL_quic_write_level.
>
> As explained in BoringSSL change[1], levels were introduced in the original
> QUIC API to draw a line between when keys are released and when are active.
> In the new QUIC API they are released in separate calls when it's needed.
> BoringSSL has then a consideration to remove levels API, hence the change.
>
> In most places such as feeding SSL handshake with CRYPTO payload, we already
> have such information from a QUIC packet header. If not, it is taken based
> on keys availability. The only real use of levels is to prevent using app
> keys before they are active in QuicTLS that provides old BoringSSL QUIC API,
> it is replaced with an equivalent check of c->ssl->handshaked.
>
> This change also removes OpenSSL compat shims since they are no longer used.
> The only exception left is keeping write level in the internal field which
> is a handy equivalent of checking keys availability.
>
> [1] https://boringssl.googlesource.com/boringssl/+/1e859054
>
> diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
> --- a/src/event/quic/ngx_event_quic.c
> +++ b/src/event/quic/ngx_event_quic.c
> @@ -509,8 +509,17 @@ ngx_quic_close_connection(ngx_connection
> * to terminate the connection immediately.
> */
>
> - qc->error_level = c->ssl ? SSL_quic_read_level(c->ssl->connection)
> - : ssl_encryption_initial;
> + if (ngx_quic_keys_available(qc->keys, ssl_encryption_application)) {
> + qc->error_level = ssl_encryption_application;
> +
> + } else if (ngx_quic_keys_available(qc->keys,
> + ssl_encryption_handshake))
> + {
> + qc->error_level = ssl_encryption_handshake;
> +
> + } else {
> + qc->error_level = ssl_encryption_initial;
> + }
>
> if (qc->error == (ngx_uint_t) -1) {
> qc->error = NGX_QUIC_ERR_INTERNAL_ERROR;
> @@ -964,10 +973,7 @@ ngx_quic_handle_payload(ngx_connection_t
> #if !defined (OPENSSL_IS_BORINGSSL)
> /* OpenSSL provides read keys for an application level before it's ready */
>
> - if (pkt->level == ssl_encryption_application
> - && SSL_quic_read_level(c->ssl->connection)
> - < ssl_encryption_application)
> - {
> + if (pkt->level == ssl_encryption_application && !c->ssl->handshaked) {
> ngx_log_error(NGX_LOG_INFO, c->log, 0,
> "quic no %s keys ready, ignoring packet",
> ngx_quic_level_name(pkt->level));
> diff --git a/src/event/quic/ngx_event_quic_openssl_compat.c b/src/event/quic/ngx_event_quic_openssl_compat.c
> --- a/src/event/quic/ngx_event_quic_openssl_compat.c
> +++ b/src/event/quic/ngx_event_quic_openssl_compat.c
> @@ -44,7 +44,6 @@ struct ngx_quic_compat_s {
> const SSL_QUIC_METHOD *method;
>
> enum ssl_encryption_level_t write_level;
> - enum ssl_encryption_level_t read_level;
>
> uint64_t read_record;
> ngx_quic_compat_keys_t keys;
> @@ -213,7 +212,6 @@ ngx_quic_compat_keylog_callback(const SS
>
> } else {
> com->method->set_read_secret((SSL *) ssl, level, cipher, secret, n);
> - com->read_level = level;
> com->read_record = 0;
>
> (void) ngx_quic_compat_set_encryption_secret(c->log, &com->keys, level,
> @@ -583,32 +581,6 @@ ngx_quic_compat_create_record(ngx_quic_c
> }
>
>
> -enum ssl_encryption_level_t
> -SSL_quic_read_level(const SSL *ssl)
> -{
> - ngx_connection_t *c;
> - ngx_quic_connection_t *qc;
> -
> - c = ngx_ssl_get_connection(ssl);
> - qc = ngx_quic_get_connection(c);
> -
> - return qc->compat->read_level;
> -}
> -
> -
> -enum ssl_encryption_level_t
> -SSL_quic_write_level(const SSL *ssl)
> -{
> - ngx_connection_t *c;
> - ngx_quic_connection_t *qc;
> -
> - c = ngx_ssl_get_connection(ssl);
> - qc = ngx_quic_get_connection(c);
> -
> - return qc->compat->write_level;
> -}
> -
> -
> int
> SSL_set_quic_transport_params(SSL *ssl, const uint8_t *params,
> size_t params_len)
> diff --git a/src/event/quic/ngx_event_quic_openssl_compat.h b/src/event/quic/ngx_event_quic_openssl_compat.h
> --- a/src/event/quic/ngx_event_quic_openssl_compat.h
> +++ b/src/event/quic/ngx_event_quic_openssl_compat.h
> @@ -48,8 +48,6 @@ ngx_int_t ngx_quic_compat_init(ngx_conf_
> int SSL_set_quic_method(SSL *ssl, const SSL_QUIC_METHOD *quic_method);
> int SSL_provide_quic_data(SSL *ssl, enum ssl_encryption_level_t level,
> const uint8_t *data, size_t len);
> -enum ssl_encryption_level_t SSL_quic_read_level(const SSL *ssl);
> -enum ssl_encryption_level_t SSL_quic_write_level(const SSL *ssl);
> int SSL_set_quic_transport_params(SSL *ssl, const uint8_t *params,
> size_t params_len);
> void SSL_get_peer_quic_transport_params(const SSL *ssl,
> diff --git a/src/event/quic/ngx_event_quic_ssl.c b/src/event/quic/ngx_event_quic_ssl.c
> --- a/src/event/quic/ngx_event_quic_ssl.c
> +++ b/src/event/quic/ngx_event_quic_ssl.c
> @@ -43,7 +43,8 @@ static int ngx_quic_add_handshake_data(n
> static int ngx_quic_flush_flight(ngx_ssl_conn_t *ssl_conn);
> static int ngx_quic_send_alert(ngx_ssl_conn_t *ssl_conn,
> enum ssl_encryption_level_t level, uint8_t alert);
> -static ngx_int_t ngx_quic_crypto_input(ngx_connection_t *c, ngx_chain_t *data);
> +static ngx_int_t ngx_quic_crypto_input(ngx_connection_t *c, ngx_chain_t *data,
> + enum ssl_encryption_level_t level);
>
>
> #if (NGX_QUIC_BORINGSSL_API)
> @@ -354,7 +355,7 @@ ngx_quic_handle_crypto_frame(ngx_connect
> }
>
> if (f->offset == ctx->crypto.offset) {
> - if (ngx_quic_crypto_input(c, frame->data) != NGX_OK) {
> + if (ngx_quic_crypto_input(c, frame->data, pkt->level) != NGX_OK) {
> return NGX_ERROR;
> }
>
> @@ -372,7 +373,7 @@ ngx_quic_handle_crypto_frame(ngx_connect
> cl = ngx_quic_read_buffer(c, &ctx->crypto, (uint64_t) -1);
>
> if (cl) {
> - if (ngx_quic_crypto_input(c, cl) != NGX_OK) {
> + if (ngx_quic_crypto_input(c, cl, pkt->level) != NGX_OK) {
> return NGX_ERROR;
> }
>
> @@ -384,7 +385,8 @@ ngx_quic_handle_crypto_frame(ngx_connect
>
>
> static ngx_int_t
> -ngx_quic_crypto_input(ngx_connection_t *c, ngx_chain_t *data)
> +ngx_quic_crypto_input(ngx_connection_t *c, ngx_chain_t *data,
> + enum ssl_encryption_level_t level)
> {
> int n, sslerr;
> ngx_buf_t *b;
> @@ -397,15 +399,10 @@ ngx_quic_crypto_input(ngx_connection_t *
>
> ssl_conn = c->ssl->connection;
>
> - ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> - "quic SSL_quic_read_level:%d SSL_quic_write_level:%d",
> - (int) SSL_quic_read_level(ssl_conn),
> - (int) SSL_quic_write_level(ssl_conn));
> -
> for (cl = data; cl; cl = cl->next) {
> b = cl->buf;
>
> - if (!SSL_provide_quic_data(ssl_conn, SSL_quic_read_level(ssl_conn),
> + if (!SSL_provide_quic_data(ssl_conn, level,
> b->pos, b->last - b->pos))
This can fit in one line now.
> {
> ngx_ssl_error(NGX_LOG_INFO, c->log, 0,
> @@ -416,11 +413,6 @@ ngx_quic_crypto_input(ngx_connection_t *
>
> n = SSL_do_handshake(ssl_conn);
>
> - ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> - "quic SSL_quic_read_level:%d SSL_quic_write_level:%d",
> - (int) SSL_quic_read_level(ssl_conn),
> - (int) SSL_quic_write_level(ssl_conn));
> -
> ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL_do_handshake: %d", n);
>
> if (n <= 0) {
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
Looks ok
More information about the nginx-devel
mailing list