Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Prevent unqualified CIS and STIGID references #6871

Merged
merged 4 commits into from Apr 26, 2021
Merged

Prevent unqualified CIS and STIGID references #6871

merged 4 commits into from Apr 26, 2021

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented Apr 21, 2021 •
edited

Description:

Prevent unqualified CIS and STIGID references just about sums it up. A couple of rules have unqualified CIS references; this removes them and enforces it at build time.

Rationale:

Per discussion on #6416, I don't think it is generally valid to have a
unqualified (by product) CIS reference identifier. While there is a
hierarchy to the CIS benchmarks (from General Linux to e.g., Debian
Family to e.g., Ubuntu), references aren't necessarily the same across
families (e.g., Ubuntu to RHEL) and order of the sections can differ
significantly. The same holds for STIG: unless a product undergoes STIG
certification (thus assigning rules a specific STIG ID for the product),
we shouldn't reuse one product's STIG IDs for other, unrelated products.

In the future, we might want to offer a "hybrid" form (e.g., cis@ubuntu)
applying to all versions of a product, but otherwise I think this is the
best route to go.

Start enforcing this at build time, to ensure we (and other
contributors) don't accidentally undo this work in the future.


Mostly this has been replaced with rhel7/rhel8 references (my apologies
to the SUSE and Oracle contributors), but one Fedora-specific and one
OL-specific rule was caught and fixed appropriately.

See also: comment thread on #6416.

@cipherboy cipherboy added enhancement General enhancements to the project. Infrastructure Our content build system standards Benchmarks related. labels Apr 21, 2021
@cipherboy cipherboy self-assigned this Apr 21, 2021
@pep8speaks
Copy link

pep8speaks commented Apr 21, 2021 •
edited

Hello @cipherboy! 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-04-23 13:02:23 UTC

@cipherboy cipherboy added this to the 0.1.56 milestone Apr 21, 2021
@openscap-ci
Copy link
Collaborator

openscap-ci commented Apr 21, 2021 •
edited

Changes identified:
Others:
 Changes in Python files.

Show details

Others:
 Python abstract syntax tree change found in ssg/build_yaml.py.

Recommended tests to execute:
 (cd build && cmake ../ && ctest -j4)

@cipherboy cipherboy force-pushed the prevent-unqualified-cis-references branch from 641ea53 to f89770b Compare April 21, 2021 14:18
@cipherboy cipherboy mentioned this pull request Apr 21, 2021
Closed
@cipherboy
Copy link
Contributor Author

/retest

@yuumasato yuumasato self-assigned this Apr 22, 2021
@cipherboy
Copy link
Contributor Author

Thanks for checking the RHEL references, @yuumasato!

@yuumasato
Copy link
Member

yuumasato commented Apr 22, 2021 •
edited

I'm checking the rest. I think I submitted the review by accident.

@yuumasato
Copy link
Member

yuumasato commented Apr 22, 2021 •
edited

Tip, when in "Changed files" tab, you can batch the approval of suggestions.

Edited for clarity.

yuumasato
yuumasato reviewed Apr 22, 2021
View reviewed changes
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.

Thanks for adding this validation during build time.

I made a bunch of suggestion regarding RHEL references.

linux_os/guide/services/obsolete/talk/package_talk-server_removed/rule.yml Outdated Show resolved Hide resolved
linux_os/guide/services/obsolete/talk/package_talk_removed/rule.yml Outdated Show resolved Hide resolved
linux_os/guide/services/obsolete/telnet/package_telnet-server_removed/rule.yml Outdated Show resolved Hide resolved
linux_os/guide/services/obsolete/telnet/service_telnet_disabled/rule.yml Outdated Show resolved Hide resolved
linux_os/guide/services/obsolete/tftp/service_tftp_disabled/rule.yml Outdated Show resolved Hide resolved
..._os/guide/system/auditing/auditd_configure_rules/audit_rules_usergroup_modification/rule.yml Outdated Show resolved Hide resolved
..._configure_rules/audit_kernel_module_loading/audit_rules_kernel_module_loading_init/rule.yml Outdated Show resolved Hide resolved
...configure_rules/audit_kernel_module_loading/audit_rules_kernel_module_loading_finit/rule.yml Outdated Show resolved Hide resolved
...onfigure_rules/audit_kernel_module_loading/audit_rules_kernel_module_loading_delete/rule.yml Outdated Show resolved Hide resolved
ssg/build_yaml.py Show resolved Hide resolved
@yuumasato
Copy link
Member

@brett060102 @teacup-on-rockingchair @freddieRv @iokomin Hi, just a heads up that this will affect some CIS references for some rules available for OL and SLE.

@cipherboy
Prevent unqualified CIS and STIGID references
58e73f1 
Per discussion on ComplianceAsCode#6416, I don't think it is generally valid to have a
unqualified (by product) CIS reference identifier. While there is a
hierarchy to the CIS benchmarks (from General Linux to e.g., Debian
Family to e.g., Ubuntu), references aren't necessarily the same across
families (e.g., Ubuntu to RHEL) and order of the sections can differ
significantly. The same holds for STIG: unless a product undergoes STIG
certification (thus assigning rules a specific STIG ID for the product),
we shouldn't reuse one product's STIG IDs for other, unrelated products.

In the future, we might want to offer a "hybrid" form (e.g., cis@ubuntu)
applying to all versions of a product, but otherwise I think this is the
best route to go.

Start enforcing this at build time, to ensure we (and other
contributors) don't accidentally undo this work in the future.

Signed-off-by: Alexander Scheel <alex.scheel@canonical.com>
@cipherboy
Replace unqualified CIS references with qualified
813c090 
Mostly this has been replaced with rhel7/rhel8 references (my apologies
to the SUSE and Oracle contributors), but one Fedora-specific and one
OL-specific rule was caught and fixed appropriately.

Signed-off-by: Alexander Scheel <alex.scheel@canonical.com>
@cipherboy cipherboy force-pushed the prevent-unqualified-cis-references branch from b41263f to 1bd59f5 Compare April 22, 2021 17:06
@cipherboy @yuumasato
Apply suggestions from wsato's code review
f9b0e28 
These fix up various CIS identifiers in RHEL 7 and RHEL 8 content.

Co-authored-by: Watson Yuuma Sato <wsato@redhat.com>
Signed-off-by: Alexander Scheel <alex.scheel@canonical.com>
@cipherboy cipherboy force-pushed the prevent-unqualified-cis-references branch from 1bd59f5 to f9b0e28 Compare April 22, 2021 17:07
@cipherboy
Copy link
Contributor Author

I guess the only other question I have left is, do we want this as part of build or as part of test suite? It seems easy enough to add it here during build and enforces logic we already had, so I'm inclined to leave it as submitted in this PR.

yuumasato
yuumasato reviewed Apr 23, 2021
View reviewed changes
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.

I guess the only other question I have left is, do we want this as part of build or as part of test suite? It seems easy enough to add it here during build and enforces logic we already had, so I'm inclined to leave it as submitted in this PR.

Yeah, I'd rather check this during build time. The sooner the better.

linux_os/guide/system/software/updating/ensure_fedora_gpgkey_installed/rule.yml Outdated Show resolved Hide resolved
@cipherboy @yuumasato
Update linux_os/guide/system/software/updating/ensure_fedora_gpgkey_i…
8b5d34f 
…nstalled/rule.yml

Co-authored-by: Watson Yuuma Sato <wsato@redhat.com>
@JAORMX
Copy link
Contributor

JAORMX commented Apr 23, 2021

/retest

@JAORMX
Copy link
Contributor

JAORMX commented Apr 23, 2021

Seems OCP infra is having issues deploying clusters for testing :/

@cipherboy
Copy link
Contributor Author

/retest

@yuumasato
Copy link
Member

All products build and pass CI.

@yuumasato yuumasato merged commit 20b7e4d into ComplianceAsCode:master Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matusmarhefka matusmarhefka matusmarhefka left review comments

@yuumasato yuumasato yuumasato approved these changes

@JAORMX JAORMX JAORMX approved these changes

@matejak matejak Awaiting requested review from matejak

Assignees

@cipherboy cipherboy

@yuumasato yuumasato

Labels
enhancement General enhancements to the project. Infrastructure Our content build system standards Benchmarks related.
Projects
None yet
Milestone
0.1.56
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@cipherboy @pep8speaks @openscap-ci @yuumasato @JAORMX @matusmarhefka