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

Use domain crate NSEC3 support. #6

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Conversation

ximon18
Copy link
Member

@ximon18 ximon18 commented Oct 15, 2024

Currently depends on domain branch initial-nsec3-generation, see NLnetLabs/domain#416.

Contains several kinds of tests:

  • Unit tests.
  • Integration tests in tests/ (ignored by default) that compare outputs to those of a host installed ldns-nsec3-hash command.
  • Fuzz tests in fuzz/ (must be explicitly run using cargo +nightly fuzz)

Pending issues:

  • The integration test is failing because dnst exits with code 2 on command line argument parsing failure, while ldns-nsec3-hash exits with code 1. As this is perhaps something that we need to deal with for all dnst xxx vs ldns-xxx commands and as PR Modify Clap usage to better match LDNS #13 showed one way to deal with this, I don't currently attempt to solve it in this PR.

  • Ideally we want to set the default number of iterations to 0 for dnst nsec3-hash and leave it at 1 for backward compatibility for ldns-nsec3-hash. This PR doesn't contain a way to handle that "muliticall" support or have different defaults for different binary names/sym links via which we are invoked. PR Modify Clap usage to better match LDNS #13 showed one way to deal with this but has not been accepted, and PR Implement ldns-update and dnst update #10 also intends to offer a way to deal with this. Once the way forward is clear this PR should be updated to use the chosen approach.

  • This PR also thus doesn't yet test invocation of itself under the name ldns-nsec3-hash.

@ximon18 ximon18 marked this pull request as draft October 15, 2024 14:44
@ximon18 ximon18 marked this pull request as ready for review October 17, 2024 13:32
@ximon18 ximon18 mentioned this pull request Oct 17, 2024
12 tasks
@ximon18 ximon18 changed the base branch from main to add-ci-workflow October 30, 2024 00:09
@ximon18 ximon18 changed the base branch from add-ci-workflow to main October 30, 2024 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant