Fix template mount_option_removable_partitions #5278
Conversation
There are multiple problems that we try to address by this patch. 1. The test scenario dvd_bad_opts.fail.sh for rule mount_option_noexec_removable_partitions was failing because scanner returned pass instead of expected fail. That is because the rule checks if path specified by var_removable_partition exists and if it doesn't exist the rule fail. This variable is always set to /dev/cdrom in all profiles, as the variable definition has only this value. See /linux_os/guide/system/permissions/partitions/var_removable_partition.var Unless tailoring is used, the rule always passes if /dev/cdrom doesn't exist. As the test scenario didn't create /dev/cdrom, but /dev/dvd, the rule passed. 2. The test scenario dvd_good_opts.pass.sh for rule mount_option_noexec_removable_partitions was also broken even if it returned OK. The test scenario was creating /dev/dvd. So again, the OVAL check passed because /dev/cdrom didn't exist. Therefore the test scenario was not testing the actual regular expression matching in /etc/fstab. 3. The Bash and Ansible remediation weren't functional, because they operated with a mount point (second column of /etc/fstab) but they were passed values of the block device (which is in first column of /etc/fstab). I think that people can mount the removable media to arbitrary directories so we should base our checks on the block device path instead of mount points. Actually, in all 3 rules we passed the device path (/dev/cdrom) as a mount point. That was probably a wrong understanding of the remediation code. The remediations are reworked to find entry starting with var_removable_partition value. 4. The OVAL check wasn't checking only /etc/fstab configuration, but was also checking runtime state. That didn't conform to the rule description. My understanding of the rule description is that the rule is only about configuration, there is no mention of runtime. Moreover, the removable device doesn't have to be mounted at the moment of system evaluation. 5. The test scenarios didn't cover multiple situation, so apart of fixing the broken scenarios, new test scenarios have been added. We still have a problem that /dev/cdrom is not the only possible removable device that could exist. Think of USB flash drives, memory cards, etc. The rule descriptions speak about removable media and mention USB keys, but the checks don't check them. The challenge will be recognizing removable media. Resolves: RHBZ#1691579
|
I wonder what happens if there is no explicit entry in fstab. Will defaults will be used? Where can we find them? Should we check for them as well? Should the remediation create a fstab entry if it doesn't exist? If it should, which directory should be used as mountpoint? |
There was a problem hiding this comment.
3\. The Bash and Ansible remediation weren't functional, because they operated with a mount point (second column of /etc/fstab) but they were passed values of the block device (which is in first column of /etc/fstab). I think that people can mount the removable media to arbitrary directories so we should base our checks on the block device path instead of mount points. Actually, in all 3 rules we passed the device path (/dev/cdrom) as a mount point. That was probably a wrong understanding of the remediation code. The remediations are reworked to find entry starting with var_removable_partition value.
I think it makes sense to base the rules on the devices instead of the mount points.
4\. The OVAL check wasn't checking only /etc/fstab configuration, but was also checking runtime state. That didn't conform to the rule description. My understanding of the rule description is that the rule is only about configuration, there is no mention of runtime. Moreover, the removable device doesn't have to be mounted at the moment of system evaluation.
Yes, I think the essential part is checking that /etc/fstab sets the correct mount options for the devices.
Checking that it is mounted all the time doesn't make sense for removable media. And I think this is the main issue in the BZ.
On the other hand, I think that if a known removable media device is mounted, it is worth checking its mount options.
| assert_mount_point_in_fstab "${{{ MOUNTPOINT }}}" || { echo "Not remediating, because there is no record of ${{{ MOUNTPOINT }}} in /etc/fstab" >&2; return 1; } | ||
| fi | ||
|
|
||
| ensure_mount_option_in_fstab "${{{ MOUNTPOINT }}}" "{{{ MOUNTOPTION }}}" "{{{ FILESYSTEM }}}" "{{{ TYPE }}}" |
There was a problem hiding this comment.
I cannot find where {{{ FILESYSTEM }}} was set.
There was a problem hiding this comment.
it wasn't set anywhere
| - name: Ensure permission {{{ MOUNTOPTION }}} are set on var_removable_partition | ||
| lineinfile: | ||
| path: /etc/fstab | ||
| regexp: '^\s*({{ var_removable_partition }})\s+([^\s]*)\s+([^\s]*)\s+([^\s]*)(.*)$' | ||
| backrefs: yes | ||
| line: '\1 \2 \3 \4,{{{ MOUNTOPTION }}} \5' |
There was a problem hiding this comment.
I'm not sure we should expect /etc/fstab to already contain an entry for the removable device in question.
I tend to think the remediation should add it.
There was a problem hiding this comment.
Probably yes, sounds reasonable
There was a problem hiding this comment.
But, I have realized another thing now. The rule should pass when /etc/fstab doesn't contain the entry. We have a test scenario no_partitions.pass.sh and this scenario simply erases all contents of /etc/fstab. I understand it was created to cover https://bugzilla.redhat.com/show_bug.cgi?id=1403905. That is an old BZ where the reporter asks to no fail the scan if the devices are not mounted. I am not sure if the scenario meets his expectation.
There was a problem hiding this comment.
Is it possible for the device not to be mounted and still have the entry in /etc/fstab?
There was a problem hiding this comment.
I think yes, when the removable media is removed.
There was a problem hiding this comment.
Concerning e.g. DVDs, if there is no device file, it implies that the computer doesn't have a drive, so there is nothing needed in fstab and the rule should simply pass.
The remediation could add the fstab line if the device file exists, but that's outside of what Ansible can do easily, and we aren't even sure how exactly should the line components look like. So I think that this is OK.
There was a problem hiding this comment.
The rule passes when /dev/cdrom doesn't exist and this isn't changed by this PR.
However, the rule also passes when /dev/cdrom exists but there is no line with /dev/cdrom in /etc/fstab. This is because of the old bugzilla https://bugzilla.redhat.com/show_bug.cgi?id=1403905. See the regression test for this BZ in https://github.com/ComplianceAsCode/content/blob/master/linux_os/guide/system/permissions/partitions/mount_option_noexec_removable_partitions/tests/no_partitions.pass.sh. From the test scenario you can see that we expect the rule to pass when /dev/cdrom exists but it isn't configured in /etc/fstab. I have revisited this old BZ and I think that this interpretation might be incorrect, because the mount option isn't enforced in this situation. Instead, I think that the rule should always require explicit configuration in /etc/fstab. On the other hand, if we change the behavior in this situation, we create a regression for the customer that reported the old BZ.
I don't know if it's possible to add the fstab line when the fstab line doesn't exist, because we don't know to which mountpoint the user wishes to mount the device. Should we use some predefined directory eg. /media/cdrom?
There was a problem hiding this comment.
Thanks for your answer - the request in that BZ is really strange. However, as you point out, that rule doesn't have a remediation for cases when the entry in /etc/fstab is missing, which means no all-green state after the remediation. Creating a remediation that would fix fstab and that wouldn't introduce an antipattern is out of scope of this PR, as nobody complains about that aspect of the check.
I think yes, they are probably defined by
If the device is mounted, I think it will be visible through the
The way to customize default mount options is by creating an entry in
I would probably use some distro default place. |
I'm thinking right now that the variable is actually useless. CIS benchmarks mentions removable media like Floppy and CD-ROM. I think the rule is expected to check for all types of removable devices, with all being defined as the pretty common ones, like floppy disks, CD-ROM, DVD-ROM, USB sticks. |
Yes, I concur, that's mentioned later in the pull request description. |
| comment="Check if removable partition is using '{{{ MOUNTOPTION }}}' mount option in /etc/fstab" /> | ||
| <criterion test_ref="test_{{{ MOUNTOPTION }}}_runtime_not_cd_dvd_drive" |
There was a problem hiding this comment.
Runtime needs to be kept and doesn't really need to be mentioned in the prose. There is a expectation which is mentioned in the prose to reboot after configuration has happened when a rule fails.
There was a problem hiding this comment.
Maybe I am missing something - it seems to me that the check won't pass if there is no removable partition mounted (no objects would be found in that case). Removable devices are not supposed to be mounted most of the time, so forcing them to be present during the scan feels inappropriate.
There was a problem hiding this comment.
I also don't understand the meaning of the runtime check in this OVAL. Would that mean that users are supposed to pass this check only with a CD inserted in the drive?
There was a problem hiding this comment.
I think the idea is that if a removable partition is found, it needs to have the correct mount options, not to force that it is mounted.
There was a problem hiding this comment.
I agree with @yuumasato
|
For your reference, this subject has been discussed on the public mailing list, and one thing is clear - removable drives landscape in Linux is huge, it pertains many areas, and it is full of exceptions and false friends. Therefore, I propose to restrict this PR to only fix issues reported by the referenced Bugzilla, and do any additional enhancement to the removable devices handling in other pull requests. |
|
I think that based on the test scenarios the BZ is fixed by this PR. I think that the problem about passing when /etc/fstab doesn't contain the removable media entry isn't directly related to the BZ and therefore can be fixed separately. |
|
@openscap-ci test this please |
|
I can confirm that tests from the test suite pass as well. Therefore - it's time to merge the PR. Remaining issues can be addressed in subsequent PRs, however be sure to review the discussion on the public mailing list and a old bugzilla before going for a fix, as the information there provides the essential context. |
There are multiple problems that we try to address by this patch.
The test scenario dvd_bad_opts.fail.sh for rule
mount_option_noexec_removable_partitions was failing because scanner
returned pass instead of expected fail. That is because the rule checks
if path specified by var_removable_partition exists and if it doesn't
exist the rule fail. This variable is always set to /dev/cdrom in all
profiles, as the variable definition has only this value. See
/linux_os/guide/system/permissions/partitions/var_removable_partition.var
Unless tailoring is used, the rule always passes if /dev/cdrom doesn't
exist. As the test scenario didn't create /dev/cdrom, but /dev/dvd,
the rule passed.
The test scenario dvd_good_opts.pass.sh for rule
mount_option_noexec_removable_partitions was also broken even if it
returned OK. The test scenario was creating /dev/dvd. So again, the
OVAL check passed because /dev/cdrom didn't exist. Therefore the test
scenario was not testing the actual regular expression matching
in /etc/fstab.
The Bash and Ansible remediation weren't functional, because they
operated with a mount point (second column of /etc/fstab) but they
were passed values of the block device (which is in first column of
/etc/fstab). I think that people can mount the removable media to
arbitrary directories so we should base our checks on the block device
path instead of mount points. Actually, in all 3 rules we passed the
device path (/dev/cdrom) as a mount point. That was probably a wrong
understanding of the remediation code. The remediations are reworked
to find entry starting with var_removable_partition value.
The OVAL check wasn't checking only /etc/fstab configuration, but was
also checking runtime state. That didn't conform to the rule
description. My understanding of the rule description is that the rule
is only about configuration, there is no mention of runtime. Moreover,
the removable device doesn't have to be mounted at the moment of system
evaluation.
The test scenarios didn't cover multiple situation, so apart of
fixing the broken scenarios, new test scenarios have been added.
We still have a problem that /dev/cdrom is not the only possible
removable device that could exist. Think of USB flash drives, memory
cards, etc. The rule descriptions speak about removable media and
mention USB keys, but the checks don't check them. The challenge
will be recognizing removable media.
Resolves: RHBZ#1691579
This is opened for discussion. I wasn't able to understand this rule. Maybe I accidentally removed something that is intentional. Please review and let me know.