Skip to content

ospf: Add RFC 5613 support #77

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 30, 2025
Merged

ospf: Add RFC 5613 support #77

merged 5 commits into from
Jun 30, 2025

Conversation

nrybowski
Copy link
Member

@nrybowski nrybowski commented Jun 26, 2025 •
edited
Loading

This PR introduces Link-Local Signaling data block appended to OSPF Hello and DbDesc packets. The current code is shared between OSPFv2 and OSPFv3.

See RFC 5613.

TODO

  • compute LLS data block checksum upon encoding.
  • validate block checksum upon decoding.
  • authenticate the LLS data block if OSPF authentication is enabled.
  • support CA-TLV (OSPFv2 only).
  • configure LLS on interfaces through OSPF YANG model.
    - [ ] get LLS data from interfaces configuration. => not specified in RFC 5613.
  • fix tests when LLS data block is not present
  • Check LLS checksum when authentication is disabled
  • Ignore LLS data block if the checksum check fails.
  • Check Crypto Authentication TLV digest when authentication is enabled for OSPFv2.
  • add encoding and decoding tests
    • OSPFv3
    • OSPFv2
  • Bugs to fix
    • The LLS data block checksum encoding for OSPFv3 is not correct.
    • There is an off-by-one in the seqno received in OSPFv2 LLS CA-TLV.
  • Some code cleanup.

@nrybowski
Copy link
Member Author

nrybowski commented Jun 26, 2025 •
edited
Loading

Even if there are some todos left, I already mark this PR as ready for a first review since there are some open questions about LLS data block authentication.

The answer to my question is in RFC 7166 Section 2. The LLS data block is appended before the authentication trailer.

@nrybowski nrybowski marked this pull request as ready for review June 26, 2025 19:30
nrybowski
nrybowski commented Jun 26, 2025
View reviewed changes
nrybowski
nrybowski commented Jun 26, 2025
View reviewed changes
@nrybowski nrybowski marked this pull request as draft June 26, 2025 21:11
@nrybowski nrybowski force-pushed the rfc5613 branch 3 times, most recently from de21dc2 to 4bf4eca Compare June 27, 2025 16:26
nrybowski
nrybowski commented Jun 27, 2025
View reviewed changes
@nrybowski nrybowski force-pushed the rfc5613 branch 2 times, most recently from 8e2363e to 4038927 Compare June 27, 2025 22:05
rwestphal
rwestphal reviewed Jun 28, 2025
View reviewed changes
Copy link
Member

@rwestphal rwestphal left a comment

Choose a reason for hiding this comment

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

Wow, amazing work!

I used to think LLS was mostly used for proprietary extensions, but I see the IANA registry for LLS TLV types is quite big and references a number of RFCs. Is this prep work for something else you're planning to implement, or just general groundwork?

Overall this looks really good. I tested LLS combined with cryptographic authentication, for both OSPFv2 and v3, and I was pleased to see it's working (this is tricky business!). I've left a few review comments, mostly minor nits that should be easy to address.

Lastly, please do not forget to update the list of supported RFCs in the README :)

nrybowski
nrybowski commented Jun 30, 2025
View reviewed changes
Copy link
Member Author

@nrybowski nrybowski left a comment

Choose a reason for hiding this comment

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

I used to think LLS was mostly used for proprietary extensions, but I see the IANA registry for LLS TLV types is quite big and references a number of RFCs. Is this prep work for something else you're planning to implement, or just general groundwork?

You guessed it right :) My first plan was adding RFC 9339 but it depends on LLS data blocks. So, I started by adding them :)

Lastly, please do not forget to update the list of supported RFCs in the README :)

Sure, I will do that.

I will rewrite the git history to divide this series in 4 self-contained patches:

  1. Some preparation work by modifying current interfaces, i.e., using references in V::decode_auth_validate, extending PacketVersion with an options function, and moving the auth digest check in a dedicated function.
  2. The actual addition of RFC 5613 along with the repaired existing tests.
  3. Encode / decode tests for OSPFv2.
  4. Encode / decode tests for OSPFv3.

@nrybowski
ospf: Preparation work for RFC 5613.
9238c1a 
- Update V::decode_auth_validate() signature to use references rather
than taking ownership of PacketHdrAuth and AuthDecodeCtx. These
structures are required for OSPFv2 LLS data blocks authentication

- Extend PacketVersion interface with  function to retrieve Options
field from Hello and DbDesc packets.

- Move digest validation logic in a separate function as it will be
required for LLS CA-TLV authentication.

Signed-off-by: Nicolas Rybowski <[email protected]>
nrybowski added a commit to nrybowski/holo that referenced this pull request Jun 30, 2025
@nrybowski
ospf: Add RFC 5613 support (Link Local Signaling)
09c52bd 
This commit introduces LLS data block appended to OSPF Hello and DbDesc
packets. The current code is shared between OSPFv2 and OSPFv3.

- About authentication:
For OSPFv{2,3}, if the authentication is disabled, the LLS
data block checksum is verified.

For OSPFv2, when the authentication is enabled, the CA-TLV digest is
verified.

For OSPFv3, when the authentication is enabled, there is nothing to do
as the LLS data block is included in the authentication digest. This
behavior is documented in RFC 7166 Section 2.

- About enabling LLS data blocks through YANG:
LLS data blocks are enabled per interface. That enables generating LLS
data block for Hello and DbDesc messages orginated for a that specific
interface. If some LLS data is specified through OSPF configuration, the
L-bit is enabled in the packet options.

This feature also enables examining LLS data block for received messages
if the option is enabled and the L-bit flag is present in the message
options.

Currently, the operation detailed above are mainly placeholders for future
behavior since RFC 5613 does not mention how to configure the TLVs it
specifies.

- About the presence of EO TLV in LlsDbDescData:
RFC 4811 Section 2.1 specifies that EO TLV may be included in DbDesc
packets.

Signed-off-by: Nicolas Rybowski <[email protected]>
---
v0 -> v1: Fixed nits mentioned in Renato's review at
  holo-routing#77
nrybowski added a commit to nrybowski/holo that referenced this pull request Jun 30, 2025
@nrybowski
ospf: Add RFC 5613 support (Link Local Signaling)
f261ca8 
This commit introduces LLS data block appended to OSPF Hello and DbDesc
packets. The current code is shared between OSPFv2 and OSPFv3.

- About authentication:
For OSPFv{2,3}, if the authentication is disabled, the LLS
data block checksum is verified.

For OSPFv2, when the authentication is enabled, the CA-TLV digest is
verified.

For OSPFv3, when the authentication is enabled, there is nothing to do
as the LLS data block is included in the authentication digest. This
behavior is documented in RFC 7166 Section 2.

- About enabling LLS data blocks through YANG:
LLS data blocks are enabled per interface. That enables generating LLS
data block for Hello and DbDesc messages orginated for a that specific
interface. If some LLS data is specified through OSPF configuration, the
L-bit is enabled in the packet options.

This feature also enables examining LLS data block for received messages
if the option is enabled and the L-bit flag is present in the message
options.

Currently, the operation detailed above are mainly placeholders for future
behavior since RFC 5613 does not mention how to configure the TLVs it
specifies.

- About the presence of EO TLV in LlsDbDescData:
RFC 4811 Section 2.1 specifies that EO TLV may be included in DbDesc
packets.

Signed-off-by: Nicolas Rybowski <[email protected]>
---
v0 -> v1: Fixed nits mentioned in Renato's review at
  holo-routing#77
nrybowski added a commit to nrybowski/holo that referenced this pull request Jun 30, 2025
@nrybowski
ospf: Add RFC 5613 support (Link Local Signaling)
4750177 
This commit introduces LLS data block appended to OSPF Hello and DbDesc
packets. The current code is shared between OSPFv2 and OSPFv3.

- About authentication:
For OSPFv{2,3}, if the authentication is disabled, the LLS
data block checksum is verified.

For OSPFv2, when the authentication is enabled, the CA-TLV digest is
verified.

For OSPFv3, when the authentication is enabled, there is nothing to do
as the LLS data block is included in the authentication digest. This
behavior is documented in RFC 7166 Section 2.

- About enabling LLS data blocks through YANG:
LLS data blocks are enabled per interface. That enables generating LLS
data block for Hello and DbDesc messages orginated for a that specific
interface. If some LLS data is specified through OSPF configuration, the
L-bit is enabled in the packet options.

This feature also enables examining LLS data block for received messages
if the option is enabled and the L-bit flag is present in the message
options.

Currently, the operation detailed above are mainly placeholders for future
behavior since RFC 5613 does not mention how to configure the TLVs it
specifies.

- About the presence of EO TLV in LlsDbDescData:
RFC 4811 Section 2.1 specifies that EO TLV may be included in DbDesc
packets.

Signed-off-by: Nicolas Rybowski <[email protected]>
---
v0 -> v1: Fixed nits mentioned in Renato's review at
  holo-routing#77
nrybowski added 4 commits June 30, 2025 19:02
@nrybowski
ospf: Add RFC 5613 support (Link Local Signaling)
13d6816 
This commit introduces LLS data block appended to OSPF Hello and DbDesc
packets. The current code is shared between OSPFv2 and OSPFv3.

- About authentication:
For OSPFv{2,3}, if the authentication is disabled, the LLS
data block checksum is verified.

For OSPFv2, when the authentication is enabled, the CA-TLV digest is
verified.

For OSPFv3, when the authentication is enabled, there is nothing to do
as the LLS data block is included in the authentication digest. This
behavior is documented in RFC 7166 Section 2.

- About enabling LLS data blocks through YANG:
LLS data blocks are enabled per interface. That enables generating LLS
data block for Hello and DbDesc messages orginated for a that specific
interface. If some LLS data is specified through OSPF configuration, the
L-bit is enabled in the packet options.

This feature also enables examining LLS data block for received messages
if the option is enabled and the L-bit flag is present in the message
options.

Currently, the operation detailed above are mainly placeholders for future
behavior since RFC 5613 does not mention how to configure the TLVs it
specifies.

- About the presence of EO TLV in LlsDbDescData:
RFC 4811 Section 2.1 specifies that EO TLV may be included in DbDesc
packets.

Signed-off-by: Nicolas Rybowski <[email protected]>
---
v0 -> v1: Fixed nits mentioned in Renato's review at
  holo-routing#77
@nrybowski
ospf: Update tests to take into account empty LLS block
d228165 
Signed-off-by: Nicolas Rybowski <[email protected]>
@nrybowski
ospfv3: Add encode/decode tests for LLS data block
fd7ce8b 
Hello messages are tested with and without packet authentication.

DbDesc messages are only tested without packet authentication.

Signed-off-by: Nicolas Rybowski <[email protected]>
---
v0 -> v1: Add DbDesc tests as RFC 4811 Section 2.1 mention that LLS
  EO-TLV may be included in DbDesc packets.
@nrybowski
ospfv2: Add encode/decode tests for LLS data block
cb38d26 
Hello messages are tested with and without packet authentication.

DbDesc messages are only tested without packet authentication.

Signed-off-by: Nicolas Rybowski <[email protected]>
---
v0 -> v1: Add DbDesc tests as RFC 4811 Section 2.1 mention that LLS
  EO-TLV may be included in DbDesc packets.
@nrybowski
Copy link
Member Author

There we go, the PR is ready for review :)

@nrybowski nrybowski marked this pull request as ready for review June 30, 2025 17:11
@nrybowski
Copy link
Member Author

We could squash d228165 with 13d6816 as running the tests on 13d6816 will fail without d228165.

nrybowski added a commit to nrybowski/holo that referenced this pull request Jun 30, 2025
@nrybowski
ospf: Add support for RFC 9339
5dfa951 
This patch depends on RFC 5613, added with
holo-routing#77.

Signed-off-by: Nicolas Rybowski <[email protected]>
@nrybowski nrybowski mentioned this pull request Jun 30, 2025
Draft
10 tasks
rwestphal
rwestphal approved these changes Jun 30, 2025
View reviewed changes
Copy link
Member

@rwestphal rwestphal left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. Fantastic work! Merging...

You guessed it right :) My first plan was adding RFC 9339 but it depends on LLS data blocks. So, I started by adding them :)

Woot, that's great to hear!

@rwestphal rwestphal merged commit fde8b09 into holo-routing:master Jun 30, 2025
7 checks passed
nrybowski added a commit to nrybowski/holo that referenced this pull request Jul 1, 2025
@nrybowski
ospf: Add support for RFC 9339
ebcd533 
This patch adds Reverse Metric and Reverse TE Metric TLVs to LLS data
blocks.

This patch hence depends on RFC 5613, added with
holo-routing#77.

Signed-off-by: Nicolas Rybowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwestphal rwestphal rwestphal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nrybowski @rwestphal