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

Support ASPA payload in RTR. #250

Merged
merged 6 commits into from
Mar 27, 2023
Merged

Support ASPA payload in RTR. #250

merged 6 commits into from
Mar 27, 2023

Conversation

partim
Copy link
Member

@partim partim commented Mar 23, 2023

This PR adds support for protocol version 2 of RTR. This introduces a new PDU type for ASPA payload.

This PR also fixes a bug in the RTR server code that included router key PDUs in the updates even if protocol version 0 was negotiated. (The client code accepts all PDU types in all versions.)

The PR also changes the methods and functions in the rtr::pdu module to use the type Asn instead of u32 for ASNs.

This is a breaking change.

@partim partim requested a review from DRiKE March 23, 2023 10:09
@AlexanderBand AlexanderBand changed the title Suport ASPA payload in RTR. Support ASPA payload in RTR. Mar 23, 2023
src/rtr/payload.rs Outdated Show resolved Hide resolved
src/rtr/pdu.rs Outdated Show resolved Hide resolved
src/rtr/pdu.rs Outdated Show resolved Hide resolved
@partim partim merged commit 1ed8ab1 into main Mar 27, 2023
@partim partim deleted the aspa-for-rtr branch March 27, 2023 11:22
///
/// # Panics
///
/// The function panics if the iterator produces more than 65,535 items.
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps not new to this PR but if we panic here, do we have any assurance that we would reject objects with excessive provider sets in the first place?

I think panic'ing may be dangerous here otherwise. ASPA object of death and all.

Copy link
Member Author

@partim partim Mar 28, 2023

Choose a reason for hiding this comment

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

I added this panic analogous to the router key where a large key will lead to a panic. But I realise that that is actually different – you are unlikely to have a large key.

There’s going to be a second PR soon since I realized that using the imutable and wire-format ProviderAsns type from the pdu module in the actual payload::Payload is rather quite inconvenient, so there’s going to be a new type for that atop a vec of ASNs with all the set goodness. This one will also include non-panic-y ways to deal with large provider sets.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 21, 2023
Pkgsrc changes:
 * Update cargo-depends.mk, update checksums.

Upstream changes:

Release v0.13.0 -- 'Should Have Started This in a Screen'

New

* Added support for ASPA. Processing needs to be enabled via the
  new option `enable-aspa` which is only available if the `aspa`
  feature is explicitly selected during compilation. This is due to
  the specification still changing. The implementation currently
  conforms with [draft-ietf-sidrops-aspa-profile-15].  ([#847],
  [#873], [#874], [#878])
* Added support for version 2 of the RTR protocol. This primarly
  means support for the ASPA payload type. ([#847])
* Sending SIGUSR2 to Routinator will re-open a log file if logging
  to a file is enabled. ([#859])
* The HTTP server provides a new endpoint `/json-delta/notify` that
  can be used to wait for updated data similar to the RTR Notify PDU.
  ([#863])
* Added support for filtering and adding router keys via local
  exception files. ([#865])
* The `vrps` command and the HTTP payload output endpoints now
  allow excluding specific payload types for output. ([#866])
* Added a new member `payload` to the output of the `/api/v1/status`
  endpoint that gives an overall summary of the produced payload.
  ([#867])
* Added new members `generated` and `generatedTime` to the JSON
  object produced by the `/json-delta` endpoint. ([#868])

Breaking Changes

* A new field `aspa` was added to the jsonext format. See the manual
  page for more information. ([#847])
* A number of ASPA-related fields have been added to all metrics
  and status formats. ([#847])
* Renamed functions and attributes that refer to standalone end
  entity certificates to refer to router certificates so they don't
  get confused with the end entity certificates included with signed
  objects. ([#854])
* Renamed the JSON member in the HTTP status API from `validEECerts`
  to `validRouterCerts`. The old name is still available but may be
  removed in the future. ([#854])
* The regular `json` output format now includes router key and ASPA
  output. Since both are disabled by default, the format will still
  be compatible by default. ([#866])
* The minimal required Rust version has been increased to 1.70.
  ([#847], [#853], [#869], [#879])

Bug Fixes

* Fixed a bug in the RTR server where it would include router key
  PDUs even if the negotiated protocol version was 0. (via [rpki-rs
  #250])
* Restored the ability to parse ASNs in JSON input to the `validity`
  command as string or number. ([#861])
* Update bcder to at least 0.7.3 to fix various decoding issues
  that could lead to a panic when processing invalid RPKI objects.
* Check the request URI when generating a path for storing a copy
  of a RRDP response with the `rrdp-keep-responses` option to avoid
  path traversal. ([#894]. Found by Haya Shulman, Donika Mirdita and
  Niklas Vogel. Assigned CVE-2023-39916.)


Other Changes

* The log message for missing manifest now include the URI of the
  CA certificate for which the manifest is missing. ([#864])
* Binary packages are now also built for Debian _bookworm._ ([#881])

[#847]: NLnetLabs/routinator#847
[#853]: NLnetLabs/routinator#853
[#854]: NLnetLabs/routinator#854
[#859]: NLnetLabs/routinator#859
[#861]: NLnetLabs/routinator#861
[#863]: NLnetLabs/routinator#863
[#864]: NLnetLabs/routinator#864
[#865]: NLnetLabs/routinator#865
[#866]: NLnetLabs/routinator#866
[#867]: NLnetLabs/routinator#867
[#868]: NLnetLabs/routinator#868
[#869]: NLnetLabs/routinator#869
[#873]: NLnetLabs/routinator#873
[#874]: NLnetLabs/routinator#874
[#878]: NLnetLabs/routinator#878
[#879]: NLnetLabs/routinator#879
[#881]: NLnetLabs/routinator#881
[#894]: NLnetLabs/routinator#894
[rpki-rs #250]: NLnetLabs/rpki-rs#250
[draft-ietf-sidrops-aspa-profile-15]: https://datatracker.ietf.org/doc/draft-ietf-sidrops-aspa-profile/15/
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.

3 participants