Skip to content

Add 'bls_audit_option' rule#5793

Merged
yuumasato merged 1 commit intoComplianceAsCode:masterfrom
evgenyz:bls-options
Jun 24, 2020
Merged

Add 'bls_audit_option' rule#5793
yuumasato merged 1 commit intoComplianceAsCode:masterfrom
evgenyz:bls-options

Conversation

@evgenyz
Copy link
Copy Markdown
Member

@evgenyz evgenyz commented May 28, 2020

For checking BLS-compatible boot loader entries for presence of audit=1 parameter. The rule is based on the 'bls_entries_option' template.

The prodtype of the rule is set to fedora,rhel8 but it should be changed to fcos, rhcos (when we will have them), this rule and template won't work on regular Fedora and RHEL. See #5775, #5285.

This will fix #5285.

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented May 28, 2020

/ok-to-test

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented May 28, 2020

@evgenyz could you change the directory to kubernetes instead of ignition? Also, could you add this to a profile like moderate or e8?

Copy link
Copy Markdown
Contributor

@JAORMX JAORMX left a comment

Choose a reason for hiding this comment

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

Let's add this to a profile

@evgenyz
Copy link
Copy Markdown
Member Author

evgenyz commented May 28, 2020

Let's add this to a profile

I assume that you want to add this rule to ocp4 profiles? But here is the problem: OpenShift 4 could have nodes with RHEL and RHCOS, so, depending on the OS, the profile should contain either grub2_audit_argument rule or bls_audit_option (respectively). They are mutually exclusive, you can't have both of them as one of them will always fail.

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented May 28, 2020

Let's add this to a profile

I assume that you want to add this rule to ocp4 profiles? But here is the problem: OpenShift 4 could have nodes with RHEL and RHCOS, so, depending on the OS, the profile should contain either grub2_audit_argument rule or bls_audit_option (respectively). They are mutually exclusive, you can't have both of them as one of them will always fail.

Got it, let's wait until we have the RHCOS product then. which comes here #5775 . And then add it to a profile there.

@yuumasato
Copy link
Copy Markdown
Member

But here is the problem: OpenShift 4 could have nodes with RHEL and RHCOS, so, depending on the OS, the profile should contain either grub2_audit_argument rule or bls_audit_option (respectively).

We plan to update RHEL rules to check BLS settings in the close future. It is not clear to me if we should write new rules or change the current ones. And zIPL is also coming into the scene, see #5784

With that in mind, I'm thinking that the basic building blocks for BLS should be done as Jinja macros and the templates made specific for platforms.
Thoughts on this?

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented Jun 4, 2020

Well, the RHCOS product merged. Can you add a check that uses it to a profile in that product?

@openshift-ci-robot openshift-ci-robot added the needs-rebase Used by openshift-ci bot. label Jun 4, 2020
Comment thread linux_os/guide/system/auditing/bls_audit_option/rule.yml Outdated
@evgenyz
Copy link
Copy Markdown
Member Author

evgenyz commented Jun 5, 2020

With that in mind, I'm thinking that the basic building blocks for BLS should be done as Jinja macros and the templates made specific for platforms.
Thoughts on this?

I seems to me that the most of recent bootloaders would be using BLS entries, including zIPL. And it would make sense to convert it to a Jinja template. But I have no idea how to handle that pesky $kernelopts case.

@evgenyz
Copy link
Copy Markdown
Member Author

evgenyz commented Jun 5, 2020

Okay, now when we are done with rhcos4 product it is time to get to the main course. There is a BZ (https://bugzilla.redhat.com/show_bug.cgi?id=1814210) which describes the problem we have when cluster has a RHEL (7.x in that case) node. Please also check out the link to the ML discussion in that BZ.

Now, in ocp4 profiles (let's take moderate as an example) we have the rule grub2_audit_argument which would check for presence of audit=1 argument for the kernel. This rule is suitable for RHEL8 and RHEL7, but not for RHCOS. For RHOCS we have bls_audit_option rule (this PR).

The only way out of this I could imagine is to have separate profiles for nodes and for the cluster itself. Thoughts?

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented Jun 5, 2020

Okay, now when we are done with rhcos4 product it is time to get to the main course. There is a BZ (https://bugzilla.redhat.com/show_bug.cgi?id=1814210) which describes the problem we have when cluster has a RHEL (7.x in that case) node. Please also check out the link to the ML discussion in that BZ.

Now, in ocp4 profiles (let's take moderate as an example) we have the rule grub2_audit_argument which would check for presence of audit=1 argument for the kernel. This rule is suitable for RHEL8 and RHEL7, but not for RHCOS. For RHOCS we have bls_audit_option rule (this PR).

The only way out of this I could imagine is to have separate profiles for nodes and for the cluster itself. Thoughts?

that's exactly where we're heading and why I introduced the RHCOS product in the first place.

OCP4 will change to be for API/platform checks only. and node checks will use rhcos4/rhel7/rhel8 profiles respectively.

@evgenyz
Copy link
Copy Markdown
Member Author

evgenyz commented Jun 5, 2020

By the way, another question is how to remediate RHEL7 nodes in the cluster?

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented Jun 5, 2020

By the way, another question is how to remediate RHEL7 nodes in the cluster?

This is a good question.

We could apply MachineConfigs to RHEL7 nodes just like we do with RHCOS and RHEL8 nodes... but we'll need to have MachineConfig remediations that apply and are relevant to RHEL7.

@evgenyz
Copy link
Copy Markdown
Member Author

evgenyz commented Jun 5, 2020

that's exactly where we're heading and why I introduced the RHCOS product in the first place.

OCP4 will change to be for API/platform checks only. and node checks will use rhcos4/rhel7/rhel8 profiles respectively.

Gosh, I just remember my introductory PR for OCP4 and regret not standing my ground. All this madness (including extra rules argument) could have been dodged.

I'm sorry. I don't know what is wrong with me. I edit comments instead if quoting them recently. :(

@openshift-ci-robot openshift-ci-robot removed the needs-rebase Used by openshift-ci bot. label Jun 5, 2020
@yuumasato
Copy link
Copy Markdown
Member

/retest

@openshift-ci-robot
Copy link
Copy Markdown
Collaborator

openshift-ci-robot commented Jun 23, 2020

@evgenyz: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-e8 ebf6498 link /test e2e-aws-e8

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@JAORMX
Copy link
Copy Markdown
Contributor

JAORMX commented Jun 23, 2020

Seems you might need to rebase.

# AU-3
- package_audit_installed
- grub2_audit_argument
- bls_audit_option
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The way how to check BLS entry options may be similar across OSes or arches, but description and OCIL can vary a lot. Take for example zipl_audit_argument, description and OCIL mention a few commands and config files specific for zipl.

So I wonder if the rule should be renamed to grub2_bls_audit_option.
And conversely, zipl_audit_argument renamed to zipl_bls_audit_option.

The template name can remain boot loader neutral, but I would put in singular: bls_entry_option.
@evgenyz Thoughts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Entries are plural because we are checking ALL entries: <ind:filepath operation="pattern match">^/boot/loader/entries/.*\.conf$</ind:filepath>.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh, wouldn't it be the case that this won't pass even after the remediation is applied? The deployment keeps the previous configuration around which wouldn't have the compliant option.

Copy link
Copy Markdown
Member Author

@evgenyz evgenyz Jun 23, 2020

Choose a reason for hiding this comment

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

Now, zipl_audit_argument (as it seems to me) holds 2 different unrelated checks: for option in BLS-compatible entry and for settings in /etc/zipl.conf. I think that they should be separated (bls_audit_option + zipl_config_blah_blah). The bls_audit_option is bootloader-agnostic and could be reused in that way for all BLS-compatible bootloaders.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh, wouldn't it be the case that this won't pass even after the remediation is applied? The deployment keeps the previous configuration around which wouldn't have the compliant option.

Yes, it would be the case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Later we could add something like current=True parameter to this template.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Later, this template can also be enhanced to consider cases when $kernelopts is present in the entry.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Later, this template can also be enhanced to consider cases when $kernelopts is present in the entry.

I would be careful with that, it will make the rule not compatible with BLS.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But, on the other hand we can parametrize the template to create grub2_bls_audit_option.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The template can have conditionals for how different products handle BLS.
RHEL for example may check for all entries, not just the current.

entries for presence of audit=1 parameter. The rule is based on
the 'bls_entries_option' template.
@mildas
Copy link
Copy Markdown
Contributor

mildas commented Jun 23, 2020

Changes identified:
Profile moderate on rhcos4:
 Rule bls_audit_option added to moderate profile.
 Rule grub2_audit_argument removed from moderate profile.
Others:
 Python abstract syntax tree change found in ssg/templates.py.

Recommended tests to execute:
 (cd build && cmake ../ && ctest -j4)
 build_product rhcos4
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhcos4-ds.xml moderate

@yuumasato yuumasato self-assigned this Jun 24, 2020
@yuumasato yuumasato merged commit af21aa1 into ComplianceAsCode:master Jun 24, 2020
@yuumasato yuumasato added this to the 0.1.51 milestone Jun 24, 2020
@evgenyz evgenyz deleted the bls-options branch July 9, 2021 07:30
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.

RHCOS does not write kernel parameters into grub.conf

7 participants