security/acme-client: fix IPv6 support#5281
Conversation
|
|
||
| // Check if IPv6 support is enabled | ||
| if (isset($configObj->system->ipv6allow) && ($configObj->system->ipv6allow == '1')) { | ||
| if (!isset($configObj->system->ipv6allow) || ($configObj->system->ipv6allow == '1')) { |
There was a problem hiding this comment.
It does not exist in this configuration spot anymore
There was a problem hiding this comment.
I'm not sure I understand. Are you saying a later commit removes this? Yes, that intentional to ensure commit is consistent in itself. This way you can revert one of the later commits and don't end up in a broken state where this change is missing. Even if the revert no longer applies cleanly you should still see that you need to touch this file.
There was a problem hiding this comment.
There was a problem hiding this comment.
I see. I guess I never actually tested with IPv6 disabled, who would in 2026. Let me see if I can setup IPv4-based access to my test OPNsense instance to actually test it.
There was a problem hiding this comment.
Yep, I missed that a plugin might use it. The flip of the meaning was intended to address your first point here. I think you just need to chase these new locations. The inversion of the logic is already a good step :)
There was a problem hiding this comment.
I've now switched to the new property / auxiliary function and actually tested things with IPv6 disabled.
Thanks for pointing out my mistake.
| $ipv6_redirect_addr = null; | ||
| if ($_ipv6_enabled == true) { | ||
| $backend = new \OPNsense\Core\Backend(); | ||
| $interface = "wan"; |
There was a problem hiding this comment.
Why are you hardcoding wan here? The interface is specified in $this->config->tlsalpn_acme_interface. It's not always wan, the IP might be configured on a different interface.
There was a problem hiding this comment.
Redirecting to ::1 isn't possible, as described in the commit message. This is simply an attempt find any address in the right scope. Any address really, on any interface, that belongs to this host. I could use tlsalpn_acme_interface but, to my knowledge, there is no guarantee that this interface is set.
| # bind to port | ||
| server.bind = "127.0.0.1" | ||
| server.port = {{OPNsense.AcmeClient.settings.challengePort}} | ||
| $SERVER["socket"] == "127.0.0.1:{{OPNsense.AcmeClient.settings.challengePort}}" { } |
There was a problem hiding this comment.
Why are you removing this? It's there for a reason.
There was a problem hiding this comment.
My understanding is that this was added to have the web server only bind to localhost (::1 and 127.0.0.1). However, as stated in the commit message, we cannot do this for IPv6 because ::1 is a scope that doesn't allow traffic coming from another interface. Removing this reverts binding to the default behavior, namely, binding to all interfaces.
I could try to only bind to specific addresses but this seems a bit pointless. It would require to generate this config for every Challenge Type. Addresses my differ between them. I also looked at some other services running on OPNsense and binding to all interfaces and then using the firewall to restrict access seems the norm:
# netstat -an -f inet6
Active Internet connections (including servers)
Proto Recv-Q Send-Q Local Address Foreign Address (state)
...
tcp6 0 0 *.53 *.* LISTEN
tcp6 0 0 *.53 *.* LISTEN
tcp6 0 0 *.53 *.* LISTEN
tcp6 0 0 *.53 *.* LISTEN
tcp6 0 0 *.22 *.* LISTEN
tcp6 0 0 *.443 *.* LISTEN
tcp6 0 0 *.43580 *.* LISTEN
...
There was a problem hiding this comment.
Actually found some issues with this while testing with IPv6 disabled and I ended up readding explicit binding for IPv4 and IPv6.
|
One thing I should probably point out is that IP Auto-Discovery still doesn't work with IPv6. Maybe I'll look at it later, not sure. |
`ipv6allow` was renamed to `disableipv6`. It is now always set and its meaning was flipped.
…6 address Note that this will still not include virtual IPs but only the main IPs.
…nges Underlying issue: IPv6 features multiple scopes restricting where the IP address is valid [1]. ::1 belongs to link-local scope which is not allowed to be routed. As result FreeBSD will reject the connection after rewriting, as it will come from global scope (the internet) and going to ::1 [2]. This isn't allowed. The fix / workaround: Instead of redirecting to ::1, the following is tried: * If there is a WAN interface with an IPv6 address defined, redirect to this address. I expect most setup with IPv6 to have a WAN interface with a suitable (=allowed scope) IPv6 address. * Else, only redirect the port and leave the address unchanged. This will only work if we are issuing a certificate for ourselves (rather than a host behind the firewall). A better solution would be to pick an arbitrary IPv6 address of the host with a suitable scope. However, I believe this would be considerably more complex to implement and test. I propose we use this simplified approach, at least for now, which should already work for the vast majority of users. [1]: https://en.wikipedia.org/wiki/IPv6_address#Address_scopes [2]: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=193568
3 participants
Important notices
Before you submit a pull request, we ask you kindly to acknowledge the following:
If AI was used, please disclose:
n/a
Related issue
Fixes #5228
Describe the problem
Issuing certificates for IPv6-only hosts failed.
Describe the proposed solution
See commit messages.