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

Allow access to sealed secret services/proxy to any authenticated user #208

Merged
merged 1 commit into from
Jul 31, 2019

Conversation

mkmik
Copy link
Collaborator

@mkmik mkmik commented Jul 31, 2019

This allows kubeseal to fetch the certificate public key (and perform other actions such as /verify and /rotate endpoints) even if the caller doesn't have otherwise the rights to access the kube-system namespace (or any other namespace where the sealed-secrets controller might have been deployed), as it often happens that users are not granted such broad permissions on production clusters.

We historically suggested users to just distribute the certificate out of bound and use the --cert flag.
However, with the advent of master key rotation, this is becoming increasingly more cumbersome, especially since
it's critical that users end up using the right certificate (i.e. the certificate has to be authenticated).
Master key rotation also requires users to periodically rotate the secrets, which requires access to the /rotate endpoint.

This change includes a fine-grained RBAC rule that allows access to the sealed-secrets controller HTTP API to any authenticated user in the cluster.
Users are still free to disable this feature by applying an override during deployment, but our default RBAC config should include it.

The controller currently exposes the following endpoints:

  • `/healthz'
  • /v1/verify
  • /v1/rotate
  • /v1/cert.pem

The controller already must not expose any secrets via the HTTP endpoint, since while RBAC would prevent
end-users to access the service via the proxy, nothing prevents any unprivileged workload in the cluster unless
admins have explicitly configured a strict network policy rule set.

Closes #166
Rel #137

@mkmik mkmik requested review from atomatt and jjo July 31, 2019 09:09
@mkmik mkmik added this to the v0.8.2 milestone Jul 31, 2019
Copy link

@atomatt atomatt left a comment

Choose a reason for hiding this comment

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

lgtm

Is it worth also adding a large, obvious notice where the http mux is configured in httpserver warning that it must never expose anything secret because of the default rbac?

],
resourceNames: [
'http:sealed-secrets-controller:', // kubeseal uses net.JoinSchemeNamePort when crafting proxy subresource URLs
'sealed-secrets-controller', // but often services are referred by name only, let's not make it unnecessary cryptic
Copy link

Choose a reason for hiding this comment

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

s/unnecessary/unnecessarily

Copy link

@jjo jjo left a comment

Choose a reason for hiding this comment

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

LGTM

@mkmik
Copy link
Collaborator Author

mkmik commented Jul 31, 2019

@atomatt as I mentioned in the PR description, even before this RBAC change leaking secrets was a terrible idea. That said, it doesn't hurt to add such a warning in the code

This allows kubeseal to fetch the certificate public key (and perform other actions such as /verify and /rotate endpoints) even if the caller doesn't have otherwise the rights to access the kube-system namespace (or any other namespace where the sealed-secrets controller might have been deployed), as it often happens that users are not granted such broad permissions on production clusters.

We historically suggested users to just distribute the certificate out of bound and use the `--cert` flag.
However, with the advent of master key rotation, this is becoming increasingly more cumbersome, especially since
it's critical that users end up using the right certificate (i.e. the certificate has to be authenticated).
Master key rotation also requires users to periodically rotate the secrets, which requires access to the /rotate endpoint.

This change includes a fine-grained RBAC rule that allows access to the sealed-secrets controller HTTP API to any authenticated user in the cluster.
Users are still free to disable this feature by applying an override during deployment, but our default RBAC config should include it.

The controller currently exposes the following endpoints:

- `/healthz'
- `/v1/verify`
- `/v1/rotate`
- `/v1/cert.pem`

The controller already must not expose any secrets via the HTTP endpoint, since while RBAC would prevent
end-users to access the service via the proxy, nothing prevents any unprivileged workload in the cluster unless
admins have explicitly configured a strict network policy rule set.

Closes #166
@mkmik
Copy link
Collaborator Author

mkmik commented Jul 31, 2019

bors r+

bors bot added a commit that referenced this pull request Jul 31, 2019
208: Allow access to sealed secret services/proxy to any authenticated user r=mkmik a=mkmik

This allows kubeseal to fetch the certificate public key (and perform other actions such as /verify and /rotate endpoints) even if the caller doesn't have otherwise the rights to access the kube-system namespace (or any other namespace where the sealed-secrets controller might have been deployed), as it often happens that users are not granted such broad permissions on production clusters.

We historically suggested users to just distribute the certificate out of bound and use the `--cert` flag.
However, with the advent of master key rotation, this is becoming increasingly more cumbersome, especially since
it's critical that users end up using the right certificate (i.e. the certificate has to be authenticated).
Master key rotation also requires users to periodically rotate the secrets, which requires access to the /rotate endpoint.

This change includes a fine-grained RBAC rule that allows access to the sealed-secrets controller HTTP API to any authenticated user in the cluster.
Users are still free to disable this feature by applying an override during deployment, but our default RBAC config should include it.

The controller currently exposes the following endpoints:

- `/healthz'
- `/v1/verify`
- `/v1/rotate`
- `/v1/cert.pem`

The controller already must not expose any secrets via the HTTP endpoint, since while RBAC would prevent
end-users to access the service via the proxy, nothing prevents any unprivileged workload in the cluster unless
admins have explicitly configured a strict network policy rule set.

Closes #166
Rel #137

Co-authored-by: Marko Mikulicic <mkm@bitnami.com>
@bors
Copy link
Contributor

bors bot commented Jul 31, 2019

Build succeeded

@bors bors bot merged commit 516503c into master Jul 31, 2019
@bors bors bot deleted the rbacproxy branch July 31, 2019 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install in an isolated namespace
3 participants