Add TSIG middleware. #380
Merged
Add TSIG middleware. #380
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tain number of bytes should be reserved (to make space for post-processing adding EDNS options to the response), and (b) strongly typed passing of arbitrary metadata between middleware that produces a type and middleware that consumes the type (e.g. a TSIG key name).
…to be part of this PR branch.
…to be part of this PR branch.
…nded to be part of this PR branch.
…nded to be part of this PR branch.
… a function is no longer possible.
… fix-edns-middleware / PR #355.
…o that a wrapper client (such as a future TSIG client) can augment the message before it gets sent.
- Added a simple UDP client that doesn't interfere with requests before sending (for TSIG testing). - Added support for receiving multiple responses (for XFR testing). - Added support for connection timeout errors - Added support for connection termination errors (for EDNS testing). - Added support for specifying the TSIG key to use (for TSIG testing). - Added support for $ORIGIN in zone file fragments. - Simplified rcode checking and use it instead of the yxrrset BADCOOKIE hack. - In memory channel changes: - Fixed a trace message that incorrectly referred to client instead of server. - Fixed a too-tight connection read loop that was preventing Tokio task switching. - Added connection shutdown detection. - Fixed incorrect setting of TCP mode to true when UDP mode was requested.
…an error message.
…t' into stelline-server-testing-changes
…t' into stelline-server-testing-changes. Update Stelline based server tests to work with new multiple response capable Rust types and alternate stream end detection mechanism.
|
That needs to listed as a limitation. Certainly for UPDATE that is something we need. |
…f available, via a new TruncationContext type. - Truncate for TCP as well as UDP. - Return ServiceError::InternalError if truncation fails. - Implement handling of ServiceError in DgramServer and stream::Connection. - Break DgramServer and stream::Connection dispatch to service code out into helper RequestDispatcher types. - Terminate the response stream if ServiceError::InternalError occurs.
Done. |
ximon18
commented
Sep 16, 2024
There was a problem hiding this comment.
I think that for a middleware TSIG layer it is best to reject what we cannot support at the moment. So if get_relevant_question fails then don't proceed.
After some internal discussion it seems to me that there is no reason to restrict the opcode or query type on which TSIG verification is performed. If the request has a TSIG RR and the server stack includes TSIG middleware then that middleware should verify the TSIG signature and also sign any corresponding response. So I'm going to remove this restriction entirely.
Philip-NLnetLabs
approved these changes
Sep 16, 2024
Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>
Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>
Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>
tertsdiepraam
approved these changes
Sep 16, 2024
ximon18
added a commit
that referenced
this pull request
Oct 3, 2024
- Add TSIG response signing middleware. - Update Stelline server integration tests to use the new TSIG middleware. - Add TSIG Stelline test recipe. - TSIG module changes: - Derive Clone for Key. - Added Key::compose_len() for determining how many response bytes to reserve. - Added ClientTransaction::wrapped_key(), ClientSequence::wrapped_key() and SigningContext::wrapped_key() to access the real underlying "wrapped" key type. - Added From<ServerTransaction<K>> for ServerSequence<K>. - Various minor RustDoc improvements. - Make Time48 obey mock (predictable and controllable) time so that TSIG signing uses mock time during Stelline tests. Other: - Remove unnecessary Unpin bounds on net::server::message::Request. - Remove unnecessary Clone bound on impl SendRequest for net::client::dgram::Connection. - Remove unnecessary mutex lock on middleware post-processing response state and rename PostprocessingConfig to PostprocessingState to better reflect its mutable nature. - Don't set the AA flag on test service responses as (a) actual zone serving doesn't do this yet and this still needs fixing, and (b) it violates the expectations of the TSIG Stelline test that verifies at a byte level the TSIG response signature. --------- Co-authored-by: Philip Homburg <philip@nlnetlabs.nl> Co-authored-by: Philip-NLnetLabs <93709748+Philip-NLnetLabs@users.noreply.github.com> Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Taken from the
xfrbranch in order to split PR #335 into several smaller PRs.Notable changes:
net::server::message::Request.CloneforKey.Key::compose_len()for determining how many response bytes to reserve.From<ServerTransaction<K>> for ServerSequence<K>.tsig::ServerError::unsigned()public.Time48obey mock (predictable and controllable) time so that TSIG signing uses mock time during Stelline tests.This PR will also benefit from merging PR #334 and PR #390.