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

Sergey Kandaurov pluknet at nginx.com
Mon Jul 25 22:36:37 UTC 2022


> On 31 May 2022, at 11:06, Roman Arutyunyan <arut at nginx.com> wrote:
> 
> # HG changeset patch
> # User Vladimir Homutov <vl at 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



More information about the nginx-devel mailing list