Welcome! Log In Create A New Profile

Advanced

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

Sergey Kandaurov
October 13, 2023 11:14AM
> On 19 Sep 2023, at 17:53, Roman Arutyunyan <arut@nginx.com> wrote:
>
> 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.

While this may look error prone compared with the cleanup handler,
you convinced me to remove it because of ngx_quic_send_early_cc().
To be merged:

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
@@ -227,7 +227,6 @@ 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));
@@ -240,14 +239,6 @@ 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;
-
qc->version = pkt->version;

ngx_rbtree_init(&qc->streams.tree, &qc->streams.sentinel,
@@ -344,6 +335,7 @@ ngx_quic_new_connection(ngx_connection_t
qc->validated = pkt->validated;

if (ngx_quic_open_sockets(c, qc, pkt) != NGX_OK) {
+ ngx_quic_keys_cleanup(qc->keys);
return NULL;
}

@@ -594,6 +586,8 @@ ngx_quic_close_connection(ngx_connection

ngx_quic_close_sockets(c);

+ ngx_quic_keys_cleanup(qc->keys);
+
ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0, "quic close completed");

/* may be tested from SSL callback during SSL shutdown */
diff --git a/src/event/quic/ngx_event_quic_output.c b/src/event/quic/ngx_event_quic_output.c
--- a/src/event/quic/ngx_event_quic_output.c
+++ b/src/event/quic/ngx_event_quic_output.c
@@ -941,13 +941,17 @@ ngx_quic_send_early_cc(ngx_connection_t
res.data = dst;

if (ngx_quic_encrypt(&pkt, &res) != NGX_OK) {
+ ngx_quic_keys_cleanup(pkt.keys);
return NGX_ERROR;
}

if (ngx_quic_send(c, res.data, res.len, c->sockaddr, c->socklen) < 0) {
+ ngx_quic_keys_cleanup(pkt.keys);
return NGX_ERROR;
}

+ ngx_quic_keys_cleanup(pkt.keys);
+
return NGX_DONE;
}

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
@@ -189,10 +189,16 @@ ngx_quic_keys_set_initial_secret(ngx_qui
}

if (ngx_quic_crypto_init(ciphers.c, server, 1, log) == NGX_ERROR) {
- return NGX_ERROR;
+ goto failed;
}

return NGX_OK;
+
+failed:
+
+ ngx_quic_keys_cleanup(keys);
+
+ return NGX_ERROR;
}


@@ -793,10 +799,8 @@ failed:


void
-ngx_quic_keys_cleanup(void *data)
+ngx_quic_keys_cleanup(ngx_quic_keys_t *keys)
{
- ngx_quic_keys_t *keys = data;
-
size_t i;
ngx_quic_secrets_t *secrets;

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
@@ -103,7 +103,7 @@ 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);
+void ngx_quic_keys_cleanup(ngx_quic_keys_t *keys);
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);


>
>> 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.

I don't see either, without introducing some state (see below).

>
>> + cln = ngx_pool_cleanup_add(c->pool, 0);
>> + if (cln == NULL) {
>
> Cleanup peer_secret here?
>
> Alternatively, move this block up.

Fixed, thanks.

With introducing keys->cleanup:

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
@@ -25,6 +25,7 @@
typedef struct {
ngx_quic_secret_t secret;
ngx_uint_t cipher;
+ ngx_uint_t cleanup; /* unsigned cleanup:1 */
} ngx_quic_compat_keys_t;


@@ -269,15 +270,11 @@ ngx_quic_compat_set_encryption_secret(ng
}
}

- 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) {
+ if (!keys->cleanup) {
+ keys->cleanup = 1;
+
cln = ngx_pool_cleanup_add(c->pool, 0);
if (cln == NULL) {
return NGX_ERROR;
@@ -287,6 +284,12 @@ ngx_quic_compat_set_encryption_secret(ng
cln->data = peer_secret;
}

+ ngx_quic_crypto_cleanup(peer_secret);
+
+ if (ngx_quic_crypto_init(ciphers.c, peer_secret, 1, c->log) == NGX_ERROR) {
+ return NGX_ERROR;
+ }
+
return NGX_OK;
}


>
>> + 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?

Yes, it is needed for ngx_quic_send_early_cc().
Added.

> 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).

See the above patch.

>
>> + 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.

I'm a little concerned this can impair code readability and
slightly decrease efficiency due to additional branching
(though it's compensated with branching removal in other places,
the net effect should be negligible).
Other than that I don't feel why it can't be combined
at least for the OpenSSL part.

# HG changeset patch
# User Sergey Kandaurov <pluknet@nginx.com>
# Date 1697131103 -14400
# Thu Oct 12 21:18:23 2023 +0400
# Node ID d6dc53fe48b3522954cc9ec6ac949354d0aae512
# Parent 7055f7a2992f9440a27952f1b60ed1f606aea77e
QUIC: common code for crypto open and seal operations.

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,6 +28,10 @@ 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);
+#ifndef OPENSSL_IS_BORINGSSL
+static ngx_int_t ngx_quic_crypto_common(ngx_quic_secret_t *s, ngx_str_t *out,
+ u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log);
+#endif
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);

@@ -420,65 +424,19 @@ 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)
+ if (EVP_AEAD_CTX_open(s->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;
+#else
+ return ngx_quic_crypto_common(s, out, nonce, in, ad, log);
+#endif
}


@@ -486,67 +444,96 @@ 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)
+ if (EVP_AEAD_CTX_seal(s->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;
}
+
+ return NGX_OK;
#else
- int len;
+ return ngx_quic_crypto_common(s, out, nonce, in, ad, log);
+#endif
+}
+
+
+#ifndef OPENSSL_IS_BORINGSSL
+
+static ngx_int_t
+ngx_quic_crypto_common(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce,
+ ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log)
+{
+ int len, enc;
+ ngx_quic_crypto_ctx_t *ctx;

- if (EVP_EncryptInit_ex(ctx, NULL, NULL, NULL, nonce) != 1) {
- ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptInit_ex() failed");
+ ctx = s->ctx;
+ enc = EVP_CIPHER_CTX_encrypting(ctx);
+
+ if (EVP_CipherInit_ex(ctx, NULL, NULL, NULL, nonce, enc) != 1) {
+ ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CipherInit_ex() failed");
return NGX_ERROR;
}

+ if (enc == 0) {
+ 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_EncryptUpdate(ctx, NULL, &len, NULL, in->len) != 1)
+ && EVP_CipherUpdate(ctx, NULL, &len, NULL, in->len) != 1)
{
- ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
+ ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CipherUpdate() failed");
return NGX_ERROR;
}

- if (EVP_EncryptUpdate(ctx, NULL, &len, ad->data, ad->len) != 1) {
- ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
+ if (EVP_CipherUpdate(ctx, NULL, &len, ad->data, ad->len) != 1) {
+ ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CipherUpdate() failed");
return NGX_ERROR;
}

- if (EVP_EncryptUpdate(ctx, out->data, &len, in->data, in->len) != 1) {
- ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
+ if (EVP_CipherUpdate(ctx, out->data, &len, in->data, in->len) != 1) {
+ ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CipherUpdate() failed");
return NGX_ERROR;
}

out->len = len;

- if (EVP_EncryptFinal_ex(ctx, out->data + out->len, &len) <= 0) {
- ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptFinal_ex failed");
+ if (EVP_CipherFinal_ex(ctx, out->data + out->len, &len) <= 0) {
+ ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CipherFinal_ex failed");
return NGX_ERROR;
}

out->len += len;

- if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, NGX_QUIC_TAG_LEN,
- out->data + out->len)
- == 0)
- {
- ngx_ssl_error(NGX_LOG_INFO, log, 0,
- "EVP_CIPHER_CTX_ctrl(EVP_CTRL_AEAD_GET_TAG) failed");
- return NGX_ERROR;
+ if (enc == 1) {
+ if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, NGX_QUIC_TAG_LEN,
+ out->data + out->len)
+ == 0)
+ {
+ 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;
}

- out->len += NGX_QUIC_TAG_LEN;
-#endif
-
return NGX_OK;
}

+#endif
+

void
ngx_quic_crypto_cleanup(ngx_quic_secret_t *s)


>
>> +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) {}".
>

Indeed, it's made with the knowledge that both API have a check for NULL.
Added the check, I've no strict preference on this.

>> +#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 ?

Fixed, tnx.

>
>> }
>>
>>
>> @@ -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.

Fixed, tnx.

I was pondering on reusing a static crypto context
to make generating Retry packets more lightweight.
Known fixed values for key and nonce make it possible to create
a single context and reuse it over all Retry packets.

Note that the context memory is kept for reuse after the first
retry, it will be freed eventually on process exit,
the operating system will take care of it.
Not sure though this is a good solution.

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
@@ -872,7 +872,6 @@ ngx_quic_create_retry_packet(ngx_quic_he
{
u_char *start;
ngx_str_t ad, itag;
- ngx_quic_secret_t secret;
ngx_quic_ciphers_t ciphers;

/* 5.8. Retry Packet Integrity */
@@ -882,6 +881,8 @@ ngx_quic_create_retry_packet(ngx_quic_he
"\x46\x15\x99\xd3\x5d\x63\x2b\xf2\x23\x98\x25\xbb";
static ngx_str_t in = ngx_string("");

+ static ngx_quic_secret_t secret;
+
ad.data = res->data;
ad.len = ngx_quic_create_retry_itag(pkt, ad.data, &start);

@@ -893,6 +894,10 @@ ngx_quic_create_retry_packet(ngx_quic_he
"quic retry itag len:%uz %xV", ad.len, &ad);
#endif

+ if (secret.ctx) {
+ goto seal;
+ }
+
if (ngx_quic_ciphers(0, &ciphers, pkt->level) == NGX_ERROR) {
return NGX_ERROR;
}
@@ -905,14 +910,14 @@ ngx_quic_create_retry_packet(ngx_quic_he
return NGX_ERROR;
}

+seal:
+
if (ngx_quic_crypto_seal(&secret, &itag, nonce, &in, &ad, pkt->log)
!= NGX_OK)
{
return NGX_ERROR;
}

- ngx_quic_crypto_cleanup(&secret);
-
res->len = itag.data + itag.len - start;
res->data = start;


>
>> 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);

--
Sergey Kandaurov
_______________________________________________
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 469 September 07, 2023 11:18AM

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

Sergey Kandaurov 81 September 07, 2023 11:18AM

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

Roman Arutyunyan 81 September 21, 2023 09:30AM

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

Sergey Kandaurov 76 September 07, 2023 11:18AM

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

Roman Arutyunyan 83 September 18, 2023 03:10AM

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

Sergey Kandaurov 93 October 13, 2023 11:10AM

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

Sergey Kandaurov 76 September 07, 2023 11:18AM

[PATCH 4 of 8] QUIC: renamed protection functions

Sergey Kandaurov 74 September 07, 2023 11:18AM

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

Roman Arutyunyan 83 September 21, 2023 09:32AM

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

Sergey Kandaurov 80 September 07, 2023 11:18AM

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

Roman Arutyunyan 90 September 19, 2023 09:54AM

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

Sergey Kandaurov 75 October 13, 2023 11:14AM

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

Sergey Kandaurov 75 October 17, 2023 06:40AM

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

Sergey Kandaurov 93 October 23, 2023 06:38PM

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

Sergey Kandaurov 76 September 07, 2023 11:18AM

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

Roman Arutyunyan 73 September 20, 2023 08:14AM

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

Sergey Kandaurov 71 October 13, 2023 11:14AM

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

Sergey Kandaurov 80 September 07, 2023 11:18AM

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

Roman Arutyunyan 90 September 20, 2023 08:28AM

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

Sergey Kandaurov 83 October 13, 2023 11:16AM

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

Sergey Kandaurov 74 September 07, 2023 11:18AM

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

Roman Arutyunyan 77 September 21, 2023 09:30AM

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

Sergey Kandaurov 70 October 13, 2023 11:16AM

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

Sergey Kandaurov 74 October 18, 2023 11:28AM

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

Sergey Kandaurov 72 October 18, 2023 11:28AM

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

Sergey Kandaurov 72 October 18, 2023 11:28AM

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

Sergey Kandaurov 71 October 18, 2023 11:28AM

[PATCH 04 of 11] QUIC: renamed protection functions

Sergey Kandaurov 67 October 18, 2023 11:28AM

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

Sergey Kandaurov 71 October 18, 2023 11:28AM

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

Sergey Kandaurov 71 October 18, 2023 11:28AM

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

Sergey Kandaurov 69 October 18, 2023 11:30AM

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

Sergey Kandaurov 70 October 18, 2023 11:30AM

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

Sergey Kandaurov 66 October 18, 2023 11:30AM

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

Sergey Kandaurov 72 October 18, 2023 11:30AM

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

Sergey Kandaurov 70 October 18, 2023 11:38AM

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

Roman Arutyunyan 77 October 20, 2023 03:28AM



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

Online Users

Guests: 85
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