Commit Graph

213 Commits

Author SHA1 Message Date
Albin Kerouanton
d216084185 libnet/d/bridge: drop remote connections to port mapped on lo
Traditionally when Linux receives remote packets with daddr set to a
loopback address, it reject them as 'martians'. However, when a NAT rule
is applied through iptables this doesn't happen. Our current DNAT rule
used to map host ports to containers is applied unconditionally, even
for such 'martian' packets.

This means a neighbor host (ie. a host connected to the same L2
segment) can send packets to a port mapped on a loopback address. The
purpose of publishing on a loopback address is to make ports
inaccessible to remote hosts -- lack of proper filtering defeats that.

This commit adds an iptables rule to the raw-PREROUTING chain to drop
packets with a loopback dest address and coming from any interface other
than lo.

To accomodate WSL2 mirrored mode, another rule is inserted beforehand to
specifically accept packets coming from the loopback0 interface.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2025-01-27 18:41:20 +01:00
Albin Kerouanton
27adcd596b libnet/d/bridge: port mappings: drop direct-access when gw_mode=nat
When a NAT-based port mapping is created, the daemon adds a DNAT rule in
nat-DOCKER to replace the dest addr with the container IP. However, the
daemon never sets up rules to filter packets destined directly to the
container port. This allows a rogue neighbor (ie. a host that shares a
L2 segment with the host) to send packets directly to the container on
its container-side exposed port.

For instance, if container port 5000 is mapped to host port 6000, a
neighbor could send packets directly to the container on its port 5000.

Since nat-DOCKER mangles the dest addr, and the nat table forbids DROP
rules, this change adds a new rule in the raw-PREROUTING chain to filter
ingress connections targeting the container's IP address.

This filtering is only done when gw_mode=nat. For the unprotected
variant, no filtering is done.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2025-01-27 18:41:20 +01:00
Rob Murray
eaa84bc8f4 Send unsolicited ARP/NA requests when bringing up interfaces
Co-authored-by: Cory Snider <csnider@mirantis.com>
Co-authored-by: Rob Murray <rob.murray@docker.com>
Signed-off-by: Rob Murray <rob.murray@docker.com>
2025-01-22 16:59:27 +00:00
Albin Kerouanton
fc7caf96d2 Revert "libnet/d/bridge: port mappings: filter by input iface"
This reverts commit 433b1f9b17.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2025-01-20 14:11:51 +01:00
Albin Kerouanton
433b1f9b17 libnet/d/bridge: port mappings: filter by input iface
When a NAT-based port mapping is created with a HostIP specified, we
insert a DNAT rule in nat-DOCKER to replace the dest addr with the
container IP. Then, in filter chains, we allow access to the container
port for any packet not coming from the container's network itself (if
hairpinning is disabled), nor from another host bridge.

However we don't set any rule that prevents a rogue neighbor that shares
a L2 segment with the host, but not the one where the port binding is
expected to be published, from sending packets destined to that HostIP.

For instance, if a port binding is created with HostIP == '127.0.0.1',
this port should not be accessible from anything but the lo interface.
That's currently not the case and this provides a false sense of
security.

Since nat-DOCKER mangles the dest addr, and the nat table rejects DROP
rules, this change adds rules into raw-PREROUTING to filter ingress
packets destined to mapped ports based on the input interface, the dest
addr and the dest port.

Interfaces are dynamically resolved when packets hit the host, thanks
to iptables' addrtype extension. This extension does a fib lookup of the
dest addr and checks that it's associated with the interface reached.

Also, when a proxy-based port mapping is created, as is the case when an
IPv6 HostIP is specified but the container is only IPv4-capable, we
don't set any sort of filtering. So the same issue might happen. The
reason is a bit different - in that case, that's just how the kernel
works. But, in order to stay consistent with NAT-based mappings, these
rules are also applied.

