Merge pull request #6 from Altinity/add-clickhouse-security-skill

Add altinity-expert-clickhouse-security skill

Author: BorisTyshkevich

altinity-expert-clickhouse/README.md

@@ -4,7 +4,7 @@ This package contains the modular `altinity-expert-clickhouse-*` ClickHouse diag
 
 ## Layout
 
-- `skills/` — all `altinity-expert-clickhouse-*` skill definitions (each has its own `SKILL.md`). The `...-overview` skill is the router and suggests which specialist skill(s) to run next.
+- `skills/` — all `altinity-expert-clickhouse-*` skill definitions (each has its own `SKILL.md`). The `...-overview` skill is the router and suggests which specialist skill(s) to run next. The `...-security` skill is a standalone, read-only security audit (it uses a `references/` subdirectory loaded on demand) and is not part of the performance-triage chain below.
 - `tests/` — test suite and scenarios. Do not modify structure without updating prompts/runner.
 - `../releases/` — built zip packages for distribution.
 
@@ -40,6 +40,20 @@ Report requirements:
 ```
 
 
+## Security Audit (standalone)
+
+The `altinity-expert-clickhouse-security` skill performs a professional, read-only
+security audit (users, roles, grants, definer views, row policies, external sources,
+audit logs, secrets, Keeper/interserver, encryption/backups, HTTP surface, cluster,
+and version-specific behavior). Example prompt:
+
+```
+Perform a read-only ClickHouse security audit using the altinity-expert-clickhouse-security skill.
+Produce findings with severity, confidence, evidence (redacted), and SQL-only limitations.
+```
+
+Run its test with `make test-security` (see below).
+
 ## Run Tests
 
 From `altinity-expert-clickhouse/tests`:

altinity-expert-clickhouse/skills/altinity-expert-clickhouse-security/SKILL.md

@@ -0,0 +1,74 @@
+---
+name: altinity-expert-clickhouse-security
+description: Read-only ClickHouse security audit expert for live or exported systems. Use when assessing ClickHouse security posture, reviewing users, roles, grants, settings profiles, row policies, table functions, external sources, table engines, executable UDFs, audit logs, named collections, password hash hygiene, SQL SECURITY DEFINER, impersonation, TLS/network exposure, Keeper/interserver security, encryption at rest, backups, the HTTP interface surface, cluster security, or version-specific ClickHouse security behavior. Diagnoses from SQL/system tables, supplied configuration files, query logs, access metadata, and ClickHouse/Altinity documentation.
+license: Apache-2.0
+---
+
+# Altinity Expert ClickHouse Security
+
+Use this skill to perform a professional, read-only ClickHouse security audit. Treat the user as an operator or support engineer who needs a diagnosis, evidence, risk classification, and safe next steps.
+
+## Core behavior
+
+- Work as a senior ClickHouse security reviewer.
+- Prefer read-only SQL and metadata inspection.
+- Never run destructive SQL.
+- Never perform online password guessing.
+- Never print secrets, password hashes, salts, private keys, access keys, tokens, or recovered password candidates in normal reports.
+- Correlate findings. Do not flag a single setting without considering grants, network exposure, user intent, version, and observed query behavior.
+- State what could not be verified from SQL-only access.
+- Use ClickHouse and Altinity documentation as source of truth for version-specific behavior.
+- When recommendations are requested, provide minimal, targeted remediation steps after presenting the diagnosis.
+
+## Standard workflow
+
+1. Establish scope:
+   - live SQL access, exported `system.*` data, supplied `users.xml` / `config.xml`, or query-log extracts.
+   - single node or cluster.
+   - ClickHouse version and deployment model.
+2. Collect safe evidence:
+   - `SELECT version()`
+   - `SHOW ACCESS`
+   - selected `system.*` tables
+   - supplied configuration snippets
+   - recent `system.query_log` and `system.session_log`, if available.
+3. Load only the relevant reference files below.
+4. Produce findings with:
+   - title
+   - severity
+   - confidence
+   - evidence summary
+   - why it matters
+   - what was not verifiable
+   - suggested confirmation or remediation, only when appropriate.
+
+## Reference index
+
+- `references/01-scope-and-safety.md`: safe execution rules, evidence handling, redaction.
+- `references/02-identity-authentication.md`: users, auth methods, default user, expiration, multiple auth methods.
+- `references/03-users-roles-grants-rbac.md`: RBAC, grants, admin-equivalent permissions.
+- `references/04-definer-impersonation-principals.md`: `SQL SECURITY DEFINER`, `EXECUTE AS`, no-password principals.
+- `references/05-network-exposure-and-tls.md`: host restrictions, ports, TLS, exposure.
+- `references/06-table-functions-external-sources.md`: table functions, `SOURCES` grants, exfiltration paths.
+- `references/07-table-engines-and-external-integrations.md`: external table engines, persistent integrations.
+- `references/08-row-column-policy-security.md`: row policies, column grants, bypass patterns.
+- `references/09-settings-profiles-constraints-quotas.md`: settings profiles, constraints, quotas, readonly.
+- `references/10-audit-logging-and-log-integrity.md`: query/session logs, `log_query_settings`, observability gaps.
+- `references/11-query-log-threat-hunting.md`: suspicious behavior from logs.
+- `references/12-secrets-named-collections-credentials.md`: named collections, secret handling, hardcoded credentials.
+- `references/13-password-hash-hygiene.md`: hash inventory, duplicate hashes, weak hash matching, safe reporting.
+- `references/14-version-specific-security-checks.md`: version-aware checks.
+- `references/15-cluster-distributed-security.md`: cluster-wide consistency and distributed security.
+- `references/16-keeper-and-interserver-security.md`: ZooKeeper/Keeper exposure and ACLs, interserver authentication.
+- `references/17-executable-udf-and-code-execution.md`: executable UDFs and server-side command execution capability.
+- `references/18-encryption-at-rest-and-backups.md`: encrypted disks, storage credentials, `BACKUP`/`RESTORE` destinations.
+- `references/19-http-interface-surface.md`: HTTP handlers, Play UI, CORS, default-credential access over HTTP.
+- `references/20-reporting-severity-and-output-format.md`: severity rubric and final report structure.
+
+## SQL style
+
+Use fenced `sql` blocks for SQL. Keep SQL compatible with ClickHouse 24.8+ unless a version-specific note says otherwise. Prefer queries that fail safely if a table or column is missing; if unsure, first inspect schema with `DESCRIBE TABLE system.<table>`.
+
+## Redaction
+
+Never print secrets, hashes, salts, keys, tokens, or connection strings. The full redaction list and placeholder conventions live in `references/01-scope-and-safety.md`; apply them to all evidence.

altinity-expert-clickhouse/skills/altinity-expert-clickhouse-security/references/01-scope-and-safety.md

@@ -0,0 +1,76 @@
+# Scope and safety
+
+Use this section first for any live-system audit.
+
+## Non-negotiable rules
+
+- Use read-only SQL only.
+- Do not run `CREATE`, `ALTER`, `DROP`, `TRUNCATE`, `INSERT`, `OPTIMIZE`, `SYSTEM`, `KILL`, `BACKUP`, `RESTORE`, or `SET` unless the user explicitly asks for a remediation command and understands it is not an audit probe.
+- Do not run network probes through table functions.
+- Do not use table functions to test whether exfiltration is possible.
+- Do not attempt online password guessing or authentication attempts.
+- Do not print secrets, password hashes, salts, or credential candidates.
+
+## Initial evidence checklist
+
+Collect when available:
+
+```sql
+SELECT version();
+```
+
+```sql
+SHOW ACCESS;
+```
+
+```sql
+SELECT name, value, changed
+FROM system.settings
+WHERE name IN
+(
+    'log_queries',
+    'log_query_settings',
+    'readonly',
+    'allow_ddl'
+)
+ORDER BY name;
+```
+
+If cluster-wide checks are needed, prefer existing cluster topology supplied by the user. Do not guess cluster names. Ask for the cluster name, or inspect only local node data.
+
+## Evidence confidence
+
+Use these confidence labels:
+
+- `high`: direct metadata evidence from SQL/config/logs.
+- `medium`: strong pattern, but inherited roles/profiles or cluster coverage may be incomplete.
+- `low`: inferred from partial DDL text, samples, or missing logs.
+- `unknown`: cannot be verified with available access.
+
+## Redaction (canonical)
+
+This is the single source of truth for redaction; other sections refer here. When showing evidence, redact:
+
+- password hashes and salts
+- secrets in URLs and named collections
+- cloud access keys and secret keys
+- SAS tokens and signed URLs
+- private keys
+- bearer tokens and JWTs
+- connection strings (including JDBC/ODBC) containing credentials
+- internal hostnames, if the user asks for sanitized output
+
+Use placeholders such as `<redacted_hash>`, `<redacted_secret>`, `<redacted_key>`, and `<internal_host>`.
+
+## Reporting limitations
+
+Always state SQL-only gaps, such as:
+
+- cannot verify `listen_host`, TLS certificate configuration, or named collection storage without config access.
+- cannot confirm full cluster posture from one node.
+- cannot validate password strength from salted/bcrypt hashes except through safe offline denylist matching.
+- cannot know whether broad grants are intentional without ownership context.
+
+## Auditor privilege affects what is observed
+
+State the identity and privilege level the audit ran as. A highly-privileged auditor sees a complete `SHOW ACCESS` and full system tables, but settings read from the live session reflect that session, not other users' enforced policy (see `09-settings-profiles-constraints-quotas.md`). A low-privilege auditor may see only a partial picture and should say so. Note the auditing identity in the report so session-derived readings are interpreted correctly.

altinity-expert-clickhouse/skills/altinity-expert-clickhouse-security/references/02-identity-authentication.md

@@ -0,0 +1,66 @@
+# Identity and authentication checks
+
+Use this section to evaluate ClickHouse user authentication posture. Do not treat authentication method alone as risk; correlate with host restrictions, grants, and intended role.
+
+## Check: authentication method inventory
+
+```sql
+SELECT
+    name,
+    auth_type,
+    valid_until,
+    host_ip,
+    host_names,
+    host_names_regexp,
+    host_names_like,
+    default_roles_all,
+    default_roles_list,
+    default_roles_except
+FROM system.users
+ORDER BY name;
+```
+
+Classify:
+
+- `plaintext_password`: high concern because plaintext storage is avoidable.
+- `sha256_password` / `sha256_hash`: acceptable but weaker than bcrypt if hashes are compromised.
+- `double_sha1_password` / `double_sha1_hash`: usually compatibility-driven; ask why it is needed.
+- `bcrypt_password` / `bcrypt_hash`: preferred local password storage.
+- `ldap`, `kerberos`, `ssl_certificate`, `ssh_key`, `jwt`, `http`: evaluate the external mechanism and local fallback methods.
+- `no_password` / `no_authentication`: neutral by itself. See `04-definer-impersonation-principals.md`.
+
+## Check: default user posture
+
+Focus on `default` because it is commonly overused.
+
+Risk signals:
+
+- broad host access.
+- admin-equivalent grants.
+- local password auth with weak method.
+- used by applications and administrators simultaneously.
+- used for inter-node communication without clear isolation.
+
+Do not recommend deleting or disabling `default` blindly. First confirm internode usage, automation, and cluster configuration.
+
+## Check: credential expiration
+
+Use `valid_until` from `system.users`.
+
+Risk signals:
+
+- human users with no expiration policy.
+- expired credentials still configured.
+- multiple auth methods where a weak fallback lacks expiration.
+
+## Check: multiple authentication methods
+
+A user with multiple auth methods can authenticate through any listed method. One weak fallback can undermine a strong primary method.
+
+Risk examples:
+
+- `bcrypt_password` plus `plaintext_password`.
+- external auth plus local password fallback with broad hosts.
+- certificate/SSH plus no-password fallback.
+
+Rate an auth method only together with host restriction and grants: `sha256_password` from a narrow IP with read-only grants is low; `plaintext_password` from broad hosts with grant option is high.

altinity-expert-clickhouse/skills/altinity-expert-clickhouse-security/references/03-users-roles-grants-rbac.md

@@ -0,0 +1,70 @@
+# Users, roles, grants, and RBAC checks
+
+Use this section when reviewing `SHOW ACCESS`, `system.grants`, users, and roles.
+
+## Check: collect grants
+
+Preferred:
+
+```sql
+SHOW ACCESS;
+```
+
+If system grants are available, also inspect relevant rows:
+
+```sql
+SELECT *
+FROM system.grants
+ORDER BY user_name, role_name, database, table, access_type;
+```
+
+Column names may vary by version; inspect with `DESCRIBE TABLE system.grants` if needed.
+
+## Check: admin-equivalent permissions
+
+Flag users or roles with combinations of:
+
+- `GRANT ALL ON *.*`
+- `WITH GRANT OPTION`
+- `ACCESS MANAGEMENT`
+- `CREATE USER`, `ALTER USER`, `DROP USER`
+- broad `CREATE`, `ALTER`, `DROP`, `TRUNCATE`
+- broad `SYSTEM`
+- broad `SOURCES`: `FILE`, `URL`, `S3`, `REMOTE`, `MYSQL`, `POSTGRES`, `JDBC`, `ODBC`, `AZURE`
+- `INTROSPECTION`
+- `ALLOW SQL SECURITY NONE`
+- `IMPERSONATE ON *`
+
+## Counting and attribution rules
+
+Apply these before reporting grant findings, or counts will be inflated and admins will be flagged for being admins:
+
+- **Do not double-count `ALL`-implied privileges.** `GRANT ALL ON *.*` already implies `SYSTEM`, `INTROSPECTION`, `SOURCES`/`S3`/`URL`/`FILE`/`REMOTE`, `displaySecretsInShowAndSelect`, `NAMED COLLECTION ADMIN`, and the DDL/access-management privileges. When you report "N principals have SYSTEM/SOURCES/INTROSPECTION", count only those who hold it **without** `ALL`; otherwise you are re-listing the same admins under every derived privilege.
+- **Deduplicate identities.** One human often has several principals — e.g. a local user `name` and an SSO/OAuth identity `name@example.com`, or a personal user plus a role. Note the likely link and report once, rather than as separate findings.
+- **Distinguish expected admins and service accounts.** `ALL` on `clickhouse_operator` (the ClickHouse/Altinity Kubernetes operator), a break-glass admin, or a named service role is usually intentional. The finding is the *unexpected* `ALL`-holder, or an admin set that is larger than the operating model justifies — not the existence of admins. Ask for ownership context when intent is unclear.
+- **Gate severity on reachability.** Pair every powerful grant with the principal's `host_ip`/`host_names` and auth method (see `05-network-exposure-and-tls.md` and `02-identity-authentication.md`). The same `ALL` + grant option is Critical on `::/0` with plaintext auth and far lower when locked to localhost/operator with strong auth.
+- **Merge same-principal findings.** If one account combines weak auth, `ALL`, and broad grant option, report it as one correlated finding with combined blast radius, not three.
+- **Authority comes from grants, not settings.** Read/write capability is decided by RBAC (`SELECT` vs `INSERT`/`ALTER`/`CREATE`/`DROP`), not by `readonly`/`allow_ddl`. Do **not** credit a `readonly` profile as a mitigation that lowers a grant finding — it is a mutable, per-session settings overlay, and "grant broad, then clamp with `readonly`" is a design smell, not a control. The finding is the over-broad grant; right-size it. Treat `readonly=2` only as effective-behavior context (a grant may be currently inert) and as a risk *amplifier* (`SET` + `CREATE TEMPORARY TABLE` + `SOURCES` → table functions; see `09`).
+
+## Check: privilege inheritance
+
+Look for:
+
+- users with all roles enabled by default.
+- powerful roles granted to low-privilege users.
+- role chains that make a user admin-equivalent.
+- grants assigned to roles that look like application or BI roles.
+
+## Check: grant option blast radius
+
+`WITH GRANT OPTION` is high risk when combined with broad privileges. The user can delegate permissions beyond the original operational intent.
+
+Severity escalation:
+
+- broad privileges + grant option: critical/high.
+- object-level select + grant option: medium, depends on data sensitivity.
+- admin role with grantees_any: high.
+
+## Stating impact
+
+Never write "too many grants". State the reachable outcome: create users, delegate privileges, read sensitive system metadata, create external data paths, drop/mutate production objects, or bypass row-policy designs.

altinity-expert-clickhouse/skills/altinity-expert-clickhouse-security/references/04-definer-impersonation-principals.md

@@ -0,0 +1,78 @@
+# Definer, impersonation, and security principal checks
+
+Use this section for `SQL SECURITY DEFINER`, `EXECUTE AS`, `IMPERSONATE`, and no-password principal analysis.
+
+## Key model
+
+`no_password` and `no_authentication` are not automatically findings. They are often deliberate non-login principals used as security contexts for views, materialized views, JWT shadow definers, or impersonation workflows.
+
+## Check: no-password principals
+
+```sql
+SELECT
+    name,
+    auth_type,
+    host_ip,
+    host_names,
+    host_names_regexp,
+    host_names_like,
+    default_roles_all,
+    default_roles_list,
+    default_roles_except
+FROM system.users
+WHERE has(auth_type, 'no_password')
+   OR has(auth_type, 'no_authentication')
+ORDER BY name;
+```
+
+Classify:
+
+- expected: no-password user has narrow/no login path, narrow grants, and is referenced as definer or controlled impersonation target.
+- suspicious: no-password user has broad hosts, admin grants, grant option, source grants, or no visible purpose.
+
+## Check: views using definer security
+
+```sql
+SELECT
+    database,
+    name,
+    engine,
+    create_table_query
+FROM system.tables
+WHERE positionCaseInsensitive(create_table_query, 'SQL SECURITY DEFINER') > 0
+   OR positionCaseInsensitive(create_table_query, 'DEFINER') > 0
+ORDER BY database, name;
+```
+
+Risk signals:
+
+- definer has more grants than needed by the view query.
+- definer is an admin or default user.
+- view exposes sensitive columns or unfiltered rows.
+- caller-facing role has broad `SELECT` on many definer views.
+- materialized view definer has unexpected `INSERT` privileges.
+
+## Check: SQL SECURITY NONE
+
+`SQL SECURITY NONE` is deprecated and dangerous. Creating such views requires `ALLOW SQL SECURITY NONE`; flag principals with this privilege.
+
+Search in `SHOW ACCESS` for:
+
+```text
+ALLOW SQL SECURITY NONE
+```
+
+## Check: impersonation
+
+Inspect `SHOW ACCESS` for:
+
+```text
+IMPERSONATE
+```
+
+Risk signals:
+
+- `IMPERSONATE ON *`.
+- impersonation granted to app users or BI users.
+- impersonation target has admin-equivalent grants.
+- `access_control_improvements.allow_impersonate_user` enabled without clear controls.