Skip to content

Commit 937e4ee

Browse files
piotrpdevpiotrpdev
committed
feat: proposal for minimum file permissions
Signed-off-by: Piotr Płaczek <piotrpdev@gmail.com>
1 parent d6220fe commit 937e4ee

1 file changed

Lines changed: 173 additions & 0 deletions

File tree

‎proposals/121-enforce-minimum-file-permissions.md‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
# 121 - Enforce minimum access rights on confidential files
2+
3+
Kroxylicious should validate that confidential files (TLS private keys, keystores, truststores,
4+
password files, and KMS credential files) have suitably restrictive filesystem permissions before
5+
reading them, similar to how the `ssh` command refuses to use a world-readable private key.
6+
7+
## Current situation
8+
9+
Kroxylicious reads confidential material from the filesystem - TLS private keys, keystores,
10+
truststores, and passwords - without checking whether those files are accessible by users other
11+
than the owner. A world-readable private key file (`0644`) is silently accepted and used. This
12+
violates the principle of least privilege and increases the risk of credential exposure through
13+
over-permissive filesystem configurations.
14+
15+
## Motivation
16+
17+
Security best practices require that private key material is accessible only to the process that
18+
owns it. Tools such as `ssh`, `gpg`, and many TLS libraries enforce this by refusing to operate
19+
on files with group or other read bits set. Kroxylicious should provide equivalent protection.
20+
21+
The threat model includes:
22+
- Accidental over-permissive file creation (e.g. default `umask` producing `0644`).
23+
- Multi-tenant environments where other users on the same host could read Kroxylicious credentials.
24+
- Kubernetes deployments where secret volumes default to world-readable `0644` unless explicitly
25+
configured otherwise.
26+
27+
## Proposal
28+
29+
### File permission policy
30+
31+
Introduce a configurable `security.filePermissions.policy` setting with three modes:
32+
33+
- `STRICT` - files must be owner-only (equivalent to `chmod 400` or `chmod 600`). Any group or
34+
other read/write/execute bits cause an `IllegalStateException` at startup. Mirrors SSH behaviour.
35+
- `RELAXED` - other-user bits are forbidden, but group bits are permitted. This supports
36+
Kubernetes deployments where `fsGroup` is used to grant a specific GID read access to mounted
37+
secrets (e.g. `defaultMode: 0440`).
38+
- `DISABLED` - no enforcement. A warning is logged for files that would be rejected by other
39+
policies, but startup is never rejected. This is the default for backward compatibility; see
40+
the [Delivery](#delivery) section for how the default will eventually change to `STRICT`.
41+
42+
### Scope of enforcement
43+
44+
The policy applies before reading any of the following:
45+
46+
- TLS private key files (`key.privateKeyFile`)
47+
- TLS keystore and truststore files (`key.storeFile`, `trust.storeFile`)
48+
- Password files referenced by `FilePassword` providers (including KMS credentials)
49+
- AWS IRSA web identity token files
50+
- AWS EKS Pod Identity authorization token files
51+
52+
### Configuration
53+
54+
```yaml
55+
---
56+
management:
57+
# ...
58+
virtualClusters:
59+
- name: "one"
60+
targetCluster:
61+
# ...
62+
gateways:
63+
# ...
64+
security:
65+
filePermissions:
66+
policy: "STRICT"
67+
```
68+
69+
### Global policy propagation
70+
71+
`FilePermissionValidator` holds a static `AtomicReference<Policy>` that is set once when
72+
`Configuration` is constructed. This allows `FilePassword` and KMS credential providers - which
73+
live in modules that do not depend on `kroxylicious-runtime` - to enforce the configured policy
74+
without requiring changes to their public interfaces.
75+
76+
### Kubernetes operator
77+
78+
The operator mounts all secret volumes with `defaultMode: 0440` (group-readable, no world
79+
access).
80+
81+
On plain Kubernetes, it sets `fsGroup` and `runAsGroup` to the
82+
[Kroxylicious Dockerfile](kroxylicious-app/src/main/docker/proxy.dockerfile) GID (185)
83+
so the kubelet chowns volume files to that GID and the container process can read them via group
84+
membership.
85+
86+
On OpenShift,
87+
[`fsGroup` and `runAsGroup` are omitted](https://www.redhat.com/en/blog/a-guide-to-openshift-and-uids);
88+
[the `restricted-v2` SCC's `MustRunAs` strategy injects the namespace-allocated GID automatically](https://docs.redhat.com/en/documentation/openshift_container_platform/4.22/html/authentication_and_authorization/managing-pod-security-policies#security-context-constraints-example_configuring-internal-oauth).
89+
90+
The `KafkaProxy` CRD exposes `spec.security.filePermissions.policy` (default `RELAXED`) to allow
91+
users to override the policy.
92+
93+
## Affected/not affected projects
94+
95+
**Affected:**
96+
- `kroxylicious-security` - new module; contains `FilePermissionValidator` and `FilePermissionConfig`
97+
- `kroxylicious-api` - `FilePassword.getProvidedPassword()` now validates permissions
98+
- `kroxylicious-runtime` - `Configuration`, `NettyKeyProvider`, `NettyTrustProvider`, `VirtualClusterModel`, `ServerConnectionStateMachine`
99+
- `kroxylicious-kms-providers` / `kroxylicious-kms-provider-aws-kms` - IRSA and Pod Identity providers validate their token files
100+
- `kroxylicious-kubernetes` / `kroxylicious-operator` - secret volume `defaultMode`, conditional `fsGroup`, `KafkaProxy` CRD field
101+
102+
**Not affected:**
103+
- `kroxylicious-filters` - no file reading
104+
- `kroxylicious-authorizer-api`, `kroxylicious-authorizer-providers` - no file reading
105+
- KMS providers other than AWS (vault token, Azure, Fortanix) - covered transitively via `FilePassword`
106+
107+
## Delivery
108+
109+
In order to comply with the project's [deprecation policy](https://github.com/kroxylicious/kroxylicious/blob/main/DEV_GUIDE.md#deprecation-policy),
110+
the change in default policy should be staged across two releases.
111+
112+
**Stage 1 (this proposal):** Introduce the feature with `DISABLED` as the default for backward
113+
compatibility. When `DISABLED` is the effective policy, Kroxylicious logs a `WARN` for every
114+
confidential file whose permissions would be rejected by `STRICT`. This gives users a
115+
deprecation period to identify and harden their file permissions. The deprecation of `DISABLED`
116+
as the default should be announced in the `CHANGELOG` under "Changes, deprecations and removals".
117+
118+
**Stage 2 (subsequent release, following the deprecation policy):** Change the default to `STRICT`.
119+
This will be a breaking change for deployments that have confidential files with overly permissive
120+
permissions and have not explicitly configured a policy. Deployments that set
121+
`security.filePermissions.policy: DISABLED` explicitly will be unaffected.
122+
The change in default should be documented in the `CHANGELOG` as a breaking change.
123+
124+
## Compatibility
125+
126+
### Backward compatibility
127+
128+
The default policy in Stage 1 is `DISABLED`, so existing deployments are unaffected. Warnings are
129+
emitted for insecure files to assist users in identifying files to harden before the default
130+
changes to `STRICT` in Stage 2.
131+
132+
### `FilePassword.getProvidedPassword()` behaviour change
133+
134+
With a non-`DISABLED` policy, `FilePassword.getProvidedPassword()` can now throw
135+
`IllegalStateException` if the password file has group or other read bits set. This is an
136+
unchecked exception that did not previously occur. Filter authors using `FilePassword` directly
137+
should be aware of this.
138+
139+
### API additions
140+
141+
`FilePermissionValidator` and `FilePermissionConfig` in the new `kroxylicious-security` module
142+
become accessible to consumers of `kroxylicious-api` (which depends on `kroxylicious-security`).
143+
`FilePermissionValidator.setGlobalPolicy()` is necessarily public because `Configuration` (in
144+
`kroxylicious-runtime`) and `FilePermissionValidator` (in `kroxylicious-security`) are in
145+
different modules and Java's access control cannot express "accessible to exactly one other module"
146+
without JPMS. This is a known design limitation: third-party code could in principle call
147+
`setGlobalPolicy()` and alter the global policy for all validations. A future improvement could
148+
adopt JPMS module encapsulation to restrict the method to the `kroxylicious.runtime` module only.
149+
150+
## Rejected alternatives
151+
152+
### Move `FilePermissionValidator` to `kroxylicious-runtime`
153+
154+
`FilePassword` (in `kroxylicious-api`) needs to call the validator to enforce permissions before
155+
reading a password file. `kroxylicious-runtime` already depends on `kroxylicious-api`, so
156+
`kroxylicious-api` depending back on `kroxylicious-runtime` would create a circular dependency.
157+
The validator therefore cannot live in `kroxylicious-runtime` if `FilePassword` is to use it
158+
directly.
159+
160+
### Move `FilePermissionValidator` to `kroxylicious-api`
161+
162+
Moving the validator directly to `kroxylicious-api` (rather than creating `kroxylicious-security`)
163+
was considered. Rejected because `kroxylicious-api` is conceptually a contract/interface layer;
164+
adding a logging-heavy implementation utility (with SLF4J, `AtomicBoolean`, `ConcurrentHashMap`)
165+
would pollute the API module with infrastructure concerns. A dedicated `kroxylicious-security`
166+
module is the right architectural home.
167+
168+
### System property override
169+
170+
A JVM system property (`-Dkroxylicious.security.filePermissionPolicy=STRICT`) was considered as
171+
a secondary configuration mechanism alongside the YAML field. Rejected because system properties
172+
are undiscoverable, untestable, and bypass the normal configuration validation path. The YAML
173+
field is the right single mechanism.

0 commit comments

Comments
 (0)