Skip to content

Commit

Permalink
Merge pull request #10842 from omoerbeek/rec-appliedPolicyTrigger-value
Browse files Browse the repository at this point in the history
rec: correct appliedPolicyTrigger value for IP matches
  • Loading branch information
omoerbeek committed Oct 18, 2021
2 parents 0dd3e0b + e4387f4 commit d7104b9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 22 deletions.
33 changes: 17 additions & 16 deletions pdns/filterpo.cc
Expand Up @@ -50,13 +50,21 @@ bool DNSFilterEngine::Zone::findExactQNamePolicy(const DNSName& qname, DNSFilter

bool DNSFilterEngine::Zone::findExactNSPolicy(const DNSName& qname, DNSFilterEngine::Policy& pol) const
{
return findExactNamedPolicy(d_propolName, qname, pol);
if (findExactNamedPolicy(d_propolName, qname, pol)) {
pol.d_trigger = qname;
pol.d_trigger.appendRawLabel(rpzNSDnameName);
return true;
}
return false;
}

bool DNSFilterEngine::Zone::findNSIPPolicy(const ComboAddress& addr, DNSFilterEngine::Policy& pol) const
{
if (const auto fnd = d_propolNSAddr.lookup(addr)) {
pol = fnd->second;
pol.d_trigger = Zone::maskToRPZ(fnd->first);
pol.d_trigger.appendRawLabel(rpzNSIPName);
pol.d_hit = addr.toString();
return true;
}
return false;
Expand All @@ -66,6 +74,9 @@ bool DNSFilterEngine::Zone::findResponsePolicy(const ComboAddress& addr, DNSFilt
{
if (const auto fnd = d_postpolAddr.lookup(addr)) {
pol = fnd->second;
pol.d_trigger = Zone::maskToRPZ(fnd->first);
pol.d_trigger.appendRawLabel(rpzIPName);
pol.d_hit = addr.toString();
return true;
}
return false;
Expand All @@ -75,6 +86,9 @@ bool DNSFilterEngine::Zone::findClientPolicy(const ComboAddress& addr, DNSFilter
{
if (const auto fnd = d_qpolAddr.lookup(addr)) {
pol = fnd->second;
pol.d_trigger = Zone::maskToRPZ(fnd->first);
pol.d_trigger.appendRawLabel(rpzClientIPName);
pol.d_hit = addr.toString();
return true;
}
return false;
Expand Down Expand Up @@ -179,17 +193,13 @@ bool DNSFilterEngine::getProcessingPolicy(const DNSName& qname, const std::unord
}
if (z->findExactNSPolicy(qname, pol)) {
// cerr<<"Had a hit on the nameserver ("<<qname<<") used to process the query"<<endl;
pol.d_trigger = qname;
pol.d_trigger.appendRawLabel(rpzNSDnameName);
pol.d_hit = qname.toStringNoDot();
return true;
}

for (const auto& wc : wcNames) {
if (z->findExactNSPolicy(wc, pol)) {
// cerr<<"Had a hit on the nameserver ("<<qname<<") used to process the query"<<endl;
pol.d_trigger = wc;
pol.d_trigger.appendRawLabel(rpzNSDnameName);
// Hit is not the wildcard passed to findExactQNamePolicy but the actual qname!
pol.d_hit = qname.toStringNoDot();
return true;
}
Expand All @@ -214,10 +224,6 @@ bool DNSFilterEngine::getProcessingPolicy(const ComboAddress& address, const std

if(z->findNSIPPolicy(address, pol)) {
// cerr<<"Had a hit on the nameserver ("<<address.toString()<<") used to process the query"<<endl;
// XXX should use ns RPZ
pol.d_trigger = Zone::maskToRPZ(address);
pol.d_trigger.appendRawLabel(rpzNSIPName);
pol.d_hit = address.toString();
return true;
}
}
Expand Down Expand Up @@ -294,15 +300,13 @@ bool DNSFilterEngine::getQueryPolicy(const DNSName& qname, const std::unordered_

if (z->findExactQNamePolicy(qname, pol)) {
// cerr<<"Had a hit on the name of the query"<<endl;
pol.d_trigger = qname;
pol.d_hit = qname.toStringNoDot();
return true;
}

for (const auto& wc : wcNames) {
if (z->findExactQNamePolicy(wc, pol)) {
// cerr<<"Had a hit on the name of the query"<<endl;
pol.d_trigger = wc;
// Hit is not the wildcard passed to findExactQNamePolicy but the actual qname!
pol.d_hit = qname.toStringNoDot();
return true;
}
Expand Down Expand Up @@ -356,9 +360,6 @@ bool DNSFilterEngine::getPostPolicy(const DNSRecord& record, const std::unordere
}

if (z->findResponsePolicy(ca, pol)) {
pol.d_trigger = Zone::maskToRPZ(ca);
pol.d_trigger.appendRawLabel(rpzIPName);
pol.d_hit = ca.toString();
return true;
}
}
Expand Down
28 changes: 22 additions & 6 deletions pdns/recursordist/test-filterpo_cc.cc
Expand Up @@ -33,19 +33,19 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
const DNSName blockedWildcardName("*.wildcard-blocked.");
const ComboAddress responseIP("192.0.2.254");
BOOST_CHECK_EQUAL(zone->size(), 0U);
zone->addClientTrigger(Netmask(clientIP, 32), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ClientIP));
zone->addClientTrigger(Netmask(clientIP, 31), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ClientIP));
BOOST_CHECK_EQUAL(zone->size(), 1U);
zone->addQNameTrigger(blockedName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::QName));
BOOST_CHECK_EQUAL(zone->size(), 2U);
zone->addQNameTrigger(blockedWildcardName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::QName));
BOOST_CHECK_EQUAL(zone->size(), 3U);
zone->addNSIPTrigger(Netmask(nsIP, 32), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSIP));
zone->addNSIPTrigger(Netmask(nsIP, 31), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSIP));
BOOST_CHECK_EQUAL(zone->size(), 4U);
zone->addNSTrigger(nsName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSDName));
BOOST_CHECK_EQUAL(zone->size(), 5U);
zone->addNSTrigger(nsWildcardName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSDName));
BOOST_CHECK_EQUAL(zone->size(), 6U);
zone->addResponseTrigger(Netmask(responseIP, 32), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ResponseIP));
zone->addResponseTrigger(Netmask(responseIP, 31), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ResponseIP));
BOOST_CHECK_EQUAL(zone->size(), 7U);

