Table.Close() had a value receiver, so setting t.t = nil only modified
the callee's copy of the handle. The caller's Table was left looking
valid:
t, _ := nftables.NewTable(...)
t.Close()
t.IsValid() // true
Worse, Close() only dropped the nftables handle, and nftApply() opens a
new one whenever it finds none. So Apply() and Reload() on a closed table
silently reopened a handle and carried on updating the ruleset.
At the root of it, a Table looked like a plain value but behaved like a
reference, and nothing stopped it from being copied. So embed table in
Table by value and hand out *Table instead. The Table/table split is
still needed - table's fields have to be exported for text/template -
but reference semantics are now visible at every call site, and they're
enforced: because table contains a sync.Mutex, "go vet" reports both a
copy of a Table and a method or function that takes one by value, so the
shape of this bug is no longer expressible.
Close() therefore can't invalidate the table by clearing a pointer, it
has to record the state. Add a closed flag, and refuse to open a new
nftables handle for a table that's been closed.
Apply() checked neither for a closed table nor for a nil *Table, which
would have panicked. It now reports an error, checking the closed state
with applyLock held so that it can't race with Close(), and before the
in-memory table is touched so that a rejected update isn't recorded as
applied.
The invalid table is now a nil *Table rather than a zero-value Table,
which also removes the need for consumers to return an empty Table
alongside an error. That made it obvious that the nftabler was leaking
the table it had just created when it gave up on setting up IPv6, so
close it.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correct the version referenced in the comment describing the migration
performed by Network.dropLegacyFilterDirectAccess.
The migration was introduced in v28.2.0, not v28.0.2. Also update the
TODO to clarify that the migration can be removed once we no longer
expect upgrades from v28.0.x or v28.1.x directly to a release that
includes the migration.
Refs: a0ff0a361e ("libnetwork: drop
legacy direct access filter rules")
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The early return duplicated the length check, making the custom error
path unreachable. As a result, a nil error was wrapped and returned
instead of the intended error message.
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Stale rules in one firewall backend could persist if the daemon's
firewall backend is switched without rebooting the host, which could
interfere with the rules being programmed for the current firewall
backend. Have the overlay network driver delete any stale nftables table
when starting in iptables mode, and delete any stale iptables per-VNI
encryption rules when programming encryption for the VNI in nftables
mode.
Signed-off-by: Cory Snider <csnider@mirantis.com>
When the daemon is linked against libnftables it programs the kernel
without invoking the `nft` command. Allow the nftables firewall backend
to be enabled when libnftables is used, irrespective of whether `nft` is
installed on the host.
Update the bridge network driver to clean up stale nftables tables in
iptables mode without depending on the `nft` command.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Port the firewall ruleset for encrypted overlay networks to nftables.
Maximize compatibility with the most distros by only using nftables
features that are widely available. Use the deprecated 'meta secpath
exists' expression instead of the more modern 'meta ipsec exists'.
Extract the VNI from VXLAN packets using the more widely available '@th'
raw payload expressions instead of '@ih' or 'vxlan vni' expressions.
Signed-off-by: Cory Snider <csnider@mirantis.com>
An nftables vmap is just a map whose element values are of type
`verdict`. Generalize VMap, VMapElement and Set to support any element
type.
Add a fluent API to build arbitrary tuple and mapping types from the
composition of other types or the 'typeof' an nftables expression.
Block the invalid composition of named types and `typeof` expressions at
compile time.
There are only a handful of contexts where the data type needs to be
specified: set and map definitions. Modelling set and map types as a
singular "nft type" does not align well with the semantics of nftables.
Map types are always composite types with a key and a value part. Set
types do not have a value part; it is an error to create a map with a
set type or vice versa. Encode this distinction into the Go type system
so it is a compile-time error to try to use a set type in a map context
or a map type in a set context.
Drop the 'NftType' prefix from the primitive set-type constants. The
prefix stutters with the package name and, as discussed above, it is not
accurate to call them "nft types." Verdicts cannot be used as set
elements or map keys. Provide dedicated methods to construct verdict-map
types from set types instead of modelling verdicts as types themselves.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Now `dockerd-rootless.sh` launches RootlessKit with `--detach-netns`
so as to run the daemon in the host network namespace.
The libnetwork namespaces are allocated inside the "detached" netns
(`$ROOTLESSKIT_STATE_DIR/netns`) that is associated with slirp4netns,
vpnkit, pasta, etc., as the rootless daemon has no `CAP_NET_ADMIN` for
the host network namespace.
This will enable:
- Accelerated (and deflaked) `docker pull`, `docker push`, `docker build`, etc
- Proper support for `docker pull 127.0.0.1:.../...`
- Proper support for `dockern run --net=host`
See also:
- rootless-containers/rootlesskit PR 379
- containerd/nerdctl PR 2723
NOTE: libnetwork contains code generated by Claude Code
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
The Cleanup function oportunistically tries to cleanup old rule (if any).
In cases where the rules didn't exist, it would log the error returned
by the command, but this would always be `exit status 1`, which doesn't
provide details about the actual failure (i.e., if it's the expected
"does not exist").
We could improve the code by first calling `nft list table` to check if
rules were found, but that would expose a similar problem (error could
be due to the rules not present, or "other cause".
This patch just changes the logs to be more informative, and passes the
context to allow cancelling the command if the context is cancelled.
Before this patch:
INFO[2026-02-21T13:30:10.428457589Z] Deleting nftables IPv4 rules error="exit status 1"
INFO[2026-02-21T13:30:10.453012214Z] Deleting nftables IPv6 rules error="exit status 1"
With this patch:
INFO[2026-02-21T14:10:06.183933878Z] Deleting nftables IPv4 rules error="exit status 1" output="Error: Could not process rule: No such file or directory\ndelete table ip docker-bridges"
INFO[2026-02-21T14:10:06.212198878Z] Deleting nftables IPv6 rules error="exit status 1" output="Error: Could not process rule: No such file or directory\ndelete table ip6 docker-bridges"
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Security Parameter Index value signals to the recipient which key to
decrypt the packet with. The overlay driver derives the SPI value for a
flow from a hash digest of the source and destination IP addresses. The
source and destination need to derive the same digest given the same
information as the SPI values are not signaled over the overlay driver's
control plane. Refactoring the overlay driver to use netip types
accidentally changed the hash function to digest IPv4 addresses in
4-byte form, causing newer engines to calculate a different SPI value
for a flow than older engines would. Restore the original calculation
by hashing IPv4 addresses in their 16-byte form, and refactor the
buildSPI function to take netip.Addr parameters to prevent 16-byte vs
4-byte mixups from being possible in the future.
Signed-off-by: Cory Snider <csnider@mirantis.com>
In rootless mode, the Engine needs to call the rootless port driver to
know which IP address it should bind to inside of its network namespace.
The slirp4netns port drivers doesn't support binding to IPv6 address, so
we need to detect that before listening on the port.
Before commit 201968cc0, this wasn't a problem because the Engine was
binding the port, then calling rootless port driver to learn whether the
proto/IP family was supported, and listen on the port if so.
Starting with that commit, the Engine does bind + listen in one go, and
then calls the port driver — this is too late. Fix the bug by checking
if the port driver supports the PortBindingReq, and only allocate the
port if so.
Signed-off-by: Albin Kerouanton <albin.kerouanton@docker.com>
Make the DiscoverNew switch only responsible for asserting the correct
data type, and push the conversion logic into the setKeys and updateKeys
methods.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Inline some vars and align between drivers
- Remove nested if's where possible
- Use `WithError` for some logs, and use the context if available
- Scope variables locally where only used locally and, the reverse,
make it clear where a (function-)global variable is used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was calling driver.getNetworks, which copies the networks map
into a new slice. As we're not mutating the networks, we can just use the
networks map itself to check if there's any networks configured with the
same parent.
While changing;
- Also change the signature to accept the parent to compare to as a string
- Return early once we determined there's more than one user
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Both were added as part of the initial implementation in commit [moby@ea30113]
([libnetwork@1d6f2c5]), but never used.
[moby@ea30113]: ea30113303
[libnetwork@1d6f2c5]: 1d6f2c59c4
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This utility was only called with two constant values;
const (
defaultV4RouteCidr = "0.0.0.0/0"
defaultV6RouteCidr = "::/0"
)
However;
- calling it would always execute a `net.ParseCIDR`
- verify if it would produce an error (which would be very unlikely)
- it used a `staticRoute` struct that was ONLY used for this function
- and immediately deconstructed into its components
- furthermore, the `NextHop` field would be discarded by jinfo.AddStaticRoute,
which only used the third argument for `routeType == types.NEXTHOP`
This patch:
- removes the `ifaceGateway` and associated `staticRoute` and consts
- defines two package-level vars for `defaultV4Net` and `defaultV6Net`,
which can be reused (no need to parse / construct them for every join)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These drivers did not do anything meaningful in the `Leave` method; they
would check if the network and/or endpoint were missing, in which case
they produced an error, but the network and endpoint (if present) would
not be used, so it was only validation.
Such validation could still be relevant elsewhere, but looking at where
this method is called; the `Driver.Leave()` is called in two places, both
of which don't handle the error, other than logging it as a warning / error;
It's called by `Endpoint.sbJoin()`, as part of the rollback;
d5c838dc5e/daemon/libnetwork/endpoint.go (L539-L545)
And `Endpoint.sbLeave()`, which also discards the error;
d5c838dc5e/daemon/libnetwork/endpoint.go (L772-L776)
Based on he above, this code looks to be redundant, so replacing it with
a stub; returning `nil`.
As replacing the code removed the use of network.getEndpoint, which was effectively
a copy of network.endpoint (which didn't have error handling), I merged the two
methods, and removed custom error-handling elsewhere.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These methods were just wrappers around getSubnetforIP; let's peel away the
abstraction and call it directly; we're already checking for n.config.Ipv4Subnet
and n.config.Ipv6Subnets on the call-site, so may as well just pass it in.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There were some missing checks whether ep.addr, ep.addrv6 were nil,
which could panic in getSubnetForIP.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The function was fetching a reference to the endpoint twice; while this
did give the option for an early return, in practice it didn't mean much,
because it could still fail if the endpoint was removed in between.
This code still has a race condition, because while a reference to the
endpoint is retrieved while acquiring a lock, the result is mutated without.
This probably needs to either have some accessor, or the function should
keep a lock for the whole operation (possibly switching to an RWMutex).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When the bridge driver encounters an error during network
creation, delete the bridge device if one has been added.
Signed-off-by: Rob Murray <rob.murray@docker.com>
These utilities are very handy to use in integration tests, too. Move
the package so it can be imported by them.
Signed-off-by: Cory Snider <csnider@mirantis.com>
These utilities are going to be needed elsewhere in the daemon to handle
netip values from API requests.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Allow tests to run in parallel with separate network namespaces,
without modifying the global-state namespace/netlink handles in
the "ns" package ... only useful for tests that don't depend on
package "ns".
Use the new option in iptabler/nftabler tests.
Signed-off-by: Rob Murray <rob.murray@docker.com>
