Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 1 of 4] QUIC: fixed-length buffers for secrets

Sergey Kandaurov
July 25, 2022 06:38PM
> On 31 May 2022, at 11:06, Roman Arutyunyan <arut@nginx.com> wrote:
>
> # HG changeset patch
> # User Vladimir Homutov <vl@nginx.com>
> # Date 1645524401 -10800
> # Tue Feb 22 13:06:41 2022 +0300
> # Branch quic
> # Node ID a881ff28070262f3810517d5d3cb4ff67a4b7121
> # Parent 5b1011b5702b5c5db2ba3d392a4da25596183cc2
> QUIC: fixed-length buffers for secrets.
>
> 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
> @@ -17,6 +17,9 @@
>
> #define NGX_QUIC_AES_128_KEY_LEN 16
>
> +/* largest hash used in TLS is SHA-384 */
> +#define NGX_QUIC_MAX_MD_SIZE 48
> +
> #define NGX_AES_128_GCM_SHA256 0x1301
> #define NGX_AES_256_GCM_SHA384 0x1302
> #define NGX_CHACHA20_POLY1305_SHA256 0x1303
> @@ -30,6 +33,18 @@
>
>
> typedef struct {
> + size_t len;
> + u_char data[NGX_QUIC_MAX_MD_SIZE];
> +} ngx_quic_md_t;
> +
> +
> +typedef struct {
> + size_t len;
> + u_char data[NGX_QUIC_IV_LEN];
> +} ngx_quic_iv_t;
> +
> +
> +typedef struct {
> const ngx_quic_cipher_t *c;
> const EVP_CIPHER *hp;
> const EVP_MD *d;
> @@ -37,10 +52,10 @@ typedef struct {
>
>
> typedef struct ngx_quic_secret_s {
> - ngx_str_t secret;
> - ngx_str_t key;
> - ngx_str_t iv;
> - ngx_str_t hp;
> + ngx_quic_md_t secret;
> + ngx_quic_md_t key;
> + ngx_quic_iv_t iv;
> + ngx_quic_md_t hp;
> } ngx_quic_secret_t;
>
>
> @@ -57,6 +72,25 @@ struct ngx_quic_keys_s {
> };
>
>
> +typedef struct {
> + size_t out_len;
> + u_char *out;
> +
> + size_t prk_len;
> + const uint8_t *prk;
> +
> + size_t label_len;
> + const u_char *label;
> +} ngx_quic_hkdf_t;
> +
> +#define ngx_quic_hkdf_set(label, out, prk) \
> + { \
> + (out)->len, (out)->data, \
> + (prk)->len, (prk)->data, \
> + (sizeof(label) - 1), (u_char *)(label), \
> + }
> +
> +
> static ngx_int_t ngx_hkdf_expand(u_char *out_key, size_t out_len,
> const EVP_MD *digest, const u_char *prk, size_t prk_len,
> const u_char *info, size_t info_len);
> @@ -78,8 +112,8 @@ static ngx_int_t ngx_quic_tls_seal(const
> ngx_str_t *ad, ngx_log_t *log);
> static ngx_int_t ngx_quic_tls_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_hkdf_expand(ngx_pool_t *pool, const EVP_MD *digest,
> - ngx_str_t *out, ngx_str_t *label, const uint8_t *prk, size_t prk_len);
> +static ngx_int_t ngx_quic_hkdf_expand(ngx_quic_hkdf_t *hkdf,
> + const EVP_MD *digest, ngx_pool_t *pool);
>
> static ngx_int_t ngx_quic_create_packet(ngx_quic_header_t *pkt,
> ngx_str_t *res);
> @@ -204,28 +238,20 @@ ngx_quic_keys_set_initial_secret(ngx_poo
> client->iv.len = NGX_QUIC_IV_LEN;
> server->iv.len = NGX_QUIC_IV_LEN;
>
> - struct {
> - ngx_str_t label;
> - ngx_str_t *key;
> - ngx_str_t *prk;
> - } seq[] = {
> + ngx_quic_hkdf_t seq[] = {
> /* labels per RFC 9001, 5.1. Packet Protection Keys */
> - { ngx_string("tls13 client in"), &client->secret, &iss },
> - { ngx_string("tls13 quic key"), &client->key, &client->secret },
> - { ngx_string("tls13 quic iv"), &client->iv, &client->secret },
> - { ngx_string("tls13 quic hp"), &client->hp, &client->secret },
> - { ngx_string("tls13 server in"), &server->secret, &iss },
> - { ngx_string("tls13 quic key"), &server->key, &server->secret },
> - { ngx_string("tls13 quic iv"), &server->iv, &server->secret },
> - { ngx_string("tls13 quic hp"), &server->hp, &server->secret },
> + ngx_quic_hkdf_set("tls13 client in", &client->secret, &iss),
> + ngx_quic_hkdf_set("tls13 quic key", &client->key, &client->secret),
> + ngx_quic_hkdf_set("tls13 quic iv", &client->iv, &client->secret),
> + ngx_quic_hkdf_set("tls13 quic hp", &client->hp, &client->secret),
> + ngx_quic_hkdf_set("tls13 server in", &server->secret, &iss),
> + ngx_quic_hkdf_set("tls13 quic key", &server->key, &server->secret),
> + ngx_quic_hkdf_set("tls13 quic iv", &server->iv, &server->secret),
> + ngx_quic_hkdf_set("tls13 quic hp", &server->hp, &server->secret),
> };
>
> for (i = 0; i < (sizeof(seq) / sizeof(seq[0])); i++) {
> -
> - if (ngx_quic_hkdf_expand(pool, digest, seq[i].key, &seq[i].label,
> - seq[i].prk->data, seq[i].prk->len)
> - != NGX_OK)
> - {
> + if (ngx_quic_hkdf_expand(&seq[i], digest, pool) != NGX_OK) {
> return NGX_ERROR;
> }
> }
> @@ -235,40 +261,41 @@ ngx_quic_keys_set_initial_secret(ngx_poo
>
>
> static ngx_int_t
> -ngx_quic_hkdf_expand(ngx_pool_t *pool, const EVP_MD *digest, ngx_str_t *out,
> - ngx_str_t *label, const uint8_t *prk, size_t prk_len)
> +ngx_quic_hkdf_expand(ngx_quic_hkdf_t *h, const EVP_MD *digest, ngx_pool_t *pool)
> {
> size_t info_len;
> uint8_t *p;
> uint8_t info[20];
>
> - if (out->data == NULL) {
> - out->data = ngx_pnalloc(pool, out->len);
> - if (out->data == NULL) {
> + if (h->out == NULL) {
> + h->out = ngx_pnalloc(pool, h->out_len);
> + if (h->out == NULL) {
> return NGX_ERROR;
> }
> }

The condition can be removed now.
I see no reason to postpone this to the next change.

>
> - info_len = 2 + 1 + label->len + 1;
> + info_len = 2 + 1 + h->label_len + 1;
>
> info[0] = 0;
> - info[1] = out->len;
> - info[2] = label->len;
> - p = ngx_cpymem(&info[3], label->data, label->len);
> + info[1] = h->out_len;
> + info[2] = h->label_len;
> +
> + p = ngx_cpymem(&info[3], h->label, h->label_len);
> *p = '\0';
>
> - if (ngx_hkdf_expand(out->data, out->len, digest,
> - prk, prk_len, info, info_len)
> + if (ngx_hkdf_expand(h->out, h->out_len, digest,
> + h->prk, h->prk_len, info, info_len)
> != NGX_OK)
> {
> ngx_ssl_error(NGX_LOG_INFO, pool->log, 0,
> - "ngx_hkdf_expand(%V) failed", label);
> + "ngx_hkdf_expand(%*s) failed", h->label_len, h->label);
> return NGX_ERROR;
> }
>
> #ifdef NGX_QUIC_DEBUG_CRYPTO
> - ngx_log_debug3(NGX_LOG_DEBUG_EVENT, pool->log, 0,
> - "quic expand %V key len:%uz %xV", label, out->len, out);
> + ngx_log_debug5(NGX_LOG_DEBUG_EVENT, pool->log, 0,
> + "quic expand \"%*s\" key len:%uz %*xs",
> + h->label_len, h->label, h->out_len, h->out_len, h->out);
> #endif

While here, I'd also drop "key" from the message (an 64a484fd40a9 leftover).

[..]

--
Sergey Kandaurov

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[PATCH 0 of 4] avoid pool allocations

Roman Arutyunyan 454 May 31, 2022 03:08AM

[PATCH 1 of 4] QUIC: fixed-length buffers for secrets

Roman Arutyunyan 113 May 31, 2022 03:08AM

Re: [PATCH 1 of 4] QUIC: fixed-length buffers for secrets

Sergey Kandaurov 91 July 25, 2022 06:38PM

[PATCH 2 of 4] QUIC: avoided pool usage in ngx_quic_protection.c

Roman Arutyunyan 98 May 31, 2022 03:08AM

[PATCH 4 of 4] QUIC: avoided pool usage in token calculation

Roman Arutyunyan 102 May 31, 2022 03:08AM

[PATCH 3 of 4] QUIC: removed ngx_quic_keys_new()

Roman Arutyunyan 115 May 31, 2022 03:08AM

Re: [PATCH 3 of 4] QUIC: removed ngx_quic_keys_new()

Sergey Kandaurov 167 July 25, 2022 07:02PM



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

Online Users

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