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

Test and use rules to components mapping #10693

Merged
merged 27 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
766503d
Assign network_implement_access_control to firewalld component
jan-cerny May 30, 2023
4ef6a60
Enforce component mapping
jan-cerny May 23, 2023
2dacfff
Add a simple test for component consistency
jan-cerny May 25, 2023
06c2448
Add test for component based on platforms
jan-cerny May 25, 2023
699a453
Add a test for components based on groups
jan-cerny May 25, 2023
e2fcfbe
remove env_yaml
jan-cerny May 26, 2023
5569fc7
Add a unit test for the ssg.components module
jan-cerny May 26, 2023
32aa53b
Extract method
jan-cerny May 26, 2023
06726fb
Fix CodeClimate PEP8 problem
jan-cerny May 26, 2023
1487959
Reduce complexity of main function
jan-cerny May 26, 2023
1c21ea0
Reduce amount of parameters
jan-cerny May 26, 2023
caeb48d
Avoid many return statements
jan-cerny May 26, 2023
f442dd4
Use ssg.components module in _load_components
jan-cerny May 31, 2023
a13ed04
Allow products to specify the components directory
jan-cerny Jun 7, 2023
3a0b1eb
Extract function
jan-cerny Jun 7, 2023
a6c8c87
Simplify function test_platform
jan-cerny Jun 7, 2023
6211e68
Assign rule package_iptables-services_removed to a component
jan-cerny Jun 13, 2023
aef1955
Update example product.yml in Developer's guide
jan-cerny Jun 13, 2023
daa860f
Replace double underscore by single underscore
jan-cerny Jun 13, 2023
4c97b55
Add components to resolved rules
jan-cerny Jun 13, 2023
9649fc9
Make the parameters required
jan-cerny Jun 13, 2023
052e0c1
Create template testing plugins
jan-cerny Jun 13, 2023
c7fbf85
Replace must by should
jan-cerny Jun 13, 2023
f424f0d
Fix a traceback
jan-cerny Jun 13, 2023
0156763
Remove a space
jan-cerny Jun 13, 2023
4cdee0c
Fix CodeClimate problem
jan-cerny Jun 13, 2023
5d20bde
Change double underscore to a single underscore
jan-cerny Jun 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions components/chrony.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ name: chrony
packages:
- chrony
rules:
- chronyd_configure_pool_and_server
- chronyd_run_as_chrony_user
- chronyd_server_directive
- chronyd_specify_remote_server
Expand Down
1 change: 1 addition & 0 deletions components/firewalld.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ rules:
- firewalld-backend
- firewalld_loopback_traffic_restricted
- firewalld_loopback_traffic_trusted
- network_implement_access_control
- package_firewalld_installed
- package_firewalld_removed
- service_firewalld_disabled
Expand Down
1 change: 1 addition & 0 deletions components/iptables.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ rules:
- package_iptables-persistent_installed
- package_iptables-persistent_removed
- package_iptables-services_installed
- package_iptables-services_removed
- package_iptables_installed
- service_ip6tables_enabled
- service_iptables_enabled
4 changes: 4 additions & 0 deletions docs/manual/developer/03_creating_content.md
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,8 @@ type: platform

benchmark_root: "../../linux_os/guide"

components_root: "../../components"

profiles_root: "./profiles"

pkg_manager: "yum"
Expand Down Expand Up @@ -1254,3 +1256,5 @@ YAML file keys:
- `changelog` (list) - records substantial changes in the given component that affected rules and remediations (optional)

Each rule in the benchmark in the `/linux_os/guide` directory must be a member of at least 1 component.

Mab879 marked this conversation as resolved.
Show resolved Hide resolved
Products specify a path to the directory with component files by the `components_root` key in the `product.yml`.
1 change: 1 addition & 0 deletions products/example/product.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ type: platform

benchmark_id: EXAMPLE
benchmark_root: "../../linux_os/guide"
components_root: "../../components"

profiles_root: "./profiles"

Expand Down
1 change: 1 addition & 0 deletions products/fedora/product.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ type: platform

benchmark_id: FEDORA
benchmark_root: "../../linux_os/guide"
components_root: "../../components"

profiles_root: "./profiles"

Expand Down
1 change: 1 addition & 0 deletions products/rhel7/product.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ type: platform

benchmark_id: RHEL-7
benchmark_root: "../../linux_os/guide"
components_root: "../../components"

profiles_root: "./profiles"

Expand Down
1 change: 1 addition & 0 deletions products/rhel8/product.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ type: platform

benchmark_id: RHEL-8
benchmark_root: "../../linux_os/guide"
components_root: "../../components"

profiles_root: "./profiles"

Expand Down
1 change: 1 addition & 0 deletions products/rhel9/product.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ type: platform

benchmark_id: RHEL-9
benchmark_root: "../../linux_os/guide"
components_root: "../../components"

profiles_root: "./profiles"

Expand Down
47 changes: 38 additions & 9 deletions ssg/build_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import print_function

from copy import deepcopy
import collections
import datetime
import json
import os
Expand All @@ -12,6 +13,7 @@


import ssg.build_remediations
import ssg.components
from .build_cpe import CPEALLogicalTest, CPEALCheckFactRef, ProductCPEs
from .constants import (XCCDF12_NS,
OSCAP_BENCHMARK,
Expand Down Expand Up @@ -657,6 +659,7 @@ class Rule(XCCDFEntity, Templatable):
rationale=lambda: "",
severity=lambda: "",
references=lambda: dict(),
components=lambda: list(),
identifiers=lambda: dict(),
ocil_clause=lambda: None,
ocil=lambda: None,
Expand Down Expand Up @@ -1331,13 +1334,45 @@ def __init__(
self.stig_references = None
if stig_reference_path:
self.stig_references = ssg.build_stig.map_versions_to_rule_ids(stig_reference_path)
self.rule_to_components = self._load_components()

def _load_components(self):
if "components_root" not in self.env_yaml:
return None
product_dir = self.env_yaml["product_dir"]
components_root = self.env_yaml["components_root"]
components_dir = os.path.abspath(
os.path.join(product_dir, components_root))
components = ssg.components.load(components_dir)
rule_to_components = ssg.components.rule_component_mapping(
components)
return rule_to_components

def _process_values(self):
for value_yaml in self.value_files:
value = Value.from_yaml(value_yaml, self.env_yaml)
self.all_values[value.id_] = value
self.loaded_group.add_value(value)

def _process_rule(self, rule):
if self.rule_to_components is not None and rule.id_ not in self.rule_to_components:
Copy link
Member

Choose a reason for hiding this comment

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

This is motivated mainly by feedback to contributors that forget to assign components, and this case is not caught by tests. A build failure provides an early feedback, and the contributor will have to think of components early on.

However, there are disadvantages as well:

  • We may end up with an "other" component, but its rules would not share reasons to change, which is the main driving force behind components.
  • Although there is an existing division by package related to files that the check/remediation operates on, this structure again may not be beneficial.
  • If a rule should be assigned to more components, this error alone won't assure that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We may end up with an "other" component, but its rules would not share reasons to change, which is the main driving force behind components.

We can do it a more clever way, for rules that don't belong to any component we can put them to some predefined categories that aren't neccesary a RHEL component but make sense for the assessment, for example "filesystem", "users", "crypto".

Although there is an existing division by package related to files that the check/remediation operates on, this structure again may not be beneficial.

It's not a problem of this check, it's the problem of the way we did the initial assignment that is already merged to master.

If a rule should be assigned to more components, this error alone won't assure that.

Yes, but it would at least assure that the rule has at least one component. Once the reviewer of the PR will see that a component is modified, he might think about possible other components.

raise ValueError(
"The rule '%s' isn't mapped to any component! Insert the "
"rule ID at least once to the rule-component mapping." %
(rule.id_))
prodtypes = parse_prodtype(rule.prodtype)
if "all" not in prodtypes and self.product not in prodtypes:
return False
self.all_rules[rule.id_] = rule
self.loaded_group.add_rule(
rule, env_yaml=self.env_yaml, product_cpes=self.product_cpes)
rule.normalize(self.env_yaml["product"])
if self.stig_references:
rule.add_stig_references(self.stig_references)
if self.rule_to_components is not None:
rule.components = self.rule_to_components[rule.id_]
return True

def _process_rules(self):
for rule_yaml in self.rule_files:
try:
Expand All @@ -1346,16 +1381,8 @@ def _process_rules(self):
except DocumentationNotComplete:
# Happens on non-debug build when a rule is "documentation-incomplete"
continue
prodtypes = parse_prodtype(rule.prodtype)
if "all" not in prodtypes and self.product not in prodtypes:
if not self._process_rule(rule):
continue
self.all_rules[rule.id_] = rule
self.loaded_group.add_rule(
rule, env_yaml=self.env_yaml, product_cpes=self.product_cpes)

rule.normalize(self.env_yaml["product"])
if self.stig_references:
rule.add_stig_references(self.stig_references)

def _get_new_loader(self):
loader = BuildLoader(
Expand All @@ -1364,6 +1391,8 @@ def _get_new_loader(self):
loader.sce_metadata = self.sce_metadata
# Do it this way so we only have to parse the STIG references once.
loader.stig_references = self.stig_references
# Do it this way so we only have to parse the component metadata once.
loader.rule_to_components = self.rule_to_components
return loader

def export_group_to_file(self, filename):
Expand Down
49 changes: 49 additions & 0 deletions ssg/components.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
from __future__ import print_function

from collections import defaultdict
import os

import ssg.yaml


def load(components_dir):
components = {}
for component_filename in os.listdir(components_dir):
components_filepath = os.path.join(components_dir, component_filename)
component = Component(components_filepath)
components[component.name] = component
return components


def __reverse_mapping(components, attribute):
Copy link
Member

Choose a reason for hiding this comment

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

Double underscore here as well.

mapping = defaultdict(list)
for component in components.values():
for item in getattr(component, attribute):
mapping[item].append(component.name)
return mapping


def package_component_mapping(components):
return __reverse_mapping(components, "packages")


def template_component_mapping(components):
return __reverse_mapping(components, "templates")


def group_component_mapping(components):
return __reverse_mapping(components, "groups")


def rule_component_mapping(components):
return __reverse_mapping(components, "rules")


class Component:
def __init__(self, filepath):
yaml_data = ssg.yaml.open_raw(filepath)
self.name = yaml_data["name"]
self.rules = yaml_data["rules"]
self.packages = yaml_data["packages"]
self.templates = yaml_data.get("templates", [])
self.groups = yaml_data.get("groups", [])
7 changes: 7 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,15 @@ if(PYTHON_VERSION_MAJOR GREATER 2)
set_tests_properties("install-vm" PROPERTIES LABELS quick)
endif()

if(SSG_PRODUCT_RHEL9)
add_test(
NAME "automatus-sanity"
COMMAND env "PYTHONPATH=$ENV{PYTHONPATH}" "${PYTHON_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/tests/automatus.py" "--help"
)
set_tests_properties("automatus-sanity" PROPERTIES LABELS quick)

add_test(
Mab879 marked this conversation as resolved.
Show resolved Hide resolved
NAME "components"
COMMAND env "PYTHONPATH=$ENV{PYTHONPATH}" "${PYTHON_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/test_components.py" --build-dir "${CMAKE_BINARY_DIR}" --source-dir "${CMAKE_SOURCE_DIR}" --product "rhel9"
)
endif()
1 change: 1 addition & 0 deletions tests/data/product_stability/fedora.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ basic_properties_derived: true
benchmark_id: FEDORA
benchmark_root: ../../linux_os/guide
chrony_conf_path: /etc/chrony.conf
components_root: ../../components
cpes:
- fedora_40:
check_id: installed_OS_is_fedora
Expand Down
1 change: 1 addition & 0 deletions tests/data/product_stability/rhel7.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ centos_major_version: '7'
centos_pkg_release: 53a7ff4b
centos_pkg_version: f4a80eb5
chrony_conf_path: /etc/chrony.conf
components_root: ../../components
cpes:
- rhel7:
check_id: installed_OS_is_rhel7
Expand Down
1 change: 1 addition & 0 deletions tests/data/product_stability/rhel8.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ centos_major_version: '8'
centos_pkg_release: 5ccc5b19
centos_pkg_version: 8483c65d
chrony_conf_path: /etc/chrony.conf
components_root: ../../components
cpes:
- rhel8:
check_id: installed_OS_is_rhel8
Expand Down
1 change: 1 addition & 0 deletions tests/data/product_stability/rhel9.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ centos_major_version: '9'
centos_pkg_release: 5ccc5b19
centos_pkg_version: 8483c65d
chrony_conf_path: /etc/chrony.conf
components_root: ../../components
cpes:
- rhel9:
check_id: installed_OS_is_rhel9
Expand Down