8 Commits

Author SHA1 Message Date
Rob Murray
b3fabedecc Create docker-proxy TCP/UDP listener sockets in the daemon
Before commit 4f09af6, when allocating host ports for a new
port mapping, iptables rules were set up then docker-proxy was
started. If the host port was already in-use, docker-proxy exited
with an error, and the iptables rules were removed. That could
potentially interfere with a non-docker service that was already
using the host port for something unrelated.

Commit 4f09af6 swapped that problem for a different one... in
order to check that a port was available before creating iptables
rules, it attempted to start docker-proxy first. If it failed, it
could then try a different host port, without interfering with
any other service. The problem with that is docker-proxy would
start listening before the iptables rules were in place, so it
could accept connections then become unusable because new NAT
rules diverted packets directly to the container. This would leave
the client with a broken connection, causing at-least a delay
while it figured that out and reconnected.

This change creates and binds the socket in the daemon, before
creating iptables rules. If the bind fails, it may try a different
port. When or if the bind succeeds, iptables rules are created,
then the daemon calls listen on the socket. If docker-proxy is
needed, the socket is handed over to it at that point.

In rootless mode, the ports have to be bound to an address in the
rootless network namespace (where dockerd is running). DNAT rules
now use the same address.

If docker-proxy is not needed ("--userland-proxy=false"), the daemon
still listens on TCP sockets as the old dummyProxy would have done.
This makes the socket show up in "netstat" output.

The dummyProxy is no longer needed on Linux. Its job was to bind the
host ports if docker-proxy was disabled, but that's now already
handled by binding the sockets early.

This change doesn't affect SCTP, because it's not currently possible
for docker-proxy to convert the file descriptor into an SCTPListener.
So, docker-proxy is still started early, and the window for lost
connections remains.

If the user has an old docker-proxy in their path and it's given a
listener docker with '-use-listen-fd', it'll fail because of the
unknown option. In this case, the daemon's error message suggests
checking $PATH.

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-08-05 14:04:05 +01:00
Rob Murray
4f09af6267 Allocate same port for IPv4/IPv6 for 'any interface' mappings.
The bridge driver now does its own port-mapping, rather than using the
portmapper module (which ran as two completely separate instances, for
IPv4 and IPv6).

When asked for a mapping from any host address (0.0.0.0/0) with a range
of host ports, the same port will be allocated for IPv4 and IPv6, or the
mapping will fail with an error if that's not possible.

The bridge driver now manages its own port mappings. So, remove
linux-specific PortMapper code and make what's left Windows-only.

Also, replace the portmapper.userlandProxy interface with StartProxy().

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-06-11 16:50:15 +00:00
Sebastiaan van Stijn
4f9db655ed portmapper: move userland-proxy lookup to daemon config
When mapping a port with the userland-proxy enabled, the daemon would
perform an "exec.LookPath" for every mapped port (which, in case of
a range of ports, would be for every port in the range).

This was both inefficient (looking up the binary for each port), inconsistent
(when running in rootless-mode, the binary was looked-up once), as well as
inconvenient, because a missing binary, or a mis-configureed userland-proxy-path
would not be detected daeemon startup, and not produce an error until starting
the container;

    docker run -d -P nginx:alpine
    4f7b6589a1680f883d98d03db12203973387f9061e7a963331776170e4414194
    docker: Error response from daemon: driver failed programming external connectivity on endpoint romantic_wiles (7cfdc361821f75cbc665564cf49856cf216a5b09046d3c22d5b9988836ee088d): fork/exec docker-proxy: no such file or directory.

However, the container would still be created (but invalid);

    docker ps -a
    CONTAINER ID   IMAGE          COMMAND                  CREATED          STATUS    PORTS     NAMES
    869f41d7e94f   nginx:alpine   "/docker-entrypoint.…"   10 seconds ago   Created             romantic_wiles

This patch changes how the userland-proxy is configured;

- The path of the userland-proxy is now looked up / configured at daemon
  startup; this is similar to how the proxy is configured in rootless-mode.
- A warning is logged when failing to lookup the binary.
- If the daemon is configured with "userland-proxy" enabled, an error is
  produced, and the daemon will refuse to start.
- The "proxyPath" argument for newProxyCommand() (in libnetwork/portmapper)
  is now required to be set. It no longer looks up the executable, and
  produces an error if no path was provided. While this change was not
  required, it makes the daemon config the canonical source of truth, instead
  of logic spread accross multiplee locations.

