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

WIP: (A/I)XFR-in/out (with TSIG), NOTIFY-in/out + demo zone persistence. #335

Draft
wants to merge 93 commits into
base: service-layering
Choose a base branch
from

Conversation

ximon18
Copy link
Member

@ximon18 ximon18 commented Jun 11, 2024

Based on the service-layering branch.

Manually tested as both primary and secondary with NSD and BIND.

Still lots to do:

  • Design review (and revisit if feedback requires it). E.g. some of the streaming support might warrant being handled less as a hack/extension of the existing client code and perhaps instead as a separate new XFR client module?
  • Sync with latest changes on main.
  • Remove notions of primary and secondary, instead allow XFR-in/out and NOTIFY to be enabled/allow per direction per zone ala how NSD supports XFR and NOTIFY. (Update: Primary and secondary notions are still present but no longer mutually exclusive)
  • More error handling & remove unwraps/panics.
  • Code cleanup.
  • Add tests (e.g. Stelline tests based on existing Unbound and NSD tests).
  • Extend Catalog to actually act like an RFC 9432 Catalog zone?
  • Revisit the various related RFCs and implement/omit/defer missing logic.
    • Add IXFR diff purging.
    • Add IXFR diff condensation.
  • Remove NOTIFY-set discovery? (Update: NOTIFY-set discovery is still present but disabled by default)
  • TSIG signing of both XFR-in and XFR-out.
  • Improve w.r.t memory usage, minimizing copies, support for Cargo features such as no std, etc.
  • Add and use more traits to make smaller units reusable instead of making the new middleware components dependent on one catalog impl, and add more strategy traits for making certain logic replaceable?
  • Documentation
  • Improved examples?

Introduces the following new major components:

Component Description
Catalog - For primary zones: "on-zone-change" triggered sending of NOTIFY messages zones to "secondaries". - For "secondary" zones: NOTIFY-in and SOA timer based SOA query and (A/I)XFR-out.
XfrMiddlewareSvc Handling of (A/I)XFR-in requests. Requires a Catalog.
NotifyMiddlewareSvc Handling of NOTIFY-in requests. Requires a Catalog.
TsigMiddlewareSvc Verify TSIG signed requests and sign responses.
net::client::auth::Connection TSIG signing and validation client layer for signing request(-stream)s and validating response(-stream)s. Used by Catalog.

Zone persistence on change (either edit to a local primary zone or sync of a local secondary zone with changes obtained from a remote primary) is demonstrated in examples/serve-zone.rs via the ArchiveZone impl of the ZoneStore trait and by using the same Zone wrapping "hack" that Catalog uses to monitor zones for changes.

ximon18 added 12 commits May 1, 2024 11:39
…al I/O errors occur (such as the other end has disconnected) and for not aborting response stream processing if there is no space in the write queue. These issues were hit when testing with the default dig timeout of 5 seconds and a large zone that took a while to start transferring.
…very many Tokio tasks, otherwise we compete with other actions being performed by the applicaition. Instead use a dedicated thread pool for zone walking for AXFR transfers. Also use a Tokio unbounded receiver stream instead of a ordered futures stream to avoid creating N boxed futures, to avoid a mutex lock around the stream, and to avoid waiting for the initial zone walk to finish before sending any AXFR records.
…imited support for A/IXFR in/out and NOTIFY, e.g. doesn't honor RETRY or EXPIRE SOA timers yet, and needs a lot of cleaning up. Has been seen successfully exchanging zones with NSD via XFR acting both as primary and secondary.
Relevant changes:
- Adds net::client::auth: a new client layer for doing TSIG request signing and response validation.
- Extends Catalog to use net::client::auth.
- Add Message::is_streaming() and ComposeRequest::is_streaming() which returns true if the QTYPE of the first question indicates a type of request which may cause a stream of responses instead of a single response, limited to AXFR and IXFR at the moment.
- Adds GetResponse::stream_complete() and GetResponse::is_stream_complete() to signal from a caller that the last message of a response stream has been seen (as only the caller can know this, the DNS protocol does not include a general purpose end marker for response streams if the TCP stream is kept open for further use) as TSIG stream validation needs to know when the last message of the stream has been encountered.
- Factors the code of ComposeRequest::to_message() out to the call sites (cache.rs, dgram.rs) and removes ComposeRequest::to_vec() as they were not compatible with net::client::auth.
- Extends Request::is_answer() to NOT require AXFR subsequent responses to have a question (as it is optional per RFC 5936).
- Extends net::client::stream support more than a single response to a single request:
  - Adds Config::set_streaming_response_timeout() and Config::set_initial_idle_timeout().
  - Adds Request::stream_complete.
  - Extends send_request() to behave differently if ComposeRequest::is_streaming() is true.
  - Preserves
