Merged
Conversation
justahero
reviewed
Jul 22, 2024
justahero
reviewed
Jul 22, 2024
listochkin
force-pushed
the
al-nsec3-validation
branch
from
f998ff5 to
9ae6ab0
Compare
listochkin
force-pushed
the
al-nsec3-validation
branch
2 times, most recently
from
3438abb to
6fc2740
Compare
listochkin
requested review from
bluejekyll,
djc,
japaric,
marcus0x62 and
pvdrz
as code owners
listochkin
force-pushed
the
al-nsec3-validation
branch
3 times, most recently
from
ff2a614 to
5b80ab6
Compare
listochkin
force-pushed
the
al-nsec3-validation
branch
5 times, most recently
from
d0825c8 to
cd3cc54
Compare
marcus0x62
reviewed
Aug 28, 2024
Collaborator
marcus0x62
left a comment
marcus0x62
left a comment
There was a problem hiding this comment.
@listochkin I've added some initial feedback - I should have the rest of it in by tomorrow.
djc
reviewed
Aug 29, 2024
listochkin
force-pushed
the
al-nsec3-validation
branch
8 times, most recently
from
e8ba933 to
e1891aa
Compare
pvdrz
reviewed
Aug 29, 2024
pvdrz
reviewed
Aug 29, 2024
Contributor
pvdrz
left a comment
pvdrz
left a comment
There was a problem hiding this comment.
This looks good to me. I left some comments about minor things that don't change the PR logic in any significant way.
I have one question regarding queries where the QTYPE is DS as those are handled specially according to the RFC and I didn't see any special handling of those here, could it happen that the NSEC3 response provided by a nameserver fails this validation if it's a response to a query for DS records?
listochkin
force-pushed
the
al-nsec3-validation
branch
8 times, most recently
from
c65a61f to
c87ad7d
Compare
marcus0x62
reviewed
Sep 3, 2024
listochkin
force-pushed
the
al-nsec3-validation
branch
from
c87ad7d to
b497e75
Compare
This was referenced Sep 6, 2024
djc
approved these changes
Sep 9, 2024
Member
djc
left a comment
djc
left a comment
There was a problem hiding this comment.
Approved with comments addressed.
listochkin
force-pushed
the
al-nsec3-validation
branch
4 times, most recently
from
7b26a2c to
cfcd2bd
Compare
Addresses three possible cases (referred as cases 2, 3, 4): Case 2: `query_name` exists but it doesn't have a record of requested type Case 3: `query_name` is serviced by wildcard that *has* a record of this type Case 4: `query_name` is serviced by wildcard that *does not have* a record of this type
listochkin
force-pushed
the
al-nsec3-validation
branch
from
cfcd2bd to
950ea61
Compare
1 task
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
5 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.
Adds support for NSEC3 records validation for the resolver