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

Add support for PCI DSS v3.2.1 for SLE12 #9613

Merged

Conversation

teacup-on-rockingchair
Copy link
Contributor

Description:

  • Add PCI-DSS control for v.3.2.1 and SLE profiles to include it

Rationale:

  • Add sle12 platform and missing CCE and PCIDSS references to rules related to PCIDSS effort for SLE platforms

@openshift-ci
Copy link

openshift-ci bot commented Oct 4, 2022

Hi @teacup-on-rockingchair. 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.

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Oct 4, 2022
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

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

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@ggbecker
Copy link
Member

ggbecker commented Oct 5, 2022

I have realized that maybe we could have a single pcidss_v3 that would contain both content for OCP4 and other products as well, since the controls file will include rules that are applicable to a given built product, so let's say you build a profile using this controls file, it will include only the rules that have prodtype set to that given product.

@teacup-on-rockingchair
Copy link
Contributor Author

I have realized that maybe we could have a single pcidss_v3 that would contain both content for OCP4 and other products as well, since the controls file will include rules that are applicable to a given built product, so let's say you build a profile using this controls file, it will include only the rules that have prodtype set to that given product.

I thought about it also but what stopped me of reusing the existing control was not rules that were mostly OCP4 specific, but the notes in the pcidss_ocp4 control file which had a lot of references to Open Shift specific behaviour and OCP environment. If you think that it would be ok to put prodtype specific jinja clauses around those notes, then I guess we can easily aggregate both controls.

@ggbecker
Copy link
Member

ggbecker commented Oct 7, 2022

I have realized that maybe we could have a single pcidss_v3 that would contain both content for OCP4 and other products as well, since the controls file will include rules that are applicable to a given built product, so let's say you build a profile using this controls file, it will include only the rules that have prodtype set to that given product.

I thought about it also but what stopped me of reusing the existing control was not rules that were mostly OCP4 specific, but the notes in the pcidss_ocp4 control file which had a lot of references to Open Shift specific behaviour and OCP environment. If you think that it would be ok to put prodtype specific jinja clauses around those notes, then I guess we can easily aggregate both controls.

It's probably ok to have a separate file then. There is also the fact that it can create a misinterpretation of the status field depending on which rules are applicable to which product. So let's keep them separate, at least for now. Then this pcidss_3 can be used for the general linux distros.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Used by openshift-ci bot. label Oct 8, 2022
@openshift-merge-robot openshift-merge-robot added needs-rebase Used by openshift-ci bot. and removed needs-rebase Used by openshift-ci bot. labels Oct 9, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Used by openshift-ci bot. label Oct 13, 2022
@teacup-on-rockingchair teacup-on-rockingchair marked this pull request as draft October 13, 2022 19:27
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 13, 2022
@teacup-on-rockingchair teacup-on-rockingchair marked this pull request as ready for review October 13, 2022 19:28
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 13, 2022
SMEs:
- abergmann

reference: https://docs-prv.pcisecuritystandards.org/PCI%20DSS/Standard/PCI-DSS-v4_0.pdf
Copy link
Member

Choose a reason for hiding this comment

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

3.2.1 instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be ok in 0e0c5db

Copy link
Member

@ggbecker ggbecker left a comment

Choose a reason for hiding this comment

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

Hi, I've run the profile diff tool and notice that the following rules that were previously in the pci-dss.profile are not present in the pci-dss controls file:

$ ./build-scripts/profile_tool.py sub --profile1 products/sle15/profiles/pci-dss-from-main-branch.profile --profile2 build/sle15/profiles/pci-dss.profile --product sle15 --build-config-yaml build/build_config.yml --ssg-root .

selections:
- accounts_minimum_age_login_defs
- accounts_no_uid_except_zero
- accounts_password_warn_age_login_defs
- accounts_tmout
- accounts_umask_etc_bashrc
- accounts_umask_etc_login_defs
- accounts_umask_etc_profile
- cracklib_accounts_password_pam_dcredit
- cracklib_accounts_password_pam_lcredit
- cracklib_accounts_password_pam_minlen
- cracklib_accounts_password_pam_ocredit
- cracklib_accounts_password_pam_retry
- cracklib_accounts_password_pam_ucredit
- file_permissions_sshd_private_key
- file_permissions_sshd_pub_key
- no_direct_root_logins
- package_audit_installed
- package_chrony_installed
- package_openldap-clients_removed
- package_sudo_installed
- package_telnet-server_removed
- package_vsftpd_removed
- package_ypserv_removed
- postfix_network_listening_disabled
- securetty_root_login_console_only
- sshd_disable_empty_passwords
- sshd_disable_root_login
- sshd_do_not_permit_user_env
- sshd_enable_warning_banner
- sshd_set_loglevel_verbose
- sudo_add_use_pty
- sudo_custom_logfile
- var_smartcard_drivers=cac

Maybe you would like to include these rule as well so you don't end up with a different profile, at least for the transition part.

@teacup-on-rockingchair
Copy link
Contributor Author

Hi, I've run the profile diff tool and notice that the following rules that were previously in the pci-dss.profile are not present in the pci-dss controls file:

$ ./build-scripts/profile_tool.py sub --profile1 products/sle15/profiles/pci-dss-from-main-branch.profile --profile2 build/sle15/profiles/pci-dss.profile --product sle15 --build-config-yaml build/build_config.yml --ssg-root .

selections:
- accounts_minimum_age_login_defs
- accounts_no_uid_except_zero
- accounts_password_warn_age_login_defs
- accounts_tmout
- accounts_umask_etc_bashrc
- accounts_umask_etc_login_defs
- accounts_umask_etc_profile
- cracklib_accounts_password_pam_dcredit
- cracklib_accounts_password_pam_lcredit
- cracklib_accounts_password_pam_minlen
- cracklib_accounts_password_pam_ocredit
- cracklib_accounts_password_pam_retry
- cracklib_accounts_password_pam_ucredit
- file_permissions_sshd_private_key
- file_permissions_sshd_pub_key
- no_direct_root_logins
- package_audit_installed
- package_chrony_installed
- package_openldap-clients_removed
- package_sudo_installed
- package_telnet-server_removed
- package_vsftpd_removed
- package_ypserv_removed
- postfix_network_listening_disabled
- securetty_root_login_console_only
- sshd_disable_empty_passwords
- sshd_disable_root_login
- sshd_do_not_permit_user_env
- sshd_enable_warning_banner
- sshd_set_loglevel_verbose
- sudo_add_use_pty
- sudo_custom_logfile
- var_smartcard_drivers=cac

Maybe you would like to include these rule as well so you don't end up with a different profile, at least for the transition part.

Those rules were added in the context of PCIDSS v4 effort, thanks for the feedback. For now I think is best if I include them in the context of different profile file. Once I have built a control file for that version of the standard will include them and rework the profile file.

@ggbecker
Copy link
Member

We should probably shift towards having version 4 as the default one. This basically means that the pci-dss.profile should point to the latest 4 version. Version 3 should eventually go away when there is the sunset and then we are left only with the pci-dss.profile identifier for example. The same for the pcidss reference identifier. We should update the URL links to the latest version and create a pcidss_3 reference URI to point to the older policy. But one problem might be that the already assigned references will be inconsistent with the v4 for example. What do you think?

@teacup-on-rockingchair
Copy link
Contributor Author

We should probably shift towards having version 4 as the default one. This basically means that the pci-dss.profile should point to the latest 4 version. Version 3 should eventually go away when there is the sunset and then we are left only with the pci-dss.profile identifier for example. The same for the pcidss reference identifier. We should update the URL links to the latest version and create a pcidss_3 reference URI to point to the older policy. But one problem might be that the already assigned references will be inconsistent with the v4 for example. What do you think?

In general I agree, but would prefer to do that, in the context of another PR, once I generate the control file for v4 and then will be able to rename the profiles per product. Because the v4 for SLE platforms is WIP, and I am not really aware the state of the art for the other platforms.

@ggbecker
Copy link
Member

We should probably shift towards having version 4 as the default one. This basically means that the pci-dss.profile should point to the latest 4 version. Version 3 should eventually go away when there is the sunset and then we are left only with the pci-dss.profile identifier for example. The same for the pcidss reference identifier. We should update the URL links to the latest version and create a pcidss_3 reference URI to point to the older policy. But one problem might be that the already assigned references will be inconsistent with the v4 for example. What do you think?

In general I agree, but would prefer to do that, in the context of another PR, once I generate the control file for v4 and then will be able to rename the profiles per product. Because the v4 for SLE platforms is WIP, and I am not really aware the state of the art for the other platforms.

That's true, ideally it would be good if we do this transition before the next release, otherwise shuffling profile IDs can cause some implications for people that already started using that specific profile ID for example. But we should be able to manage that.

@ggbecker
Copy link
Member

Also, if you could please rebase on top of latest main branch to check if there is no conflict with the CCE assignment since rumch-se has been including a bunch of them, just to be sure that the test will be able to catch any duplicate. Thanks. Otherwise I think we can probably merge this one.

@teacup-on-rockingchair
Copy link
Contributor Author

Also, if you could please rebase on top of latest main branch to check if there is no conflict with the CCE assignment since rumch-se has been including a bunch of them, just to be sure that the test will be able to catch any duplicate. Thanks. Otherwise I think we can probably merge this one.

Thanks for the reminder, I just did the rebase and run the uniqe and missing cce tests for sle platforms.

@codeclimate
Copy link

codeclimate bot commented Oct 18, 2022

Code Climate has analyzed commit 0b7000e 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 41.2% (0.0% change).

View more on Code Climate.

@ggbecker ggbecker self-assigned this Oct 18, 2022
@ggbecker ggbecker added this to the 0.1.65 milestone Oct 18, 2022
@ggbecker ggbecker added SLES SUSE Linux Enterprise Server product related. Update Profile Issues or pull requests related to Profiles updates. pci-dss labels Oct 18, 2022
@ggbecker ggbecker merged commit c42383c into ComplianceAsCode:master Oct 18, 2022
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. pci-dss SLES SUSE Linux Enterprise Server product related. Update Profile Issues or pull requests related to Profiles updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants