Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

recursor: DNSSEC related query flag processing #3752

Merged
merged 5 commits into from Apr 29, 2016
Merged

recursor: DNSSEC related query flag processing #3752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7e6ad9d
correctly test for cd absense with dnssec=off
pieterlexis Apr 22, 2016
35b5fd1
Stop skipping tests
pieterlexis Apr 21, 2016
f3fe4ae
Treat AD bit in query header correctly
pieterlexis Apr 21, 2016
9eec8c9
Only process CD flags in a dnssec mode
pieterlexis Apr 22, 2016
407404c
Add AD flag to queries in the simple tests
pieterlexis Apr 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
85 changes: 45 additions & 40 deletions pdns/pdns_recursor.cc
View file
Open in desktop
Expand Up @@ -736,12 +736,17 @@ void startDoResolve(void *p)
sr.d_requestor=dc->d_remote;
}

if(g_dnssecmode != DNSSECMode::Off)
if(g_dnssecmode != DNSSECMode::Off) {
sr.d_doDNSSEC=true;

if(pw.getHeader()->cd || (edo.d_Z & EDNSOpts::DNSSECOK)) {
DNSSECOK=true;
g_stats.dnssecQueries++;

// Does the requestor want DNSSEC records?
if(edo.d_Z & EDNSOpts::DNSSECOK) {
DNSSECOK=true;
g_stats.dnssecQueries++;
}
} else {
// Ignore the client-set CD flag
pw.getHeader()->cd=0;
}

bool tracedQuery=false; // we could consider letting Lua know about this too
Expand Down Expand Up @@ -925,48 +930,48 @@ void startDoResolve(void *p)
else {
pw.getHeader()->rcode=res;

// FIXME: haveEDNS is not the way to handle initiation of validation, we
// should look for the AD bit in the header, see #3682
if(haveEDNS || g_dnssecmode == DNSSECMode::ValidateAll || g_dnssecmode==DNSSECMode::ValidateForLog) {
if(g_dnssecmode != DNSSECMode::Off && ((edo.d_Z & EDNSOpts::DNSSECOK) || g_dnssecmode == DNSSECMode::ValidateAll || g_dnssecmode==DNSSECMode::ValidateForLog)) {
// Does the validation mode or query demand validation?
if(g_dnssecmode == DNSSECMode::ValidateAll || g_dnssecmode==DNSSECMode::ValidateForLog || (dc->d_mdp.d_header.ad && g_dnssecmode==DNSSECMode::Process)) {
if(sr.doLog()) {
L<<Logger::Warning<<"Starting validation of answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<endl;
}

auto state=validateRecords(ret);
if(state == Secure) {
if(sr.doLog()) {
L<<Logger::Warning<<"Starting validation of answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<endl;
L<<Logger::Warning<<"Answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<" validates correctly"<<endl;
}
auto state=validateRecords(ret);
if(state == Secure) {
if(sr.doLog()) {
L<<Logger::Warning<<"Answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<" validates correctly"<<endl;
}

pw.getHeader()->ad=1;
}
else if(state == Insecure) {
if(sr.doLog()) {
L<<Logger::Warning<<"Answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<" validates as Insecure"<<endl;
}

pw.getHeader()->ad=0;
}
else if(state == Bogus ) {
// Is the query source interested in the value of the ad-bit?
if (dc->d_mdp.d_header.ad)
pw.getHeader()->ad=1;
}
else if(state == Insecure) {
if(sr.doLog()) {
L<<Logger::Warning<<"Answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<" validates as Insecure"<<endl;
}

pw.getHeader()->ad=0;
}
else if(state == Bogus) {
if(sr.doLog()) {
L<<Logger::Warning<<"Answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<" validates as Bogus"<<endl;
}

// Does the query or validation mode sending out a SERVFAIL on validation errors?
if(!pw.getHeader()->cd && (g_dnssecmode == DNSSECMode::ValidateAll || (dc->d_mdp.d_header.ad && g_dnssecmode != DNSSECMode::Off))) {
if(sr.doLog()) {
L<<Logger::Warning<<"Answer to "<<dc->d_mdp.d_qname<<" for "<<dc->d_remote.toStringWithPort()<<" validates as Bogus"<<endl;
L<<Logger::Warning<<"Sending out SERVFAIL for "<<dc->d_mdp.d_qname<<" because recursor or query demands it for Bogus results"<<endl;
}

if(!pw.getHeader()->cd && (g_dnssecmode == DNSSECMode::ValidateAll || (edo.d_Z & EDNSOpts::DNSSECOK))) {
if(sr.doLog()) {
L<<Logger::Warning<<"Sending out SERVFAIL for "<<dc->d_mdp.d_qname<<" because recursor or query demands it for Bogus results"<<endl;
}

pw.getHeader()->rcode=RCode::ServFail;
goto sendit;
} else {
if(sr.doLog()) {
L<<Logger::Warning<<"Not sending out SERVFAIL for "<<dc->d_mdp.d_qname<<" Bogus validation since neither config nor query demands this"<<endl;
}

pw.getHeader()->rcode=RCode::ServFail;
goto sendit;
} else {
if(sr.doLog()) {
L<<Logger::Warning<<"Not sending out SERVFAIL for "<<dc->d_mdp.d_qname<<" Bogus validation since neither config nor query demands this"<<endl;
}
}
}
}
}
}


Expand Down
31 changes: 3 additions & 28 deletions regression-tests.recursor-dnssec/test_Flags.py
View file
Open in desktop
Expand Up @@ -99,7 +99,6 @@ def testProcess_Secure_None(self):
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testValidate_Secure_None(self):
msg = self.getQueryForSecure()
res = self.sendUDPQuery(msg, 'validate')
Expand All @@ -109,31 +108,26 @@ def testValidate_Secure_None(self):
##
# +AD -CD -DO
##
@unittest.skip("See #3682")
def testOff_Secure_AD(self):
msg = self.getQueryForSecure('AD')
res = self.sendUDPQuery(msg, 'off')
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])

# Raises because #3682
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testProcess_Secure_AD(self):
msg = self.getQueryForSecure('AD')
res = self.sendUDPQuery(msg, 'process')
self.assertMessageIsAuthenticated(res)
self.assertMessageHasFlags(res, ['AD', 'QR', 'RA', 'RD'])
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testValidate_Secure_AD(self):
msg = self.getQueryForSecure('AD')
res = self.sendUDPQuery(msg, 'validate')

self.assertMessageIsAuthenticated(res)
self.assertMessageHasFlags(res, ['AD', 'RD', 'RA', 'QR'])
# Raises because #3682
self.assertNoRRSIGsInAnswer(res)

##
Expand All @@ -146,7 +140,6 @@ def testOff_Secure_ADDO(self):
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testProcess_Secure_ADDO(self):
msg = self.getQueryForSecure('AD', 'DO')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand All @@ -172,7 +165,7 @@ def testOff_Secure_ADDOCD(self):
msg = self.getQueryForSecure('AD CD', 'DO')
res = self.sendUDPQuery(msg, 'off')

self.assertMessageHasFlags(res, ['QR', 'RA', 'RD', 'CD'])
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])