The env var `DOCKER_DISABLE_INPUT_IFACE_FILTERING` can be set to any
true-ish value to globally disable this behavior.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2025-01-13 19:04:25 +01:00
Derek McGowan
9c368a93b6 Split internal idtools functionality
Separare idtools functionality that is used internally from the
functionlality used by importers. The `pkg/idtools` package is now
much smaller and more generic.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-01-07 11:18:46 -08:00
Paweł Gronowski
0f7a43271c daemon/events: Move metrics to internal/metrics
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-01-07 14:13:10 +01:00
Paweł Gronowski
90e2afd0be daemon: Move direct usages of go-metrics to internal/metrics
Add wrapper for StartTimer inside the internal package

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-01-07 14:13:08 +01:00
Paweł Gronowski
438f5801e1 daemon/images: Move ImageActions to metrics
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-01-07 14:13:07 +01:00
Paweł Gronowski
51c2689427 daemon/metrics: Move out to internal/metrics
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-01-07 14:13:06 +01:00
Sebastiaan van Stijn
b7c88502fb internal/testutils/networking: use lazyregexp
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-01-02 21:37:35 +01:00
Sebastiaan van Stijn
43b3aaa62d implement lazyregexp package
Based on the "lazyregexp" package in golang.org/x/mod;
https://cs.opensource.google/go/x/mod/+/refs/tags/v0.19.0:internal/lazyregexp/lazyre.go;l=66-78

This package allows defining regular expressions that should not be
compiled until used, but still providing validation to prevent
invalid regular expressions from producing a panic at runtime.

This is largely a copy of the package from golang.org/x/mod,
with FindAllStringSubmatch and ReplaceAllStringFunc added

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-01-02 21:37:11 +01:00
Albin Kerouanton
e5bf6d8ba0 libnet: pass store as an arg to netdrivers
Before that change, we were passing the datastore to network drivers
through a `map[string]interface{}`. Then, each driver that needed the
store would cast the datastore to the correct type.

This was not a good design, as it was not clear which drivers were using
the store and which were not. Not all unit tests were passing the store,
leading to logs about uninitialized store being written.

This change makes the store a parameter of the `RegisterX` functions.
All unit tests are now passing a valid datastore to the drivers. A new
testutil func is added for that purpose.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2024-12-20 17:51:53 +01:00
Rob Murray
6473d37002 Run commands in an L3Segment netns without failing the test.
Return an error from runCommand instead of calling t.Fatal, and add
Host.Run to return that error.

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-12-17 09:14:25 +00:00
Rob Murray
3bf9a80818 Rename L3Segment Host.Run -> Host.MustRun
Like netip.MustParseIP, it fails on error.

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-12-17 09:14:25 +00:00
Paweł Gronowski
d8358ebc87 otel: Use non-noop tracer provider for grpc
Needed for Buildkit history

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-12-12 19:31:00 +01:00
Rob Murray
4740820716 Use ioctl to try to trigger kernel module loads
An ioctl() call to get the "interface index" for a kernel module triggers
the kernel to try to load the module, if the process is running with
CAP_SYS_MODULE. This tends to be more reliable than "modprobe" for
docker-in-docker.

If the ioctl() method fails, fall back to trying "modprobe".

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-12-06 10:27:24 +00:00
Rob Murray
af0b973595 Allow IPv4 and IPv6 host-gateway-ip addresses
Running a container with "--add-host blah:host-gateway" adds an /etc/hosts
entry for host "blah" and an address on the docker host - to give the
container a convenient way of reaching the host.

If no --host-gateway-ip option is supplied, the IPv4 address of the
default bridge is used - and that's been fine until now, it's a host
address we know will exist. But, in a container that's only connected
to IPv6-only networks, that doesn't work.

So:
- if the default bridge has an IPv6 address, create an additional
  /etc/hosts entry with that adddress
- allow two --host-gateway-ip options
  - at most one IPv4 and one IPv6 address
- in daemon.json, allow a JSON array value in --host-gateway-ips (plural)
  - for a single address, a JSON string is also allowed

For example:
  --host-gateway-ip 192.0.2.1 --host-gateway-ip 2001:db8::1111
And the daemon.json version would be:
  "host-gateway-ips": ["192.0.2.1", "2001:db8::1111"]
But, this is also still valid:
  "host-gateway-ip": "192.0.2.1"

Note that the /etc/hosts entries follow the usual rules. If IPv6 is
disabled in a container (by sysctl, or lack of kernel support), IPv6
addresses are not included in the file. In other cases, IPv4 and IPv6
addresses will both be included, whether or not the container currently
has network endpoints that support IPv4 or IPv6.

buildx has its own code to interpret the host-gateway-ip option. When
it's updated to understand two addresses, moby will need to pass it
both. For now, it passes an IPv4 address if there is one, else IPv6.

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-11-26 11:47:31 +00:00
Sebastiaan van Stijn
b034dc41a2 deprecate pkg/platform and move internal
Functions in this package are only used internally in the daemon for
the `/info` endpoint (Architecture), and as part of `stats` (NumProcs).

