-
Notifications
You must be signed in to change notification settings - Fork 706
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
Add script to update generated SDS to SCAP 1.3 #4302
Add script to update generated SDS to SCAP 1.3 #4302
Conversation
Hello @yuumasato! 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 2019-04-29 14:41:25 UTC |
64ef48a
to
2d3f3e2
Compare
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.
Please also fix PEP8 issues.
component_id = component_id[1:] | ||
|
||
# Locate the <xccdf:check> of the <xccdf:Rule> with id security_patches_up_to_date | ||
oval_check = datastreamtree.find(".//ds:component[@id='%s']//xccdf:Rule[@id='xccdf_org.ssgproject.content_rule_security_patches_up_to_date']/xccdf:check[@system='%s']" % (component_id, oval_ns), ns ) |
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.
This way of using namespaces us not supported in Python 2.6. You need to use eg.
find(".//{%s}Benchmark" % (ssg.constants.XCCDF12_NS))
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 have stopped using the namespace dictionary in favor of suggested namespace approach.
component_id = component_id[1:] | ||
|
||
# Locate the <xccdf:check> of the <xccdf:Rule> with id security_patches_up_to_date | ||
oval_check = datastreamtree.find(".//ds:component[@id='%s']//xccdf:Rule[@id='xccdf_org.ssgproject.content_rule_security_patches_up_to_date']/xccdf:check[@system='%s']" % (component_id, oval_ns), ns ) |
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.
Pyhton 2.6 doesn't support XPath attributes, you have to loop and filter manually:
for rule in rules:
id_ = rule.get("id")
if id_ == "xccdf_org.ssgproject.content_rule_security_patches_up_to_date":
checks = ....
...
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.
That is unfortunate.
I have implemented the filtering manually.
|
||
# Add the component-ref to list of datastreams' checks | ||
ds_checks = datastreamtree.find(".//ds:checks", ns) | ||
check_component_ref = ds_checks.findall("ds:component-ref[@id='%s']" % component_ref_uri[1:], ns) |
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.
It would be nice to have a comment explaining this [1:]
.
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 have made it explicit, and also added a comment.
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.
Great, but there are still 2 other occurrences of this [1:]
which might not be clear.
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.
Sure, thank you for asking for these clarifications.
I have added some comments.
It is not supported in python 2.6
Use of 'attrib' parameter is not supported in python 2.7.
Python 2.6 doesn't support attribute filtering in XPath expressions. For every findall, run through the elements lookcing for the one which interests us.
705809d
to
bbcd271
Compare
|
||
# The component-ref ID is the catalog uri without leading '#' | ||
component_ref_feed_id = component_ref_uri[1:] | ||
|
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.
When I build RHEL 7 data stream I get in scap_org.open-scap_cref_ssg-rhel7-pcidss-xccdf-1.2.xml component-ref this element:
<ns2:uri name="ecurity-data-oval-com.redhat.rhsa-RHEL7.xml.bz2" uri="#scap_org.open-scap_cref_ecurity-data-oval-com.redhat.rhsa-RHEL7.xml.bz2"/>
. There is missing s
2 times.
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.
Nice catch, I ended up introducing a bug during manual filtering. Fixed in ce7ec59.
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.
oval_ns = ssg.constants.oval_namespace | ||
xccdf_ns = ssg.constants.XCCDF12_NS | ||
ds_ns = ssg.constants.datastream_namespace | ||
xlink_ns = ssg.constants.xlink_namespace |
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.
This is just renaming variables. Please use them directly.
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 have updated the import definition and changed code to use them.
The inspection completed: 4 updated code elements |
@jan-cerny Thank you for all the reviews. |
Here is validation report from rhel8 content, with |
With these changes, OpenSCAP 1.3.0 will fail to scan if
This should be fixed in OpenSCAP. |
@yuumasato Moreover, |
Great job! |
Description:
How to test
Use scapval-1.3.2
openjdk-1.8.0
on a RHEL environment. (for some reason it doesn't work on Fedora)-Xmx1024m
Workaround bugs in scap-schematron-rules-1.3.2 (just to test that scapval passes):
group_id
anduser_id
cannot be zero, tracked here Multiple schematron false-positives OVAL-Community/OVAL#23sed -i -E "s/(group_id .*\">)/\1-1/" ./ssg-rhel7-ds.xml
sed -i -E "s/(user_id .*\">)/\1-1/" ./ssg-rhel7-ds.xml
security_patches_up_to_date
is causing fail of SRC-377, when requirement is about OVAL checks. I have e-mailed NIST about this to try to sort out what is the issue.awk '/check-content-ref.*up_to_date_ocil/{for(x=NR-1;x<=NR+1;x++)d[x];}{a[NR]=$0}END{for(i=1;i<=NR;i++)if(!(i in d))print a[i]}' ./ssg-rhel7-ds.xml > ./ssg-rhel7-ds-1.3.xml
Validate it:
java -Xmx1024m -jar scapval-1.3.2.jar -file ./ssg-rhel7-ds-1.3.xml -scapversion 1.3 > scap-1.3.log
Report of scapval with workarounds: https://fedorapeople.org/~wsato/scap-1.3/validation-report.html
Rationale:
oscap ds sds-compose