Opened 10 years ago

Closed 8 years ago

#75 closed defect (fixed)

auth server: incorrect handling with wildcard matching

Reported by: jinmei Owned by: shane
Priority: medium Milestone:
Component: b10-auth Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket:
Total Hours: Internal?: no

Description

If we have

*.wild.jinmei.org. 86400 IN CNAME mon.jinmei.org.
mon.jinmei.org. 86400 IN A 149.20.54.162

and ask foo.wild.jinmei.org/A, BIND10 auth server returns NXDOMAIN.

This is incorrect (or at least incompatible with BIND9). BIND9 returns:

foo.wild.jinmei.org. 86400 IN CNAME mon.jinmei.org.
mon.jinmei.org. 86400 IN A 149.20.54.162

Also, if we have

*.wild2.jinmei.org. 86400 IN A 149.20.54.162
(but no AAAA for *.wild2)

and ask foo.wild2.jinmei.org/AAAA, BIND10 auth server returns NXDOMAIN.

This is incorrect (or at least incompatible with BIND9). BIND9 returns a "no ereror + no anser" response.

Subtickets

Change History (25)

comment:1 Changed 10 years ago by each

  • Status changed from new to accepted

Both bugs are addressed in r1317.

Leaving the ticket open as I still have to add a unit test for one of the fixes.

comment:2 Changed 10 years ago by each

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

Unit test added in r1338.

comment:3 follow-up: Changed 10 years ago by jinmei

  • Resolution fixed deleted
  • Status changed from closed to reopened

r1317 is not 100% correct for wildcard + NXRRSET case. If it's a DNSSEC enabled response, it should also return *.wild2.jinmei.org/NSEC,RRSIG. It currently only returns SOA + its RRSIG.

Reopening the ticket.

comment:4 Changed 10 years ago by jinmei

  • Status changed from reopened to assigned

comment:5 in reply to: ↑ 3 ; follow-up: Changed 10 years ago by each

r1317 is not 100% correct for wildcard + NXRRSET case. If it's a DNSSEC enabled response, it should also return *.wild2.jinmei.org/NSEC,RRSIG. It currently only returns SOA + its RRSIG.

Thank you for noticing this bug. I believe it's fixed. I also added unit tests to cover wildcard->CNAME->NXRRSET and wildcard->CNAME->NXDOMAIN, and corrected the existing unit tests to look for the NSEC records.

comment:6 Changed 10 years ago by each

  • Owner changed from each to jinmei

comment:7 in reply to: ↑ 5 ; follow-up: Changed 10 years ago by jinmei

  • Owner changed from jinmei to each

Replying to each:

r1317 is not 100% correct for wildcard + NXRRSET case. If it's a DNSSEC enabled response, it should also return *.wild2.jinmei.org/NSEC,RRSIG. It currently only returns SOA + its RRSIG.

Thank you for noticing this bug. I believe it's fixed. I also added unit tests to cover wildcard->CNAME->NXRRSET and wildcard->CNAME->NXDOMAIN, and corrected the existing unit tests to look for the NSEC records.

Not looking at the code, but this doesn't seem to be fixed. If I ask foo.wild2.jinmei.org/AAAA (qname matches *.wild2.jinmei.org, but it doesn't have AAAA) the response is:

% dig @127.0.0.1 -p 5300 foo.wild2.jinmei.org aaaa +dnssec

;; AUTHORITY SECTION:
jinmei.org. 86400 IN SOA ns.jinmei.org. jinmei.kame.net. 2010030608 7200 3600 2592000 1200
jinmei.org. 86400 IN RRSIG SOA 5 2 86400 [...]

BIND9 returns the following, which is correct:

;; AUTHORITY SECTION:
jinmei.org. 1200 IN SOA ns.jinmei.org. jinmei.kame.net. 2010030608 7200 3600 2592000 1200
jinmei.org. 1200 IN RRSIG SOA 5 2 86400 [...]
*.wild2.jinmei.org. 1200 IN NSEC www.jinmei.org. A RRSIG NSEC
*.wild2.jinmei.org. 1200 IN RRSIG NSEC 5 3 1200 [...]

comment:8 Changed 9 years ago by shane

  • Component changed from Unclassified to b10-auth

comment:9 in reply to: ↑ 7 ; follow-up: Changed 9 years ago by each

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

Replying to jinmei:

Not looking at the code, but this doesn't seem to be fixed. If I ask foo.wild2.jinmei.org/AAAA (qname matches *.wild2.jinmei.org, but it doesn't have AAAA) the response is:

Interestingly, Jinmei seems to have found mistakes in both BIND 9 and BIND 10 here. BIND 9 should have included two NSEC records, not one. BIND 10 should have included an SOA record.

I have a proposed fix in r2280. This also fixes a few unit tests that were written based on my incorrect understanding of the "correct behavior".

Please review.

comment:10 follow-up: Changed 9 years ago by shane

  • Owner changed from UnAssigned to jinmei

Jinmei, can you please look at this fix? It seems small so should be relatively straightforward to review (although of course that is not necessarily true). :)

comment:11 in reply to: ↑ 10 Changed 9 years ago by jinmei

Replying to shane:

Jinmei, can you please look at this fix? It seems small so should be relatively straightforward to review (although of course that is not necessarily true). :)

I took a quick look at it, but it didn't seem to be a 10 minute work. I don't mind reviewing it later, but that will probably be after the next release. If you like please find a different reviewer.

comment:12 Changed 9 years ago by shane

No worries Jinmei. This is a fix, important but not urgent. It can quite easily wait until after this release.

comment:13 in reply to: ↑ 9 Changed 9 years ago by jinmei

  • Owner changed from jinmei to each

Replying to each:

Not looking at the code, but this doesn't seem to be fixed. If I ask foo.wild2.jinmei.org/AAAA (qname matches *.wild2.jinmei.org, but it doesn't have AAAA) the response is:

Interestingly, Jinmei seems to have found mistakes in both BIND 9 and BIND 10 here. BIND 9 should have included two NSEC records, not one. BIND 10 should have included an SOA record.

I have a proposed fix in r2280. This also fixes a few unit tests that were written based on my incorrect understanding of the "correct behavior".

Please review.

Before looking into the code I have a question. Do you mean, by "BIND 9 should have included two NSEC records", we should return two NSECs in the authority section if we have *.wild2.jinmei.org/A (but no AAAA) and we're asked for foo.wild2.jinmei.org/AAAA, and the patch tries to do so?

If so, I don't think it's the correct behavior (and I believe BIND 9 behaves correctly) because this is not an NXDOMAIN case. *.wild2.jinmei.org/NSEC should prove both that there's a matching name (wildcard) and that there's no AAAA of that name. For that matter NSD behaves same as BIND 9 in this case.

Please clarify.

comment:14 Changed 9 years ago by each

I may be confused about which case we're talking about here, but let me go over my understanding of negative wildcard answers, and you tell me where you disagree. (It's very possible I have something wrong.)

We have:

*.wild.example A 192.0.2.2
*.wilder.example CNAME www.example
*.wildest.example CNAME spork.example
www.example A 192.0.2.1

...and spork.example doesn't exist.

Query for: foo.wild.example/A
Returns: foo.wild.example/A

*.wild.example/NSEC

Because: NSEC record proves original qname doesn't exist

Query for: foo.wild.example/AAAA
Returns: NOERROR/NODATA

example/SOA
*.wild.example/NSEC

Because: NSEC record proves *.wild.example has no AAAA

SOA claims authority for negative answer

Query for: foo.wilder.example/A
Returns: foo.wilder.example/CNAME

www.example/A
*.wild.example/NSEC

Because: NSEC record proves foo.wilder.example does not exist

Query for: foo.wilder.example/AAAA
Returns: foo.wilder.example/CNAME

*.wilder.example/NSEC
<covering NSEC for www.example>
example/SOA

Because: First NSEC proves foo.wilder.example doesn't exist

Second NSEC proves www.example has no AAAA
SOA claims authority for negative answer

Query for: foo.wildest.example/A
Returns: foo.wildest.example/CNAME

*.wildest.example/NSEC
<covering NSEC for spork.example>
example/NSEC
example/SOA

Because: First NSEC proves foo.wildest.example doesn't exist

Second NSEC proves spork.example doesn't exist
Third NSEC proves *.example doesn't exist
SOA claims authority for negative answer.

Query for foo.wildest.example/AAAA is the same as foo.wildest.example/A.

My understanding was that your query matched the fourth case, above -- wildcard pointing to CNAME, but no type match. If you only got one NSEC record, then if I'm correct in my understanding of the protocol, then BIND 9 was misbehaving. I'm told by Mark that BIND 9 did have this exact bug at some point (though 9.7 definitely doesn't).

On re-reading, I think your query was really the second case -- wildcard match, type mismatch, no CNAME. In that case there should be a single NSEC, as you noted.

I think with this fix, BIND 10 handles all of the above cases correctly.

comment:15 Changed 9 years ago by each

<covering NSEC for www.example>

Oops correction, I meant www.example/NSEC there. Was thinking of the spork.example. case for a moment.

comment:16 Changed 9 years ago by shane

  • Milestone changed from 05. 3rd Incremental Release: Serious Secondary to 06. 4th Incremental Release

comment:17 Changed 9 years ago by jinmei

  • billable set to 1
  • Internal? unset

This code still looks incorrect but with a different failure mode.

See DataSrcTest?.DISABLED_WildcardAgainstMultiLabel of trunk.

Please also add more checks in WildcardNodata? (check expected records
like in WildcardCname?).

I also suggest creating a branch so that we can easily check the
revised behavior.

comment:18 Changed 9 years ago by shane

  • Milestone changed from 06. 4th Incremental Release to feature backlog item
  • Owner changed from each to shane
  • Status changed from reviewing to accepted

This requires a new fix, so I'm moving this out of reviewing and moving to the backlog.

comment:19 Changed 9 years ago by shane

  • Owner changed from shane to UnAssigned
  • Status changed from accepted to assigned

comment:20 Changed 9 years ago by stephen

  • Milestone feature backlog item deleted

Milestone feature backlog item deleted

comment:21 follow-up: Changed 8 years ago by shane

  • Defect Severity set to N/A
  • Milestone set to New Tasks
  • Owner changed from UnAssigned to jinmei
  • Sub-Project set to DNS

I guess this has been overtaken by events, but I am not sure. Jinmei, can you tell me if this ticket needs to remain open?

comment:22 Changed 8 years ago by shane

  • Milestone New Tasks deleted

comment:23 in reply to: ↑ 21 Changed 8 years ago by jinmei

Replying to shane:

I guess this has been overtaken by events, but I am not sure. Jinmei, can you tell me if this ticket needs to remain open?

I believe that this problem will be a non issue once we complete the
datasrc refactoring. Assuming it will surely happen maybe we can
simply close this old one in order to keep the ticket system more
maintainable.

(The same argument will apply to many of bug tickets specific to the
current sqlite3-based data source implementation).

comment:24 Changed 8 years ago by jinmei

  • Owner changed from jinmei to shane

comment:25 Changed 8 years ago by shane

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

Okay, I'm resolving this, with the expectation that our refactored code will fix it.

Note: See TracTickets for help on using tickets.