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] add a DNSSEC conformance test suite #2155

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

japaric
Copy link
Contributor

@japaric japaric commented Feb 27, 2024

This proposal aims to produce a high reliability and high assurance test suite that will drive the DNSSEC implementation in hickory-dns to completion.

Rendered version of the RFC

Both the test framework and conformance test suite are already being developed out of tree. You are welcome to try them out at ferrous-systems/dnssec-tests

Note that even if the project does not have a formal RFC process, we still find it useful to use the pull request format to discuss the proposal.

RFC & implementation co-authors: @amanjeev @justahero @listochkin

@djc
Copy link
Collaborator

djc commented Feb 29, 2024

What is the purpose of this PR? Given that, ostensibly, large parts of the work have already been done, what is this PR/review intended to achieve?

Why does the test suite rely on networking? Could a similar level of testing be achieved by exchanging DNS messages in-process? If not, why not?

@japaric
Copy link
Contributor Author

japaric commented Mar 1, 2024

What is the purpose of this PR? Given that, ostensibly, large parts of the work have already been done, what is this PR/review intended to achieve?

Integration has yet to happen and how and when to proceed is up for discussion. There's the main proposal of merging the test suite into this repository and using the #[ignore] attribute at the source level to track implementation progress, but there's also the alternative of keeping the tests in a separate repository and tracking progress using a different mechanism, e.g. a lockfile.

There's an implementation of the test framework (library) part of the proposal but its API and feature set are not final. Third-party review and experimentation can provide valuable feedback on what's missing to thoroughly test a DNS implementation.

The bulk of the work which is writing the tests themselves is still pending. Hence the module organization can still be easily changed. Perhaps folks will bring ideas about new categories of tests that would warrant a different module organization or even a split of the test suite in multiple crates.

Why does the test suite rely on networking? Could a similar level of testing be achieved by exchanging DNS messages in-process? If not, why not?

DNS is a network protocol. Testing with networking let us test the implementation in a way that's closest to actual real world / end user usage.

RFCs covering DNS specify how an implementation must behave in networks with small MTUs (see TC bit) and in presence of IP fragmentation. Testing such behaviors using in-memory message passing / library calls would be difficult. Other common features not specified in RFC documents like IP allow and block lists would be similarly hard to test without actual networking.

Interoperability with other implementations being used in production is an important element of testing implementations of networking protocols. Here, networking also makes the task easier.

Testing the resolver without networking using library API is going to be a labor intensive and error prone task since it would involve mocking name servers. Using an existing name server implementation, like BIND or unbound, in actual end-to-end tests that involve networking is less work and less error prone.

Using this test suite lets us to detect differences in behavior and responses compared to other implementations used in production and ensure hickory is able to correctly handle the same scenarios.

In summary, having an end-to-end test suite that uses local, isolated networks support

  • testing aspects outlined in RFCs that library tests cannot cover
  • testing any network configuration required to match conditions outlined in RFCs, having full control over said configuration
  • checking interoperability ("does it play nicely with X?") and compatibility ("does it behave like X?") with implementations used in production, e.g. unbound, BIND

@bluejekyll
Copy link
Member

First, I don't know if we necessarily want the RFC process, is this project large enough for that? Maybe... the writeup you linked is very good, thank you for that.

Second, at a high level I like the proposal here. The big question I really have is, can we incorporate the existing work in Hickory's battery suite into this framework? It would be nice for this new framework to supplant these tests.

I spent some time to handle a few of the cases laid out above. For compatibility tests, we have this suite, currently only supporting BIND, for example the SIG0 compatibility tests: https://github.com/hickory-dns/hickory-dns/blob/main/tests/compatibility-tests/tests/sig0_tests.rs

Similarly, we have the battery suite (not network based) that is aimed at proving conformance with serving records properly, this is aimed at the server, for example, the DNSSEC conformance on record lookup: https://github.com/hickory-dns/hickory-dns/blob/main/crates/server/tests/authority_battery/dnssec.rs#L26

I think for the forwarder/resolver, we want a similar suite of tests, and ideally, it would all be self contained (i.e. not relying on remote request).

@justahero
Copy link
Contributor

@bluejekyll thanks for the feedback, really appreciate that you like the proposal.

First, I don't know if we necessarily want the RFC process

Our reasoning here was, writing an RFC has the advantage of following a known process, but we are not bound to it. We thought the proposal allowed for getting feedback early and was a way to determine if the approach is worth adding to the project, especially when the addition is quite big in scope.

The big question I really have is, can we incorporate the existing work in Hickory's battery suite into this framework? It would be nice for this new framework to supplant these tests.

When we researched the DNSSEC related RFCs we tried to find a way to test RFC requirements on one hand & ensure compatibility / conformance with existing tools on the other. At a first glance Hickory's battery test suite follows a similar approach, by setting up an authority to which queries are sent, then the received response records are checked. Imho these are good candidates to be covered by the dns test suite & would enable testing against multiple implementations. It can also help separating the roles hickory covers more clearly.

