Details

Reviewers

hrs
glebius
zarychtam_plan-b.pwste.edu.pl
cognet
lytboris_gmail.com

Commits

rG31ec8b6407fd: sys/netinet6: Implement RFC 7217

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.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 63613
Build 60497: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

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.

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.

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.

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!

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.

Harbormaster completed remote builds in B63487: Diff 153616.Apr 13 2025, 7:15 PM

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

Harbormaster completed remote builds in B63494: Diff 153629.Apr 14 2025, 8:48 AM

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

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.

Harbormaster completed remote builds in B63507: Diff 153647.Apr 14 2025, 4:29 PM

Fixed some whitespace issues I introduced by mistake.

Harbormaster completed remote builds in B63509: Diff 153649.Apr 14 2025, 4:38 PM

glebius added inline comments.Apr 14 2025, 6:47 PM

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 added inline comments.Apr 14 2025, 7:33 PM

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.

Use u_char for the prefixlen argument.

Harbormaster completed remote builds in B63514: Diff 153659.Apr 14 2025, 8:50 PM

madpilot marked an inline comment as done.Apr 14 2025, 8:51 PM

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

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.

@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.

Do mark the use_stableaddr sysctl as tunable set.

Harbormaster completed remote builds in B63531: Diff 153720.Apr 16 2025, 11:08 AM

madpilot retitled this revision from Implement IPV6 RFC 7217 to Implement IPv6 RFC 7217.Apr 20 2025, 6:07 PM

Fix minor issues:

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

Harbormaster completed remote builds in B63613: Diff 154022.Apr 21 2025, 1:10 PM

cognet accepted this revision.Apr 26 2025, 2:12 PM

lwhsu added a subscriber: lwhsu.May 7 2025, 11:26 PM

There is also D50108 implementing RFC 8981, I think these 2 patches can be reviewed together, at least the part of getting an interface identifier, as suggested in RFC 8981 3.3.2.

And sorry for the possible bikeshed and I'm not an IPv6 expert -- I am a bit confused by the naming "stable" here. To my understanding, RFC 7217 suggests a way to generate stable address which provides better privacy, than the original EUI-64, which could also be considered "stable" with less privacy.
So, perhaps it can be something like "stableopaque", and only needed by <15 branches; for >=15 we can just use it by default without any option, as suggested in RFC 8064.
Or, make "stable" here mean that if in6_get_stableifid() fails in get_ifid(), it aborts trying other methods. Or still trying EUI-64 and other methods can generate "stable" address (but less opaque), but not get_rand_ifid().

BTW, don't forget to bump .Dd field of the manual page. :-)

sys/netinet6/nd6_rtr.c
1692

In D49681#1146197, @lwhsu wrote:

There is also D50108 implementing RFC 8981, I think these 2 patches can be reviewed together, at least the part of getting an interface identifier, as suggested in RFC 8981 3.3.2.

While the algorithms are similar they have clear differences. You suggest the two reviews should be merged?

If strictly necessary I can try that but would require a lot of extra work, I think it is better to keep the work separated, the implementation can be adjusted adding functionality progressively.

Please let me know what is required to get this moving and I'll try to do it, but I need a clear plan before performing major changes.

And sorry for the possible bikeshed and I'm not an IPv6 expert -- I am a bit confused by the naming "stable" here. To my understanding, RFC 7217 suggests a way to generate stable address which provides better privacy, than the original EUI-64, which could also be considered "stable" with less privacy.

the "stable" wording I took from other OSes.

For example linux systemd-networkd calls it "prefixstable" (from systemd.network(5) man page), most linux GUI tools also name it variation of "stable". The idea is to have a stable address that does not expose the MAC address, yes.

Anyway I can change the wording, but trying to keep it short, "opaque", "privstable"?

So, perhaps it can be something like "stableopaque", and only needed by <15 branches; for >=15 we can just use it by default without any option, as suggested in RFC 8064.

"stableopaque" is slightly long but it's fine by me.

I personally think we should not pull any trigger too early and for now I'd keep this opt-in. I'd anyway keep an opt-out option even on head. If I understand correctly the RFC the opt-out is a requirement.

Or, make "stable" here mean that if in6_get_stableifid() fails in get_ifid(), it aborts trying other methods. Or still trying EUI-64 and other methods can generate "stable" address (but less opaque), but not get_rand_ifid().

RFC7217 clearly states that if the algorith described there fails no other tries should be nade with any variation of the algorithm of other algorithms (last paragraph of section 6), so, if we want to be conformant we can't do such a thing.

BTW, don't forget to bump .Dd field of the manual page. :-)

Will fix that!

In D49681#1146197, @lwhsu wrote:

There is also D50108 implementing RFC 8981, I think these 2 patches can be reviewed together, at least the part of getting an interface identifier, as suggested in RFC 8981 3.3.2.

Thank you for mentioning review D50108. The part you are referring to was implemented by F. Gont using the method described in 3.3.1 in this RFC, so there is no overlapping or redundant code here.
The best way will be to take one step at a time. The patch from review D50108 doesn't collide with this one. Moreover, I am now testing both of them simultaneously on a couple of machines running stable/14 and main.

To encourage the reviewers to accept this @madpilot's enhancement, please follow this old thread on the RIPE ipv6-wg mailing list.

https://mailman.ripe.net/archives/list/ipv6-wg@ripe.net/thread/IV46DM2TD4XUTMJITSF3T43OUC3V3RND/

p.mousavizadeh_protonmail.com added a subscriber: p.mousavizadeh_protonmail.com.May 15 2025, 6:36 PM

zarychtam_plan-b.pwste.edu.pl mentioned this in D50114: nd6: allow address autoconf to be disabled.May 18 2025, 2:06 PM

Updating patch with improved functionality.

I added a sysctl to configure which source is used for the Net_Iface parameter of the algorithm

Allowed values are:

0, Interace name
1, Interface ID (not very stable)
2, MAC address (if one can be extracted for the interface, not everything is ethernet like)

The default is 0, Interface name

These options are inspired by the RFC Appendix A, but I did prefer Interface name as potentiually more stable than the interface ID in our stack.

Anyway I have no problem switching the default if that's required to get consensus.

Harbormaster completed remote builds in B65130: Diff 157753.Jun 30 2025, 4:24 PM

zarychtam_plan-b.pwste.edu.pl added a reviewer: lytboris_gmail.com.Jul 1 2025, 6:46 PM

Thanks for these patches, a long-waited change as for me. There are some minor things to fix and we're good to go.

Although I marked it as a side note, it would be really great to have this differential update our IPv6 code the way to ease future RFC 8981 adoption.

sbin/ifconfig/ifconfig.8
1021	sysctl IPV6CTL_STABLEADDR_NETIFSRC is worth mentioning here as well
sys/netinet6/in6_ifattach.c
439	s/bbe/be/
440	s/has/hash/
441	s/oover and over./over and over again./
455	There should be NO `if` here to comply with RFC 7217 section 5: A pseudorandom function (PRF) that MUST NOT be computable from the outside (without knowledge of the secret key). If we keep this `if` one could use a guess that to compute digest value for offset > 0 with a knowledge of digest value without it. A side node: please take a look on RFC 8981 section 3.3.2 and validate that `in6_get_stableifid()` could be re-used for one that will update our RFC 4941 implementation.
sys/netinet6/nd6_rtr.c
1696	Do we really need this flag? We check it once and set it even fail to form a stable address. If offset loop in `in6_get_stableifid()` ultimately fails (yeah, an unlikely but possible scenario) we skip configuring temporary address. Is it desirable?

This revision now requires changes to proceed.Jul 6 2025, 12:54 PM

Will upload a patch addressing most of these issue soon

sbin/ifconfig/ifconfig.8
1021	Strange, I was sure I did add documentation for that, I must have lost it rebasing the code at some point. I'll add a description of it with the updated patch.
sys/netinet6/in6_ifattach.c
455	While I have no problem removing the conditional, I think it would involve the same level of difficulty to guess a digest where a 0 becomes a 1 and then a 2 with all the rest of the content unmodified. Regarding RFC 8981 section 3.3.2 I see the differences are they require to use the MAC address, which is already possible here, and add a timetamp, suggesting unix time is a viable option. That would be trivial to add, but out of scope for this patch. This function can be easily adapted, when needed, by adding a parameter to signal which RFC is being targeted by the generation and adding the required fields in that case. Could you please elaborate more clearly what you're expecting here? I'd rather avoid adding parameters and conditionals that will be only used in the future.
sys/netinet6/nd6_rtr.c
1696	Unluckily yes, due to how the code is laid out a new temporary address would be created each time a stableaddress is tried. For how I understand it the logic at 1774 and later would add a temporary address when a main address is created. If no main address is created do we want to create a temporary address? The previous code clearly subordinated temporary addresses to main ones.

madpilot added inline comments.Jul 7 2025, 5:57 PM

sys/netinet6/in6_ifattach.c
455	While I have no problem removing the conditional, I think it would involve the same level of difficulty to guess a digest where a 0 becomes a 1 and then a 2 with all the rest of the content unmodified. Commenting myself, now that I look at the code, I do understand what you mean. Not completely trivial. Anyway no problem removing the conditional.

I addressed your comments.

I also decided to change the algorithm to use sha256, as suggested
in RFC 8981

This should make it easier to add support later without breaking
the algorithm.

Obviously the addresses generated by this new version of the code
will be different from the ones generated by previuous versions.

Harbormaster completed remote builds in B65410: Diff 158458.Jul 14 2025, 10:19 AM

madpilot marked 3 inline comments as done.Jul 14 2025, 10:20 AM

LGTM

cperciva added a subscriber: cperciva.Aug 7 2025, 1:08 AM

cperciva added inline comments.

sys/netinet6/in6_ifattach.c
450	Can you switch this to use HMAC-SHA256 instead of SHA256?

I will try to provide an update as fast as I can, but being away from home it could take just a little more than a week to have something ready.

sys/netinet6/in6_ifattach.c
450	Thanks for the review/suggestion. I'll be working on this. I was hoping to find an in kernel implementation to leverage but I see nothing generic (correct me if I'm wrong). I'll be rolling out some inline code, based on the HMAC description and taking inspiration from `sys/geom/eli/g_eli_hmac.c`, using hostuuid as the base for HMAC key. I'm not experienced in writing crypto code so, please, correct me if anything I said above is wrong, and then review thoroughly the code I will submit.

cperciva added inline comments.Aug 7 2025, 5:09 PM

sys/netinet6/in6_ifattach.c
450	Ugh. My crypto libraries have HMAC code in the same file as SHA256, but those bits didn't get imported into FreeBSD. It looks like Kyle has touched those bits recently; maybe I can trick him into pulling in those pieces too... And yes, happy to review the crypto parts here. I have no idea what the rest of this commit is about, but crypto I can help with.

I've added HMAC, it was easier than I thought, actually. I based my work on the Wikipedia description and some code borrowed from sys/geom/eli/g_eli_hmac.c. I did not bother with clearing the keys used since the source material is not really high security secrets in this case. I noticed SHA256_Final already zeroes the context value, so I decided to reuse it.

I also preferred using a define for the IPAD and OPAD constants, even though I'm using those only once. Magic numbers look bad inline :)

I used hostuuid as secret for HMAC, since I'm already using it as the secret parts of the hashed value.

I also did this inline, since we do not have a global in kernel HMAC implementation. If one materialized I'll be happy to update the code to use that.

Also, changed the IPv6CTL_USESTABLEADDR define as requested earlier by hrs, which, for some reason I forgot to do.

I've tested this in jails and am using it on my machines with no issues.

Obviously, having modified the derivation algorithm, the generated IPv6 addresses will be different from the previous patch.

Thanks again for reviewing this and I hope this can be pushed soon!

Harbormaster completed remote builds in B66282: Diff 160425.Aug 16 2025, 8:36 AM

madpilot marked 2 inline comments as done.Aug 16 2025, 8:37 AM

Looks like commit 9e792f7ef7298080c058fbc2d36a4e60e596dae9 causes a conflict in this patch. I'm adjusting my changes, will post a new patch soon.

Fix patch after commit 9e792f7ef7298080c058fbc2d36a4e60e596dae9

Harbormaster completed remote builds in B67016: Diff 162005.Sun, Sep 14, 7:57 PM

Added kp and Reid Linnemann to subscribers since the patch touches on their recent commit, so they can make sure I'm not braking it.

Thanks in advance!

Guido, do you need an explicit approval to push your change to src? IMHO, you got "reviewed by" from couple src committers, thus you are good to go with a push. If you seek for more reviews before going ahead, that's also fine.

In D49681#1201519, @glebius wrote:

Guido, do you need an explicit approval to push your change to src? IMHO, you got "reviewed by" from couple src committers, thus you are good to go with a push. If you seek for more reviews before going ahead, that's also fine.

+1
I’ve been testing it 24/7, now on 16.0-CURRENT, and the latest patch still works flawlessly. I see a lot of potential for IPv6 stack enhancements once it’s committed. It won’t break anything - though ifconfig and the kernel do need to be updated in sync. That’s more of an inconvenience than a real showstopper or breakage.

This revision was not accepted when it landed; it landed in state Needs Review.Sat, Sep 20, 12:32 PM

Closed by commit rG31ec8b6407fd: sys/netinet6: Implement RFC 7217 (authored by madpilot). · Explain Why

This revision was automatically updated to reflect the committed changes.

madpilot added a commit: rG31ec8b6407fd: sys/netinet6: Implement RFC 7217.

Implement IPv6 RFC 7217
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 154022

sbin/ifconfig/af_inet6.c

sbin/ifconfig/af_nd6.c

sbin/ifconfig/ifconfig.8

sys/netinet6/in6.h

sys/netinet6/in6_ifattach.h

sys/netinet6/in6_ifattach.c

sys/netinet6/in6_proto.c

sys/netinet6/ip6_input.c

sys/netinet6/ip6_var.h

sys/netinet6/nd6.h

sys/netinet6/nd6.c

sys/netinet6/nd6_nbr.c

sys/netinet6/nd6_rtr.c

usr.sbin/ndp/ndp.c

Implement IPv6 RFC 7217ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 154022

sbin/ifconfig/af_inet6.c

sbin/ifconfig/af_nd6.c

sbin/ifconfig/ifconfig.8

sys/netinet6/in6.h

sys/netinet6/in6_ifattach.h

sys/netinet6/in6_ifattach.c

sys/netinet6/in6_proto.c

sys/netinet6/ip6_input.c

sys/netinet6/ip6_var.h

sys/netinet6/nd6.h

sys/netinet6/nd6.c

sys/netinet6/nd6_nbr.c

sys/netinet6/nd6_rtr.c

usr.sbin/ndp/ndp.c

Implement IPv6 RFC 7217
ClosedPublic
Actions

Revision Contents
Changeset List