Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#2504 closed defect (fixed)

Problem in inmem NSEC denial of existence handling

Reported by: jelte Owned by: muks
Priority: medium Milestone: Sprint-20121218
Component: data source Version:
Keywords: Cc:
CVSS Scoring: 4.3 Parent Tickets:
Sensitive: no Defect Severity: High
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no


Similar to the problem I just reported in #2503, the NSEC case is even worse, instead of returning SERFVAIL, it dies on an assert:

lt-b10-auth: isc::datasrc::memory::{anonymous}::ConstNodeRRset isc::datasrc::memory::{anonymous}::getClosestNSEC(const isc::datasrc::memory::ZoneData&, isc::datasrc::memory::ZoneChain&, isc::datasrc::ZoneFinder::FindOptions): Assertion `false' failed.

in this case the zone I used was:

}}}	600	IN	SOA 2005080901 28800 7200 604800 18000	600	IN	A	3600	IN	DNSKEY	256 3 5 AwEAAag7J/qxkLI1keftNSKe0fqa2mU1GaeZGQoOSgVZvJAMh6LK9F1NMmWP2MeaeJvfTkmZs2UggGnFjkb7QQygrZnfxxCOVu5fG8x3Na5Z7jAdMgVfuJ1FTmsi2Bj4W/+b3fuvv/eVGqlEsFdth+sGA+BKH9mdBwmt+aXJf9gohXpx ;{id = 34791 (zsk), size = 1024b}	600	IN	NS	18000	IN	NSEC A NS SOA RRSIG NSEC DNSKEY	600	IN	RRSIG	SOA 5 7 600 20200101000000 20120627091929 34791 Z+LjS0NtrUkOje1PZht8a3TylxEKabDltJSye3NO/n6tE9268S5EqQ6r0nZ0hO78c9r4lYxMlvnfyTAj/5rR/zdApCS0EnpQenVyvwagIAVdIcNqpZxr7i/tFmOvxyQK3NRhWqH83WhNwqRE1E4thUfNovtyscSLR+TOhuEBKlg=	600	IN	RRSIG	A 5 7 600 20200101000000 20120627091929 34791 oYbwX46pDyaTNIeZByyLnUVc4RNgFmGeUuJ4sn0aIqrX6UqfNJwJpZiCspcyBRBNIZye6ypVNFD9RUHesrELHjFOAustg86GFws1jeEoDbi4W6wmeNQqiUfnyaqME3rHD9WE7I99Nqiyp8ZEBseem/SIOCsEXxPLrhoAL+Fa1+s=	600	IN	RRSIG	NS 5 7 600 20200101000000 20120627091929 34791 KBRov1y79yN75feRo2Bv5B+UpmDAyEr/kTgWgL9dYtkuQXvT7Y1Lg5jhcFmyrAkpYud70i5Un2DEUtmnI2oUR7XUh1RDnMQZRgKkaDXNXj5D379hLxpD2jtN+A311ShReaM4laj44UiDzu0pjGtrDT7BY1c4Qb5XI0RLyI0VKSs=	3600	IN	RRSIG	DNSKEY 5 7 3600 20200101000000 20120627091929 34791 NriwgSl8QskWiQMlIPzn3WvWQwfQDy8pnJ4U2qlOjWFeJ4wH7DS/P5BDh9+JXjKamh5Bdy2umSf1uY1BeodSP+Ga4yKxNb1sigGbToYGDsbLP1NfPPq6uijl5nRhqBQAAzjCmyU9sm6GUr/CyOpSTTxtFjosXZQ1t1GFya6Tgho=	18000	IN	RRSIG	NSEC 5 7 18000 20200101000000 20120627091929 34791 ijT2etFxkmNLMdbugWsIrnPuJqxdPPO/ilWUb0ZapcuwtqV4mTZCalUlb3KSpcczHPfyClF7WvMEKdluLrX55TPKEBxx1AIdOduhwSTxCxgZpOp4LFIR3rg5hTH9bupEUvbFPWU8RX5oX0GLE0V/EpfNYpHOtvvG3rnegwynML4=


Change History (13)

comment:1 Changed 7 years ago by jinmei

Which query caused the assertion failure?

comment:2 Changed 7 years ago by jwright

  • Defect Severity changed from N/A to High
  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:3 Changed 7 years ago by jelte

I believe I simply queried for any nonexistent name

comment:4 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20121218

comment:5 Changed 7 years ago by muks

  • Owner set to muks
  • Status changed from new to assigned


comment:6 Changed 7 years ago by muks

  • Owner changed from muks to UnAssigned
  • Status changed from assigned to reviewing

Up for review.

comment:7 Changed 7 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:8 Changed 7 years ago by jelte

  • Owner changed from jelte to muks

The change looks fine, and even though you included my zone in the test, I have confirmed it works :)

However, I don't think we should use my personal zone data in the test (I have no problem with it, but we should stick to ''-like data as much as possible.

I've recreated a similar zone '', correctly signed, and updated the test to query a nonexistent name in that.

If that looks ok, it can be merged. We do need a changelog for this though :)

comment:9 Changed 7 years ago by muks

ChangeLog entry for this bug:

+519.   [bug]           muks
+       Fixed a problem in inmem NSEC lookup which caused assert failures
+       in some cases.
+       (Trac #2504, git 835553eb309d100b062051f7ef18422d2e8e3ae4)

comment:10 Changed 7 years ago by muks

I also made another minor update that checks the returned value of find():

@@ -1446,6 +1446,9 @@ TEST_F(InMemoryZoneFinderTest, NSECNonExistentTest) {
     ZoneFinderContextPtr find_result(
                                  RRType::A(), ZoneFinder::FIND_DNSSEC));
+    // We don't find the domain, but find() must complete (not throw or
+    // assert).
+    EXPECT_EQ(ZoneFinder::NXDOMAIN, find_result->code);

I pushed it directly, as I don't think another review is necessary for this check.

comment:11 Changed 7 years ago by muks

  • Resolution set to fixed
  • Status changed from reviewing to closed

Pushed to master branch in commit 835553eb309d100b062051f7ef18422d2e8e3ae4:

* a6b558c [2504] Check result of find()
* 06eeaf0 [2504] use zone for test
* 4a9ea53 [2504] Fix getClosestNSEC() when tree.find() results in SUBDOMAIN
* cbf6d38 [2504] Add testcase that reproduces the problem

Resolving as fixed. Thank you for the review and the zone patch Jelte.

comment:12 Changed 7 years ago by shane

  • CVSS Scoring set to 5


comment:13 Changed 7 years ago by shane

  • CVSS Scoring changed from 5 to 4.3

Only applies to certain zones


Note: See TracTickets for help on using tickets.