diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 9ecf0d6e48ce..859372e2dad7 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -8,6 +8,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. removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` 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); 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 d03cc3e177f0..aa698ba161a3 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) { 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);