Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve ALIAS handling #3733

Merged
merged 15 commits into from May 10, 2016
Merged

improve ALIAS handling #3733

merged 15 commits into from May 10, 2016

Conversation

@Habbie
Copy link
Member

Habbie commented Apr 18, 2016

This branch expands ALIAS to the backend A/AAAA on AXFR. It also adds some testing (but the DNSSEC tests are broken because of it).

  • make ALIAS expansion in AXFR optional (yes/no/reject, default to reject I think) (implemented without reject)
  • make AXFR abort when the lookups yield anything other than a -very- clear result (either 'data' or 'very clear NXDOMAIN/NODATA')
  • investigate report of wildcard ALIAS leading to three identical A records in expansion
  • finish #3808
  • #3831 was caused by us punting a query to nproxy even if the client was TCP. Need to fix this.
  • move ALIAS testing records from example.com to a smaller zone that we -do- check in verify-dnssec-zone
@nickmarden

This comment has been minimized.

Copy link

nickmarden commented Apr 18, 2016

Thanks @Habbie! This is going to be very useful for a lot of people :-)

Can you explain the semantics of make ALIAS expansion in AXFR optional? I think you mean:

  • yes: Expand ALIAS records into the underlying A/AAAA values, failing the AXFR whenever the underlying A/AAAA records cannot be resolved
  • no: Send ALIAS records as ALIAS records
  • reject: Refuse to AXFR zones that contain ALIAS records

Is that correct?

@Habbie

This comment has been minimized.

Copy link
Member Author

Habbie commented Apr 18, 2016

Hi Nick, yes, that is my thinking for the three options! Note that for 'reject' the refusal may come halfway and thus in the behavioural sense that will be like 'failing the AXFR' on the yes side of things. Client are expected to deal with this by not replacing their local copy of the zone at all.

@Habbie Habbie force-pushed the Habbie:alias-testing branch from c460590 to 72e0512 May 2, 2016
@Habbie

This comment has been minimized.

Copy link
Member Author

Habbie commented May 3, 2016

@nickmarden I have added the flag but it's a bool for now, missing the refuse option. Does that seem fine to you?

@nickmarden

This comment has been minimized.

Copy link

nickmarden commented May 3, 2016

Sounds great to me. I'd definitely be using the yes value, not the refuse value :-)

@nickmarden

This comment has been minimized.

Copy link

nickmarden commented May 5, 2016

@Habbie, I see that a related issue is tagged auth-4.0.0, but I was wondering if there is an ETA for this PR to be on PowerDNS master?

@Habbie

This comment has been minimized.

Copy link
Member Author

Habbie commented May 5, 2016

@nickmarden as I may have mentioned to you (or not), the getaddrinfo-based infrastructure in this PR is unreliable, it does not tell me the difference between 'everything is terrible' and 'name does not exist'. To fix this, I have done some refactoring in #3802 so the ALIAS code can also use a reliably resolver. This was merged a few days ago, and I need to update this PR to use that code. After that (should be sometime next week) I feel we can merge this as it will make everything just a lot better. There are some nits on #3808, and for ALIAS purposes the 'this seems optimistic' bullet (currently the 4th one) in #3808 might be relevant, but that's incremental work.

TLDR I hope to finish and merge this PR sometime next week :)

@nickmarden

This comment has been minimized.

Copy link

nickmarden commented May 5, 2016

OK thanks @Habbie - I now understand the dependencies and timeline better. Godspeed!

@pieterlexis pieterlexis added this to the auth-4-alpha3 milestone May 9, 2016
@Habbie Habbie force-pushed the Habbie:alias-testing branch 4 times, most recently from c5bf091 to fe550c4 May 9, 2016
@Habbie Habbie force-pushed the Habbie:alias-testing branch from 6d10e8b to f9925ad May 9, 2016
@Habbie

This comment has been minimized.

Copy link
Member Author

Habbie commented May 9, 2016

Expecting travis to go green any second. Reviews welcome. Note that not all bullets at the top have been fixed - this is fine, they will go into a new ticket. As far as I can tell this PR is a strong improvement over the existing situation and more polish will be applied after user testing.

@nickmarden

This comment has been minimized.

Copy link

nickmarden commented May 9, 2016

@Habbie I'm not sufficiently familiar with the PowerDNS internals to offer useful commentary on most of the code changes, but the new option outgoing-axfr-expand-alias is exactly as we've discussed and so I give that a hearty 👍

@Habbie Habbie changed the title [WIP] improve ALIAS handling improve ALIAS handling May 10, 2016
@Habbie Habbie merged commit 4aa7ed8 into PowerDNS:master May 10, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Habbie Habbie deleted the Habbie:alias-testing branch May 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.