Opened 7 years ago

Closed 6 years ago

#2951 closed defect (fixed)

NXDOMAIN case at root server results in SERVFAIL w/ sqlite3 datasrc

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

Description

It can be easily reproduced: load any valid root zone into sqlite3 DB
file using b10-loadzone and serve that zone from b10-auth without
in-memory cache.

Then a query for a non existent domain would result in:

   2013-05-09 17:23:00.110 ERROR [b10-auth.auth/18196] AUTH_PROCESS_FAIL message processing failure: duplicate period in *..

this is because DatabaseClient::Finder::findWildcardMatch() naively
prepends '*.' to create a possible wildcard name:

        const string wildcard("*." + superdomain.toText());

It works for almost all cases but not for the apex of the root zone.
Fix should be easy.

Subtickets

Change History (5)

comment:1 Changed 6 years ago by muks

  • Milestone set to Sprint-20131015
  • Owner set to UnAssigned
  • Status changed from new to reviewing

This is now ready for review.

We create a Name object for * and concatenate() the super-domain to it. The Name construction for * is slightly less optimal, but this is the safest approach. Similar to LabelSequence, it is possible to have a static Name::WILDCARD() function that returns a reference to a global wildcard Name object, but this seems to be the only place in code that constructs the absolute '*.' name.

Proposed ChangeLog entry:

+XYZ.   [bug]           muks
+       b10-auth now correctly processes NXDOMAIN results in the root zone
+       when using a SQLite3 data source.
+       (Trac #2951, git ...)

comment:2 Changed 6 years ago by kean

  • Owner changed from UnAssigned to muks

The config file was written pre #2904, and therefore uses the old port number which has changed. Commit 5bfc72950099a121838afff9fd2bf9e1230e5118 changes that to the new port number.

Running lettuce fails in one of your new tests:

A query for . type SOA should have rcode NOERROR   # features/terrain/querying.py:206
    Traceback (most recent call last):
      File "/usr/lib/python2.7/site-packages/lettuce/core.py", line 143, in __call__
        ret = self.function(self.step, *args, **kw)
      File "/home/kean/isc/bind10/tests/lettuce/features/terrain/querying.py", line 250, in query
        "Expected: " + rcode + ", got " + query_result.rcode
    AssertionError: Expected: NOERROR, got NO_ANSWER

Please see if this fails for you too. This was on FC19.

comment:3 Changed 6 years ago by muks

  • Owner changed from muks to kean

Hi Kean

I made the port number the same as other tests as the branch would not pass lettuce tests without it. This is because the terrain would try to query at the old port number unless this was merged to master.

Running lettuce fails in one of your new tests:

On my machine, the lettuce test passes in the branch that was put to review:

  Scenario: Querying non-existing name in root zone from sqlite3 should work                                              # features/queries.feature:416
    Given I have bind10 running with configuration root.config                                                            # features/terrain/bind10_control.py:107
    And wait for bind10 stderr message BIND10_STARTED_CC                                                                  # features/terrain/steps.py:34
    And wait for bind10 stderr message CMDCTL_STARTED                                                                     # features/terrain/steps.py:34
    And wait for bind10 stderr message AUTH_SERVER_STARTED                                                                # features/terrain/steps.py:34
    bind10 module Auth should be running                                                                                  # features/terrain/bind10_control.py:334
    And bind10 module Stats should be running                                                                             # features/terrain/bind10_control.py:334
    And bind10 module Resolver should not be running                                                                      # features/terrain/bind10_control.py:334
    And bind10 module Xfrout should not be running                                                                        # features/terrain/bind10_control.py:334
    And bind10 module Zonemgr should not be running                                                                       # features/terrain/bind10_control.py:334
    And bind10 module Xfrin should not be running                                                                         # features/terrain/bind10_control.py:334
    And bind10 module StatsHttpd should not be running                                                                    # features/terrain/bind10_control.py:334
    A query for . type SOA should have rcode NOERROR                                                                      # features/terrain/querying.py:206
    A query for nonexistent. type A should have rcode NXDOMAIN                                                            # features/terrain/querying.py:206
    Then wait for bind10 stderr message AUTH_SEND_NORMAL_RESPONSE not AUTH_PROCESS_FAIL                                   # features/terrain/steps.py:34

Did this lettuce failure happen after your modification of the port number in the branch?

Can you attach the lettuce debug output and also the name of the failing test?

From the snippet you have attached, master branch also has such similar errors, but these are on Mac OS for multi-auth feature:
http://git.bind10.isc.org/~tester/builder/BIND10-lettuce/20140122120059-MacOSX10.6-x86_64-GCC/logs/lettuce.out

comment:4 Changed 6 years ago by kean

  • Owner changed from kean to muks

Sorry Mukund you are exactly right, it was my change preparing for a merge to master that broke the test. Your branch in the original form passes all lettuce tests. Do you want me to back out that change and you make it directly on master after doing the merge? I don't think that should be necessary, you can merge the branch as is (with my change in place) so that master will build and test cleanly, but I am happy to revert the change if that is what you want.

comment:5 Changed 6 years ago by muks

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

Merged to master branch in commit 13685cc4580660eaf5b041b683a2d2f31fd24de3:

* 5bfc729 [2951] Change the port number to 56176 after trac2904_v3 merge
* 6271645 [2951] Add lettuce test for non-existing name in root zone from SQLite3
* b0f04be [2951] Fix bad concatenation to construct wildcard name (for root zone)

Resolving as fixed. Thank you for the reviews Kean.

Note: See TracTickets for help on using tickets.