- Extends the serve-zone example to pass a key store with a hard-coded HMAC-SHA256 TSIG key to the Catalog.

Unrelated changes:
- Also extend the serve-zone example to persist a zone to disk on commit, and optionally load it at startup.
- Removes an unnecessary problematic Clone bound on dgram.impl SendRequest for Connection in dgram.rs.
- Adds impl Display for net::client::stream::ConnState.
- Logs at trace level the reason a net::client::stream is closed.
- Logs failed handling of NOTIFY requests and returns an error to the client instead of panicking.
- Logs XFR-in progress.
- Replace the large match data block in Catalog with much simpler code.
@ximon18 ximon18 requested a review from a team June 11, 2024 21:54
@ximon18 ximon18 changed the title WIP: (A/I)XFR-in/out, NOTIFY-in/out and TSIG support. WIP: (A/I)XFR-in/out (with TSIG), NOTIFY-in/out Jun 11, 2024
@ximon18 ximon18 changed the title WIP: (A/I)XFR-in/out (with TSIG), NOTIFY-in/out WIP: (A/I)XFR-in/out (with TSIG) and NOTIFY-in/out Jun 11, 2024
@ximon18 ximon18 changed the title WIP: (A/I)XFR-in/out (with TSIG) and NOTIFY-in/out WIP: (A/I)XFR-in/out (with TSIG), NOTIFY-in/out + demo zone persistence. Jun 11, 2024
…n. Currently seems to break some other Stelline tests.
- Batch as many RRs per AXFR response as will fit.
- Support backward compatible AXFR out (one RRset per response) if configured for the client IP.
- Reject AXFR over UDP unless we're (a) transferring the entire zone for IXFR because there is no diff available, and (b) (TODO) the response fits in a single datagram.
- Disable the TCP idle timer while in a transaction to avoid timing out XFR responses (especially in Tokio paused time test mode but could also occur normally).
- Honour stream shutdown in the Stelline ClientServerChannel to avoid a never ending test.
- Use 127.0.0.1 as the client in Stelline server tests, not ::.
- Support EXTRA_PACKET in Stelline SECTION ANSWER blocks for streaming response matching.
- Add an initial IXFR Stelline test.
Changed and fixed the data structures and logic for XFR diffs, also enabling received IXFRs to be kept as diffs.
Changed how a connection factory is passed to the Catalog to enable it to be replaced with Stelline transports when testing.
Renamed ZoneType to ZoneConfig.
Add support for "allow-notify" and "request-xfr" in Stelline server configs.
Add support for specifying the Stelline request OPCODE.
Added a get_rrset) fn to WritableZoneNode.
The transmit loop looped from 0..max_retries _exclusive_, so with the default max_retries value of 5 it would go round the loop 5 times, i.e. 1 try and 4 retries, not 5 retries.

This commit adjusts the logic and allowed minimum value to match the actual behaviour.

This commit also fixes a typo in the word "capped" in the doc strings.
…ll be changed in a separate commit).

- Rename some tests.
- Use spaces not tabs.
- Fix indentation.
- Use the pattern QUERY in step N then CHECK_ANSWER in step N+1.
… dependency between NotifyMiddlewareSvc and Catalog and simplifying the trait bounds of NotifyMiddlewareSvc.
…d dependency between XfrMiddlewareSvc and Catalog and simplifying the trait bounds of XfriddlewareSvc. Also removed some unnecessary trait bounds along the way.
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

2 participants