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
First part of ZONEMD support #11100
First part of ZONEMD support #11100
Conversation
1. Skip RSIGS for apex ZONEMD 2. Process records individually for RRSIG rrsets , since they might have different original TTLs.
…me malformed ones.
@check-spelling-bot ReportUnrecognized words, please review:
To accept these unrecognized words as correct, run the following commands... in a clone of the git@github.com:omoerbeek/pdns.git repository
If the flagged items do not appear to be textIf items relate to a ...
|
pdns/sha.hh
Outdated
md = EVP_sha512(); | ||
break; | ||
default: | ||
throw std::runtime_error("SHADigest: unsupported size"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use std::out_of_range
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It reports errors that are consequence of attempt to access elements out of defined range.
So close, but not really it. std::invalid_argument
fits better imo.
Co-authored-by: Remi Gacogne <github@coredump.fr>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't dive too much into zonemdVerify()
as I don't know ZONEMD and found it a bit hard to read (lots of abbreviations that are lowercased/uppercased), but LGTM as it's tested and the API is clear and speaks for itself
Good point, I'll rename some vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work. I posted a few nits, but nothing major.
{ | ||
bool operator()(const RRSetKey_t& a, const RRSetKey_t& b) const | ||
{ | ||
// FIXME surely we can be smarter here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is my FIXME, I'll have a go at it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, can't use std::tie
because of the custom comparison function. I'll leave it for now..
Short description
Based on POC from @Habbie, with many changes.
Things to be done I can think of (details to be decided):
For the Recursor:
For the Authoritative Server:
Both should be handled in separate PRs imo.
Checklist
I have: