Fix STIG IDs reference processing #4725
Conversation
|
The |
|
Yes. This is very intentional and should not be changed as it breaks downstream users functionality. |
|
@redhatrises Could you please explain in greater detail? This PR allows rules to have multiple STIG IDs, and it doesn't change anything for rules that have one or zero STIG IDs. The product-prefix I was referring to is related to input, i.e. to yaml files, the STIG ID appearance in the datastream won't change. |
jan-cerny
force-pushed
the
stigid_comma
branch
from
e3e330f
to
5beaaa5
Compare
There can only be 1 STIGID to a rule not multiple. Enabling multiple STIG IDs to a rule is introducing a bug. |
|
@redhatrises Aha, OK, I didn't know that. Then this patch isn't needed. But, because this limitation is not obvious, I suggest adding a check to the Python code to verify that there is a single ID and its format is correct. We also still can merge the unit test change from in 2ba63a9 and then we could remove the code that handled inconsistent usage of |
|
I think that there is nothing wrong with this PR - it gets some things right, and as a side-effect, it opens door to multiple STIG IDs in the content. If @jan-cerny adds some assert to @redhatrises Could you please provide a reference that we could use to document the test, so it is easy to determine why two and more STIG IDs are a no-go? I was unable to find this claim on my own. |
Not sure what is meant -- reference to document the test? Each configuration rule must attest a single configuration action, and receive a single CCE. Each configuration rule should have a single DISA reference as well. Sometimes DISA's rules accidentally evaluate two configuration settings in a single rule; in those cases we alert them to the mistake and they're separated out. |
|
@shawndwells Why have you closed that PR? Right now, there is nothing that could stop content authors to define multiple STIG IDs to one rule. This PR can fix this easily. |
|
@matejak I have changed the code to raise an exception when somebody tries to have multiple STIG IDs in a rule. |
|
I believe that these changes are good to go. I will create a PR to talk about reference handling in the documentation... I thought I did already did that to reduce confusion for authors, but I must have dreamt it! |
We have stopped using the prefixed format RHEL-06- in 5897bac After that we don't need the functionality anymore therefore we don't need to test it.
This PR allows to assign a rule multiple STIG IDs using a comma-separated list which is the same way we use for the other references. There were 2 basic problems: 1. Python code converting YAML to shorthand was not splitting the string at comma. 2. XSLT template that converts shorhand to XCCDF had a special treatment for STIG IDs which always copies the whole attribute value instead of each identifier which lead to duplicate `reference` elements.
The code will raise an exception when rule author tries to define multiple STIG IDs in a single rule using a comma-separated list. The patch also extends unit tests to cover this case.
jan-cerny
force-pushed
the
stigid_comma
branch
from
fef7897
to
8496078
Compare
|
@matejak rebased on the top of master |
|
Great, thank you for the effort and everybody for remarks! |
Description:
This PR
allows to assign a rule multiple STIG IDs using a comma-separated list which is the same way we use for the other references.prevents people from assigning a rule multiple STIG IDs using a comma-separated list because a rule can have only 1 STIG ID.There were 2 basic problems:
referenceelements.Note: there was some
stigid-concatwhich I can't find anywhere and it seems to be evaluated as empty string which means result of concatenating it with any string is that string.Rationale:
In #4706 we encountered that using a comma separated list for RHEL7 STIG ID doesn't work.