Repository navigation
security(ansible): an unresolved network_subnet silently widens five firewall rules to any source #17642
Description
An unresolved network_subnet means "allow from anywhere", silently, in five firewall rules
Settled from the installed module source, not by applying a rule. Nothing was
modified and no firewall was touched.
This came out of scoping #17500. It is a separate defect: it spans two roles and
is not specific to the cache.
The mechanism
community.general 9.5.2, plugins/modules/ufw.py:405:
def execute(cmd, ignore_error=False):
cmd = ' '.join(map(itemgetter(-1), filter(itemgetter(0), cmd)))and the fragment that carries the source restriction, at :568-571:
for (key, template) in [('from_ip', "from %s"), ('from_port', "port %s"), ...]:
value = params[key]
cmd.append([value, template % (value)])filter(itemgetter(0), cmd) keeps a fragment only when its value is truthy.
So when from_ip is the empty string, the pair is ['', 'from '], the filter
drops it, and the from clause is not emitted at all. The command becomes
ufw allow port <port> proto tcp comment '<comment>'
which is valid, returns rc 0, and — because ufw applies no source restriction
when none is given — allows from anywhere. No error, no warning, and the task
reports its normal changed/ok.
Note the module does declare from_ip=dict(type='str', default='any', ...) at
:354. That default is a red herring here: it only applies when the parameter is
omitted, and these tasks pass it explicitly. The widening comes from the drop, not
the default. Either way the destination is the same — anywhere.
Every definition of the variable already means "anywhere" when it does not resolve
Eight definitions, two fallback families, one outcome:
| Fallback | Where |
|---|---|
or '' |
inventory/hosts.yml:14 · roles/redis/defaults/main.yml:7 · roles/browser/defaults/main.yml:30 · roles/frontend/defaults/main.yml:7 |
or '0.0.0.0/0' |
inventory/group_vars/infrastructure.yml:30 · inventory/hosts-router.yml:16 · inventory/production.yml:21 · inventory/slm-nodes.yml:29 |
The 0.0.0.0/0 family is at least honest — it says "the whole internet" in the
repo, where a reader can see it. The '' family arrives at the identical rule
without saying anything.
And the file that calls itself canonical is in the second group.
group_vars/infrastructure.yml:27-30 reads:
# Canonical network configuration - referenced by all group_vars.
# ... every install detects its own subnet automatically.
network_subnet: "{{ ... or '0.0.0.0/0' }}"
"Detects its own subnet automatically" is true when detection works. Its
unresolved state is the entire internet.
The five rules that consume it, none of them guarded
| Rule | Location |
|---|---|
| cache port | roles/redis/tasks/main.yml:180 |
| cache inspector port | roles/redis/tasks/main.yml:191 |
| browser rules ×3 | roles/browser/tasks/main.yml:347, :361, :374 |
No when: on any of them, and nothing asserts the variable is non-empty before it
is used as a source restriction. roles/frontend/defaults/main.yml:7 defines the
variable too but I found no from_ip use site in that role.
Two from_ip sites are not affected and are worth naming so a fix does not
churn them: roles/common/tasks/main.yml:145 passes a literal loopback address,
and :161 uses item.src | default('any') — an explicit, deliberate "any".
Latent, not currently active — stated precisely
I am not claiming the deployment is open right now. #17635's own read-only
measurement records the cache as allowed from a routable /24, which means
network_subnet did resolve on that node. So this is a reachable latent path: a
host where neither the environment variable nor the install-time secrets file
resolves gets wide-open rules, and nothing reports that it happened. The rules
look identical in the playbook output either way.
Why it got this way — and why the earlier fixes are the cause
This variable has been fixed twice for the opposite symptom:
- fix(ansible): network_subnet used without default in bootstrap-node.yml — fails on fresh single-host installs #3301 — "
network_subnetused without default … fails on fresh single-host
installs". Its body says the empty value "renders as an empty string or causes
an Ansible undefined variable error, breaking firewall rule setup". - fix(install.sh): generated localhost.yml missing network_subnet/network_gateway → firewall merge fails on 00-SLM-Manager #7162 — "generated inventory missing
network_subnet/network_gateway→
firewall merge fails".
Both were closed as completed, and both were fixed by making the variable
resolve — adding the fallbacks above. That removed a loud failure and
installed a silent widening in its place. Nobody asked what the fallback means
when the consumer is a firewall source restriction, because the bug being fixed
was "the play stops", not "the rule is wrong".
That is the shape #13264 tracks: a migration that made a config default silently
wrong, fixed one site at a time.
What the fix has to do
One fix for the class, not five patches:
- Fail loudly instead of widening. A firewall source restriction that cannot
be resolved must stop the play, not fall back. That means dropping every
or ''/or '0.0.0.0/0'fallback on this variable and replacing it with an
assertion at the point of use, or amandatoryfilter at the definition. - If a genuinely-open rule is ever intended, it says so explicitly at the task
— the wayroles/common/tasks/main.yml:161already does with
default('any')— never by a variable that failed to resolve. - Evidence must be a refusal: a play run with the variable unresolved stops
with a named error. "The rules looked right" is exactly the observation that
holds today while the defect is present.
Worth checking in the same pass: three further from_ip sites derive the source
from a URL via regex_replace (roles/redis/tasks/redis_exporter.yml:92,
roles/slm_agent/tasks/main.yml:420, roles/slm_agent/tasks/node_exporter.yml:183).
Those have non-empty defaults, and on a non-matching input regex_replace returns
the subject unchanged, so they would hand ufw an invalid address and fail —
the better outcome. I did not verify that end to end and am not claiming it; it
just means they are likely not part of this defect.
Activity
mrveiss commentedon Sep 28, 2026
Chunking triage — a proposal, not an assignment
priority: critical— not applied. Setting it is the milestone owner's call.network_subnetsilently widens five firewall rules to any sourceorigin/mainsince 2026-08-14 references this issue.Nothing was relabelled, moved or closed by this pass.
mrveiss commentedon Sep 28, 2026
Chunk triage —
v0.9.2 node plane and deploy. Read-only atorigin/main7adaca8c29; nothing labelled or moved.Premise verified, and it is worse in the specific way that makes it easy to miss:
network_subnetis not undefined. It is defined as an env lookup with no final default.autobot-slm-backend/ansible/inventory/group_vars/infrastructure.yml:30(and the same expression inhosts.yml:14,hosts-router.yml:16,production.yml:21,slm-nodes.yml:29):Two fallbacks and no third. Compare the line four below it —
dns_nameserversendsor '8.8.8.8', a real default. So when the env var is unset and the grep finds nothing, this yields the empty string rather than failing.Two consequences worth stating separately:
lookup('pipe', …)runs on the controller, not the node. Any controller without/etc/autobot/slm-secrets.env— a dev machine, a CI runner, a freshly imaged manager — silently produces empty. That is the same controller-versus-target confusion as thecode_sourceclass (CRITICAL(deploy): backend provisioning aborts on every non-manager host — a task executes a script out of code_source, which exists only on the controller (rc=127) #17242/deploy: five more tasks read code_source on hosts that do not have it — three are in the builtin updater (#17242 class) #17243), in a different guise.network_subnet:finds five definitions, so the obvious check says the variable exists. Only reading the expression shows that "exists" and "resolves" are different questions.ansible/— six inaiml.yml, eight inbackend.yml, seven inbrowser.yml, plus database/frontend/slm group_vars,bootstrap-node.yml, and the redis/browser/xrdp roles. The five ufw rules this issue names are the exposure; the rest is blast radius if the value is wrong rather than empty.priority: criticalthat is the first gap to close, and the shape is not in doubt: fail closed when the value is empty (anassertin the role or a| mandatory), give the lookup a final default only if one is genuinely safe (it is not — there is no safe default subnet), and add a guard that no ufw rule'ssrccan be a bare unvalidated variable.mrveiss commentedon Sep 28, 2026
Proposed acceptance criteria — a proposal for the owner to accept, amend or reject, not a ruling. Read-only; nothing changed.
This issue is
priority: criticalwith no acceptance criteria, so there is nothing for a closer to tick and nothing for an implementer to aim at. Below is what I would write, with the reasoning attached so each one can be argued with separately.The constraint that shapes all of them
There is no safe default subnet.
dns_nameserversfour lines below can endor '8.8.8.8'because a wrong DNS server is a broken lookup; a wrong — or empty —srcon a ufw rule is an open port. So the fix cannot be "add a default"; it has to be "refuse to proceed". That is why the criteria below are fail-closed rather than fall-back.Proposed
network_subnetfails the play, before any ufw task runs. Anassertin the role (or| mandatoryon the lookup) whose message names the variable, the inventory file that defines it, andNETWORK_SUBNET//etc/autobot/slm-secrets.envas the two sources — so the operator reads what to set, not that a task failed.srcmay be a bare unvalidated variable. A guard overautobot-slm-backend/ansible/fails when aufw:task'ssrcis a template expression that has not passed the validation above. This is the criterion that stops the next instance, and it is the reason the others are not enough on their own.network_subnetempty and asserts the play fails rather than emitting a rule without afromclause. Per#15826's remedy: a planted empty value must be caught, and a valid subnet must pass — both directions, or the guard is untested.lookup('pipe', 'grep … /etc/autobot/slm-secrets.env')runs on the controller, not the target; the inventory comment should say so, because "detected at install time" reads as per-node detection and is not. A dev machine, a CI runner or a freshly imaged manager has no such file.NETWORK_SUBNET=yesis non-empty and is not a subnet;Acquire-style truthiness is what produced this class in the first place.Deliberately not proposed
No host verification criterion. Everything above is decidable in the repository, and the issue was settled from the installed module source without touching a firewall — adding a "verified on a live node" criterion would move a closable critical into
v0.9-hostfor no gain.No baseline or grandfather list. Five rules is small enough to fix outright; a shrink-only baseline here would record an exposure rather than remove it.
One note on the class
This is the third instance today of the same confusion in a different guise — a controller-side value assumed to exist target-side (#17242's
code_sourceexecution, #17331's baked rewrite path, this). The first two were rc=127 and a missing constraints file; this one is a firewall rule that silently widens. The failure gets quieter as the value gets more important, which is an argument for the guard in criterion 2 over any amount of care at the five call sites.mrveiss commentedon Sep 28, 2026
B5 spec — third in the batch, the long pole, and it carries the guard the other two reuse
Batch B5. Full spec at
~/b5-spec-2026-09-28.md. The proposed acceptance criteria are in my earlier comment; this is the build spec.The constraint that shapes every change: there is no safe default subnet. A wrong DNS server is a broken lookup; an empty
srcis an open port. So everything here is fail-closed, never fall-back — which is whydns_nameserverscan endor '8.8.8.8'four lines below andnetwork_subnetcannot.Changes, in order: (1) an
assert/| mandatorybefore any ufw task, namingnetwork_subnet,NETWORK_SUBNETand/etc/autobot/slm-secrets.envso the operator reads what to set; (2) validate as a CIDR, not as non-empty —NETWORK_SUBNET=yesis non-empty and is not a subnet; (3) state in the inventory comment thatlookup('pipe', …)runs on the controller — "detected at install time" currently reads as per-node detection and is not.Test file:
repo_tests/ansible_ufw_src_is_validated_17642_test.py(new).Assertion: no
ufw:task'ssrcis an unvalidated template expression; a rendered rule with an emptysrcfails the play rather than emitting a rule with nofromclause.Mutation: render the five rules with
network_subnetempty → the play must fail. Negative control: a valid CIDR must pass. Plus a reach floor — finding no ufw tasks at all must fail, not pass by matching nothing.One scale note for the reviewer: the name is referenced across 21 files in
ansible/. The five ufw rules are the exposure; the rest is blast radius if the value is wrong rather than empty. Triage those separately — only the first is a security finding.