changeset 8370:262396242352 quic

Reworked macros for parsing/assembling packet types. Previously, macros checking a packet type with the long header also checked whether this is a long header. Now it requires a separate preceding check.
author Vladimir Homutov <vl@nginx.com>
date Thu, 30 Apr 2020 12:38:38 +0300
parents bddf704d62c1
children 9d9531431c8c
files src/event/ngx_event_quic.c src/event/ngx_event_quic_protection.c src/event/ngx_event_quic_transport.c src/event/ngx_event_quic_transport.h
diffstat 4 files changed, 105 insertions(+), 110 deletions(-) [+]
line wrap: on
line diff
--- a/src/event/ngx_event_quic.c
+++ b/src/event/ngx_event_quic.c
@@ -2655,16 +2655,19 @@ ngx_quic_send_frames(ngx_connection_t *c
 
     pkt.secret = &keys->server;
 
+    pkt.flags = NGX_QUIC_PKT_FIXED_BIT;
+
     if (start->level == ssl_encryption_initial) {
-        pkt.flags = NGX_QUIC_PKT_INITIAL;
+        pkt.flags |= NGX_QUIC_PKT_LONG | NGX_QUIC_PKT_INITIAL;
         pkt.token = initial_token;
 
     } else if (start->level == ssl_encryption_handshake) {
-        pkt.flags = NGX_QUIC_PKT_HANDSHAKE;
+        pkt.flags |= NGX_QUIC_PKT_LONG | NGX_QUIC_PKT_HANDSHAKE;
 
     } else {
-        // TODO: macro, set FIXED bit
-        pkt.flags = 0x40 | (c->quic->key_phase ? NGX_QUIC_PKT_KPHASE : 0);
+        if (c->quic->key_phase) {
+            pkt.flags |= NGX_QUIC_PKT_KPHASE;
+        }
     }
 
     ngx_quic_set_packet_number(&pkt, ctx);
--- a/src/event/ngx_event_quic_protection.c
+++ b/src/event/ngx_event_quic_protection.c
@@ -948,7 +948,7 @@ ssize_t
 ngx_quic_encrypt(ngx_quic_header_t *pkt, ngx_ssl_conn_t *ssl_conn,
     ngx_str_t *res)
 {
-    if (pkt->level == ssl_encryption_application) {
+    if (ngx_quic_short_pkt(pkt->flags)) {
         return ngx_quic_create_short_packet(pkt, ssl_conn, res);
     }
 
--- a/src/event/ngx_event_quic_transport.c
+++ b/src/event/ngx_event_quic_transport.c
@@ -66,6 +66,8 @@ static u_char *ngx_quic_read_bytes(u_cha
 static u_char *ngx_quic_copy_bytes(u_char *pos, u_char *end, size_t len,
     u_char *dst);
 
+static ngx_int_t ngx_quic_frame_allowed(ngx_quic_header_t *pkt,
+    ngx_uint_t frame_type);
 static size_t ngx_quic_create_ack(u_char *p, ngx_quic_ack_frame_t *ack);
 static size_t ngx_quic_create_crypto(u_char *p,
     ngx_quic_crypto_frame_t *crypto);
@@ -528,11 +530,9 @@ ngx_quic_parse_frame(ngx_quic_header_t *
     ngx_quic_frame_t *f)
 {
     u_char      *p;
-    uint8_t      flags;
     uint64_t     varint;
     ngx_uint_t   i;
 
-    flags = pkt->flags;
     p = start;
 
     p = ngx_quic_parse_int(p, end, &varint);
@@ -544,14 +544,14 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     f->type = varint;
 
+    if (ngx_quic_frame_allowed(pkt, f->type) != NGX_OK) {
+        return NGX_DECLINED;
+    }
+
     switch (f->type) {
 
     case NGX_QUIC_FT_CRYPTO:
 
-        if (ngx_quic_pkt_zrtt(flags)) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.crypto.offset);
         if (p == NULL) {
             goto error;
@@ -580,8 +580,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_PADDING:
 
-        /* allowed in any packet type */
-
         while (p < end && *p == NGX_QUIC_FT_PADDING) {
             p++;
         }
@@ -591,10 +589,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
     case NGX_QUIC_FT_ACK:
     case NGX_QUIC_FT_ACK_ECN:
 
-        if (ngx_quic_pkt_zrtt(flags)) {
-            goto not_allowed;
-        }
-
         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))
@@ -644,17 +638,10 @@ ngx_quic_parse_frame(ngx_quic_header_t *
         break;
 
     case NGX_QUIC_FT_PING:
-
-        /* allowed in any packet type */
-
         break;
 
     case NGX_QUIC_FT_NEW_CONNECTION_ID:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.ncid.seqnum);
         if (p == NULL) {
             goto error;
@@ -686,19 +673,10 @@ ngx_quic_parse_frame(ngx_quic_header_t *
         break;
 
     case NGX_QUIC_FT_CONNECTION_CLOSE2:
-
-        if (!ngx_quic_short_pkt(flags)) {
-            goto not_allowed;
-        }
-
         /* fall through */
 
     case NGX_QUIC_FT_CONNECTION_CLOSE:
 
-        if (ngx_quic_pkt_zrtt(flags)) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.close.error_code);
         if (p == NULL) {
             goto error;
@@ -751,10 +729,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
     case NGX_QUIC_FT_STREAM6:
     case NGX_QUIC_FT_STREAM7:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         f->u.stream.type = f->type;
 
         f->u.stream.off = ngx_quic_stream_bit_off(f->type);
@@ -807,10 +781,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_MAX_DATA:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.max_data.max_data);
         if (p == NULL) {
             goto error;
@@ -823,10 +793,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_RESET_STREAM:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         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,
@@ -844,10 +810,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_STOP_SENDING:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.stop_sending.id);
         if (p == NULL) {
             goto error;
@@ -867,10 +829,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
     case NGX_QUIC_FT_STREAMS_BLOCKED:
     case NGX_QUIC_FT_STREAMS_BLOCKED2:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.streams_blocked.limit);
         if (p == NULL) {
             goto error;
@@ -886,16 +844,7 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
         break;
 
-    case NGX_QUIC_FT_HANDSHAKE_DONE:
-        /* only sent by server, not by client */
-        goto not_allowed;
-
     case NGX_QUIC_FT_NEW_TOKEN:
-
-        if (!ngx_quic_short_pkt(flags)) {
-            goto not_allowed;
-        }
-
         /* TODO: implement */
 
         ngx_log_error(NGX_LOG_ALERT, pkt->log, 0,
@@ -906,10 +855,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
     case NGX_QUIC_FT_MAX_STREAMS:
     case NGX_QUIC_FT_MAX_STREAMS2:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.max_streams.limit);
         if (p == NULL) {
             goto error;
@@ -925,10 +870,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_MAX_STREAM_DATA:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.max_stream_data.id);
         if (p == NULL) {
             goto error;
@@ -947,10 +888,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_DATA_BLOCKED:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.data_blocked.limit);
         if (p == NULL) {
             goto error;
@@ -963,10 +900,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_STREAM_DATA_BLOCKED:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.stream_data_blocked.id);
         if (p == NULL) {
             goto error;
@@ -986,10 +919,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_RETIRE_CONNECTION_ID:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_parse_int(p, end, &f->u.retire_cid.sequence_number);
         if (p == NULL) {
             goto error;
@@ -1003,10 +932,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_PATH_CHALLENGE:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_copy_bytes(p, end, 8, f->u.path_challenge.data);
         if (p == NULL) {
             goto error;
@@ -1023,10 +948,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     case NGX_QUIC_FT_PATH_RESPONSE:
 
-        if (!(ngx_quic_short_pkt(flags) || ngx_quic_pkt_zrtt(flags))) {
-            goto not_allowed;
-        }
-
         p = ngx_quic_copy_bytes(p, end, 8, f->u.path_response.data);
         if (p == NULL) {
             goto error;
@@ -1049,15 +970,6 @@ ngx_quic_parse_frame(ngx_quic_header_t *
 
     return p - start;
 
-not_allowed:
-
-    ngx_log_error(NGX_LOG_INFO, pkt->log, 0,
-                  "quic frame type 0x%xi is not "
-                  "allowed in packet with flags 0x%xi",
-                  f->type, pkt->flags);
-
-    return NGX_DECLINED;
-
 error:
 
     ngx_log_error(NGX_LOG_INFO, pkt->log, 0,
@@ -1067,6 +979,75 @@ error:
 }
 
 
+static ngx_int_t
+ngx_quic_frame_allowed(ngx_quic_header_t *pkt, ngx_uint_t frame_type)
+{
+    uint8_t  ptype;
+
+    /* frame permissions per packet: 4 bits: IH01: 12.4, Table 3 */
+    static uint8_t ngx_quic_frame_masks[] = {
+         /* PADDING  */              0xF,
+         /* PING */                  0xF,
+         /* ACK */                   0xD,
+         /* ACK_ECN */               0xD,
+         /* RESET_STREAM */          0x3,
+         /* STOP_SENDING */          0x3,
+         /* CRYPTO */                0xD,
+         /* NEW_TOKEN */             0x1,
+         /* STREAM0 */               0x3,
+         /* STREAM1 */               0x3,
+         /* STREAM2 */               0x3,
+         /* STREAM3 */               0x3,
+         /* STREAM4 */               0x3,
+         /* STREAM5 */               0x3,
+         /* STREAM6 */               0x3,
+         /* STREAM7 */               0x3,
+         /* MAX_DATA */              0x3,
+         /* MAX_STREAM_DATA */       0x3,
+         /* MAX_STREAMS */           0x3,
+         /* MAX_STREAMS2 */          0x3,
+         /* DATA_BLOCKED */          0x3,
+         /* STREAM_DATA_BLOCKED */   0x3,
+         /* STREAMS_BLOCKED */       0x3,
+         /* STREAMS_BLOCKED2 */      0x3,
+         /* NEW_CONNECTION_ID */     0x3,
+         /* RETIRE_CONNECTION_ID */  0x3,
+         /* PATH_CHALLENGE */        0x3,
+         /* PATH_RESPONSE */         0x3,
+         /* CONNECTION_CLOSE */      0xD,
+         /* CONNECTION_CLOSE2 */     0x1,
+         /* HANDSHAKE_DONE */        0x0, /* only sent by server */
+    };
+
+    if (ngx_quic_long_pkt(pkt->flags)) {
+
+        if (ngx_quic_pkt_in(pkt->flags)) {
+            ptype = 8; /* initial */
+
+        } else if (ngx_quic_pkt_hs(pkt->flags)) {
+            ptype = 4; /* handshake */
+
+        } else {
+            ptype = 2; /* zero-rtt */
+        }
+
+    } else {
+        ptype = 1; /* application data */
+    }
+
+    if (ptype & ngx_quic_frame_masks[frame_type]) {
+        return NGX_OK;
+    }
+
+    ngx_log_error(NGX_LOG_INFO, pkt->log, 0,
+                  "quic frame type 0x%xi is not "
+                  "allowed in packet with flags 0x%xi",
+                  frame_type, pkt->flags);
+
+    return NGX_DECLINED;
+}
+
+
 ssize_t
 ngx_quic_parse_ack_range(ngx_quic_header_t *pkt, u_char *start, u_char *end,
     uint64_t *gap, uint64_t *range)
--- a/src/event/ngx_event_quic_transport.h
+++ b/src/event/ngx_event_quic_transport.h
@@ -12,20 +12,31 @@
 #include <ngx_core.h>
 
 
-#define ngx_quic_long_pkt(flags)  ((flags) & 0x80)            /* 17.2   */
-#define ngx_quic_short_pkt(flags) (((flags) & 0x80) == 0)     /* 17.3   */
+/* QUIC flags in first byte, see quic-transport 17.2 and 17.3 */
+
+#define NGX_QUIC_PKT_LONG       0x80  /* header form */
+#define NGX_QUIC_PKT_FIXED_BIT  0x40
+#define NGX_QUIC_PKT_TYPE       0x30  /* in long packet */
+#define NGX_QUIC_PKT_KPHASE     0x04  /* in short packet */
+
+#define ngx_quic_long_pkt(flags)  ((flags) & NGX_QUIC_PKT_LONG)
+#define ngx_quic_short_pkt(flags)  (((flags) & NGX_QUIC_PKT_LONG) == 0)
 
 /* Long packet types */
-#define NGX_QUIC_PKT_INITIAL                             0xC0 /* 17.2.2 */
-#define NGX_QUIC_PKT_ZRTT                                0xD0 /* 17.2.3 */
-#define NGX_QUIC_PKT_HANDSHAKE                           0xE0 /* 17.2.4 */
-#define NGX_QUIC_PKT_RETRY                               0xF0 /* 17.2.5 */
-#define NGX_QUIC_PKT_KPHASE                              0x04 /* 17.3   */
+#define NGX_QUIC_PKT_INITIAL    0x00
+#define NGX_QUIC_PKT_ZRTT       0x10
+#define NGX_QUIC_PKT_HANDSHAKE  0x20
+#define NGX_QUIC_PKT_RETRY      0x30
 
-#define ngx_quic_pkt_in(flags)     (((flags) & 0xF0) == NGX_QUIC_PKT_INITIAL)
-#define ngx_quic_pkt_zrtt(flags)   (((flags) & 0xF0) == NGX_QUIC_PKT_ZRTT)
-#define ngx_quic_pkt_hs(flags)     (((flags) & 0xF0) == NGX_QUIC_PKT_HANDSHAKE)
-#define ngx_quic_pkt_retry(flags)  (((flags) & 0xF0) == NGX_QUIC_PKT_RETRY)
+#define ngx_quic_pkt_in(flags)                                                \
+    (((flags) & NGX_QUIC_PKT_TYPE) == NGX_QUIC_PKT_INITIAL)
+#define ngx_quic_pkt_zrtt(flags)                                              \
+    (((flags) & NGX_QUIC_PKT_TYPE) == NGX_QUIC_PKT_ZRTT)
+#define ngx_quic_pkt_hs(flags)                                                \
+    (((flags) & NGX_QUIC_PKT_TYPE) == NGX_QUIC_PKT_HANDSHAKE)
+#define ngx_quic_pkt_retry(flags)                                             \
+    (((flags) & NGX_QUIC_PKT_TYPE) == NGX_QUIC_PKT_RETRY)
+
 
 /* 12.4.  Frames and Frame Types */
 #define NGX_QUIC_FT_PADDING                              0x00