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

DNSSec (RFC 4034 et al) support #251

Closed
wants to merge 50 commits into from
Closed

DNSSec (RFC 4034 et al) support #251

wants to merge 50 commits into from

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Jan 17, 2021

This is work in progress, some bits and pieces are still missing (such as NSEC RR, canonicalization of Domain_name.t and RRSet). Feedback welcome.

@hannesm
Copy link
Member Author

hannesm commented Jan 18, 2021

  • key tag computation missing
  • CI shows failures on 32bit systems that should be resolved

src/dns.mli Show resolved Hide resolved
@avsm
Copy link
Member

avsm commented Jan 18, 2021

This is a surprisingly small PR for such a big feature! Nice work.

@hannesm
Copy link
Member Author

hannesm commented Nov 3, 2021

now rebased onto the main branch

@hannesm
Copy link
Member Author

hannesm commented Jan 8, 2022

(likely incomplete) leftovers:

  • cname proofs (needs dns-client compress changes DNS client "compresses" answers with CNAMEs #215)
  • nsec3 support: base32, hashing, proofs of non-existance, opt-out
  • dealing with superfluous (and badly signed) nsec/nsec3 (embedded by some person in the middle)
  • only support <= 150 iterations in nsec3 https://kb.isc.org/docs/dnssec-signed-zones-best-practice-guidance-for-nsec3-iterations
  • "aggressive caching" (resolver) RFC 8198
  • warn on SHA1 usage
  • distinguish dnssec errors: bad signature, unsupported algorithms, signature expired (not yet incepted), missing RR
  • resolver: support DO, AD, CD bits (and act accordingly) [also integrate dnssec validation into resolver)
  • server side (+support in zone reader)

new RFCs supported (more?)

  • RFC 7129 (denial of existence)
  • RFC 4034 (dnssec RR)
  • RFC 4035 (dnssec protocol)
  • RFC 4033 (dnssec base)
  • RFC 5155 (nsec3): TODO nsec3param, some missing logic
  • RFC 4648 (base32)
  • RFC 4592 (wildcard)
  • RFC 4470 (minimal covering nsec) client/resolver side fine

@hannesm
Copy link
Member Author

hannesm commented Jan 8, 2022

some cname stuff is there, but _build/default/app/odnssec.exe --host trac.ietf.org --type NS -vv --nameserver 9.9.9.9 results in "error from resolver invalid reply, couldn't find SOA" (using other resolvers works fine, but also results in different replies) -- it is the answer of the query for DS trac.ietf.org...

EDIT: now getting a more complete reply, unclear what to do about it... defered to later.

@hannesm
Copy link
Member Author

hannesm commented Jan 11, 2022

about the nsec denial of existence for *.se issue, https://lists.dns-oarc.net/pipermail/dns-operations/2022-January/021502.html should be read and the code adapted to accept the answers

@hannesm
Copy link
Member Author

hannesm commented Jan 12, 2022

now the tests are working, maybe need to re-review the code, merge it, and get back to the resolver implementation (+client)

@hannesm
Copy link
Member Author

hannesm commented Jan 14, 2022

ok, after a round of code review and implementing the tests from 5155 (nsec3):

  • all tests are passing that have valid chains
  • unsigned delegations are atm not accepted (4035 B5 / 5155 B3), and are commented out
  • what about nsec_chain when the nsec has been wildcard-expanded (and used_name is not owner_name)? figure out an example
  • in nsec3 handling, there's the requirement (8.3) to sometimes check for certain bits set (Ns/Soa), this is not implemented
  • nsec no-data should also ensure that cname bit is not set
  • 8.5 vs 8.6 in RFC5155 (no data reply for DS/non-DS) -- esp. with errata of that RFC https://www.rfc-editor.org/errata/eid3441

the next question is about API:

  • what is the common use case (only signed / allow unsigned replies)?
  • should the dnssec API be disjoint (different module & package from the dns-client / dns-resolver)?
  • should there be a dns-client implementation that uses dnssec?
  • recursive dnssec resolver (aggressive caching / only store signed stuf / ...)

but I also think this PR is already large enough - and should be merged once the three tasks above are satisfied (and we convinced ourselves that the code validates correctly). //cc @reynir

@hannesm
Copy link
Member Author

hannesm commented Jan 17, 2022

TODO: research whether the rrsig signer_name should be verified somewhere?

@hannesm
Copy link
Member Author

hannesm commented Jan 18, 2022

rebased, squashed and merged manually.

@hannesm hannesm closed this Jan 18, 2022
@hannesm hannesm deleted the dnssec branch January 18, 2022 16:05
@hannesm hannesm mentioned this pull request Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants