Page MenuHomeFreeBSD

Implement IPV6 RFC 7217
Changes PlannedPublic

Authored by madpilot on Sun, Apr 6, 2:40 PM.
Tags
None
Referenced Files
F114109141: D49681.diff
Tue, Apr 8, 1:27 AM
F114057570: D49681.diff
Mon, Apr 7, 11:03 AM
Unknown Object (File)
Sun, Apr 6, 6:38 PM
Unknown Object (File)
Sun, Apr 6, 2:49 PM

Details

Reviewers
hrs
Summary

I created an implementation of RFC 7217, A Method for Generating Semantically Opaque Interface Identifiers with IPv6 Stateless Address Autoconfiguration (SLAAC)

I have added a sysctl (net.inet6.ip6.use_stableaddr, disabled by default) to allow switching on/off the functionality.

As far as I can see this should be conforming to the RFC, although not implementing some "should" conditions (random interval between retries mainly).

Further improvements that could be added at a later time include allowing configuring the per interface dad_failures counter starting value (maybe via sysctl, but the per interface requirement makes it tricky) and/or saving it to permanent storage (difficult, IMHO, also considering the presence of storage is not a given).

Some notes:

  • The algorithm follows the RFC recommendation to use SHA1, also uses host UUID as "secret". While the UUID is not really secret, it is randomly generated and constant between reboots. It's also not accessible or easily discoverable without user access to the machine, so I think it is a reasonable input for randomness.
  • I have hardcoded the three retries value, it could be exposed as a sysctl if deemed important enough.
  • To account for DAD failures, which need to be counter per interface, I added a dad_failures counter(9) element to in6_ifextra, which is reset to zero once an IP is successfully assigned.
  • When DAD finds a duplicate the IP is marked as DUPLICATED as usual, but I added logic to skip them when checking if an IP has already been assigned, so at the next chance a new one is assigned and tested again, up to the maximum retries value. This keeps the previous code flow. Otherwise a different flow could be used generating a new address in the nd6_dad_duplicated() method directly, but this would require much more refactoring.

Obviously all the names of sysctl/functions/variables can be improved if indication is given.

Test Plan

I have tested this in bhyve VMs, also generating some DAD duplicates and verified the logic works correctly.

I'm running this on my laptop with success.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 63348
Build 60232: arc lint + arc unit

Event Timeline

Forgot to mention: Many thanks to cognet who helped me with suggestions, especially about how to implement the dad_failures counter.

sys/netinet6/in6_ifattach.c
372

Perhaps it's worth adding a few more checks here to not fall into these traps too:

IANA Ethernet block: 0200:5EFF:FE00:0000-0200:5EFF:FE00:5212
Proxy Mobile IPv6: 0200:5EFF:FE00:5213
IANA Ethernet block: 0200:5EFF:FE00:5214-0200:5EFF:FEFF:FFFF

hrs requested changes to this revision.Mon, Apr 7, 9:52 AM
hrs added a subscriber: hrs.
hrs added inline comments.
libexec/rc/rc.conf
529

The stable-address function should be implemented as a per-interface knob (i.e., a flag for ifconfig), not a global sysctl. More specifically, the sysctl should be used as a global default for the per-interface flag to configure the stable address. You can find IPV6CTL_AUTO_LINKLOCAL and NF6_IFF_AUTO_LINKLOCAL as a similar instance.

libexec/rc/rc.d/netoptions
110

See L.529 in rc.conf.

sys/netinet6/in6.c
2621

Please put the failure counter under struct nd_ifinfo because DAD is logically a function of NDP.

sys/netinet6/in6.h
620

I prefer IPv6CTL_USESTABLEADDR for consistency with others.

sys/netinet6/in6_ifattach.c
351

Please use uintN_t even though most of the source code under net/netinet6 uses u_N_t types.

359

The hostuuid should be checked to see whether it is DEFAULT_HOSTUUID.

sys/netinet6/in6_proto.c
312

See L.529 in rc.conf.

sys/netinet6/nd6_rtr.c
1230

These 6 lines from here are duplications of code for the LLA-based address. Please consider using ib = &new; or something to avoid it.

1642

/sill/still/

This revision now requires changes to proceed.Mon, Apr 7, 9:52 AM

Thanks for the feedback!

I'll be addressing it in an upcoming new patch, as soon as I have it ready.

I'm adding two replies, one of which is a request for further direction if possible.

The comments I have not replied to, I simply agree and will address.

sys/netinet6/in6_ifattach.c
359

My uncertainty with this is, what should I do if it is the default value? Just refuse to provide an interface id? (that is return -1;) or something else?

372

This is a reasonable request.

I'll factor the check out in a separate function though, so it can be easily reused, if needed.

madpilot marked 2 inline comments as not done.Mon, Apr 7, 2:00 PM

I just realized that when checking if the generated ID is valid I simply return failure, the stack will just keep generating the same invalid address over and over, without stopping and never getting a chance to generate a valid one.

So, my solution, would be to wrap the generating function in a do while cycle, exiting on a valid ID adding an offset to the the counter, so that at each iteration (if the previous was invalid), a valid one would be generated.

This would be deterministic and not modify the existing algorithm derived from the RFC description.

Would this strategy be acceptable?

I just realized that when checking if the generated ID is valid I simply return failure, the stack will just keep generating the same invalid address over and over, without stopping and never getting a chance to generate a valid one.

So, my solution, would be to wrap the generating function in a do while cycle, exiting on a valid ID adding an offset to the the counter, so that at each iteration (if the previous was invalid), a valid one would be generated.

Thinking of it, offesetting would not really work that well, unless the offset is preserved. Another option would be to concatenate a further counter to the hash function when and only when invalid values are generated, so a different one is output.

I think I'll go in this direction, adding a comment explaining the logic, obviously.