[PATCH 6 of 8] QUIC: reusing crypto contexts for header protection

Roman Arutyunyan arut at nginx.com
Wed Sep 20 12:12:16 UTC 2023


Hi,

On Thu, Sep 07, 2023 at 07:13:58PM +0400, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1694099424 -14400
> #      Thu Sep 07 19:10:24 2023 +0400
> # Node ID cdc5b59309dbdc234c71e53fca142502884e6177
> # Parent  28f7491bc79771f9cfa882b1b5584fa48ea42e6b
> QUIC: reusing crypto contexts for header protection.
> 
> diff --git a/src/event/quic/ngx_event_quic_protection.c b/src/event/quic/ngx_event_quic_protection.c
> --- a/src/event/quic/ngx_event_quic_protection.c
> +++ b/src/event/quic/ngx_event_quic_protection.c
> @@ -28,8 +28,12 @@ static uint64_t ngx_quic_parse_pn(u_char
>  
>  static ngx_int_t ngx_quic_crypto_open(ngx_quic_secret_t *s, ngx_str_t *out,
>      u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log);
> -static ngx_int_t ngx_quic_crypto_hp(ngx_log_t *log, const EVP_CIPHER *cipher,
> -    ngx_quic_secret_t *s, u_char *out, u_char *in);
> +
> +static ngx_int_t ngx_quic_crypto_hp_init(const EVP_CIPHER *cipher,
> +    ngx_quic_secret_t *s, ngx_log_t *log);
> +static ngx_int_t ngx_quic_crypto_hp(ngx_quic_secret_t *s,
> +    u_char *out, u_char *in, ngx_log_t *log);
> +static void ngx_quic_crypto_hp_cleanup(ngx_quic_secret_t *s);
>  
>  static ngx_int_t ngx_quic_create_packet(ngx_quic_header_t *pkt,
>      ngx_str_t *res);
> @@ -192,6 +196,14 @@ ngx_quic_keys_set_initial_secret(ngx_qui
>          return NGX_ERROR;
>      }
>  
> +    if (ngx_quic_crypto_hp_init(ciphers.hp, client, log) == NGX_ERROR) {
> +        return NGX_ERROR;
> +    }
> +
> +    if (ngx_quic_crypto_hp_init(ciphers.hp, server, log) == NGX_ERROR) {
> +        return NGX_ERROR;
> +    }

Again, as before, in case of errors all ctx's created here should be freed,
since we don't always have a cleanup handler for them, see
ngx_quic_send_early_cc().

Also, in ngx_quic_send_early_cc() there's no cleanup at all, and ctx's will
leak in case of successful creation.

>      return NGX_OK;
>  }
>  
> @@ -561,53 +573,88 @@ ngx_quic_crypto_cleanup(ngx_quic_secret_
>  
>  
>  static ngx_int_t
> -ngx_quic_crypto_hp(ngx_log_t *log, const EVP_CIPHER *cipher,
> -    ngx_quic_secret_t *s, u_char *out, u_char *in)
> +ngx_quic_crypto_hp_init(const EVP_CIPHER *cipher, ngx_quic_secret_t *s,
> +    ngx_log_t *log)
>  {
> -    int              outlen;
>      EVP_CIPHER_CTX  *ctx;
> -    u_char           zero[NGX_QUIC_HP_LEN] = {0};
>  
>  #ifdef OPENSSL_IS_BORINGSSL
> -    uint32_t         cnt;
> -
> -    ngx_memcpy(&cnt, in, sizeof(uint32_t));
> -
> -    if (cipher == (const EVP_CIPHER *) EVP_aead_chacha20_poly1305()) {
> -        CRYPTO_chacha_20(out, zero, NGX_QUIC_HP_LEN, s->hp.data, &in[4], cnt);
> +    if (cipher == (EVP_CIPHER *) EVP_aead_chacha20_poly1305()) {
> +        /* some bogus value to distinguish ChaCha20 cipher */
> +        s->hp_ctx = (EVP_CIPHER_CTX *) cipher;

What if we use NULL as the special value?

>          return NGX_OK;
>      }
>  #endif
>  
>      ctx = EVP_CIPHER_CTX_new();
>      if (ctx == NULL) {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CIPHER_CTX_new() failed");
> +        return NGX_ERROR;
> +    }
> +
> +    if (EVP_EncryptInit_ex(ctx, cipher, NULL, s->hp.data, NULL) != 1) {
> +        EVP_CIPHER_CTX_free(ctx);
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptInit_ex() failed");
>          return NGX_ERROR;
>      }
>  
> -    if (EVP_EncryptInit_ex(ctx, cipher, NULL, s->hp.data, in) != 1) {
> +    s->hp_ctx = ctx;
> +    return NGX_OK;
> +}
> +
> +
> +static ngx_int_t
> +ngx_quic_crypto_hp(ngx_quic_secret_t *s, u_char *out, u_char *in,
> +    ngx_log_t *log)
> +{
> +    int              outlen;
> +    EVP_CIPHER_CTX  *ctx;
> +    u_char           zero[NGX_QUIC_HP_LEN] = {0};
> +
> +    ctx = s->hp_ctx;
> +
> +#ifdef OPENSSL_IS_BORINGSSL
> +    uint32_t         cnt;
> +
> +    if (ctx == (EVP_CIPHER_CTX *) EVP_aead_chacha20_poly1305()) {
> +        ngx_memcpy(&cnt, in, sizeof(uint32_t));
> +        CRYPTO_chacha_20(out, zero, NGX_QUIC_HP_LEN, s->hp.data, &in[4], cnt);
> +        return NGX_OK;
> +    }
> +#endif
> +
> +    if (EVP_EncryptInit_ex(ctx, NULL, NULL, NULL, in) != 1) {
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptInit_ex() failed");
> -        goto failed;
> +        return NGX_ERROR;
>      }
>  
>      if (!EVP_EncryptUpdate(ctx, out, &outlen, zero, NGX_QUIC_HP_LEN)) {
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
> -        goto failed;
> +        return NGX_ERROR;
>      }
>  
>      if (!EVP_EncryptFinal_ex(ctx, out + NGX_QUIC_HP_LEN, &outlen)) {
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptFinal_Ex() failed");
> -        goto failed;
> +        return NGX_ERROR;
>      }
>  
> -    EVP_CIPHER_CTX_free(ctx);
> -
>      return NGX_OK;
> +}
>  
> -failed:
> +
> +static void
> +ngx_quic_crypto_hp_cleanup(ngx_quic_secret_t *s)
> +{
>  
> -    EVP_CIPHER_CTX_free(ctx);
> +#ifdef OPENSSL_IS_BORINGSSL
> +    if (s->hp_ctx == (EVP_CIPHER_CTX *) EVP_aead_chacha20_poly1305()) {
> +        s->hp_ctx = NULL;
> +        return;
> +    }
> +#endif
>  
> -    return NGX_ERROR;
> +    EVP_CIPHER_CTX_free(s->hp_ctx);
> +    s->hp_ctx = NULL;
>  }
>  
>  
> @@ -668,6 +715,10 @@ ngx_quic_keys_set_encryption_secret(ngx_
>          return NGX_ERROR;
>      }
>  
> +    if (ngx_quic_crypto_hp_init(ciphers.hp, peer_secret, log) == NGX_ERROR) {
> +        return NGX_ERROR;
> +    }
> +
>      return NGX_OK;
>  }
>  
> @@ -695,6 +746,9 @@ ngx_quic_keys_discard(ngx_quic_keys_t *k
>  
>      ngx_quic_crypto_cleanup(client);
>      ngx_quic_crypto_cleanup(server);
> +
> +    ngx_quic_crypto_hp_cleanup(client);
> +    ngx_quic_crypto_hp_cleanup(server);
>  }
>  
>  
> @@ -747,11 +801,13 @@ ngx_quic_keys_update(ngx_event_t *ev)
>      next->client.key.len = current->client.key.len;
>      next->client.iv.len = NGX_QUIC_IV_LEN;
>      next->client.hp = current->client.hp;
> +    next->client.hp_ctx = current->client.hp_ctx;
>  
>      next->server.secret.len = current->server.secret.len;
>      next->server.key.len = current->server.key.len;
>      next->server.iv.len = NGX_QUIC_IV_LEN;
>      next->server.hp = current->server.hp;
> +    next->server.hp_ctx = current->server.hp_ctx;
>  
>      ngx_quic_hkdf_set(&seq[0], "tls13 quic ku",
>                        &next->client.secret, &current->client.secret);
> @@ -802,6 +858,9 @@ ngx_quic_keys_cleanup(void *data)
>          secrets = &keys->secrets[i];
>          ngx_quic_crypto_cleanup(&secrets->client);
>          ngx_quic_crypto_cleanup(&secrets->server);
> +
> +        ngx_quic_crypto_hp_cleanup(&secrets->client);
> +        ngx_quic_crypto_hp_cleanup(&secrets->server);
>      }
>  
>      secrets = &keys->next_key;
> @@ -848,9 +907,7 @@ ngx_quic_create_packet(ngx_quic_header_t
>      }
>  
>      sample = &out.data[4 - pkt->num_len];
> -    if (ngx_quic_crypto_hp(pkt->log, ciphers.hp, secret, mask, sample)
> -        != NGX_OK)
> -    {
> +    if (ngx_quic_crypto_hp(secret, mask, sample, pkt->log) != NGX_OK) {
>          return NGX_ERROR;
>      }
>  
> @@ -1077,9 +1134,7 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt,
>  
>      /* header protection */
>  
> -    if (ngx_quic_crypto_hp(pkt->log, ciphers.hp, secret, mask, sample)
> -        != NGX_OK)
> -    {
> +    if (ngx_quic_crypto_hp(secret, mask, sample, pkt->log) != NGX_OK) {
>          return NGX_DECLINED;
>      }
>  
> diff --git a/src/event/quic/ngx_event_quic_protection.h b/src/event/quic/ngx_event_quic_protection.h
> --- a/src/event/quic/ngx_event_quic_protection.h
> +++ b/src/event/quic/ngx_event_quic_protection.h
> @@ -51,6 +51,7 @@ typedef struct {
>      ngx_quic_iv_t             iv;
>      ngx_quic_md_t             hp;
>      ngx_quic_crypto_ctx_t    *ctx;
> +    EVP_CIPHER_CTX           *hp_ctx;
>  } ngx_quic_secret_t;
>  
>  
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Roman Arutyunyan


More information about the nginx-devel mailing list