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

RFC: Allow AXFR of nsec3 narrow zones #5074

Closed
wants to merge 4 commits into from

Conversation

baloo
Copy link
Contributor

@baloo baloo commented Feb 23, 2017

I believe it's okay to allow axfr on narrow zone and that it does not
introduces security problems.

The narrow mode will dynamically create a nsec3 range around the
record.

Something like:

$ dig +dnssec aaaab.meeeee.eu.
;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 28784
;; AUTHORITY SECTION:
hkvb2h0gtd2nbiuk741aab6cbkl1fsg8.meeeee.eu. 10800 IN NSEC3 1 0 3 EA33014A HKVB2H0GTD2NBIUK741AAB6CBKL1FSGA

In this example, the hash of aaaab.meeeee.eu. is hkvb2h0gtd2nbiuk741aab6cbkl1fsg9.meeeee.eu
which translate to a NSEC3 record like:

hkvb2h0gtd2nbiuk741aab6cbkl1fsg8.meeeee.eu. 10800 IN NSEC3 1 0 3 EA33014A HKVB2H0GTD2NBIUK741AAB6CBKL1FSGA
\------------------------------/                           \------------/ \------------------------------/
                     |                                           |                      |
Start of the range --/                                           |                      |
Alg + Salt (NSEC3PARAM) -----------------------------------------/                      |
End of the range   ---------------------------------------------------------------------/

While correct, this is not exact. The range could be larger than
that. In my case, the largest range could be:

ftdcumdfkc5rqsv077mjp9v0bb19c6ev.meeeee.eu. 10800 IN NSEC3 1 0 3 EA33014A I2UF1DNHSOTNACGN056KBFGBFPIICNMG A AAAA RRSIG

