Generated Bash scripts should separate individual Bash remediations

[jan-cerny]

Description of Problem:

When the bash remediation script is generated by the oscap xccdf generate fix command from a SCAP source data stream, OpenSCAP simply iterates over all rules in the data stream and blindly concatenates all remediations together.
But, some remediations for some rules can be influenced by previous remediations in that script. The code might depend on assumptions that the variables used in the remediations don't exist or are empty or unset. But, the variables aren't cleaned so they leak and might influence the code in next remediations.

When people write the remediations in the content project, they assume that it's a standalone script. It's also executed for each remediation separately when you run oscap xccdf eval --remediate. So the only situation when every remediation isn't a separate script is when you generate bash script using oscap xccdf generate fix. So we think that the right place where to fix this problem is in the OpenSCAP's generator code.

This applies to both profile and results oriented scripts.

OpenSCAP Version:

all, current upstream as of 2021-09-17

Operating System & Version:

all

Steps to Reproduce:

1. oscap xccdf generate fix --profile $your_profile_id $scap_source_data_stream_file > fix.sh
2. view fix.sh

Actual Results:

remediation scripts for multiple rules are just concatenated and possibly influence each other when the script is executed

Expected Results:

We think that we solve the problem this way: Each rule remediation is wrapped inside a function and this function is the only statement executed in the remediation. The variables used in a function won't influence other functions therefore the variables used in one rule's remediation script shouldn't influence other remediation scripts.

Additional Information / Debugging Steps:

This has been revealed in the ComplianceAsCode/content project when we converted some legacy bash remediation functions to Jinja macros. Jinja macros aren't functions, so some variables that were local inside a function (their scope was limited to the function body) have suddenly become global and influencing the code below them.

We probably can't expect people that they will unset every variable after use or clean every variable before use. That would be difficult to enforce, hard to test and it would made the bash code more complex.

CC @dodys and @j-ode for awareness.

[matejak]

The problem is related to the question "to what degree should the scanner understand the structure of underlying remediations?". The scanner can't do much here either. In Bash, variables in functions are global, unless explicitly declared as local, so wrapping them in functions won't solve anything. I don't get the idea about "... this function is the only statement executed in the remediation" - if remediations are executed in the --remediate mode, then I assume that there is no issue, and if we are talking about a profile Bash remediation, then I want everything fixed, not one rule only.

Our Bash remediations are complex, they also contain here-documents, so they are fragile (sensitive even to indentation), and given that Bash can't be parsed easily, the amount of reasonable assumptions one can make about the code is extremely low.

[evgenyz]

Oh, we can do

$(function_1)
$(function_2)
...

This way they won't pollute parent shell and each other.

[evgenyz]

And we to some degree dive inside all our remediations: Ansible, Blueprint :)

[matejak]

Oh, we can do

$(function_1)
$(function_2)
...

This way they won't pollute parent shell and each other.

That's correct, that would work. However, I can't get myself to like auto-wrap of Bash code to functions, it seems too defensive with potential disadvantages. Auto-formatting code is risky, for instance this would create problems if the original code already contained defined functions. We can avoid this in the ComplianceAsCode content, but what about people out there?

[evgenyz]

Auto-formatting code is risky, for instance this would create problems if the original code already contained defined functions.

What problems? Having a function-in-a-function is pretty much OK in Bash, AFAIK. Do you have something special on your mind?

Okay, okay, I get it. Everything has its drawbacks. But what are you proposing instead?

And, by the way, this wrapping is attractive from the point of view of progress update during the first boot remediation. That's why it got me.

[evgenyz]

Fixed in #2072.