security: Update on CIS benchmarks

[mayankfz]

Summary

security: Update on CIS benchmarks
Doc -> https://microsoftapc-my.sharepoint.com/:w:/g/personal/mayansingh_microsoft_com/IQBPQGV0CWHsTLuKSDKsTOGfASfMARMQJnRTFUxFPwuuA6I?e=dclE6g

Change Log

• security: Update on CIS benchmarks

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

https://dev.azure.com/mariner-org/ACL/_workitems/edit/19664

Test Methodology

• Test details:
  aks-e2e failures are same with/without my changes seems to be flaky behaviour.
  aks-e2e -> https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1161851&view=results
  aks-e2e (without changes) -> https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1162283&view=results
  acldevel -> https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1161789&view=results

Merge Checklist

All applicable boxes should be checked before merging

• 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

[Copilot]

Pull request overview

This PR updates the RPM-mode image hardening steps to better align with CIS benchmarks by tightening SSH configuration, blacklisting additional kernel modules, and improving log file permission hardening during image build.

Changes:

• Tighten permissions on generated sshd_config.d snippets and add a new CIS-focused SSH hardening drop-in (access limitation, forwarding disabled, MaxAuthTries).
• Extend CIS kernel module blacklisting to include sctp alongside cramfs.
• Add waagent service hardening (UMask drop-in) and a tmpfiles-based attempt to enforce /var/log/azure permissions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

build_library/rpm/build_image_util.sh:1397

• The tmpfiles directive type z does not create the path if it’s missing, so this won’t ensure /var/log/azure exists “at boot” (it only adjusts perms/label if already present). Add a d entry to create the directory, then keep z to restore SELinux context/permissions.

    sudo tee "${root_fs_dir}/usr/lib/tmpfiles.d/cis-logfiles.conf" > /dev/null <<'TMPFILES_LOG'
z /var/log/azure 0750 root root - -
TMPFILES_LOG

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

build_library/rpm/build_image_util.sh:1394

• The tmpfiles comment says it "ensures" /var/log/azure is 0750 at boot, but the z tmpfiles directive only applies to an existing path (it won’t create the directory if missing). Also, the referenced waagent drop-in umask is documented as 0137 here, but the waagent systemd drop-in being added sets UMask=0027. Please correct the comments to match the actual behavior/setting.

    # tmpfiles.d: ensure /var/log/azure dir is 0750 at boot (CIS 6.1.3.1).
    # File perms handled by waagent UMask=0137 drop-in.

[Jiri Appl (jiria)]

mayankfz ?

[Jiri Appl (jiria)]

AllowUsers/AllowGroups/DenyUsers/DenyGroups are the additive keywords in OpenSSH — they don't follow the usual "first obtained value wins" rule, servconf.c appends each occurrence to the same list. So shipping AllowUsers * here isn't a no-op in the way the comment suggests: a downstream drop-in adding AllowUsers azureuser produces the effective list * azureuser, i.e. still everyone. Ordering doesn't help — it's a union either way.

The practical effect is that the image ships a permanent "allow everyone" ACL that can't be tightened by adding another drop-in, only by deleting/overwriting this file. And it fails quietly: someone who adds an allow-list and checks sshd -T will see their user and assume it worked.

CIS 5.2.7 only requires at least one of the four directives, so the Deny pair on its own should satisfy it:

Suggested change

	# The assessor requires all four directives to be present in sshd -T output.
	# AllowUsers/AllowGroups * are effectively no-ops (permit everyone) but satisfy
	# the check. DenyUsers/DenyGroups root block root SSH login.
	# CIS 5.2.7 - Ensure SSH access is limited
	# Deny root SSH login. CIS requires at least one of
	# AllowUsers/AllowGroups/DenyUsers/DenyGroups; the Deny pair satisfies it
	# without shipping a wildcard allow-list (these keywords are additive, so a
	# wildcard here would union with — and neutralise — any future allow-list).
	DenyUsers root
	DenyGroups root

If the assessor genuinely rejects that and wants all four present, could we use a real allow-list (AllowGroups wheel sudo core or similar) instead of *?

[mayankfz]

Tested on a live VM, the assessor explicitly requires allowusers in sshd -T output (checks regex .+$). With only DenyUsers/DenyGroups it reports NonCompliant: "Option 'allowusers' not found in SSH daemon configuration."
A static allow-list (AllowGroups sudo) won't work either, WALinuxAgent provisions the admin user in its own dynamic group, not sudo/wheel. So, the wildcard is unfortunately required by the current MOF.
Updated the comment to be clear about the additive behavior and the constraint. Long-term we should request a MOF fix to accept Deny-only as sufficient.

[Jiri Appl (jiria)]

DisableForwarding is special-cased in OpenSSH — sshd_config(5) says it "overrides all other forwarding-related options", which means it can't be relaxed later by a Match block, a lower-numbered drop-in, or anything AKS does at provisioning time. It's a one-way door for TCP forwarding, direct-tcpip, agent forwarding, X11 and StreamLocal all at once.

