Welcome! Log In Create A New Profile

Advanced

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

Sergey Kandaurov
October 13, 2023 11:14AM
> On 20 Sep 2023, at 16:12, Roman Arutyunyan <arut@nginx.com> wrote:
>
> Hi,
>
> On Thu, Sep 07, 2023 at 07:13:58PM +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 cdc5b59309dbdc234c71e53fca142502884e6177
>> # Parent 28f7491bc79771f9cfa882b1b5584fa48ea42e6b
>> QUIC: reusing crypto contexts for header protection.
>>
>> diff --git a/src/event/quic/ngx_event_quic_protection.c b/src/event/quic/ngx_event_quic_protection.c
>> --- a/src/event/quic/ngx_event_quic_protection.c
>> +++ b/src/event/quic/ngx_event_quic_protection.c
>> @@ -28,8 +28,12 @@ static uint64_t ngx_quic_parse_pn(u_char
>>
>> static ngx_int_t ngx_quic_crypto_open(ngx_quic_secret_t *s, ngx_str_t *out,
>> u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log);
>> -static ngx_int_t ngx_quic_crypto_hp(ngx_log_t *log, const EVP_CIPHER *cipher,
>> - ngx_quic_secret_t *s, u_char *out, u_char *in);
>> +
>> +static ngx_int_t ngx_quic_crypto_hp_init(const EVP_CIPHER *cipher,
>> + ngx_quic_secret_t *s, ngx_log_t *log);
>> +static ngx_int_t ngx_quic_crypto_hp(ngx_quic_secret_t *s,
>> + u_char *out, u_char *in, ngx_log_t *log);
>> +static void ngx_quic_crypto_hp_cleanup(ngx_quic_secret_t *s);
>>
>> static ngx_int_t ngx_quic_create_packet(ngx_quic_header_t *pkt,
>> ngx_str_t *res);
>> @@ -192,6 +196,14 @@ ngx_quic_keys_set_initial_secret(ngx_qui
>> return NGX_ERROR;
>> }
>>
>> + if (ngx_quic_crypto_hp_init(ciphers.hp, client, log) == NGX_ERROR) {
>> + return NGX_ERROR;
>> + }
>> +
>> + if (ngx_quic_crypto_hp_init(ciphers.hp, server, log) == NGX_ERROR) {
>> + return NGX_ERROR;
>> + }
>
> Again, as before, in case of errors all ctx's created here should be freed,
> since we don't always have a cleanup handler for them, see
> ngx_quic_send_early_cc().
>
> Also, in ngx_quic_send_early_cc() there's no cleanup at all, and ctx's will
> leak in case of successful creation.

Converted error path to "goto failed"
(based on the updated change in the previous patch thread).

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

Agree, it looks more clean.
Hopefully, TLS won't evolve more ciphers lacking EVP in BoringSSL.

# HG changeset patch
# User Sergey Kandaurov <pluknet@nginx.com>
# Date 1697199167 -14400
# Fri Oct 13 16:12:47 2023 +0400
# Node ID 226da28965e4ed95b200516ed9aa4d5b4804ae24
# Parent 26f1c0864c67f5aac529f06592d915d7de9adb6e
QUIC: reusing crypto contexts for header protection.

diff --git a/src/event/quic/ngx_event_quic_protection.c b/src/event/quic/ngx_event_quic_protection.c
--- a/src/event/quic/ngx_event_quic_protection.c
+++ b/src/event/quic/ngx_event_quic_protection.c
@@ -32,8 +32,12 @@ static ngx_int_t ngx_quic_crypto_open(ng
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);
+
+static ngx_int_t ngx_quic_crypto_hp_init(const EVP_CIPHER *cipher,
+ ngx_quic_secret_t *s, ngx_log_t *log);
+static ngx_int_t ngx_quic_crypto_hp(ngx_quic_secret_t *s,
+ u_char *out, u_char *in, ngx_log_t *log);
+static void ngx_quic_crypto_hp_cleanup(ngx_quic_secret_t *s);

static ngx_int_t ngx_quic_create_packet(ngx_quic_header_t *pkt,
ngx_str_t *res);
@@ -196,6 +200,14 @@ ngx_quic_keys_set_initial_secret(ngx_qui
goto failed;
}

+ if (ngx_quic_crypto_hp_init(ciphers.hp, client, log) == NGX_ERROR) {
+ goto failed;
+ }
+
+ if (ngx_quic_crypto_hp_init(ciphers.hp, server, log) == NGX_ERROR) {
+ goto failed;
+ }
+
return NGX_OK;

failed:
@@ -556,53 +568,82 @@ ngx_quic_crypto_cleanup(ngx_quic_secret_


static ngx_int_t
-ngx_quic_crypto_hp(ngx_log_t *log, const EVP_CIPHER *cipher,
- ngx_quic_secret_t *s, u_char *out, u_char *in)
+ngx_quic_crypto_hp_init(const EVP_CIPHER *cipher, ngx_quic_secret_t *s,
+ ngx_log_t *log)
{
- int outlen;
EVP_CIPHER_CTX *ctx;
- u_char zero[NGX_QUIC_HP_LEN] = {0};

#ifdef OPENSSL_IS_BORINGSSL
- uint32_t cnt;
-
- ngx_memcpy(&cnt, in, sizeof(uint32_t));
-
- if (cipher == (const EVP_CIPHER *) EVP_aead_chacha20_poly1305()) {
- CRYPTO_chacha_20(out, zero, NGX_QUIC_HP_LEN, s->hp.data, &in[4], cnt);
+ if (cipher == (EVP_CIPHER *) EVP_aead_chacha20_poly1305()) {
+ /* no EVP interface */
+ s->hp_ctx = NULL;
return NGX_OK;
}
#endif

ctx = EVP_CIPHER_CTX_new();
if (ctx == NULL) {
+ ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_CIPHER_CTX_new() failed");
+ return NGX_ERROR;
+ }
+
+ if (EVP_EncryptInit_ex(ctx, cipher, NULL, s->hp.data, NULL) != 1) {
+ EVP_CIPHER_CTX_free(ctx);
+ ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptInit_ex() failed");
return NGX_ERROR;
}

- if (EVP_EncryptInit_ex(ctx, cipher, NULL, s->hp.data, in) != 1) {
+ s->hp_ctx = ctx;
+ return NGX_OK;
+}
+
+
+static ngx_int_t
+ngx_quic_crypto_hp(ngx_quic_secret_t *s, u_char *out, u_char *in,
+ ngx_log_t *log)
+{
+ int outlen;
+ EVP_CIPHER_CTX *ctx;
+ u_char zero[NGX_QUIC_HP_LEN] = {0};
+
+ ctx = s->hp_ctx;
+
+#ifdef OPENSSL_IS_BORINGSSL
+ uint32_t cnt;
+
+ if (ctx == NULL) {
+ ngx_memcpy(&cnt, in, sizeof(uint32_t));
+ CRYPTO_chacha_20(out, zero, NGX_QUIC_HP_LEN, s->hp.data, &in[4], cnt);
+ return NGX_OK;
+ }
+#endif
+
+ if (EVP_EncryptInit_ex(ctx, NULL, NULL, NULL, in) != 1) {
ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptInit_ex() failed");
- goto failed;
+ return NGX_ERROR;
}

if (!EVP_EncryptUpdate(ctx, out, &outlen, zero, NGX_QUIC_HP_LEN)) {
ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptUpdate() failed");
- goto failed;
+ return NGX_ERROR;
}

if (!EVP_EncryptFinal_ex(ctx, out + NGX_QUIC_HP_LEN, &outlen)) {
ngx_ssl_error(NGX_LOG_INFO, log, 0, "EVP_EncryptFinal_Ex() failed");
- goto failed;
+ return NGX_ERROR;
}

- EVP_CIPHER_CTX_free(ctx);
-
return NGX_OK;
+}

-failed:

- EVP_CIPHER_CTX_free(ctx);
-
- return NGX_ERROR;
+static void
+ngx_quic_crypto_hp_cleanup(ngx_quic_secret_t *s)
+{
+ if (s->hp_ctx) {
+ EVP_CIPHER_CTX_free(s->hp_ctx);
+ s->hp_ctx = NULL;
+ }
}


@@ -663,6 +704,10 @@ ngx_quic_keys_set_encryption_secret(ngx_
return NGX_ERROR;
}

+ if (ngx_quic_crypto_hp_init(ciphers.hp, peer_secret, log) == NGX_ERROR) {
+ return NGX_ERROR;
+ }
+
return NGX_OK;
}

@@ -690,6 +735,9 @@ ngx_quic_keys_discard(ngx_quic_keys_t *k

ngx_quic_crypto_cleanup(client);
ngx_quic_crypto_cleanup(server);
+
+ ngx_quic_crypto_hp_cleanup(client);
+ ngx_quic_crypto_hp_cleanup(server);
}


@@ -742,11 +790,13 @@ ngx_quic_keys_update(ngx_event_t *ev)
next->client.key.len = current->client.key.len;
next->client.iv.len = NGX_QUIC_IV_LEN;
next->client.hp = current->client.hp;
+ next->client.hp_ctx = current->client.hp_ctx;

next->server.secret.len = current->server.secret.len;
next->server.key.len = current->server.key.len;
next->server.iv.len = NGX_QUIC_IV_LEN;
next->server.hp = current->server.hp;
+ next->server.hp_ctx = current->server.hp_ctx;

ngx_quic_hkdf_set(&seq[0], "tls13 quic ku",
&next->client.secret, &current->client.secret);
@@ -795,6 +845,9 @@ ngx_quic_keys_cleanup(ngx_quic_keys_t *k
secrets = &keys->secrets[i];
ngx_quic_crypto_cleanup(&secrets->client);
ngx_quic_crypto_cleanup(&secrets->server);
+
+ ngx_quic_crypto_hp_cleanup(&secrets->client);
+ ngx_quic_crypto_hp_cleanup(&secrets->server);
}

secrets = &keys->next_key;
@@ -841,9 +894,7 @@ ngx_quic_create_packet(ngx_quic_header_t
}

sample = &out.data[4 - pkt->num_len];
- if (ngx_quic_crypto_hp(pkt->log, ciphers.hp, secret, mask, sample)
- != NGX_OK)
- {
+ if (ngx_quic_crypto_hp(secret, mask, sample, pkt->log) != NGX_OK) {
return NGX_ERROR;
}

@@ -1075,9 +1126,7 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt,

/* header protection */

- if (ngx_quic_crypto_hp(pkt->log, ciphers.hp, secret, mask, sample)
- != NGX_OK)
- {
+ if (ngx_quic_crypto_hp(secret, mask, sample, pkt->log) != NGX_OK) {
return NGX_DECLINED;
}

diff --git a/src/event/quic/ngx_event_quic_protection.h b/src/event/quic/ngx_event_quic_protection.h
--- a/src/event/quic/ngx_event_quic_protection.h
+++ b/src/event/quic/ngx_event_quic_protection.h
@@ -51,6 +51,7 @@ typedef struct {
ngx_quic_iv_t iv;
ngx_quic_md_t hp;
ngx_quic_crypto_ctx_t *ctx;
+ EVP_CIPHER_CTX *hp_ctx;
} ngx_quic_secret_t;



--
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 593 September 07, 2023 11:18AM

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

Sergey Kandaurov 134 September 07, 2023 11:18AM

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

Roman Arutyunyan 131 September 21, 2023 09:30AM

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

Sergey Kandaurov 136 September 07, 2023 11:18AM

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

Roman Arutyunyan 137 September 18, 2023 03:10AM

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

Sergey Kandaurov 162 October 13, 2023 11:10AM

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

Sergey Kandaurov 129 September 07, 2023 11:18AM

[PATCH 4 of 8] QUIC: renamed protection functions

Sergey Kandaurov 136 September 07, 2023 11:18AM

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

Roman Arutyunyan 147 September 21, 2023 09:32AM

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

Sergey Kandaurov 133 September 07, 2023 11:18AM

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

Roman Arutyunyan 175 September 19, 2023 09:54AM

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

Sergey Kandaurov 146 October 13, 2023 11:14AM

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

Sergey Kandaurov 135 October 17, 2023 06:40AM

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

Sergey Kandaurov 170 October 23, 2023 06:38PM

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

Sergey Kandaurov 118 September 07, 2023 11:18AM

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

Roman Arutyunyan 140 September 20, 2023 08:14AM

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

Sergey Kandaurov 129 October 13, 2023 11:14AM

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

Sergey Kandaurov 134 September 07, 2023 11:18AM

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

Roman Arutyunyan 143 September 20, 2023 08:28AM

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

Sergey Kandaurov 140 October 13, 2023 11:16AM

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

Sergey Kandaurov 129 September 07, 2023 11:18AM

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

Roman Arutyunyan 125 September 21, 2023 09:30AM

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

Sergey Kandaurov 132 October 13, 2023 11:16AM

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

Sergey Kandaurov 131 October 18, 2023 11:28AM

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

Sergey Kandaurov 132 October 18, 2023 11:28AM

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

Sergey Kandaurov 143 October 18, 2023 11:28AM

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

Sergey Kandaurov 137 October 18, 2023 11:28AM

[PATCH 04 of 11] QUIC: renamed protection functions

Sergey Kandaurov 141 October 18, 2023 11:28AM

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

Sergey Kandaurov 132 October 18, 2023 11:28AM

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

Sergey Kandaurov 131 October 18, 2023 11:28AM

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

Sergey Kandaurov 134 October 18, 2023 11:30AM

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

Sergey Kandaurov 128 October 18, 2023 11:30AM

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

Sergey Kandaurov 118 October 18, 2023 11:30AM

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

Sergey Kandaurov 129 October 18, 2023 11:30AM

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

Sergey Kandaurov 132 October 18, 2023 11:38AM

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

Roman Arutyunyan 138 October 20, 2023 03:28AM



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

Online Users

Guests: 199
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready