CrunchyData / postgres-operator

Production PostgreSQL for Kubernetes, from high availability Postgres clusters to full-scale database-as-a-service.
https://access.crunchydata.com/documentation/postgres-operator/v5/
Apache License 2.0
3.94k stars 593 forks source link

Add Optional TLS Usage in 5.0 #2534

Closed Roia5 closed 3 years ago

Roia5 commented 3 years ago

Overview

From the version 5.0 documentation:

All connections in PGO use TLS to encrypt communication between components.

We would like to have an option to disable TLS since it can degrade performance significantly. Our workloads are very performance-aware and any degradation can outweigh the benefits of added security.

Use Case

An option to disable TLS.

Desired Behavior

Maybe a PostgresCluster (CR) field to enable/disable TLS?

jkatz commented 3 years ago

TLS is required in PGO 5.0. You can choose to bring your own TLS infrastructure or use the defaults provided.

If you can provide benchmarks that demonstrate that the delta between enabling/diabling TLS is significant enough to consider disabling it, we can reconsider.

You can also use the PGO 4.x series, where TLS is opt in.

funbits-ci commented 3 years ago

Forcing TLS is a good default, but having the ability to opt-out is a must for some old applications with no TLS support to work such as GeoServer. Zalando operator do the same

Additionally, we have production microservices that need code change to support TLS, so we need to disable TLS until we upgrade all our services.

PGO 4.x series is not an option for those with full GitOps practice.

Finally, for some workloads we enforce Mutual-TLS in the service mesh layer.

So please consider providing an option to opt-out. Thanks!

cortopy commented 3 years ago

TLS shouldn't be required in clusters where e2e encryption is provided by other means: CNIs like calico and cilium can do this, service meshes like Istio or linkerd with mTLS

I don't understand why forcing TLS is a must for the postgres operator

jkatz commented 3 years ago

TLS shouldn't be required in clusters where e2e encryption is provided by other means: CNIs like calico and cilium can do this, service meshes like Istio or linkerd with mTLS

It would be great if you can provide examples that show these methods support TLS communication fully end-to-end. I've often see the TLS termination occur too soon and the final hop to Postgres occurring over an insecure, accessible network channel.

That said, 5.0.1 will include a way to bypass the TLS endpoint.

jkatz commented 3 years ago

2583 is now merged. In an upcoming release, you can bypass TLS by doing the following in your spec file:

spec:
  patroni:
    dynamicConfiguration:
      postgresql:
        pg_hba:
          - "hostnossl all all all md5"

(note that passwords hashed using SCRAM; SCRAM still occurs even with md5 marked here).

cortopy commented 3 years ago

@jkatz this was so fast! thank you so much

TLS shouldn't be required in clusters where e2e encryption is provided by other means: CNIs like calico and cilium can do this, service meshes like Istio or linkerd with mTLS

It would be great if you can provide examples that show these methods support TLS communication fully end-to-end. I've often see the TLS termination occur too soon and the final hop to Postgres occurring over an insecure, accessible network channel.

That said, 5.0.1 will include a way to bypass the TLS endpoint.

My example has to do with service meshes with mTLS configured as a default. If the postgres instance is part of a service mesh like Istio or Linkerd, it will get a proxy injected in the pod. From that moment the database container cannot receive connections that bypass the proxy as during injection all network traffic is configured to pass via the Istio/Linkerd proxy.

In fact, when patroni starts I can see that the first few requests fail until the proxy is ready because the container can't make any requests until the proxy is ready.

There is no strict/enforcing mode in linkerd, but there are ways to make sure that all pods are configured to utilise mtls too. In the case of Istio this is normally configured with automatic mTLS in strict mode: https://istio.io/latest/docs/concepts/security/#peer-authentication

Users should know what they are doing in this case though. Having an opt-out rather than opt-in option for tls is an amazing thing for postgres operator. Thanks again!

jkatz commented 3 years ago

Thanks for the explanation. 5.0.1 is released, so https://github.com/CrunchyData/postgres-operator/issues/2534#issuecomment-891202207 will work.

jkatz commented 3 years ago

Noting that I updated the example in https://github.com/CrunchyData/postgres-operator/issues/2534#issuecomment-891202207 to use hostnossl. as host allows both TLS and non-TLS connections. Given many client libraries default sslmode to prefer, they would attempt to try a TLS connection first and thus succeed.

Thus the updated example shows how to explicitly disable TLS connections from the server side.

Roia5 commented 2 years ago

In case anyone encounters errors about SSL even when following https://github.com/CrunchyData/postgres-operator/issues/2534#issuecomment-891202207:

When using pgbouncer you should also set the following configuration: spec.proxy.pgBouncer.config.global.server_tls_sslmode: disable

This is needed because the pgBouncer itself attempts to connect using TLS even when adding the pg_hba entry as described.

EugenMayer commented 8 months ago

2583 is now merged. In an upcoming release, you can bypass TLS by doing the following in your spec file:

spec:
  patroni:
    dynamicConfiguration:
      postgresql:
        pg_hba:
          - "hostnossl all all all md5"

(note that passwords hashed using SCRAM; SCRAM still occurs even with md5 marked here).

Be aware that using this will block all your SSL connections. This is since the default rule hostssl all all all md5 that is deployed by default, no longer is part of the configuration. I assume, that this is the actual default of patroni....pg_hba and we overide the entire array without deep merging it.

To allow SSL and non SSL connections, one would rather use

spec:
  patroni:
     dynamicConfiguration:
       postgresql:
         pg_hba:
           - "hostssl all all all md5"
           - "hostnossl all all all md5"
tuan-phan commented 2 months ago

Ensure you update the password_encryption method to md5 too

  patroni:
    dynamicConfiguration:
      postgresql:
        parameters:
          shared_preload_libraries: "timescaledb"
          ssl: "off"
          password_encryption: "md5"
        pg_hba:
          - "hostnossl all all all md5"