[RFC] Trust Store for TLS and SSH #3366
Conversation
pjbgf
added
area/rfc
| Object-level trust store expansion is disabled by default. To enable it start | ||
| the controller with: |
There was a problem hiding this comment.
I would love to see this expanded a little to provide reasoning for this default setting. With the current Flux version 0.37 the trust store can always be expanded for objects such as HelmRepositories or GitRepositories by defining a caFile field in the referenced Secret. Given many admins will unlikely deviate from the defaults, users would no longer be able to do that. Please correct me if I'm wrong here.
There was a problem hiding this comment.
This is certainly a point up for debate. The security invariant here was "Flux should only connect to trusted servers" and the criteria used was that most users would expect Flux to be secure by default.
Given many admins will unlikely deviate from the defaults
This is exactly the point. If something works, users won't look up the documentation and try understand the impact of security sensitive settings. Hence why ideally we should always prioritise security by design and by default.
The idea is that, by pre-populating the controller level SSH trust store with the top SaaS Git providers, a considerable amount of users would not have to worry about this. It will simply securely work - just like for TLS.
Then enterprise customers should configure this once, at controller level and be done with it, for both single and multi-tenancy. The only use case I see for Object-level trust stores to be used in production, would be in a Namespace as a Service multi-tenancy model, which would further weaken an already fragile security posture.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
pjbgf
force-pushed
the
rfc-trust-store
branch
from
c11ba29
to
1739284
Compare
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
pjbgf
force-pushed
the
rfc-trust-store
branch
from
1739284
to
763f262
Compare
pjbgf
changed the title
| the main Git SaaS providers. As a result, users will only need to update their SSH | ||
| Trust Store for custom or less well known servers. | ||
|
|
||
| #### TLS |
There was a problem hiding this comment.
| #### TLS | |
| ### TLS |
| ecosystem (e.g. OPA) to enable users to achieve the same outcome. | ||
|
|
||
| Due to the importance that Flux has in the bootstrapping of clusters, such an | ||
| important requirement (enforce trust at controller level) should be inherit to |
There was a problem hiding this comment.
maybe
| important requirement (enforce trust at controller level) should be inherit to | |
| important requirement (enforce trust at controller level) should be inherent to |
| The new fields `spec.trustStore.tls` and `spec.trustStore.ssh` analogous | ||
| to Kubernetes `EnvFromSource`, in which it can be used to define either a | ||
| `configMapRef` or a `secretRef`, but not both. | ||
|
|
There was a problem hiding this comment.
Maybe we can also have some details about how the controllers would behave for different values of --insecure-object-trust-store={tls-only,ssh-only,both,disabled}.
When it's disabled but a source definition specifies a trust store, should the object reconciliation stall? Because the configuration is invalid based on the current controller level configuration and can only be fixed by restarting the controller with new config or updating the source definition with different config.
Also, maybe some description about the error messaging when it's completely disabled and partially disabled and how it'd be communicated to the user.
| A new field is to be introduced into the existing kinds `ImageUpdateAutomation` and | ||
| `GitRepository`, to allow users to expand on the controller-level known hosts for | ||
| SSH operations: |
There was a problem hiding this comment.
I find this very problematic, why would we allow for a namespaced object own by a tenant to alter the trust store for all tenants? This also makes Flux bootstrap break, since you can't control which CR is reconciled first, if people add a trust store in one CR we can't reconcile that first to make it available to others.
There was a problem hiding this comment.
It does not alter the trust store for all tenants, but rather only expands the trust store for that object only.
There was a problem hiding this comment.
How does this cope with the know_hosts from the secretRef? Can you explain the differences in the doc please?
There was a problem hiding this comment.
I will rephrase to make that clearer. But basically that's what we already do at present based on the CA bundles provided by secrets, however this would be based on a field instead.
Fixes #3078.