fix(filter): inspect libpod network writes - #385
Draft
scttbnsn wants to merge 8 commits into
Draft
Conversation
scttbnsn
commented
Aug 29, 2026
scttbnsn
commented
Contributor
- fix(filter): gate POST /libpod/images/pull on the image-pull allowlist
- fix(filter): gate libpod network connect/disconnect on libpod_network
- fix(filter): gate libpod network DNS resolvers on allow_dns_servers
- fix(config): check endpoint_config exclusivity on libpod_network too
matchesLibpodNetworkInspection matched only POST /libpod/networks/create, so
request_body.libpod_network's allow_endpoint_config, endpoint_config.* and
allow_disconnect_force were enforced on the Docker-compat connect/disconnect
paths and silently not enforced on the libpod spelling of the same
operations, with no startup warning when a rule opened them. A client
speaking libpod to the same Podman socket could pin a static IP or a static
MAC onto a container, attach per-endpoint driver options, or force-disconnect
a container, under an operator policy that denied every one of those over the
Docker-compat path. stripVersionPrefix consumes only /vN[.N[.N]]/, so
/v5.0/libpod/... was open too.
The middleware entry and the inspector now share isLibpodNetworkWritePath, so
the routing table cannot admit a narrower set of paths than the inspector
handles, and both spellings run through the one denyEndpointConfigReason /
denyDisconnectReason gate rather than a second copy.
The two libpod routes are not the same kind of endpoint, verified against
Podman v5.8.1's pkg/api/server/register_networks.go. Connect is served by
libpod.Connect, which decodes entities.NetworkConnectOptions: `container`
plus an EMBEDDED, untagged libnetwork/types.PerNetworkOptions
(go.podman.io/common v0.67.0), so the endpoint fields are top-level and
snake_case (static_ips, static_mac, aliases, options) and Docker's
{"EndpointConfig":{...}} never appears on the wire — reading the Docker
spelling here would have found nothing and allowed every connect. Those four
fields are projected onto Docker's EndpointSettings shape, the inverse of
what Podman's own compat.Connect does when it lowers EndpointConfig into
PerNetworkOptions, so allow_static_addressing, allow_mac_pinning,
allow_aliases and the fail-closed DriverOpts rule all apply unchanged.
static_mac is checked as a raw string rather than as a MAC, because
types.HardwareAddr.UnmarshalJSON falls back to a plain []byte decode when
net.ParseMAC fails, so a base64 blob pins a MAC exactly as a colon-separated
one does. Disconnect is the opposite case: Podman registers it directly on
the Docker-compat compat.Disconnect handler, so the body really is
docker/api/types/network.DisconnectOptions and networkDisconnectRequest is
reused verbatim.
interface_name is deliberately not gated: it names an interface inside the
container's own netns, has no Docker EndpointSettings analog for the shared
gate to reuse, and carries no host-side privilege. allow_link_local_ips and
allow_gw_priority have no libpod analog and never fire on this path.
No shipped preset allows either libpod path, so the exposure was reachable
only through an operator-authored allow rule.
POST /libpod/networks/{name}/update had no request-body inspector and was
absent from the body-sensitive write catalog, so an operator who allowed it
got no gate and no startup warning. The primitive is redirection with a
retroactive blast radius: ic.NetworkUpdate rewrites the resolver list on an
EXISTING network, so every container already attached starts resolving names
through whatever the caller supplied, including containers the caller neither
owns nor created. That reaches further than anything allow_endpoint_config
gates, which only ever affects the one endpoint being attached.
It is inspected rather than parked behind insecure_allow_body_blind_writes.
The body is two string arrays, so the "sockguard cannot model this shape"
argument that justifies the acknowledgment for play/kube does not apply, and
an acknowledgment where a decode struct suffices is the weaker control.
Nothing legitimate breaks: the Engine API has no network-update route at any
version so no Docker-compat client can reach it, and none of the tools
sockguard ships presets for issues it — it is a `podman network update` admin
operation, so a fail-closed default costs nothing operationally.
RemoveDNSServers is gated alongside AddDNSServers rather than treated as the
benign direction: dropping the entry that was answering a name falls
resolution through to whatever is next, and both directions mutate state
shared with containers the caller does not own. A body carrying neither field
is allowed, since Podman applies two nil slices and leaves the network as it
was.
The new knob also gates network_dns_servers on POST /libpod/networks/create,
which sets the identical per-network resolvers and was equally ungated.
Shipping allow_dns_servers covering only update would have named a surface it
governs half of, which is the same drift this branch exists to close.
The wire spellings are the trap and are read from the right types, verified
against Podman v5.8.1. libpod.UpdateNetwork json.Decodes
entities.NetworkUpdateOptions, tagged `adddnsservers`/`removednsservers` —
run together, no underscores. The snake_case `add_dns_servers` pair belongs to
libnetwork/types.NetworkUpdateOptions, which abi.NetworkUpdate copies into
AFTER the decode and which never touches a request. encoding/json's fallback
is case-insensitive but not separator-insensitive, so a gate written against
the snake_case spelling would have matched nothing and allowed every change.
Create is the other spelling again: it decodes types.Network, whose field is
snake_case `network_dns_servers`.
Routing needed no new path list — update slots into the isLibpodNetworkWritePath
predicate the middleware and inspector already share.
No shipped preset allows either path.
request_body.network rejects allow_endpoint_config: true combined with an explicit endpoint_config block, because the whole-object flag already admits every EndpointSettings field and a simultaneous granular block is ambiguous. request_body.libpod_network carries the identical block and was exempt, originally on the belief that libpod had no network-connect endpoint to gate. It does. An operator who wrote both keys there got silence where the Docker spelling gave them an error, which teaches someone their config means something it does not. Both groups are now checked, from one message template so the two cannot drift into differently-worded advice for the identical mistake. The provenance pass that distinguishes "the operator wrote this block" from "this block defaulted" — needed because endpoint_config.allow_aliases defaults to true, so the merged struct is never the Go zero value — grew a second flag and now expands one shared list of leaf keys per group instead of a hand-maintained list per group. That factoring is the point: a per-group list is how libpod_network came to be exempt, and a future granular field now lands on both groups at once. Both groups are answered from a single Viper pass, so the config file is still read once. Also corrects the third and last instance of the stale "endpoint_config has no libpod analog" claim, in EndpointConfigRequestBodyConfig's doc comment and in network_endpoint_config_test.go's header. The first two were fixed when the libpod connect inspector landed. Configs setting only one of the two keys under either group are unaffected.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
scttbnsn
changed the title
…k-connect-inspection # Conflicts: # CHANGELOG.md # docs/content/docs/configuration.mdx # docs/content/docs/podman.mdx
|
Deployment failed for project sockguard-website with the following error: Learn More: https://vercel.com/codeswhat?upgradeToPro=build-rate-limit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.