Add handling for firewalld's StrictForwardPorts setting #1165
Conversation
mheon
force-pushed
the
firewalld_detect_strict_port_forward
branch
from
ade567a to
9114d77
Compare
|
I don't think this actually generates + installs the manpage, need to figure that bit out, but the contents should be solid |
mheon
force-pushed
the
firewalld_detect_strict_port_forward
branch
2 times, most recently
from
b3a0733 to
33d2a4a
Compare
|
Ephemeral COPR build failed. @containers/packit-build please check. |
1 similar comment
|
Ephemeral COPR build failed. @containers/packit-build please check. |
mheon
force-pushed
the
firewalld_detect_strict_port_forward
branch
2 times, most recently
from
59f93a4 to
6995211
Compare
|
@Luap99 PTAL, particularly at the manpage |
docs/Makefile
Outdated
| %.5: %.5.md | ||
| $(GOMD2MAN) -in $^ -out $@ |
There was a problem hiding this comment.
properly should look into de-duplicating this with .1 one. Though not a blocker
| @@ -0,0 +1,85 @@ | |||
| % NETAVARK-FIREWALLD 5 Netavark Firewalld Interactions Man Page | |||
There was a problem hiding this comment.
The name here is different from the actual file name netavark-firewalld-forwarding NETAVARK-FIREWALLD seems good enough so I suggest you rename the file.
Second, section 5 is wrong. That is for file formats/config files per man-pages(7). IT should be in section 7 based on that.
| It can be enabled by setting `firewall_driver` to `firewalld` in `containers.conf`. | ||
| The native firewalld driver offers better integration with firewalld, but presently suffers from several limitations. | ||
| It does not support isolation (the `--opt isolate=` option to `podman network create`. | ||
| It also does not currently support connections to a forwarded port of a container on the same host via public IP, only via the IPv4 localhost IP (127.0.0.1). |
There was a problem hiding this comment.
given most users are behind A NAT (for ipv4 at least) public IP may be misunderstood to refer to the public internet ip.
Maybe the wording should be reversed,i.e.:
When initiating a connection from the host system to the forwarded port it ONLY works when connecting to the IPv4 localhost (127.0.0.1). Using the host ips of other interface will not work. However when the connection comes from an external host it will work.
That raises the question about 127.0.0.0/8 ips. Users can do something like -p 127.0.0.10:8080:8080 to bind another localhost ip? Does that still work or is only 127.0.0.1 special?
|
|
||
| To forward a port externally, the following command should be run, substituting the desired host and container port numbers, protocol, and the container's IP. | ||
| ``` | ||
| # firewall-cmd --permanent --zone public --add-forward-port=port={HOST_PORT_NUMBER}:proto={PROTOCOL}:toport={CONTAINER_PORT_NUMBER}:toaddr={CONTAINER_IP} |
There was a problem hiding this comment.
I guess the zone here should be a variable? There is no need to use the public zone if users don't need it.
| It does not support isolation (the `--opt isolate=` option to `podman network create`. | ||
| It also does not currently support connections to a forwarded port of a container on the same host via public IP, only via the IPv4 localhost IP (127.0.0.1). | ||
|
|
||
| ### Strict Port Forwarding |
There was a problem hiding this comment.
We should note that this only applies to rootful podman. Rootless binds the port on the host and needs a regular accept port rule like it always did.
|
|
||
| The associated `localhostForward` policy does not need to be removed. | ||
|
|
||
| Please also note that, at present, it is not possible to access forwarded ports of a container on the same host via the host's public IP; the rich rule above must be applied instead and access must be via 127.0.0.1, not the public IP of the host. |
There was a problem hiding this comment.
same comment as on the other section, public ip might be confusing
| Please also note that, at present, it is not possible to access forwarded ports of a container on the same host via the host's public IP; the rich rule above must be applied instead and access must be via 127.0.0.1, not the public IP of the host. | ||
|
|
||
| Please note that the firewalld driver presently bypasses this protection, and will still allow traffic through the firewall when `StrictForwardPorts` is enabled without manual forwarding through `firewall-cmd`. | ||
| This may be changed in a future release. |
There was a problem hiding this comment.
Do we/you actual plan to change this?
There was a problem hiding this comment.
Reserving the right to do so; folks actually using the feature can tell us if they care.
src/firewall/firewalld.rs
Outdated
| /// Returns false if firewalld is not installed, not running, or there is any | ||
| /// error with the process. | ||
| pub fn is_firewalld_strict_forward_enabled() -> bool { | ||
| let conn = match Connection::system() { |
There was a problem hiding this comment.
looking at firewalld::add_firewalld_if_possible() it might be nice if to share the connecting and not open a new time for every single thing which seems super inefficient for no good reason.
We could call Connection::system() once and is_firewalld_running once as well and then just pass an Option<Connection> around.
mheon
force-pushed
the
firewalld_detect_strict_port_forward
branch
from
6995211 to
d1724e1
Compare
|
Comments should all be addressed |
docs/netavark-firewalld.7.md
Outdated
| @@ -0,0 +1,90 @@ | |||
| % NETAVARK-FIREWALLD 5 Netavark Firewalld Interactions Man Page | |||
There was a problem hiding this comment.
The number here needs to be updated too.
This mostly amounts to throwing a sensible error in cases where the setting is enabled and the user has requested ports be forwarded. There are no tests at present because firewalld 2.3.x is currently restricted to Rawhide, and is required for effective testing. A manpage has also been added with details on how to work with the StrictForwardPorts setting (as well as some other tidbits on firewalld + Podman integration). Signed-off-by: Matt Heon <[email protected]>
mheon
force-pushed
the
firewalld_detect_strict_port_forward
branch
from
d1724e1 to
6e5de55
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This mostly amounts to throwing a sensible error in cases where the setting is enabled and the user has requested ports be forwarded.
There are no tests at present because firewalld 2.3.x is currently restricted to Rawhide, and is required for effective testing.
A manpage has also been added with details on how to work with the StrictForwardPorts setting (as well as some other tidbits on firewalld + Podman integration).