I was not able to find external consumers, but deprecating the package
first, so that we can remove / dismantle the package in a follow-up.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-11-13 15:05:56 +01:00
Sebastiaan van Stijn
7db58fefdc internal/mod: remove redundant capturing of loop vars (copyloopvar)
internal/mod/mod_test.go:62:3: The copy of the 'for' variable "tt" can be deleted (Go 1.22+) (copyloopvar)
            tt := tt
            ^

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-11-12 14:02:15 +01:00
Sebastiaan van Stijn
6a85a13c0a internal/testutils/specialimage: remove redundant capturing of loop vars (copyloopvar)
internal/testutils/specialimage/partial.go:41:3: The copy of the 'for' variable "platform" can be deleted (Go 1.22+) (copyloopvar)
            platform := platform
            ^

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-11-12 14:02:13 +01:00
Sebastiaan van Stijn
b453aa65fa update go:build tags to use go1.22
commit a0807e7cfe configured golangci-lint
to use go1.23 semantics, which alowed linters like `copyloopvar` to lint
using thee correct semantics.

go1.22 now creates a copy of variables when assigned in a loop; make sure we
don't have files that may downgrade semantics to go1.21 in case that also means
disabling that feature; https://go.dev/ref/spec#Go_1.22

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-11-12 14:02:09 +01:00
Sebastiaan van Stijn
ad8196f48a Merge pull request #48722 from vvoland/c8d-save-addtests
c8d/save: Add tests
2024-11-05 14:35:06 +01:00
Paweł Gronowski
db68a019fd c8d/save: Add tests
Test saving a shallow/partial image

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-10-28 13:23:10 +01:00
jinjiadu
3cc736e95c chore: fix some function names
Signed-off-by: jinjiadu <jinjiadu@aliyun.com>
2024-10-28 18:20:43 +08:00
Sebastiaan van Stijn
61433c2a6e Merge pull request #48774 from thaJeztah/safepath_nits
internal/safepath: log some unhandled errors, and remove workaround for ECI / Sysbox
2024-10-28 10:39:38 +01:00
Sebastiaan van Stijn
75e8f57579 internal/safepath: Join(): remove workaround for ECI / Sysbox
This workaround was added in 9a0cde66ba to
work around an issue on Docker Desktop with ECI (Enhanced Container Isolation)
enabled, which uses the Sysbox runtime under the hood.

A comment was added during review of the PR that added it (see [1]), and the
internal discussion on Slack tracked down the issue to code in [nestybox/sysfs].

That issue was resolved Sysbox EE, and upstreamed to Sysbox CE through
[nestybox/sysbox-fs@9cf74e4], which is part of Sysbox CE v0.6.3, so we
can remove this workaround.

[1]: https://github.com/moby/moby/pull/45687#discussion_r1280867905
[nestybox/sysfs]: 30fd49edbd/process/process.go (L644-L684)
[nestybox/sysbox-fs@9cf74e4]: 9cf74e4cbf

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-27 13:13:55 +01:00
Sebastiaan van Stijn
4b60c68803 internal/safepath: Join(): log some unhandled errors
Similar to the kubernetesSafeOpen function.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-27 13:13:50 +01:00
Sebastiaan van Stijn
5fc5b0574b internal/safepath: kubernetesSafeOpen: explicitly suppress unhandled err
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-27 12:35:47 +01:00
Sebastiaan van Stijn
5c48736863 remove redundant alias for runtime-spec
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-26 18:31:39 +02:00
Albin Kerouanton
bb364cc0f3 inte/t/networking: delete veth ifaces before netns
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2024-10-24 18:18:13 +02:00
Rob Murray
aba8df74a1 Add TestDirectRoutingOpenPorts
Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-10-22 14:38:13 +01:00
Rob Murray
3ba06cf6b9 Merge pull request #48593 from robmry/iptables_doc
Internal documentation for iptables rules
2024-10-15 15:41:30 +01:00
Rob Murray
20571e453b Generate iptables documentation
In an integration test - run a daemon, capture iptables, and feed them
to a markdown text/template describing them.

Prep for repeating that, for different network configurations.

Fail the test if the generated markdown differs from a "golden" version.

