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

Minor improvements in configure_opensc_nss_db #11044

Merged

Conversation

marcusburghardt
Copy link
Member

Description:

Minor improvements in remediation.

Rationale:

Review Hints:

Currently this rule is used in very limited cases, but still worth to be fixed.
Automatus tests should be enough.

@marcusburghardt marcusburghardt added bugfix Fixes to reported bugs. Ansible Ansible remediation update. Bash Bash remediation update. labels Sep 4, 2023
@marcusburghardt marcusburghardt added this to the 0.1.70 milestone Sep 4, 2023
@marcusburghardt marcusburghardt changed the title Configure opensc nss db Minor improvements in configure_opensc_nss_db Sep 4, 2023
@github-actions
Copy link

github-actions bot commented Sep 4, 2023

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

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

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@github-actions
Copy link

github-actions bot commented Sep 4, 2023

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

Click here to see the full diff
New content has different text for rule 'xccdf_org.ssgproject.content_rule_configure_opensc_nss_db'.
--- xccdf_org.ssgproject.content_rule_configure_opensc_nss_db
+++ xccdf_org.ssgproject.content_rule_configure_opensc_nss_db
@@ -9,6 +9,11 @@
 command:
 $ sudo pkcs11-switch opensc
 
+[warning]:
+NSS modules information are stored in NSS database which is in binary format. Currently
+it is not possible to check NSS database using OVAL. This is the reason there is no OVAL
+check for this rule.
+
 [reference]:
 1
 

New datastream is missing OVAL for rule 'xccdf_org.ssgproject.content_rule_configure_opensc_nss_db'.
bash remediation for rule 'xccdf_org.ssgproject.content_rule_configure_opensc_nss_db' differs.
--- xccdf_org.ssgproject.content_rule_configure_opensc_nss_db
+++ xccdf_org.ssgproject.content_rule_configure_opensc_nss_db
@@ -4,7 +4,7 @@
 PKCSSW=$(/usr/bin/pkcs11-switch)
 
 if [ ${PKCSSW} != "opensc" ] ; then
-    ${PKCSSW} opensc
+    echo -e "\n" | ${PKCSSW} opensc
 fi
 
 else

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_configure_opensc_nss_db' differs.
--- xccdf_org.ssgproject.content_rule_configure_opensc_nss_db
+++ xccdf_org.ssgproject.content_rule_configure_opensc_nss_db
@@ -1,5 +1,5 @@
-- name: Check existence of pkcs11-switch
-  stat:
+- name: Configure NSS DB To Use opensc - Check Existence of pkcs11-switch
+  ansible.builtin.stat:
     path: /usr/bin/pkcs11-switch
   register: pkcs11switch
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
@@ -21,8 +21,8 @@
   - medium_severity
   - no_reboot_needed
 
-- name: Get NSS database smart card configuration
-  command: /usr/bin/pkcs11-switch
+- name: Configure NSS DB To Use opensc - Get NSS Database Smart Card Configuration
+  ansible.builtin.command: /usr/bin/pkcs11-switch
   changed_when: true
   register: pkcsw_output
   when:
@@ -46,8 +46,8 @@
   - medium_severity
   - no_reboot_needed
 
-- name: Configure NSS DB To Use opensc
-  command: /usr/bin/pkcs11-switch opensc
+- name: Configure NSS DB To Use opensc - Select opensc Module
+  ansible.builtin.shell: echo -e "\n" | /usr/bin/pkcs11-switch opensc
   when:
   - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   - pkcs11switch.stat.exists and pkcsw_output.stdout != "opensc"

@vojtapolasek
Copy link
Collaborator

Hello @marcusburghardt and thank you for the improvement. Do you plan to add some simple automatus test scenarios as well?

@marcusburghardt
Copy link
Member Author

Hello @marcusburghardt and thank you for the improvement. Do you plan to add some simple automatus test scenarios as well?

I was not in my plans @vojtapolasek . I think the existing tests are enough considering this rule is not so used and I believe will be obsolete soon.

@marcusburghardt
Copy link
Member Author

Hello @marcusburghardt and thank you for the improvement. Do you plan to add some simple automatus test scenarios as well?

I was not in my plans @vojtapolasek . I think the existing tests are enough considering this rule is not so used and I believe will be obsolete soon.

Sorry. I confused with another rule. I will quickly include some very simple test scenarios for this rule.

@marcusburghardt
Copy link
Member Author

Actually, the OVAL in this rule is wrong. : (
I don't know why the check was relying on /etc/pki/nssdb/pkcs11.txt but this is not correct. The modules information are in the NSS database which is in binary format.

The OVAL check in configure_opensc_nss_db was wrong and unreliable. It
was expecting to find the opensc module information in pkcs11.txt file
but it is not expected to find this information there. This information
should be properly checked by consulting the NSS database through proper
commands. Currently, there is no file to be consulted and no OVAL check
for NSS database. Removing this OVAL is the correct approach.
@ggbecker
Copy link
Member

ggbecker commented Sep 5, 2023

@marcusburghardt Please check the following (not merged) closed PRs for more context:

#4776
#7017

There were a few attempts in the past to make this rule better, but due to the nature of how the configuration is stored (binary), it was not possible to create reliable automated content.

- general: |-
NSS modules information are stored in NSS database which is in binary format. Currently
it is not possible to check NSS database using OVAL. This is the reason there is no OVAL
check for this rule.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello @marcusburghardt and thank you for the update. Please just add a new line here and the PR is good to be merged. Thank you.

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. Thanks

Make it clear why there is no OVAL check for this rule.
@marcusburghardt
Copy link
Member Author

@marcusburghardt Please check the following (not merged) closed PRs for more context:

#4776 #7017

There were a few attempts in the past to make this rule better, but due to the nature of how the configuration is stored (binary), it was not possible to create reliable automated content.

Thanks @ggbecker . I just removed the OVAL. It is wrong and unreliable.

@codeclimate
Copy link

codeclimate bot commented Sep 5, 2023

Code Climate has analyzed commit 2359eec 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 53.8% (0.0% change).

View more on Code Climate.

Copy link
Collaborator

@vojtapolasek vojtapolasek 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. I tested the Ansible playbook and the rule no longer causes the playbook to hang.

@vojtapolasek vojtapolasek self-assigned this Sep 5, 2023
@vojtapolasek vojtapolasek added the OVAL OVAL update. Related to the systems assessments. label Sep 5, 2023
@vojtapolasek vojtapolasek merged commit f0a29dd into ComplianceAsCode:master Sep 5, 2023
34 of 38 checks passed
@marcusburghardt marcusburghardt deleted the configure_opensc_nss_db branch September 5, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ansible Ansible remediation update. Bash Bash remediation update. 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.

Ansible remediation of configure_opensc_nss_db in rhel7 never finishes
3 participants