size_t zoneIdx = dfe.addZone(zone);
Expand Down Expand Up @@ -81,6 +81,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
const auto matchingPolicy = dfe.getProcessingPolicy(DNSName("sub.sub.wildcard.wolf."), std::unordered_map<std::string, bool>(), DNSFilterEngine::maximumPriority);
BOOST_CHECK(matchingPolicy.d_type == DNSFilterEngine::PolicyType::NSDName);
BOOST_CHECK(matchingPolicy.d_kind == DNSFilterEngine::PolicyKind::Drop);
BOOST_CHECK_EQUAL(matchingPolicy.d_trigger, DNSName("*.wildcard.wolf.rpz-nsdname"));
BOOST_CHECK_EQUAL(matchingPolicy.d_hit, "sub.sub.wildcard.wolf");

/* looking for wildcard.wolf. should not match *.wildcard-blocked. */
const auto notMatchingPolicy = dfe.getProcessingPolicy(DNSName("wildcard.wolf."), std::unordered_map<std::string, bool>(), DNSFilterEngine::maximumPriority);
Expand All @@ -92,6 +94,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
/* except if we look exactly for the wildcard */
BOOST_CHECK(zone->findExactNSPolicy(nsWildcardName, zonePolicy));
BOOST_CHECK(zonePolicy == matchingPolicy);
BOOST_CHECK_EQUAL(zonePolicy.d_trigger, DNSName("*.wildcard.wolf.rpz-nsdname"));
BOOST_CHECK_EQUAL(zonePolicy.d_hit, nsWildcardName.toStringNoDot());
}