Because ]hkvb2h0gtd2nbiuk741aab6cbkl1fsg8;hkvb2h0gtd2nbiuk741aab6cbkl1fsga[
is a subset of ]ftdcumdfkc5rqsv077mjp9v0bb19c6ev;i2uf1dnhsotnacgn056kbfgbfpiicnmg[
both are correct, the later being the largest range possible.

This would make the primary reply differently than a slave. While this
may looks strange, I believe this is accepted by all the RFC I know.

Not being 100% sure, I'd prefer to ask. Please let me know if I forgot
something.

Short description

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@Habbie
Copy link
Member

Habbie commented Feb 23, 2017

I believe it's okay to allow axfr on narrow zone and that it does not
introduces security problems.

Security is not the reason - the reason is that it is impossible to generate valid signed zone files from a NARROW zone.

What your patch effectively does is: when somebody tries to axfr a NARROW zone, you pretend it is not NARROW but just NSEC3, and the NSEC3 ranges generated are in fact 'full size', not narrow.



Furthermore, allowing AXFR of NARROW zones by default (in whatever shape this AXFR will come) is in fact a security issue, so it should be optional and default to off. However, I see no reason to have this feature at all.

@pieterlexis
Copy link
Contributor

Have you tested this? I think the slave (if it is PowerDNS) will set PRESIGNED for this domain on AXFR, using the signatures from the AXFR. As the key-material is not available on the slave, it will generate NSEC3's where it cannot find a signature for and fail validation (unless the cryptokeys are also on the slave).

Another issue I can think of is the fact that one needs to set NSEC3NARROW on the slave for this domain.

@Habbie
Copy link
Member

Habbie commented Feb 23, 2017

Have you tested this? I think the slave (if it is PowerDNS) will set PRESIGNED for this domain on AXFR, using the signatures from the AXFR. As the key-material is not available on the slave, it will generate NSEC3's where it cannot find a signature for and fail validation (unless the cryptokeys are also on the slave).

Another issue I can think of is the fact that one needs to set NSEC3NARROW on the slave for this domain.

The patch (minor some glitches) actually emits a non-narrow nsec3 zone on axfr, so assuming this is what anybody wants, it does make sense.

@Habbie
Copy link
Member

Habbie commented Feb 23, 2017

So, to even consider merging (not promising anything yet), we need:

  • tests that actually exercise this feature
  • configurability (needs to default to off)
  • documentation

@baloo baloo force-pushed the baloo/features/narrow-axfr branch 3 times, most recently from 8b13f72 to 81ef728 Compare February 24, 2017 18:32
@baloo
Copy link
Contributor Author

baloo commented Feb 24, 2017

This includes the commit from #5083 which fixes axfr rectify (no nsec3 records for ENT with unsafe DS)

@baloo baloo force-pushed the baloo/features/narrow-axfr branch 8 times, most recently from 36bdd4e to 45ad222 Compare February 28, 2017 01:07
@baloo
Copy link
Contributor Author

baloo commented Feb 28, 2017

@Habbie Let me know what you think.

@baloo baloo force-pushed the baloo/features/narrow-axfr branch 7 times, most recently from c749bc4 to 13896b5 Compare March 1, 2017 18:53
@baloo
Copy link
Contributor Author

baloo commented Mar 1, 2017

test passed \o/

@baloo baloo force-pushed the baloo/features/narrow-axfr branch from 13896b5 to e12c027 Compare March 21, 2017 18:24
echo --- $validator $zone
if [ "$validator" = "named-checkzone" ]
if [[ $skipreasons = *narrow* ]] &&\
[ "$validator" = "ldns-verify-zone -V2" ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Power Rangers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? I don't understand

@Habbie Habbie self-assigned this Jun 1, 2017
Arthur Gautier added 4 commits June 1, 2017 17:33
I believe it's okay to allow axfr on narrow zone and that it does not
introduces security problems.

The narrow mode will dynamically create a nsec3 range around the
record.

Something like:
```
$ dig +dnssec aaaab.meeeee.eu. CAA
;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 28784
;; AUTHORITY SECTION:
hkvb2h0gtd2nbiuk741aab6cbkl1fsg8.meeeee.eu. 10800 IN NSEC3 1 0 3 EA33014A HKVB2H0GTD2NBIUK741AAB6CBKL1FSGA
```

In this example, the hash of `aaaab.meeeee.eu.` is `hkvb2h0gtd2nbiuk741aab6cbkl1fsg9.meeeee.eu`
which translate to a NSEC3 record like:
```
hkvb2h0gtd2nbiuk741aab6cbkl1fsg8.meeeee.eu. 10800 IN NSEC3 1 0 3 EA33014A HKVB2H0GTD2NBIUK741AAB6CBKL1FSGA
\------------------------------/                           \------------/ \------------------------------/
                     |                                           |                      |
Start of the range --/                                           |                      |
Salt (NSEC3PARAM)  ----------------------------------------------/                      |
End of the range   ---------------------------------------------------------------------/
```

While correct, this is not **exact**. The range could be larger than
that. In my case, the largest range could be:
```
ftdcumdfkc5rqsv077mjp9v0bb19c6ev.meeeee.eu. 10800 IN NSEC3 1 0 3 EA33014A I2UF1DNHSOTNACGN056KBFGBFPIICNMG A AAAA RRSIG
```

Because `]hkvb2h0gtd2nbiuk741aab6cbkl1fsg8;hkvb2h0gtd2nbiuk741aab6cbkl1fsga[`
is a subset of `]ftdcumdfkc5rqsv077mjp9v0bb19c6ev;i2uf1dnhsotnacgn056kbfgbfpiicnmg[`
both are correct, the later being the largest range possible.

This would make the primary reply differently than a slave. While this
may looks strange, I believe this is accepted by all the RFC I know.

Not being 100% sure, I'd prefer to ask. Please let me know if I forgot
something or if you'd prefer to hide this under a tunable (a domain
metadata maybe).

Signed-off-by: Arthur Gautier <baloo@gandi.net>
Signed-off-by: Arthur Gautier <baloo@gandi.net>
Signed-off-by: Arthur Gautier <baloo@gandi.net>
Signed-off-by: Arthur Gautier <baloo@gandi.net>
@baloo baloo force-pushed the baloo/features/narrow-axfr branch from e12c027 to ff2dd5f Compare June 1, 2017 17:34
@@ -40,7 +40,7 @@ gsql_master()
--dnsupdate=yes --resolver=8.8.8.8 --outgoing-axfr-expand-alias=yes \
--expand-alias=yes \
--cache-ttl=$cachettl --dname-processing \
--disable-axfr-rectify=yes $lua_prequery &
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe my tests are wrong. I shouldn't remove disable-axfr-rectify=yes there, probably a new test in regression-tests.nobackend would be more appropriate

@Habbie
Copy link
Member

Habbie commented Jun 19, 2017

This PR has conflicts.

@Habbie Habbie removed their assignment Feb 8, 2018
@Habbie
Copy link
Member

Habbie commented May 31, 2018

Closing due to

  • inactivity
  • us not understanding why anybody would want this

@Habbie Habbie closed this May 31, 2018
@baloo
Copy link
Contributor Author

baloo commented May 31, 2018

@Habbie Our datastructure does not have an efficient way to store nsec3-hashed previous/after records per zone (therefor we need the nsec3-narrow). To implement the AXFR feature, I relied on this "hack" that makes our server reply to AXFR with a non-narrow zone (thanksfuly powerdns will rectify the zone before signing it and replying).

This branch is inactive, but I use it in production and I'd be happy to provide rebased version of it.

Let me know if you have interest to have this merged.

@Habbie
Copy link
Member

Habbie commented Jun 1, 2018

@baloo so remind me - if a correctly rectified zone comes out, why do you need NARROW on this server? How about just not answering queries from this machine?

@Habbie Habbie reopened this Jun 1, 2018
@baloo
Copy link
Contributor Author

baloo commented Jun 4, 2018

if a correctly rectified zone comes out, why do you need NARROW on this server?

We would need dedicated AXFR servers and have others with narrow.

How about just not answering queries from this machine?

That would work, but would make things weird (server would SERVFAIL to all queries, making it weird for an admin on the machine, servfail is not an expected behavior).

@Habbie
Copy link
Member

Habbie commented Jun 7, 2018

Those 'dedicated servers' could just be another pdns process on the same machines. I'm not a huge fan of this PR.

@mind04
Copy link
Contributor

mind04 commented Jun 7, 2018

The pull is 'fixing' a very specific problem. But it is also adding confusion in the process.

Security is not the reason - the reason is that it is impossible to generate valid signed zone files from a NARROW zone.

If the zone is not valid why are you serving it to the outside world?

There is a good alternative, and the use case is questionable, so i agree with @Habbie

@baloo
Copy link
Contributor Author

baloo commented Jun 11, 2018

The zone is valid, but I can't serve non-narrow nsec3 records from my backend (it's not easy for our datastructure to maintain this). This PR is hack to have powerdns rectify the zone on the fly (and generate non-narrow nsec3 records) when doing an AXFR and resign the whole thing.

I do not see running two dns servers as a good alternative myself. And I would need three different process (with a dnsdist in front to split between axfr and regular queries) on this machine while a simple (or what I think is a simple) change would do.

I do maintain this patchset and have been for over a year now (not with the travis tests though), and it's integrated in our build system. If you don't want it, I'll just continue to carry on because it ease things for my deployment.

PS: no hard feelings here, this is not my software, I do not take half as much care for it as you do. If you do not see as much interest as I do in this patchset, closing this is perfectly fine with me. I'll just continue to maintain it separately :)

@aerique
Copy link
Member

aerique commented Nov 2, 2018

Status of this PR is unclear to me. @Habbie @mind04 do we need this feature in PDNS?

@Habbie
Copy link
Member

Habbie commented Nov 2, 2018

@baloo sorry, closing this.

@Habbie Habbie closed this Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants