Simplify banner text syntax and add utility to generate banner regular expression

[dahaic]

Description:

• The login banner text syntax is simplified. With banner texts having every whitespace replaced with more complex regular expression, it's not really readable in that form.
• Introduced banner_regexify filter. This filter should provide way to write human readable text in source, and get machine readable text as the output. Banner text can be written as if writing a printf statement.
• Removed unnecessary sed calls in Bash remediations
• Added / fixed Ansible remediations for banners
• Added utility to generate regular expressions for banners. This utility can be used when customizing content.

Rationale:

• Usability of banner text sources is dismal.
• Source for banner should be human readable, machine readable banner can be complex.

Fixes: #4574
Fixes: #4387
Fixes: RHBZ#1776780

[dahaic]

The idea is this macro will be used in the variable definition. Unfortunately variables are parsed without jinja macro support. So I am stuck :D Help @yuumasato

[yuumasato]

@dahaic The patch to expand macros during variable loading may be as simples as:

--- a/ssg/build_yaml.py
+++ b/ssg/build_yaml.py
@@ -305,7 +305,7 @@ class Value(object):
 
     @staticmethod
     def from_yaml(yaml_file, env_yaml=None):
-        yaml_contents = open_and_expand(yaml_file, env_yaml)
+        yaml_contents = open_and_macro_expand(yaml_file, env_yaml)
         if yaml_contents is None:
             return None
 

I haven't tested, :)

[dahaic]

@dahaic The patch to expand macros during variable loading may be as simples as:

--- a/ssg/build_yaml.py
+++ b/ssg/build_yaml.py
@@ -305,7 +305,7 @@ class Value(object):
 
     @staticmethod
     def from_yaml(yaml_file, env_yaml=None):
-        yaml_contents = open_and_expand(yaml_file, env_yaml)
+        yaml_contents = open_and_macro_expand(yaml_file, env_yaml)
         if yaml_contents is None:
             return None
 

I haven't tested, :)

It seems to work perfectly, thank you! <3

[yuumasato]

I've noticed two artifacts.

1. There is an extra ) at the end of some banners.
2. dod_short banner generates an artifact when expanding the escape of a single quote.

[dahaic]

I've also changed the macro a little bit - there's no need to replace \r so I have removed that replace call. \n is the expected newline sequence.

[redhatrises]

\n and spaces need to be maintained across all the banners. It's missing in some of the banners, and therefore, changes the legality of the banner.

[dahaic]

Updated sources again, as \' was reduced to ', and removed whitespace regex right after "bullet points" in DOD banners.

There are still some changes - I have just checked the outputs, and there are following differences against master:

• dod_short (in linux_os/guide/system/accounts/accounts-banners/login_banner_text.var only!) begins with I(\\\')*(\')*ve which definitely seems to be a bug.
• dss_odaa_default (in linux_os/guide/services/http/securing_httpd/httpd_secure_content/var_web_login_banner_text.var only!) begins with [\\s\\n]+ which again seems to be a bug.
• rest of the differences is original regexes having [\s\n]* pretty randomly in the text. This again looks like a bug (I am pretty sure that banner that contains words like systemmonitored is invalid :) ).

For the evaluation, I have used following command to generate stable output:
grep 'dod_banners\|dod_default\|dod_short\|dss_odaa_default\|usgcb_default' build/ssg-rhel7-ds.xml

[redhatrises]

Updated sources again, as \' was reduced to ', and removed whitespace regex right after "bullet points" in DOD banners.

There are still some changes - I have just checked the outputs, and there are following differences against master:

* `dod_short` (in `linux_os/guide/system/accounts/accounts-banners/login_banner_text.var` only!) begins with `I(\\\')*(\')*ve` which definitely seems to be a bug.

* `dss_odaa_default` (in `linux_os/guide/services/http/securing_httpd/httpd_secure_content/var_web_login_banner_text.var` only!) begins with `[\\s\\n]+` which again seems to be a bug.

* rest of the differences is original regexes having `[\s\n]*` pretty randomly in the text. This again looks like a bug (I am pretty sure that banner that contains words like `systemmonitored` is invalid :) ).

For the evaluation, I have used following command to generate stable output:
grep 'dod_banners\|dod_default\|dod_short\|dss_odaa_default\|usgcb_default' build/ssg-rhel7-ds.xml

Just an fyi, we will need to evaluate both space and newline in the macro.

[dahaic]

Just an fyi, we will need to evaluate both space and newline in the macro.

Huh, can you give me an example? Because macro replaces space with [\s\n]+ and newline is replaced with (\\n)*(\n)*. What's missing?

[redhatrises]

Just an fyi, we will need to evaluate both space and newline in the macro.

Huh, can you give me example? Because macro replaces space with [\s\n]+ and newline is replaced with (\\n)*(\n)*. What's missing?

You have removed \n from the variable with the jinja macro. Formatting is everything. In fact, exact spacing and newlines are no longer in the variable because of the macro. So, how do you create a new banner that meets formatting requirements?

[dahaic]

Again, I don't follow. Macro changes \n to (\\n)*(\n)*. It follows original regexes. Differences I described above are between datastream of master and datastream of this PR - no newline processing differences I would be aware of.

Fixed all stuff I found - no artifacts are known to me.

[yuumasato]

@dahaic All banners are rendering correctly, except dod_banners, it is used by the STIG profile and is not remediating correctly, both for el8 and el7.
This command:
oscap xccdf eval --remediate --profile profile_stig --rule xccdf_org.ssgproject.content_rule_banner_etc_issue ./ssg-rhel7-ds.xml
Renders the following banner:

^(You are accessing a U.S. Government (USG) Information System (IS) that is 
provided for USG-authorized use only. By using this IS (which includes any 
device attached to this IS), you consent to the following conditions:
-The USG routinely intercepts and monitors communications on this IS for 
purposes including, but not limited to, penetration testing, COMSEC monitoring, 
network operations and defense, personnel misconduct (PM), law enforcement 
(LE), and counterintelligence (CI) investigations.
-At any time, the USG may inspect and seize data stored on this IS.
-Communications using, or data stored on, this IS are not private, are subject 
to routine monitoring, interception, and search, and may be disclosed or used 
for any USG-authorized purpose.
-This IS includes security measures (e.g., authentication and access controls) 
to protect USG interests--not for your personal benefit or privacy.
-Notwithstanding the above, using this IS does not constitute consent to PM, LE 
or CI investigative searching or monitoring of the content of privileged 
communications, or work product, related to personal representation or services 
by attorneys, psychotherapists, or clergy, and their assistants. Such 
communications and work product are private and confidential. See User 
Agreement for details.|I've read & consent to terms in IS user agreem't.)$

Note at the end the | and the short banner.

Also, the new lines were a bit different before this patch

You are accessing a U.S. Government (USG) Information System (IS) that is 
provided for USG-authorized use only. By using this IS (which includes any 
device attached to this IS), you consent to the following conditions:

-The USG routinely intercepts and monitors communications on this IS for 
purposes including, but not limited to, penetration testing, COMSEC monitoring, 
network operations and defense, personnel misconduct (PM), law enforcement 
(LE), and counterintelligence (CI) investigations.

-At any time, the USG may inspect and seize data stored on this IS.

-Communications using, or data stored on, this IS are not private, are subject 
to routine monitoring, interception, and search, and may be disclosed or used 
for any USG-authorized purpose.

-This IS includes security measures (e.g., authentication and access controls) 
to protect USG interests--not for your personal benefit or privacy.

-Notwithstanding the above, using this IS does not constitute consent to PM, LE 
or CI investigative searching or monitoring of the content of privileged 
communications, or work product, related to personal representation or services 
by attorneys, psychotherapists, or clergy, and their assistants. Such 
communications and work product are private and confidential. See User 
Agreement for details.

[yuumasato]

The same variable login_banner_text.var is reutilized for rule dconf_gnome_login_banners.
But syntax for the dconf gdm banner is different, and remediation for dconf_gnome_login_banners relies on the quirks of the regex.

Before the text rendered as:

