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.

Sign up for GitHub

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

Jump to bottom

Rewrite of the specification of DNSSEC03 #1189

Merged
merged 11 commits into from
Nov 22, 2023
Merged

Rewrite of the specification of DNSSEC03 #1189

merged 11 commits into from
Nov 22, 2023

Conversation

matsduf
Copy link
Contributor

@matsduf matsduf commented Jul 28, 2023 •
edited

Purpose

This is a complete rewrite of the specification for DNSSEC03:

  • Format based on template
  • Updated to match new RFC
  • Extended to check not only iteration value, but also the other parameters in the NSEC3 record.

Context

Resolves #1177, #1047, #1039 and #948. Resolves does not necessarily mean that this PR follow the issues, but when this PR has been completed those issues can be closed.

This PR introduces a new argument name, which means that Argument list must be updated (-> see #1190).

When this PR has been completed, the implementation must also be rewritten.

Test zones are available in #1218.

How to test this PR

Review the specification.

@matsduf matsduf added the A-TestCase Area: Test case specification or implementation of test case label Jul 28, 2023
@matsduf matsduf added this to the v2023.2 milestone Jul 28, 2023
This was referenced Jul 28, 2023
Closed
Open
Open
Open
This was linked to issues Jul 29, 2023
support RFC 9276-guidance for NSEC3 in DNSSEC03 testcase #1177
Closed
Update DNSSEC03 #1047
Open
NSEC3 iterations #948
Open
Missing case in DNSSEC03 #1039
Open
matsduf added a commit to matsduf/zonemaster that referenced this pull request Jul 30, 2023
@matsduf
Adds "int" name needed for DNSSEC03 as updated by zonemaster#1189
b67e63f
@matsduf matsduf mentioned this pull request Jul 30, 2023
Merged
tgreenx
tgreenx reviewed Aug 1, 2023
View reviewed changes
Copy link
Contributor

@tgreenx tgreenx left a comment

Choose a reason for hiding this comment

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

The specification reads well and I have not found any errors in the Test Procedure.

docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
matsduf
matsduf commented Aug 1, 2023
View reviewed changes
Copy link
Contributor Author

@matsduf matsduf left a comment

Choose a reason for hiding this comment

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

@tgreenx, thanks for your review!

docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
@matsduf matsduf requested a review from tgreenx August 1, 2023 11:06
tgreenx
tgreenx previously approved these changes Aug 2, 2023
View reviewed changes
docs/public/specifications/tests/DNSSEC-TP/dnssec03.md Outdated Show resolved Hide resolved
@matsduf
Copy link
Contributor Author

matsduf commented Aug 2, 2023

@mustafa-alrifaee writes in #1177 (comment):

Regarding the new modification on the test case DNSSEC03, what is the expected behavior now if the NSEC3 Parameter Settings in the zone do not comply with RFC 9276? Will it trigger a warning? Does it make sense for the recommendation to be just a warning without failing the test?

Both non-zero iteration and non-empty salt will result in message tags (DS03_ILLEGAL_ITERATION_VALUE and DS03_ILLEGAL_SALT_LENGTH, respectively) have ERROR level as default values according to the proposal.

@mustafa-alrifaee, doesn't that meet your proposal?

@matsduf matsduf requested a review from tgreenx August 2, 2023 15:53
tgreenx
tgreenx previously approved these changes Aug 2, 2023
View reviewed changes
marc-vanderwal
marc-vanderwal previously approved these changes Sep 13, 2023
View reviewed changes
@matsduf matsduf mentioned this pull request Oct 20, 2023
Merged
@matsduf
Copy link
Contributor Author

matsduf commented Oct 25, 2023

However, as discussed in the last F2F, before we merge we should also have the specification of test zones (here or in another PR) for this Test Case.

Test zones are now available in #1218.

@matsduf
Copy link
Contributor Author

matsduf commented Oct 25, 2023

@tgreenx, please re-review this and #1218.

tgreenx added a commit to tgreenx/zonemaster-engine that referenced this pull request Nov 15, 2023
@tgreenx
Update implementation of DNSSEC03
d8469a5 
Follows speficiation update (zonemaster/zonemaster#1189).
Note that Prefix Suffix List check (step 13.2.2.1) is not yet implemented, as this service is not yet provided by Zonemaster.
tgreenx added a commit to tgreenx/zonemaster-engine that referenced this pull request Nov 15, 2023
@tgreenx
Update implementation of DNSSEC03
25036e6 
Follows specifiation update (zonemaster/zonemaster#1189).
Note that Prefix Suffix List check (step 13.2.2.1) is not yet implemented, as this service is not yet provided by Zonemaster.

Also note that this commit does not update unit tests yet.
@tgreenx tgreenx mentioned this pull request Nov 15, 2023
Merged
tgreenx added a commit to tgreenx/zonemaster-engine that referenced this pull request Nov 16, 2023
@tgreenx
Update implementation of DNSSEC03
82e128e 
Follows specifiation update (zonemaster/zonemaster#1189).
Note that Prefix Suffix List check (step 13.2.2.1) is not yet implemented, as this service is not yet provided by Zonemaster.

Also note that this commit does not update unit tests yet.
@matsduf @tgreenx
Update docs/public/specifications/tests/DNSSEC-TP/dnssec03.md
3d88f17 
Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>
tgreenx
tgreenx previously approved these changes Nov 21, 2023
View reviewed changes
@tgreenx tgreenx self-requested a review November 21, 2023 17:23
@matsduf @tgreenx
Update docs/public/specifications/tests/DNSSEC-TP/dnssec03.md
82a8411 
Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>
@matsduf
Copy link
Contributor Author

matsduf commented Nov 21, 2023

@tgreenx and @marc-vanderwal, please re-review.

marc-vanderwal
marc-vanderwal approved these changes Nov 22, 2023
View reviewed changes
Copy link
Contributor

@marc-vanderwal marc-vanderwal left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@matsduf matsduf merged commit c85ca49 into zonemaster:develop Nov 22, 2023
@matsduf matsduf deleted the update-dnssec03 branch November 22, 2023 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgreenx tgreenx tgreenx approved these changes

@marc-vanderwal marc-vanderwal marc-vanderwal approved these changes

@mattias-p mattias-p Awaiting requested review from mattias-p

@hannaeko hannaeko Awaiting requested review from hannaeko

Assignees
No one assigned
Labels
A-TestCase Area: Test case specification or implementation of test case
Projects
None yet
Milestone
v2023.2
3 participants
@matsduf @tgreenx @marc-vanderwal