-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
Don't skip children of apex in RecordsIter if apex is not there #314
Don't skip children of apex in RecordsIter if apex is not there #314
Conversation
Thank you for the PR! The intention of the current code was to only sign a complete zone. In particular, the code relies on that to create a correct NSEC chain. In this case, you always have to have the apex itself because this is where the DNSKEY records go. It looks like at least I think, however, it should be a separate check before the look rather than testing in every iteration. |
I chose to put it in every iteration because theoretically there could be records that would be before the apex, and then records that are children of the apex, but not the apex itself. However I'm not sure if that's actually something reasonable to consider. |
Ah yes, you are right – I forgot about that case. But then, shouldn’t just checking for |
5c59696
to
07db82c
Compare
Yes. But I don't fully understand what the point of
I moved the class check to be done first at the top of the loop so it would just skip any records that don't have a class matching the apex. |
Apologies for the delay! This looks good now – I will merge it. |
New * Allow AllRecordData’s parsing impls to accept an unsized [u8] as the source octets. ([#310] by [@xofyarg]) * Made `sign::records::FamilyName` public. ([#312] by [@achow101]) * Added an impl of `FromStr` for `Question`. ([#317]) Bug fixes * Accept an empty record type bitmap when scanning NSEC/NSEC3 data. ([#310] by [@xofyarg]) * Fix serialization of ProtoRrsig to conform with RFC 4034. ([#313 by [@achow101]) * Add `?Sized` bounds to `Message::is_answer` and `ParsedRecord::to_record`. ([#318] by [@xofyarg], [#325] by [@hunts]) * Bring back `MessageBuilder::as_target`. ([#318] by [@xofyarg]) * Bring back `impl FreezeBuilder for StaticCompressor`. ([#318] by [@xofyarg]) * `sign::records::RecordsIter::skip_before` now stops at the first name in zone even if the apex itself doesn’t appear. ([#314] by [@achow101]) * Fix a counting error in `SliceLabelsIter::next` that broke compression via `StaticCompressor`. ([#321] by [@hunts]) Unstable features * New unstable feature `unstable-stelline` for the Stelline testing framework as a “normal” module of _domain._ ([#315]) * Renamed the domain name types in `zonetree` from `Dname` to `Name`. ([#308]) Other changes * The minimum Rust version is now 1.78. ([#320])
If none of the records in the
RecordsIter
have the apex as their name, but they are in the apex's zone, don't skip those children. Otherwise rrsigs will not be created for any queries for just subdomains.