Direct kola SSH is fine (nothing in mantle calls RequestAgentForwarding), but network/jump.go and the external platform open direct-tcpip channels through an intermediate host — so this would break the external platform if a jump host ever runs an ACL image. Same for any operator flow that tunnels to a node-local port (kubelet :10250, containerd debug socket).

The granular directives give the same hardening and each shows up individually in sshd -T, but stay overridable when someone needs a break-glass path:

Suggested change

	DisableForwarding yes
	# CIS 5.2.10 - Ensure SSH forwarding is disabled
	# Granular directives rather than DisableForwarding, which overrides all other
	# forwarding options and cannot be relaxed by a Match block or a later drop-in.
	AllowTcpForwarding no
	AllowAgentForwarding no
	AllowStreamLocalForwarding no
	X11Forwarding no
	PermitTunnel no

Does the assessor accept the granular form for 5.2.10?

[mayankfz]

Tested it, assessor checks disableforwarding in sshd -T with match: yes. Granular directives make it report disableforwarding no -> NonCompliant.
MOF doesn't accept the equivalent form, so we need DisableForwarding yes for now. Will file a MOF bug to accept the granular directives as equivalent. If needed, a higher-priority drop-in (70-*) can override it per-host.

[SeanDougherty]

One accuracy correction before resolving the thread: a later 70-* drop-in cannot override this setting. OpenSSH scalar options use the first obtained value, and included files are parsed in lexical order. I reproduced this with OpenSSH 9.8: DisableForwarding yes followed by DisableForwarding no still resolves to yes; only putting no before the 60-* file changes the result. Please correct the documented escape hatch - for example, replacing this file or using an earlier-numbered policy file - and link the promised assessor/MOF issue.

[Jiri Appl (jiria)]

This changes six runtime behaviours of the shipped image (SSH access control, forwarding, MaxAuthTries, sshd config modes, sctp availability, /var/log/azure perms) with no test to lock any of them in. There don't appear to be any CIS kola tests in the repo today, so right now the only validation is "the assessor passed on one pipeline run" — a point-in-time check outside our own CI.

These are cheap to assert since they're all visible in sshd -T. Something like an acl.security.cis.sshd kola test:

out := c.MustSSH(m, "sudo sshd -T")
for _, want := range []string{
    "denyusers root", "denygroups root", "maxauthtries 4",
    "allowtcpforwarding no", "x11forwarding no",
} {
    if !strings.Contains(strings.ToLower(string(out)), want) {
        c.Fatalf("sshd -T missing %q", want)
    }
}

