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

Introduced rule to disable XDMCP in gdm #5997

Merged
merged 5 commits into from Sep 8, 2020

Conversation

matejak
Copy link
Member

@matejak matejak commented Aug 11, 2020

Introduce a new rule to disable the insecure xdmcp.

TBD:

  • Ansible remediation
  • Make the Bash ini snippet a jinja macro.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1807173

@matejak matejak added this to the 0.1.52 milestone Aug 11, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 11, 2020
@matejak
Copy link
Member Author

matejak commented Aug 11, 2020

 The rule doesn't occur in any profile nor product.

@mildas That's wrong, it applies to all products. You are right that it doesn't belong to any profile.

@matejak matejak changed the title [WIP] Introduced rule to disable XDMCP in gdm. Introduced rule to disable XDMCP in gdm. Aug 12, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 12, 2020
to have those thin clients on a separate network that cannot be accessed by the outside world, and can only connect to the server.
The only point from which you need to access outside is the server. This type of set up should never use an unmanaged hub or other sniffable network.

severity: medium
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it is unencrypted, I believe that the severity is a high.

Comment on lines 26 to 33
Check whether there is the xdmcp section in the gdm configuration - run <code>grep '^\s*\[xdmcp\]' /etc/gdm/custom.conf</code>
If no results are returned, the whole section is missing, in which case you can add it to the file
using printf - <code>printf '%\n' "" "[xdmcp]" "Enable=false" >> /etc/gdm/custom.conf</code>

If grep returned results, you have to open the custom.conf file in a text editor,
and make sure that there is only one assignment to Enable,
namely Enable=false between the beginning of the [xdmcp] section
and another section or the end of the file.
Copy link
Contributor

Choose a reason for hiding this comment

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

To ensure that XDMCP is disabled in /etc/gdm/custom.conf, run the following command:

grep -Pzo "\[xdmcp\]\nEnable=false" /etc/gdm/custom.conf

The output should return the following:

[xdmcp]
Enable=false

elif grep -qs '[[:space:]]*\[{{{ section }}}]' '{{{ filename }}}'; then
sed -i '/[[:space:]]*\[{{{ section }}}]/a {{{ key }}}={{{ value }}}' '{{{ filename }}}'
else
mkdir -p {{{ "/".join(filename.split("/")[:-1]) }}}
Copy link
Contributor

Choose a reason for hiding this comment

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

/etc/gdm and specifically /etc/gdm/custom.conf is owned by the gdm package. It would be better to check if gdm package exists or /etc/gdm/custom.conf exists rather than mkdir the directory and adding the file in the event that /etc/gdm/ doesn't exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

The OVAL evaluates as not applicable if gdm is not installed, so this case is covered in case of oscap ... --remediate. Generally, that can be said about almost every config file that we remediate. Your point is legitimate, but if we decide to enforce this behavior, let it be a project-wide coordinated effort. The worst thing is a mix of approaches in similar remediations, which is inconsistent, and provides poor guidance to inexperienced content authors that learn by example.

Copy link
Contributor

Choose a reason for hiding this comment

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

oscap shouldn't remediate if not applicable and remediating something that isn't installed is pointless.

Copy link
Member Author

Choose a reason for hiding this comment

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

oscap won't execute this remediation if gdm is not installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Why are we making directories? Seems like we shouldn't be doing this a part of remediations. Are there remediations that need to have a directory created that isn't provided by a rpm?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point, we can probably abort the remediation if the directory doesn't exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, non-applicability is assumed if the parent folder of the config file doesn't exist.

@matejak
Copy link
Member Author

matejak commented Aug 17, 2020

@redhatrises I am happy to incorporate your suggestions, thanks for those!
The question of OCIL is not 100% clear to me, and neither is the way how we do remediations of config files. Could you please check those out?

@jan-cerny jan-cerny changed the title Introduced rule to disable XDMCP in gdm. Introduced rule to disable XDMCP in gdm Aug 19, 2020
@JAORMX
Copy link
Contributor

JAORMX commented Aug 31, 2020

/retest

1 similar comment
@jan-cerny
Copy link
Collaborator

/retest

@openshift-ci-robot openshift-ci-robot added the needs-rebase Used by openshift-ci bot. label Sep 8, 2020
@matejak matejak force-pushed the xdmcp_disabled branch 2 times, most recently from c867e1f to 9af0340 Compare September 8, 2020 11:58
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Used by openshift-ci bot. label Sep 8, 2020
matejak and others added 5 commits September 8, 2020 16:00
Co-authored-by: Gabe Alford <redhatrises@gmail.com>
If the parent folder of the config file to be remediated doesn't exist,
don't do anything and just say that we think that remediation is not applicable.
Co-authored-by: Gabe Alford <redhatrises@gmail.com>
@mildas
Copy link
Contributor

mildas commented Sep 8, 2020

Changes identified:
Rule gnome_gdm_disable_xdmcp:
 The rule doesn't occur in any profile nor product.
 OVAL check is newly added.
 Ansible remediation newly added.
 Bash remediation is newly added.
 Templatization usage changed.
Macro ansible_ini_file_set:
 In Ansible remediation for gnome_gdm_disable_xdmcp.
Macro bash_ini_file_set:
 In Bash remediation for gnome_gdm_disable_xdmcp.

@openshift-ci-robot
Copy link
Collaborator

@matejak: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-rhcos4-moderate 0fd3aed link /test e2e-aws-rhcos4-moderate
ci/prow/e2e-aws-ocp4-moderate 0fd3aed link /test e2e-aws-ocp4-moderate
ci/prow/e2e-aws-rhcos4-e8 0fd3aed link /test e2e-aws-rhcos4-e8

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@redhatrises
Copy link
Contributor

I believe that everything has been addressed here. Merging. Any new issues can be fixed in follow up PRs.

@redhatrises redhatrises merged commit fbab84b into ComplianceAsCode:master Sep 8, 2020
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

7 participants