Rewrite of the specification of DNSSEC10#1179
Merged
Merged
Conversation
tgreenx
reviewed
Jul 18, 2023
tgreenx
left a comment
Contributor
There was a problem hiding this comment.
I don't have much to say in the test procedure of this specification. The logic looks good to me as is.
tgreenx
reviewed
Jul 18, 2023
tgreenx
left a comment
Contributor
There was a problem hiding this comment.
- The section "Terminology" can be removed.
- In section "Special procedural requirements", use the proper sentence from the template.
This was
linked to
issues
Jul 18, 2023
Contributor
Author
I think it is better to keep it, but with a comment.
Fixed. |
marc-vanderwal
left a comment
Contributor
There was a problem hiding this comment.
Just one more typo :-)
marc-vanderwal
previously approved these changes
Jul 25, 2023
Contributor
Author
|
@marc-vanderwal, see my response in #1179 (comment). Any comment? @marc-vanderwal, see my response in #1179 (comment). Any comment? @tgreenx, see my response in #1179 (comment). Any comment? |
tgreenx
reviewed
Jul 25, 2023
Contributor
Author
|
The added reference to DNSSEC05 should be reviewed in the context of PR #1183. |
tgreenx
reviewed
Jul 25, 2023
Contributor
Author
|
@tgreenx and @marc-vanderwal, please re-review. |
tgreenx
reviewed
Jul 25, 2023
marc-vanderwal
previously approved these changes
Nov 7, 2024
tgreenx
previously approved these changes
Nov 7, 2024
Contributor
Author
|
@tgreenx, can I merge this now? |
This update make the test continue to the NSEC3PARAM query even in the case where the response to the NSEC query gives neither NSEC in answer section nor NSEC3 in authority section. The handling of the NSEC3PARAM query has been updated in the same way even though it does not does not have any practical effect, but makes the two equal. In both cases two levels were merged and all bullets bellow moved up one level.
Contributor
Author
|
@tgreenx and @marc-vanderwal, please re-review. Commit 7383674 has been added. The change in commit 7383674 is smaller than it looks like. Under 6.5 (NSEC query), 6.5.3, 6.5.3.1 and 6.5.3.2 were changed and merged. Everything under 6.5.3.2 are unchanged, but moved up one level and now exist directly under 6.5.3. Under 6.6 (NSEC3PARAM query), 6.6.3, 6.6.3.1 and 6.6.3.2 were changed and merged. Everything under 6.6.3.2 are unchanged, but moved up one level and now exist directly under 6.6.3. This should have a small impact on implementation. The change for NSEC query makes the test continue to NSEC3PARAM query even if there is no NSEC record in answer section and no NSEC3 record in authority section. The change for NSEC3PARAM query should have no practical effect, but makes the specification consistent. |
marc-vanderwal
previously approved these changes
Nov 15, 2024
marc-vanderwal
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me.
tgreenx
reviewed
Nov 19, 2024
Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>
matsduf
added a commit
to matsduf/zonemaster
that referenced
this pull request
Nov 20, 2024
matsduf
added a commit
to matsduf/zonemaster
that referenced
this pull request
Nov 20, 2024
matsduf
added a commit
to matsduf/zonemaster
that referenced
this pull request
Nov 20, 2024
tgreenx
approved these changes
Nov 26, 2024
This was referenced Dec 9, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Purpose
There are several issues concerning problems with both the specification and implementation if DNSSEC10:
The old version of the test case tries to test NSEC/NSEC3 by creating a non-existing domain name, which is quite complex when the zone has a wildcard. NSEC3 opt out increases the complexity.
This rewrite changes the way NSEC/NSEC3 is tested. NSEC/NSEC3 records have two purposes in DNSSEC:
The current version focuses on non-exiting domain names. The new version provided in this PR focuses instead on record type that do not exist, and it queries for record types that cannot exist in the zone. This will make the implementation less complex.
Context
The issues above.
Test zones will be added to this PR, but the specification is complete and ready for review.
Changes
The rewrite of the specification requires a rewrite of the implementation.
How to test this PR
This has to be reviewed.