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

Roman Arutyunyan arut at nginx.com
Tue May 31 06:32:26 UTC 2022


On Tue, Feb 22, 2022 at 01:23:27PM +0300, Vladimir Homutov wrote:
> On Mon, Feb 21, 2022 at 05:51:42PM +0300, Sergey Kandaurov wrote:
> > On Mon, Feb 21, 2022 at 02:10:31PM +0300, Vladimir Homutov wrote:
> > > Patch subject is complete summary.
> > >
> > >
> > >  src/event/quic/ngx_event_quic_protection.c |  202 +++++++++++++++-------------
> > >  1 files changed, 105 insertions(+), 97 deletions(-)
> > >
> > >
> >
> > > # HG changeset patch
> > > # User Vladimir Homutov <vl at nginx.com>
> > > # Date 1645440604 -10800
> > > #      Mon Feb 21 13:50:04 2022 +0300
> > > # Branch quic
> > > # Node ID 1a0a12bef7f00b5422d449b2d4642fff39e0a47e
> > > # Parent  55b38514729b8f848709b31295e72d6886a7a433
> > > 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,8 @@
> > >
> > >  #define NGX_QUIC_AES_128_KEY_LEN      16
> > >
> > > +#define NGX_QUIC_KEY_LEN              32
> > > +
> > >  #define NGX_AES_128_GCM_SHA256        0x1301
> > >  #define NGX_AES_256_GCM_SHA384        0x1302
> > >  #define NGX_CHACHA20_POLY1305_SHA256  0x1303
> > > @@ -30,6 +32,27 @@
> > >
> > >
> > >  typedef struct {
> > > +    size_t                    len;
> > > +    u_char                    data[SHA256_DIGEST_LENGTH];
> > > +} ngx_quic_okm_t;
> > > +
> > > +typedef struct {
> > > +    size_t                    len;
> > > +    u_char                    data[NGX_QUIC_KEY_LEN];
> > > +} ngx_quic_key_t;
> > > +
> > > +typedef struct {
> > > +    size_t                    len;
> > > +    u_char                    data[NGX_QUIC_KEY_LEN];
> > > +} ngx_quic_hp_t;
> > > +
> > > +typedef struct {
> > > +    size_t                    len;
> > > +    u_char                    data[NGX_QUIC_IV_LEN];
> > > +} ngx_quic_iv_t;
> >
> > Style: two empty lines between struct declarations.
> 
> thanks, fixed this
> 
> >
> > > +
> > > +
> > > +typedef struct {
> > >      const ngx_quic_cipher_t  *c;
> > >      const EVP_CIPHER         *hp;
> > >      const EVP_MD             *d;
> > > @@ -37,10 +60,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_okm_t            secret;
> > > +    ngx_quic_key_t            key;
> > > +    ngx_quic_iv_t             iv;
> > > +    ngx_quic_hp_t             hp;
> > >  } ngx_quic_secret_t;
> > >
> > >
> > > @@ -57,6 +80,29 @@ 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;
> > > +
> > > +    size_t                    info_len;
> > > +    uint8_t                   info[20];
> > > +} 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),                               \
> > > +        0, { 0 }                                                              \
> > > +    }
> > > +
> > > +
> > >  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 +124,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 +250,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 +273,39 @@ 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;
> > >          }
> > >      }
> > >
> > > -    info_len = 2 + 1 + label->len + 1;
> > > +    h->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);
> > > +    h->info[0] = 0;
> > > +    h->info[1] = h->out_len;
> > > +    h->info[2] = h->label_len;
> >
> > Why?
> > info/info_len aren't used/useful outside of ngx_quic_hkdf_expand(),
> > they are barely one-time local storages to produce traffic secrets.
> 
> sorry, this is a leftover. Indeed, there is no meaning in keeping this.
> I had some ideas how to initialize it on declaration and merge 'info'
> with 'label', but the result was too clumsy and unsafe to use.
> 
> removed info from structure and restored local variable..
> 
> >
> > > +
> > > +    p = ngx_cpymem(&h->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, h->info, h->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
> > >
> > >      return NGX_OK;
> > > @@ -652,6 +689,7 @@ ngx_quic_keys_set_encryption_secret(ngx_
> > >      const SSL_CIPHER *cipher, const uint8_t *secret, size_t secret_len)
> > >  {
> > >      ngx_int_t            key_len;
> > > +    ngx_str_t            secret_str;
> > >      ngx_uint_t           i;
> > >      ngx_quic_secret_t   *peer_secret;
> > >      ngx_quic_ciphers_t   ciphers;
> > > @@ -668,8 +706,9 @@ ngx_quic_keys_set_encryption_secret(ngx_
> > >          return NGX_ERROR;
> > >      }
> > >
> > > -    peer_secret->secret.data = ngx_pnalloc(pool, secret_len);
> > > -    if (peer_secret->secret.data == NULL) {
> > > +    if (sizeof(peer_secret->secret.data) < secret_len) {
> > > +        ngx_log_error(NGX_LOG_ERR, pool->log, 0,
> > > +                      "unexpected secret len: %uz", secret_len);
> > >          return NGX_ERROR;
> > >      }
> >
> > This won't work with cipher suite hash algorithms used to produce
> > HKDF Hash.length (read: secret length) above SHA256_DIGEST_LENGTH,
> > such as TLS_AES_256_GCM_SHA384 or any future TLSv1.3 cipher suites
> > with SHA384 or above.  The same for hardcoding NGX_QUIC_KEY_LEN.
> 
> yes, exactly. currently boringssl just defines max size 48 to keep
> SHA384, and I think we can safely follow it. If anything will appear
> in future, we can always adjust size.
> 
> I've renamed NGX_QUIC_KEY_LEN to NGX_QUIC_MAX_MD_SIZE and renamed
> structure holding buffer to ngx_quic_md_t, and use it for secret,
> key and hp, as it makes sens to use them uniformly.
> 
> > The error, if ever leave it there, deserves rasing logging level
> > to "alert" as clearly a programmatic error.
> >
> agreed. This place is the worst in patch - we get size from the library
> and hope that our bufer is big enough, otherwise fail with a runtime error.
> 
> On the other side, it is pretty clear that size here is just digest
> length, and we can expect reasonable numbers here.
> 

> # 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 bb1717365759760bc8175b8d8084819a6ec35c26
> # Parent  55b38514729b8f848709b31295e72d6886a7a433
> 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;
>          }
>      }
>  
> -    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
>  
>      return NGX_OK;
> @@ -652,6 +679,7 @@ ngx_quic_keys_set_encryption_secret(ngx_
>      const SSL_CIPHER *cipher, const uint8_t *secret, size_t secret_len)
>  {
>      ngx_int_t            key_len;
> +    ngx_str_t            secret_str;
>      ngx_uint_t           i;
>      ngx_quic_secret_t   *peer_secret;
>      ngx_quic_ciphers_t   ciphers;
> @@ -668,8 +696,9 @@ ngx_quic_keys_set_encryption_secret(ngx_
>          return NGX_ERROR;
>      }
>  
> -    peer_secret->secret.data = ngx_pnalloc(pool, secret_len);
> -    if (peer_secret->secret.data == NULL) {
> +    if (sizeof(peer_secret->secret.data) < secret_len) {
> +        ngx_log_error(NGX_LOG_ALERT, pool->log, 0,
> +                      "unexpected secret len: %uz", secret_len);
>          return NGX_ERROR;
>      }
>  
> @@ -680,22 +709,17 @@ ngx_quic_keys_set_encryption_secret(ngx_
>      peer_secret->iv.len = NGX_QUIC_IV_LEN;
>      peer_secret->hp.len = key_len;
>  
> -    struct {
> -        ngx_str_t       label;
> -        ngx_str_t      *key;
> -        const uint8_t  *secret;
> -    } seq[] = {
> -        { ngx_string("tls13 quic key"), &peer_secret->key, secret },
> -        { ngx_string("tls13 quic iv"),  &peer_secret->iv,  secret },
> -        { ngx_string("tls13 quic hp"),  &peer_secret->hp,  secret },
> +    secret_str.len = secret_len;
> +    secret_str.data = (u_char *) secret;
> +
> +    ngx_quic_hkdf_t seq[] = {
> +        ngx_quic_hkdf_set("tls13 quic key", &peer_secret->key, &secret_str),
> +        ngx_quic_hkdf_set("tls13 quic iv", &peer_secret->iv, &secret_str),
> +        ngx_quic_hkdf_set("tls13 quic hp", &peer_secret->hp, &secret_str),
>      };
>  
>      for (i = 0; i < (sizeof(seq) / sizeof(seq[0])); i++) {
> -
> -        if (ngx_quic_hkdf_expand(pool, ciphers.d, seq[i].key, &seq[i].label,
> -                                 seq[i].secret, secret_len)
> -            != NGX_OK)
> -        {
> +        if (ngx_quic_hkdf_expand(&seq[i], ciphers.d, pool) != NGX_OK) {
>              return NGX_ERROR;
>          }
>      }
> @@ -769,49 +793,23 @@ ngx_quic_keys_update(ngx_connection_t *c
>      next->server.iv.len = NGX_QUIC_IV_LEN;
>      next->server.hp = current->server.hp;
>  
> -    struct {
> -        ngx_str_t   label;
> -        ngx_str_t  *key;
> -        ngx_str_t  *secret;
> -    } seq[] = {
> -        {
> -            ngx_string("tls13 quic ku"),
> -            &next->client.secret,
> -            &current->client.secret,
> -        },
> -        {
> -            ngx_string("tls13 quic key"),
> -            &next->client.key,
> -            &next->client.secret,
> -        },
> -        {
> -            ngx_string("tls13 quic iv"),
> -            &next->client.iv,
> -            &next->client.secret,
> -        },
> -        {
> -            ngx_string("tls13 quic ku"),
> -            &next->server.secret,
> -            &current->server.secret,
> -        },
> -        {
> -            ngx_string("tls13 quic key"),
> -            &next->server.key,
> -            &next->server.secret,
> -        },
> -        {
> -            ngx_string("tls13 quic iv"),
> -            &next->server.iv,
> -            &next->server.secret,
> -        },
> +    ngx_quic_hkdf_t seq[] = {
> +        ngx_quic_hkdf_set("tls13 quic ku",
> +                           &next->client.secret, &current->client.secret),

A space is missing here ^^

> +        ngx_quic_hkdf_set("tls13 quic key",
> +                          &next->client.key, &next->client.secret),
> +        ngx_quic_hkdf_set("tls13 quic iv",
> +                          &next->client.iv, &next->client.secret),
> +        ngx_quic_hkdf_set("tls13 quic ku",
> +                          &next->server.secret, &current->server.secret),
> +        ngx_quic_hkdf_set("tls13 quic key",
> +                          &next->server.key, &next->server.secret),
> +        ngx_quic_hkdf_set("tls13 quic iv",
> +                          &next->server.iv, &next->server.secret),
>      };
>  
>      for (i = 0; i < (sizeof(seq) / sizeof(seq[0])); i++) {
> -
> -        if (ngx_quic_hkdf_expand(c->pool, ciphers.d, seq[i].key, &seq[i].label,
> -                                 seq[i].secret->data, seq[i].secret->len)
> -            != NGX_OK)
> -        {
> +        if (ngx_quic_hkdf_expand(&seq[i], ciphers.d, c->pool) != NGX_OK) {
>              return NGX_ERROR;
>          }
>      }
> @@ -909,7 +907,7 @@ ngx_quic_create_retry_packet(ngx_quic_he
>      }
>  
>      secret.key.len = sizeof(key);
> -    secret.key.data = key;
> +    ngx_memcpy(secret.key.data, key, sizeof(key));
>      secret.iv.len = NGX_QUIC_IV_LEN;
>  
>      if (ngx_quic_tls_seal(ciphers.c, &secret, &itag, nonce, &in, &ad, pkt->log)

Overall, the patch looks good to me.

This version of the patch affects the following patches in the series.
I will resend the whole series.



More information about the nginx-devel mailing list