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

Add TLS configuration for PostgreSQL #849

Merged
merged 2 commits into from
Oct 14, 2020

Conversation

wxing1292
Copy link
Contributor

@wxing1292 wxing1292 commented Oct 14, 2020

What changed?
Add TLS configuration for PostgreSQL.

Why?
Improve security.

How did you test it?

  • Make PostgreSQL use TLS

  • Make Temporal server use TLS

  • Create keys / certs

openssl genrsa -aes256 -out ca-key.pem 4096
openssl req -new -x509 -days 365 -key ca-key.pem -sha256 -out ca.pem

openssl genrsa -out server-key.pem 4096
openssl req -subj "/CN=localhost" -sha256 -new -key server-key.pem -out server.csr
openssl x509 -req -days 365 -sha256 -in server.csr -CA ca.pem -CAkey ca-key.pem -CAcreateserial -out server-cert.pem

openssl genrsa -out client-key.pem 4096
openssl req -subj '/CN=localhost' -new -key client-key.pem -out client.csr
openssl x509 -req -days 365 -sha256 -in client.csr -CA ca.pem -CAkey ca-key.pem -CAcreateserial -out client-cert.pem

rm -v client.csr server.csr

chmod -v 0400 ca-key.pem server-key.pem client-key.pem 
chmod -v 0444 ca.pem server-cert.pem client-cert.pem
  • Config PostgreSQL

pg_hba.conf:

hostssl    all             all             127.0.0.1/32            md5 clientcert=1                                                      
hostssl    all             all             ::1/128            md5 clientcert=1     

postgresql.conf:

ssl = on
ssl_cert_file = '<path to PostgreSQL cert>'
ssl_key_file = '<path to PostgreSQL key>'
ssl_ca_file = '<path to PostgreSQL CA>'
  • Config Temporal server
datastores:
    postgres-default:
      sql:
        pluginName: "postgres"
        databaseName: "temporal"
        connectAddr: "127.0.0.1:5432"
        connectProtocol: "tcp"
        user: "temporal"
        password: "temporal"
        maxConns: 20
        maxIdleConns: 20
        maxConnLifetime: "1h"
        tls:
          enabled: true
          certFile: "<path to server cert>"
          keyFile: "<path to server key>"
          caFile: "<path to server CA>"
    postgres-visibility:
      sql:
        pluginName: "postgres"
        databaseName: "temporal_visibility"
        connectAddr: "127.0.0.1:5432"
        connectProtocol: "tcp"
        user: "temporal"
        password: "temporal"
        maxConns: 2
        maxIdleConns: 2
        maxConnLifetime: "1h"
        tls:
          enabled: true
          certFile: "<path to server cert>"
          keyFile: "<path to server key>"
          caFile: "<path to server CA>"

Potential risks
N/A

@wxing1292 wxing1292 requested review from samarabbas, sergeybykov and a team October 14, 2020 00:25
// TODO: create a way to set MinVersion and CipherSuites via cfg.
tlsConfig := auth.NewTLSConfigForServer(host)

if cfg.TLS.CaFile != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly also support CaData like we do for cassandra

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one thing at a time.

this only try to make mysql / postgresql similar in terms of file structure, not changing anything for mysql

@wxing1292 wxing1292 merged commit 07a3a63 into temporalio:master Oct 14, 2020
@wxing1292 wxing1292 deleted the persistence-rewrite-17 branch October 14, 2020 02:00
@@ -47,7 +41,7 @@ import (
const (
// PluginName is the name of the plugin
PluginName = "mysql"
dsnFmt = "%s:%s@%v(%v)/%s"
dsnFmt = "%v:%v@%v(%v)/%v"
Copy link
Member

Choose a reason for hiding this comment

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

what is the point of having this as separate const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean this?

const dsnFmt = "%v:%v@%v(%v)/%v"

usually, all string const are defined at the very top

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.

None yet

4 participants