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

Add signzone command. #8

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

Conversation

ximon18
Copy link
Member

@ximon18 ximon18 commented Oct 17, 2024

Currently depends on domain branch initial-nsec3-generation, which has had multiple branches/PRs merged into it (see NLnetLabs/domain#416)

Supports:

  • The basic command line arguments zonefile key [key [key]] and the NSEC3 arguments -n, -a, -t, -s and -p.
  • Additional command line arguments -o, -i, d, -e, -f,-u, -A, -U and -v.
  • -z and -Z for ZONEMD

Partially supports:

  • Command line argument -b (support for Bubble Babble DS comment output is not planned at present).

Lacks but should have support for:

Lacks and do not plan to add support for:

  • OpenSSL engine related arguments.
  • Bubble Babble DS comment output.

Other:

  • Partial signing and re-signing: LDNS has strange behaviour here, so dnst removes DNSSEC records on loading already signed zonefiles.
  • Verify that it is expected that the -U option causes a warning from dnssec-verify (it also does so for the original ldns-signzone when using -U so I think this is fine). We should think do we want to support the -U option for dnst signzone?
  • Rendering zonefile entries may not exactly match the output of ldns-signzone as the behaviour is determined by the domain crate. (see ldns_rr2buffer_str_fmt() in LDNS). Known differences are:
    • Some RDATA values are cased differently, but all known examples relate to RFCs that say that the text in question is "case-insensitive", so this is a difference but not an error.

Additional related DRAFT PRs:

This PR adds automated tests but has also been tested manually against the original ldns-signzone and dnssec-signzone.

@ximon18 ximon18 requested a review from a team October 17, 2024 13:32
@ximon18 ximon18 marked this pull request as ready for review October 17, 2024 13:32
@ximon18 ximon18 changed the title Add ldns-sign-zone like support. Add ldns-signzone like support. Oct 17, 2024
@ximon18 ximon18 mentioned this pull request Oct 17, 2024
12 tasks
@ximon18 ximon18 marked this pull request as draft October 28, 2024 12:55
Copy link
Contributor

@bal-e bal-e left a comment

Choose a reason for hiding this comment

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

Good work, @ximon18! We'll have to see whether the argument parsing needs to be changed to separate ldns / dnst, but the code generally looks good.

Cargo.toml Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
src/commands/signzone.rs Outdated Show resolved Hide resolved
@ximon18
Copy link
Member Author

ximon18 commented Jan 7, 2025

dnst-signzone currently allows not specifying a key to sign with. In that case it just copies the original zone over into the .signed file (with no signatures ofc).

That's a bug.

This has been resolved.

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.

4 participants