Some of this logic is a change of behavior, but these changes were made with
the assumption that we don't want to support;

- installing the userland proxy _after_ the daemon was started
- moving the userland proxy (or installing a proxy with a higher
  preference in PATH)

With this patch:

Validating the config produces an error if the binary is not found:

    dockerd --validate
    WARN[2023-12-29T11:36:39.748699591Z] failed to lookup default userland-proxy binary       error="exec: \"docker-proxy\": executable file not found in $PATH"
    userland-proxy is enabled, but userland-proxy-path is not set

Disabling userland-proxy prints a warning, but validates as "OK":

    dockerd --userland-proxy=false --validate
    WARN[2023-12-29T11:38:30.752523879Z] ffailed to lookup default userland-proxy binary       error="exec: \"docker-proxy\": executable file not found in $PATH"
    configuration OK

Speficying a non-absolute path produces an error:

    dockerd --userland-proxy-path=docker-proxy --validate
    invalid userland-proxy-path: must be an absolute path: docker-proxy

Befor this patch, we would not validate this path, which would allow the daemon
to start, but fail to map a port;

    docker run -d -P nginx:alpine
    4f7b6589a1680f883d98d03db12203973387f9061e7a963331776170e4414194
    docker: Error response from daemon: driver failed programming external connectivity on endpoint romantic_wiles (7cfdc361821f75cbc665564cf49856cf216a5b09046d3c22d5b9988836ee088d): fork/exec docker-proxy: no such file or directory.

Specifying an invalid userland-proxy-path produces an error as well:

    dockerd --userland-proxy-path=/usr/local/bin/no-such-binary --validate
    userland-proxy-path is invalid: stat /usr/local/bin/no-such-binary: no such file or directory

    mkdir -p /usr/local/bin/not-a-file
    dockerd --userland-proxy-path=/usr/local/bin/not-a-file --validate
    userland-proxy-path is invalid: exec: "/usr/local/bin/not-a-file": is a directory

    touch /usr/local/bin/not-an-executable
    dockerd --userland-proxy-path=/usr/local/bin/not-an-executable --validate
    userland-proxy-path is invalid: exec: "/usr/local/bin/not-an-executable": permission denied

Same when using the daemon.json config-file;

    echo '{"userland-proxy-path":"no-such-binary"}' > /etc/docker/daemon.json
    dockerd --validate
    unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: invalid userland-proxy-path: must be an absolute path: no-such-binary

    dockerd --userland-proxy-path=hello --validate
    unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: userland-proxy-path: (from flag: hello, from file: /usr/local/bin/docker-proxy)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-12-29 16:23:18 +01:00
Cory Snider
1f22b15030 Lock OS threads when exec'ing with Pdeathsig
On Linux, when (os/exec.Cmd).SysProcAttr.Pdeathsig is set, the signal
will be sent to the process when the OS thread on which cmd.Start() was
executed dies. The runtime terminates an OS thread when a goroutine
exits after being wired to the thread with runtime.LockOSThread(). If
other goroutines are allowed to be scheduled onto a thread which called
cmd.Start(), an unrelated goroutine could cause the thread to be
terminated and prematurely signal the command. See
https://github.com/golang/go/issues/27505 for more information.

Prevent started subprocesses with Pdeathsig from getting signaled
prematurely by wiring the starting goroutine to the OS thread until the
subprocess has exited. No other goroutines can be scheduled onto a
locked thread so it will remain alive until unlocked or the daemon
process exits.

Signed-off-by: Cory Snider <csnider@mirantis.com>
2022-10-05 12:18:03 -04:00
Eng Zer Jun
c55a4ac779 refactor: move from io/ioutil to io and os package
The io/ioutil package has been deprecated in Go 1.16. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
2021-08-27 14:56:57 +08:00
Sebastiaan van Stijn
ac8c80d6f1 portmapper: change userlandProxyCommandName to a const
it's not overridden anywhere, so may as well be a const

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-06-17 10:27:33 +02:00
Sebastiaan van Stijn
4231dbca23 portmapper: don't compile linux-only code on Windows
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-06-17 10:27:29 +02:00
Puneet Pruthi
a48b541da3 libnetwork support for Solaris
Signed-off-by: Puneet Pruthi <puneetpruthi@gmail.com>
2016-10-14 16:38:23 -07:00