(So, at-least the golden markdown will need to be updated if the
iptables rules are deliberately changed - hopefully the corresponding
description in the template will also be updated.)

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-10-15 14:02:49 +01:00
Rob Murray
a0a0bbae6c Discard ErrDumpInterrupted and return data after maxRetries
Returning possibly inconsistent data avoids retrying indefinitely, and
matches the behaviour of vishvananda/netlink prior to version 1.2.1, in
which the NLM_F_DUMP_INTR flag was ignored.

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-10-15 11:35:23 +01:00
Rob Murray
2f02f456f5 Fix comments
Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-10-07 18:32:06 +01:00
Albin Kerouanton
5875b6e8cf inte/networking: test access to published ports from remote host
Along with this new test, a new struct `L3Segment` is introduced to
simulate hosts connected on a same switched network. This struct will
let us test various scenarios where published ports and containers
should or should not be accessible from remote hosts.

The new test introduced, `TestAccessPublishedPortFromRemoteHost`, skips
link-local address as port publishing doesn't work on those addresses
currently. This will be fixed in a future commit.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
2024-10-04 20:09:05 +02:00
Brian Goff
fe09cab7fe Merge pull request #48407 from robmry/48400_netlink_eintr
Retry on EINTR from netlink dump calls
2024-09-16 09:08:53 -07:00
Sebastiaan van Stijn
c2fc1f4a40 internal/opts: SetOpts: invalidate empty option-names
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-16 11:00:06 +02:00
Sebastiaan van Stijn
908bb959e7 internal/opts: SetOpts.Set: remove redundant var assignment
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-16 10:39:20 +02:00
Sebastiaan van Stijn
758cca6036 internal/opts: SetOpts,NamedSetOpts: test for optional value
The value is optional for SetOpts (and NamedSetOpts), and implied
"true" when omitted.

This patch adds a test-case for this.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-16 10:38:32 +02:00
Rob Murray
edaa0eb56d Add linting rules to spot use of un-wrapped netlink functions.
Spot netlink functions that may return EINTR because
network configuration changed during a state dump, and
point at the nlutil wrappers.

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-09-15 12:28:58 +01:00
Rob Murray
00bf437d84 Add nlutil functions to retry on netlink EINTR
A recent change to the vishvananda/netlink package exposes
NLM_F_DUMP_INTR in some netlink responses as an EINTR (with
no data).

Retry the requests when that happens, up to five times, before
returning the error. The limit of five is arbitrary, on most
systems a single retry will be rare but, there's no guarantee
that a retry will succeed. So, on a very busy or misbehaving
system the error may still be returned. In most cases, this
will lead to failure of the operation being attempted (which
may lead to daemon startup failure, network initialisation
failure etc).

Signed-off-by: Rob Murray <rob.murray@docker.com>
2024-09-15 12:28:58 +01:00
Sebastiaan van Stijn
164cae56ed Merge pull request #48167 from dmcgowan/add-feature-flags-daemon
Add `--feature` to daemon flags
2024-09-12 19:15:19 +02:00
Derek McGowan
f13c08246d Add feature to daemon flags
Signed-off-by: Derek McGowan <derek@mcg.dev>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-12 15:35:15 +02:00
Paweł Gronowski
f143f4ec51 image/save&load: Support Platform parameter
Add `Platform` parameter that allows to select a specific platform to
save/load.

This is a breaking change to the Go client as it changes the signatures
of `ImageLoad` and `ImageSave`.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-09-11 19:44:35 +02:00
Sebastiaan van Stijn
16d686c7bc internal/unix_noeintr: fix godoc for package
Put the comment in the expected format :)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-09-09 19:32:11 +02:00
Nathan Baulch
59eba0ae13 Fix typos
Signed-off-by: Nathan Baulch <nathan.baulch@gmail.com>
2024-09-06 21:53:09 +10:00
Paweł Gronowski
980275dc90 Merge pull request #48399 from vvoland/c8d-list-nonimage
c8d/list: Don't exclude non-container images
2024-08-29 19:16:23 +02:00
Paweł Gronowski
f4e82e2fb0 c8d/list: Don't exclude non-container images
Before this, the image list would not show images that are not a valid
container image, but could be a valid artifact.

While they're not directly usable by docker, we should still show them
so the user can still discover them and at least be able to delete them.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-08-29 11:56:17 +02:00