Skip to content

fix: CIS regressions — re-apply /etc/issue banners + comprehensive logfile permissions for scan VM#8317

Open
djsly wants to merge 5 commits intomainfrom
fix/cis-etc-issue-overwrite
Open

fix: CIS regressions — re-apply /etc/issue banners + comprehensive logfile permissions for scan VM#8317
djsly wants to merge 5 commits intomainfrom
fix/cis-etc-issue-overwrite

Conversation

@djsly
Copy link
Copy Markdown
Collaborator

@djsly djsly commented Apr 15, 2026

Problem

Multiple CIS rules are regressing on Ubuntu VHD builds, blocking every PR on pipeline 119535:

  • CIS 1.6.2/1.6.3 (login banners): apt_get_dist_upgrade --force-confnew overwrites custom /etc/issue and /etc/issue.net banners when base-files package upgrades
  • CIS 6.1.3.1 (22.04) / 6.1.4.1 (24.04) (logfile permissions): Scan VM boot-time daemons (syslog, journal) create new log files with default permissions/ownership that violate CIS rules

Root Causes

/etc/issue (1.6.2/1.6.3)

  1. copyPackerFiles() in pre-install-dependencies.sh copies custom banners early in the build
  2. apt_get_dist_upgrade() runs with --force-confnew, overwriting conffiles with maintainer versions
  3. Custom banners are replaced with default Ubuntu content containing \n \l escape sequences

Logfile permissions (6.1.3.1/6.1.4.1)

  1. cis.sh fixes log permissions during VHD build (runs last among config scripts)
  2. VHD is captured and scan VM boots from it
  3. Boot-time daemons create new log files with wrong permissions/group ownership
  4. The previous scan-time fix (chmod 640) only handled file mode, not directory perms or group ownership

Fixes

Commit 1: Re-copy banners in post-install-dependencies.sh

Re-copy custom login banners from the packer staging area after all apt operations complete. This ensures banners survive any base-files upgrade.

Commit 2: Comprehensive logfile fix in cis-report.sh

Replace the simple find /var/log -type f -exec chmod 640 {} \; with comprehensive treatment matching cis.sh:

  • File permissions: at most 0640 (clear execute, group-write, other-all, setuid/setgid/sticky)
  • Directory permissions: at most 0750 (clear group-write, other-all)
  • Group ownership: must be root, adm, or syslog

This supersedes PR #8299 which attempted to fix the logfile issue but removed the scan-time fix, making things worse.

Impact

  • Fixes CIS 1.6.2, 1.6.3, 6.1.3.1 (22.04), and 6.1.4.1 (24.04) regressions
  • No impact on Azure Linux/Mariner (CIS scan only runs for Ubuntu)
  • No runtime/CSE impact — changes only affect VHD build and scan time

apt_get_dist_upgrade uses --force-confnew which forces dpkg to overwrite
all conffiles with the package maintainer's version. When a new base-files
package is published to Ubuntu repos, /etc/issue and /etc/issue.net get
replaced with the default Ubuntu content (containing \n \l escape
sequences), failing CIS rules 1.6.2 and 1.6.3.

Fix by re-copying the custom login banners from the packer staging area
after all apt operations complete in post-install-dependencies.sh.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Re-applies custom /etc/issue and /etc/issue.net login banners after Ubuntu apt operations to prevent CIS 1.6.2/1.6.3 regressions caused by base-files conffile overwrites during dist-upgrade.

Changes:

  • Adds a post-apt step in post-install-dependencies.sh to copy the custom local and remote login banners back into place.
  • Documents why apt_get_dist_upgrade --force-confnew can revert the banner files.

The scan VM boots from the VHD and boot-time daemons (syslog, journal)
create new log files with default permissions that violate CIS 6.1.3.1
(22.04) / 6.1.4.1 (24.04). The previous simple chmod 640 only handled
file mode but not directory perms or group ownership.

Replace with comprehensive fix matching cis.sh assignFilePermissions():
- File permissions: at most 0640 (clear execute, group-write, other-all)
- Directory permissions: at most 0750 (clear group-write, other-all)
- Group ownership: must be root, adm, or syslog

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@djsly djsly changed the title fix: re-apply /etc/issue banners after apt dist-upgrade (CIS 1.6.2/1.6.3 regression) fix: CIS regressions — re-apply /etc/issue banners + comprehensive logfile permissions for scan VM Apr 15, 2026
The ETC_ISSUE_CONFIG_SRC/DEST variables are defined in packer_source.sh
which is not sourced by post-install-dependencies.sh. Use the literal
paths directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 15, 2026 02:10
Move /etc/issue re-copy logic into a reapplyBanners() function in
packer_source.sh so post-install-dependencies.sh uses the same source
of truth as copyPackerFiles(). Source packer_source.sh in post-install
(safe — it only defines functions, no top-level side effects) matching
the pattern already used by pre-install-dependencies.sh.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 3 out of 3 changed files in this pull request and generated 2 comments.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2026

PR Title Lint Failed ❌

Current Title: fix: CIS regressions — re-apply /etc/issue banners + comprehensive logfile permissions for scan VM

Your PR title doesn't follow the expected format. Please update your PR title to follow one of these patterns:

Conventional Commits Format:

  • feat: add new feature - for new features
  • fix: resolve bug in component - for bug fixes
  • docs: update README - for documentation changes
  • refactor: improve code structure - for refactoring
  • test: add unit tests - for test additions
  • chore: remove dead code - for maintenance tasks
  • chore(deps): update dependencies - for updating dependencies
  • ci: update build pipeline - for CI/CD changes

Guidelines:

  • Use lowercase for the type and description
  • Keep the description concise but descriptive
  • Use imperative mood (e.g., "add" not "adds" or "added")
  • Don't end with a period

Examples:

  • feat(windows): add secure TLS bootstrapping for Windows nodes
  • fix: resolve kubelet certificate rotation issue
  • docs: update installation guide
  • Added new feature
  • Fix bug.
  • Update docs

Please update your PR title and the lint check will run again automatically.

Address review feedback:
- Expand -perm masks to /7137 (files) and /7027 (dirs) to also catch
  setuid/setgid/sticky bits that violate CIS rules.
- Prefer adm group over syslog when both exist, as adm is the
  conventional log group on Ubuntu. Only fall back to syslog when
  adm is absent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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