You are accessing a U.S. Government (USG) Information System (IS) that is provided for USG-authorized use only. By using this IS (which includes any device attached to this IS), you consent to the following conditions:\n\n- The USG routinely intercepts and monitors communications on this IS for purposes including, but not limited to, penetration testing, COMSEC monitoring, network operations and defense, personnel misconduct (PM), law enforcement (LE), and counterintelligence (CI) investigations.\n\n- At any time, the USG may inspect and seize data stored on this IS.\n\n- Communications using, or data stored on, this IS are not private, are subject to routine monitoring, interception, and search, and may be disclosed or used for any USG-authorized purpose.\n\n- This IS includes security measures (e.g., authentication and access controls) to protect USG interests--not for your personal benefit or privacy.\n\n- Notwithstanding the above, using this IS does not constitute consent to PM, LE or CI investigative searching or monitoring of the content of privileged communications, or work product, related to personal representation or services by attorneys, psychotherapists, or clergy, and their assistants. Such communications and work product are private and confidential. See User Agreement for details.

With the macro it becomes:

You are accessing a U.S. Government (USG) Information System (IS) that is provided for USG-authorized use only. By using this IS (which includes any device attached to this IS), you consent to the following conditions:[n]+-The USG routinely intercepts and monitors communications on this IS for purposes including, but not limited to, penetration testing, COMSEC monitoring, network operations and defense, personnel misconduct (PM), law enforcement (LE), and counterintelligence (CI) investigations.[n]+-At any time, the USG may inspect and seize data stored on this IS.[n]+-Communications using, or data stored on, this IS are not private, are subject to routine monitoring, interception, and search, and may be disclosed or used for any USG-authorized purpose.[n]+-This IS includes security measures (e.g., authentication and access controls) to protect USG interests--not for your personal benefit or privacy.[n]+-Notwithstanding the above, using this IS does not constitute consent to PM, LE or CI investigative searching or monitoring of the content of privileged communications, or work product, related to personal representation or services by attorneys, psychotherapists, or clergy, and their assistants. Such communications and work product are private and confidential. See User Agreement for details.

