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.

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

Remove incorrect compose_len from ProtoRrsig #313

Merged
merged 1 commit into from
May 10, 2024

Conversation

achow101
Copy link
Contributor

@achow101 achow101 commented May 5, 2024

ProtoRrsig is used to produce the data that is signed. According to https://datatracker.ietf.org/doc/html/rfc4034#autoid-23, this serialization begins with the first rdata field which is type_covered, however ProtoRrsig is inserting a length at the beginning. As such, signatures produced by SortedRecords::sign() are invalid. Fix this by removing that length from the serialization.

@partim
Copy link
Member

partim commented May 6, 2024

Thank you for the PR! Looks like we lost a fix at some point as I’m sure this was all working.

Before we can merge the PR, could you perchance merge main into your branch just so that the CI is happy?

@achow101
Copy link
Contributor Author

achow101 commented May 6, 2024

Rebased

@partim partim merged commit 053dbc1 into NLnetLabs:main May 10, 2024
12 checks passed
partim added a commit that referenced this pull request Jun 3, 2024
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])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants