[PATCH 5 of 8] QUIC: reusing crypto contexts for packet protection

Roman Arutyunyan arut at nginx.com
Tue Sep 19 13:53:43 UTC 2023


Hi,

On Thu, Sep 07, 2023 at 07:13:57PM +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 28f7491bc79771f9cfa882b1b5584fa48ea42e6b
> # Parent  24e5d652ecc861f0c68607d20941abbf3726fdf1
> QUIC: reusing crypto contexts for packet protection.
> 
> 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
> @@ -225,6 +225,7 @@ ngx_quic_new_connection(ngx_connection_t
>  {
>      ngx_uint_t              i;
>      ngx_quic_tp_t          *ctp;
> +    ngx_pool_cleanup_t     *cln;
>      ngx_quic_connection_t  *qc;
>  
>      qc = ngx_pcalloc(c->pool, sizeof(ngx_quic_connection_t));
> @@ -237,6 +238,14 @@ ngx_quic_new_connection(ngx_connection_t
>          return NULL;
>      }
>  
> +    cln = ngx_pool_cleanup_add(c->pool, 0);
> +    if (cln == NULL) {
> +        return NULL;
> +    }
> +
> +    cln->handler = ngx_quic_keys_cleanup;
> +    cln->data = qc->keys;

I think it's better to cleanup keys in ngx_quic_close_connection().
We do the same with sockets by calling ngx_quic_close_sockets().
We just have to carefully handle the errors later in this function and cleanup
keys when ngx_quic_open_sockets() fails.

>      qc->version = pkt->version;
>  
>      ngx_rbtree_init(&qc->streams.tree, &qc->streams.sentinel,
> 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
> @@ -54,9 +54,10 @@ struct ngx_quic_compat_s {
>  
>  
>  static void ngx_quic_compat_keylog_callback(const SSL *ssl, const char *line);
> -static ngx_int_t ngx_quic_compat_set_encryption_secret(ngx_log_t *log,
> +static ngx_int_t ngx_quic_compat_set_encryption_secret(ngx_connection_t *c,
>      ngx_quic_compat_keys_t *keys, enum ssl_encryption_level_t level,
>      const SSL_CIPHER *cipher, const uint8_t *secret, size_t secret_len);
> +static void ngx_quic_compat_cleanup_encryption_secret(void *data);
>  static int ngx_quic_compat_add_transport_params_callback(SSL *ssl,
>      unsigned int ext_type, unsigned int context, const unsigned char **out,
>      size_t *outlen, X509 *x, size_t chainidx, int *al, void *add_arg);
> @@ -214,14 +215,14 @@ ngx_quic_compat_keylog_callback(const SS
>          com->method->set_read_secret((SSL *) ssl, level, cipher, secret, n);
>          com->read_record = 0;
>  
> -        (void) ngx_quic_compat_set_encryption_secret(c->log, &com->keys, level,
> +        (void) ngx_quic_compat_set_encryption_secret(c, &com->keys, level,
>                                                       cipher, secret, n);
>      }
>  }
>  
>  
>  static ngx_int_t
> -ngx_quic_compat_set_encryption_secret(ngx_log_t *log,
> +ngx_quic_compat_set_encryption_secret(ngx_connection_t *c,
>      ngx_quic_compat_keys_t *keys, enum ssl_encryption_level_t level,
>      const SSL_CIPHER *cipher, const uint8_t *secret, size_t secret_len)
>  {
> @@ -231,6 +232,7 @@ ngx_quic_compat_set_encryption_secret(ng
>      ngx_quic_hkdf_t      seq[2];
>      ngx_quic_secret_t   *peer_secret;
>      ngx_quic_ciphers_t   ciphers;
> +    ngx_pool_cleanup_t  *cln;
>  
>      peer_secret = &keys->secret;
>  
> @@ -239,12 +241,12 @@ ngx_quic_compat_set_encryption_secret(ng
>      key_len = ngx_quic_ciphers(keys->cipher, &ciphers, level);
>  
>      if (key_len == NGX_ERROR) {
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "unexpected cipher");
> +        ngx_ssl_error(NGX_LOG_INFO, c->log, 0, "unexpected cipher");
>          return NGX_ERROR;
>      }
>  
>      if (sizeof(peer_secret->secret.data) < secret_len) {
> -        ngx_log_error(NGX_LOG_ALERT, log, 0,
> +        ngx_log_error(NGX_LOG_ALERT, c->log, 0,
>                        "unexpected secret len: %uz", secret_len);
>          return NGX_ERROR;
>      }
> @@ -262,15 +264,42 @@ ngx_quic_compat_set_encryption_secret(ng
>      ngx_quic_hkdf_set(&seq[1], "tls13 iv", &peer_secret->iv, &secret_str);
>  
>      for (i = 0; i < (sizeof(seq) / sizeof(seq[0])); i++) {
> -        if (ngx_quic_hkdf_expand(&seq[i], ciphers.d, log) != NGX_OK) {
> +        if (ngx_quic_hkdf_expand(&seq[i], ciphers.d, c->log) != NGX_OK) {
>              return NGX_ERROR;
>          }
>      }
>  
> +    ngx_quic_crypto_cleanup(peer_secret);
> +
> +    if (ngx_quic_crypto_init(ciphers.c, peer_secret, 1, c->log) == NGX_ERROR) {
> +        return NGX_ERROR;
> +    }
> +
> +    /* register cleanup handler once */
> +
> +    if (level == ssl_encryption_handshake) {

Does not look perfect, but I don't see a simpler and better solution.

> +        cln = ngx_pool_cleanup_add(c->pool, 0);
> +        if (cln == NULL) {

Cleanup peer_secret here?

Alternatively, move this block up.

> +            return NGX_ERROR;
> +        }
> +
> +        cln->handler = ngx_quic_compat_cleanup_encryption_secret;
> +        cln->data = peer_secret;
> +    }
> +
>      return NGX_OK;
>  }
>  
>  
> +static void
> +ngx_quic_compat_cleanup_encryption_secret(void *data)
> +{
> +    ngx_quic_secret_t *secret = data;
> +
> +    ngx_quic_crypto_cleanup(secret);
> +}
> +
> +
>  static int
>  ngx_quic_compat_add_transport_params_callback(SSL *ssl, unsigned int ext_type,
>      unsigned int context, const unsigned char **out, size_t *outlen, X509 *x,
> @@ -568,8 +597,7 @@ ngx_quic_compat_create_record(ngx_quic_c
>      ngx_memcpy(nonce, secret->iv.data, secret->iv.len);
>      ngx_quic_compute_nonce(nonce, sizeof(nonce), rec->number);
>  
> -    if (ngx_quic_crypto_seal(ciphers.c, secret, &out,
> -                             nonce, &rec->payload, &ad, rec->log)
> +    if (ngx_quic_crypto_seal(secret, &out, nonce, &rec->payload, &ad, rec->log)
>          != NGX_OK)
>      {
>          return NGX_ERROR;
> 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
> @@ -26,9 +26,8 @@ static ngx_int_t ngx_hkdf_extract(u_char
>  static uint64_t ngx_quic_parse_pn(u_char **pos, ngx_int_t len, u_char *mask,
>      uint64_t *largest_pn);
>  
> -static ngx_int_t ngx_quic_crypto_open(const ngx_quic_cipher_t *cipher,
> -    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_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);
>  
> @@ -108,13 +107,14 @@ ngx_int_t
>  ngx_quic_keys_set_initial_secret(ngx_quic_keys_t *keys, ngx_str_t *secret,
>      ngx_log_t *log)
>  {
> -    size_t              is_len;
> -    uint8_t             is[SHA256_DIGEST_LENGTH];
> -    ngx_str_t           iss;
> -    ngx_uint_t          i;
> -    const EVP_MD       *digest;
> -    ngx_quic_hkdf_t     seq[8];
> -    ngx_quic_secret_t  *client, *server;
> +    size_t               is_len;
> +    uint8_t              is[SHA256_DIGEST_LENGTH];
> +    ngx_str_t            iss;
> +    ngx_uint_t           i;
> +    const EVP_MD        *digest;
> +    ngx_quic_hkdf_t      seq[8];
> +    ngx_quic_secret_t   *client, *server;
> +    ngx_quic_ciphers_t   ciphers;
>  
>      static const uint8_t salt[20] =
>          "\x38\x76\x2c\xf7\xf5\x59\x34\xb3\x4d\x17"
> @@ -180,6 +180,18 @@ ngx_quic_keys_set_initial_secret(ngx_qui
>          }
>      }
>  
> +    if (ngx_quic_ciphers(0, &ciphers, ssl_encryption_initial) == NGX_ERROR) {
> +        return NGX_ERROR;
> +    }
> +
> +    if (ngx_quic_crypto_init(ciphers.c, client, 0, log) == NGX_ERROR) {
> +        return NGX_ERROR;
> +    }
> +
> +    if (ngx_quic_crypto_init(ciphers.c, server, 1, log) == NGX_ERROR) {

Call ngx_quic_crypto_cleanup() for client here?
This function is called from ngx_quic_send_early_cc(), which has no keys
cleanup handler (and I propose we remove it from regular QUIC connections as
well).

> +        return NGX_ERROR;
> +    }
> +
>      return NGX_OK;
>  }
>  
> @@ -343,9 +355,9 @@ failed:
>  }
>  
>  
> -static ngx_int_t
> -ngx_quic_crypto_open(const ngx_quic_cipher_t *cipher, ngx_quic_secret_t *s,
> -    ngx_str_t *out, u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log)
> +ngx_int_t
> +ngx_quic_crypto_init(const ngx_quic_cipher_t *cipher, ngx_quic_secret_t *s,
> +    ngx_int_t enc, ngx_log_t *log)
>  {
>  
>  #ifdef OPENSSL_IS_BORINGSSL
> @@ -357,19 +369,7 @@ ngx_quic_crypto_open(const ngx_quic_ciph
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_AEAD_CTX_new() failed");
>          return NGX_ERROR;
>      }
> -
> -    if (EVP_AEAD_CTX_open(ctx, out->data, &out->len, out->len, nonce, s->iv.len,
> -                          in->data, in->len, ad->data, ad->len)
> -        != 1)
> -    {
> -        EVP_AEAD_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_AEAD_CTX_open() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    EVP_AEAD_CTX_free(ctx);
>  #else
> -    int              len;
>      EVP_CIPHER_CTX  *ctx;
>  
>      ctx = EVP_CIPHER_CTX_new();
> @@ -378,114 +378,9 @@ ngx_quic_crypto_open(const ngx_quic_ciph
>          return NGX_ERROR;
>      }
>  
> -    if (EVP_DecryptInit_ex(ctx, cipher, NULL, NULL, NULL) != 1) {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptInit_ex() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    in->len -= NGX_QUIC_TAG_LEN;
> -
> -    if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, NGX_QUIC_TAG_LEN,
> -                            in->data + in->len)
> -        == 0)
> -    {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0,
> -                      "EVP_CIPHER_CTX_ctrl(EVP_CTRL_AEAD_SET_TAG) failed");
> -        return NGX_ERROR;
> -    }
> -
> -    if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_IVLEN, s->iv.len, NULL)
> -        == 0)
> -    {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0,
> -                      "EVP_CIPHER_CTX_ctrl(EVP_CTRL_AEAD_SET_IVLEN) failed");
> -        return NGX_ERROR;
> -    }
> -
> -    if (EVP_DecryptInit_ex(ctx, NULL, NULL, s->key.data, nonce) != 1) {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptInit_ex() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    if (EVP_CIPHER_mode(cipher) == EVP_CIPH_CCM_MODE
> -        && EVP_DecryptUpdate(ctx, NULL, &len, NULL, in->len) != 1)
> -    {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptUpdate() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    if (EVP_DecryptUpdate(ctx, NULL, &len, ad->data, ad->len) != 1) {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptUpdate() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    if (EVP_DecryptUpdate(ctx, out->data, &len, in->data, in->len) != 1) {
> +    if (EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, enc) != 1) {
>          EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptUpdate() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    out->len = len;
> -
> -    if (EVP_DecryptFinal_ex(ctx, out->data + out->len, &len) <= 0) {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptFinal_ex failed");
> -        return NGX_ERROR;
> -    }
> -
> -    out->len += len;
> -
> -    EVP_CIPHER_CTX_free(ctx);
> -#endif
> -
> -    return NGX_OK;
> -}
> -
> -
> -ngx_int_t
> -ngx_quic_crypto_seal(const ngx_quic_cipher_t *cipher, ngx_quic_secret_t *s,
> -    ngx_str_t *out, u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log)
> -{
> -
> -#ifdef OPENSSL_IS_BORINGSSL
> -    EVP_AEAD_CTX  *ctx;
> -
> -    ctx = EVP_AEAD_CTX_new(cipher, s->key.data, s->key.len,
> -                           EVP_AEAD_DEFAULT_TAG_LENGTH);
> -    if (ctx == NULL) {
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_AEAD_CTX_new() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    if (EVP_AEAD_CTX_seal(ctx, out->data, &out->len, out->len, nonce, s->iv.len,
> -                          in->data, in->len, ad->data, ad->len)
> -        != 1)
> -    {
> -        EVP_AEAD_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_AEAD_CTX_seal() failed");
> -        return NGX_ERROR;
> -    }
> -
> -    EVP_AEAD_CTX_free(ctx);
> -#else
> -    int              len;
> -    EVP_CIPHER_CTX  *ctx;
> -
> -    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, NULL, NULL) != 1) {
> -        EVP_CIPHER_CTX_free(ctx);
> -        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptInit_ex() failed");
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CipherInit_ex() failed");
>          return NGX_ERROR;
>      }
>  
> @@ -509,28 +404,121 @@ ngx_quic_crypto_seal(const ngx_quic_ciph
>          return NGX_ERROR;
>      }
>  
> -    if (EVP_EncryptInit_ex(ctx, NULL, NULL, s->key.data, nonce) != 1) {
> +    if (EVP_CipherInit_ex(ctx, NULL, NULL, s->key.data, NULL, enc) != 1) {
>          EVP_CIPHER_CTX_free(ctx);
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CipherInit_ex() failed");
> +        return NGX_ERROR;
> +    }
> +#endif
> +
> +    s->ctx = ctx;
> +    return NGX_OK;
> +}
> +
> +
> +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)
> +{
> +    ngx_quic_crypto_ctx_t  *ctx;
> +
> +    ctx = s->ctx;
> +
> +#ifdef OPENSSL_IS_BORINGSSL
> +    if (EVP_AEAD_CTX_open(ctx, out->data, &out->len, out->len, nonce, s->iv.len,
> +                          in->data, in->len, ad->data, ad->len)
> +        != 1)
> +    {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_AEAD_CTX_open() failed");
> +        return NGX_ERROR;
> +    }
> +#else
> +    int  len;
> +
> +    if (EVP_DecryptInit_ex(ctx, NULL, NULL, NULL, nonce) != 1) {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptInit_ex() failed");
> +        return NGX_ERROR;
> +    }
> +
> +    in->len -= NGX_QUIC_TAG_LEN;
> +
> +    if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, NGX_QUIC_TAG_LEN,
> +                            in->data + in->len)
> +        == 0)
> +    {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0,
> +                      "EVP_CIPHER_CTX_ctrl(EVP_CTRL_AEAD_SET_TAG) failed");
> +        return NGX_ERROR;
> +    }
> +
> +    if (EVP_CIPHER_mode(EVP_CIPHER_CTX_cipher(ctx)) == EVP_CIPH_CCM_MODE
> +        && EVP_DecryptUpdate(ctx, NULL, &len, NULL, in->len) != 1)
> +    {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptUpdate() failed");
> +        return NGX_ERROR;
> +    }
> +
> +    if (EVP_DecryptUpdate(ctx, NULL, &len, ad->data, ad->len) != 1) {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptUpdate() failed");
> +        return NGX_ERROR;
> +    }
> +
> +    if (EVP_DecryptUpdate(ctx, out->data, &len, in->data, in->len) != 1) {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptUpdate() failed");
> +        return NGX_ERROR;
> +    }
> +
> +    out->len = len;
> +
> +    if (EVP_DecryptFinal_ex(ctx, out->data + out->len, &len) <= 0) {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_DecryptFinal_ex failed");
> +        return NGX_ERROR;
> +    }
> +
> +    out->len += len;
> +#endif
> +
> +    return NGX_OK;
> +}
> +
> +
> +ngx_int_t
> +ngx_quic_crypto_seal(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce,
> +    ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log)
> +{
> +    ngx_quic_crypto_ctx_t  *ctx;
> +
> +    ctx = s->ctx;
> +
> +#ifdef OPENSSL_IS_BORINGSSL
> +    if (EVP_AEAD_CTX_seal(ctx, out->data, &out->len, out->len, nonce, s->iv.len,
> +                          in->data, in->len, ad->data, ad->len)
> +        != 1)
> +    {
> +        ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_AEAD_CTX_seal() failed");
> +        return NGX_ERROR;
> +    }
> +#else
> +    int  len;
> +
> +    if (EVP_EncryptInit_ex(ctx, NULL, NULL, NULL, nonce) != 1) {
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptInit_ex() failed");
>          return NGX_ERROR;
>      }
>  
> -    if (EVP_CIPHER_mode(cipher) == EVP_CIPH_CCM_MODE
> +    if (EVP_CIPHER_mode(EVP_CIPHER_CTX_cipher(ctx)) == EVP_CIPH_CCM_MODE
>          && EVP_EncryptUpdate(ctx, NULL, &len, NULL, in->len) != 1)
>      {
> -        EVP_CIPHER_CTX_free(ctx);
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
>          return NGX_ERROR;
>      }
>  
>      if (EVP_EncryptUpdate(ctx, NULL, &len, ad->data, ad->len) != 1) {
> -        EVP_CIPHER_CTX_free(ctx);
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
>          return NGX_ERROR;
>      }
>  
>      if (EVP_EncryptUpdate(ctx, out->data, &len, in->data, in->len) != 1) {
> -        EVP_CIPHER_CTX_free(ctx);
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
>          return NGX_ERROR;
>      }
> @@ -538,7 +526,6 @@ ngx_quic_crypto_seal(const ngx_quic_ciph
>      out->len = len;
>  
>      if (EVP_EncryptFinal_ex(ctx, out->data + out->len, &len) <= 0) {
> -        EVP_CIPHER_CTX_free(ctx);
>          ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptFinal_ex failed");
>          return NGX_ERROR;
>      }
> @@ -549,21 +536,30 @@ ngx_quic_crypto_seal(const ngx_quic_ciph
>                              out->data + out->len)
>          == 0)
>      {
> -        EVP_CIPHER_CTX_free(ctx);
>          ngx_ssl_error(NGX_LOG_INFO, log, 0,
>                        "EVP_CIPHER_CTX_ctrl(EVP_CTRL_AEAD_GET_TAG) failed");
>          return NGX_ERROR;
>      }
>  
>      out->len += NGX_QUIC_TAG_LEN;
> -
> -    EVP_CIPHER_CTX_free(ctx);
>  #endif
>  
>      return NGX_OK;
>  }

Now that we have universal ngx_quic_crypto_open() which receives "enc", it's
tempting more than ever to combine ngx_quic_crypto_seal() and
ngx_quic_crypto_open() in a single function with "enc".  Not a part of this
work though.

> +void
> +ngx_quic_crypto_cleanup(ngx_quic_secret_t *s)
> +{

Although we know these functions ignore a NULL argument, I think the code
would still look better under "if (s->ctx) {}".

> +#ifdef OPENSSL_IS_BORINGSSL
> +    EVP_AEAD_CTX_free(s->ctx);
> +#else
> +    EVP_CIPHER_CTX_free(s->ctx);
> +#endif
> +    s->ctx = NULL;
> +}
> +
> +
>  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)
> @@ -666,6 +662,12 @@ ngx_quic_keys_set_encryption_secret(ngx_
>          }
>      }
>  
> +    if (ngx_quic_crypto_init(ciphers.c, peer_secret, is_write, log)
> +        == NGX_ERROR)
> +    {
> +        return NGX_ERROR;
> +    }
> +
>      return NGX_OK;
>  }
>  
> @@ -675,10 +677,10 @@ ngx_quic_keys_available(ngx_quic_keys_t 
>      enum ssl_encryption_level_t level, ngx_uint_t is_write)
>  {
>      if (is_write == 0) {
> -        return keys->secrets[level].client.key.len != 0;
> +        return keys->secrets[level].client.ctx != NULL;
>      }
>  
> -    return keys->secrets[level].server.key.len != 0;
> +    return keys->secrets[level].server.ctx != 0;

Maybe != NULL ?

>  }
>  
>  
> @@ -686,8 +688,13 @@ void
>  ngx_quic_keys_discard(ngx_quic_keys_t *keys,
>      enum ssl_encryption_level_t level)
>  {
> -    keys->secrets[level].client.key.len = 0;
> -    keys->secrets[level].server.key.len = 0;
> +    ngx_quic_secret_t  *client, *server;
> +
> +    client = &keys->secrets[level].client;
> +    server = &keys->secrets[level].server;
> +
> +    ngx_quic_crypto_cleanup(client);
> +    ngx_quic_crypto_cleanup(server);
>  }
>  
>  
> @@ -699,6 +706,9 @@ ngx_quic_keys_switch(ngx_connection_t *c
>      current = &keys->secrets[ssl_encryption_application];
>      next = &keys->next_key;
>  
> +    ngx_quic_crypto_cleanup(&current->client);
> +    ngx_quic_crypto_cleanup(&current->server);
> +
>      tmp = *current;
>      *current = *next;
>      *next = tmp;
> @@ -762,6 +772,16 @@ ngx_quic_keys_update(ngx_event_t *ev)
>          }
>      }
>  
> +    if (ngx_quic_crypto_init(ciphers.c, &next->client, 0, c->log) == NGX_ERROR)
> +    {
> +        goto failed;
> +    }
> +
> +    if (ngx_quic_crypto_init(ciphers.c, &next->server, 1, c->log) == NGX_ERROR)
> +    {
> +        goto failed;
> +    }
> +
>      return;
>  
>  failed:
> @@ -770,6 +790,26 @@ failed:
>  }
>  
>  
> +void
> +ngx_quic_keys_cleanup(void *data)
> +{
> +    ngx_quic_keys_t *keys = data;
> +
> +    size_t               i;
> +    ngx_quic_secrets_t  *secrets;
> +
> +    for (i = 0; i < NGX_QUIC_ENCRYPTION_LAST; i++) {
> +        secrets = &keys->secrets[i];
> +        ngx_quic_crypto_cleanup(&secrets->client);
> +        ngx_quic_crypto_cleanup(&secrets->server);
> +    }
> +
> +    secrets = &keys->next_key;
> +    ngx_quic_crypto_cleanup(&secrets->client);
> +    ngx_quic_crypto_cleanup(&secrets->server);
> +}
> +
> +
>  static ngx_int_t
>  ngx_quic_create_packet(ngx_quic_header_t *pkt, ngx_str_t *res)
>  {
> @@ -801,8 +841,7 @@ ngx_quic_create_packet(ngx_quic_header_t
>      ngx_memcpy(nonce, secret->iv.data, secret->iv.len);
>      ngx_quic_compute_nonce(nonce, sizeof(nonce), pkt->number);
>  
> -    if (ngx_quic_crypto_seal(ciphers.c, secret, &out,
> -                             nonce, &pkt->payload, &ad, pkt->log)
> +    if (ngx_quic_crypto_seal(secret, &out, nonce, &pkt->payload, &ad, pkt->log)
>          != NGX_OK)
>      {
>          return NGX_ERROR;
> @@ -862,13 +901,18 @@ ngx_quic_create_retry_packet(ngx_quic_he
>      ngx_memcpy(secret.key.data, key, sizeof(key));
>      secret.iv.len = NGX_QUIC_IV_LEN;
>  
> -    if (ngx_quic_crypto_seal(ciphers.c, &secret, &itag, nonce, &in, &ad,
> -                             pkt->log)
> +    if (ngx_quic_crypto_init(ciphers.c, &secret, 1, pkt->log) == NGX_ERROR) {
> +        return NGX_ERROR;
> +    }
> +
> +    if (ngx_quic_crypto_seal(&secret, &itag, nonce, &in, &ad, pkt->log)
>          != NGX_OK)
>      {

Need to call ngx_quic_crypto_cleanup() here.

>          return NGX_ERROR;
>      }
>  
> +    ngx_quic_crypto_cleanup(&secret);
> +
>      res->len = itag.data + itag.len - start;
>      res->data = start;
>  
> @@ -999,7 +1043,7 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt,
>      u_char              *p, *sample;
>      size_t               len;
>      uint64_t             pn, lpn;
> -    ngx_int_t            pnl, rc;
> +    ngx_int_t            pnl;
>      ngx_str_t            in, ad;
>      ngx_uint_t           key_phase;
>      ngx_quic_secret_t   *secret;
> @@ -1088,9 +1132,9 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt,
>      pkt->payload.len = in.len - NGX_QUIC_TAG_LEN;
>      pkt->payload.data = pkt->plaintext + ad.len;
>  
> -    rc = ngx_quic_crypto_open(ciphers.c, secret, &pkt->payload,
> -                              nonce, &in, &ad, pkt->log);
> -    if (rc != NGX_OK) {
> +    if (ngx_quic_crypto_open(secret, &pkt->payload, nonce, &in, &ad, 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
> @@ -26,8 +26,10 @@
>  
>  #ifdef OPENSSL_IS_BORINGSSL
>  #define ngx_quic_cipher_t             EVP_AEAD
> +#define ngx_quic_crypto_ctx_t         EVP_AEAD_CTX
>  #else
>  #define ngx_quic_cipher_t             EVP_CIPHER
> +#define ngx_quic_crypto_ctx_t         EVP_CIPHER_CTX
>  #endif
>  
>  
> @@ -48,6 +50,7 @@ typedef struct {
>      ngx_quic_md_t             key;
>      ngx_quic_iv_t             iv;
>      ngx_quic_md_t             hp;
> +    ngx_quic_crypto_ctx_t    *ctx;
>  } ngx_quic_secret_t;
>  
>  
> @@ -100,14 +103,17 @@ void ngx_quic_keys_discard(ngx_quic_keys
>      enum ssl_encryption_level_t level);
>  void ngx_quic_keys_switch(ngx_connection_t *c, ngx_quic_keys_t *keys);
>  void ngx_quic_keys_update(ngx_event_t *ev);
> +void ngx_quic_keys_cleanup(void *data);
>  ngx_int_t ngx_quic_encrypt(ngx_quic_header_t *pkt, ngx_str_t *res);
>  ngx_int_t ngx_quic_decrypt(ngx_quic_header_t *pkt, uint64_t *largest_pn);
>  void ngx_quic_compute_nonce(u_char *nonce, size_t len, uint64_t pn);
>  ngx_int_t ngx_quic_ciphers(ngx_uint_t id, ngx_quic_ciphers_t *ciphers,
>      enum ssl_encryption_level_t level);
> -ngx_int_t ngx_quic_crypto_seal(const ngx_quic_cipher_t *cipher,
> -    ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce, ngx_str_t *in,
> -    ngx_str_t *ad, ngx_log_t *log);
> +ngx_int_t ngx_quic_crypto_init(const ngx_quic_cipher_t *cipher,
> +    ngx_quic_secret_t *s, ngx_int_t enc, ngx_log_t *log);
> +ngx_int_t ngx_quic_crypto_seal(ngx_quic_secret_t *s, ngx_str_t *out,
> +    u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log);
> +void ngx_quic_crypto_cleanup(ngx_quic_secret_t *s);
>  ngx_int_t ngx_quic_hkdf_expand(ngx_quic_hkdf_t *hkdf, const EVP_MD *digest,
>      ngx_log_t *log);
>  
> _______________________________________________
> 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