Roman Arutyunyan
September 19, 2023 09:54AM
Hi,

On Thu, Sep 07, 2023 at 07:13:57PM +0400, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov <pluknet@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@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Roman Arutyunyan
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH 0 of 8] [quic] reusing crypto contexts, and more

Sergey Kandaurov 485 September 07, 2023 11:18AM

[PATCH 1 of 8] QUIC: split keys availability checks to read and write sides

Sergey Kandaurov 86 September 07, 2023 11:18AM

Re: [PATCH 1 of 8] QUIC: split keys availability checks to read and write sides

Roman Arutyunyan 85 September 21, 2023 09:30AM

[PATCH 2 of 8] QUIC: added check to prevent packet output with discarded keys

Sergey Kandaurov 82 September 07, 2023 11:18AM

Re: [PATCH 2 of 8] QUIC: added check to prevent packet output with discarded keys

Roman Arutyunyan 89 September 18, 2023 03:10AM

Re: [PATCH 2 of 8] QUIC: added check to prevent packet output with discarded keys

Sergey Kandaurov 100 October 13, 2023 11:10AM

[PATCH 3 of 8] QUIC: prevented output of ACK frame when discarding handshake keys

Sergey Kandaurov 80 September 07, 2023 11:18AM

[PATCH 4 of 8] QUIC: renamed protection functions

Sergey Kandaurov 80 September 07, 2023 11:18AM

Re: [PATCH 4 of 8] QUIC: renamed protection functions

Roman Arutyunyan 88 September 21, 2023 09:32AM

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

Sergey Kandaurov 85 September 07, 2023 11:18AM

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

Roman Arutyunyan 101 September 19, 2023 09:54AM

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

Sergey Kandaurov 85 October 13, 2023 11:14AM

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

Sergey Kandaurov 80 October 17, 2023 06:40AM

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

Sergey Kandaurov 97 October 23, 2023 06:38PM

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

Sergey Kandaurov 81 September 07, 2023 11:18AM

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

Roman Arutyunyan 80 September 20, 2023 08:14AM

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

Sergey Kandaurov 75 October 13, 2023 11:14AM

[PATCH 7 of 8] QUIC: cleaned up now unused ngx_quic_ciphers() calls

Sergey Kandaurov 86 September 07, 2023 11:18AM

Re: [PATCH 7 of 8] QUIC: cleaned up now unused ngx_quic_ciphers() calls

Roman Arutyunyan 95 September 20, 2023 08:28AM

Re: [PATCH 7 of 8] QUIC: cleaned up now unused ngx_quic_ciphers() calls

Sergey Kandaurov 87 October 13, 2023 11:16AM

[PATCH 8 of 8] QUIC: explicitly zero out unused keying material

Sergey Kandaurov 79 September 07, 2023 11:18AM

Re: [PATCH 8 of 8] QUIC: explicitly zero out unused keying material

Roman Arutyunyan 83 September 21, 2023 09:30AM

Re: [PATCH 8 of 8] QUIC: explicitly zero out unused keying material

Sergey Kandaurov 74 October 13, 2023 11:16AM

[PATCH 00 of 11] [quic] reusing crypto contexts, and more #2

Sergey Kandaurov 80 October 18, 2023 11:28AM

[PATCH 01 of 11] QUIC: split keys availability checks to read and write sides

Sergey Kandaurov 76 October 18, 2023 11:28AM

[PATCH 02 of 11] QUIC: added safety belt to prevent using discarded keys

Sergey Kandaurov 78 October 18, 2023 11:28AM

[PATCH 03 of 11] QUIC: prevented generating ACK frames with discarded keys

Sergey Kandaurov 80 October 18, 2023 11:28AM

[PATCH 04 of 11] QUIC: renamed protection functions

Sergey Kandaurov 73 October 18, 2023 11:28AM

[PATCH 05 of 11] QUIC: reusing crypto contexts for packet protection

Sergey Kandaurov 76 October 18, 2023 11:28AM

[PATCH 06 of 11] QUIC: common code for crypto open and seal operations

Sergey Kandaurov 76 October 18, 2023 11:28AM

[PATCH 07 of 11] QUIC: reusing crypto contexts for header protection

Sergey Kandaurov 74 October 18, 2023 11:30AM

[PATCH 08 of 11] QUIC: cleaned up now unused ngx_quic_ciphers() calls

Sergey Kandaurov 73 October 18, 2023 11:30AM

[PATCH 09 of 11] QUIC: simplified ngx_quic_ciphers() API

Sergey Kandaurov 72 October 18, 2023 11:30AM

[PATCH 10 of 11] QUIC: removed key field from ngx_quic_secret_t

Sergey Kandaurov 76 October 18, 2023 11:30AM

[PATCH 11 of 11] QUIC: explicitly zero out unused keying material

Sergey Kandaurov 74 October 18, 2023 11:38AM

Re: [PATCH 00 of 11] [quic] reusing crypto contexts, and more #2

Roman Arutyunyan 81 October 20, 2023 03:28AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 308
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready