Skip to content

Introduce centralised policy definitions #6499

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

Merged
merged 65 commits into from
Dec 22, 2020

Conversation

matejak
Copy link
Member

@matejak matejak commented Dec 22, 2020

This is a follow-up to #6089, as the source branch was updated. There are indications that the predecessor PR was misunderstood by reviewers at that time, and this is the most straightforward way of introducing it for reveiw again.

This PR presents a fully-functional PoC of a build system extension that allows to introduce product-agnostic elements to profile definitions.
Right now, profiles are tightly coupled with products. If there are two products that are very similar, respective profiles are almost or entirely identical. This creates following problems:

  • Profile development is product-dependent - if a profile is defined for more than one products, then profile experts will have to choose a "primary" product to develop the profile. This means that maintainers of other products and profiles will have to be aware of the development effort and that a particular profile is the "primary" one, as they will have to watch it and react to its development.
  • When a profile issue is discovered in a profile outside of the concentrated development effort, an there is therefore not a "primary" profile that everybody watches, there is no easy way how to propose the fix to other products. A proper way that ensures that the information is not overlooked is to open pull requests to equivalent profiles so maintainers can merge them. In case of ANSSI, that profile is duplicated 8 times in the project, so one could have to open up to 7 PRs.

The centralised policy definition is able to overcome this - products automatically "take" selections from the policy, so it can be the target of interest when working with profile experts, as well as target of bugfixes, when a bug is discovered by a product user. Individual profiles can add product-specific rules outside of the centralised policy if they wish, and profiles can be protected against introduction of unwanted rules using the profile stability tests.

Additional benefit of centralised policies is that it brings together the structure of our content to the structure of policies how they are defined in respective documents - see this example from the ANSSI policy:

image

Regarding the implementation aspect of the PR:

The controls/anssi.yml file contains ANSSI BP28 requirements stored in YAML format. The file is both machine-readable and human-readable. Each entry in the file represents an ANSSI requirement and contains the hardening level and a list of XCCDF rules mapped to this requirement. It concentrates the most important information into a single file. This form enables easier discussion and PRs on the rules mapping because it follows the logical structure of the original document.

The profile definitions leverage the fact that the controls/anssi.yml contains the rules and the profile levels they belong to. The profiles selection list in all 4 ANSSI profile files is replaced by a single statement that selects rules for the profile automatically.

The core part of the PR is in ssg/controls.py Python module. It contains simple classes Control, Policy and ControlsManager. These classes implement parsing the controls file and contains methods that resolve them to a list of rules. ControlsManager is then used by the build system. There is also a simple unit test in tests/unit/ssg-module/test_controls.py which tests these classes. There are a few example files used by the unit test in tests/unit/ssg-module/data/.

The ssg/build_yaml.py has been refactored . Then, there is a new class ProfileWithInlinePolicies which inhertis from the ResolvableProfile class. It can resolve the controls in the profile selections list, but it can still process the profiles that consist of individual rules. The class replaces its parent it during profile compilation ProfileWithInlinePolicies.

The build system change is subtle. The order of steps is still the same. Only the build-scripts/compile_profiles.py script which compiles the profiles is changed to use the controls during profile resolution. The compiled profiles still look the same, they contain a resolved list of rules, which means next build steps aren’t changed.

To avoid any confusion regarding possible unwanted implications PRs of this scale could introduce wrt content authors or developers, I emphasize that this PR

  • doesn't change anything regarding evaluation (scanning), it
  • doesn't change anything for profile authors that don't like this new format - they can still use the old one, and it even
  • doesn't change anything for people that would like to analyze profile composition using the traditional "profile = linear list of rules" approach, because one can analyze the compiled profile file. In fact, one should set up compiled profile stability test and have this partially automated.

Its main use case is to assist content development of ANSSI profiles, where the ability to record rationale of rule selections has a high added value.

jan-cerny and others added 30 commits December 18, 2020 16:44
The selections in child profiles will add or remove rules from
the parent profile and override the values of XCCDF values.
This will allow us to use the controls in our profiles and will
resolve the controls to the rulesduring the build process.
Using special keyword 'all' instead of list of controls in profile.yaml
all the controls from given policy will be selected. This is practical
for profiles that implement the whole controls catalog.
- Restructure profile classes, so their behavior can be extended by inheritance.
- Made ResolvableProfile generic profile that knows the concept of controls, without implementation details.
- Introduced ProfileWithInlinePolicies, renamed existing implementation to ProfileWithSeparatePolicies.
- Extended the test suite, so it is able to test more implementations in a scalable way.
- Added test for the all-inline case.
- Stramlined the inheritance relation of profiles.
- Removed _control_to_items as it was not helpful.
- Prevented conversion from control to ID and to control again in _controls_to_items
- Removed _policy_to_items
- Modified the flow, so now controls are obtained in a list, and then merged to the profile one-by-one.
In this format, the items of policies lists are tuples that consists
of a policy ID and control IDs separated by a colon.
and updated the profile accordingly.
Added the ANSSI minimal profile to the list of stable profiles.
Policies, controls and profiles can have a level assigned to them.
Low-level controls are supposed to be essential and always be present in a profile,
while the higher-level controls may be used only in strict profiles.
NT refers to the outdated version 1.1, whereas BP is for 1.2.
There is no need to target 1.1
Don't parse other files than those that end by .yml
ANSSI R5 is about defense in-depth, which is broad topic.
Note a few aspect of defense in-depth and add a few related rules.
yuumasato and others added 25 commits December 18, 2020 16:44
Add rules to addres one aspect of the requiremnt:
- password hashing algorithm
- salt size
- number of rounds
The selected rule assumes that rsyslog stores its logs in the
default path, /var/log/audit
The rule is about enforcing accountability of operations.
In version 1.2, the restriction requirement changed to the /boot
directory
The requirement is not totatlly automatable.
Discuss and select a few rules for it.
This rule is required by sshd_set_idle_timeout and must be selected
otherwise the OVAL check will always fail.
Co-authored-by: Gabriel Becker <ggasparb@redhat.com>
The profile is under intense development and doesn't make sense keep the
stability test for now.
It doesn't make sense to select a value called "default" since it's not
defined in the benchmark.
@matejak matejak added this to the 0.1.54 milestone Dec 22, 2020
@matejak matejak requested a review from carlosmmatos December 22, 2020 15:08
@openscap-ci
Copy link
Collaborator

Changes identified:
Profiles:
 anssi_nt28_enhanced on rhel7
 anssi_nt28_high on rhel7
 anssi_nt28_intermediary on rhel7
 anssi_nt28_minimal on rhel7
Others:
 Changes in Python files.

Show details

Profile anssi_nt28_enhanced on rhel7:
 New key in profile.
 Variable sshd_idle_timeout_value=5_minutes, var_accounts_user_umask=077 removed from anssi_nt28_enhanced profile.
 Rule !selinux_state, anssi:all:enhanced added to anssi_nt28_enhanced profile.
 Rule file_permissions_unauthorized_sgid, accounts_umask_etc_login_defs, accounts_tmout, accounts_umask_etc_profile, grub2_password, sshd_set_idle_timeout, file_permissions_unauthorized_suid, grub2_uefi_password, file_permissions_systemmap, sysctl_kernel_yama_ptrace_scope, sshd_set_keepalive removed from anssi_nt28_enhanced profile.
Profile anssi_nt28_high on rhel7:
 New key in profile.
 Variable var_selinux_policy_name=targeted, var_selinux_state=enforcing removed from anssi_nt28_high profile.
 Rule anssi:all:high added to anssi_nt28_high profile.
 Rule sebool_ssh_sysadm_login, grub2_enable_iommu_force, aide_verify_ext_attributes, sebool_secure_mode_insmod, selinux_state, package_setroubleshoot_removed, selinux_policytype, aide_build_database, aide_verify_acls, package_aide_installed, aide_scan_notification, aide_periodic_cron_checking removed from anssi_nt28_high profile.
Profile anssi_nt28_intermediary on rhel7:
 New key in profile.
 Rule anssi:all:intermediary added to anssi_nt28_intermediary profile.
 Rule sysctl_net_ipv4_conf_all_accept_source_route, sysctl_fs_protected_symlinks, sysctl_kernel_randomize_va_space, sysctl_net_ipv4_conf_default_send_redirects, sysctl_net_ipv4_conf_all_send_redirects, file_owner_etc_shadow, partition_for_home, mount_option_tmp_nosuid, file_owner_etc_gshadow, sysctl_net_ipv4_ip_forward, file_permissions_etc_passwd, sysctl_fs_protected_hardlinks, partition_for_var_log, sysctl_net_ipv4_conf_all_secure_redirects, sshd_disable_root_login, file_permissions_etc_shadow, sysctl_kernel_perf_event_paranoid, rsyslog_files_groupownership, sysctl_net_ipv4_conf_default_secure_redirects, sysctl_net_ipv6_conf_all_accept_source_route, sysctl_net_ipv6_conf_default_accept_source_route, ensure_logrotate_activated, sysctl_fs_suid_dumpable, file_permissions_etc_group, mount_option_var_tmp_nodev, file_permissions_etc_gshadow, mount_option_var_tmp_nosuid, sysctl_net_ipv4_conf_all_log_martians, sysctl_kernel_kptr_restrict, rsyslog_remote_loghost, sysctl_kernel_dmesg_restrict, sysctl_net_ipv6_conf_all_accept_redirects, rsyslog_files_permissions, mount_option_tmp_nodev, partition_for_var_tmp, sysctl_net_ipv4_conf_all_accept_redirects, partition_for_var, sysctl_net_ipv4_conf_all_rp_filter, partition_for_tmp, sysctl_net_ipv4_conf_default_accept_source_route, mount_option_tmp_noexec, mount_option_var_tmp_noexec, mount_option_home_nosuid, sysctl_net_ipv4_icmp_ignore_bogus_error_responses, sysctl_net_ipv6_conf_default_accept_redirects, sysctl_net_ipv4_conf_default_accept_redirects, rsyslog_files_ownership, sysctl_net_ipv4_conf_default_rp_filter, mount_option_home_nodev, no_direct_root_logins, sysctl_net_ipv4_tcp_syncookies removed from anssi_nt28_intermediary profile.
 Variable sysctl_net_ipv4_conf_all_accept_redirects_value=disabled, sysctl_net_ipv4_conf_default_accept_redirects_value=disabled removed from anssi_nt28_intermediary profile.
Profile anssi_nt28_minimal on rhel7:
 Rule anssi:all:minimal added to anssi_nt28_minimal profile.
 Rule ensure_redhat_gpgkey_installed, ensure_gpgcheck_never_disabled, set_password_hashing_algorithm_logindefs, service_rsyslog_enabled, security_patches_up_to_date, ensure_gpgcheck_globally_activated, package_sendmail_removed, sudo_remove_no_authenticate, package_rsyslog_installed, sudo_remove_nopasswd, package_dhcp_removed, package_telnetd_removed, ensure_gpgcheck_local_packages removed from anssi_nt28_minimal profile.
Others:
 Python abstract syntax tree change found in build-scripts/compile_profiles.py.
 Python abstract syntax tree change found in ssg/build_yaml.py.
 Python file ssg/controls.py is newly added.
 Python file tests/unit/ssg-module/test_controls.py is newly added.

Recommended tests to execute:
 build_product rhel7
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel7-ds.xml anssi_nt28_enhanced
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel7-ds.xml anssi_nt28_high
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel7-ds.xml anssi_nt28_intermediary
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel7-ds.xml anssi_nt28_minimal
 (cd build && cmake ../ && ctest -j4)

Copy link
Contributor

@carlosmmatos carlosmmatos left a comment

Choose a reason for hiding this comment

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

As per our discussions - this is fine to implement as it doesn't impact previous mechanisms of profile generation. Will merge based on the outcome of CI

@carlosmmatos carlosmmatos merged commit 7766dde into ComplianceAsCode:master Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants