-
Notifications
You must be signed in to change notification settings - Fork 24
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
CMP-2401: Add STIG reference parser #494
CMP-2401: Add STIG reference parser #494
Conversation
/hold for test |
@yuumasato It is weird. I noticed there is stig annotation for stig in the master branch without the PR:
|
@xiaojiey The This PR adds the following annotations listing more specific information about the STIG requirement implemented: And extends the
vs
|
Verification pass with 4.16.0-0.nightly-2024-02-29-062601:
|
/unhold |
@yuumasato: This pull request references CMP-2401 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.16.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
@Vincent056 @rhmdnd This should be ready for review |
@rhmdnd @Vincent056 I have added a parser for SRGs. When content from ComplianceAsCode/content#11647 is deployed, we can see the following annotations:
Do you have any opinion on the control name |
/hold for test |
Got the same result with comment #494 (comment) when content from ComplianceAsCode/content#11647 deployed |
/unhold |
/label qe-approved |
pkg/profileparser/profileparser.go
Outdated
} | ||
srgperr := p.registerStandard("SRG-APP-CTR",`^https://public\.cyber\.mil/stigs/downloads/\?_dl_facet_stigs=container-platform$`) | ||
if srgperr != nil { | ||
log.Error(nciperr, "Could not register SRG-APP-CTR reference parser") // not much we can do here.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see where someone reading the logs might get confused by what this means compared to STIG one above.
If they're both related to the STIG, could we use the same error message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my, you are totally correct.
Both the STIG ID CNTR-OS-XXXXXX
and container SRG SRG-APP-XXXXXX-CTR-XXXXXX
are found at https://public.cyber.mil/stigs/downloads/?_dl_facet_stigs=container-platform
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With same URI for both the STIGIDs and SRGs, the references will be listed together under the same name STIG
:
$oc get rule ocp4-api-server-tls-security-profile -o=jsonpath={.metadata.annotations} | jq -r
{
"compliance.openshift.io/image-digest": "pb-ocp4jm9bc",
"compliance.openshift.io/profiles": "ocp4-nerc-cip,ocp4-moderate-rev-4,ocp4-high-rev-4,ocp4-high,ocp4-moderate,ocp4-stig-v1r1,ocp4-stig",
"compliance.openshift.io/rule": "api-server-tls-security-profile",
"control.compliance.openshift.io/NIST-800-53": "SC-8;SC-8(1)",
"control.compliance.openshift.io/STIG": "SRG-APP-000014-CTR-000040;SRG-APP-000560-CTR-001340;CNTR-OS-000020",
"policies.open-cluster-management.io/controls": "SC-8,SC-8(1),SRG-APP-000014-CTR-000040,SRG-APP-000560-CTR-001340,CNTR-OS-000020",
"policies.open-cluster-management.io/standards": "NIST-800-53,STIG"
}
"control.compliance.openshift.io/STIG": "SRG-APP-000014-CTR-000040;SRG-APP-000560-CTR-001340;CNTR-OS-000020",
This should align with the cells in the STIG spreadsheets, right? |
This ensures that rules part of STIG profile contain annotations with the STIGID.
4b1088a
to
6458b8e
Compare
Yes, but not necessarily. |
@Vincent056 should be ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@GroceryBoyJr: changing LGTM is restricted to collaborators In response to this:
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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GroceryBoyJr, rhmdnd, yuumasato The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This ensures that rules part of STIG profile contain annotations with the STIGID.
One can see the STIG references as annotation on the rule when deployed together with content from ComplianceAsCode/content#11593