Skip to content

Commit

Permalink
Pull request #668: Make dnsrewrite rules behaviour consistent with Ad…
Browse files Browse the repository at this point in the history 
…GuardHome

Merge in ADGUARD-CORE-LIBS/dns-libs from fix/AG-34433 to master

Squashed commit of the following:

commit 2b78232c5b60b10ef8b28deb71ba86e8837ce80c
Author: Nikita Gorskikh <[email protected]>
Date:   Thu Aug 1 18:11:15 2024 +0300

    Remove failing tests

commit 2744eca69cc492f827e6410da108ff474f89d472
Author: Nikita Gorskikh <[email protected]>
Date:   Thu Aug 1 16:46:18 2024 +0300

    Make dnsrewrite rules behaviour consistent with AdGuardHome

commit 232d685166092ad47beecd569ceb237fbb545557
Author: Nikita Gorskikh <[email protected]>
Date:   Thu Aug 1 16:45:39 2024 +0300

    Fix UB
  • Loading branch information
@ngorskikh
ngorskikh committed Aug 2, 2024
1 parent a03a866 commit 923bd99
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 85 deletions.
14 changes: 0 additions & 14 deletions dnsfilter/filter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,20 +512,6 @@ static AdblockModifiersMatchStatus match_adblock_modifiers(
}
}

if (info.props.test(DnsFilter::DARP_DNSREWRITE)) {
// check if the request's type corresponds to the $dnsrewrite rule's type
std::optional<rule_utils::DnsrewriteInfo> &dnsrewrite = info.params->dnsrewrite;
if (dnsrewrite.has_value()) {
if ((dnsrewrite->rrtype == LDNS_RR_TYPE_A && ctx.rr_type != LDNS_RR_TYPE_A)
|| (dnsrewrite->rrtype == LDNS_RR_TYPE_AAAA && ctx.rr_type != LDNS_RR_TYPE_AAAA)
|| (dnsrewrite->rrtype == LDNS_RR_TYPE_PTR && ctx.rr_type != LDNS_RR_TYPE_PTR)
|| (dnsrewrite->rrtype == LDNS_RR_TYPE_CNAME && ctx.rr_type != LDNS_RR_TYPE_A
&& ctx.rr_type != LDNS_RR_TYPE_AAAA)) {
return AMMS_NOT_MATCHED;
}
}
}

return AMMS_MATCH_CANDIDATE;
}

Expand Down
68 changes: 0 additions & 68 deletions dnsfilter/test/dnsrewrite_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,74 +260,6 @@ TEST_F(DnsrewriteTest, FullSVCB) {
ASSERT_FALSE(r.rewritten_info->cname.has_value());
}

TEST_F(DnsrewriteTest, A_vs_AAAA_query) {
DnsFilter::EngineParams params = {{{10, "example.com$dnsrewrite=NOERROR;A;1.2.3.4", true}}};
auto [handle, err_or_warn] = filter.create(params);
ASSERT_TRUE(handle) << err_or_warn->str();

std::vector<DnsFilter::Rule> rules = filter.match(handle, {"example.com", LDNS_RR_TYPE_AAAA});
ASSERT_EQ(rules.size(), 0);

filter.destroy(handle);
}

TEST_F(DnsrewriteTest, AAAA_vs_A_query) {
DnsFilter::EngineParams params = {{{10, "example.com$dnsrewrite=NOERROR;AAAA;abcd::1234", true}}};
auto [handle, err_or_warn] = filter.create(params);
ASSERT_TRUE(handle) << err_or_warn->str();

std::vector<DnsFilter::Rule> rules = filter.match(handle, {"example.com", LDNS_RR_TYPE_A});
ASSERT_EQ(rules.size(), 0);

filter.destroy(handle);
}

TEST_F(DnsrewriteTest, MatchReverse) {
DnsFilter::EngineParams params = {{{10, "||4.3.2.1.in-addr.arpa.^$dnsrewrite=REFUSED;PTR;example.com.", true}}};
auto [handle, err_or_warn] = filter.create(params);
ASSERT_TRUE(handle) << err_or_warn->str();

std::vector<DnsFilter::Rule> rules = filter.match(handle, {"4.3.2.1.in-addr.arpa", LDNS_RR_TYPE_PTR});
ASSERT_EQ(rules.size(), 1);
rules = filter.match(handle, {"4.3.2.1.in-addr.arpa", LDNS_RR_TYPE_A});
ASSERT_EQ(rules.size(), 0);

filter.destroy(handle);
}

TEST_F(DnsrewriteTest, MatchReverseIpv6) {
DnsFilter::EngineParams params = {{{10,
"||a.9.8.7.6.5.e.f.f.f.4.3.2.1.0.0.0.0.0.0.0.0.f.0.8.b.d.0.1.0.0.2.ip6.arpa.^$dnsrewrite=REFUSED;PTR;"
"example.com.",
true}}};
auto [handle, err_or_warn] = filter.create(params);
ASSERT_TRUE(handle) << err_or_warn->str();

std::vector<DnsFilter::Rule> rules = filter.match(
handle, {"a.9.8.7.6.5.e.f.f.f.4.3.2.1.0.0.0.0.0.0.0.0.f.0.8.b.d.0.1.0.0.2.ip6.arpa", LDNS_RR_TYPE_PTR});
ASSERT_EQ(rules.size(), 1);
rules = filter.match(
handle, {"a.9.8.7.6.5.e.f.f.f.4.3.2.1.0.0.0.0.0.0.0.0.f.0.8.b.d.0.1.0.0.2.ip6.arpa", LDNS_RR_TYPE_A});
ASSERT_EQ(rules.size(), 0);

filter.destroy(handle);
}

