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.

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

Select rules on the command line #1832

Merged
merged 10 commits into from
Jan 18, 2022
Merged

Conversation

jan-cerny
Copy link
Member

We introduce some changes to let users modify the list of rules to be evaluated more easily by command line options. Often users want to skip some rules or evaluate just a couple of rules. Currently, they need to create a tailoring file or build a new profile to do that. With this PR, they will be able to do it on the fly by adding command line options.

Specifically, this will

  • Enable multiple --rule options to allow users to cherry-pick more rules at once (rhbz#2020581)
  • Add new --skip-rule option to allow users to skip some rules (rhbz#2020580)

For more details, please read commit message of every commit.

@jan-cerny jan-cerny added this to the 1.3.6 milestone Nov 25, 2021
@evgenyz
Copy link
Contributor

evgenyz commented Nov 26, 2021

I have an important question. How this feature supposed to interact with requires and conflicts?

For example, if in /tests/API/XCCDF/unittests/test_xccdf_requires_conflicts.sh you change the command line to $OSCAP xccdf eval --rule xccdf_moc.elpmaxe.www_rule_2 --results "$result" "$source", what should be the result? And what about $OSCAP xccdf eval --rule xccdf_moc.elpmaxe.www_rule_2 --rule xccdf_moc.elpmaxe.www_rule_3 --results "$result" "$source"?

Also, what if --profile is provided as well?

@jan-cerny
Copy link
Member Author

Great question. The --rule feature is another layer on the top of resolved selections, profile selections, requires, and conflicts. It doesn't influence the requires and conflicts resolution. The scanner resolves the profiles, the requires and conflict as usual, according to specification, and then simply skips the rules that are not listed in --rule. That means:

  • if the rule is not a part of the selected profile it won't be evaluated because it wouldn't be evaluated even without the --rule option
  • if the rule conflicts with some other rule it won't be evaluated because it wouldn't be evaluated even without the --rule option
  • if rule A requires rule B and user provides only --rule A it will evaluate only rule A and skip rule B

Now your answers are a consequence of this behavior:

  • oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_2 - even if rule 2 is selected in the default profile, oscap won't evaluate anything because rule 2 conflicts with rule 3 which is also selected in the default profile.
  • oscap xccdf eval --profile xccdf_org.open-scap_profile_override --rule xccdf_moc.elpmaxe.www_rule_2 - same as previous, the profile doesn't change the selections of the rule 2 or 3, so the result is the same
  • oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_2 --rule xccdf_moc.elpmaxe.www_rule_3 evaluates only rule 3 , rule 2 isn't evaluated because rule 2 conflicts with rule 3, rule 4 isn't evaluated despite it's required by rule 3 because it wasn't listed on the command line, once you provide a --rule option, oscap evaluates only the rules explicitly listed
  • oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_2 --rule xccdf_moc.elpmaxe.www_rule_3 --profile xccdf_org.open-scap_profile_override evaluates only rule 3 because rule 2 conflicts with rule 3, the profile doesn't change the selections of the rule 2 or 3, so the result is the same
  • oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_3 evaluates only rule 3, despite the fact that rule 3 requires rule 4, but the rule 4 got skipped because it isn't listed, once you provide a --rule option, oscap evaluates only the rules explictely listed
  • oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_3 --rule xccdf_moc.elpmaxe.www_rule_4 evaluates both rule 3 and 4, it doesn't evaluate rule 1, despite rule 4 requires rule 1, but rule 1 wasn't listed on command line
  • oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_3 --rule xccdf_moc.elpmaxe.www_rule_4 --profile xccdf_org.open-scap_profile_override evaluates only rule 3, it doesn't evaluate rule 4, because rule 4 requires rule 1, but rule 1 is unselected by the override profile therefore rule 4 can't be evalauted

Now the question is whether the behavior is expected or isn't stupid.

For example, in the aforementioned situation oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_3, an user will be confused why rule 4 wasn't evaluated if rule 3 has the <requires idref="rule 4"> element. But if we change the behavior and oscap will evaluate both rules 3 and 4 in this situation, a different user will be confused, because he wanted to evaluate just rule 3 and will wonder why rule 4 is evaluated.

For example, in the aforemenation situation oscap xccdf eval --rule xccdf_moc.elpmaxe.www_rule_2 an user will be confused why the rule 2 wasn't evaluated if he explicetly provided the --rule option. But if we change the behavior to evaluate the rule 2 in this situation, a different user will be confused because the rule is in conflict with other rule and yet it's evaluated.

My impression is that the goal of the feature was to provide user a simple way to select a single rule or 2-3 rules so maybe we might want to ignore the requires and conflicts here?

@evgenyz
Copy link
Contributor

evgenyz commented Nov 26, 2021

I have no idea how this feature should work, it's better to ask stakeholders (who they are BTW?). But I could suggest either extending test_xccdf_requires_conflicts.sh with --rule cases or adding some requires and conflicts to test_skip_rule and co. Whatever path we choose we must ensure that it won't be broken later.

@jan-cerny
Copy link
Member Author

There is a discussion with the BZ reporter in comment 4 https://bugzilla.redhat.com/show_bug.cgi?id=2020581#c4

Actually I like his view for development and testing:

For my development and testing scenarios, evaluating all rules specified with --rule (irrespectively of selections and conflicts) and including required rules would make the most sense.

Copy link
Contributor

@evgenyz evgenyz left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM.

assert_exists 1 '//rule-result[@idref="xccdf_moc.elpmaxe.www_rule_17"]/result[text()="notselected"]'


# test the "conflicts" lemeent eith the --rule option
Copy link
Contributor

Choose a reason for hiding this comment

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

WUT? lemeent eith?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# test the "conflicts" lemeent eith the --rule option
# test the "conflicts" element with the --rule option

@jan-cerny
Copy link
Member Author

@evgenyz The only thing missing is the automated addition of required rules. Is that correct?

@jan-cerny
Copy link
Member Author

I'm afraid that the change of behavior could break SSGTS, because that uses --rule and expects that a single rule will be evaluated. Moreover, it wouldn't be an easy change: Now you iterate over the benchmark and evaluate or skip as you go, to implement automated evaluation of the rules you would have to iterate twice - first to identify and collect all the dependencies and second to evaluate all rules and their dependencies. It's a tree structure, not a list. So instead of implementing logic that would bring the required rules into the scope I have added a warning to inform the user that there is a required dependency.

/* If at least one --rule option has been provided by the user on the
* command line skip reporting for the other rules - only the selected
* rule(s) will be reported. */
if (oscap_htable_itemcount(policy->rules) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not _user_specified_rule_mode(policy)?

/* If user wants to evaluate only specific rules and the rule currently
* being evaluated is not among these rules, do not evaluate it and mark it
* as notselected. */
if (_user_specified_rule_mode(policy) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Or use oscap_htable_itemcount(policy->rules) > 0 here and get rid of that function.

utils/oscap.8 Outdated
@@ -108,7 +108,12 @@ Select a particular profile from XCCDF document. If "(all)" is given a virtual p
.TP
\fB\-\-rule RULE\fR
.RS
Select a particular rule from XCCDF document. Only this rule will be evaluated. Rule will use values according to the selected profile. If no profile is selected, default values are used.
Select a particular rule from XCCDF document. Only this rule will be evaluated. Rule will use values according to the selected profile. If no profile is selected, default values are used. This option can be used multiple times to specify multiple rules at once.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth mentioning that dependencies won't be evaluated?

utils/oscap.8 Outdated
@@ -108,7 +108,7 @@ Select a particular profile from XCCDF document. If "(all)" is given a virtual p
.TP
\fB\-\-rule RULE\fR
.RS
Select a particular rule from XCCDF document. Only this rule will be evaluated. Rule will use values according to the selected profile. If no profile is selected, default values are used. This option can be used multiple times to specify multiple rules at once.
Select a particular rule from XCCDF document. Only this rule will be evaluated. Any other rules required by this rule rule won't be evaluated. Rule will use values according to the selected profile. If no profile is selected, default values are used. This option can be used multiple times to specify multiple rules at once.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Select a particular rule from XCCDF document. Only this rule will be evaluated. Any other rules required by this rule rule won't be evaluated. Rule will use values according to the selected profile. If no profile is selected, default values are used. This option can be used multiple times to specify multiple rules at once.
Select a particular rule from XCCDF document. Only this rule will be evaluated. Any other rules required by this rule won't be evaluated. Rule will use values according to the selected profile. If no profile is selected, default values are used. This option can be used multiple times to specify multiple rules at once.

@evgenyz
Copy link
Contributor

evgenyz commented Jan 18, 2022

@jan-cerny Can you please rebase it?

When multiple --rule options will be specified on the command line,
OpenSCAP will evaluate multiple rules at once.

Resolves: rhbz#2020581
The option will allow users to easily tell oscap to skip some
rules without having to create a tailoring file. User will simply
provide --skip-rule RULE_ID option on the command line to skip
evaluation of the rule RULE_ID.

Resolves: rhbz#2020580
The rule `xccdf_moc.elpmaxe.www_rule_17` requires group
`xccdf_moc.elpmaxe.www_group_1_2` which is a typo or mistake because
group `xccdf_moc.elpmaxe.www_group_1_2` doesn't exist. Instead, there is
group `xccdf_moc.elpmaxe_group_1_2` (the correct group ID doesn't
contain `.www`). To fix this, we could simply fix the ID in the `requires`
element of rule `xccdf_moc.elpmaxe.www_rule_17`. But, since all other
IDs in this XCCDF are in a form of `xccdf_moc.elpmaxe.www_.*` and we
like consistency we will instead rename the group so that all IDs follow
the same format.
The purpose of these new test cases is to preserve the current behavior
of rules with requires and conflicts elements when `oscap` is invoked
with the `--rule` option selecting specific rule or rules. The tests are
based on current behavior, not on any specification, as `--rule` option
is our custom feature, but at the same time we want notice behavior
changes in future.
This allows us to modify the final selection at other places without
consulting the real selection in the XCCDF document.
If the user explicitly requests a rule using --rule, they expect all
the rules would be evaluated regardless selections and conflicts.
The use-case for the `--rule` option is development and testing
and in these situation it anyways behaves as an override.

However, this patch doesn't solve the "requires" element handling.
Required rules are still not added.
If at least one `--rule` option is provided by the user, only the rules
listed as arguments of `--rule` options are evaluated. It ignores the
`requires` element in the rule. The required rules aren't added
automatically and aren't evaluated unless they're explicitely listed.
This way we keep the behavior of `--rule` option in previous versions of
OpenSCAP, because SSGTS relies on the fact that single occurence of
`--rule` options means that at most 1 rule is evaluated.

To avoid confusion, OpenSCAP will now report a warning that the rule
they are evaluating a requires another rule.
@jan-cerny
Copy link
Member Author

@evgenyz rebased, thanks

@evgenyz evgenyz merged commit 57154f2 into OpenSCAP:maint-1.3 Jan 18, 2022
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.

None yet

2 participants