diff --git a/applications/openshift-virtualization/kubevirt-restrict-migration-tools-access/rule.yml b/applications/openshift-virtualization/kubevirt-restrict-migration-tools-access/rule.yml new file mode 100644 index 000000000000..2981753c5ad3 --- /dev/null +++ b/applications/openshift-virtualization/kubevirt-restrict-migration-tools-access/rule.yml @@ -0,0 +1,33 @@ +documentation_complete: true + +title: 'Restrict Namespace Administrator Access to Migration Tools' + +description: |- + Because the set of authorized subjects is specific to each environment, + this rule requires manual verification. Organizations can create a + CustomRule using CEL to automate this check by evaluating + ClusterRoleBindings and ClusterRoles for create and update access to + VirtualMachineInstanceMigration and + MigrationPolicy resources, verifying that only approved + subjects are bound to those permissions. + +rationale: |- + Virtual machine live migration moves a running VM between nodes. + Granting the ability to create VirtualMachineInstanceMigration + or MigrationPolicy objects to untrusted or unnecessary + subjects increases the risk of unplanned resource contention, + denial of service through excessive migrations, and potential + exposure of workload data during the migration process. Restricting + access to these resources ensures that only approved administrators + can initiate or influence VM migration behavior. + +severity: medium + +ocil_clause: 'unauthorized subjects can create vmim or migrationpolicy resources' + +ocil: |- + Run the following commands to check which subjects can create + migration-related resources: +
$ oc adm policy who-can create vmim+
$ oc adm policy who-can create migrationpolicy+ Verify that only authorized subjects are listed in the output. diff --git a/build-scripts/build_cel_content.py b/build-scripts/build_cel_content.py index 74786b6b9185..acf270887495 100755 --- a/build-scripts/build_cel_content.py +++ b/build-scripts/build_cel_content.py @@ -61,28 +61,35 @@ def setup_logging(log_level_str): logging.basicConfig(format=MESSAGE_FORMAT, level=numeric_level) -def load_cel_rules(rules_dir): +def load_rules(rules_dir): """ - Load all rules that use the CEL checking engine. + Load all compiled rules, separating rules with CEL check from the rest. + + Scans the directory once, returning CEL rules (with expression and + inputs) and all remaining rules keyed by ID. Args: rules_dir: Directory containing resolved rule JSON files Returns: - dict: Dictionary of rule_id -> rule object for rules with CEL checks + tuple: (cel_rules, all_rules) where cel_rules is a dict of + rule_id -> rule object for rules with CEL checks, and + all_rules is a dict of rule_id -> rule object for all rules Raises: ValueError: If a rule with CEL checks is missing required fields """ cel_rules = {} + all_rules = {} if not os.path.isdir(rules_dir): - return cel_rules + return cel_rules, all_rules for rule_file in os.listdir(rules_dir): rule_path = os.path.join(rules_dir, rule_file) try: rule = ssg.build_yaml.Rule.from_compiled_json(rule_path) + all_rules[rule.id_] = rule # Check if this rule has CEL checks by looking for CEL-specific fields # A rule uses CEL if it has both expression and inputs @@ -91,7 +98,6 @@ def load_cel_rules(rules_dir): has_inputs = hasattr(rule, 'inputs') and rule.inputs if has_expression and has_inputs: - # Validate required CEL fields rule_name = rule_id_to_name(rule.id_) if not hasattr(rule, 'check_type') or not rule.check_type: @@ -102,16 +108,14 @@ def load_cel_rules(rules_dir): cel_rules[rule.id_] = rule except ssg.build_yaml.DocumentationNotComplete: - # Skip documentation-incomplete rules in non-debug builds continue except ValueError: - # Re-raise validation errors raise except Exception as e: logging.warning("Failed to load rule from %s: %s", rule_file, e) continue - return cel_rules + return cel_rules, all_rules def load_profiles(profiles_dir, cel_rule_ids): @@ -300,27 +304,55 @@ def profile_to_cel_dict(profile, cel_rule_ids): return cel_profile -def generate_cel_content(cel_rules, profiles): +def generate_cel_content(cel_rules, profiles, all_rules=None): """ Generate the complete CEL content structure. Args: cel_rules: Dictionary of rules with CEL checks profiles: List of profiles targeting the CEL checking engine + all_rules: Dictionary of all compiled rule objects keyed by ID + (used to distinguish manual rules from nonexistent rules) Returns: dict: Complete CEL content structure Raises: - ValueError: If duplicate rule names found or profile references unknown rules + ValueError: If duplicate rule names found or profile references + a rule that does not exist """ cel_rule_ids = set(cel_rules.keys()) + if all_rules is None: + all_rules = {} + all_rule_ids = set(all_rules.keys()) + + # Process profiles first to identify manual rules + output_rules = dict(cel_rules) + for profile in profiles: + profile_name = rule_id_to_name(profile.id_) + for rule_id in profile.selected: + if rule_id not in cel_rule_ids: + rule_name = rule_id_to_name(rule_id) + if rule_id in all_rule_ids: + logging.warning( + "profile '%s' references rule '%s' without CEL checks " + "- adding as manual rule", + profile_name, rule_name, + ) + output_rules[rule_id] = all_rules[rule_id] + else: + raise ValueError( + f"profile '{profile_name}' references unknown rule " + f"'{rule_name}'" + ) + + output_rule_ids = set(output_rules.keys()) # Generate rules section and check for duplicates cel_rules_list = [] rule_names_seen = set() - for rule_id in sorted(cel_rules.keys()): - rule = cel_rules[rule_id] + for rule_id in sorted(output_rules.keys()): + rule = output_rules[rule_id] cel_rule = rule_to_cel_dict(rule) # Check for duplicate rule names @@ -331,19 +363,10 @@ def generate_cel_content(cel_rules, profiles): cel_rules_list.append(cel_rule) - # Generate profiles section and validate rule references + # Generate profiles section cel_profiles = [] for profile in profiles: - # Validate that all selected rules have CEL checks - profile_name = rule_id_to_name(profile.id_) - for rule_id in profile.selected: - if rule_id not in cel_rule_ids: - rule_name = rule_id_to_name(rule_id) - raise ValueError( - f"profile '{profile_name}' references unknown rule '{rule_name}'" - ) - - cel_profile = profile_to_cel_dict(profile, cel_rule_ids) + cel_profile = profile_to_cel_dict(profile, output_rule_ids) if cel_profile: cel_profiles.append(cel_profile) @@ -360,17 +383,18 @@ def main(): args = parse_args() setup_logging(args.log) - # Load rules with CEL checks - cel_rules = load_cel_rules(args.resolved_rules_dir) + # Load all rules in a single pass + cel_rules, all_rules = load_rules(args.resolved_rules_dir) - if not cel_rules: + # Load profiles + cel_rule_ids = set(cel_rules.keys()) + profiles = load_profiles(args.profiles_dir, cel_rule_ids) + + if not cel_rules and not profiles: content = {'profiles': [], 'rules': []} else: - # Load profiles - profiles = load_profiles(args.profiles_dir, set(cel_rules.keys())) - # Generate CEL content - content = generate_cel_content(cel_rules, profiles) + content = generate_cel_content(cel_rules, profiles, all_rules) # Write output YAML os.makedirs(os.path.dirname(args.output), exist_ok=True) diff --git a/docs/manual/developer/13_cel_content.md b/docs/manual/developer/13_cel_content.md index 91cbddf9c15d..a9b2734942cc 100644 --- a/docs/manual/developer/13_cel_content.md +++ b/docs/manual/developer/13_cel_content.md @@ -141,7 +141,7 @@ selections: - kubevirt-persistent-reservation-disabled ``` -**Important:** CEL profiles can only select CEL rules. If a profile includes both CEL and OVAL rules, only the CEL rules will be included in the generated CEL content file. +**Important:** CEL profiles can select both CEL and SCAP rules. Rules selected by a CEL profile that don't have a CEL check (no cel/shared.yml) will be included as manual rules with a build warning. ## Creating a CEL Rule @@ -347,8 +347,8 @@ The build system validates CEL content automatically: **Profile Validation:** - `selected` field must contain at least one rule -- All selected rules must exist in CEL rules -- Profile cannot reference OVAL rules +- Rules without CEL checks are included as manual rules with a warning +- Both CEL rules and manual rules are included in the generated content **Content Validation:** - No duplicate rule names (after underscore-to-hyphen conversion) @@ -434,9 +434,10 @@ cel-spec '{"resource": {"spec": {"enabled": true}}}' 'resource.spec.enabled == t **Error: `CEL profile 'profile-name' has no rules`** - Add rules to the `selections` field in the profile -**Error: `profile 'profile-name' references unknown rule 'rule-name'`** -- Verify the rule exists and has CEL checks (has `cel/shared.yml` with `expression` and `inputs`) -- Check the rule ID matches the profile selection +**Warning: `profile 'profile-name' references rule 'rule-name' without CEL checks - adding as manual rule`** +- This is expected for rules that have no automated CEL check +- The rule will be included in CEL content output without expression and inputs +- If this is unintentional, verify the rule has `cel/shared.yml` with `expression` and `inputs` ### CEL Content Not Generated diff --git a/products/ocp4/profiles/cis-vm-extension.profile b/products/ocp4/profiles/cis-vm-extension.profile index e1d0497b613e..850b75720679 100644 --- a/products/ocp4/profiles/cis-vm-extension.profile +++ b/products/ocp4/profiles/cis-vm-extension.profile @@ -35,3 +35,4 @@ selections: - kubevirt-localnet-vlan-required - kubevirt-sriov-spoofchk-on - kubevirt-bridge-mac-spoof-filtering + - kubevirt-restrict-migration-tools-access diff --git a/tests/unit/ssg-module/test_build_cel_content.py b/tests/unit/ssg-module/test_build_cel_content.py index c0fbbe67fa85..dacfd198a1ad 100644 --- a/tests/unit/ssg-module/test_build_cel_content.py +++ b/tests/unit/ssg-module/test_build_cel_content.py @@ -239,25 +239,29 @@ def test_extract_controls_from_references(): assert controls_empty == {} -def test_load_cel_rules(temp_rules_dir): +def test_load_rules(temp_rules_dir): """Test loading rules with CEL checks from directory.""" - cel_rules = build_cel_content.load_cel_rules(temp_rules_dir) + cel_rules, all_rules = build_cel_content.load_rules(temp_rules_dir) - # Should load only the rule with CEL checks (identified by presence of expression + inputs) + # Should load only the rule with CEL checks assert len(cel_rules) == 1 assert 'kubevirt_nonroot_feature_gate_is_enabled' in cel_rules + # all_rules should contain both CEL and non-CEL rules + assert len(all_rules) == 2 + assert 'some_oval_rule' in all_rules + rule = cel_rules['kubevirt_nonroot_feature_gate_is_enabled'] - # Rules with CEL checks are identified by presence of expression and inputs assert hasattr(rule, 'expression') and rule.expression assert hasattr(rule, 'inputs') and rule.inputs assert rule.title == 'Ensure NonRoot Feature Gate is Enabled' -def test_load_cel_rules_nonexistent_dir(): - """Test loading rules with CEL checks from nonexistent directory.""" - cel_rules = build_cel_content.load_cel_rules('/nonexistent/path') +def test_load_rules_nonexistent_dir(): + """Test loading rules from nonexistent directory.""" + cel_rules, all_rules = build_cel_content.load_rules('/nonexistent/path') assert cel_rules == {} + assert all_rules == {} def test_load_profiles(temp_profiles_dir): @@ -469,12 +473,13 @@ def test_load_cel_rules_missing_expression(): # Should not raise error - rule is not identified as CEL without both expression and inputs # This rule will be skipped since it doesn't have both fields - cel_rules = build_cel_content.load_cel_rules(tmpdir) - assert len(cel_rules) == 0 # Rule should be skipped + cel_rules, all_rules = build_cel_content.load_rules(tmpdir) + assert len(cel_rules) == 0 # Not a CEL rule + assert len(all_rules) == 1 # But still loaded as a rule def test_load_cel_rules_missing_inputs(): - """Test that rule without inputs is skipped.""" + """Test that rule without inputs is skipped from CEL rules.""" with tempfile.TemporaryDirectory() as tmpdir: # Create rule without inputs (but with expression - incomplete for CEL checks) rule_dict = { @@ -493,10 +498,9 @@ def test_load_cel_rules_missing_inputs(): with open(rule_path, 'w') as f: json.dump(rule_dict, f) - # Should not raise error - rule is not identified as CEL without both expression and inputs - # This rule will be skipped since it doesn't have both fields - cel_rules = build_cel_content.load_cel_rules(tmpdir) - assert len(cel_rules) == 0 # Rule should be skipped + cel_rules, all_rules = build_cel_content.load_rules(tmpdir) + assert len(cel_rules) == 0 # Not a CEL rule + assert len(all_rules) == 1 # But still loaded as a rule def test_load_profiles_no_rules(): @@ -574,6 +578,8 @@ def test_generate_cel_content_unknown_rule_reference(): 'existing_rule': rule1 } + all_rules = {'existing_rule': rule1} + # Create a profile that references a non-existent rule profile = ssg.build_yaml.Profile('test_profile') profile.id_ = 'test_profile' @@ -584,7 +590,7 @@ def test_generate_cel_content_unknown_rule_reference(): profiles = [profile] with pytest.raises(ValueError, match="references unknown rule 'nonexistent-rule'"): - build_cel_content.generate_cel_content(cel_rules, profiles) + build_cel_content.generate_cel_content(cel_rules, profiles, all_rules) def test_validation_empty_expression(): @@ -608,13 +614,13 @@ def test_validation_empty_expression(): with open(rule_path, 'w') as f: json.dump(rule_dict, f) - # Empty expression means rule is not identified as CEL and is skipped - cel_rules = build_cel_content.load_cel_rules(tmpdir) + cel_rules, all_rules = build_cel_content.load_rules(tmpdir) assert len(cel_rules) == 0 + assert len(all_rules) == 1 def test_validation_empty_inputs(): - """Test that rule with empty inputs list is skipped.""" + """Test that rule with empty inputs list is not identified as CEL but is included in all rules.""" with tempfile.TemporaryDirectory() as tmpdir: # Create rule with empty inputs rule_dict = { @@ -634,9 +640,9 @@ def test_validation_empty_inputs(): with open(rule_path, 'w') as f: json.dump(rule_dict, f) - # Empty inputs means rule is not identified as CEL and is skipped - cel_rules = build_cel_content.load_cel_rules(tmpdir) + cel_rules, all_rules = build_cel_content.load_rules(tmpdir) assert len(cel_rules) == 0 + assert len(all_rules) == 1 def test_validation_profile_with_empty_selections(): @@ -663,7 +669,7 @@ def test_validation_profile_with_empty_selections(): def test_validation_mixed_oval_and_cel_in_profile(): - """Test that profile with both OVAL and CEL checks only includes rules with CEL checks.""" + """Test that profile with both OVAL and CEL checks are output as CEL and manual rules.""" # Create rule with CEL checks cel_rule = ssg.build_yaml.Rule('cel_rule') cel_rule.id_ = 'cel_rule' @@ -679,19 +685,73 @@ def test_validation_mixed_oval_and_cel_in_profile(): 'cel_rule': cel_rule } + # oval_rule exists as a compiled rule but has no CEL checks + oval_rule = ssg.build_yaml.Rule('oval_rule') + oval_rule.id_ = 'oval_rule' + oval_rule.title = 'OVAL Rule' + oval_rule.description = 'Description' + oval_rule.rationale = 'Rationale' + oval_rule.severity = 'medium' + oval_rule.references = {} + + all_rules = {'cel_rule': cel_rule, 'oval_rule': oval_rule} + # Create a CEL profile that references both CEL and OVAL rules - # (OVAL rules won't be in cel_rule_ids) profile = ssg.build_yaml.Profile('mixed_profile') profile.id_ = 'mixed_profile' profile.title = 'Mixed Profile' profile.description = 'Test' - profile.selected = ['cel_rule', 'oval_rule'] # oval_rule doesn't have CEL checks + profile.selected = ['cel_rule', 'oval_rule'] profiles = [profile] - # This should fail because oval_rule doesn't have CEL checks - with pytest.raises(ValueError, match="references unknown rule 'oval-rule'"): - build_cel_content.generate_cel_content(cel_rules, profiles) + # Should warn about oval_rule and include it as a manual rule + content = build_cel_content.generate_cel_content(cel_rules, profiles, all_rules) + assert len(content['rules']) == 2 + rule_ids = [r['id'] for r in content['rules']] + assert 'cel_rule' in rule_ids + assert 'oval_rule' in rule_ids + + # Verify the CEL rule has expression and inputs + cel_output = next(r for r in content['rules'] if r['id'] == 'cel_rule') + assert 'expression' in cel_output + assert 'inputs' in cel_output + + # Verify the manual rule does NOT have expression or inputs + oval_output = next(r for r in content['rules'] if r['id'] == 'oval_rule') + assert 'expression' not in oval_output + assert 'inputs' not in oval_output + + # Verify the profile includes both rules + assert len(content['profiles']) == 1 + assert len(content['profiles'][0]['rules']) == 2 + + +def test_generate_cel_content_manual_only_profile(): + """Test that a profile with only manual rules (no CEL rules) is output correctly.""" + manual_rule = ssg.build_yaml.Rule('manual_only_rule') + manual_rule.id_ = 'manual_only_rule' + manual_rule.title = 'Manual Only Rule' + manual_rule.description = 'Description' + manual_rule.rationale = 'Rationale' + manual_rule.severity = 'medium' + manual_rule.references = {} + + all_rules = {'manual_only_rule': manual_rule} + + profile = ssg.build_yaml.Profile('manual_profile') + profile.id_ = 'manual_profile' + profile.title = 'Manual Profile' + profile.description = 'Profile with only manual rules' + profile.selected = ['manual_only_rule'] + + content = build_cel_content.generate_cel_content({}, [profile], all_rules) + assert len(content['rules']) == 1 + assert content['rules'][0]['id'] == 'manual_only_rule' + assert 'expression' not in content['rules'][0] + assert 'inputs' not in content['rules'][0] + assert len(content['profiles']) == 1 + assert len(content['profiles'][0]['rules']) == 1 def test_validation_integration_full_flow(): @@ -732,7 +792,7 @@ def test_validation_integration_full_flow(): json.dump(profile_dict, f) # Load and validate - cel_rules = build_cel_content.load_cel_rules(rules_dir) + cel_rules, all_rules = build_cel_content.load_rules(rules_dir) assert len(cel_rules) == 1 assert 'valid_cel_rule' in cel_rules @@ -741,7 +801,7 @@ def test_validation_integration_full_flow(): assert len(profiles) == 1 # Generate content - content = build_cel_content.generate_cel_content(cel_rules, profiles) + content = build_cel_content.generate_cel_content(cel_rules, profiles, all_rules) assert len(content['rules']) == 1 assert len(content['profiles']) == 1 assert content['rules'][0]['name'] == 'valid-cel-rule'