Skip to content

Change rhcos4/moderate kernel argument checks to use coreos check#6131

Merged
JAORMX merged 2 commits intoComplianceAsCode:masterfrom
JAORMX:rhcos-kernel-args
Oct 7, 2020
Merged

Change rhcos4/moderate kernel argument checks to use coreos check#6131
JAORMX merged 2 commits intoComplianceAsCode:masterfrom
JAORMX:rhcos-kernel-args

Conversation

@JAORMX
Copy link
Copy Markdown
Contributor

@JAORMX JAORMX commented Sep 30, 2020

A recent commit [1] introduced an enhanced check for kernel arguments
that works in CoreOS. This commit takes them into use in rhcos4's
moderate profile. The needed checks were created with appropriate text.

[1] #6088

@JAORMX JAORMX changed the title Change rhcos4/moderate kernel argument checks to use coreos check WIP: Change rhcos4/moderate kernel argument checks to use coreos check Sep 30, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Sep 30, 2020
@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Sep 30, 2020

Pull-request updated, HEAD is now aad47f0

@ggbecker
Copy link
Copy Markdown
Member

ggbecker commented Sep 30, 2020

@openscap-ci test this please

@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Sep 30, 2020

Pull-request updated, HEAD is now 51be1ae

@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Sep 30, 2020

Pull-request updated, HEAD is now 8bf09e5

@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Sep 30, 2020

Pull-request updated, HEAD is now 2d0e3b3

@JAORMX JAORMX changed the title WIP: Change rhcos4/moderate kernel argument checks to use coreos check Change rhcos4/moderate kernel argument checks to use coreos check Sep 30, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Sep 30, 2020
@JAORMX JAORMX requested review from evgenyz and yuumasato September 30, 2020 11:58
yuumasato
yuumasato requested changes Sep 30, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

In general LGTM.

Comment thread shared/templates/template_KUBERNETES_coreos_kernel_option Outdated
Comment thread linux_os/guide/system/permissions/restrictions/poisoning/grub2_page_poison_argument/rule.yml
identifiers:
cce@rhel7: CCE-82158-7
cce@rhel8: CCE-80944-2
cce@rhcos4: CCE-82673-5
Copy link
Copy Markdown
Member

@yuumasato yuumasato Sep 30, 2020

Choose a reason for hiding this comment

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

CCE-82673-5 went from rule grub2_page_poison_argument to coreos_page_poison_kernel_argument.
The configuration checked/remediated is almost the same, the difference is that coreos_page_poison_kernel_argument checks only the last boot entry.

I'm not sure if this CEE migration is okay, maybe @redhatrises has thoughts on it.

Copy link
Copy Markdown
Contributor

@redhatrises redhatrises Oct 6, 2020

Choose a reason for hiding this comment

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

I am okay with this until we actually deliver something. Ultimately, I think that creating duplicate rules for something that is subtly unique is not a great. This is something to figure out in a separate PR.

Comment thread shared/templates/template_OVAL_coreos_kernel_option Outdated
<unix:file_object id="object_coreos_{{{ SANITIZED_ARG_NAME }}}_entry_2_does_not_exists"
version="1">
<unix:filepath operation="pattern match">^/boot/loader/entries/ostree-2-*\.conf</unix:filepath>
<unix:filepath operation="pattern match">^/boot/loader/entries/ostree-2-.*\.conf</unix:filepath>
Copy link
Copy Markdown
Member

@yuumasato yuumasato Sep 30, 2020

Choose a reason for hiding this comment

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

Noice.

@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Sep 30, 2020

Pull-request updated, HEAD is now 86978b9

@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 1, 2020

Pull-request updated, HEAD is now 00ad321

@JAORMX JAORMX force-pushed the rhcos-kernel-args branch from 86978b9 to 00ad321 Compare October 1, 2020 12:45
@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 1, 2020

/retest

1 similar comment
@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 2, 2020