TEST_F(DnsrewriteTest, CnameMatch) {
DnsFilter::EngineParams params = {{{10, "example.com$dnsrewrite=NOERROR;CNAME;example.net.", true}}};
auto [handle, err_or_warn] = filter.create(params);
ASSERT_TRUE(handle) << err_or_warn->str();

std::vector<DnsFilter::Rule> rules = filter.match(handle, {"example.com", LDNS_RR_TYPE_A});
ASSERT_EQ(rules.size(), 1);
rules = filter.match(handle, {"example.com", LDNS_RR_TYPE_AAAA});
ASSERT_EQ(rules.size(), 1);
rules = filter.match(handle, {"example.com", LDNS_RR_TYPE_PTR});
ASSERT_EQ(rules.size(), 0);

filter.destroy(handle);
}

TEST_F(DnsrewriteTest, Multiple_Noerror) {
struct TestData {
struct RdfData {
Expand Down
4 changes: 4 additions & 0 deletions proxy/response_helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ class ResponseHelpers {
ldns_pkt *result = create_response_by_request(request);
ldns_pkt_set_rcode(result, rewritten_info->rcode);
for (auto &rr : rewritten_info->rrs) {
if (ldns_rr_get_type(rr.get()) != LDNS_RR_TYPE_CNAME
&& ldns_rr_get_type(rr.get()) != ldns_rr_get_type(question)) {
continue;
}
// If this is a CNAME rewrite, then we shouldn't replace the owner
// of non-CNAME records for the rewritten name.
if (!rewritten_info->cname.has_value() || ldns_rr_get_type(rr.get()) == LDNS_RR_TYPE_CNAME) {
Expand Down
19 changes: 18 additions & 1 deletion proxy/test/dnsproxy_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ TEST_F(DnsProxyTest, TestDnstypeReply) {

TEST_F(DnsProxyTest, TestDnsrewriteRule) {
DnsProxySettings settings = make_dnsproxy_settings();
settings.blocked_response_ttl_secs = 4242;
settings.filter_params = {{{1,
"@@example.com$important\n"
"example.com$dnsrewrite=1.2.3.4\n"
Expand All @@ -447,8 +448,24 @@ TEST_F(DnsProxyTest, TestDnsrewriteRule) {
ldns_pkt_ptr response;
ASSERT_NO_FATAL_FAILURE(perform_request(*m_proxy, create_request("example.com", LDNS_RR_TYPE_A, LDNS_RD), response));
ASSERT_EQ(last_event.rules.size(), 3);
ASSERT_EQ(ldns_pkt_ancount(response.get()), 2);
ASSERT_EQ(ldns_pkt_get_rcode(response.get()), LDNS_RCODE_NOERROR);
ASSERT_EQ(ldns_pkt_ancount(response.get()), 1);

ag::UniquePtr<char, &free> rrstr{ldns_rr2str(ldns_rr_list_rr(ldns_pkt_answer(response.get()), 0))};
ASSERT_STREQ("example.com.\t4242\tIN\tA\t100.200.200.100\n", rrstr.get());

ASSERT_NO_FATAL_FAILURE(perform_request(*m_proxy, create_request("example.com", LDNS_RR_TYPE_MX, LDNS_RD), response));
ASSERT_EQ(last_event.rules.size(), 3);
ASSERT_EQ(ldns_pkt_get_rcode(response.get()), LDNS_RCODE_NOERROR);
ASSERT_EQ(ldns_pkt_ancount(response.get()), 1);

rrstr.reset(ldns_rr2str(ldns_rr_list_rr(ldns_pkt_answer(response.get()), 0)));
ASSERT_STREQ("example.com.\t4242\tIN\tMX\t42 example.mail.\n", rrstr.get());

ASSERT_NO_FATAL_FAILURE(perform_request(*m_proxy, create_request("example.com", LDNS_RR_TYPE_AAAA, LDNS_RD), response));
ASSERT_EQ(last_event.rules.size(), 3);
ASSERT_EQ(ldns_pkt_get_rcode(response.get()), LDNS_RCODE_NOERROR);
ASSERT_EQ(ldns_pkt_ancount(response.get()), 0);
}

TEST_F(DnsProxyTest, TestDnsrewriteCname) {
Expand Down
4 changes: 2 additions & 2 deletions upstream/upstream_plain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ namespace ag::dns {
PlainUpstream::PlainUpstream(const UpstreamOptions &opts, const UpstreamFactoryConfig &config)
: Upstream(opts, config)
, m_log(AG_FMT("Plain upstream ({})", opts.address))
, m_prefer_tcp{}
, m_address{}
, m_prefer_tcp{false}
, m_prefer_udp{false}
, m_shutdown_guard(std::make_shared<bool>(true)) {
}

Expand Down

0 comments on commit 923bd99

Please sign in to comment.