Opened 8 years ago

Closed 8 years ago

#1424 closed defect (fixed)

query_acl rejecting even though matches

Reported by: jreed Owned by: jinmei
Priority: high Milestone: Sprint-20120110
Component: resolver Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket: none
Estimated Difficulty: 4 Add Hours to Ticket: 0
Total Hours: 2.72 Internal?: no

Description

 2011-11-23 14:38:47.139 INFO  [b10-resolver.resolver] RESOLVER_QUERY_REJECTED query rejected: 'apple.com./A/IN' from 127.0.0.1#63161

But I have default query_acl

Workaround with:

> config set Resolver/query_acl[0]/from "any4"
> config commit

I have a custom listen_on port; I didn't test yet if that trigged the problem. I also didn't test if the acl issue is with other components.

Subtickets

Change History (12)

comment:1 Changed 8 years ago by jinmei

This seems to be a duplicate of #1182 (I'll close the old one).

I could reproduced it by:

  • start BIND 10 without resolver
  • start resolver via bindctl
  • change the listen port from 53 to 5300 then commit
    > config set Resolver/listen_on[0]/port 5300
    > config set Resolver/listen_on[1]/port 5300
    > config commit
    

Then send query. It was rejected due to ACL. When I shutdown BIND 10
and restarted everything it worked.

comment:2 Changed 8 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:3 Changed 8 years ago by shane

  • Feature Depending on Ticket set to none

comment:4 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0 to 4

comment:5 Changed 8 years ago by jelte

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

comment:6 Changed 8 years ago by jelte

  • Priority changed from major to critical

comment:7 Changed 8 years ago by jinmei

trac1424 is ready for review.

The problem is (in my understanding) that when resolver first starts
it tries to listen on a privileged port (53). But if the run time
user doesn't have the privilege it will fail, and the configuration
handler will simply skip installing all other default config
parameters (so the problem shouldn't be specific to the ACL).

The fix in this branch is to change the behavior so that on startup
other parameters will be installed even if listen_on fails.

I actually don't like this hack, and since I expect the introduction
of the socket creator will change the scenario a bit and the
configuration handler will need to be refactored to a cleaner
design/implementation anyway (the current monolithic implementation
won't scale and we can easily face a similar situation as we add more
config parameters), so one option is to defer the fix until that
point.

I really thought about that option, but the symptom would be quite
confusing and (although ugly) the fix itself is not a big change, I
decided to provide a patch anyway. I'm okay with either approach:
applying the fix for now or completely defer it.

If we fix it now, this is the proposed changelog entry:

361.?	[bug]		jinmei
	b10-resolver ignored default configuration parameters if listen_on
	failed (this can easily happen especially for a test environment
	where the run time user doesn't have root privilege), and even if
	listen_on was updated later the resolver wouldn't work correctly
	unless it's fully restarted (for example, all queries would be
	rejected due to an empty ACL).
	(Trac #1424, git TBD)

comment:8 Changed 8 years ago by jinmei

  • Owner set to UnAssigned
  • Status changed from new to reviewing

comment:9 Changed 8 years ago by jelte

  • Owner changed from UnAssigned to jinmei

I have no strong opinion on whether this should be merged; I went through the code, and the changes are ok, and I have no objections to merging this. I also think that the socketcreator should at least solve this specific problem, so I also have no problems deferring it :) (though even with the socketcreator one could still come up with a scenario where this happens if we handle it the exact same way, but it might be a bit artificial).

For a more fundamental fix, yes, the config handler should be refactored (and/or the 'protocol' changed to better handle initial configs and defaults), but we may disagree on the way it should (for instance, IMHO the way auth handles it is overly complicated).

comment:10 Changed 8 years ago by jelte

  • Total Hours changed from 0 to 0.5

comment:11 Changed 8 years ago by jinmei

After working on #1522 and looking at #805, I'm now confident that
merging these won't solve this problem. So I've merged this hack,
and will close this ticket.

comment:12 Changed 8 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Total Hours changed from 0.5 to 2.72
Note: See TracTickets for help on using tickets.