plus a check that the sshd_config.d/*.conf files are 0600. Would catch a Flatcar rebase reordering the drop-ins, or a future drop-in quietly undoing any of this. Happy for it to be a follow-up if you'd rather not grow this PR.

[mayankfz]

This changes six runtime behaviours of the shipped image (SSH access control, forwarding, MaxAuthTries, sshd config modes, sctp availability, /var/log/azure perms) with no test to lock any of them in. There don't appear to be any CIS kola tests in the repo today, so right now the only validation is "the assessor passed on one pipeline run" — a point-in-time check outside our own CI.

These are cheap to assert since they're all visible in sshd -T. Something like an acl.security.cis.sshd kola test:

out := c.MustSSH(m, "sudo sshd -T")
for _, want := range []string{
    "denyusers root", "denygroups root", "maxauthtries 4",
    "allowtcpforwarding no", "x11forwarding no",
} {
    if !strings.Contains(strings.ToLower(string(out)), want) {
        c.Fatalf("sshd -T missing %q", want)
    }
}

plus a check that the sshd_config.d/*.conf files are 0600. Would catch a Flatcar rebase reordering the drop-ins, or a future drop-in quietly undoing any of this. Happy for it to be a follow-up if you'd rather not grow this PR.

Agreed, will add as a follow-up. This PR is already large enough. Will create an acl.security.cis.sshd test covering sshd -T assertions + file mode checks, and a separate acl.security.cis.modprobe for the sctp/cramfs blacklisting.

[Copilot]

Pull request overview

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

Suppressed comments (1)

build_library/rpm/build_image_util.sh:475

• The code comment says sshd_config must be "600 root:root", but only the mode is enforced. If the base rootfs ever ships (or later creates) sshd_config with non-root ownership, sshd may reject it and the CIS check may still fail. Explicitly set ownership to root:root alongside the chmod.

    # CIS 5.2.1: sshd_config must be 600 root:root regardless of how it was created
    sudo chmod 600 "${sshd_config}"

[Copilot]

Pull request overview

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

Suppressed comments (1)

sdk_container/src/third_party/coreos-overlay/coreos-base/oem-azure/files/manglefs_rpm.sh:26

• The CIS waagent drop-in is created unconditionally. If the rootfs doesn't include waagent.service (or if it’s removed in a future variant), this script will still create /usr/lib/systemd/system/waagent.service.d/cis-umask.conf, which can be surprising and makes the modification inconsistent with the guarded waagent.service edits above/below. Gate the drop-in creation on the presence of the unit file so the change is applied only when waagent is actually shipped.

# CIS 6.1.3.1: waagent creates logs with 644 (umask 022). Set UMask=0027 so
# waagent.log and extension logs get 640 while directories stay traversable (0750).
mkdir -p "${rootfs}/usr/lib/systemd/system/waagent.service.d"
cat > "${rootfs}/usr/lib/systemd/system/waagent.service.d/cis-umask.conf" <<'EOF'
[Service]
UMask=0027
EOF
chmod 0644 "${rootfs}/usr/lib/systemd/system/waagent.service.d/cis-umask.conf"

[Copilot]

Pull request overview

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

[mayankfz]

This changes six runtime behaviours of the shipped image (SSH access control, forwarding, MaxAuthTries, sshd config modes, sctp availability, /var/log/azure perms) with no test to lock any of them in. There don't appear to be any CIS kola tests in the repo today, so right now the only validation is "the assessor passed on one pipeline run" — a point-in-time check outside our own CI.
These are cheap to assert since they're all visible in sshd -T. Something like an acl.security.cis.sshd kola test:

out := c.MustSSH(m, "sudo sshd -T")
for _, want := range []string{
    "denyusers root", "denygroups root", "maxauthtries 4",
    "allowtcpforwarding no", "x11forwarding no",
} {
    if !strings.Contains(strings.ToLower(string(out)), want) {
        c.Fatalf("sshd -T missing %q", want)
    }
}

plus a check that the sshd_config.d/*.conf files are 0600. Would catch a Flatcar rebase reordering the drop-ins, or a future drop-in quietly undoing any of this. Happy for it to be a follow-up if you'd rather not grow this PR.

Agreed, will add as a follow-up. This PR is already large enough. Will create an acl.security.cis.sshd test covering sshd -T assertions + file mode checks, and a separate acl.security.cis.modprobe for the sctp/cramfs blacklisting.

CIS kola tests - microsoft/azure-container-linux-mantle#33
pipeline run - https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1175042&view=results

[SeanDougherty]

I reviewed the additional validation evidence. Run 1175042 materially improves confidence in the fresh-image implementation: all three CIS tests passed on QEMU amd64, Azure amd64, and Azure aarch64. On QEMU aarch64, SSH and log permissions passed; the modprobe case was interrupted by the unrelated serial-getty dependency failure. I also consider the aarch64 Azure smoke OSProvisioningTimedOut likely infrastructural because Azure aarch64 Kola successfully provisioned the same build. I am treating those QEMU aarch64 and Azure provisioning failures as expected infrastructure flakiness rather than PR blockers.

I cannot conclude that the AKS E2E failures are the same with and without this change. The changed-image run had 17 failed scenarios versus 12 in the baseline. All 12 baseline failures overlap, but these five occurred only with the changed image:

• Scenario_AzureContainerLinux_KSCR
• Scenario_AzureContainerLinux_OpenServiceMeshAddon
• Scenario_AzureContainerLinux_RBAC_Disabled
• Scenario_AzureContainerLinux_Reset_SP
• Scenario_AzureContainerLinux_VirtualNodeAddon

That does not prove causation, but it does mean the current comparison does not establish regression freedom. Please triage those five results or provide matched reruns, and update the test-methodology text in the meantime.

I am requesting changes until:

1. The five changed-only AKS failures are triaged or cleared by matched reruns.
2. Existing logs on stateful /var are covered by remediation and a test.
3. AllowGroups * is removed or its independent assessor requirement and downstream configuration impact are documented.

DisableForwarding yes is required for this CIS control. The existing thread only needs its override guidance corrected.

[SeanDougherty]

Blocking for update-time compliance: this d entry fixes or creates the /var/log/azure directory, but it does not change files already below it. UMask=0027 also affects only files created after waagent starts. Because ACL's /var is stateful, an existing 0644 waagent or extension log remains world-readable after an OS update.

Could we add a file-only remediation path for existing logs and cover the update case with a test that starts with an existing 0644 log? This should avoid a recursive 0640 operation over directories, which would remove their execute bit. The current cis.logperms test checks the directory and drop-in text, but no actual log-file mode.

[SeanDougherty]

The comment explains that the assessor specifically requires a non-empty allowusers value, but it does not establish that allowgroups is also required. OpenSSH appends repeated AllowGroups directives, so shipping AllowGroups * prevents a downstream consumer from narrowing access with a normal later drop-in.

Unless the assessor independently requires allowgroups, can we remove this directive and the corresponding Kola assertion? If it is required, please link the exact assessor rule or bug and document that consumers must replace the shipped file to apply a group allow-list.