Skip to content

Do not add dconf value type for login banner text#3679

Merged
ggbecker merged 7 commits intoComplianceAsCode:masterfrom
yuumasato:dconf_gnome_banner_text_regex
Jun 28, 2019
Merged

Do not add dconf value type for login banner text#3679
ggbecker merged 7 commits intoComplianceAsCode:masterfrom
yuumasato:dconf_gnome_banner_text_regex

Conversation

@yuumasato
Copy link
Copy Markdown
Member

@yuumasato yuumasato commented Jan 9, 2019

Description:

  • Make regular expression for dconf_gnome_login_banner_text stricter
  • Do not add string during login banner remediations

Rationale:

@yuumasato yuumasato added bugfix Fixes to reported bugs. OVAL OVAL update. Related to the systems assessments. labels Jan 9, 2019
@yuumasato yuumasato added this to the 0.1.43 milestone Jan 9, 2019
@redhatrises
Copy link
Copy Markdown
Contributor

@yuumasato does this change also need to happen in remediation scripts?

@yuumasato
Copy link
Copy Markdown
Member Author

@redhatrises I had checked the bash remediation, and it looks good.
But the Ansible remediation needs an update. It needs to filter out regular expression elements, just like it is done in Bash.
Will look into it soon.

@yuumasato yuumasato changed the title Make regular expression for dconf_gnome_login_banner_text stricter [WIP] Make regular expression for dconf_gnome_login_banner_text stricter Jan 21, 2019
@yuumasato yuumasato force-pushed the dconf_gnome_banner_text_regex branch from 411ad17 to 35a0b13 Compare January 31, 2019 14:07
@scrutinizer-notifier
Copy link
Copy Markdown

The inspection completed: 2 new issues

@yuumasato yuumasato changed the title [WIP] Make regular expression for dconf_gnome_login_banner_text stricter [WIP] Do not add dconf value type for login banner text Jan 31, 2019
@yuumasato
Copy link
Copy Markdown
Member Author

The remediations wont add string anymore.
The Ansible remediation still needs to be addressed.

@yuumasato yuumasato modified the milestones: 0.1.43, 0.1.44 Feb 21, 2019
@yuumasato yuumasato modified the milestones: 0.1.44, 0.1.45 May 2, 2019
@dahaic
Copy link
Copy Markdown
Contributor

dahaic commented Jun 20, 2019

@yuumasato do you plan to continue work on this one? Is it possible to merge just part that is ready?

@yuumasato yuumasato changed the title [WIP] Do not add dconf value type for login banner text Do not add dconf value type for login banner text Jun 21, 2019
@yuumasato
Copy link
Copy Markdown
Member Author

@dahaic Sure, it has been lingering for long.

@yuumasato yuumasato added Bash Bash remediation update. Text and removed OVAL OVAL update. Related to the systems assessments. labels Jun 24, 2019
@matejak
Copy link
Copy Markdown
Member

matejak commented Jun 27, 2019

@yuumasato Could you please rebase? It looks like I don't have rights. The rebase should be easy - only one file has conflicts and they all follow the same pattern - the master branch is wrong, whereas the commit is right.

The regex was matching the first banner with the second appended.
Remediation would also append the second banner along with the first.

Note: dod_banners is about a regex to match login banner and short login
banner.
@ggbecker
Copy link
Copy Markdown
Member

305a2b9 Resolves #4546

- Escaped backlashes in the banner itself, so sed doesn't interpret them.
- Introduced readarray to create arrays.
- Changed matching of existing keys in keyfiles.
- Added double backslash escaping inside double quotes.
@yuumasato yuumasato force-pushed the dconf_gnome_banner_text_regex branch from 305a2b9 to 759cc97 Compare June 27, 2019 13:18
@yuumasato
Copy link
Copy Markdown
Member Author

I have rebased and solved some conflicts.
@matejak please, check if I didn't break anything.

@ggbecker ggbecker self-assigned this Jun 28, 2019
@matejak
Copy link
Copy Markdown
Member

matejak commented Jun 28, 2019

I find it OK, moreover the test suite tests pass as well.

@ggbecker
Copy link
Copy Markdown
Member

PR seems good. I'll complement with a couple of test scenarios which covers RHEL7 stig profile (dod_banner).

Found issues:
The banner:

dod_short: I(\\\')*(\')*ve[\s\n]+read[\s\n]+\&[\s\n]+consent[\s\n]+to[\s\n]+terms[\s\n]+in[\s\n]+IS[\s\n]+user[\s\n]+agreement.

Still fails to check after remediating. This banner is not used by any profile so it is not a blocker for this PR but test coverage should be increased to include all possible values for this variable.

@ggbecker
Copy link
Copy Markdown
Member

I've created a new issue to handle the ansible playbook issue (filter out regular expression elements) so the PR doesn't get blocked. #4574

Otherwise LGTM. Merging it.

@ggbecker ggbecker merged commit 3805f16 into ComplianceAsCode:master Jun 28, 2019
@yuumasato yuumasato deleted the dconf_gnome_banner_text_regex branch June 28, 2019 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bash Bash remediation update. bugfix Fixes to reported bugs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants