[PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Sergey Kandaurov pluknet at nginx.com
Tue Nov 15 12:28:30 UTC 2022


> On 21 Oct 2022, at 04:10, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> On Tue, Oct 11, 2022 at 02:35:52PM +0400, Sergey Kandaurov wrote:
> 
>> # HG changeset patch
>> # User Sergey Kandaurov <pluknet at nginx.com>
>> # Date 1665484414 -14400
>> #      Tue Oct 11 14:33:34 2022 +0400
>> # Branch quic
>> # Node ID c0165ddcb1c6981f8e5230081f03a277f62d20c3
>> # Parent  caced81ce0a9cb218ae8cdd6176c12e0614acee9
>> QUIC: support for setting QUIC methods with LibreSSL.
>> 
>> Setting QUIC methods is converted to use C99 designated initializers
>> for simplicity, as LibreSSL 3.6.0 has different SSL_QUIC_METHOD layout.
> 
> I'm somewhat sceptical about C99 designated initializers.  These 
> aren't supported by MSVC till 2012: in particular, this includes 
> all MSVC versions available via wineticks, as well as MSVC 
> versions currently used to build nginx win32 binaries.
> 
> A more portable solution might be to use run-time initialization 
> instead.

While I agree in principle, fixing build with MSVC would require
a larger work to implement support for UDP connections on win32.
Specifically, nginx-quic uses c->udp->buffer and sockets API that
needs porting to win32, see ngx_quic_send() / ngx_quic_recvmsg().

Other than that, currently the build fails in other places
as seen with MSVC 2010.  Below the patches to address this.

# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1668509694 -14400
#      Tue Nov 15 14:54:54 2022 +0400
# Branch quic
# Node ID 322f11b1272fd3a0b67fc4cc29b12b6c09e64c6f
# Parent  572a9a4a0642514fa4ef1d8c8121dfb14cadf7c0
QUIC: fixed C4706 warnings with MSVC 2010.

The fix is to avoid assignments within conditional expression.

diff --git a/src/event/quic/ngx_event_quic_transport.c b/src/event/quic/ngx_event_quic_transport.c
--- a/src/event/quic/ngx_event_quic_transport.c
+++ b/src/event/quic/ngx_event_quic_transport.c
@@ -804,11 +804,23 @@ ngx_quic_parse_frame(ngx_quic_header_t *
     case NGX_QUIC_FT_ACK:
     case NGX_QUIC_FT_ACK_ECN:
 
-        if (!((p = ngx_quic_parse_int(p, end, &f->u.ack.largest))
-              && (p = ngx_quic_parse_int(p, end, &f->u.ack.delay))
-              && (p = ngx_quic_parse_int(p, end, &f->u.ack.range_count))
-              && (p = ngx_quic_parse_int(p, end, &f->u.ack.first_range))))
-        {
+        p = ngx_quic_parse_int(p, end, &f->u.ack.largest);
+        if (p == NULL) {
+            goto error;
+        }
+
+        p = ngx_quic_parse_int(p, end, &f->u.ack.delay);
+        if (p == NULL) {
+            goto error;
+        }
+
+        p = ngx_quic_parse_int(p, end, &f->u.ack.range_count);
+        if (p == NULL) {
+            goto error;
+        }
+
+        p = ngx_quic_parse_int(p, end, &f->u.ack.first_range);
+        if (p == NULL) {
             goto error;
         }
 
@@ -818,10 +830,11 @@ ngx_quic_parse_frame(ngx_quic_header_t *
         for (i = 0; i < f->u.ack.range_count; i++) {
 
             p = ngx_quic_parse_int(p, end, &varint);
-            if (p) {
-                p = ngx_quic_parse_int(p, end, &varint);
+            if (p == NULL) {
+                goto error;
             }
 
+            p = ngx_quic_parse_int(p, end, &varint);
             if (p == NULL) {
                 goto error;
             }
@@ -833,10 +846,18 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
         if (f->type == NGX_QUIC_FT_ACK_ECN) {
 
-            if (!((p = ngx_quic_parse_int(p, end, &f->u.ack.ect0))
-                  && (p = ngx_quic_parse_int(p, end, &f->u.ack.ect1))
-                  && (p = ngx_quic_parse_int(p, end, &f->u.ack.ce))))
-            {
+            p = ngx_quic_parse_int(p, end, &f->u.ack.ect0);
+            if (p == NULL) {
+                goto error;
+            }
+
+            p = ngx_quic_parse_int(p, end, &f->u.ack.ect1);
+            if (p == NULL) {
+                goto error;
+            }
+
+            p = ngx_quic_parse_int(p, end, &f->u.ack.ce);
+            if (p == NULL) {
                 goto error;
             }
 
@@ -989,11 +1010,18 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_RESET_STREAM:
 
-        if (!((p = ngx_quic_parse_int(p, end, &f->u.reset_stream.id))
-              && (p = ngx_quic_parse_int(p, end, &f->u.reset_stream.error_code))
-              && (p = ngx_quic_parse_int(p, end,
-                                         &f->u.reset_stream.final_size))))
-        {
+        p = ngx_quic_parse_int(p, end, &f->u.reset_stream.id);
+        if (p == NULL) {
+            goto error;
+        }
+
+        p = ngx_quic_parse_int(p, end, &f->u.reset_stream.error_code);
+        if (p == NULL) {
+            goto error;
+        }
+
+        p = ngx_quic_parse_int(p, end, &f->u.reset_stream.final_size);
+        if (p == NULL) {
             goto error;
         }
 
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1668510614 -14400
#      Tue Nov 15 15:10:14 2022 +0400
# Branch quic
# Node ID eff714f6d07aa595b0f4f552b76a228b5dd5758f
# Parent  322f11b1272fd3a0b67fc4cc29b12b6c09e64c6f
QUIC: moved variable declaration to fix build with MSVC 2010.

Previously, ngx_quic_hkdf_t variables used declaration with assignment
in the middle of a function, which is not supported by MSVC 2010.
Fixing this also required to rewrite the ngx_quic_hkdf_set macro
and to switch to an explicit array size.

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
@@ -48,12 +48,10 @@ typedef struct {
     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),                               \
-    }
+#define ngx_quic_hkdf_set(seq, _label, _out, _prk)                            \
+    (seq)->out_len = (_out)->len; (seq)->out = (_out)->data;                  \
+    (seq)->prk_len = (_prk)->len, (seq)->prk = (_prk)->data,                  \
+    (seq)->label_len = (sizeof(_label) - 1); (seq)->label = (u_char *)(_label);
 
 
 static ngx_int_t ngx_hkdf_expand(u_char *out_key, size_t out_len,
@@ -151,6 +149,7 @@ ngx_quic_keys_set_initial_secret(ngx_qui
     uint8_t             is[SHA256_DIGEST_LENGTH];
     ngx_uint_t          i;
     const EVP_MD       *digest;
+    ngx_quic_hkdf_t     seq[8];
     ngx_quic_secret_t  *client, *server;
 
     static const uint8_t salt[20] =
@@ -203,17 +202,15 @@ ngx_quic_keys_set_initial_secret(ngx_qui
     client->iv.len = NGX_QUIC_IV_LEN;
     server->iv.len = NGX_QUIC_IV_LEN;
 
-    ngx_quic_hkdf_t seq[] = {
-        /* labels per RFC 9001, 5.1. Packet Protection Keys */
-        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),
-    };
+    /* labels per RFC 9001, 5.1. Packet Protection Keys */
+    ngx_quic_hkdf_set(&seq[0], "tls13 client in", &client->secret, &iss);
+    ngx_quic_hkdf_set(&seq[1], "tls13 quic key", &client->key, &client->secret);
+    ngx_quic_hkdf_set(&seq[2], "tls13 quic iv", &client->iv, &client->secret);
+    ngx_quic_hkdf_set(&seq[3], "tls13 quic hp", &client->hp, &client->secret);
+    ngx_quic_hkdf_set(&seq[4], "tls13 server in", &server->secret, &iss);
+    ngx_quic_hkdf_set(&seq[5], "tls13 quic key", &server->key, &server->secret);
+    ngx_quic_hkdf_set(&seq[6], "tls13 quic iv", &server->iv, &server->secret);
+    ngx_quic_hkdf_set(&seq[7], "tls13 quic hp", &server->hp, &server->secret);
 
     for (i = 0; i < (sizeof(seq) / sizeof(seq[0])); i++) {
         if (ngx_quic_hkdf_expand(&seq[i], digest, log) != NGX_OK) {
@@ -639,6 +636,7 @@ ngx_quic_keys_set_encryption_secret(ngx_
     ngx_int_t            key_len;
     ngx_str_t            secret_str;
     ngx_uint_t           i;
+    ngx_quic_hkdf_t      seq[3];
     ngx_quic_secret_t   *peer_secret;
     ngx_quic_ciphers_t   ciphers;
 
@@ -670,11 +668,10 @@ ngx_quic_keys_set_encryption_secret(ngx_
     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),
-    };
+    ngx_quic_hkdf_set(&seq[0], "tls13 quic key",
+                      &peer_secret->key, &secret_str);
+    ngx_quic_hkdf_set(&seq[1], "tls13 quic iv", &peer_secret->iv, &secret_str);
+    ngx_quic_hkdf_set(&seq[2], "tls13 quic hp", &peer_secret->hp, &secret_str);
 
     for (i = 0; i < (sizeof(seq) / sizeof(seq[0])); i++) {
         if (ngx_quic_hkdf_expand(&seq[i], ciphers.d, log) != NGX_OK) {
@@ -720,6 +717,7 @@ ngx_int_t
 ngx_quic_keys_update(ngx_connection_t *c, ngx_quic_keys_t *keys)
 {
     ngx_uint_t           i;
+    ngx_quic_hkdf_t      seq[6];
     ngx_quic_ciphers_t   ciphers;
     ngx_quic_secrets_t  *current, *next;
 
@@ -744,20 +742,18 @@ ngx_quic_keys_update(ngx_connection_t *c
     next->server.iv.len = NGX_QUIC_IV_LEN;
     next->server.hp = current->server.hp;
 
-    ngx_quic_hkdf_t seq[] = {
-        ngx_quic_hkdf_set("tls13 quic ku",
-                          &next->client.secret, &current->client.secret),
-        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),
-    };
+    ngx_quic_hkdf_set(&seq[0], "tls13 quic ku",
+                      &next->client.secret, &current->client.secret);
+    ngx_quic_hkdf_set(&seq[1], "tls13 quic key",
+                      &next->client.key, &next->client.secret);
+    ngx_quic_hkdf_set(&seq[2], "tls13 quic iv",
+                      &next->client.iv, &next->client.secret);
+    ngx_quic_hkdf_set(&seq[3], "tls13 quic ku",
+                      &next->server.secret, &current->server.secret);
+    ngx_quic_hkdf_set(&seq[4], "tls13 quic key",
+                      &next->server.key, &next->server.secret);
+    ngx_quic_hkdf_set(&seq[5], "tls13 quic iv",
+                      &next->server.iv, &next->server.secret);
 
     for (i = 0; i < (sizeof(seq) / sizeof(seq[0])); i++) {
         if (ngx_quic_hkdf_expand(&seq[i], ciphers.d, c->log) != NGX_OK) {
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1668515080 -14400
#      Tue Nov 15 16:24:40 2022 +0400
# Branch quic
# Node ID bec3d3cdc80388fb71204df2cf778c8d9f2a0321
# Parent  eff714f6d07aa595b0f4f552b76a228b5dd5758f
QUIC: avoid using C99 designated initializers.

They are not supported by MSVC till 2012.

SSL_QUIC_METHOD initialization is moved to run-time to preserve portability
among SSL library implementations, which allows to reduce its visibility.
Note using of a static storage to keep SSL_set_quic_method() reference valid.

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
@@ -147,6 +147,7 @@ ngx_quic_keys_set_initial_secret(ngx_qui
 {
     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];
@@ -176,10 +177,8 @@ ngx_quic_keys_set_initial_secret(ngx_qui
         return NGX_ERROR;
     }
 
-    ngx_str_t iss = {
-        .data = is,
-        .len = is_len
-    };
+    iss.len = is_len;
+    iss.data = is;
 
     ngx_log_debug0(NGX_LOG_DEBUG_EVENT, log, 0,
                    "quic ngx_quic_set_initial_secret");
diff --git a/src/event/quic/ngx_event_quic_ssl.c b/src/event/quic/ngx_event_quic_ssl.c
--- a/src/event/quic/ngx_event_quic_ssl.c
+++ b/src/event/quic/ngx_event_quic_ssl.c
@@ -39,19 +39,6 @@ static int ngx_quic_send_alert(ngx_ssl_c
 static ngx_int_t ngx_quic_crypto_input(ngx_connection_t *c, ngx_chain_t *data);
 
 
-static SSL_QUIC_METHOD quic_method = {
-#if defined OPENSSL_IS_BORINGSSL || defined LIBRESSL_VERSION_NUMBER
-    .set_read_secret = ngx_quic_set_read_secret,
-    .set_write_secret = ngx_quic_set_write_secret,
-#else
-    .set_encryption_secrets = ngx_quic_set_encryption_secrets,
-#endif
-    .add_handshake_data = ngx_quic_add_handshake_data,
-    .flush_flight = ngx_quic_flush_flight,
-    .send_alert = ngx_quic_send_alert,
-};
-
-
 #if defined OPENSSL_IS_BORINGSSL || defined LIBRESSL_VERSION_NUMBER
 
 static int
@@ -533,13 +520,14 @@ ngx_quic_crypto_input(ngx_connection_t *
 ngx_int_t
 ngx_quic_init_connection(ngx_connection_t *c)
 {
-    u_char                 *p;
-    size_t                  clen;
-    ssize_t                 len;
-    ngx_str_t               dcid;
-    ngx_ssl_conn_t         *ssl_conn;
-    ngx_quic_socket_t      *qsock;
-    ngx_quic_connection_t  *qc;
+    u_char                  *p;
+    size_t                   clen;
+    ssize_t                  len;
+    ngx_str_t                dcid;
+    ngx_ssl_conn_t          *ssl_conn;
+    ngx_quic_socket_t       *qsock;
+    ngx_quic_connection_t   *qc;
+    static SSL_QUIC_METHOD   quic_method;
 
     qc = ngx_quic_get_connection(c);
 
@@ -551,6 +539,18 @@ ngx_quic_init_connection(ngx_connection_
 
     ssl_conn = c->ssl->connection;
 
+    if (!quic_method.send_alert) {
+#if defined OPENSSL_IS_BORINGSSL || defined LIBRESSL_VERSION_NUMBER
+        quic_method.set_read_secret = ngx_quic_set_read_secret;
+        quic_method.set_write_secret = ngx_quic_set_write_secret;
+#else
+        quic_method.set_encryption_secrets = ngx_quic_set_encryption_secrets;
+#endif
+        quic_method.add_handshake_data = ngx_quic_add_handshake_data;
+        quic_method.flush_flight = ngx_quic_flush_flight;
+        quic_method.send_alert = ngx_quic_send_alert;
+    }
+
     if (SSL_set_quic_method(ssl_conn, &quic_method) == 0) {
         ngx_log_error(NGX_LOG_INFO, c->log, 0,
                       "quic SSL_set_quic_method() failed");
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1668513723 -14400
#      Tue Nov 15 16:02:03 2022 +0400
# Branch quic
# Node ID 42a9a02b402a44a48cef2b35e09e7fbec7ee8e1e
# Parent  004338233abf98def885a35ea465349d190eb19b
QUIC: fixed C4389 MSVC warning about signed/unsigned mismatch.

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
@@ -988,8 +988,9 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt,
     u_char              *p, *sample;
     size_t               len;
     uint64_t             pn, lpn;
-    ngx_int_t            pnl, rc, key_phase;
+    ngx_int_t            pnl, rc;
     ngx_str_t            in, ad;
+    ngx_uint_t           key_phase;
     ngx_quic_secret_t   *secret;
     ngx_quic_ciphers_t   ciphers;
     uint8_t              nonce[NGX_QUIC_IV_LEN], mask[NGX_QUIC_HP_LEN];
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1668513724 -14400
#      Tue Nov 15 16:02:04 2022 +0400
# Branch quic
# Node ID d4f3464f71f03a28d16484f861057be03ffdd72a
# Parent  42a9a02b402a44a48cef2b35e09e7fbec7ee8e1e
Added shutdown macros for win32 required for QUIC.

diff --git a/src/os/win32/ngx_socket.h b/src/os/win32/ngx_socket.h
--- a/src/os/win32/ngx_socket.h
+++ b/src/os/win32/ngx_socket.h
@@ -14,6 +14,8 @@
 
 
 #define NGX_WRITE_SHUTDOWN SD_SEND
+#define NGX_READ_SHUTDOWN  SD_RECEIVE
+#define NGX_RDWR_SHUTDOWN  SD_BOTH
 
 
 typedef SOCKET  ngx_socket_t;
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1668513728 -14400
#      Tue Nov 15 16:02:08 2022 +0400
# Branch quic
# Node ID d760c2e49d4c4647621765fef3404c34d1aef81b
# Parent  d4f3464f71f03a28d16484f861057be03ffdd72a
QUIC: plug MSVC warning about potentially uninitialized variable.

diff --git a/src/event/quic/ngx_event_quic_tokens.c b/src/event/quic/ngx_event_quic_tokens.c
--- a/src/event/quic/ngx_event_quic_tokens.c
+++ b/src/event/quic/ngx_event_quic_tokens.c
@@ -178,6 +178,10 @@ ngx_quic_validate_token(ngx_connection_t
     u_char             addr_hash[20];
     u_char             tdec[NGX_QUIC_MAX_TOKEN_SIZE];
 
+#if NGX_SUPPRESS_WARN
+    ngx_str_null(&odcid);
+#endif
+
     /* Retry token or NEW_TOKEN in a previous connection */
 
     cipher = EVP_aes_256_cbc();
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1668513998 -14400
#      Tue Nov 15 16:06:38 2022 +0400
# Branch quic
# Node ID 1ba7992c7bb0f801e069adb15b3378b5211a85a8
# Parent  d760c2e49d4c4647621765fef3404c34d1aef81b
QUIC: silenced C4334 MSVC warning about 32 to 64 bits convertion.

diff --git a/src/event/quic/ngx_event_quic_ack.c b/src/event/quic/ngx_event_quic_ack.c
--- a/src/event/quic/ngx_event_quic_ack.c
+++ b/src/event/quic/ngx_event_quic_ack.c
@@ -195,7 +195,7 @@ ngx_quic_rtt_sample(ngx_connection_t *c,
     } else {
         qc->min_rtt = ngx_min(qc->min_rtt, latest_rtt);
 
-        ack_delay = ack->delay * (1 << qc->ctp.ack_delay_exponent) / 1000;
+        ack_delay = ack->delay * (1ULL << qc->ctp.ack_delay_exponent) / 1000;
 
         if (c->ssl->handshaked) {
             ack_delay = ngx_min(ack_delay, qc->ctp.max_ack_delay);

-- 
Sergey Kandaurov



More information about the nginx-devel mailing list