Implement IPv6 RFC 7217

[madpilot]

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.

UPDATE: algorithm is now enabled via ifconfig(8) flag "stableaddr"

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.

[madpilot]

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

[zarychtam_plan-b.pwste.edu.pl]

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]

libexec/rc/rc.conf

529 ↗

(On Diff #153205)

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 ↗

(On Diff #153205)

See L.529 in rc.conf.

sys/netinet6/in6.c

2621 ↗

(On Diff #153205)

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

sys/netinet6/in6.h

621

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

313

See L.529 in rc.conf.

sys/netinet6/nd6_rtr.c

1258

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

1633

/sill/still/

[zarychtam_plan-b.pwste.edu.pl]

Here's the reference to complete list of Reserved IPv6 Interface Identifiers:
https://www.iana.org/assignments/ipv6-interface-ids/ipv6-interface-ids.xhtml

[madpilot]

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]

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?

[madpilot]

In D49681#1133055, @madpilot wrote:

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.

[madpilot]

I addressed all reported feedback I hope.

For address validation I added a function to do it, and added a do/while loop to generate more addresses if the ones generated is not valid. Logic is the one I described in previous comment.

I also:

• Added description of the new flag in the ifconfig man page
• Improved the counter increment logic, to not increment counter once the max retries are exhausted and report the "manual intervention required" message to the user at that point.
• replaced bcopy/bcmp with memcpy/memcmp in all new code I'm creating

[madpilot]

sys/netinet6/in6_ifattach.c

372

I modified code to generate a random value in such a situation.

[emaste]

We should also enable by default - probably as a separate commit (which might not be MFCd if there are POLA concerns).

[madpilot]

In D49681#1133961, @emaste wrote:

We should also enable by default - probably as a separate commit (which might not be MFCd if there are POLA concerns).

The main POLA issue is that users would find their autoassigned IPs changed, I guess. Maybe that's a problem with stable, but will happen anyway when a user updates to 15.0.

I agree it would be nice to ship with this feature enabled.

Having added a flag to ifconfig maybe there is some issue with ABI? KBI should not have changed since I have touched no public kernel structures, but I'm not too expert about this, so please verify what I'm saying.

[glebius]

sys/netinet6/in6_ifattach.c

372

General style in the FreeBSD kernel is that functions either are bool that return true/false, or if you want to differentiate between different error codes, then return 0 or a positive error code from <errno.h>. This particular validate_ifid() should be bool.

P.S. There is style(9) guideline to use braces with return statements:

return (0);

381

This function should be void.

409

bool

450

With void generate_stable_ifid() loop gets simplier. I would even inline it here in full, since its used only here. Easier to read, don't memcpy() before validating. Smth like:

for(offset = 0; offset < UINT8_MAX); offset++) {
        SHA1_CTX ctxt;
        uint8_t digest[SHA1_RESULTLEN];

        SHA1Init(&ctxt);
        SHA1Update(&ctxt, prefix, prefix_len);
        SHA1Update(&ctxt, netiface, netiface_len);
        SHA1Update(&ctxt, (uint8_t *)&dad_failures, 8);
        SHA1Update(&ctxt, secret, secret_len);
        if (offset > 0)
                SHA1Update(&ctxt, &offset, 1);
        SHA1Final(digest, &ctxt);

        if (validate_ifid(digest)) { 
                 /* assumes sizeof(digest) > sizeof(ifid) */
                 memcpy(&in6->s6_addr[8], digest, 8);
                 return (true);
       }
}
return (false);

sys/netinet6/in6_proto.c

313

Should be SYSCTL_BOOL and variable bool, too. Most of the code is not like that, basically cause this type of sysctl didn't exist back then.

sys/netinet6/nd6_rtr.c

1604

uint64_t

[zarychtam_plan-b.pwste.edu.pl]

In D49681#1133961, @emaste wrote:

We should also enable by default - probably as a separate commit (which might not be MFCd if there are POLA concerns).

Hmm... but why, RFC 7217 does not suggest deprecating the EUI-64/MAC method. It only proposes an alternative method that is more privacy-friendly.

On the other hand, once RFC 8981 is implemented in the IPv6 stack, making it the default behavior may become a viable and reasonable option to consider.

sys/netinet6/nd6_nbr.c

1527

Unfortunately, we can't give up on anonymisation and generate the interface identifier in the old way (e.g., using EUI-64 based on the MAC address), since RFC 7217 has forbidden it, but perhaps disabling this anonymization should be suggested here? All cloned systems, where the host ID wasn't reset properly, could easily fall into this trap.

[madpilot]

sys/netinet6/nd6_nbr.c

1527

With this code the same message is used also when a conflict happens without this feature enabled.

I can change the logic to deliver a different message when RFC 7217 is enabled, something like:

%s: manual intervention required, consider disabling "stabledaddr" on the interface

[madpilot]

Updating the patch, addressing reported feedback.

I converted types as suggested and added a different error message when exhausting retries.

@zarychtam_plan-b.pwste.edu.pl Regarding the jails with same hostuuid issue this has been raised in the mailing lists at [1].

It has been suggested to factor in the jail name (for jailed systems only, obviously) in the sha1 hash. I'm not too happy about this idea though, since the RFC is quite detailed on what should be included and adding other random things is not exactly compliant.

I also have other reserves about this idea:

• while it would "fix" it for jails, virtual machines (bhyve and any other technology) would experience the same issue anyway
• It is not clear to me what should be really expected. Reading the RFC, if I clone a system and don't change anything else I'd really expect to get the same address. Assuming it should change instead, while handy in some use cases, would be not conforming to the RFC.

So, in the mailing lists I asked for opinions/consensus, if consensus is jail name should be used in some cases I can add it, but unless there is string consensus in that direction I'd avoid it.

It is important to note that once we have the hashing function in place changing any of it would be disruptive, vuiolate POLA etc.

[1] https://lists.freebsd.org/archives/freebsd-net/2025-April/006762.html

[madpilot]

I'm evaluating updating the patch to make the number of retries configurable via a sysctl (like net.inet6.ip6.stableaddr_maxretries?), defaulting to 3, obviously.

[zarychtam_plan-b.pwste.edu.pl]

I’ve just updated to the latest version of the patch, and now I no longer see a stableaddr on the interface - only an address with an EUI-64 suffix is present. The initial version of the patch was working fine for an entire week.

It’s possible there’s a conflict with another patch I’m testing[1]. However, both patches were working together as expected last week, with no apparent interference between them.

1. https://bz-attachments.freebsd.org/attachment.cgi?id=252676

[madpilot]

In D49681#1135624, @zarychtam_plan-b.pwste.edu.pl wrote:

I’ve just updated to the latest version of the patch, and now I no longer see a stableaddr on the interface - only an address with an EUI-64 suffix is present. The initial version of the patch was working fine for an entire week.

It’s possible there’s a conflict with another patch I’m testing[1]. However, both patches were working together as expected last week, with no apparent interference between them.

1. https://bz-attachments.freebsd.org/attachment.cgi?id=252676

The actual assignment of the IP is now controlled via ifconfig per interface, and the setting needs to be already in place when the interface is brought up. A good way to do this is via rc.conf, like this:

ifconfig_net0_ipv6="inet6 accept_rtadv stableaddr"

If you want it enabled globally you should set the default sysctl via tunable, in loader.conf. The sysctl value is used as a default when creating interfaces, and unluckily changing it value via sysctl.conf cannot work because when that is applied interfaces connected at boot have already been created with the default value.

[zarychtam_plan-b.pwste.edu.pl]

In D49681#1135632, @madpilot wrote:

In D49681#1135624, @zarychtam_plan-b.pwste.edu.pl wrote:

I’ve just updated to the latest version of the patch, and now I no longer see a stableaddr on the interface - only an address with an EUI-64 suffix is present. The initial version of the patch was working fine for an entire week.

It’s possible there’s a conflict with another patch I’m testing[1]. However, both patches were working together as expected last week, with no apparent interference between them.

1. https://bz-attachments.freebsd.org/attachment.cgi?id=252676

The actual assignment of the IP is now controlled via ifconfig per interface, and the setting needs to be already in place when the interface is brought up. A good way to do this is via rc.conf, like this:

ifconfig_net0_ipv6="inet6 accept_rtadv stableaddr"

If you want it enabled globally you should set the default sysctl via tunable, in loader.conf. The sysctl value is used as a default when creating interfaces, and unluckily changing it value via sysctl.conf cannot work because when that is applied interfaces connected at boot have already been created with the default value.

Thanks for the clue! That was indeed the missing piece. Interestingly, for WLAN interfaces, it wasn’t needed - stableaddr was still assigned just like before, with no difference between the initial patch and this one.

By the way, I’ve also cherry-picked it to stable/14 to broaden the testing scope a bit!

[madpilot]

Created an updated patch:

• Added a sysctl net.inet6.ip6.stableaddr_maxretries, defaulting to IP6_IDGEN_RETRIES ( = 3) to make the value configurable, if needed.
• Fixed the code writing an error message when retries are exhausted, the logic for displaying it was buggy.
• Reworded the net.inet6.ip6.use_stableaddr sysctl description to make it clear it is a default value applied to new interfaces, not a global switch.
• Remove the rc parts. After making the sysctl a default value, the rc script changes are applied too late so already connected interfaces tend to get a default value different from ones connected at a later time. This mechanism is not the right one to configure this feature.
• Improved the man page description of the feature in ifconfig(8) clearly stating it is better configured as a tunable via loader.conf. also added a descrioption for the new net.inet6.ip6.stableaddr_maxretries sysctl, clearly stating that values below 3 are discouraged.

I'd really like to get this in the main tree, so at this point I'm asking for review and formal approval to commit to the main tree, or indication of which steps are required to get there.

Obviously if firther changes are required I'll try to follo any indications.

[madpilot]

Updating patch to fix an error with an if statement content.

[madpilot]

There is an issue when stablesaddr and net.inet6.ip6.use_tempaddr are simultaneously enabled.

Working on a fix.

Reported by: @zarychtam_plan-b.pwste.edu.pl

[madpilot]

Fixed issues when ipv6_privacy is also enabled.

Temporary addresses work "in parallel" with the main autoassinged interface address.

To cater for this situation I need to check the temporary flag ion the IPv6 in various cases:

• When no DAD failures happen I need to reset the counter ONLY if the related IP is not temporary.
• When I get a DAD failure I also need to ignore DAD failures related to temporary addresses.
• When generating a new address on a new RA (usually after setting an IPv6 duplicate after a DAD failure), if a temporary address is already present on the interface do not generate an extra one unconditionally. To make this work I moved a section of code after the two hour rule, to make sure it is applied anyway to temporary addresses. Temporary address expiration and regeneration is handled in another part(nd6_timer()) of the code and should be unaffected.

[madpilot]

Fixed some whitespace issues I introduced by mistake.

[glebius]

sys/netinet6/in6_ifattach.h

43

IMHO, the prefixlen argument should be unsigned. The function is called with either hardcoded 64 or with pr->ndpr_len, which is u_char. So the source is always unsigned. The function uses prefixlen as argument to sha1_loop() which takes size_t, also unsigned. So the destination is also unsigned.

[madpilot]

sys/netinet6/in6_ifattach.h

43

Just to explain why I did what I did, the reason I used int is because prefixlen in nd6_rtr.c, function in6_ifadd() the prefixlen variable is declared int, so I followed suit.

But now that you point this out I tend to agree, so I think I should use u_char too.

[madpilot]

Use u_char for the prefixlen argument.

[glebius]

Disclaimer: I'm not an expert in the IPv6 protocol. Let's wait for more "accepts".

[zarychtam_plan-b.pwste.edu.pl]

I am not the committer, nor a real programmer, so my acceptance is not relevant, but I tested this patch extensively. Our IPv6 stack has not been getting many updates recently, so I am fully supporting this enhancement.

My only concern is how it will cope with systems having cloned HOSTUUIDs. Perhaps in this case, hwaddr as another entropy source will help, but we discussed it with the author, and duplicated HOSTUUIDs are problematic in many ways. Moreover, the proposed solution is 100% compliant with the RFC document.

When testing DAD I found and reported some incompatibilities, but I was testing this patch along with Fernando's patch (RFC 8981 Temporary Address Extensions for Stateless Address Autoconfiguration in IPv6, see PR 245103 on Bugzilla), thus the results might be biased.

Maybe stableaddr should be enabled by default for all the interfaces, as suggested by @emaste, so that when it goes into main, more people can test it and report bugs. Without making this enhancement widely used, it will take a long time to find all the bugs and corner cases.

[madpilot]

@glebius Thanks for the review and the endorsement!

@zarychtam_plan-b.pwste.edu.pl Thanks too, regarding the incompatibilities, the only issue I found was when ipv6 privacy was also simultaneously turned on, the last version of the patch addresses that. So it should now work as expected.

As I explained above factoring in the hwaddr for the device in the hash beats the point of the RFC, making the IP less "stable". The RFC also states that the source for this can be made configurable, but I think this is an improvement that can be added at a later time. I did choose the interface name because it looks like the one that generates the most stable addresses. At a later time a sysctl to allow configuring the system to use the hwaddr could be added.

[madpilot]

Do mark the use_stableaddr sysctl as tunable set.

[madpilot]

Fix minor issues:

• Use uint64_t in place of deprecated u_int64_t
• Fix wrding and grammar of a pair of comments