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

auth-api: restrict creation of OPT and TSIG rrsets #6614

Merged
merged 2 commits into from May 17, 2018

Conversation

Projects
None yet
4 participants
@chbruyand
Member

chbruyand commented May 16, 2018

Short description

OPT and TSIG rrsets are already ignored in incoming AXFR. This restricts creation of such records through the authoritative API and fix #6441

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@chbruyand chbruyand added this to the auth-4.1.x milestone May 16, 2018

@rgacogne rgacogne requested a review from zeha May 16, 2018

@zeha

This misses adding records.
If you move the check to gatherRecords(...), it should catch that case too.

Also what's wrong with rr.qtype.getName here?

@zeha

This comment has been minimized.

Collaborator

zeha commented May 17, 2018

gatherRecordsFromZone would need the same check though, except if ZoneParserTNG already does it. So maybe moving this check into a new func that is called from gatherRecords and gatherRecordsFromZone would make more sense.

@chbruyand

This comment has been minimized.

Member

chbruyand commented May 17, 2018

Also what's wrong with rr.qtype.getName here?

Sorry, but I don't get your point. Could please you rephrase ?

@zeha

This comment has been minimized.

Collaborator

zeha commented May 17, 2018

My suggestion would be to use qtype.getName() instead of stringFromJson(rrset, "type") in the error message.

@chbruyand

This comment has been minimized.

Member

chbruyand commented May 17, 2018

Oh yeah, thanks! did change that while refactoring. This was a dumb copy/paste indeed.

@zeha

zeha approved these changes May 17, 2018

@zeha

This comment has been minimized.

Collaborator

zeha commented May 17, 2018

I can see some code branches that could also be moved around, but lets do this in another PR.

@rgacogne rgacogne merged commit c12e8b3 into PowerDNS:master May 17, 2018

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: C/C++ No alert changes
Details
lgtm analysis: JavaScript No alert changes
Details
lgtm analysis: Python No alert changes
Details

@chbruyand chbruyand deleted the chbruyand:auth-opt-rr branch May 17, 2018

@pieterlexis pieterlexis referenced this pull request May 23, 2018

Merged

Auth 4.1.3 backports #6656

4 of 8 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment