Skip to content

[metal3] ironic: let dnsmasq use its NET_BIND_SERVICE file-cap (#256) - #257

Merged
diconico07 merged 2 commits into
suse-edge:mainfrom
ndreno:fix/ironic-dnsmasq-cap-net-bind-256
Aug 28, 2026
Merged

diconico07 merged 2 commits into
suse-edge:mainfrom
ndreno:fix/ironic-dnsmasq-cap-net-bind-256

Conversation

@ndreno

@ndreno ndreno commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor

What

Implements Option 1 from #256 (the direction @diconico07 favoured): keep the ironic-dnsmasq container non-root, and make the file-caps it's granted actually effective so it can bind DHCP (67) / TFTP (69).

Rebased on top of #254 (Metal3 0.15.1 / Ironic 0.13.1) and split into two commits as @hardys requested:

  1. Manual chart changes under packages/*
  2. Generated output from make charts && make html (charts/, assets/, index.*)

Two functional changes, both in the ironic subchart:

  1. values.yaml — dnsmasqSecurityContext:

    • allowPrivilegeEscalation: true — without no_new_privs off, the binary's file-caps never take effect, so dnsmasq can't bind privileged ports.
    • add NET_BIND_SERVICE to the capability set — it must be in the bounding set for the +e file-cap to apply.
    • Container stays runAsUser: 10475 / runAsNonRoot: true — smallest possible deviation from the current hardening.
  2. templates/deployment.yaml — merge order:

    -          {{- merge .Values.securityContext .Values.dnsmasqSecurityContext | toYaml | nindent 10 }}
    +          {{- merge .Values.dnsmasqSecurityContext .Values.securityContext | toYaml | nindent 10 }}

    merge favours the first dict, so the shared securityContext was silently overriding the per-container dnsmasqSecurityContext (allowPrivilegeEscalation: false always won). The per-container context must take precedence.

Rendered result (helm template, dnsmasq container) — non-root preserved:

securityContext:
  allowPrivilegeEscalation: true
  capabilities:
    add: [NET_ADMIN, NET_RAW, NET_BIND_SERVICE]
    drop: [ALL]
  runAsNonRoot: true
  seccompProfile:
    type: RuntimeDefault

Versions

Bumps ironic 0.13.1 → 0.13.2 and metal3 0.15.1 → 0.15.2 (metal3 appVersion stays 0.15.1 — this is a chart-only fix, no upstream app change); regenerated charts/, assets/, index.yaml, index.html via make charts + make html.

Dependency / sequencing

As @diconico07 noted in #256, this is inert until the downstream ironic image carries the setcap — it's already present upstream in metal3-io/ironic-image configure-nonroot.sh (setcap "cap_net_raw,cap_net_admin,cap_net_bind_service=+eip" /usr/sbin/dnsmasq) but wasn't propagated into the SLE rebuild. Happy to hold/rebase this until that lands.

Verified working on RKE2 + SELinux-enforcing during a Metal3 bare-metal PoC (dnsmasq bound DHCP/TFTP, a real node PXE-booted).

Closes #256

@hardys

hardys commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Hi @ndreno - thanks a lot for the PR!

Apologies but this needs a rebase because #254 from @diconico07 merged - when doing this I'd suggest splitting the PR into two commits:

  • The changes under packages/* e.g the manual changes to the chart
  • Then run make charts && make html and commit the resulting changes in charts/ assets/ and index.*

If you check recently merged PRs you will see a similar pattern - this makes review much easier, and also rebases when needed (you can just drop the "make charts" commit and run make again to generate it)

ndreno added 2 commits July 7, 2026 17:17
…edge#256)

The ironic-dnsmasq container runs non-root (runAsUser 10475) but must bind
privileged ports (DHCP 67, TFTP 69). The ironic image already grants the
needed file-caps:

  setcap cap_net_bind_service,cap_net_raw,cap_net_admin+eip /usr/sbin/dnsmasq

but file-caps are inert while no_new_privs is on, so dnsmasq still fails to
bind. Two fixes, both in the ironic subchart:

1. values.yaml: dnsmasqSecurityContext now sets allowPrivilegeEscalation:
   true (so the file-caps take effect) and adds NET_BIND_SERVICE to the
   bounding set (required for the +e file-cap to apply). Container stays
   non-root.

2. deployment.yaml: flip the `merge` argument order for the dnsmasq
   securityContext. `merge` favours the first dict, so the shared
   securityContext was overriding the per-container dnsmasqSecurityContext
   (allowPrivilegeEscalation:false always won). dnsmasqSecurityContext must
   take precedence.

Bumps ironic 0.13.1 -> 0.13.2 and metal3 0.15.1 -> 0.15.2.

NOTE: this is inert until the downstream ironic image carries the upstream
setcap (already present in metal3-io/ironic-image's configure-nonroot.sh).

Refs: suse-edge#256
Signed-off-by: Nicolas Dreno <ndreno@gmail.com>
Signed-off-by: Nicolas Dreno <ndreno@gmail.com>
@ndreno
ndreno force-pushed the fix/ironic-dnsmasq-cap-net-bind-256 branch from c2ae87d to 126d74f Compare July 7, 2026 15:21
@ndreno

ndreno commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @hardys! Rebased onto main (past #254) and split into two commits: the manual packages/* changes, then make charts && make html. Re-targeted versions to ironic 0.13.2 / metal3 0.15.2 since #254 already used 0.13.1/0.15.1.

@hardys
hardys requested a review from diconico07 July 14, 2026 08:19
@ndreno

ndreno commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

hey guys, any update?

@hardys

hardys commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

hey guys, any update?

Sorry for the slow follow up on this @ndreno - thanks a lot for the contribution!

It LGTM, I guess we still need to resolve this issue in the downstream image before merging?

NOTE: this is inert until the downstream ironic image carries the upstream
setcap (already present in metal3-io/ironic-image's configure-nonroot.sh).

@hardys

hardys commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Actually I see you fixed that in https://src.opensuse.org/suse-edge/Factory/commit/410d878f5d328dd4c769370573b0a1a7a7de5d9a11a3b6faaf07b8e2e800df8a (thanks!) but it's not yet been released to https://build.opensuse.org/project/show/isv:SUSE:Edge:Containers - I will speak with @diconico07 and we can get that updated so we can proceed with this PR, thanks for your patience!

@ndreno

ndreno commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, keep me posted please :)

@diconico07

Copy link
Copy Markdown
Collaborator

Unfortunately some automation bumped the Ironic RPM to v38 (used in our builds of Ironic image) before I published the updated image, so I'll merge this one but it will remain inert until I update the chart here to use Ironic image v38 (and publish final v38 image).
I'll do it as soon as I get confirmation from our QA team it's all good with v38 image

@diconico07
diconico07 merged commit c158c9a into suse-edge:main Aug 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

metal3: ironic-dnsmasq cannot bind DHCP socket as non-root (added NET_* caps not effective; no filecaps/ambient)

3 participants