{
Expand All @@ -110,6 +114,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
DNSFilterEngine::Policy zonePolicy;
BOOST_CHECK(zone->findNSIPPolicy(nsIP, zonePolicy));
BOOST_CHECK(zonePolicy == matchingPolicy);
BOOST_CHECK_EQUAL(zonePolicy.d_trigger, DNSName("31.0.2.0.192.rpz-nsip"));
BOOST_CHECK_EQUAL(zonePolicy.d_hit, nsIP.toString());
}

{
Expand All @@ -128,6 +134,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
DNSFilterEngine::Policy zonePolicy;
BOOST_CHECK(zone->findExactQNamePolicy(blockedName, zonePolicy));
BOOST_CHECK(zonePolicy == matchingPolicy);
BOOST_CHECK_EQUAL(zonePolicy.d_trigger, blockedName);
BOOST_CHECK_EQUAL(zonePolicy.d_hit, blockedName.toStringNoDot());

/* but a subdomain should not be blocked (not a wildcard, and this is not suffix domain matching */
matchingPolicy = dfe.getQueryPolicy(DNSName("sub") + blockedName, std::unordered_map<std::string, bool>(), DNSFilterEngine::maximumPriority);
Expand All @@ -140,6 +148,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
const auto matchingPolicy = dfe.getQueryPolicy(DNSName("sub.sub.wildcard-blocked."), std::unordered_map<std::string, bool>(), DNSFilterEngine::maximumPriority);
BOOST_CHECK(matchingPolicy.d_type == DNSFilterEngine::PolicyType::QName);
BOOST_CHECK(matchingPolicy.d_kind == DNSFilterEngine::PolicyKind::Drop);
BOOST_CHECK_EQUAL(matchingPolicy.d_trigger, blockedWildcardName);
BOOST_CHECK_EQUAL(matchingPolicy.d_hit, "sub.sub.wildcard-blocked");

/* looking for wildcard-blocked. should not match *.wildcard-blocked. */
const auto notMatchingPolicy = dfe.getQueryPolicy(DNSName("wildcard-blocked."), std::unordered_map<std::string, bool>(), DNSFilterEngine::maximumPriority);
Expand All @@ -151,6 +161,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
/* except if we look exactly for the wildcard */
BOOST_CHECK(zone->findExactQNamePolicy(blockedWildcardName, zonePolicy));
BOOST_CHECK(zonePolicy == matchingPolicy);
BOOST_CHECK_EQUAL(zonePolicy.d_trigger, blockedWildcardName);
BOOST_CHECK_EQUAL(zonePolicy.d_hit, blockedWildcardName.toStringNoDot());
}

{
Expand All @@ -161,6 +173,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
DNSFilterEngine::Policy zonePolicy;
BOOST_CHECK(zone->findClientPolicy(clientIP, zonePolicy));
BOOST_CHECK(zonePolicy == matchingPolicy);
BOOST_CHECK_EQUAL(zonePolicy.d_trigger, DNSName("31.128.2.0.192.rpz-client-ip"));
BOOST_CHECK_EQUAL(zonePolicy.d_hit, clientIP.toString());
}

{
Expand All @@ -183,6 +197,8 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
DNSFilterEngine::Policy zonePolicy;
BOOST_CHECK(zone->findResponsePolicy(responseIP, zonePolicy));
BOOST_CHECK(zonePolicy == matchingPolicy);
BOOST_CHECK_EQUAL(zonePolicy.d_trigger, DNSName("31.254.2.0.192.rpz-ip"));
BOOST_CHECK_EQUAL(zonePolicy.d_hit, responseIP.toString());
}

{
Expand All @@ -197,19 +213,19 @@ BOOST_AUTO_TEST_CASE(test_filter_policies_basic)
}

