Close() frees the libnftables context and nils out the handle, so passing
a closed nftCtx to Apply() would hand a nil pointer to libnftables and
crash the daemon.
No caller can do that today: table.nftApply() nil-checks its *nftCtx and
creates a new context when it has been closed, and RunCmd() owns its
context for the duration of a single call. Return an error anyway, rather
than depending on every future caller to get the lifecycle right.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Cory Snider <csnider@mirantis.com>
The libnftables context is C-allocated memory which is only freed when
nftCtx.Close() is called. Attach a runtime cleanup so that a context
which is dropped without being closed is freed once it becomes
unreachable, instead of being leaked for the lifetime of the process.
runtime.AddCleanup needs a pointer to a Go-heap object to attach to, so
wrap the libnftables handle in a Go struct rather than type-defining the
C struct. Keep the nftCtx reachable across the C calls in Apply(), and
across Cleanup.Stop() in Close(): a receiver may otherwise become
unreachable at its last mention, and Stop() has no effect once the
cleanup has already been queued for execution.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Cory Snider <csnider@mirantis.com>
SetBaseChainPolicy read the table's Chains map and modified the chain it
found without holding applyLock, only taking the lock when it called
Apply to make the change. Racing with an Apply that adds a chain, the map
read is a fatal "concurrent map read and map write".
Nothing calls it yet - the nftabler doesn't implement filterForwardDrop -
so this isn't a live bug, but the API shouldn't come with the race
attached.
Hold applyLock for the whole read-modify-apply. That needs an unexported
apply which assumes the lock is held, so split the body out of
Table.Apply, leaving the exported method as the wrapper that checks the
table and takes the lock, much like Reload and table.reload.
While here, make Reload report a closed table the same way as the other
two, rather than relying on the check in table.nftApply and reporting a
generic "invalid table" for a table that raced with Close.
The happy path of SetBaseChainPolicy had no test coverage at all, which
now matters more because it applies the table with applyLock held - a
deadlock would be silent. Add one.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
From the [DB.FreelistType] GoDoc:
// FreelistType sets the backend freelist type. There are two options. Array which is simple but endures
// dramatic performance degradation if database is large and fragmentation in freelist is common.
// The alternative one is using hashmap, it is faster in almost all circumstances
// but it doesn't guarantee that it offers the smallest page id available. In normal case it is safe.
// The default type is array
FreelistType FreelistType
While the default is still `FreelistArrayType`, the package shows that
the intent is to make `FreelistMapType` the default in future;
https://pkg.go.dev/go.etcd.io/bbolt@v1.5.0#pkg-constants
// TODO(ahrtr): eventually we should (step by step)
// 1. default to `FreelistMapType`;
// 2. remove the `FreelistArrayType`, do not export `FreelistMapType`
// and remove field `FreelistType' from both `DB` and `Options`;
[DB.FreelistType]: https://pkg.go.dev/go.etcd.io/bbolt@v1.5.0#DB.FreelistType
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add support for defining nftables maps and sets with size and timeout
specified, which are required values for maps and sets that are updated
from the packet path.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Errors when parsing and applying templates reference the offending line
number and column within the template text. It's not so easy to
correlate the position mentioned in the error with the template source
when the template is inlined as string literals in a Go source file.
Extract the nftables reload and incremental-update templates to
dedicated files and embed them into the program with //go:embed
directives so the offset mentioned in error messages is the offset
within the template file.
Having the templates in dedicated files also enables more rich IDE
integration. gopls for instance will highlight template directives (when
the `semanticTokens` setting is enabled) and report template syntax
errors inline when a template is in a dedicated file.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Looks like the linter didn't detect this in de71462b97
daemon/libnetwork/internal/nftables/fluentapi_linux.go:8:20: ST1016: methods on the same type should have the same receiver name (seen 1x "b", 2x "cd") (staticcheck)
func (b BaseChain) Builder() chainBuilder {
^
daemon/libnetwork/internal/nftables/fluentapi_linux.go:14:16: ST1016: methods on the same type should have the same receiver name (seen 1x "c", 2x "cd") (staticcheck)
func (c Chain) Builder() chainBuilder {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Change (*nftables.Table).Apply() to take an arbitrary number of
Modifiers to be applied as a single atomic unit. Having the ability to
atomically apply multiple modifiers to a table enables some powerful
patterns such as atomically replacing one collection of rules or
elements with another:
var update nftables.Modifier
update.Create(/* ... */)
t.Apply(reversePrevious, update)
reversePrevious = update.Reverse()
Signed-off-by: Cory Snider <csnider@mirantis.com>
With most of the dynamism of nftables rulesets being powered by named
maps and sets, the rules of a chain are often initialized when the chain
is added, and never touched again. Add a fluent API for adding the
creation of a chain and all its rules to a modifier without having to
repeat the chain name for each rule.
Signed-off-by: Cory Snider <csnider@mirantis.com>
As libnftables uses `select(2)` on the netlink socket the process is
aborted if the socket's file descriptor is >= 1024. A dockerd process
could easily exceed 1024 open file descriptors at a time under normal
circumstances, so there is a risk of libnftables killing dockerd at a
random time through no fault of dockerd. Default to programming nftables
rulesets by exec'ing `nft -f` until libnftables is updated to be
compatible with processes that open a large number of file descriptors
by using `poll(2)` or `epoll(2)` instead of `select(2)`.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Follow-up to PR 52804, applying thaJeztah's review suggestion: check
IsLoopback first for both address families (preserving any requested
loopback address), and only fall back to the canonical loopback for
the family otherwise. No behavior change; ::1 now returns through the
loopback-preserving branch instead of the IPv6 fallback, with the same
result.
Signed-off-by: Andrew Liu <andrewjliu22@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
Afford applying nft commands via libnftables without needing to go
through our table abstraction. Make the table abstraction responsible
for lazily allocating an nft context.
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>
In rootless mode, ChildHostIP maps every IPv4 host address to 127.0.0.1
in the child network namespace. Port bindings on the same port but
distinct loopback addresses (e.g. 127.0.1.2:80 and 127.0.1.3:80) were
therefore both reserved as 127.0.0.1:80 by the port allocator in the
child namespace, and the second binding failed with "Bind for
127.0.0.1:8080 failed: port is already allocated" even though the
requested addresses do not conflict.
Preserve IPv4 loopback host addresses as the child host IP instead. The
child namespace's lo interface covers all of 127.0.0.0/8, so the
addresses are bindable as-is, and RootlessKit's builtin port driver
both listens on the requested parent address and dials the requested
child address verbatim. Port drivers that disallow loopback child IPs
(slirp4netns) are unaffected: their forced non-loopback childIP is
selected before the loopback fallback.
Signed-off-by: Andrew Liu <andrewjliu22@gmail.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>
Replace the setmatrix package's use of golang-set with a small local
map-backed set implementation. The package already serializes access with
its own mutex, so the external dependency was not buying us much and made
this small utility harder to keep self-contained.
Removing it trims the dependency surface and avoids carrying an otherwise
single-purpose module for a narrow internal use case.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.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>
When attempting to read a (malformed) resolv.conf with a very long line,
a obscure error would be produced that didn't provide much context to
identify the problem;
Handler for POST /v1.51/containers/mariadb11/start returned error: bufio.Scanner: token too long
This patch adds some additional error-handling to detect this situation,
and includes the filename of the resolv.conf to help the user locating
the file that failed to be parsed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
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>
The `ErrBackendNotSupported` error was no longer used since [moby@37cbdeb].
[moby@37cbdeb]: 37cbdeb1f2
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `BOLTDB` const and related `Backend` type are no longer used since
[moby@ed08486].
[moby@ed08486]: ed08486ec7
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With this tag, a dynamically linked binary will exec
the nft tool instead of using cgo to call libnftables
directly.
Signed-off-by: Rob Murray <rob.murray@docker.com>
On API v1.52 and newer, the GET /networks/{id} endpoint returns
statistics about the IPAM state for the subnets assigned to the network.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The uint128 type is very convenient for manipulating 128 bit-wide
quantities, as tends to come up in several contexts when working with
IPv6. Move it into a libnetwork/internal/ package so it can be reused
elsewhere within libnetwork.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Add methods to count the number of addresses in the set which have a
particular prefix. The returned counts are 128 bits wide to accommodate
sets containing more than 2**64 addresses.
Signed-off-by: Cory Snider <csnider@mirantis.com>
