fix(5.2.2): ensured correct host key permissions are checked on RedHa…

[amlodzianowski]

While the CIS DIL Benchmark expects 0600 root:root permissions on all private host key files, this appears to be undesired behavior on RedHat systems where the openssh package creates ssh host key files with the group ssh_keys and permissions 0640 for user host-based ssh authentification to work (the setuid helper program /usr/libexec/openssh/ssh-keysign is used to read the keys and requires these permissions).

We are thinking that instead of waivering this control ourselves, this profile should be updated to reflect the recommended RedHat configuration.

Relevant issues: https://bugzilla.redhat.com/show_bug.cgi?id=1372070 https://github.com/ComplianceAsCode/content/issues/1542 https://github.com/ComplianceAsCode/content/pull/1556/files https://github.com/xcat2/xcat-core/issues/2617

[Spencer-Doak]

Just weighing in with some thoughts, the CIS Distribution Independent Linux (DIL) benchmarks v2.0.0 are pretty explicit on this control:

5.2.2 Ensure permissions on SSH private host key files are configured (Scored)

... Description: An SSH private key is one of two files used in SSH public key authentication. In this authentication method, The possession of the private key is proof of identity. Only a private key that corresponds to a public key will be able to authenticate successfully. The private keys need to be stored and handled carefully, and no copies of the private key should be distributed. Rationale: If an unauthorized user obtains the private SSH host key file, the host could be impersonated Audit: Run the following command and verify Uid is 0/root and and Gid is 0/root. Ensure group and other do not have permissions ...

The CIS DIL benchmarks in several scenarios conflict with the recommendations of vendors, and in some cases, the recommendations are plain wrong for certain vendors. I have more than once seen recommendations in DIL that apply to one flavor of Linux, but do not really apply to another version of Linux. (E.g., stuff like "the file at /some/path file must exist, must be owned by root, and must have 0600 permissions," where /some/path was a path that exists on Debian, but does not exist and is not relevant to CentOS.) Alas, such issues are issues within the the CIS DIL benchmarks themselves, in my opinion, and are quite often not issues with dev-sec's implementation of the cis-dil-benchmark.

That being said, considering the CIS benchmarks for RHEL, CentOS and DIL all discourage host-based authentication, I am of the opinion that the keys really should be 0600 on all platforms.

[amlodzianowski]

@Spencer-Doak

I totally understand this approach and I was a little hesitant to even open this PR in the first place since this is a pretty grey area in my mind. While the CIS DIL and CIS RHEL 8 benchmarks both clearly state the desired permissions as 0600 with root:root ownership, CIS themselves are inconsistent with the implementation.

The RHEL 8 Build Kit provided by CIS (v1.0.0) has a bash function ensure_permissions_ssh_private_hostkey_files_configured() defined in the RHEL8/functions/recommendations/nix_ensure_permissions_ssh_private_hostkey_files_configured.sh file. Within that function are two different tests and fixes defined, one for 0640 root:ssh_keys ownership and once for 0600 root:root. I am not sure if I can paste the whole content here, but here is the relevant snippet:

        find -L /etc/ssh -xdev -type f -name 'ssh_host_*_key' | while read -r file; do
            if [ "$(stat -Lc "%U %G" "$file")" = "root ssh_keys" ]; then
                if ! stat -Lc "%A" "$file" | grep -Eq -- '^-[r-][w-]-[r-]-----$'; then
                    return "${XCCDF_RESULT_FAIL:-102}"
                fi
            else
                if [ "$(stat -Lc "%A" "$file" | cut -c4-10)" != "-------" ]; then
                    return "${XCCDF_RESULT_FAIL:-102}"
                fi
            fi
        done

And the fix:

            if [ "$(stat -Lc "%U %G" "$file")" = "root ssh_keys" ]; then
                if ! stat -Lc "%A" "$file" | grep -Eq -- '^-[r-][w-]-[r-]-----$'; then
                    chmod u-x,o-wx,g-rwx "$file"
                fi
            else
                if [ "$(stat -Lc "%A" "$file" | cut -c4-10)" != "-------" ]; then
                    chmod u-x,go-rwx "$file"
                fi
            fi

Additionally, we tested the AWS Marketplace RHEL 8 CIS Level 1 AMI which has the same problem - all private host key files have 0640 root:ssh_keys ownership. This goes against the RHEL 8 benchmark and could be an oversight by the people responsible for baking the AMI, but they also could've left those permissions intentionally (probably via the build kit).

In any case, I completely agree with your point that this is an issue with the CIS DIL benchmark itself, not the dev-sec implementation of this control which is completely correct. If the goal of this project is to keep the dev-sec profile true to the benchmark and drop edge cases like this (which could turn into a maintenance nightmare) then I will close this PR out.

[Spencer-Doak]

@amlodzianowski you definitely raise a good point, with regard to the build kit and the marketplace CIS level 1 AMI. (I had not looked at the RHEL 8 implementations or remediation kits, as I am a CentOS user, and, well, we all know what's going on with CentOS Stream 🙄)

I wonder if there are other Linux distributions that recommend a group like "ssh_keys". If there are, then perhaps dev-sec could add something attribute-based, allowing users to specify whether there should be a group allowed access to their keys. Something like:

if group_allowed_access.nil?
    it { should_not be_more_permissive_than('0600') }
else
    # non-nil group specified
    its('group') { should eq group_allowed_access }
    it { should_not be_more_permissive_than('0640') }
end

In any event, to be clear, I am not a cis-dil-benchmark maintainer. I am merely a user of this repo who is interested in its longevity. As an end user (and especially as a user on a related OS), I certainly appreciate your contributions, and I appreciate you identifying a discrepancy like this and bringing it to the repo's attention.