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>
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>
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>
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>
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>
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>
This method is only used by the cnmallocator to allocate Swarm-scoped
network resources. Its only concrete implementation is in the ovmanager.
Other network drivers are implementing it too to adhere to the
driverapi.Driver interface, but they all return a 'not implemented'
error.
Extract this method into a separate interface, and add a dedicated
RegisterNetworkAllocator to the driver registry. Update the cnmallocator
to load 'network allocators' instead of 'drivers'.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
If unmarshaling the peer record fails, there is no need to check if it's
a record for a local peer. Attempting to do so anyway will result in a
nil-dereference panic. Don't do that.
The Windows overlay driver has a typo: prevPeer is being checked twice
for whether it was a local-peer record. Check prevPeer once and newPeer
once each, as intended.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The eventually-consistent nature of NetworkDB means we cannot depend on
events being received in the same order that they were sent. Nor can we
depend on receiving events for all intermediate states. It is possible
for a series of entry UPDATEs, or a DELETE followed by a CREATE with the
same key, to get coalesced into a single UPDATE event on the receiving
node. Watchers of NetworkDB tables therefore need to be prepared to
gracefully handle arbitrary UPDATEs of a key, including those where the
new value may have nothing in common with the previous value.
The overlay driver naively handled events for overlay_peer_table
assuming that an endpoint leave followed by a rejoin of the same
endpoint would always be expressed as a DELETE event followed by a
CREATE. It would handle a coalesced UPDATE as a CREATE, inserting a new
entry into peerDB without removing the old one. This would
have various side effects, such as having the "transient state" of
multiple entries in peerDB with the same peer IP never settle.
Update driverapi to pass both the previous and new value of a table
entry into the driver. Modify the overlay driver to handle an UPDATE by
removing the previous peer entry from peerDB then adding the new one.
Modify the Windows overlay driver to match.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Windows and Linux overlay driver instances are interoperable, working
from the same NetworkDB table for peer discovery. As both drivers
produce and consume serialized data through the table, they both need to
have a shared understanding of the shape and semantics of that data.
The Windows overlay driver contains a duplicate copy of the protobuf
definitions used for marshaling and unmarshaling the NetworkDB peer
entries for dubious reasons. It gives us the flexibility to have the
definitions diverge, which is only really useful for shooting ourselves
in the foot.
Make daemon/libnetwork/drivers/overlay the source of truth for the peer
record definitions and the name of the NetworkDB table for distributing
peer records.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The macAddr and ipmac types are generally useful within libnetwork. Move
them to a dedicated package and overhaul the API to be more like that of
the net/netip package.
Update the overlay driver to utilize these types, adapting to the new
API.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Overlay is the only driver which makes use of the EventNotify facility,
yet all other driver implementations are forced to provide a stub
implementation. Move the EventNotify and DecodeTableEntry methods into a
new optional TableWatcher interface and remove the stubs from all the
other drivers.
Signed-off-by: Cory Snider <csnider@mirantis.com>
