# HG changeset patch # User Vladimir Homutov # Date 1589407605 -10800 # Node ID 81f85c479d7eedd37e72b1df7f760df7fa6c1a83 # Parent fb7422074258c84f4acad392638dcc2399d1d8fb 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. diff --git a/src/event/ngx_event_quic.c b/src/event/ngx_event_quic.c --- 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 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 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 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 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 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 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 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 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_ } /* 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_ 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_ 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 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 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, 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, 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 --- 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, 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, 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, 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, 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 --- 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_head "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_hea 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 --- 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)