From 66f9f8aeb49c32fc6da754e1a033cc411d2f57c9 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Tue, 9 Jul 2024 10:14:29 +0200 Subject: [PATCH 1/5] dns: don't error if header id is 0 Signed-off-by: Mike Beaumont --- source/extensions/filters/udp/dns_filter/dns_parser.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/source/extensions/filters/udp/dns_filter/dns_parser.cc b/source/extensions/filters/udp/dns_filter/dns_parser.cc index ed4b850a5c8b..2c280ff9f3c4 100644 --- a/source/extensions/filters/udp/dns_filter/dns_parser.cc +++ b/source/extensions/filters/udp/dns_filter/dns_parser.cc @@ -217,10 +217,6 @@ bool DnsMessageParser::parseDnsObject(DnsQueryContextPtr& context, } context->id_ = static_cast(context->header_.id); - if (context->id_ == 0) { - ENVOY_LOG(debug, "No ID in DNS query"); - return false; - } // Almost always, we will have only one query here. Per the RFC, QDCOUNT is usually 1 context->queries_.reserve(context->header_.questions); From 19af5fb6b2ff720ad7dd9e81fe74512fa1c7057f Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Tue, 9 Jul 2024 13:08:46 +0200 Subject: [PATCH 2/5] test: don't error if DNS message header is 0 Signed-off-by: Mike Beaumont --- .../filters/udp/dns_filter/dns_filter_test_utils.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/extensions/filters/udp/dns_filter/dns_filter_test_utils.cc b/test/extensions/filters/udp/dns_filter/dns_filter_test_utils.cc index ed873d300424..f2bb9fc1264a 100644 --- a/test/extensions/filters/udp/dns_filter/dns_filter_test_utils.cc +++ b/test/extensions/filters/udp/dns_filter/dns_filter_test_utils.cc @@ -182,10 +182,6 @@ bool DnsResponseValidator::validateDnsResponseObject(DnsQueryContextPtr& context } context->id_ = static_cast(context->header_.id); - if (context->id_ == 0) { - ENVOY_LOG(debug, "No ID in DNS Response"); - return false; - } // Almost always, we will have only one query here. Per the RFC, QDCOUNT is usually 1 context->queries_.reserve(context->header_.questions); From 4a7f4b1a439438a6903e0c524616ffc25ddf9be2 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Tue, 9 Jul 2024 21:10:35 +0200 Subject: [PATCH 3/5] test: fix expectations with ID zero Signed-off-by: Mike Beaumont --- .../extensions/filters/udp/dns_filter/dns_filter_test.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/extensions/filters/udp/dns_filter/dns_filter_test.cc b/test/extensions/filters/udp/dns_filter/dns_filter_test.cc index cd2adfa09bf4..5af78ddd4b77 100644 --- a/test/extensions/filters/udp/dns_filter/dns_filter_test.cc +++ b/test/extensions/filters/udp/dns_filter/dns_filter_test.cc @@ -1863,12 +1863,11 @@ TEST_F(DnsFilterTest, NotImplementedQueryTest) { EXPECT_EQ(0, config_->stats().downstream_rx_invalid_queries_.value()); } -TEST_F(DnsFilterTest, NoTransactionIdTest) { +TEST_F(DnsFilterTest, ZeroTransactionIdTest) { InSequence s; setup(forward_query_off_config); - // This buffer has an invalid Transaction ID. We should return an error - // to the client + // This buffer has a Transaction ID of zero. This is not an error. constexpr char dns_request[] = { 0x00, 0x00, // Transaction ID 0x01, 0x20, // Flags @@ -1888,8 +1887,8 @@ TEST_F(DnsFilterTest, NoTransactionIdTest) { sendQueryFromClient("10.0.0.1:1000", query); response_ctx_ = ResponseValidator::createResponseContext(udp_response_, counters_); - EXPECT_FALSE(response_ctx_->parse_status_); - EXPECT_EQ(DNS_RESPONSE_CODE_FORMAT_ERROR, response_ctx_->getQueryResponseCode()); + EXPECT_TRUE(response_ctx_->parse_status_); + EXPECT_EQ(DNS_RESPONSE_CODE_NOT_IMPLEMENTED, response_ctx_->getQueryResponseCode()); } TEST_F(DnsFilterTest, InvalidShortBufferTest) { From 1e8f33ccedec303f3948788a32f0ee1b96e86220 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Thu, 11 Jul 2024 09:58:59 +0200 Subject: [PATCH 4/5] docs: add to bug_fixes in changelog Signed-off-by: Mike Beaumont --- changelogs/current.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 92966fdc7a8c..37d52500f9a4 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -150,6 +150,9 @@ minor_behavior_changes: bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* +- area: dns + change: | + The DNS filter no longer returns FORMERR if a message has an ID of 0 - area: admission control change: | Fixed the thread-local controller's average RPS calculation to be calculated over the full From 6469383f8ba99826528279c3612b5d64d6095ddd Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Thu, 11 Jul 2024 16:18:30 +0200 Subject: [PATCH 5/5] docs: add punctuation to changelog Signed-off-by: Mike Beaumont --- changelogs/current.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 37d52500f9a4..4cd8b9bf2632 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -152,7 +152,7 @@ bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* - area: dns change: | - The DNS filter no longer returns FORMERR if a message has an ID of 0 + The DNS filter no longer returns FORMERR if a message has an ID of 0. - area: admission control change: | Fixed the thread-local controller's average RPS calculation to be calculated over the full