The difference is the \n\n became [n].
These banners are a nightmare, :(

[yuumasato]

Just an fyi, we will need to evaluate both space and newline in the macro.

Huh, can you give me an example? Because macro replaces space with [\s\n]+ and newline is replaced with (\\n)*(\n)*. What's missing?

What is the point in substituting space for [\s\n]+ if the banners need to be exact?
In other words, why make the regular expression more flexible if in the end one needs exact formatting?

[yuumasato]

Also, the new lines were a bit different before this patch

You are accessing a U.S. Government (USG) Information System (IS) that is 
provided for USG-authorized use only. By using this IS (which includes any 
device attached to this IS), you consent to the following conditions:

-The USG routinely intercepts and monitors communications on this IS for 
purposes including, but not limited to, penetration testing, COMSEC monitoring, 
network operations and defense, personnel misconduct (PM), law enforcement 
(LE), and counterintelligence (CI) investigations.

-At any time, the USG may inspect and seize data stored on this IS.

-Communications using, or data stored on, this IS are not private, are subject 
to routine monitoring, interception, and search, and may be disclosed or used 
for any USG-authorized purpose.

-This IS includes security measures (e.g., authentication and access controls) 
to protect USG interests--not for your personal benefit or privacy.

-Notwithstanding the above, using this IS does not constitute consent to PM, LE 
or CI investigative searching or monitoring of the content of privileged 
communications, or work product, related to personal representation or services 
by attorneys, psychotherapists, or clergy, and their assistants. Such 
communications and work product are private and confidential. See User 
Agreement for details.

Actually, from https://github.com/ComplianceAsCode/content/blob/master/shared/references/disa-stig-rhel7-v2r5-xccdf-manual.xml#L134, it doesn't seem that double newlines are expected.

[yuumasato]

I've made the banner variable usable by both rules banner_etc_issue and dconf_gnome_login_banner_text.

The last commit 2384716 maybe feel out of place for this PR, but it helps with testing.

To reviewer: I suggest you have gdm and dconf installed bedore running test_suite.py.

[yuumasato]

@dahaic can you rebase? You are the owner of the branch.

[pep8speaks]

Hello @dahaic! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file ssg/utils.py:

Line 263:1: E302 expected 2 blank lines, found 1

Comment last updated at 2020-03-12 15:11:45 UTC

[ggbecker]

The tool utils/regexify_banner.py generates a regex without anchoring such as: ^( )$. When I use this output to tailor a profile and change the banner text variable, it will match the pattern even in the middle of a file, which is not the ideal case since we want to match only the expected text. One solution is to change the OVAL of banner_etc_issue to have these anchors already in place so the user doesn't need to care about it.

I suggest creating a new test scenario that cover this case.

Also, please set execution mode for the file: utils/regexify_banner.py and put the shebang similar to other files in utils directory.

[yuumasato]

When I use this output to tailor a profile and change the banner text variable, it will match the pattern even in the middle of a file, which is not the ideal case since we want to match only the expected text.

@ggbecker Isn't this scenario already covered by https://github.com/ComplianceAsCode/content/blob/master/linux_os/guide/system/accounts/accounts-banners/banner_etc_issue/tests/banner_etc_issue_disa_double_banner.fail.sh, which has extra text at the end?

[yuumasato]

Okay, I see the issue is when there is a newline before the extra content.
I will add a test scenario and fix the OVAL.

[ggbecker]

Okay, I see the issue is when there is a newline before the extra content.
I will add a test scenario and fix the OVAL.

I also think that makes sense to test having an extra content in the beginning of the file then a newline and then the content of a valid banner. So we can ensure that the anchoring is always in place.

[yuumasato]

In 5560180 I've changed approach to consider the whole /etc/issue as the object, and the variable as the state to compare against.

I also think that makes sense to test having an extra content in the beginning of the file then a newline and then the content of a valid banner. So we can ensure that the anchoring is always in place.

🤔 Do we really need this test?

[ggbecker]

In 5560180 I've changed approach to consider the whole /etc/issue as the object, and the variable as the state to compare against.

I also think that makes sense to test having an extra content in the beginning of the file then a newline and then the content of a valid banner. So we can ensure that the anchoring is always in place.

thinking Do we really need this test?

I thought it would be good. But I can't think of a strong argument to support the idea now that the problem is fixed.

I'm one of the authors now, ;)

[yuumasato]

@ggbecker I have added the test scenario we discussed offline in 488c525. I added it for dconf_gnome_login_banner_text and RHEL7, as RHEL8 only uses dod_banners,and it is already anchored.

Thank you for the great review.

[ggbecker]

@ggbecker I have added the test scenario we discussed offline in 488c525. I added it for dconf_gnome_login_banner_text and RHEL7, as RHEL8 only uses dod_banners,and it is already anchored.

Thank you for the great review.

The scanning results are accurate now when using customized variable. But still the remediation is failing because of the regex filtering as it is described in this example (excerpt from bash remediation using custom variable generated by utils/regexify_banner.py):

#!/bin/bash
login_banner_text="^aaaaaaaaaaaaa(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)asd(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)as(?:[\n]+|(?:\\n)+)das(?:[\n]+|(?:\\n)+)d(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)dasdasdas(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)asdas(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+asdasdas(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)(?:[\n]+|(?:\\n)+)asdasd[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+[\s\n]+sadasda(?:[\n]+|(?:\\n)+)aa$"

# Multiple regexes transform the banner regex into a usable banner
# 0 - Remove anchors around the banner text
login_banner_text=$(echo "$login_banner_text" | sed 's/^\^\(.*\)\$$/\1/g')
# 1 - Keep only the first banners if there are multiple
#    (dod_banners contains the long and short banner)
login_banner_text=$(echo "$login_banner_text" | sed 's/(\(.*\)|.*$/\1/g')
# 2 - Add spaces ' '. (Transforms regex for "space or newline" into a " ")
login_banner_text=$(echo "$login_banner_text" | sed 's/\[\\s\\n\]+/ /g')
# 3 - Adds newline "tokens". (Transforms "(?:\[\\n\]+|(?:\\n)+)" into "(n)*")
login_banner_text=$(echo "$login_banner_text" | sed 's/(?:\[\\n\]+|(?:\\n)+)/(n)*/g')
# 4 - Remove any leftover backslash. (From any parethesis in the banner, for example).
login_banner_text=$(echo "$login_banner_text" | sed 's/\\//g')
# 5 - Removes the newline "token." (Transforms them into newline escape sequences "\n").
#    ( Needs to be done after 4, otherwise the escapce sequence will become just "n".
login_banner_text=$(echo "$login_banner_text" | sed 's/(n)\*/\\n/g')

echo $login_banner_text

The result of this is script is the following:

aaaaaaaaaaaaa?:[n]+|(?:n)+)\n\nasd\n\nas\ndas\nd\n\n\n\n\ndasdasdas\n\nasdas\n\n asdasdas\n\n\nasdasd sadasda(?:[n]+

[yuumasato]

@ggbecker should be fixed now. The regex to strip multiple banner regex was too loose.

[ggbecker]

@ggbecker should be fixed now. The regex to strip multiple banner regex was too loose.

Trying it again right now

[ggbecker]

The ansible remediation is still failing to remediate for a custom variable. I'm attaching the tailoring file and pasting all the commands I ran to reproduce the failure.

Here is the tailoring profile with the regex from the banner:
ssg-rhel8-ds-tailoring.zip

This is the command I used to generate the ansible remediation:

oscap xccdf generate fix --fix-type ansible --tailoring-file ssg-rhel8-ds-tailoring.xml --profile xccdf_org.ssgproject.content_profile_stig_customized --rule xccdf_org.ssgproject.content_rule_banner_etc_issue ssg-rhel8-ds.xml  > ansible_fix

And then I run with something like:

ansible-playbook --tags banner_etc_issue -i 192.168.122.208, -u root ansible_fix

I'm executing only the ansible tasks that touch the rule banner_etc_issue which modifies the file /etc/issue

Then the final scan in the machine:

oscap xccdf eval --tailoring-file ssg-rhel8-ds-tailoring.xml --profile xccdf_org.ssgproject.content_profile_stig_customized --rule xccdf_org.ssgproject.content_rule_banner_etc_issue ssg-rhel8-ds.xml

If you compare the file /etc/issue with the file issue.zip you will notice that some newlines are missing

The datastream ssg-rhel8-ds.xml is the one generated by running build_product rhel8 with the contents of this PR.

[yuumasato]

@ggbecker I think you hit a dark corner case, :), the culprit is how Ansible wordwrap works.

This is how the banner should be;

aaaaaaaaaaaaa\n\n\nasd\n\nas\ndas\nd\n\n\n\n\ndasdasdas\n\nasdas\n\n            asdasdas\n\n\nasdasd          sadasda\naa

The wordrap filter will wrap the lines to be 79 characters long, and below is ilustrated where the newline would be with a pipe |. Note there are newlines and spaces before and after where wordwrap would put its newline.

aaaaaaaaaaaaa\n\n\nasd\n\nas\ndas\nd\n\n\n\n\ndasdasdas\n\nasdas\n\n           | asdasdas\n\n\nasdasd          sadasda\naa

The wordwrap also clears up any newline and white space around its newline. This is what causes the removal of those two newlines.

I think this happens mostly because your banner is very sparse. I tried some other approaches like breaking it by line and wordrwapping each "line", but that brings other issues.

I suggest not to fix this particular issue, and see if people hit it.

[ggbecker]

@ggbecker I think you hit a dark corner case, :), the culprit is how Ansible wordwrap works.

This is how the banner should be;

aaaaaaaaaaaaa\n\n\nasd\n\nas\ndas\nd\n\n\n\n\ndasdasdas\n\nasdas\n\n            asdasdas\n\n\nasdasd          sadasda\naa

The wordrap filter will wrap the lines to be 79 characters long, and below is ilustrated where the newline would be with a pipe '|. Note there are newlines and spaces before and after where wordwrap` would put its newline.

aaaaaaaaaaaaa\n\n\nasd\n\nas\ndas\nd\n\n\n\n\ndasdasdas\n\nasdas\n\n           | asdasdas\n\n\nasdasd          sadasda\naa

The wordwrap also clears up any newline and white space around its newline. This is what causes the removal of those two newlines.

I think this happens mostly because your banner is very sparse. I tried some other approaches like breaking it by line and wordrwapping each "line", but that brings other issues.

I suggest not to fix this particular issue, and see if people hit it.

Ok, I agree that's a dark corner case and I won't be blocking the PR to be merged because of this. Overall the work done here is great and it should solve the longstanding problem with the banners and its crazy regexes. It also improves a LOT the usability.