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>
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>
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>
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>
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>
When generating the rules for an nftables chain, rather than collecting
rules into a slice and iterating over that, use an iterator.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Add nftables.Modifier, to hold a queue of commands that can be applied
using Modifier.Apply. No updates are made to the underlying Table
until Apply is called, errors in the queue if commands are deferred
until Apply.
This has the advantages that:
- less error handling is needed in code that generates update commands
- it's transactional, without needing explicit transactions
Minor disadvantages are that it's slightly more difficult to debug updates,
as it's no longer possible to step through the call making an update to
the Table manipulation in a debugger - and errors in the command, and
errors like trying to update a nonexistent chain/set/vmap, deleting an
object that doesn't exist or creating a duplicate are not reported
until the updates are applied (but, the file/line where the rule was
added is reported).
Signed-off-by: Rob Murray <rob.murray@docker.com>
