Discard packets without fixed bit or reserved bits set.

Section 17.2 and 17.3 of QUIC transport:

Fixed bit: Packets containing a zero value for this bit are not
valid packets in this version and MUST be discarded.

Reserved bit: An endpoint MUST treat receipt of a packet that has
a non-zero value for these bits, after removing both packet and
header protection, as a connection error of type PROTOCOL_VIOLATION.

4 files changed, 64 insertions, 13 deletions

diff --git a/src/event/ngx_event_quic.c b/src/event/ngx_event_quic.c
index bbe1e79c3..26e307bbe 100644
--- a/src/event/ngx_event_quic.c
+++ b/src/event/ngx_event_quic.c
@@ -564,8 +564,9 @@ ngx_quic_new_connection(ngx_connection_t *c, ngx_ssl_t *ssl, ngx_quic_tp_t *tp,
         return NGX_ERROR;
     }
 
-    if (ngx_quic_parse_long_header(pkt) != NGX_OK) {
-        return NGX_ERROR;
+    rc = ngx_quic_parse_long_header(pkt);
+    if (rc != NGX_OK) {
+        return rc;
     }
 
     if (!ngx_quic_pkt_in(pkt->flags)) {
@@ -676,6 +677,7 @@ ngx_quic_new_connection(ngx_connection_t *c, ngx_ssl_t *ssl, ngx_quic_tp_t *tp,
     ctx = ngx_quic_get_send_ctx(qc, pkt->level);
 
     if (ngx_quic_decrypt(pkt, NULL, &ctx->largest_pn) != NGX_OK) {
+        qc->error = pkt->error;
         return NGX_ERROR;
     }
 
@@ -1420,6 +1422,7 @@ ngx_quic_skip_zero_padding(ngx_buf_t *b)
 static ngx_int_t
 ngx_quic_retry_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 {
+    ngx_int_t               rc;
     ngx_quic_secrets_t     *keys;
     ngx_quic_send_ctx_t    *ctx;
     ngx_quic_connection_t  *qc;
@@ -1435,8 +1438,9 @@ ngx_quic_retry_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
         return NGX_OK;
     }
 
-    if (ngx_quic_parse_long_header(pkt) != NGX_OK) {
-        return NGX_ERROR;
+    rc = ngx_quic_parse_long_header(pkt);
+    if (rc != NGX_OK) {
+        return rc;
     }
 
     if (ngx_quic_pkt_zrtt(pkt->flags)) {
@@ -1484,6 +1488,7 @@ ngx_quic_retry_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
     ctx = ngx_quic_get_send_ctx(qc, pkt->level);
 
     if (ngx_quic_decrypt(pkt, NULL, &ctx->largest_pn) != NGX_OK) {
+        qc->error = pkt->error;
         return NGX_ERROR;
     }
 
@@ -1507,6 +1512,7 @@ ngx_quic_retry_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 static ngx_int_t
 ngx_quic_initial_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 {
+    ngx_int_t             rc;
     ngx_ssl_conn_t       *ssl_conn;
     ngx_quic_secrets_t   *keys;
     ngx_quic_send_ctx_t  *ctx;
@@ -1516,8 +1522,9 @@ ngx_quic_initial_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 
     ssl_conn = c->ssl->connection;
 
-    if (ngx_quic_parse_long_header(pkt) != NGX_OK) {
-        return NGX_ERROR;
+    rc = ngx_quic_parse_long_header(pkt);
+    if (rc != NGX_OK) {
+        return rc;
     }
 
     if (ngx_quic_parse_initial_header(pkt) != NGX_OK) {
@@ -1533,6 +1540,7 @@ ngx_quic_initial_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
     ctx = ngx_quic_get_send_ctx(c->quic, pkt->level);
 
     if (ngx_quic_decrypt(pkt, ssl_conn, &ctx->largest_pn) != NGX_OK) {
+        c->quic->error = pkt->error;
         return NGX_ERROR;
     }
 
@@ -1543,6 +1551,7 @@ ngx_quic_initial_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 static ngx_int_t
 ngx_quic_handshake_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 {
+    ngx_int_t               rc;
     ngx_quic_secrets_t     *keys;
     ngx_quic_send_ctx_t    *ctx;
     ngx_quic_connection_t  *qc;
@@ -1561,8 +1570,9 @@ ngx_quic_handshake_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
     }
 
     /* extract cleartext data into pkt */
-    if (ngx_quic_parse_long_header(pkt) != NGX_OK) {
-        return NGX_ERROR;
+    rc = ngx_quic_parse_long_header(pkt);
+    if (rc != NGX_OK) {
+        return rc;
     }
 
     if (ngx_quic_check_peer(qc, pkt) != NGX_OK) {
@@ -1580,6 +1590,7 @@ ngx_quic_handshake_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
     ctx = ngx_quic_get_send_ctx(qc, pkt->level);
 
     if (ngx_quic_decrypt(pkt, c->ssl->connection, &ctx->largest_pn) != NGX_OK) {
+        qc->error = pkt->error;
         return NGX_ERROR;
     }
 
@@ -1590,6 +1601,7 @@ ngx_quic_handshake_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 static ngx_int_t
 ngx_quic_early_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
 {
+    ngx_int_t               rc;
     ngx_quic_secrets_t     *keys;
     ngx_quic_send_ctx_t    *ctx;
     ngx_quic_connection_t  *qc;
@@ -1600,8 +1612,9 @@ ngx_quic_early_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
     qc = c->quic;
 
     /* extract cleartext data into pkt */
-    if (ngx_quic_parse_long_header(pkt) != NGX_OK) {
-        return NGX_ERROR;
+    rc = ngx_quic_parse_long_header(pkt);
+    if (rc != NGX_OK) {
+        return rc;
     }
 
     if (ngx_quic_check_peer(qc, pkt) != NGX_OK) {
@@ -1626,6 +1639,7 @@ ngx_quic_early_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
     ctx = ngx_quic_get_send_ctx(qc, pkt->level);
 
     if (ngx_quic_decrypt(pkt, c->ssl->connection, &ctx->largest_pn) != NGX_OK) {
+        qc->error = pkt->error;
         return NGX_ERROR;
     }
 
@@ -1686,8 +1700,9 @@ ngx_quic_app_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
         return NGX_DECLINED;
     }
 
-    if (ngx_quic_parse_short_header(pkt, &qc->dcid) != NGX_OK) {
-        return NGX_ERROR;
+    rc = ngx_quic_parse_short_header(pkt, &qc->dcid);
+    if (rc != NGX_OK) {
+        return rc;
     }
 
     pkt->secret = &keys->client;
@@ -1699,6 +1714,7 @@ ngx_quic_app_input(ngx_connection_t *c, ngx_quic_header_t *pkt)
     ctx = ngx_quic_get_send_ctx(qc, pkt->level);
 
     if (ngx_quic_decrypt(pkt, c->ssl->connection, &ctx->largest_pn) != NGX_OK) {
+        qc->error = pkt->error;
         return NGX_ERROR;
     }
 
diff --git a/src/event/ngx_event_quic_protection.c b/src/event/ngx_event_quic_protection.c
index 97fc58ecc..e44a13dee 100644
--- a/src/event/ngx_event_quic_protection.c
+++ b/src/event/ngx_event_quic_protection.c
@@ -1014,6 +1014,7 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt, ngx_ssl_conn_t *ssl_conn,
     uint64_t *largest_pn)
 {
     u_char               clearflags, *p, *sample;
+    uint8_t              badflags;
     uint64_t             pn;
     ngx_int_t            pnl, rc, key_phase;
     ngx_str_t            in, ad;
@@ -1048,6 +1049,7 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt, ngx_ssl_conn_t *ssl_conn,
     if (ngx_quic_tls_hp(pkt->log, ciphers.hp, secret, mask, sample)
         != NGX_OK)
     {
+        pkt->error = NGX_QUIC_ERR_CRYPTO_ERROR;
         return NGX_ERROR;
     }
 
@@ -1085,9 +1087,11 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt, ngx_ssl_conn_t *ssl_conn,
 
     if (ngx_quic_long_pkt(pkt->flags)) {
         in.len = pkt->len - pnl;
+        badflags = clearflags & NGX_QUIC_PKT_LONG_RESERVED_BIT;
 
     } else {
         in.len = pkt->data + pkt->len - p;
+        badflags = clearflags & NGX_QUIC_PKT_SHORT_RESERVED_BIT;
     }
 
     ad.len = p - pkt->data;
@@ -1124,6 +1128,24 @@ ngx_quic_decrypt(ngx_quic_header_t *pkt, ngx_ssl_conn_t *ssl_conn,
                      pkt->payload.data, pkt->payload.len);
 #endif
 
-    return rc;
+    if (rc != NGX_OK) {
+        pkt->error = NGX_QUIC_ERR_CRYPTO_ERROR;
+        return rc;
+    }
+
+    if (badflags) {
+        /*
+         * An endpoint MUST treat receipt of a packet that has
+         * a non-zero value for these bits, after removing both
+         * packet and header protection, as a connection error
+         * of type PROTOCOL_VIOLATION.
+         */
+        ngx_log_error(NGX_LOG_INFO, pkt->log, 0,
+                      "quic reserved bit set in packet");
+        pkt->error = NGX_QUIC_ERR_PROTOCOL_VIOLATION;
+        return NGX_ERROR;
+    }
+
+    return NGX_OK;
 }
 
diff --git a/src/event/ngx_event_quic_transport.c b/src/event/ngx_event_quic_transport.c
index 4dcb78abd..1732f8b0f 100644
--- a/src/event/ngx_event_quic_transport.c
+++ b/src/event/ngx_event_quic_transport.c
@@ -265,6 +265,11 @@ ngx_quic_parse_long_header(ngx_quic_header_t *pkt)
                    "quic long packet flags:%xi version:%xD",
                    pkt->flags, pkt->version);
 
+    if (!(pkt->flags & NGX_QUIC_PKT_FIXED_BIT)) {
+        ngx_log_error(NGX_LOG_INFO, pkt->log, 0, "quic fixed bit is not set");
+        return NGX_DECLINED;
+    }
+
     if (pkt->version != NGX_QUIC_VERSION) {
         ngx_log_error(NGX_LOG_INFO, pkt->log, 0,
                       "quic unsupported version: 0x%xi", pkt->version);
@@ -443,6 +448,11 @@ ngx_quic_parse_short_header(ngx_quic_header_t *pkt, ngx_str_t *dcid)
     ngx_log_debug1(NGX_LOG_DEBUG_EVENT, pkt->log, 0,
                    "quic short packet flags:%xi", pkt->flags);
 
+    if (!(pkt->flags & NGX_QUIC_PKT_FIXED_BIT)) {
+        ngx_log_error(NGX_LOG_INFO, pkt->log, 0, "quic fixed bit is not set");
+        return NGX_DECLINED;
+    }
+
     if (ngx_memcmp(p, dcid->data, dcid->len) != 0) {
         ngx_log_error(NGX_LOG_INFO, pkt->log, 0, "unexpected quic dcid");
         return NGX_ERROR;
diff --git a/src/event/ngx_event_quic_transport.h b/src/event/ngx_event_quic_transport.h
index 364f827ac..c3b2bbf01 100644
--- a/src/event/ngx_event_quic_transport.h
+++ b/src/event/ngx_event_quic_transport.h
@@ -19,6 +19,9 @@
 #define NGX_QUIC_PKT_TYPE       0x30  /* in long packet */
 #define NGX_QUIC_PKT_KPHASE     0x04  /* in short packet */
 
+#define NGX_QUIC_PKT_LONG_RESERVED_BIT   0x0C
+#define NGX_QUIC_PKT_SHORT_RESERVED_BIT  0x18
+
 #define ngx_quic_long_pkt(flags)  ((flags) & NGX_QUIC_PKT_LONG)
 #define ngx_quic_short_pkt(flags)  (((flags) & NGX_QUIC_PKT_LONG) == 0)