I spent some time to handle a few of the cases laid out above. For compatibility tests, we have this suite, currently only supporting BIND, for example the SIG0 compatibility tests: https://github.com/hickory-dns/hickory-dns/blob/main/tests/compatibility-tests/tests/sig0_tests.rs

These tests can be covered by the test suite too, as they test compatibility with existing tools using a similar approach. For now we would focus on adding DNSSEC / NSEC3 related tests, but like to encourage others to add or transfer tests that cover existing DNS requirements.

I think for the forwarder/resolver, we want a similar suite of tests, and ideally, it would all be self contained (i.e. not relying on remote request).

Using different scenarios / RFC based folders it should be easy to configure hickory with different roles, e.g. as name server or resolver. Resolver tests can run against multiple implementations to check that it conforms or is compatible to them. We found out, even the related RFCs leave some room for interpretation, therefore it's valuable to test against production used tools.

We are currently in the process of adding tests to sufficiently cover the DNSSEC / NSEC3 related RFC requirements. By running these tests against current implementations we already discovered a few minor differences, not only between the tools, but also differences in how the protocol is interpreted, e.g. return codes. The dns-test crate is still actively developed to provide an ergonomic API to set up these tests, but we hope it will be stable enough soon so others can use it as well.

With this test suite in place we will focus on implementing DNSSEC / NSEC3 related features to hickory itself. These tests acts as a high-level safety net to avoid regressions & to keep compatibility. They also allow us to explore what reference tools send over the wire. Of course more granular unit & integration tests will be added to hickory as well.

@japaric
Copy link
Contributor Author

japaric commented Apr 29, 2024

given the expressed desire to move existing integration tests into the dns-test framework and the fact that failing conformance tests are already being fixed in hickory (e.g. #2196), here's a new integration proposal that can be implemented in the very short-term:

  • the ferrous-systems/dnssec-tests repo gets merged / copy-pasted into this (hickory-dns/hickory-dns) repository. optionally, preserving the git commit history but it isn't the cleanest commit history of all time

  • the CODEOWNERS feature is used to grant ferrous' folks review permissions to merge PRs into the dns-test and conformance-tests packages / crates / directories. the hickory devs get review permissions on all files / directories

that way the ferrous' folks can continue to add conformance tests and improving the test framework without blocking on the availability of the hickory devs. this setup also lets hickory devs and contributors add / port core DNS tests to the conformance test suite with a bigger pool of reviewers able to review those PRs -- the conformance test suite could likely see lots of activity at the beginning and then gradually decrease as existing tests get ported and RFC coverage increases. PRs by ferrous' folks would need to be reviewed by hickory devs as it's the case today.

furthermore, to reduce CI load in presence of more PRs being merged in the short-term we can use a path filter to only run the existing hickory-dns CI jobs when hickory's source code is modified. that way code changes in the dns-test and conformance-tests packages do not trigger a run of hickory's unit tests. OTOH, changes in hickory's source code do trigger the execution of the conformance test suite; that way we can implement / use the #[ignore] + GitHub issue tracking mechanism proposed in the RFC text

@bluejekyll
Copy link
Member

bluejekyll commented Apr 30, 2024

the ferrous-systems/dnssec-tests repo gets merged / copy-pasted into this (hickory-dns/hickory-dns) repository. optionally, preserving the git commit history but it isn't the cleanest commit history of all time

I like this idea, @djc?

the CODEOWNERS feature is used to grant ferrous' folks review permissions to merge PRs into the dns-test and conformance-tests packages / crates / directories. the hickory devs get review permissions on all files / directories

This seems reasonable to me. I'm also open to allowing Ferrous merge capabilities as well. Narrowing it the dns-test suite/area seems reasonable. Or maybe just granting access to all of /tests/**?

@djc
Copy link
Collaborator

djc commented May 1, 2024

To check my understanding -- this would mean the existing separate repo is archived?

Sounds good to me -- I think using code owners for this makes sense. On balance, I would probably prefer that git history is brought along (using git merge --allow-unrelated-histories) even if it is kinda messy.

@japaric
Copy link
Contributor Author

japaric commented May 7, 2024

To check my understanding -- this would mean the existing separate repo is archived?

Correct. ferrous-systems/dnssec-tests would get archived. we can indicate in its README that it moved / merged into the hickory-dns/hickory-dns repo for code archaeology purposes.

Or maybe just granting access to all of /tests/**?

Ah, right. if Ferrous is to help approve PRs porting tests then we should also have perms to review code that currently lives in /tests. probably the easiest is to put the dnssec-tests code under that directory and add us as CODEOWNERS of /tests

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

4 participants