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

Use specific name in private key groups instead of gid #10622

Merged
merged 1 commit into from
May 23, 2023

Conversation

ggbecker
Copy link
Member

Description:

  • Use specific name in private key groups instead of gid.

Rationale:

  • This is an attempt to fix the following three issues:

#10594
#10593
#10592

There is a upstream test ongoing that will help us to identify if the problem was really solved since this looks like some rules are affecting others and unit tests won't help in this case.

@ggbecker ggbecker added the bugfix Fixes to reported bugs. label May 23, 2023
@ggbecker ggbecker requested a review from a team as a code owner May 23, 2023 13:58
@github-actions
Copy link

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_file_groupownership_sshd_private_key' differs.%0A--- xccdf_org.ssgproject.content_rule_file_groupownership_sshd_private_key%0A+++ xccdf_org.ssgproject.content_rule_file_groupownership_sshd_private_key%0A@@ -1,7 +1,7 @@%0A # Remediation is applicable only in certain platforms%0A if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ]; then%0A %0A-find /etc/ssh/ -maxdepth 1 -type f ! -gid 995 -regex '^.*_key$' -exec chgrp 995 {} \;%0A+find /etc/ssh/ -maxdepth 1 -type f ! -gid ssh_keys -regex '^.*_key$' -exec chgrp ssh_keys {} \;%0A %0A else%0A     >&2 echo 'Remediation is not applicable, nothing was done'%0A%0Aansible remediation for rule 'xccdf_org.ssgproject.content_rule_file_groupownership_sshd_private_key' differs.%0A--- xccdf_org.ssgproject.content_rule_file_groupownership_sshd_private_key%0A+++ xccdf_org.ssgproject.content_rule_file_groupownership_sshd_private_key%0A@@ -1,5 +1,5 @@%0A - name: Find /etc/ssh/ file(s) matching ^.*_key$%0A-  command: find -H /etc/ssh/ -maxdepth 1 -type f ! -gid 995 -regex "^.*_key$"%0A+  command: find -H /etc/ssh/ -maxdepth 1 -type f ! -gid ssh_keys -regex "^.*_key$"%0A   register: files_found%0A   changed_when: false%0A   failed_when: false%0A@@ -17,7 +17,7 @@%0A - name: Ensure group owner on /etc/ssh/ file(s) matching ^.*_key$%0A   file:%0A     path: '{{ item }}'%0A-    group: '995'%0A+    group: ssh_keys%0A     state: file%0A   with_items:%0A   - '{{ files_found.stdout_lines }}'

@Mab879 Mab879 self-assigned this May 23, 2023
@github-actions
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@codeclimate
Copy link

codeclimate bot commented May 23, 2023

Code Climate has analyzed commit 44cba45 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 52.5% (0.0% change).

View more on Code Climate.

@Mab879 Mab879 added this to the 0.1.68 milestone May 23, 2023
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Waving the automatus issues as they pass locally.

@Mab879 Mab879 merged commit abdb7e8 into ComplianceAsCode:master May 23, 2023
31 of 32 checks passed
</unix:file_state>
{{% endfor %}}

<unix:file_state id="symlink_file_groupowner{{{ FILEID }}}_uid_{{{ FILEGID }}}" version="1">
<unix:type operation="equals">symbolic link</unix:type>
</unix:file_state>

{{%- if FILEGID != '0' %}}
Copy link
Member

Choose a reason for hiding this comment

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

A explanatory variable gid_is_name would fit perfectly here, and probably should be more robust - numbers are probably IDs, not numbers are definitely names. Moreover, I believe that the template code could propagate a boolean with the template.

@matejak
Copy link
Member

matejak commented Jun 2, 2023

This is a great PR, but it has changed the API of the template, I would rename the filegid key to gid_or_name, and enhance the Python template part to try to convert the gid_or_name to int, and if it succeeds, indicate that we have gid.
Anyway, this is handled in #10666

@jan-cerny jan-cerny changed the title Use specific name in private key groups instead of gid. Use specific name in private key groups instead of gid Jun 14, 2023
@jan-cerny jan-cerny added the OVAL OVAL update. Related to the systems assessments. label Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes to reported bugs. OVAL OVAL update. Related to the systems assessments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants