IPv6: hardcoded address `fd00::2` violates RFC 4193
AkihiroSuda opened this issue · 16 comments
https://github.com/rootless-containers/slirp4netns/blob/v1.1.9/slirp4netns.1.md
Default configuration:
* MTU: 1500
* CIDR: 10.0.2.0/24
* Gateway/Host: 10.0.2.2 (network address + 2)
* DNS: 10.0.2.3 (network address + 3)
* IPv6 CIDR: fd00::/64
* IPv6 Gateway/Host: fd00::2
* IPv6 DNS: fd00::3
According to RFC 4193, the addresses in fd00::/8
seems to need to be random
https://tools.ietf.org/html/rfc4193
https://networkengineering.stackexchange.com/questions/30128/ipv6-ula-vs-link-local
(slirp4netns --enable-ipv6
is still experimental and we can have breaking changes)
@giuseppe Could you take a look? What should we do with this?
is it really a risk for slirp4netns though? IPv6 is not routed so there is not really risk of collisions, or do you think we should just pick a random IP to honor the RFC? I am fine if we go this way (and if any problem arises users of slirp4netns can force the IP addr)
I think honoring RFC is not necessarily, but probably we will eventually need to support specifying random addr.
Not an urgent task.
I think it should pick a random one. For users who want a fixed subnet, a --cidr6
option should be added.
If I can find some time, sure, but probably not in the next few weeks.
@AkihiroSuda do you want just commit pfandl@8cc216f ?
Thanks @pfandl for working on this!
- For compatibility, can we keep the current wrong behavior by default, and opt-in to the correct behavior with a new flag like
--ipv6-random
? I guess this will help Podman to support older version of slirp4netns and the new version simultaneously (cc @rhatdan) - Could you update the man page?
- Could you use
strncpy
family rather thanstrcpy
family? - Would it be possible to drop dependency on libcrypt?
Thanks @pfandl for working on this!
* For compatibility, can we keep the current wrong behavior by default, and opt-in to the correct behavior with a new flag like `--ipv6-random`? I guess this will help Podman to support older version of slirp4netns and the new version simultaneously (cc @rhatdan)
yes
* Could you update the man page?
yes
* Could you use `strncpy` family rather than `strcpy` family?
yes
* Would it be possible to drop dependency on libcrypt?
Not too sure about that, I tried to mimic the sample code from the RFC
It is not clear to me if this is just a recommendation (at step 4 to compute a SHA1 hash) rather than a requirement. If it is a requirement and somebody knows how to compute the hash without libcrypt i guess we can drop the library, yes.
Can we take the necessary bits from here? https://code.woboq.org/gcc/libiberty/sha1.c.html
Also @AkihiroSuda as stated here #253 (comment) I took the mac of the lo
device, which is zero, do you have any idea what we should take instead? See step 2 from the RFC .
@AkihiroSuda can we bump libslirp version? I am already using the hostx_fwd functions and we need a fix there for IPv6 forwarding anyway and I think it would be less work for me to bring all in at once?
Yes.
Using #if directive is more preferable, but not necessary if it too much complicates the code.
Line 379 in 631f361
SHA1
Dependency on libcrypt is fine, as it should be available on all distros.
I took the mac of the lo device, which is zero, do you have any idea what we should take instead? See step 2 from the RFC .
Why not take the MAC of tap0
?
If the MAC cannot be take, we can just use /dev/urandom
.
I think #if will work, thanks for the info. I guess it should be ready coming weekend.
@AkihiroSuda with man page, do you mean README.md?
No, I mean this
https://github.com/rootless-containers/slirp4netns/blob/master/slirp4netns.1
Generated from https://github.com/rootless-containers/slirp4netns/blob/master/slirp4netns.1.md with make generate-man
I'm late to the party. I prefer slirp4netns to support the RFC.
Thanks for your work, @pfandl. Note that the sample code for generating random Global IDs is just a sample. The minimal requirement is for the number to be pseudo-random. I believe getrandom(2)
should answer this need.
Here is the current PR for this issue: #276 (It seems that GitHub did not link them when the new PR was created)