Skip to content

implement support for multiple platforms connected with disjunction #6661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Mar 25, 2021

Conversation

vojtapolasek
Copy link
Collaborator

@vojtapolasek vojtapolasek commented Mar 5, 2021

Description:

  • modify build system so that it consumes platforms in two possible way
    either
 platform: <single_platform>

or

platforms:
  - platform_1
  - platform_2
  • internally, the list of platforms in compiled rules is called "platforms"
  • platforms are added in the xccdf rule definition, implicitly connected with OR
  • modify generation of Bash remediations so that rule-specific platforms are placed after inherited platforms and correctly connected with OR
  • the same for Ansible remediations
  • modify ssg test suite unit tests, in particular precompiled rules
  • update documentation

Rationale:

This PR is part of an initiative which plans to implement support for CPE applicability language. This support will allow content authors to specify more granular definition of rule applicability.

@vojtapolasek vojtapolasek added this to the 0.1.55 milestone Mar 5, 2021
@pep8speaks
Copy link

pep8speaks commented Mar 5, 2021

Hello @vojtapolasek! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-03-24 18:34:08 UTC

@openscap-ci
Copy link
Collaborator

openscap-ci commented Mar 5, 2021

Changes identified:
Rules:
 chronyd_or_ntpd_set_maxpoll
Others:
 Changes in Python files.

Show details

Rule chronyd_or_ntpd_set_maxpoll:
 Node moved within OVAL check.
 Deleted attribute from OVAL check.
 Attribute value changed in OVAL check.
 Node deleted from OVAL check.
 Text changed in OVAL check.
 New node inserted to OVAL check.
Others:
 Python abstract syntax tree change found in ssg/build_remediations.py.
 Python abstract syntax tree change found in ssg/build_yaml.py.

Recommended tests to execute:
 (cd build && cmake ../ && ctest -j4)
 build_product rhel8
 tests/test_suite.py rule --libvirt qemu:///system test-suite-vm --remediate-using bash --datastream build/ssg-rhel8-ds.xml chronyd_or_ntpd_set_maxpoll

@vojtapolasek vojtapolasek force-pushed the cpe_or branch 2 times, most recently from 12eb3ae to d892420 Compare March 5, 2021 12:38
@jan-cerny
Copy link
Collaborator

Since there can be multiple platforms in the platform key I would consider renaming the key to platforms. I think a singular form might confuse people to think that only a single value is allowed there.

@vojtapolasek vojtapolasek modified the milestones: 0.1.55, 0.1.56 Mar 8, 2021
@yuumasato yuumasato self-assigned this Mar 9, 2021
Copy link
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.

Looks good to me.

Do you think it makes sense to change at least one rule to have multiple platforms in this PR?
I guess good starting rules would be the chronyd_or_ntpd_*.

@yuumasato
Copy link
Member

The rhel7 build needs to be fixed too, it looks like you are importing something not supported.

@ggbecker
Copy link
Member

Since there can be multiple platforms in the platform key I would consider renaming the key to platforms. I think a singular form might confuse people to think that only a single value is allowed there.

In my opinion, keeping the platform keyword for both cases should be ok. Additionally, I would change the implementation to accept both single value and list of values, for example:

platform: machine
platform:
    - machine
platform:
    - machine
    - some_package_installed

This should be all valid. This way we don't need to change more than 300 files and we keep the old behavior.

Comment on lines +24 to +26
platforms:
- ntp
- chrony
Copy link
Member

Choose a reason for hiding this comment

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

Works nicely

[root@localhost ~]# oscap xccdf eval --profile _stig --rule xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll ./ssg-rhel7-ds.xml 
WARNING: Datastream component 'scap_org.open-scap_cref_security-data-oval-com.redhat.rhsa-RHEL7.xml' points out to the remote 'https://www.redhat.com/security/data/oval/com.redhat.rhsa-RHEL7.xml'. Use '--fetch-remote-resources' option to download it.
WARNING: Skipping 'https://www.redhat.com/security/data/oval/com.redhat.rhsa-RHEL7.xml' file which is referenced from datastream
WARNING: Skipping ./security-data-oval-com.redhat.rhsa-RHEL7.xml file which is referenced from XCCDF content
Title   Configure Time Service Maxpoll Interval
Rule    xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll
Ident   CCE-80439-3
Result  pass

[root@localhost ~]# yum remove ntp
Removed:
  ntp.x86_64 0:4.2.6p5-29.el7_8.2
Complete!

[root@localhost ~]# oscap xccdf eval --profile _stig --rule xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll ./ssg-rhel7-ds.xml 
WARNING: Datastream component 'scap_org.open-scap_cref_security-data-oval-com.redhat.rhsa-RHEL7.xml' points out to the remote 'https://www.redhat.com/security/data/oval/com.redhat.rhsa-RHEL7.xml'. Use '--fetch-remote-resources' option to download it.
WARNING: Skipping 'https://www.redhat.com/security/data/oval/com.redhat.rhsa-RHEL7.xml' file which is referenced from datastream
WARNING: Skipping ./security-data-oval-com.redhat.rhsa-RHEL7.xml file which is referenced from XCCDF content
Title   Configure Time Service Maxpoll Interval
Rule    xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll
Ident   CCE-80439-3
Result  notapplicable

[root@localhost ~]# yum install chrony
Installed:
  chrony.x86_64 0:3.4-1.el7

Complete!
^[[A[root@localhost oscap xccdf eval --profile _stig --rule xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll ./ssg-rhel7-ds.xml 
WARNING: Datastream component 'scap_org.open-scap_cref_security-data-oval-com.redhat.rhsa-RHEL7.xml' points out to the remote 'https://www.redhat.com/security/data/oval/com.redhat.rhsa-RHEL7.xml'. Use '--fetch-remote-resources' option to download it.
WARNING: Skipping 'https://www.redhat.com/security/data/oval/com.redhat.rhsa-RHEL7.xml' file which is referenced from datastream
WARNING: Skipping ./security-data-oval-com.redhat.rhsa-RHEL7.xml file which is referenced from XCCDF content
Title   Configure Time Service Maxpoll Interval
Rule    xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_set_maxpoll
Ident   CCE-80439-3
Result  pass

also change some conditions which check if platforms were actually defined, they were abusing fact that platforms were defined as lists
@openshift-ci
Copy link

openshift-ci bot commented Mar 24, 2021

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

Test name Commit Details Rerun command
ci/prow/e2e-aws-ocp4-e8 5964f0f link /test e2e-aws-ocp4-e8
ci/prow/e2e-aws-ocp4-cis-node 5964f0f link /test e2e-aws-ocp4-cis-node

Full PR test history. Your PR dashboard.

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.

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.

6 participants