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

gnrc_ipv6_nib/SLAAC: rfc7217 stable privacy addresses #20370

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

xnumad
Copy link
Contributor

@xnumad xnumad commented Feb 11, 2024

Contribution description

Implementation of RFC7217 ("stable privacy") means that for addresses generated by SLAAC, their IID (interface identifier) is

  • semantically opaque, i.e. cannot derive hwaddr from it
  • random but with such parameters that it is stable within a subnet (i.e. for the same prefix on the same interface)
  • derived from a randomly generated key (secret_key) that shouldn't be shared across devices.
    RIOT-specific: The implementation in this PR compiles the secret_key into the elffile, therefore the same elffile shouldn't be flashed onto multiple devices - in order to fulfill the RFC requirements.

Notes about implementation:

  • uses tail recursion (i.e. optimizable by compiler, or manually replacable by a loop) (affects stack size)

RFC compatibility: Requirement levels: (implemented: MUST, SHOULD. not implemented: MAY, OPTIONAL.)

--

Adaptability

Supports any link layer that has a hardware address:
https://github.com/xnumad/RIOT/blob/88363f3cde7175691fe27399d73a54e08934e6c7/sys/net/gnrc/network_layer/ipv6/nib/_nib-slaac.c#L168-L169

The usage of IIDs which do not match the link layer address causes LOWPAN_IPHC to not be able to statelessly compress the IP address anymore. An optimization to enable compression again could be to add compression contexts (feature branch for opportunistic compression contexts: https://github.com/xnumad/RIOT/tree/feature%2Fopportunistic-compression-contexts).

6LoWPAN specifics

They do not affect this PRs current functionality but are worth noting.

For link-local addresses on a 6LN iface, this PR does not use the IDGEN (interface identifier generation) mechanism described by rfc7217.

  • This is because of incompatibility with 6LoWPAN-ND (RFC6775) (and RFC8505, which would allow it, is not implemented).
  • → Not fulfilling RFC7217 requirement "MUST be employed for [...] link-local"

Testing procedure

Add CFLAGS += -DCONFIG_GNRC_IPV6_STABLE_PRIVACY=1 at the appropriate position in the Makefile of examples/gnrc_networking. Tested on BOARD=nrf52840dk

Output of ifconfig command shows that SLAAC addresses have random differing IIDs (except for link-local address on a 6LoWPAN iface, see above). The IID is stable within a subnet, i.e. it also persists across reboots.

Issues/PRs references

Commits marked with 🍒 (cherry-picked) are largely derived from the RFC8981 implementation at #20369

Copy link
Member

@chrysn chrysn left a comment

Choose a reason for hiding this comment

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

I don't have the time to do a full review at the moment, but from a quick high-level reading this looks like it's well done, and checks a lot of boxes (including careful treatment of Kconfig handling).

A few notes around here and there are really just "things that struck me" with no particular severity. Given there are a lot of fixup commits from the onset, please consider squashing (possibly after handling any of my comments), as that'll make the sequence of commits easier to understand.

sys/net/gnrc/Makefile.dep Outdated Show resolved Hide resolved
# Set another macro that is needed for this option.

##prepare value
stable_privacy_secret_key != python3 -c "import secrets; print(','.join([f'0x{byte:02x}' for byte in secrets.token_bytes(16)]))"
Copy link
Member

Choose a reason for hiding this comment

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

We'll have to think a bit about device individualization in RIOT as a whole (esp. for questions like "is this maybe better created in a HWRNG assisted way at first start?"), but until then, passing it into the build seems fine.

As someone who repeatedly flashes devices and then starts network interactions, it may be convenient to store this in the build directory and recreate only if absent, as that will make addresses stable across rebuilds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely agree, letting the board generate the secret_key by itself (and permanently store it - as per the RFC) would be more convenient, as a future improvement!

sys/net/gnrc/network_layer/ipv6/nib/_nib-slaac.c Outdated Show resolved Hide resolved
sys/net/gnrc/network_layer/ipv6/nib/_nib-slaac.c Outdated Show resolved Hide resolved
xnumad added a commit to xnumad/RIOT that referenced this pull request Feb 22, 2024
Requires Python >=3.6, which is already EOL, so not noteworthy
RIOT-OS#20370 (comment)
xnumad added a commit to xnumad/RIOT that referenced this pull request Feb 22, 2024
@xnumad
Copy link
Contributor Author

xnumad commented Feb 22, 2024

Squashed

xnumad added a commit to xnumad/RIOT that referenced this pull request Feb 22, 2024
Requires Python >=3.6, which is already EOL, so not noteworthy
RIOT-OS#20370 (comment)
xnumad added a commit to xnumad/RIOT that referenced this pull request Feb 22, 2024
@xnumad
Copy link
Contributor Author

xnumad commented Feb 22, 2024

Squashed a typo

Requires Python >=3.6, which is already EOL, so not noteworthy
RIOT-OS#20370 (comment)
Move pre-condition check to the corresponding method requiring this condition
Wrap in `if(is_rfc7217)`, use normal IDGEN if false
Remove fallback to netif.pid because presumably doesn't fulfill the RFC stability requirements of "constant across system bootstrap sequences and other network events (e.g., bringing another interface up or down)"
@xnumad
Copy link
Contributor Author

xnumad commented Sep 18, 2024

Rebased.

_auto_configure_addr -> auto_configure_addr
_auto_configure_addr_default -> _auto_configure_addr
Not only for the caller `_auto_configure_addr`. Other caller is NETOPT_IPV6_IID. Move check to callee.
Probably low severity because address conflicts are unlikely anyway.
`-Werror=maybe-uninitialized`
Example for a caller assuming idempotency: uhcp (-> `gnrc_netif_ipv6_add_prefix` -> `NETOPT_IPV6_IID`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants