Opened 9 years ago

Closed 9 years ago

#553 closed enhancement (complete)

canceling wildcard match due to an empty non terminal

Reported by: jinmei Owned by: vorner
Priority: medium Milestone: A-Team-Sprint-20110223
Component: data source Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 5.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

See the analysis ticket (#506) for the big picture.
This is a substask for a corner case of find()ing (and canceling) a wildcard.

This subtask depends on #552 (and #517).

This process is details of step 4 of #551.
It implements the following part of Section 4.3.3 of RFC1034:

   - When the query name or a name between the wildcard domain and
     the query name is know to exist.

Specifically, when example.com has the wildcard name *.example.com and
bar.foo.example.com, this process will reject aaa.foo.example.com
and zzz.foo.example.com to be matched against the wildcard.
The necessary steps are as follows:

  • get the existing previous node of the original query name, and the next node of the previous. For the query name of aaa.foo.example.com, they are *.example.com and bar.foo.example.com, respectively; for the query name of zzz.foo.example.com, they are bar.foo.example.com and something else (or none), respectively.
  • check if any ancestor name of the query name (including the qname itself) up to the node marked as "wild" (in this case example.com) is a super domain of either the previous and next node name. For aaa.foo.example.com, its 1-generation ancestor, foo.example.com is a super domain of the next, "bar.foo.example.com"; for zzz.foo.example.com, its 1-generation ancestor, again foo.example.com, is a super domain of the previous, "bar.foo.example.com". This means the query name is a subdomain of an empty non terminal under the "wild" node, foo.example.com, and should not allow wildcard match.

Subtickets

Change History (14)

comment:1 Changed 9 years ago by jinmei

  • Estimated Difficulty changed from 0.0 to 5.0

comment:2 follow-up: Changed 9 years ago by jinmei

I've realized that this task can probably be done more easily once we
merge #507.

That is, we should be able to reject this case by checking whether the
"last comparison" is "COMMONANCESTOR" (and if so, cancel wildcard
matching). With this approach we don't need #552.

comment:3 Changed 9 years ago by stephen

  • Milestone changed from A-Team-Task-Backlog to A-Team-Sprint-20110223

comment:4 Changed 9 years ago by vorner

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

comment:5 in reply to: ↑ 2 Changed 9 years ago by vorner

Replying to jinmei:

That is, we should be able to reject this case by checking whether the
"last comparison" is "COMMONANCESTOR" (and if so, cancel wildcard
matching). With this approach we don't need #552.

Seems it's not that simple, because if we cancel by COMMONANCESTOR, we cancel all wildcard matches. When we are checking for a.wildcard.example.org, the last comparison we do is with *.wildcard.example.org and that one is COMMONANCESTOR as well. I'll think a while if we can work around this somehow easily and if I come to no conclusion, I'll just pull #552 in and do it the previous way.

comment:6 Changed 9 years ago by vorner

I think it will work, the NodeChain? gives the node it compared with as well and count of the common nodes, so it can be extracted from that. I'll look into it sometime later.

comment:7 Changed 9 years ago by vorner

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

I think it is ready for review. This depends on #551, therefore shouldn't be merged before that one, but review probably can happen in parallel.

I'm not sure this branch should have a changelog, since most of the code in in-memory data source didn't (maybe there should be one „global“ changelog for it), but if it should, I propose to add this changelog entry (together with #550 and #551).

[func]    vorner
The in-memory data source handles wildcards.

comment:8 Changed 9 years ago by jinmei

  • Owner changed from UnAssigned to jinmei

comment:9 Changed 9 years ago by jinmei

It basically looks okay.

I have some minor editorial comments.

  • I've made trivial style fixes and pushed them directly.
  • s/trough/through/?
                         * internal RBTree structure, which leaks out trough this
    
  • should need a space between "expect" and "empty"
    +                        "Wrong test, don't check for wild names if you expect"
    +                        "empty answer";
    
  • maybe "multiple times" is more robust for future extensions than twice?
    +    // Internal part of the cancelWildcard test that is run twice
    
  • s/foo.example.org/foo.wild.example.org/? same for the next two SCOPED_TRACEs.
    +        SCOPED_TRACE("Canceled under foo.example.org");
    
  • s/foo.example.org/foo.wild.example.org/?
    + * *.wild.example.org and bar.foo.example.org, then we know foo.example.org
    
  • s/undef/under/?
    +    // Try putting another one undef foo.wild....
    

As for changelog, I'd propose a higher level entry as I noted in #551. Something like:

  170.? [func]		feng, jerry, jinmei, vorner
	b10-auth, src/lib/datasrc: in memory data source now works as a
	complete data source for authoritative DNS servers and b10-auth
	uses it.  It still misses major features, however, including
	DNSSEC support and zone transfer.

comment:10 Changed 9 years ago by jinmei

  • Owner changed from jinmei to vorner

comment:11 follow-up: Changed 9 years ago by vorner

  • Owner changed from vorner to jinmei

Thanks for them, I corrected the typos. Can I merge it with the corrections?

I agree with the changelog entry. Should we ask the other people mentioned in it, or just put it there?

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

Replying to vorner:

Thanks for them, I corrected the typos. Can I merge it with the corrections?

Yes, go ahead.

I agree with the changelog entry. Should we ask the other people mentioned in it, or just put it there?

No strong opinion, but I'd put it anyway and send email to the team
asking for check (and modifying it if they don't like it).

comment:13 Changed 9 years ago by jinmei

  • Owner changed from jinmei to vorner

comment:14 Changed 9 years ago by vorner

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

OK, merged and closing.

Note: See TracTickets for help on using tickets.