def testProcess_Secure_ADDOCD(self):
msg = self.getQueryForSecure('AD CD', 'DO')
Expand Down Expand Up @@ -202,7 +195,6 @@ def testOff_Secure_DO(self):
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testProcess_Secure_DO(self):
msg = self.getQueryForSecure('', 'DO')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand All @@ -211,7 +203,6 @@ def testProcess_Secure_DO(self):
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'], ['DO'])
self.assertMatchingRRSIGInAnswer(res, expected)

@unittest.skip("See #3682")
def testValidate_Secure_DO(self):
msg = self.getQueryForSecure('', 'DO')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand All @@ -223,15 +214,13 @@ def testValidate_Secure_DO(self):
##
# -AD +CD +DO
##
@unittest.skip("See #3682")
def testOff_Secure_DOCD(self):
msg = self.getQueryForSecure('CD', 'DO')
res = self.sendUDPQuery(msg, 'off')

self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testProcess_Secure_DOCD(self):
msg = self.getQueryForSecure('CD', 'DO')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand All @@ -240,7 +229,6 @@ def testProcess_Secure_DOCD(self):
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD', 'CD'], ['DO'])
self.assertMatchingRRSIGInAnswer(res, expected)

@unittest.skip("See #3682")
def testValidate_Secure_DOCD(self):
msg = self.getQueryForSecure('CD', 'DO')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand All @@ -252,7 +240,6 @@ def testValidate_Secure_DOCD(self):
##
# -AD +CD -DO
##
@unittest.skip("See #3682")
def testOff_Secure_CD(self):
msg = self.getQueryForSecure('CD')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand All @@ -262,7 +249,6 @@ def testOff_Secure_CD(self):
self.assertRRsetInAnswer(res, expected)
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testProcess_Secure_CD(self):
msg = self.getQueryForSecure('CD')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand All @@ -272,7 +258,6 @@ def testProcess_Secure_CD(self):
self.assertRRsetInAnswer(res, expected)
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testValidate_Secure_CD(self):
msg = self.getQueryForSecure('CD')
expected = dns.rrset.from_text('ns1.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.10'.format(prefix=self._PREFIX))
Expand Down Expand Up @@ -318,12 +303,10 @@ def testOff_Bogus_AD(self):
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertRcodeEqual(res, dns.rcode.NOERROR)

@unittest.skip("See #3682")
def testProcess_Bogus_AD(self):
msg = self.getQueryForBogus('AD')
res = self.sendUDPQuery(msg, 'process')
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
# These asserts trigger because of #3682
self.assertRcodeEqual(res, dns.rcode.SERVFAIL)
self.assertAnswerEmpty(res)

Expand All @@ -350,7 +333,6 @@ def testProcess_Bogus_ADDO(self):
res = self.sendUDPQuery(msg, 'process')

self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'], ['DO'])
# This assert triggers because of #3682
self.assertRcodeEqual(res, dns.rcode.SERVFAIL)
self.assertAnswerEmpty(res)

Expand All @@ -368,7 +350,7 @@ def testOff_Bogus_ADDOCD(self):
msg = self.getQueryForBogus('AD CD', 'DO')
res = self.sendUDPQuery(msg, 'off')

self.assertMessageHasFlags(res, ['QR', 'RA', 'RD', 'CD'])
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertRcodeEqual(res, dns.rcode.NOERROR)

def testProcess_Bogus_ADDOCD(self):
Expand Down Expand Up @@ -400,7 +382,6 @@ def testOff_Bogus_DO(self):
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testProcess_Bogus_DO(self):
msg = self.getQueryForBogus('', 'DO')
expected = dns.rrset.from_text('ted.bogus.example.', 0, dns.rdataclass.IN, 'A', '192.0.2.1')
Expand All @@ -421,7 +402,6 @@ def testValidate_Bogus_DO(self):
##
# -AD +CD +DO
##
@unittest.skip("See #3682")
def testOff_Bogus_DOCD(self):
msg = self.getQueryForBogus('CD', 'DO')
res = self.sendUDPQuery(msg, 'off')
Expand Down Expand Up @@ -451,7 +431,6 @@ def testValidate_Bogus_DOCD(self):
##
# -AD +CD -DO
##
@unittest.skip("See #3682")
def testOff_Bogus_CD(self):
msg = self.getQueryForBogus('CD')
expected = dns.rrset.from_text('ted.bogus.example.', 0, dns.rdataclass.IN, 'A', '192.0.2.1')
Expand All @@ -462,7 +441,6 @@ def testOff_Bogus_CD(self):
self.assertRRsetInAnswer(res, expected)
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testProcess_Bogus_CD(self):
msg = self.getQueryForBogus('CD')
expected = dns.rrset.from_text('ted.bogus.example.', 0, dns.rdataclass.IN, 'A', '192.0.2.1')
Expand All @@ -473,7 +451,6 @@ def testProcess_Bogus_CD(self):
self.assertRRsetInAnswer(res, expected)
self.assertNoRRSIGsInAnswer(res)

@unittest.skip("See #3682")
def testValidate_Bogus_CD(self):
msg = self.getQueryForBogus('CD')
expected = dns.rrset.from_text('ted.bogus.example.', 0, dns.rdataclass.IN, 'A', '192.0.2.1')
Expand Down Expand Up @@ -574,7 +551,7 @@ def testOff_Insecure_ADDOCD(self):
msg = self.getQueryForInsecure('AD CD', 'DO')
res = self.sendUDPQuery(msg, 'off')

self.assertMessageHasFlags(res, ['QR', 'RA', 'RD', 'CD'])
self.assertMessageHasFlags(res, ['QR', 'RA', 'RD'])
self.assertNoRRSIGsInAnswer(res)
self.assertRcodeEqual(res, dns.rcode.NOERROR)

Expand Down Expand Up @@ -625,7 +602,6 @@ def testValidate_Insecure_DO(self):
##
# -AD +CD +DO
##
@unittest.skip("See #3682")
def testOff_Insecure_DOCD(self):
msg = self.getQueryForInsecure('CD', 'DO')
res = self.sendUDPQuery(msg, 'off')
Expand Down Expand Up @@ -653,7 +629,6 @@ def testValidate_Insecure_DOCD(self):
##
# -AD +CD -DO
##
@unittest.skip("See #3682")
def testOff_Insecure_CD(self):
msg = self.getQueryForInsecure('CD')
res = self.sendUDPQuery(msg, 'off')
Expand Down
3 changes: 3 additions & 0 deletions regression-tests.recursor-dnssec/test_Simple.py
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ def testSOAs(self):
for zone in ['.', 'example.', 'secure.example.']:
expected = dns.rrset.from_text(zone, 0, dns.rdataclass.IN, 'SOA', self._SOA)
query = dns.message.make_query(zone, 'SOA', want_dnssec=True)
query.flags |= dns.flags.AD

res = self.sendUDPQuery(query)

Expand All @@ -20,6 +21,7 @@ def testSOAs(self):
def testA(self):
expected = dns.rrset.from_text('ns.secure.example.', 0, dns.rdataclass.IN, 'A', '{prefix}.9'.format(prefix=self._PREFIX))
query = dns.message.make_query('ns.secure.example', 'A', want_dnssec=True)
query.flags |= dns.flags.AD

res = self.sendUDPQuery(query)

Expand All @@ -29,6 +31,7 @@ def testA(self):

def testDelegation(self):
query = dns.message.make_query('example', 'NS', want_dnssec=True)
query.flags |= dns.flags.AD

expectedNS = dns.rrset.from_text('example.', 0, 'IN', 'NS', 'ns1.example.', 'ns2.example.')

Expand Down