fix: ensure log file permissions and ownership for CIS 6.1.3.1/6.1.4.1#8299
fix: ensure log file permissions and ownership for CIS 6.1.3.1/6.1.4.1#8299
Conversation
djsly
requested review from
AbelHu,
Devinwong,
SriHarsha001,
awesomenix,
calvin197,
cameronmeissner,
ganeshkumarashok,
junjiezhang1997,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
sulixu,
surajssd,
timmy-wright and
zachary-bailey
as code owners
There was a problem hiding this comment.
Pull request overview
This PR addresses a CIS benchmark regression on Ubuntu 22.04/24.04 VHD builds by tightening /var/log permissions and group ownership so CIS “Ensure access to all logfiles has been configured” passes again.
Changes:
- Update
assignFilePermissions()to also remediate/var/logdirectory permissions and group ownership, not just “other-read”. - Add pre-scan remediation in the CIS scanning VM script to align
/var/logpermissions/ownership before running cisassessor.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
vhdbuilder/packer/cis-report.sh |
Pre-CIS-scan fixups on /var/log perms and group ownership in the scanning VM. |
parts/linux/cloud-init/artifacts/cis.sh |
Enhance CIS hardening to remediate /var/log file/dir perms and group ownership for the CIS logfiles rule. |
| find /var/log -type f ! -group root ! -group adm ! -group syslog -exec chgrp syslog {} \; | ||
| find /var/log -type d ! -group root ! -group adm ! -group syslog -exec chgrp syslog {} \; |
There was a problem hiding this comment.
These find ... -group adm ... -group syslog predicates run as part of assignFilePermissions, which is executed for Mariner/Azure Linux/Flatcar as well (before applyCIS returns early). On distros where adm or syslog groups don’t exist, find will error with “unknown group” and (under bash -e) fail the VHD build. Please gate this Ubuntu-specific group-ownership logic on Ubuntu only, or dynamically build the allowed-group predicate based on getent group checks.
| find /var/log -type f ! -group root ! -group adm ! -group syslog -exec chgrp syslog {} \; | |
| find /var/log -type d ! -group root ! -group adm ! -group syslog -exec chgrp syslog {} \; | |
| local allowed_log_groups="" | |
| local target_log_group="root" | |
| if getent group adm >/dev/null 2>&1; then | |
| allowed_log_groups="${allowed_log_groups} ! -group adm" | |
| target_log_group="adm" | |
| fi | |
| if getent group syslog >/dev/null 2>&1; then | |
| allowed_log_groups="${allowed_log_groups} ! -group syslog" | |
| target_log_group="syslog" | |
| fi | |
| # shellcheck disable=SC2086 | |
| find /var/log -type f ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} \; | |
| # shellcheck disable=SC2086 | |
| find /var/log -type d ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} \; |
There was a problem hiding this comment.
Fixed in 75dd5da — now dynamically building the allowed-group predicate using getent group checks, so Mariner/Flatcar won't hit 'unknown group' errors.
| # CIS 6.1.3.1 (22.04) / 6.1.4.1 (24.04): Ensure access to all logfiles has been configured. | ||
| # The rule requires: file perms at most 0640, dir perms at most 0750, and group ownership | ||
| # must be root, adm, or syslog. Upstream package updates (via apt dist-upgrade --force-confnew) | ||
| # can create log files with wrong permissions or ownership, so we fix comprehensively here. | ||
| find /var/log -type f -perm /0137 -exec chmod 'g-wx,o-rwx' {} \; | ||
| find /var/log -type d -perm /0027 -exec chmod 'g-w,o-rwx' {} \; | ||
| find /var/log -type f ! -group root ! -group adm ! -group syslog -exec chgrp syslog {} \; | ||
| find /var/log -type d ! -group root ! -group adm ! -group syslog -exec chgrp syslog {} \; |
There was a problem hiding this comment.
This change updates a script under parts/, which (per repo contributing guidance) generally requires running the generation step so embedded artifacts and snapshot testdata stay in sync. If this PR doesn’t include regenerated outputs, please run the appropriate make/make generate target and commit the updated generated files.
There was a problem hiding this comment.
Ran make generate (specifically GENERATE_TEST_DATA=true go test ./pkg/agent...) — no generated files changed. cis.sh doesn't appear to be embedded in snapshot testdata.
| # shellcheck disable=SC2086 | ||
| find /var/log -type f ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + | ||
| # shellcheck disable=SC2086 | ||
| find /var/log -type d ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + |
There was a problem hiding this comment.
Changing group ownership for all /var/log files/dirs that are not in {root,adm,syslog} can break services that rely on a service-specific group for log write access (especially root-owned logs where group write is used by the daemon). To reduce the risk of runtime regressions, consider narrowing the chgrp scope (e.g., only root-owned files/dirs, or only known problematic paths), or preserving group ownership when the owning user is not root.
| # shellcheck disable=SC2086 | |
| find /var/log -type f ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + | |
| # shellcheck disable=SC2086 | |
| find /var/log -type d ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + | |
| # Only normalize group ownership for root-owned log paths to avoid breaking | |
| # services that intentionally use a service-specific group for log access. | |
| # shellcheck disable=SC2086 | |
| find /var/log -type f -user root ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + | |
| # shellcheck disable=SC2086 | |
| find /var/log -type d -user root ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + |
| local allowed_log_groups="" | ||
| local target_log_group="root" | ||
| if getent group adm >/dev/null 2>&1; then | ||
| allowed_log_groups="${allowed_log_groups} ! -group adm" | ||
| target_log_group="adm" | ||
| fi | ||
| if getent group syslog >/dev/null 2>&1; then | ||
| allowed_log_groups="${allowed_log_groups} ! -group syslog" | ||
| target_log_group="syslog" | ||
| fi | ||
|
|
||
| # shellcheck disable=SC2086 | ||
| find /var/log -type f ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + | ||
| # shellcheck disable=SC2086 | ||
| find /var/log -type d ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + |
There was a problem hiding this comment.
Building allowed_log_groups as a string that is later unquoted-expanded into find requires disabling ShellCheck and is brittle (whitespace/word-splitting dependent). A more maintainable approach is to use a shell array of predicates (or explicit find parentheses with -o) to avoid SC2086 and make the predicate logic clearer.
| local allowed_log_groups="" | |
| local target_log_group="root" | |
| if getent group adm >/dev/null 2>&1; then | |
| allowed_log_groups="${allowed_log_groups} ! -group adm" | |
| target_log_group="adm" | |
| fi | |
| if getent group syslog >/dev/null 2>&1; then | |
| allowed_log_groups="${allowed_log_groups} ! -group syslog" | |
| target_log_group="syslog" | |
| fi | |
| # shellcheck disable=SC2086 | |
| find /var/log -type f ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + | |
| # shellcheck disable=SC2086 | |
| find /var/log -type d ! -group root ${allowed_log_groups} -exec chgrp "${target_log_group}" {} + | |
| local -a allowed_log_group_args=() | |
| local target_log_group="root" | |
| if getent group adm >/dev/null 2>&1; then | |
| allowed_log_group_args+=(! -group adm) | |
| target_log_group="adm" | |
| fi | |
| if getent group syslog >/dev/null 2>&1; then | |
| allowed_log_group_args+=(! -group syslog) | |
| target_log_group="syslog" | |
| fi | |
| find /var/log -type f ! -group root "${allowed_log_group_args[@]}" -exec chgrp "${target_log_group}" {} + | |
| find /var/log -type d ! -group root "${allowed_log_group_args[@]}" -exec chgrp "${target_log_group}" {} + |
CIS rule 'Ensure access to all logfiles has been configured' (6.1.3.1 on Ubuntu 22.04, 6.1.4.1 on 24.04) regressed to fail on both VHD builds. The existing fix in cis.sh only removed other-read permissions (perm /o+r) but the CIS benchmark also requires: - No group-write/group-execute/any-other bits on files (perm /0137) - No group-write/any-other bits on directories (perm /0027) - Group ownership must be root, adm, or syslog Upstream package updates via apt dist-upgrade --force-confnew can create log files with incorrect group ownership, causing the assessment to fail. This commit: - Enhances cis.sh assignFilePermissions() to fix both permissions and group ownership on all /var/log files and directories - Updates cis-report.sh scanning prep to also fix directory permissions and group ownership before the CIS assessment runs Fixes: ADO#37531219 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…erm masks
- Clear u+x, setuid/setgid/sticky bits in chmod for full CIS compliance
- Gate group ownership logic on getent checks for Mariner/Flatcar safety
- Use -perm /0137 mask in cis-report.sh instead of blanket chmod 640
to avoid widening permissions on intentionally restrictive files
- Use {} + instead of {} ; for find exec performance
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reporting script should not be masking issues by fixing permissions before assessment. The proper fix belongs in cis.sh (VHD build time). If cis.sh works correctly, cis-report.sh doesn't need to compensate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
What this PR does / why we need it
Fixes CIS benchmark regression 'Ensure access to all logfiles has been configured' (rule 6.1.3.1 on Ubuntu 22.04, rule 6.1.4.1 on Ubuntu 24.04) which is failing on both VHD builds.
Detected in ADO build 160335265:
build2404gen2containerd: 6.1.4.1|pass→failbuild2204gen2containerd: 6.1.3.1|pass→failRoot Cause
The existing
assignFilePermissions()incis.shonly removes other-read permissions (perm /o+r), but the CIS benchmark requires:perm /0137)perm /0027)root,adm, orsyslogUpstream package updates via
apt_get_dist_upgrade --force-confnewcan create log files with incorrect group ownership (e.g., a new package version creates files owned by a service-specific group), causing the CIS assessment to fail.Changes
cis.sh: EnhancedassignFilePermissions()to comprehensively fix both permissions and group ownership on all/var/logfiles and directoriescis-report.sh: Updated scanning VM prep to also fix directory permissions and group ownership before the CIS assessment runsWhich issue(s) this PR fixes
Fixes ADO#37531219