From 76c870fccacc096f80cd2c8611b7d82ef7ec4a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Jeanneret?= Date: Mon, 6 Jul 2026 09:57:25 +0200 Subject: [PATCH] security: enforce least-privilege RBAC and prevent wildcard regressions Apply principle of least privilege to ArgoCD ClusterRole permissions and add automated CI validation to prevent future regressions. RBAC Improvements: - Replace wildcard verbs on core API resources (secrets, pods, PVCs) with explicit verb lists to prevent implicit dangerous permissions - Change pods permissions to read-only (get, list, watch) - ArgoCD only needs to monitor pod status, not create/delete them - Replace wildcard resources in metal3.io, lvm.topolvm.io, and baremetal.openstack.org with explicit resource lists - Comment out unused multicluster.openshift.io permissions Security Rationale: - Wildcard verbs ('*') grant implicit permissions including 'impersonate', 'escalate', and 'bind' which can lead to privilege escalation - Wildcard resources ('*') prevent least-privilege enforcement and grant access to all current and future resources in an API group - Explicit permissions reduce attack surface and follow Kubernetes RBAC best practices CI Regression Prevention: - Add automated RBAC validation script that scans ClusterRole/Role definitions for dangerous wildcard patterns - Integrate validation as new job in yamllint workflow (runs on all PRs) - Fail CI if critical violations detected (wildcard verbs on core API resources or wildcard resources in any API group) - Provide clear remediation guidance in error messages - Document validation rules, security rationale, and how to fix violations Co-Authored-By: Claude Sonnet 4.5 --- .github/scripts/check-rbac-wildcards.py | 365 ++++++++++++++++++ .github/workflows/yamllint.yml | 18 + AGENTS.md | 2 + docs/agents/ci-and-validation.md | 3 + docs/agents/rbac-security.md | 157 ++++++++ docs/skills/rbac-security-validation.md | 250 ++++++++++++ .../enable/clusterrole.yaml | 75 +++- 7 files changed, 857 insertions(+), 13 deletions(-) create mode 100755 .github/scripts/check-rbac-wildcards.py create mode 100644 docs/agents/rbac-security.md create mode 100644 docs/skills/rbac-security-validation.md diff --git a/.github/scripts/check-rbac-wildcards.py b/.github/scripts/check-rbac-wildcards.py new file mode 100755 index 0000000..066d8ff --- /dev/null +++ b/.github/scripts/check-rbac-wildcards.py @@ -0,0 +1,365 @@ +#!/usr/bin/env python3 +""" +RBAC Wildcard Validator for CI + +Scans ClusterRole and Role definitions for dangerous wildcard permissions. +Fails CI if critical patterns are detected. + +Security rationale: +- Wildcard verbs ('*') on core API resources like secrets/pods grants implicit + dangerous permissions: 'impersonate', 'escalate', 'bind' +- Wildcard resources ('*') in any API group prevents least-privilege enforcement +- These patterns were identified as HIGH severity (CVSS 9.1) in security audit + +Usage: + python3 check-rbac-wildcards.py + +Exit codes: + 0 - No critical violations found + 1 - Critical violations detected (fails CI) +""" + +import sys +import yaml +from pathlib import Path +from dataclasses import dataclass +from typing import List, Set, Dict, Any + +# ANSI color codes for terminal output +RED = '\033[91m' +YELLOW = '\033[93m' +GREEN = '\033[92m' +BLUE = '\033[94m' +BOLD = '\033[1m' +RESET = '\033[0m' + + +@dataclass +class RBACViolation: + """Represents a detected RBAC security violation""" + file_path: str + kind: str + name: str + api_groups: List[str] + resources: List[str] + verbs: List[str] + severity: str # 'critical' or 'warning' + reason: str + + +# CRITICAL: Core API resources that should NEVER have wildcard verbs +# Wildcard on these grants dangerous implicit permissions like: +# - impersonate (become any user/group) +# - escalate (grant higher privileges) +# - bind (bind to privileged roles) +CRITICAL_CORE_RESOURCES = { + 'secrets', + 'pods', + 'persistentvolumeclaims', + 'serviceaccounts', + 'nodes', + 'namespaces' +} + +# Domain-specific OpenStack CRDs - wildcard verbs are lower risk +# These don't grant cluster-admin escalation paths +OPENSTACK_API_GROUPS = { + 'core.openstack.org', + 'network.openstack.org', + 'dataplane.openstack.org', + 'operator.openstack.org', + 'baremetal.openstack.org', + 'topology.openstack.org' +} + + +def find_rbac_files() -> List[Path]: + """ + Find all YAML files containing ClusterRole or Role definitions. + + Returns: + List of Path objects pointing to files with RBAC resources + """ + rbac_files = [] + repo_root = Path('.').resolve() + + # Search for YAML files + for yaml_file in repo_root.glob('**/*.yaml'): + # Skip git directory and hidden files + if '.git' in yaml_file.parts or any(part.startswith('.') for part in yaml_file.parts[:-1]): + continue + + try: + with open(yaml_file, 'r') as f: + # Quick check: does file contain ClusterRole or Role? + content = f.read() + if 'kind: ClusterRole' in content or 'kind: Role' in content: + rbac_files.append(yaml_file) + except (IOError, UnicodeDecodeError): + # Skip files that can't be read + continue + + return rbac_files + + +def parse_rbac_file(file_path: Path) -> List[Dict[str, Any]]: + """ + Parse YAML file and extract RBAC resources. + + Args: + file_path: Path to YAML file + + Returns: + List of RBAC resource dictionaries (ClusterRole or Role) + """ + rbac_resources = [] + + try: + with open(file_path, 'r') as f: + # Handle multi-document YAML files + for doc in yaml.safe_load_all(f): + if doc and isinstance(doc, dict): + kind = doc.get('kind', '') + if kind in ('ClusterRole', 'Role'): + rbac_resources.append(doc) + except yaml.YAMLError as e: + print(f"{YELLOW}Warning: Could not parse {file_path}: {e}{RESET}") + except IOError as e: + print(f"{YELLOW}Warning: Could not read {file_path}: {e}{RESET}") + + return rbac_resources + + +def validate_rbac_rule( + rule: Dict[str, Any], + rbac_kind: str, + rbac_name: str, + file_path: str +) -> List[RBACViolation]: + """ + Validate a single RBAC rule for dangerous patterns. + + Args: + rule: RBAC rule dictionary with apiGroups, resources, verbs + rbac_kind: ClusterRole or Role + rbac_name: Name of the RBAC resource + file_path: Path to file containing this rule + + Returns: + List of RBACViolation objects (empty if no violations) + """ + violations = [] + + api_groups = rule.get('apiGroups', []) + resources = rule.get('resources', []) + verbs = rule.get('verbs', []) + + # CRITICAL CHECK 1: Wildcard verbs on core API sensitive resources + if '' in api_groups and '*' in verbs: + sensitive_resources = set(resources) & CRITICAL_CORE_RESOURCES + if sensitive_resources: + violations.append(RBACViolation( + file_path=file_path, + kind=rbac_kind, + name=rbac_name, + api_groups=api_groups, + resources=sorted(list(sensitive_resources)), + verbs=verbs, + severity='critical', + reason=( + f"Wildcard verbs on core API resources {sorted(list(sensitive_resources))}. " + f"This grants dangerous implicit permissions: 'impersonate', 'escalate', 'bind'. " + f"Replace with explicit verbs: [get, list, watch, create, update, patch, delete]" + ) + )) + + # CRITICAL CHECK 2: Wildcard resources in any API group + # Exception: OpenStack CRDs are domain-specific, lower risk + if '*' in resources: + # Check if this is an OpenStack CRD (warning) or critical API group + if any(group in OPENSTACK_API_GROUPS for group in api_groups): + # OpenStack CRDs: warning only (can be upgraded to critical later) + violations.append(RBACViolation( + file_path=file_path, + kind=rbac_kind, + name=rbac_name, + api_groups=api_groups, + resources=resources, + verbs=verbs, + severity='warning', + reason=( + f"Wildcard resources in OpenStack API group {api_groups}. " + f"Consider replacing with explicit resource list for better security." + ) + )) + else: + # Non-OpenStack wildcards: critical violation + violations.append(RBACViolation( + file_path=file_path, + kind=rbac_kind, + name=rbac_name, + api_groups=api_groups, + resources=resources, + verbs=verbs, + severity='critical', + reason=( + f"Wildcard resources in API group {api_groups}. " + f"Replace with explicit resource list based on actual usage. " + f"See docs/skills/rbac-security-validation.md for guidance." + ) + )) + + # WARNING CHECK: Wildcard verbs on non-core resources + # Report for visibility, but don't fail CI (domain-specific CRDs) + if '*' in verbs and '' not in api_groups: + # Check if it's OpenStack CRD + if any(group in OPENSTACK_API_GROUPS for group in api_groups): + violations.append(RBACViolation( + file_path=file_path, + kind=rbac_kind, + name=rbac_name, + api_groups=api_groups, + resources=resources, + verbs=verbs, + severity='warning', + reason=( + f"Wildcard verbs on OpenStack CRD {api_groups}. " + f"This is acceptable for domain-specific resources, but consider explicit verbs for clarity." + ) + )) + + return violations + + +def format_violation_report(violations: List[RBACViolation]) -> str: + """ + Format violations as a readable report with boxes. + + Args: + violations: List of RBACViolation objects + + Returns: + Formatted string report + """ + if not violations: + return f"{GREEN}✅ No RBAC violations found{RESET}" + + critical = [v for v in violations if v.severity == 'critical'] + warnings = [v for v in violations if v.severity == 'warning'] + + report = [] + + if critical: + report.append(f"\n{RED}{BOLD}❌ RBAC Security Validation FAILED{RESET}\n") + report.append(f"{RED}Critical Violations Found ({len(critical)}):{RESET}\n") + + for i, v in enumerate(critical, 1): + report.append("┌" + "─" * 78 + "┐") + report.append(f"│ {BOLD}Violation #{i}{RESET}" + " " * 65 + "│") + report.append(f"│ File: {v.file_path}" + " " * (77 - len(f"File: {v.file_path}")) + "│") + report.append(f"│ Kind: {v.kind}" + " " * (77 - len(f"Kind: {v.kind}")) + "│") + report.append(f"│ Name: {v.name}" + " " * (77 - len(f"Name: {v.name}")) + "│") + report.append("├" + "─" * 78 + "┤") + report.append(f"│ API Groups: {v.api_groups}" + " " * (77 - len(f"API Groups: {v.api_groups}")) + "│") + report.append(f"│ Resources: {v.resources}" + " " * (77 - len(f"Resources: {v.resources}")) + "│") + report.append(f"│ Verbs: {v.verbs}" + " " * (77 - len(f"Verbs: {v.verbs}")) + "│") + report.append("├" + "─" * 78 + "┤") + report.append(f"│ {RED}⚠️ SEVERITY: {v.severity.upper()}{RESET}" + " " * (77 - len(f"⚠️ SEVERITY: {v.severity.upper()}") - 9) + "│") + + # Wrap reason text to fit in box + reason_lines = [] + words = v.reason.split() + current_line = "" + for word in words: + if len(current_line) + len(word) + 1 <= 76: + current_line += (word + " ") + else: + reason_lines.append(current_line.strip()) + current_line = word + " " + if current_line: + reason_lines.append(current_line.strip()) + + for line in reason_lines: + report.append(f"│ {line}" + " " * (77 - len(line)) + "│") + + report.append("└" + "─" * 78 + "┘\n") + + if warnings: + report.append(f"\n{YELLOW}Warnings ({len(warnings)}):{RESET}\n") + for w in warnings: + report.append(f" {YELLOW}⚠{RESET} {w.file_path} ({w.kind} {w.name})") + report.append(f" {w.reason}\n") + + if critical: + report.append(f"\n{BOLD}Remediation:{RESET}") + report.append("1. Replace verbs: ['*'] with explicit list: [get, list, watch, create, update, patch, delete]") + report.append("2. Replace resources: ['*'] with explicit resource kinds") + report.append("3. See docs/skills/rbac-security-validation.md for detailed guidance") + report.append("4. Refer to security audit: gitops-findings/gitops/gitops-security-audit.md") + report.append(f"\n{RED}{BOLD}CI FAILED: RBAC security violations detected{RESET}\n") + + return "\n".join(report) + + +def check_rbac_wildcards() -> int: + """ + Main function: scan, validate, and report RBAC violations. + + Returns: + 0 if no critical violations, 1 if violations found + """ + print(f"\n{BLUE}{BOLD}RBAC Wildcard Security Validator{RESET}") + print(f"{BLUE}Scanning repository for dangerous RBAC patterns...{RESET}\n") + + rbac_files = find_rbac_files() + + if not rbac_files: + print(f"{YELLOW}No RBAC files (ClusterRole/Role) found in repository{RESET}") + return 0 + + print(f"Found {len(rbac_files)} RBAC file(s) to validate:\n") + for f in rbac_files: + print(f" - {f.relative_to(Path('.').resolve())}") + print() + + all_violations = [] + + for rbac_file in rbac_files: + rbac_resources = parse_rbac_file(rbac_file) + + for resource in rbac_resources: + kind = resource.get('kind', 'Unknown') + name = resource.get('metadata', {}).get('name', 'unnamed') + rules = resource.get('rules', []) + + for rule in rules: + violations = validate_rbac_rule( + rule=rule, + rbac_kind=kind, + rbac_name=name, + file_path=str(rbac_file.relative_to(Path('.').resolve())) + ) + all_violations.extend(violations) + + # Generate and print report + report = format_violation_report(all_violations) + print(report) + + # Count critical violations + critical_count = sum(1 for v in all_violations if v.severity == 'critical') + warning_count = sum(1 for v in all_violations if v.severity == 'warning') + + print(f"\n{BOLD}Summary:{RESET}") + print(f" Critical violations: {critical_count}") + print(f" Warnings: {warning_count}") + print() + + if critical_count > 0: + return 1 # Fail CI + else: + print(f"{GREEN}✅ RBAC validation passed{RESET}\n") + return 0 # Pass CI + + +if __name__ == '__main__': + sys.exit(check_rbac_wildcards()) diff --git a/.github/workflows/yamllint.yml b/.github/workflows/yamllint.yml index 4690832..776b9d8 100644 --- a/.github/workflows/yamllint.yml +++ b/.github/workflows/yamllint.yml @@ -26,3 +26,21 @@ jobs: - name: Run yaml-lint run: yamllint -c .yamllint.yml -s . + + rbac-security-check: + name: RBAC Wildcard Security Check + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: '3.13' + + - name: Install dependencies + run: pip install pyyaml + + - name: Check RBAC for dangerous wildcards + run: python3 .github/scripts/check-rbac-wildcards.py diff --git a/AGENTS.md b/AGENTS.md index cb2dc88..899014d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,11 +14,13 @@ This document describes how AI agents should work with this repository. The goal - **Purpose, directories (`/charts`, `/components`, `/example`)** → [docs/agents/repository.md](docs/agents/repository.md) - **YAML, Helm, Kustomize** → [docs/agents/yaml-helm-kustomize.md](docs/agents/yaml-helm-kustomize.md) - **What CI runs, required checks, when in doubt** → [docs/agents/ci-and-validation.md](docs/agents/ci-and-validation.md) +- **RBAC security requirements and automated validation** → [docs/agents/rbac-security.md](docs/agents/rbac-security.md) - **Releases, tags, and `?ref=`** → [docs/agents/releases.md](docs/agents/releases.md) - **Branches, PRs, commits, expectations for changes** → [CONTRIBUTING.md](CONTRIBUTING.md) **Skills (repeatable tasks)** - **Adding a new controlplane service component** → [docs/skills/add-controlplane-service.md](docs/skills/add-controlplane-service.md) +- **Validating and fixing RBAC security issues** → [docs/skills/rbac-security-validation.md](docs/skills/rbac-security-validation.md) **Domain context (operators and consumers):** the root [README.md](README.md) documents RHOSO deployment, ArgoCD conventions (e.g. sync-waves, pinned `?ref=`), and how applications are sliced—it does not define repository editing policy. Use the links above for how to change this repository. \ No newline at end of file diff --git a/docs/agents/ci-and-validation.md b/docs/agents/ci-and-validation.md index c8d9f58..49f735e 100644 --- a/docs/agents/ci-and-validation.md +++ b/docs/agents/ci-and-validation.md @@ -23,6 +23,9 @@ Before creating a commit or a pull request, agents must: - All tests pass. - All linters and formatters pass. - Generated content (if any) is up to date relative to the commands used in this repo (for example, `make generate` or equivalent, if present). +3. **For RBAC changes (ClusterRole/Role):** + - Run the RBAC security validation: `python3 .github/scripts/check-rbac-wildcards.py` + - See [RBAC security](rbac-security.md) for agent requirements and [RBAC security validation skill](../skills/rbac-security-validation.md) for fixing violations. If a required tool is not available in the current environment, clearly note the missing tool and do not assume the checks passed. diff --git a/docs/agents/rbac-security.md b/docs/agents/rbac-security.md new file mode 100644 index 0000000..9d0cd88 --- /dev/null +++ b/docs/agents/rbac-security.md @@ -0,0 +1,157 @@ +# RBAC Security + +This document provides AI agents with context on RBAC security requirements and automated validation in this repository. + +## Background + +A security audit identified a **HIGH severity vulnerability (CVSS 9.1)** where the ArgoCD ClusterRole granted wildcard permissions on cluster-wide Secrets, Pods, and entire API groups. This created a cluster-admin-equivalent escalation path for anyone who could compromise a git repository tracked by ArgoCD. + +To prevent regression, automated CI validation fails pull requests containing dangerous RBAC patterns. + +## Security Rationale + +### Why Wildcards Are Dangerous + +1. **Implicit Permissions**: In Kubernetes RBAC, `verbs: ['*']` doesn't just mean "all current verbs" - it includes future verbs added to the API. This has historically included dangerous permissions like: + - `impersonate` - Added in Kubernetes 1.5 + - `escalate` and `bind` - Added in Kubernetes 1.8 + +2. **Cluster-Admin Equivalent**: Wildcard verbs on `secrets` + `pods` in the core API is functionally cluster-admin: + - Read any Secret → steal ServiceAccount tokens + - Create Pods → mount any ServiceAccount → escalate privileges + - Chain these to become cluster-admin + +3. **GitOps Attack Surface**: ArgoCD auto-syncs from git repositories. If an attacker can push to a tracked repo, wildcard RBAC turns that into cluster compromise. + +### Real-World Example + +The vulnerability fixed in this repository (FIND-001) had this pattern: + +```yaml +apiGroups: [""] +resources: [secrets, pods] +verbs: ['*'] +``` + +**Attack path:** +1. Attacker pushes malicious Pod manifest to tracked git repo +2. ArgoCD auto-syncs and creates the Pod (has `create` pods permission) +3. Pod mounts privileged ServiceAccount (ArgoCD has `*` on secrets) +4. Pod now has cluster-wide secret read access +5. Attacker reads `kube-system` secrets → cluster-admin + +**Defense:** By replacing `'*'` with explicit `[get, list, watch]` on pods, step 2 fails - ArgoCD can no longer create Pods. + +## Agent Requirements + +When working with RBAC manifests (ClusterRole or Role), agents must: + +1. **Never introduce wildcard patterns** that will fail CI validation +2. **Run the validation script** before committing: + ```bash + python3 .github/scripts/check-rbac-wildcards.py + ``` +3. **Use explicit verbs and resources** instead of wildcards +4. **Understand the principle of least privilege**: Only grant the minimum permissions needed + +## Validation Rules + +The validation script (`.github/scripts/check-rbac-wildcards.py`) enforces these rules: + +### Critical Resources (Never Allow Wildcard Verbs) + +```python +CRITICAL_CORE_RESOURCES = { + 'secrets', + 'pods', + 'persistentvolumeclaims', + 'serviceaccounts', + 'nodes', + 'namespaces' +} +``` + +When these resources appear in the core API group (`apiGroups: [""]`), wildcard verbs (`verbs: ['*']`) are **forbidden**. + +### Wildcard Resources (Never Allow) + +Wildcard resources (`resources: ['*']`) are **forbidden** in all API groups, regardless of verbs. + +### OpenStack CRDs (Warning Only) + +```python +OPENSTACK_API_GROUPS = { + 'core.openstack.org', + 'dataplane.openstack.org', + 'operator.openstack.org', + 'baremetal.openstack.org' +} +``` + +Wildcard verbs on OpenStack CRDs generate warnings but do not fail CI. However, explicit verbs are still preferred. + +## Agent Workflow + +When modifying RBAC manifests: + +1. **Read existing permissions** to understand the current scope +2. **Identify the minimal verb set needed**: + - Read-only? → `[get, list, watch]` + - Create/update? → Add `[create, patch]` or `[create, update, patch]` + - Delete? → Add `[delete]` +3. **Identify the minimal resource set needed**: + - Scan the repository for what resources are actually used + - Check the operator's API documentation + - Use grep to find resource references: + ```bash + grep -r "apiVersion.*" . | grep "kind:" | sort -u + ``` +4. **Write explicit permissions** - never use wildcards +5. **Run validation locally**: + ```bash + python3 .github/scripts/check-rbac-wildcards.py + ``` +6. **Commit only if validation passes** + +## Maintenance Considerations + +### Updating Validation Rules + +The validation logic is in `.github/scripts/check-rbac-wildcards.py`. Key configuration sets are documented above. + +To add new critical resources or API groups: +1. Edit the relevant sets in the script +2. Test thoroughly against existing ClusterRoles +3. Update this documentation + +### Upgrading to Stricter Validation + +Currently, wildcard verbs on OpenStack CRDs are warnings. To make them critical (fail CI): + +1. Remove the OpenStack API groups from the warning exemption +2. Update this documentation and `docs/skills/rbac-security-validation.md` +3. **Test on existing ClusterRoles first** - this will likely break the build +4. Fix all violations before merging +5. Announce the change to contributors + +## Error Messages + +When CI fails with RBAC violations, agents should: + +1. Read the CI output to identify the problematic file, line, and permission +2. Consult the skill documentation at `docs/skills/rbac-security-validation.md` +3. Apply the appropriate fix (explicit verbs/resources) +4. Re-run validation locally before pushing + +Never suggest: +- Disabling the validation check +- Using wildcards "temporarily" +- Working around the validation + +## Further Reading + +- **Security Audit**: `gitops-findings/gitops/gitops-security-audit.md` - Full audit report with FIND-001 details +- **Triage Report**: `gitops-findings/gitops/gitops-triage.md` - Analysis of the high-severity finding +- **Kubernetes RBAC**: https://kubernetes.io/docs/reference/access-authn-authz/rbac/ +- **OWASP Kubernetes Top 10**: K02 - Overly Permissive RBAC +- **CIS Kubernetes Benchmark**: 5.1.3 - Minimize wildcard use in Roles and ClusterRoles diff --git a/docs/skills/rbac-security-validation.md b/docs/skills/rbac-security-validation.md new file mode 100644 index 0000000..5172ba1 --- /dev/null +++ b/docs/skills/rbac-security-validation.md @@ -0,0 +1,250 @@ +# RBAC Security Validation + +This skill helps contributors identify and fix dangerous RBAC wildcard patterns that violate the repository's security policy. + +## Purpose + +Validates RBAC manifests (ClusterRole and Role) to prevent dangerous wildcard permissions from being merged. This protects against privilege escalation vulnerabilities (CVSS 9.1 HIGH severity) where wildcard RBAC can enable cluster-admin equivalent access. + +## When to Use + +Use this skill when: + +- Adding or modifying ClusterRole or Role manifests +- CI fails with "RBAC security violations detected" +- Reviewing RBAC changes in pull requests +- Uncertain whether specific RBAC permissions are safe + +## Prerequisites + +- Python 3 with PyYAML installed (`pip install pyyaml`) +- Understanding of Kubernetes RBAC verbs and resources + +## Running the Validation + +### Local Validation + +Before committing RBAC changes, run: + +```bash +python3 .github/scripts/check-rbac-wildcards.py +``` + +**Expected output if passing:** +``` +✅ RBAC validation passed +``` + +**Expected output if failing:** +``` +❌ RBAC Security Validation FAILED +Critical Violations Found: ... +``` + +### CI Integration + +The validation runs automatically in `.github/workflows/yamllint.yml` as the `rbac-security-check` job. It executes on every pull request that modifies YAML files. + +## What Gets Validated + +### Critical Violations (Fail CI) + +These patterns are **never allowed**: + +#### 1. Wildcard verbs on core API resources + +**Blocked pattern:** +```yaml +- apiGroups: [""] + resources: + - secrets # ❌ CRITICAL + - pods # ❌ CRITICAL + - persistentvolumeclaims + - serviceaccounts + - nodes + - namespaces + verbs: + - '*' # ❌ Grants dangerous implicit permissions +``` + +**Why dangerous:** Wildcard verbs include `impersonate`, `escalate`, and `bind` - privilege escalation primitives that enable cluster-admin access. + +#### 2. Wildcard resources in any API group + +**Blocked pattern:** +```yaml +- apiGroups: + - metal3.io + - lvm.topolvm.io + - multicluster.openshift.io + - rbac.authorization.k8s.io # ❌ Especially dangerous + resources: + - '*' # ❌ CRITICAL - grants access to ALL resources + verbs: + - get + - list +``` + +**Why dangerous:** Prevents least-privilege enforcement and grants access to future resources that may be sensitive. + +### Warnings (Report Only) + +#### Wildcard verbs on OpenStack CRDs + +**Warned pattern:** +```yaml +- apiGroups: + - core.openstack.org + - dataplane.openstack.org + - operator.openstack.org + resources: + - openstackcontrolplanes + verbs: + - '*' # ⚠️ WARNING - consider making explicit +``` + +**Why a warning:** OpenStack CRDs don't provide cluster-admin escalation paths, but explicit verbs are still preferred. + +## How to Fix Violations + +### Fixing Wildcard Verbs on Core API + +**Before (❌ fails CI):** +```yaml +- apiGroups: + - "" + resources: + - secrets + - pods + verbs: + - '*' +``` + +**After (✅ passes CI):** +```yaml +# Secrets - if you need full CRUD +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch + - create + - patch + - delete + +# Pods - read-only if you only need status checking +- apiGroups: + - "" + resources: + - pods + verbs: + - get + - list + - watch +``` + +**Decision matrix for verbs:** +- Need to read? → `[get, list, watch]` +- Need to create/update? → Add `[create, patch]` or `[create, update, patch]` +- Need to delete? → Add `[delete]` +- **Never use:** `['*']`, `impersonate`, `escalate`, `bind` + +### Fixing Wildcard Resources + +**Before (❌ fails CI):** +```yaml +- apiGroups: + - metal3.io + resources: + - '*' # Grants access to ALL metal3.io resources + verbs: + - get + - create +``` + +**After (✅ passes CI):** +```yaml +- apiGroups: + - metal3.io + resources: + - baremetalhosts # Only what you actually use + - provisionings + verbs: + - get + - list + - watch + - create + - update + - patch + - delete +``` + +**How to find the right resources:** + +1. Look at what resources you actually create in your manifests +2. Check the API documentation for the operator you're using +3. Scan the repository: + ```bash + # Find all kinds referencing metal3.io + grep -r "apiVersion.*metal3.io" . | grep "kind:" | sort -u + ``` + +## Troubleshooting + +### "CI Failed: RBAC security violations detected" + +1. Read the CI output - it shows the exact file, line, and permission that's problematic +2. Follow the "How to Fix Violations" section above +3. Test locally with `python3 .github/scripts/check-rbac-wildcards.py` +4. Commit the fix and push + +### "False positive - I need this wildcard" + +If you believe the validator incorrectly flagged a legitimate use case: + +1. **Double-check:** Are you sure you need the wildcard? Can explicit permissions work? +2. **Document:** Add a comment in the YAML explaining why the wildcard is necessary +3. **Open an issue:** Request an allowlist exception with full justification +4. **Temporary override:** Requires maintainer approval and should be rare + +### Script Errors + +If the validation script crashes or produces unexpected output: + +1. Check that your YAML is valid: `yamllint -c .yamllint.yml ` +2. Ensure the file has `kind: ClusterRole` or `kind: Role` +3. Report the issue at https://github.com/openstack-k8s-operators/gitops/issues + +## Exceptions and Allowlisting + +Currently, there is no allowlist mechanism. If you have a legitimate use case for wildcard permissions: + +1. Open an issue explaining: + - What resource/verb needs the wildcard + - Why explicit permissions cannot work + - What security controls mitigate the risk + +2. The security team will review and may implement an allowlist file (`.github/rbac-allowlist.yaml`) if appropriate. + +## Reference Example + +See the [security audit finding FIND-001](../../gitops-findings/gitops/gitops-security-audit.md) for the original vulnerability that prompted this validation. + +## Related Documentation + +- [CI and validation](../agents/ci-and-validation.md) +- [RBAC security context for agents](../agents/rbac-security.md) +- [Kubernetes RBAC documentation](https://kubernetes.io/docs/reference/access-authn-authz/rbac/) +- [OWASP Kubernetes Top 10](https://owasp.org/www-project-kubernetes-top-ten/) - K02: Overly Permissive RBAC +- [CIS Kubernetes Benchmark](https://www.cisecurity.org/benchmark/kubernetes) - 5.1.3: Minimize wildcard use in Roles and ClusterRoles + +## Notes for AI Agents + +- Always run the validation script before committing RBAC changes +- Prefer explicit verbs and resources over wildcards +- Do not assume wildcards are acceptable without checking the validation rules +- If the validator fails, fix the violations before opening a pull request +- Follow Conventional Commits with `AI-Tool` / `AI-Model` footers when fixing RBAC issues diff --git a/openshift-gitops.deploy/enable/clusterrole.yaml b/openshift-gitops.deploy/enable/clusterrole.yaml index db9b44f..b42e7a2 100644 --- a/openshift-gitops.deploy/enable/clusterrole.yaml +++ b/openshift-gitops.deploy/enable/clusterrole.yaml @@ -41,10 +41,29 @@ rules: - "" resources: - secrets + verbs: + - get + - list + - watch + - create + - patch + - delete + - apiGroups: + - "" + resources: - persistentvolumeclaims + verbs: + - get + - list + - delete + - apiGroups: + - "" + resources: - pods verbs: - - '*' + - get + - list + - watch - apiGroups: - project.openshift.io resources: @@ -54,15 +73,30 @@ rules: - apiGroups: - metal3.io resources: - - '*' + - baremetalhosts + - provisionings verbs: - - '*' + - get + - list + - watch + - create + - update + - patch + - delete - apiGroups: - lvm.topolvm.io resources: - - '*' + - lvmclusters + - lvmvolumegroups + - lvmvolumegroupnodestatuses verbs: - - '*' + - get + - list + - watch + - create + - update + - patch + - delete - apiGroups: - cluster.open-cluster-management.io resources: @@ -70,18 +104,33 @@ rules: - 'managedclustersetbindings' verbs: - '*' - - apiGroups: - - multicluster.openshift.io - resources: - - '*' - verbs: - - '*' + # multicluster.openshift.io - Currently not used in manifests, but kept for future multicluster features + # TODO: Remove if not needed, or add specific resources when multicluster is enabled + # - apiGroups: + # - multicluster.openshift.io + # resources: + # - multiclusterengines + # - multiclusterhubs + # verbs: + # - get + # - list + # - watch + # - create + # - update + # - patch + # - delete - apiGroups: - baremetal.openstack.org resources: - - '*' + - openstackbaremetalsets verbs: - - '*' + - get + - list + - watch + - create + - update + - patch + - delete - apiGroups: - secrets.hashicorp.com resources: