diff --git a/src/proxy/http3/Http3Config.cc b/src/proxy/http3/Http3Config.cc index e66af7204cb..9f4e459deee 100644 --- a/src/proxy/http3/Http3Config.cc +++ b/src/proxy/http3/Http3Config.cc @@ -23,6 +23,8 @@ #include "proxy/http3/Http3Config.h" +#include "tscore/Diags.h" + int ts::Http3Config::_config_id = 0; // @@ -36,6 +38,16 @@ ts::Http3ConfigParams::initialize() RecEstablishStaticConfigUInt32(this->_qpack_blocked_streams, "proxy.config.http3.qpack_blocked_streams"); RecEstablishStaticConfigUInt32(this->_num_placeholders, "proxy.config.http3.num_placeholders"); RecEstablishStaticConfigUInt32(this->_max_settings, "proxy.config.http3.max_settings"); + + // The QPACK dynamic table is not implemented: XpackDynamicTable never stores + // entries (its capacity is fixed at zero and never raised). Advertising a + // non-zero table size or blocked-stream count would tell a peer it may + // insert entries our decoder silently drops and then reference them, which + // breaks decoding. Refuse to start until the dynamic table is implemented. + if (this->_header_table_size != 0 || this->_qpack_blocked_streams != 0) { + Fatal("HTTP/3 QPACK dynamic table is not implemented; proxy.config.http3.header_table_size and " + "proxy.config.http3.qpack_blocked_streams must be 0"); + } } uint32_t diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index dfdd2d278b3..d25a8690ace 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -27,6 +27,10 @@ #include "tscore/ink_defs.h" #include "tscore/ink_memory.h" +#include +#include +#include + #define QPACKDebug(fmt, ...) Dbg(dbg_ctl_qpack, "[%s] " fmt, this->_qc->cids().data(), ##__VA_ARGS__) #define QPACKDTDebug(fmt, ...) Dbg(dbg_ctl_qpack, "" fmt, ##__VA_ARGS__) @@ -69,7 +73,7 @@ const QPACK::Header QPACK::StaticTable::STATIC_HEADER_FIELDS[] = { {":status", "503" }, {"accept", "*/*" }, {"accept", "application/dns-message" }, - {"accept-encoding", "gzip, deflate, br, zstd" }, + {"accept-encoding", "gzip, deflate, br" }, {"accept-ranges", "bytes" }, {"access-control-allow-headers", "cache-control" }, {"access-control-allow-headers", "content-type" }, @@ -80,7 +84,6 @@ const QPACK::Header QPACK::StaticTable::STATIC_HEADER_FIELDS[] = { {"cache-control", "no-cache" }, {"cache-control", "no-store" }, {"cache-control", "public, max-age=31536000" }, - {"content-encoding", "zstd" }, {"content-encoding", "br" }, {"content-encoding", "gzip" }, {"content-type", "application/dns-message" }, @@ -283,11 +286,20 @@ QPACK::decode(uint64_t stream_id, const uint8_t *header_block, size_t header_blo uint64_t tmp = 0; int64_t ret = xpack_decode_integer(tmp, header_block, header_block + header_block_len, 8); - if (ret < 0 && tmp > 0xFFFF) { + if (ret < 0 || tmp > 0xFFFF) { return -1; } uint16_t largest_reference = tmp; + // A non-zero Required Insert Count references the dynamic table. We only + // store dynamic entries once a non-zero table capacity is negotiated (and we + // currently always advertise zero), so when the table capacity is zero such + // a reference can never be satisfied. Treat it as a decoding failure instead + // of queuing the stream as blocked forever. + if (largest_reference != 0 && this->_dynamic_table.maximum_size() == 0) { + return -1; + } + if (largest_reference != 0 && (this->_dynamic_table.is_empty() || this->_dynamic_table.largest_index() < largest_reference)) { // Blocked if (this->_add_to_blocked_list( @@ -913,21 +925,21 @@ QPACK::_decode_literal_header_field_with_postbase_name_ref(int16_t base_index, c int QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTPHdr &hdr) { - const uint8_t *pos = header_block; - size_t remain_len = header_block_len; - int64_t ret; + const uint8_t *pos = header_block; + const uint8_t *const end = header_block + header_block_len; + int64_t ret; // Decode Header Data Prefix - uint64_t tmp; - if ((ret = xpack_decode_integer(tmp, pos, pos + remain_len, 8)) < 0 && tmp > 0xFFFF) { + uint64_t tmp = 0; + if ((ret = xpack_decode_integer(tmp, pos, end, 8)) < 0 || tmp > 0xFFFF) { return -1; } pos += ret; uint16_t largest_reference = tmp; - uint64_t delta_base_index; + uint64_t delta_base_index = 0; uint16_t base_index; - if ((ret = xpack_decode_integer(delta_base_index, pos, pos + remain_len, 7)) < 0 && delta_base_index < 0xFFFF) { + if ((ret = xpack_decode_integer(delta_base_index, pos, end, 7)) < 0 || delta_base_index > 0xFFFF) { return -2; } @@ -944,7 +956,8 @@ QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTP uint32_t decoded_header_list_size = 0; // Decode Instructions - while (pos < header_block + header_block_len) { + while (pos < end) { + size_t remain_len = end - pos; uint32_t header_len = 0; if (pos[0] & 0x80) { // Index Header Field @@ -1226,26 +1239,42 @@ QPACK::StaticTable::lookup(uint16_t index, const char **name, size_t *name_len, const XpackLookupResult QPACK::StaticTable::lookup(const char *name, size_t name_len, const char *value, size_t value_len) { + // The static table (RFC 9204 Appendix A) is not ordered by name and its + // indices are part of the wire encoding, so it cannot be reordered. Keep an + // auxiliary index sorted by name (ties broken by ascending table index) so a + // lookup scans only the entries sharing the requested name instead of all of + // them. Returns the same index the former linear scan did: the sole exact + // match, or the highest-indexed name match otherwise. + static const auto name_order = [] { + std::array order; + for (uint16_t i = 0; i < order.size(); ++i) { + order[i] = i; + } + std::sort(order.begin(), order.end(), [](uint16_t l, uint16_t r) { + const std::string_view ln{STATIC_HEADER_FIELDS[l].name, STATIC_HEADER_FIELDS[l].name_len}; + const std::string_view rn{STATIC_HEADER_FIELDS[r].name, STATIC_HEADER_FIELDS[r].name_len}; + return ln != rn ? ln < rn : l < r; + }); + return order; + }(); + + const std::string_view target{name, name_len}; + auto it = std::lower_bound(name_order.begin(), name_order.end(), target, [](uint16_t idx, std::string_view t) { + return std::string_view{STATIC_HEADER_FIELDS[idx].name, STATIC_HEADER_FIELDS[idx].name_len} < t; + }); + XpackLookupResult::MatchType match_type = XpackLookupResult::MatchType::NONE; - uint16_t i = 0; uint16_t candidate_index = 0; - int n = countof(STATIC_HEADER_FIELDS); - - for (; i < n; ++i) { - const Header &h = STATIC_HEADER_FIELDS[i]; - if (h.name_len == name_len) { - if (memcmp(name, h.name, name_len) == 0) { - candidate_index = i; - if (value_len == h.value_len && memcmp(value, h.value, value_len) == 0) { - // Exact match - match_type = XpackLookupResult::MatchType::EXACT; - break; - } else { - // Name match -- Keep it for no exact matches - match_type = XpackLookupResult::MatchType::NAME; - } - } + for (; it != name_order.end(); ++it) { + const Header &h = STATIC_HEADER_FIELDS[*it]; + if (h.name_len != name_len || memcmp(h.name, name, name_len) != 0) { + break; // Past the run of entries sharing this name. + } + candidate_index = *it; + if (value_len == h.value_len && memcmp(value, h.value, value_len) == 0) { + return {candidate_index, XpackLookupResult::MatchType::EXACT}; } + match_type = XpackLookupResult::MatchType::NAME; } return {candidate_index, match_type}; } diff --git a/src/proxy/http3/test/test_QPACK.cc b/src/proxy/http3/test/test_QPACK.cc index 438571d1149..f499ca0cfe6 100644 --- a/src/proxy/http3/test/test_QPACK.cc +++ b/src/proxy/http3/test/test_QPACK.cc @@ -469,3 +469,83 @@ TEST_CASE("Decoding", "[qpack-decode]") } } } + +// Encode an "Indexed Header Field" (RFC 9204 4.5.2) that references a +// static-table entry, returning the number of bytes written. +static int +append_static_indexed_field(uint8_t *buf, const uint8_t *buf_end, uint16_t index) +{ + int64_t n = xpack_encode_integer(buf, buf_end, index, 6); + buf[0] |= 0xC0; // '1' Indexed Header Field + 'T'=1 static table + return static_cast(n); +} + +// The QPACK static table (RFC 9204 Appendix A) is normative: its 99 indices are +// wire values shared with every peer and must not be reordered. These cases +// span the range a previous change shifted by inserting a non-standard entry; +// decoding each index must yield the exact RFC name and value. +TEST_CASE("QPACK static table conforms to RFC 9204", "[qpack]") +{ + struct { + uint16_t index; + const char *name; + const char *value; + } const cases[] = { + {31, "accept-encoding", "gzip, deflate, br" }, + {42, "content-encoding", "br" }, + {52, "content-type", "text/html; charset=utf-8"}, + {71, ":status", "500" }, + {98, "x-frame-options", "sameorigin" }, + }; + + QUICApplicationDriver driver; + QPACK *qpack = new QPACK(driver.get_connection(), UINT32_MAX, 0, 0); + TestQPACKEventHandler *event_handler = new TestQPACKEventHandler(); + + HTTPHdr hdr; + hdr.create(HTTPType::RESPONSE); + + uint8_t block[256]; + int len = 0; + block[len++] = 0x00; // Required Insert Count = 0 + block[len++] = 0x00; // S = 0, Delta Base = 0 + for (auto const &c : cases) { + len += append_static_indexed_field(block + len, block + sizeof(block), c.index); + } + + // decode() populates the header set synchronously; only the completion event + // is scheduled, so the fields can be inspected as soon as it returns. + int ret = qpack->decode(1, block, len, hdr, event_handler, eventProcessor.all_ethreads[0]); + REQUIRE(ret == 0); + + for (auto const &c : cases) { + MIMEField *field = hdr.field_find(std::string_view{c.name}); + REQUIRE(field != nullptr); + CHECK(field->value_get() == std::string_view{c.value}); + } + + hdr.destroy(); +} + +// ATS advertises a zero-capacity QPACK dynamic table, so a header block whose +// Required Insert Count is non-zero references entries that can never exist. +// decode() must fail rather than queue the stream as blocked forever. +TEST_CASE("QPACK decode rejects a dynamic reference with no dynamic table", "[qpack]") +{ + QUICApplicationDriver driver; + QPACK *qpack = new QPACK(driver.get_connection(), UINT32_MAX, 0, 0); + TestQPACKEventHandler *event_handler = new TestQPACKEventHandler(); + + HTTPHdr hdr; + hdr.create(HTTPType::REQUEST); + + uint8_t block[8]; + int len = 0; + len += xpack_encode_integer(block + len, block + sizeof(block), 1, 8); // Required Insert Count = 1 + block[len++] = 0x00; // S = 0, Delta Base = 0 + + int ret = qpack->decode(1, block, len, hdr, event_handler, eventProcessor.all_ethreads[0]); + CHECK(ret < 0); + + hdr.destroy(); +}