BOOST_CHECK_EQUAL(zone->size(), 7U);
zone->rmClientTrigger(Netmask(clientIP, 32), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ClientIP));
zone->rmClientTrigger(Netmask(clientIP, 31), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ClientIP));
BOOST_CHECK_EQUAL(zone->size(), 6U);
zone->rmQNameTrigger(blockedName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::QName));
BOOST_CHECK_EQUAL(zone->size(), 5U);
zone->rmQNameTrigger(blockedWildcardName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::QName));
BOOST_CHECK_EQUAL(zone->size(), 4U);
zone->rmNSIPTrigger(Netmask(nsIP, 32), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSIP));
zone->rmNSIPTrigger(Netmask(nsIP, 31), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSIP));
BOOST_CHECK_EQUAL(zone->size(), 3U);
zone->rmNSTrigger(nsName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSDName));
BOOST_CHECK_EQUAL(zone->size(), 2U);
zone->rmNSTrigger(nsWildcardName, DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::NSDName));
BOOST_CHECK_EQUAL(zone->size(), 1U);
zone->rmResponseTrigger(Netmask(responseIP, 32), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ResponseIP));
zone->rmResponseTrigger(Netmask(responseIP, 31), DNSFilterEngine::Policy(DNSFilterEngine::PolicyKind::Drop, DNSFilterEngine::PolicyType::ResponseIP));
BOOST_CHECK_EQUAL(zone->size(), 0U);

/* DNSFilterEngine::clear() calls clear() on all zones, but keeps the zones */
Expand Down
21 changes: 21 additions & 0 deletions regression-tests.recursor-dnssec/test_Protobuf.py
Expand Up @@ -939,13 +939,15 @@ def generateRecursorConfig(cls, confdir):
authzone.write("""$ORIGIN example.
@ 3600 IN SOA {soa}
sub.test 3600 IN A 192.0.2.42
ip 3600 IN A 33.22.11.99
""".format(soa=cls._SOA))

rpzFilePath = os.path.join(confdir, 'zone.rpz')
with open(rpzFilePath, 'w') as rpzZone:
rpzZone.write("""$ORIGIN zone.rpz.
@ 3600 IN SOA {soa}
*.test.example.zone.rpz. 60 IN CNAME rpz-passthru.
24.0.11.22.33.rpz-ip 60 IN A 1.2.3.4
""".format(soa=cls._SOA))

super(ProtobufRPZTest, cls).generateRecursorConfig(confdir)
Expand Down Expand Up @@ -973,6 +975,25 @@ def testA(self):
self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.42')
self.checkNoRemainingMessage()

def testB(self):
name = 'ip.example.'
expected = dns.rrset.from_text(name, 0, dns.rdataclass.IN, 'A', '1.2.3.4')
query = dns.message.make_query(name, 'A', want_dnssec=True)
query.flags |= dns.flags.CD
res = self.sendUDPQuery(query)
self.assertRRsetInAnswer(res, expected)

# check the protobuf messages corresponding to the UDP query and answer
msg = self.getFirstProtobufMessage()
self.checkProtobufQuery(msg, dnsmessage_pb2.PBDNSMessage.UDP, query, dns.rdataclass.IN, dns.rdatatype.A, name)

# then the response
msg = self.getFirstProtobufMessage()
self.checkProtobufResponse(msg, dnsmessage_pb2.PBDNSMessage.UDP, res)
self.checkProtobufPolicy(msg, dnsmessage_pb2.PBDNSMessage.PolicyType.RESPONSEIP, 'zone.rpz.', '24.0.11.22.33.rpz-ip.', '33.22.11.99', dnsmessage_pb2.PBDNSMessage.PolicyKind.Custom)
self.assertEqual(len(msg.response.rrs), 1)
self.checkNoRemainingMessage()

class ProtobufRPZTagsTest(TestRecursorProtobuf):
"""
This test makes sure that we correctly export the RPZ tags in our protobuf messages
Expand Down

0 comments on commit d7104b9

Please sign in to comment.