Fix macro for extracting local interactive users

[mpurg]

Description:

Fix regex used in OVAL macro create_local_interactive_users_object to include other shell paths.

Rationale:

This macro is used to extract specific fields from /etc/passwd. Only local interactive users are considered, excluding those with shell /sbin/nologin.

This fix excludes also users with following shells:

• /bin/false
• /usr/bin/false
• /usr/sbin/nologin

Additional information:

This macro is used in the following rules:

• accounts_user_dot_group_ownership
• accounts_user_dot_user_ownership
• accounts_user_interactive_home_directory_exists
• accounts_users_home_files_groupownership
• accounts_users_home_files_ownership
• accounts_users_home_files_permissions
• file_groupownership_home_directories
• file_ownership_home_directories
• file_permissions_home_directories
• accounts_umask_interactive_users

I adapted the test for checking ignored shells accordingly.

[openshift-ci[bot]]

Hi @mpurg. 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.

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.

[github-actions[bot]]

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

rhel8 (from CTF) Environment (using Fedora as testing environment)

Fedora Testing Environment

Oracle Linux 8 Environment

[dodys]

@jan-cerny since you wrote most of this code, would take a look at it?

[jan-cerny]

/packit retest-failed

[jan-cerny]

/packit retest-failed

[dodys]

@teacup-on-rockingchair could you check the failing test results?

[teacup-on-rockingchair]

@teacup-on-rockingchair could you check the failing test results?

The problem with the failing SLE tests for 'file_groupownership_home_directories', 'accounts_users_home_files_groupownership' is the fact that chgrp there is called to change the group ownership of the home directory to cac_user group which is not created anywhere. We need groupadd in the setup of those.

The problem with home_dirs_one_absent.pass.sh for tests 'accounts_umask_interactive_users', 'accounts_umask_interactive_users' etc, is that the sed command in the test attempts text operation on all .XXX files in a home directory, which returns error in case .XXX file is directory.

[mpurg]

LGTM, maybe you can use the occasion to minimize the duplication as much as possible, say doing something like :

#!/bin/sh

per_user_shell_procedure() {
    procedure=$*
    for shell in "/sbin/nologin" \
                     "/usr/sbin/nologin" \
                     "/bin/false" \
                     "/usr/bin/false"; do

        user=cac_user${shell//\//_}
        useradd -m -s $shell $user
        
        eval "${procedure}"
    done
}

in a shared file and use: per_user_shell_procedure "do_stuff \$user"

Hi @teacup-on-rockingchair , thanks for the suggestion, I agree that there's too much duplicated code here. Where do you think would be the best place to store this code so that it could be reused by tests in different rules?

One thing I should also point out is that not all the tests use useradd -m, so that should be included in the do_stuff ...

[teacup-on-rockingchair]

LGTM, maybe you can use the occasion to minimize the duplication as much as possible, say doing something like :

#!/bin/sh

per_user_shell_procedure() {
    procedure=$*
    for shell in "/sbin/nologin" \
                     "/usr/sbin/nologin" \
                     "/bin/false" \
                     "/usr/bin/false"; do

        user=cac_user${shell//\//_}
        useradd -m -s $shell $user
        
        eval "${procedure}"
    done
}

in a shared file and use: per_user_shell_procedure "do_stuff \$user"

Hi @teacup-on-rockingchair , thanks for the suggestion, I agree that there's too much duplicated code here. Where do you think would be the best place to store this code so that it could be reused by tests in different rules?

One thing I should also point out is that not all the tests use useradd -m, so that should be included in the do_stuff ...

Best practice as far as I saw in the project is to have those as sahred file in the common tests directory and source that. I am not sure, but I guess you can test that also to make it a jinja macro, the ones we use for bash remediations, but I am not sure if it will work.

I also notice the useradd -m vs useradd -M, but I guess for the sake of using common code you can have a rm -fr /home/$user as first procedure where you need -M

[github-actions[bot]]

:robot: A k8s content image for this PR is available at: ghcr.io/complianceascode/k8scontent:11589

Click here to see how to deploy it

If you alread have Compliance Operator deployed: utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11589

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11589 make deploy-local

[mpurg]

@teacup-on-rockingchair please check out the proposed solution

[dodys]

/packit retest-failed

[teacup-on-rockingchair]

/packit retest-failed

LGTM 🙇

[codeclimate[bot]]

Code Climate has analyzed commit c2d3784a 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 58.1% (0.0% change).

View more on Code Climate.