Skip to content

Fix configure_opensc_nss_db bash remediation#7017

Closed
freddieRv wants to merge 1 commit intoComplianceAsCode:masterfrom
freddieRv:fix_opensc_nss_db_bash_remediation
Closed

Fix configure_opensc_nss_db bash remediation#7017
freddieRv wants to merge 1 commit intoComplianceAsCode:masterfrom
freddieRv:fix_opensc_nss_db_bash_remediation

Conversation

@freddieRv
Copy link
Copy Markdown
Contributor

Description:

  • Fix remediation script to compare the output value of pkcs11-switch and to run pkcs11-switch opensc inside of if statement
  • Added call to modutil since pkcs11-switch opensc does not update /etc/pki/nssdb/pkcs11.txt. Without this update rule results in fail even after successfully running the remediation

Rationale:

  • This is a bug fix

Signed-off-by: Federico Ramirez <federico.r.ramirez@oracle.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 18, 2021

Hi @freddieRv. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@openshift-ci openshift-ci Bot added the needs-ok-to-test Used by openshift-ci bot. label May 18, 2021
@openscap-ci
Copy link
Copy Markdown
Collaborator

Changes identified:
Rules:
 configure_opensc_nss_db

Show details

Rule configure_opensc_nss_db:
 Found change in bash remediation.

Recommended tests to execute:
 build_product rhel7
 tests/test_suite.py rule --libvirt qemu:///system test-suite-vm --remediate-using bash --datastream build/ssg-rhel7-ds.xml configure_opensc_nss_db

@ggbecker
Copy link
Copy Markdown
Member

I've tried to do something similar in the past (#4776) regarding this rule, but faced some technical aspects of this module that the way of configuring it was not really something that worked. I don't remember exactly the details but I can try to find conversations with maintainer to illustrate better.

Have you tried to do a functional testing with this using smart cards?

@ggbecker
Copy link
Copy Markdown
Member

I've tried to do something similar in the past (#4776) regarding this rule, but faced some technical aspects of this module that the way of configuring it was not really something that worked. I don't remember exactly the details but I can try to find conversations with maintainer to illustrate better.

Have you tried to do a functional testing with this using smart cards?

This is an excerpt from what I got from one of the maintainers:
On rhel7, if you can't run any commands, and you can't parse binary files, you are really stuck. You can examine pkcs11.txt, but there's a pretty high chance that it will not be what the application is using to load the module.

@freddieRv
Copy link
Copy Markdown
Contributor Author

@ggbecker Thanks for the feedback and the extra information. I was not aware of #4776.

Have you tried to do a functional testing with this using smart cards?

I have not. My goal with this PR was to bring the bash remediation to what it intended to do. I am also not quite familiar with this module. Only after some digging I found the modutil tool and added the statement using it in the hopes of passing the oval check.

However after reading the info you provided:

... You can examine pkcs11.txt, but there's a pretty high chance that it will not be what the application is using to load the module.

I believe the oval check could use some updating too.

# disruption = low

PKCSSW=$(/usr/bin/pkcs11-switch)
PKCSSW="/usr/bin/pkcs11-switch"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This script is in the opensc package so if the package is not installed the remediation will do result in error.

@ggbecker do you think that installing the opensc package in this remediation is a good idea? Installation of opensc feels like a different rule, however without the package we can't perform remediation of this one.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@mildas Probably yes, Imagine that somebody will update the rule in the future and this test will be executed by the CTF.

@jan-cerny
Copy link
Copy Markdown
Collaborator

@mildas @ggbecker Can you summarize what we need to do to unblock this?

@ggbecker
Copy link
Copy Markdown
Member

@mildas @ggbecker Can you summarize what we need to do to unblock this?

In theory, we have no confirmation that this rule is useful. Because the OVAL check looks for the pkcs11.txt file but that doesn't mean that the smartcard's module is actually considering contents of that file. This change is probably good because it updates the pkcs11.txt accordingly though, so from the OVAL/remediation alignment perspective it should be correct.

@jan-cerny
Copy link
Copy Markdown
Collaborator

@mildas @ggbecker So would you be in favor of closing this PR? It's a stalled PR.

@ggbecker
Copy link
Copy Markdown
Member

@mildas @ggbecker So would you be in favor of closing this PR? It's a stalled PR.

I would say so. The rule itself is very controversial and I couldn't find the proper solution back in the days. Although the check/remediation could be properly aligned, there is no evidence that the configuration actually works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Used by openshift-ci bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants