From c2823a933bd1369be1aa5814d2d0960fb77eafce Mon Sep 17 00:00:00 2001 From: Nia Weiss Date: Wed, 26 Aug 2020 16:39:11 -0400 Subject: [PATCH 1/4] add a config option to require TLS on postgres --- pg/protocol.go | 8 ++++++++ pg/server.go | 4 ++++ server/config.go | 2 ++ server/server.go | 1 + 4 files changed, 15 insertions(+) diff --git a/pg/protocol.go b/pg/protocol.go index c55decbd7..2267b1ca7 100644 --- a/pg/protocol.go +++ b/pg/protocol.go @@ -100,6 +100,8 @@ func (s *Server) handle(ctx context.Context, conn net.Conn) (err error) { } } + var hasTLS bool + startup: // Read startup packet. var buf [4]byte @@ -151,6 +153,7 @@ startup: return errors.Wrap(err, "transferring startup deadline to TLS connection") } } + hasTLS = true goto startup } @@ -163,6 +166,11 @@ startup: goto startup } + if s.RequireTLS && !hasTLS { + // Reject the unsecured connection. + return errors.Errorf("client at %s attempted to initiate an unsecured postgres conenction", conn.RemoteAddr()) + } + // Handle regular postgres. return s.handleStandard(ctx, proto, conn, data) } diff --git a/pg/server.go b/pg/server.go index 55d374005..686c86ebe 100644 --- a/pg/server.go +++ b/pg/server.go @@ -35,6 +35,10 @@ type Server struct { // TLSConfig is the TLS configuration to use to serve postgres TLS connections. TLSConfig *tls.Config + // RequireTLS rejects unencrypted postgres connections. + // This allows for mTLS to be used as auth. + RequireTLS bool + // StartupTimeout is the timeout to use for connection startup. // If a connection fails to set up a protocol before this completes, it will be terminated. StartupTimeout time.Duration diff --git a/server/config.go b/server/config.go index 7437d65ed..5ad987dcd 100644 --- a/server/config.go +++ b/server/config.go @@ -174,6 +174,8 @@ type Config struct { // TLS configuration for postgres connections. TLS TLSConfig `toml:"tls"` + RequireTLS bool `toml:"require-tls"` + StartupTimeout toml.Duration `toml:"startup-timeout"` ReadTimeout toml.Duration `toml:"read-timeout"` WriteTimeout toml.Duration `toml:"write-timout"` diff --git a/server/server.go b/server/server.go index 90061a1b4..aa9919c2b 100644 --- a/server/server.go +++ b/server/server.go @@ -191,6 +191,7 @@ func (m *Command) Start() (err error) { m.pgserver.s.WriteTimeout = time.Duration(m.Config.Postgres.WriteTimeout) m.pgserver.s.MaxStartupSize = m.Config.Postgres.MaxStartupSize m.pgserver.s.ConnectionLimit = m.Config.Postgres.ConnectionLimit + m.pgserver.s.RequireTLS = m.Config.Postgres.RequireTLS err := m.pgserver.Start(m.Config.Postgres.Addr) if err != nil { return errors.Wrap(err, "starting postgres") From 47dd6b5b8fa7bcb51b6171e94506647d95b2ea4b Mon Sep 17 00:00:00 2001 From: Nia Weiss Date: Thu, 27 Aug 2020 10:49:01 -0400 Subject: [PATCH 2/4] fix postgres endpoint config --- cmd/export.go | 2 +- cmd/import.go | 2 +- ctl/common.go | 12 ++++++------ ctl/server.go | 11 +++++++++-- pg/protocol.go | 10 ++++++++-- server/server.go | 2 +- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/cmd/export.go b/cmd/export.go index 25ca5f5a0..bceedb98a 100644 --- a/cmd/export.go +++ b/cmd/export.go @@ -50,7 +50,7 @@ The file does not contain any headers. flags.StringVarP(&Exporter.Index, "index", "i", "", "Pilosa index to export") flags.StringVarP(&Exporter.Field, "field", "f", "", "Field to export") flags.StringVarP(&Exporter.Path, "output-file", "o", "", "File to write export to - default stdout") - ctl.SetTLSConfig(flags, &Exporter.TLS.CertificatePath, &Exporter.TLS.CertificateKeyPath, &Exporter.TLS.CACertPath, &Exporter.TLS.SkipVerify, &Exporter.TLS.EnableClientVerification) + ctl.SetTLSConfig(flags, "", &Exporter.TLS.CertificatePath, &Exporter.TLS.CertificateKeyPath, &Exporter.TLS.CACertPath, &Exporter.TLS.SkipVerify, &Exporter.TLS.EnableClientVerification) return exportCmd } diff --git a/cmd/import.go b/cmd/import.go index c18db2b0d..cd091dc9a 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -63,7 +63,7 @@ omitted. If it is present then its format should be YYYY-MM-DDTHH:MM. flags.BoolVarP(&Importer.Sort, "sort", "", false, "Enables sorting before import.") flags.BoolVarP(&Importer.CreateSchema, "create", "e", false, "Create the schema if it does not exist before import.") flags.BoolVarP(&Importer.Clear, "clear", "", false, "Clear the data provided in the import.") - ctl.SetTLSConfig(flags, &Importer.TLS.CertificatePath, &Importer.TLS.CertificateKeyPath, &Importer.TLS.CACertPath, &Importer.TLS.SkipVerify, &Importer.TLS.EnableClientVerification) + ctl.SetTLSConfig(flags, "", &Importer.TLS.CertificatePath, &Importer.TLS.CertificateKeyPath, &Importer.TLS.CACertPath, &Importer.TLS.SkipVerify, &Importer.TLS.EnableClientVerification) return importCmd } diff --git a/ctl/common.go b/ctl/common.go index db788246e..8cd22451c 100644 --- a/ctl/common.go +++ b/ctl/common.go @@ -31,12 +31,12 @@ type CommandWithTLSSupport interface { } // SetTLSConfig creates common TLS flags -func SetTLSConfig(flags *pflag.FlagSet, certificatePath *string, certificateKeyPath *string, caCertPath *string, skipVerify *bool, enableClientVerification *bool) { - flags.StringVarP(certificatePath, "tls.certificate", "", "", "TLS certificate path (usually has the .crt or .pem extension)") - flags.StringVarP(certificateKeyPath, "tls.key", "", "", "TLS certificate key path (usually has the .key extension)") - flags.StringVarP(caCertPath, "tls.ca-certificate", "", "", "TLS CA certificate path (usually has the .pem extension)") - flags.BoolVarP(skipVerify, "tls.skip-verify", "", false, "Skip TLS certificate server verification (not secure)") - flags.BoolVarP(enableClientVerification, "tls.enable-client-verification", "", false, "Enable TLS certificate client verification for incoming connections") +func SetTLSConfig(flags *pflag.FlagSet, prefix string, certificatePath *string, certificateKeyPath *string, caCertPath *string, skipVerify *bool, enableClientVerification *bool) { + flags.StringVarP(certificatePath, prefix+"tls.certificate", "", "", "TLS certificate path (usually has the .crt or .pem extension)") + flags.StringVarP(certificateKeyPath, prefix+"tls.key", "", "", "TLS certificate key path (usually has the .key extension)") + flags.StringVarP(caCertPath, prefix+"tls.ca-certificate", "", "", "TLS CA certificate path (usually has the .pem extension)") + flags.BoolVarP(skipVerify, prefix+"tls.skip-verify", "", false, "Skip TLS certificate server verification (not secure)") + flags.BoolVarP(enableClientVerification, prefix+"tls.enable-client-verification", "", false, "Enable TLS certificate client verification for incoming connections") } // commandClient returns a pilosa.InternalHTTPClient for the command diff --git a/ctl/server.go b/ctl/server.go index 378f951a3..859266038 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -36,7 +36,7 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { flags.Uint64Var(&srv.Config.MaxFileCount, "max-file-count", srv.Config.MaxFileCount, "Soft limit on the maximum number of fragment files Pilosa keeps open simultaneously.") // TLS - SetTLSConfig(flags, &srv.Config.TLS.CertificatePath, &srv.Config.TLS.CertificateKeyPath, &srv.Config.TLS.CACertPath, &srv.Config.TLS.SkipVerify, &srv.Config.TLS.EnableClientVerification) + SetTLSConfig(flags, "", &srv.Config.TLS.CertificatePath, &srv.Config.TLS.CertificateKeyPath, &srv.Config.TLS.CACertPath, &srv.Config.TLS.SkipVerify, &srv.Config.TLS.EnableClientVerification) // Handler flags.StringSliceVarP(&srv.Config.Handler.AllowedOrigins, "handler.allowed-origins", "", []string{}, "Comma separated list of allowed origin URIs (for CORS/WebUI).") @@ -90,5 +90,12 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { flags.StringVarP(&srv.Config.Txsrc, "tx", "", "", "transaction/storage to use: one of roaring, rbf, badger, rbf_roaring, roaring_rbf, badger_roaring, roaring_badger, badger_rbf, or rbf_badger (default roaring)") // Postgres endpoint - flags.StringVar(&srv.Config.Postgres.Addr, "postgres.addr", "", "address to which to bind a postgres endpoint") + flags.StringVar(&srv.Config.Postgres.Addr, "postgres.addr", srv.Config.Postgres.Addr, "address to which to bind a postgres endpoint (leave blank to disable)") + SetTLSConfig(flags, "postgres.", &srv.Config.Postgres.TLS.CertificatePath, &srv.Config.Postgres.TLS.CertificateKeyPath, &srv.Config.Postgres.TLS.CACertPath, &srv.Config.Postgres.TLS.SkipVerify, &srv.Config.Postgres.TLS.EnableClientVerification) + flags.BoolVar(&srv.Config.Postgres.RequireTLS, "postgres.tls.require", srv.Config.Postgres.RequireTLS, "Require TLS on all incoming postgres connections.") + flags.DurationVar((*time.Duration)(&srv.Config.Postgres.StartupTimeout), "postgres.startup-timeout", time.Duration(srv.Config.Postgres.StartupTimeout), "Timeout for postgres connection startup. (set 0 to disable)") + flags.DurationVar((*time.Duration)(&srv.Config.Postgres.ReadTimeout), "postgres.read-timeout", time.Duration(srv.Config.Postgres.ReadTimeout), "Timeout for reads on a postgres connection. (set 0 to disable; does not include connection idling)") + flags.DurationVar((*time.Duration)(&srv.Config.Postgres.WriteTimeout), "postgres.write-timeout", time.Duration(srv.Config.Postgres.WriteTimeout), "Timeout for writes on a postgres connection. (set 0 to disable)") + flags.Uint32Var(&srv.Config.Postgres.MaxStartupSize, "postgres.max-startup-size", srv.Config.Postgres.MaxStartupSize, "Maximum acceptable size of a postgres startup packet, in bytes. (set 0 to disable)") + flags.Uint16Var(&srv.Config.Postgres.ConnectionLimit, "postgres.connection-limit", srv.Config.Postgres.ConnectionLimit, "Maximum number of simultaneous postgres connections to allow. (set 0 to disable)") } diff --git a/pg/protocol.go b/pg/protocol.go index 2267b1ca7..60c9af541 100644 --- a/pg/protocol.go +++ b/pg/protocol.go @@ -73,9 +73,17 @@ func (p Protocol) String() string { // handle reads the startup packet and dispatches an appropriate protocol handler for the connection. func (s *Server) handle(ctx context.Context, conn net.Conn) (err error) { + var hasTLS bool + defer func() { cerr := conn.Close() if cerr != nil && err == nil { + if hasTLS { + if nerr, ok := cerr.(net.Error); ok && nerr.Timeout() { + // TLS does this sometimes. + return + } + } err = errors.Wrap(cerr, "closing connection") } }() @@ -100,8 +108,6 @@ func (s *Server) handle(ctx context.Context, conn net.Conn) (err error) { } } - var hasTLS bool - startup: // Read startup packet. var buf [4]byte diff --git a/server/server.go b/server/server.go index aa9919c2b..df15981c8 100644 --- a/server/server.go +++ b/server/server.go @@ -181,7 +181,7 @@ func (m *Command) Start() (err error) { if m.Config.Postgres.TLS.CertificatePath != "" { conf, err := GetTLSConfig(&m.Config.Postgres.TLS, m.logger.Logger()) if err != nil { - return errors.Wrap(err, "settuing up postgres TLS") + return errors.Wrap(err, "setting up postgres TLS") } tlsConf = conf } From 288593231ffbd32fa29de579a007cec1041e975c Mon Sep 17 00:00:00 2001 From: Nia Weiss Date: Thu, 27 Aug 2020 10:59:40 -0400 Subject: [PATCH 3/4] require TLS when configured --- ctl/server.go | 1 - pg/protocol.go | 2 +- pg/server.go | 4 ---- server/config.go | 2 -- server/server.go | 1 - 5 files changed, 1 insertion(+), 9 deletions(-) diff --git a/ctl/server.go b/ctl/server.go index 859266038..bed61f2ef 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -92,7 +92,6 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { // Postgres endpoint flags.StringVar(&srv.Config.Postgres.Addr, "postgres.addr", srv.Config.Postgres.Addr, "address to which to bind a postgres endpoint (leave blank to disable)") SetTLSConfig(flags, "postgres.", &srv.Config.Postgres.TLS.CertificatePath, &srv.Config.Postgres.TLS.CertificateKeyPath, &srv.Config.Postgres.TLS.CACertPath, &srv.Config.Postgres.TLS.SkipVerify, &srv.Config.Postgres.TLS.EnableClientVerification) - flags.BoolVar(&srv.Config.Postgres.RequireTLS, "postgres.tls.require", srv.Config.Postgres.RequireTLS, "Require TLS on all incoming postgres connections.") flags.DurationVar((*time.Duration)(&srv.Config.Postgres.StartupTimeout), "postgres.startup-timeout", time.Duration(srv.Config.Postgres.StartupTimeout), "Timeout for postgres connection startup. (set 0 to disable)") flags.DurationVar((*time.Duration)(&srv.Config.Postgres.ReadTimeout), "postgres.read-timeout", time.Duration(srv.Config.Postgres.ReadTimeout), "Timeout for reads on a postgres connection. (set 0 to disable; does not include connection idling)") flags.DurationVar((*time.Duration)(&srv.Config.Postgres.WriteTimeout), "postgres.write-timeout", time.Duration(srv.Config.Postgres.WriteTimeout), "Timeout for writes on a postgres connection. (set 0 to disable)") diff --git a/pg/protocol.go b/pg/protocol.go index 60c9af541..f0463ecc1 100644 --- a/pg/protocol.go +++ b/pg/protocol.go @@ -172,7 +172,7 @@ startup: goto startup } - if s.RequireTLS && !hasTLS { + if s.TLSConfig != nil && !hasTLS { // Reject the unsecured connection. return errors.Errorf("client at %s attempted to initiate an unsecured postgres conenction", conn.RemoteAddr()) } diff --git a/pg/server.go b/pg/server.go index 686c86ebe..55d374005 100644 --- a/pg/server.go +++ b/pg/server.go @@ -35,10 +35,6 @@ type Server struct { // TLSConfig is the TLS configuration to use to serve postgres TLS connections. TLSConfig *tls.Config - // RequireTLS rejects unencrypted postgres connections. - // This allows for mTLS to be used as auth. - RequireTLS bool - // StartupTimeout is the timeout to use for connection startup. // If a connection fails to set up a protocol before this completes, it will be terminated. StartupTimeout time.Duration diff --git a/server/config.go b/server/config.go index 5ad987dcd..7437d65ed 100644 --- a/server/config.go +++ b/server/config.go @@ -174,8 +174,6 @@ type Config struct { // TLS configuration for postgres connections. TLS TLSConfig `toml:"tls"` - RequireTLS bool `toml:"require-tls"` - StartupTimeout toml.Duration `toml:"startup-timeout"` ReadTimeout toml.Duration `toml:"read-timeout"` WriteTimeout toml.Duration `toml:"write-timout"` diff --git a/server/server.go b/server/server.go index df15981c8..7702aca79 100644 --- a/server/server.go +++ b/server/server.go @@ -191,7 +191,6 @@ func (m *Command) Start() (err error) { m.pgserver.s.WriteTimeout = time.Duration(m.Config.Postgres.WriteTimeout) m.pgserver.s.MaxStartupSize = m.Config.Postgres.MaxStartupSize m.pgserver.s.ConnectionLimit = m.Config.Postgres.ConnectionLimit - m.pgserver.s.RequireTLS = m.Config.Postgres.RequireTLS err := m.pgserver.Start(m.Config.Postgres.Addr) if err != nil { return errors.Wrap(err, "starting postgres") From f0f8336f57a8c1f0621fd2b16b306683b623bda4 Mon Sep 17 00:00:00 2001 From: Nia Weiss Date: Thu, 27 Aug 2020 11:35:54 -0400 Subject: [PATCH 4/4] rename postgres.addr to postgres.bind for consistency --- ctl/server.go | 2 +- server/config.go | 4 ++-- server/server.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ctl/server.go b/ctl/server.go index bed61f2ef..23a269612 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -90,7 +90,7 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { flags.StringVarP(&srv.Config.Txsrc, "tx", "", "", "transaction/storage to use: one of roaring, rbf, badger, rbf_roaring, roaring_rbf, badger_roaring, roaring_badger, badger_rbf, or rbf_badger (default roaring)") // Postgres endpoint - flags.StringVar(&srv.Config.Postgres.Addr, "postgres.addr", srv.Config.Postgres.Addr, "address to which to bind a postgres endpoint (leave blank to disable)") + flags.StringVar(&srv.Config.Postgres.Bind, "postgres.bind", srv.Config.Postgres.Bind, "Address to which to bind a postgres endpoint (leave blank to disable)") SetTLSConfig(flags, "postgres.", &srv.Config.Postgres.TLS.CertificatePath, &srv.Config.Postgres.TLS.CertificateKeyPath, &srv.Config.Postgres.TLS.CACertPath, &srv.Config.Postgres.TLS.SkipVerify, &srv.Config.Postgres.TLS.EnableClientVerification) flags.DurationVar((*time.Duration)(&srv.Config.Postgres.StartupTimeout), "postgres.startup-timeout", time.Duration(srv.Config.Postgres.StartupTimeout), "Timeout for postgres connection startup. (set 0 to disable)") flags.DurationVar((*time.Duration)(&srv.Config.Postgres.ReadTimeout), "postgres.read-timeout", time.Duration(srv.Config.Postgres.ReadTimeout), "Timeout for reads on a postgres connection. (set 0 to disable; does not include connection idling)") diff --git a/server/config.go b/server/config.go index 7437d65ed..c51a7f95f 100644 --- a/server/config.go +++ b/server/config.go @@ -168,9 +168,9 @@ type Config struct { } `toml:"profile"` Postgres struct { - // Addr is the address to which to bind a postgres endpoint. + // Bind is the address to which to bind a postgres endpoint. // If this is empty, no endpoint will be created. - Addr string `toml:"addr"` + Bind string `toml:"bind"` // TLS configuration for postgres connections. TLS TLSConfig `toml:"tls"` diff --git a/server/server.go b/server/server.go index 7702aca79..546db5d41 100644 --- a/server/server.go +++ b/server/server.go @@ -176,7 +176,7 @@ func (m *Command) Start() (err error) { // Initialize postgres. m.pgserver = nil - if m.Config.Postgres.Addr != "" { + if m.Config.Postgres.Bind != "" { var tlsConf *tls.Config if m.Config.Postgres.TLS.CertificatePath != "" { conf, err := GetTLSConfig(&m.Config.Postgres.TLS, m.logger.Logger()) @@ -191,7 +191,7 @@ func (m *Command) Start() (err error) { m.pgserver.s.WriteTimeout = time.Duration(m.Config.Postgres.WriteTimeout) m.pgserver.s.MaxStartupSize = m.Config.Postgres.MaxStartupSize m.pgserver.s.ConnectionLimit = m.Config.Postgres.ConnectionLimit - err := m.pgserver.Start(m.Config.Postgres.Addr) + err := m.pgserver.Start(m.Config.Postgres.Bind) if err != nil { return errors.Wrap(err, "starting postgres") }