Skip to content

selinux-policy: complete container log reader support - #59

Merged
SeanDougherty merged 4 commits into
aclmainfrom
sdougherty/fix-container-logreader-policy
Aug 28, 2026
Merged

selinux-policy: complete container log reader support#59
SeanDougherty merged 4 commits into
aclmainfrom
sdougherty/fix-container-logreader-policy

Conversation

@SeanDougherty

@SeanDougherty SeanDougherty commented Aug 21, 2026

Copy link
Copy Markdown

Summary

Complete the confined container_logreader_t support added in #56: retain the cross-distro audit-read semantics established by containers/container-selinux, add read-only mmap access required by systemd journal readers, and document safe mount and MCS behavior.

Change Log

  • Add read-only systemd_read_journal_files() access so journalctl --file can map persistent journals; no journal write or watch permission is added.
  • Keep logging_read_audit_log(container_logreader_t) to match Fedora/RHEL/OpenSUSE container-selinux behavior and avoid pushing compatible collectors to spc_t.
  • Bump selinux-policy once from 2.20250213-9 to 2.20250213-10.
  • Document log-reader boundaries, MCS behavior, journald audit content, and separate opt-in mounts for sensitive auditd files.

Type of Change

  • Image build change (base image, sysexts, OEM images)
  • Package/SPEC update
  • CI/automation change
  • SDK/toolchain update
  • Configuration change
  • Documentation update
  • Bug fix

Does this affect the image build?

  • Yes
  • No

Associated Issues

Test Methodology

  • Compiled the targeted MCS base policy and all modules with the final patch series.
  • Built a fresh selinux-policy-2.20250213-10.azl3.noarch.rpm.
  • Inspected packaged CIL: auditd logs have read/list only; systemd journals have read/open/map and directory read/search only; neither type has write/append/create/delete/watch access.
  • Confirmed container_logreader_t remains mcs_constrained_type.
  • Live-tested a normally categorized log-reader process on the published ACL image and confirmed host-log reads succeed while writes fail.
  • Verified stock ACL stores audit transport records in the persistent journal and has no running auditd.
  • Compared semantics with authoritative containers/container-selinux, which includes logging_read_audit_log(container_logreader_t).
  • Independent final code review found no significant issues.

Merge Checklist

  • Image builds successfully with this change (or image build is not affected)
  • Any updated packages/SPECs build successfully
  • Relevant kola tests pass
  • All package sources are available
  • Source files have up-to-date hashes/manifests
  • Documentation has been updated to match any changes
  • Ready to merge

Azure Boards: AB#23415

Automated validation follow-up: AB#23492

@SeanDougherty
SeanDougherty requested a review from a team as a code owner August 21, 2026 01:19
Copilot AI lite review requested due to automatic review settings August 21, 2026 01:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Narrows the SELinux container_logreader_t domain so it no longer has access to host audit logs, and adds documentation describing ACL’s container SELinux domain choices and safe configuration patterns.

Changes:

  • Bumps selinux-policy release to 2.20250213-10 and adds patch 0061 to remove container_logreader_t audit-log access.
  • Adds a new SELinux container-domain guide (acl/docs/selinux.md) and links it from the docs index and architecture guide.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
acl/SPECS/selinux-policy/selinux-policy.spec Bumps RPM release and wires in the new refpolicy patch removing audit-log read for container_logreader_t.
acl/SPECS/selinux-policy/0061-container-Drop-audit-log-access-from-log-reader.patch Drops logging_read_audit_log(container_logreader_t) from the container module policy.
acl/docs/selinux.md Adds guidance on SELinux container workload domains, log-reader boundaries, MCS isolation, and usage examples.
acl/docs/README.md Adds the SELinux doc to the documentation index.
acl/docs/architecture.md Links to the new SELinux container domains guide from the architecture doc.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread acl/docs/selinux.md
Comment thread acl/docs/selinux.md Outdated
Comment thread acl/docs/selinux.md Outdated
Comment thread acl/docs/selinux.md
Comment thread acl/docs/selinux.md

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

acl/docs/selinux.md:112

  • In the validation snippet, the single quotes around the --file path prevent shell glob expansion (*), so journalctl will receive a literal * and the command will typically fail to open the journal file. Use an unquoted glob (or otherwise expand it) so the machine-id directory is matched.
journalctl --file='/host/var/log/journal/*/system.journal' -n 5

@SeanDougherty

Copy link
Copy Markdown
Author

Opened stacked docs-only follow-up #60 with the comprehensive customer-facing SELinux container-domain reference. It inventories all five compiled workload domains, distinguishes internal engine/file types, compares privilege and MCS boundaries, and guides teams toward narrow domains instead of blanket spc_t. Retarget #60 to aclmain after this PR merges.

Comment thread acl/SPECS/selinux-policy/selinux-policy.spec Outdated
Comment thread acl/docs/selinux.md
Copilot AI review requested due to automatic review settings August 21, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread acl/SPECS/selinux-policy/selinux-policy.spec
Comment thread acl/SPECS/selinux-policy/selinux-policy.spec
Comment thread acl/docs/selinux.md
@SeanDougherty SeanDougherty changed the title selinux-policy: narrow container log reader access selinux-policy: complete container log reader support Aug 21, 2026
@SeanDougherty