/retest

@JAORMX JAORMX requested a review from yuumasato October 5, 2020 06:12
@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 6, 2020

This is blocked on #6100

JAORMX added 2 commits October 6, 2020 14:00
@JAORMX
Change rhcos4 kernel argument checks to use coreos-specific checks
f1d7b76 
A recent commit [1] introduced an enhanced check for kernel arguments
that works in CoreOS. This commit takes them into use in rhcos4's
moderate & ncp profiles. The needed checks were created with appropriate text.

[1] ComplianceAsCode#6088
@JAORMX
kube: Replace explicit kernel argument remediations for template
8427166 
This replaces the explicit MachineConfigs for templates.
@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 6, 2020

Pull-request updated, HEAD is now 8427166

@JAORMX JAORMX force-pushed the rhcos-kernel-args branch from 00ad321 to 8427166 Compare October 6, 2020 11:08
@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 6, 2020

/test e2e-aws-rhcos4-e8

jhrozek
jhrozek reviewed Oct 6, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

Looks quite good, I was a bit confused about the tests, otherwise lgtm

Comment thread linux_os/guide/system/auditing/coreos_audit_backlog_limit_kernel_argument/rule.yml
ocil: |-
Inspect the form of all the BLS (Boot Loader Specification) entries
('options' line) in <tt>/boot/loader/entries/*.conf</tt>. If they include
<tt>audit=1</tt>, then auditing is enabled at boot time.
Copy link
Copy Markdown
Collaborator

@jhrozek jhrozek Oct 6, 2020

Choose a reason for hiding this comment

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

Should the rule talk about enabling audit?

Copy link
Copy Markdown
Contributor Author

@JAORMX JAORMX Oct 6, 2020

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Copy Markdown
Collaborator

@jhrozek jhrozek Oct 7, 2020

Choose a reason for hiding this comment

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

Pointing out that the admin should inspect the kernel command line for audit=1 seems a bit redundant, because it's not the point of this rule. But the ocil rule is not super important for coreos rules and the backlog parameter is mentioned next, so it's OK.

Comment thread ...uide/system/auditing/coreos_audit_backlog_limit_kernel_argument/tests/correct_grubby.pass.sh Outdated
Comment thread ...ide/system/auditing/coreos_audit_backlog_limit_kernel_argument/tests/correct_grubenv.pass.sh Outdated
Comment thread ...auditing/coreos_audit_backlog_limit_kernel_argument/tests/wrong_value_etcdefaultgrub.fail.sh Outdated
Comment thread ...e/system/auditing/coreos_audit_backlog_limit_kernel_argument/tests/wrong_value_rhel8.fail.sh Outdated
Comment thread linux_os/guide/system/auditing/coreos_audit_option/rule.yml
@jhrozek
Copy link
Copy Markdown
Collaborator

jhrozek commented Oct 6, 2020

OK, so really the only thing I found is the description at linux_os/guide/system/auditing/coreos_audit_backlog_limit_kernel_argument/rule.yml

@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 6, 2020

/test e2e-aws-rhcos4-e8

1 similar comment
@JAORMX
Copy link
Copy Markdown
Contributor Author

JAORMX commented Oct 7, 2020

/test e2e-aws-rhcos4-e8

jhrozek
jhrozek approved these changes Oct 7, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

/lgtm

@JAORMX JAORMX merged commit e8e24fc into ComplianceAsCode:master Oct 7, 2020
@yuumasato yuumasato added this to the 0.1.53 milestone Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evgenyz evgenyz Awaiting requested review from evgenyz

@yuumasato yuumasato Awaiting requested review from yuumasato

2 more reviewers

@redhatrises redhatrises redhatrises left review comments

@jhrozek jhrozek jhrozek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.1.53

Development

Successfully merging this pull request may close these issues.

6 participants

@JAORMX @ggbecker @jhrozek @yuumasato @redhatrises @openshift-ci-robot