Copy link
Copy Markdown
Author

CRI SELinux validation

The exact ACL gallery image used for validation (acldevel-gh-amd64/0.20260819.1185504) runs SELinux enforcing and starts containerd with /usr/share/containerd/config.toml.

  • crictl info reports .config.enableSelinux = true.
  • A CRI container created with linux.security_context.selinux_options.type = container_logreader_t received system_u:system_r:container_logreader_t:s0:c865,c992 in both the OCI runtime spec and /proc/<pid>/attr/current.
  • That CRI-created container successfully read a read-only bind mount labeled var_log_t.

The earlier focused test manually supplied an OCI label through ctr, so by itself it proved policy enforcement rather than the CRI path. The additional crictl test now covers the actual CRI SELinuxOptions path.

The reviewer is also right that this is not a universal AKS default: generic AgentBaker containerd templates omit enable_selinux. ACL differs because its embedded containerd sysext forces the setting on and its service uses that /usr/share config. PR #60 now documents CRI SELinux as a prerequisite and includes an effective-setting check for customer nodes.

@jiria

Jiri Appl (jiria) commented Aug 27, 2026

Copy link
Copy Markdown
Member

Could we add some basic validation into a smoke test or preferably a kola test? Fine for a follow up, but if not right away, please create a task to track.

@SeanDougherty

Copy link
Copy Markdown
Author

Tracking the requested automated smoke/Kola coverage in AB#23492. The unassigned Cyc5 task covers the real containerd CRI enableSelinux path, runtime type/MCS verification, positive host-log and journal reads, negative writes, an ordinary-container_t control, AVC diagnostics, and integration into automated ACL image validation.

SeanDougherty and others added 3 commits August 27, 2026 12:05
Remove access to auditd_log_t from container_logreader_t because audit logs contain host-wide security events outside the general log collector use case. Document specialized container domains, selection examples, MCS behavior, and audit-log exclusion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Sean Dougherty <sdougherty@microsoft.com>
Document the actual watch and audit boundaries on ACL, replace validation commands with stock journald equivalents, and grant read-only mmap access required by journalctl for persistent systemd journals.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Sean Dougherty <sdougherty@microsoft.com>
Retain the audit-log read semantics established by container-selinux so compatible collectors do not need spc_t, collapse the package to one release bump, and make sensitive audit mounts explicit. Keep the read-only journal mmap addition.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Sean Dougherty <sdougherty@microsoft.com>
Copilot AI review requested due to automatic review settings August 27, 2026 19:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread acl/docs/selinux.md Outdated
Comment thread acl/docs/selinux.md
State that ACL enables enforcing mode by default while allowing permissive troubleshooting, and direct operators to getenforce for the effective node state.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Sean Dougherty <sdougherty@microsoft.com>
Copilot AI review requested due to automatic review settings August 27, 2026 19:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

acl/docs/selinux.md:15

  • Grammar: “an SELinux” is incorrect here because “SELinux” is typically pronounced with an initial consonant sound (“sell…”). Use “a SELinux”.
| `container_logreader_t` | Opt-in log collector | Read-only access to objects labeled with an SELinux log type |

@jiria Jiri Appl (jiria) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the policy change against upstream SELinuxProject/refpolicy at tag RELEASE_2_20250213. The patch is correct, minimal, and does exactly what the commit message claims — approving.

Verification notes:

  • systemd_read_journal_files() calls mmap_read_files_pattern, and mmap_read_file_perms = { getattr open map read ioctl }, so map is genuinely granted. Read/list on systemd_journal_t was already covered attribute-wide by 0060's logging_read_all_logs (systemd.te calls logging_log_file(systemd_journal_t)), so this patch is strictly additive for map. No write, no watch, no over-grant.
  • The optional_policy() wrapper and its placement match how container.te already wraps its other systemd interface calls.
  • Retaining logging_read_audit_log() is not redundant, as the description says: auditd_log_t is declared with files_security_file() and carries no logfile attribute, so logging_read_all_logs doesn't reach it.
  • Spec hygiene checks out: single Release bump, one changelog entry, weekdays correct, and %autosetup -p1 means no %patch -P 61 line is needed.
  • All four documented domains exist in refpolicy container.te, including the spc_t "may be unconfined" caveat (unconfined_domain_noaudit(spc_t)).
  • journalctl --file='...*/system.journal' — the quoting is correct, journalctl glob-expands --file= itself.

I've also retracted my earlier mls_systemhigh comment above; the doc's MCS section is right and I was wrong.

The four comments below are all documentation suggestions, none blocking. Three of them share a theme worth noting: there are several ways a collector can fail here with no AVC (dontaudit'd map on audit files, DAC on the journal), while the doc's troubleshooting step looks only for AVCs. Happy for those to be follow-ups.

Nice work — the policy delta is exactly as small as it should be.

Comment thread acl/docs/selinux.md
Comment thread acl/docs/selinux.md
Comment thread acl/docs/selinux.md
Comment thread acl/docs/selinux.md
@SeanDougherty
SeanDougherty merged commit eabced6 into aclmain Aug 28, 2026
23 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.

4 participants