From 4506a3e3591195ace4e215f6f35d01b45d4208d2 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Thu, 1 Feb 2018 15:07:59 -0800 Subject: [PATCH 1/6] Gratuitously rename `configSSL()` to `configTLS()`. SSL is dead, Jim. --- conn.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/conn.go b/conn.go index 125d9032..efeba5ff 100644 --- a/conn.go +++ b/conn.go @@ -701,7 +701,7 @@ func ParseURI(uri string) (ConnConfig, error) { cp.Dial = d.Dial } - err = configSSL(url.Query().Get("sslmode"), &cp) + err = configTLS(url.Query().Get("sslmode"), &cp) if err != nil { return cp, err } @@ -779,7 +779,7 @@ func ParseDSN(s string) (ConnConfig, error) { } } - err := configSSL(sslmode, &cp) + err := configTLS(sslmode, &cp) if err != nil { return cp, err } @@ -859,7 +859,7 @@ func ParseEnvLibpq() (ConnConfig, error) { sslmode := os.Getenv("PGSSLMODE") - err := configSSL(sslmode, &cc) + err := configTLS(sslmode, &cc) if err != nil { return cc, err } @@ -874,7 +874,7 @@ func ParseEnvLibpq() (ConnConfig, error) { return cc, nil } -func configSSL(sslmode string, cc *ConnConfig) error { +func configTLS(sslmode string, cc *ConnConfig) error { // Match libpq default behavior if sslmode == "" { sslmode = "prefer" From d7f24b91f4d9ce819213e47e333d585598f17d37 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Thu, 1 Feb 2018 22:58:14 -0800 Subject: [PATCH 2/6] Make ParseURI() compatible with lib/pq's TLS keywords. Add support for: - `sslrootcert` - `sslcert` - `sslkey` All three arguments, like thir `gitub.com/lib/pq` counterparts, are filesystem paths. --- conn.go | 41 ++++++++++++++++++++++++++++++++++++++++- conn_test.go | 3 ++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/conn.go b/conn.go index efeba5ff..b9224200 100644 --- a/conn.go +++ b/conn.go @@ -4,10 +4,12 @@ import ( "context" "crypto/md5" "crypto/tls" + "crypto/x509" "encoding/binary" "encoding/hex" "fmt" "io" + "io/ioutil" "net" "net/url" "os" @@ -706,9 +708,46 @@ func ParseURI(uri string) (ConnConfig, error) { return cp, err } + // Extract optional TLS parameters and reconstruct a coherent tls.Config based + // on the DSN input. Reuse the same keywords found in github.com/lib/pq. + if cp.TLSConfig != nil { + { + caCertPool := x509.NewCertPool() + + caPath := url.Query().Get("sslrootcert") + caCert, err := ioutil.ReadFile(caPath) + if err != nil { + return cp, errors.Wrapf(err, "unable to read CA file %q", caPath) + } + + if !caCertPool.AppendCertsFromPEM(caCert) { + return cp, errors.Wrap(err, "unable to add CA to cert pool") + } + + cp.TLSConfig.RootCAs = caCertPool + cp.TLSConfig.ClientCAs = caCertPool + } + + sslcert := url.Query().Get("sslcert") + sslkey := url.Query().Get("sslkey") + if (sslcert != "" && sslkey == "") || (sslcert == "" && sslkey != "") { + return cp, fmt.Errorf(`both "sslcert" and "sslkey" are required`) + } + + cert, err := tls.LoadX509KeyPair(sslcert, sslkey) + if err != nil { + return cp, errors.Wrap(err, "unable to read cert") + } + + cp.TLSConfig.Certificates = []tls.Certificate{cert} + } + ignoreKeys := map[string]struct{}{ - "sslmode": {}, "connect_timeout": {}, + "sslcert": {}, + "sslkey": {}, + "sslmode": {}, + "sslrootcert": {}, } cp.RuntimeParams = make(map[string]string) diff --git a/conn_test.go b/conn_test.go index 6144521d..6f1d41ea 100644 --- a/conn_test.go +++ b/conn_test.go @@ -228,7 +228,8 @@ func TestConnectWithTLSFallback(t *testing.T) { } connConfig.UseFallbackTLS = true - connConfig.FallbackTLSConfig = &tls.Config{InsecureSkipVerify: true} + connConfig.FallbackTLSConfig = tlsConnConfig.TLSConfig + connConfig.FallbackTLSConfig.InsecureSkipVerify = true conn, err = pgx.Connect(connConfig) if err != nil { From 8078930406ebb0b7f182df3c9c0dfa23d133812e Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Thu, 1 Feb 2018 23:51:50 -0800 Subject: [PATCH 3/6] Add TLS arg parsing to ParseDSN(). Factor out the TLS cert handling and add it to `configTLS()` via a `struct` argument. --- conn.go | 108 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/conn.go b/conn.go index b9224200..8064a8be 100644 --- a/conn.go +++ b/conn.go @@ -703,45 +703,17 @@ func ParseURI(uri string) (ConnConfig, error) { cp.Dial = d.Dial } - err = configTLS(url.Query().Get("sslmode"), &cp) + tlsArgs := configTLSArgs{ + sslCert: url.Query().Get("sslcert"), + sslKey: url.Query().Get("sslkey"), + sslMode: url.Query().Get("sslmode"), + sslRootCert: url.Query().Get("sslrootcert"), + } + err = configTLS(tlsArgs, &cp) if err != nil { return cp, err } - // Extract optional TLS parameters and reconstruct a coherent tls.Config based - // on the DSN input. Reuse the same keywords found in github.com/lib/pq. - if cp.TLSConfig != nil { - { - caCertPool := x509.NewCertPool() - - caPath := url.Query().Get("sslrootcert") - caCert, err := ioutil.ReadFile(caPath) - if err != nil { - return cp, errors.Wrapf(err, "unable to read CA file %q", caPath) - } - - if !caCertPool.AppendCertsFromPEM(caCert) { - return cp, errors.Wrap(err, "unable to add CA to cert pool") - } - - cp.TLSConfig.RootCAs = caCertPool - cp.TLSConfig.ClientCAs = caCertPool - } - - sslcert := url.Query().Get("sslcert") - sslkey := url.Query().Get("sslkey") - if (sslcert != "" && sslkey == "") || (sslcert == "" && sslkey != "") { - return cp, fmt.Errorf(`both "sslcert" and "sslkey" are required`) - } - - cert, err := tls.LoadX509KeyPair(sslcert, sslkey) - if err != nil { - return cp, errors.Wrap(err, "unable to read cert") - } - - cp.TLSConfig.Certificates = []tls.Certificate{cert} - } - ignoreKeys := map[string]struct{}{ "connect_timeout": {}, "sslcert": {}, @@ -783,7 +755,7 @@ func ParseDSN(s string) (ConnConfig, error) { m := dsnRegexp.FindAllStringSubmatch(s, -1) - var sslmode string + tlsArgs := configTLSArgs{} cp.RuntimeParams = make(map[string]string) @@ -804,7 +776,13 @@ func ParseDSN(s string) (ConnConfig, error) { case "dbname": cp.Database = b[2] case "sslmode": - sslmode = b[2] + tlsArgs.sslMode = b[2] + case "sslrootcert": + tlsArgs.sslRootCert = b[2] + case "sslcert": + tlsArgs.sslCert = b[2] + case "sslkey": + tlsArgs.sslKey = b[2] case "connect_timeout": timeout, err := strconv.ParseInt(b[2], 10, 64) if err != nil { @@ -818,7 +796,7 @@ func ParseDSN(s string) (ConnConfig, error) { } } - err := configTLS(sslmode, &cp) + err := configTLS(tlsArgs, &cp) if err != nil { return cp, err } @@ -898,7 +876,7 @@ func ParseEnvLibpq() (ConnConfig, error) { sslmode := os.Getenv("PGSSLMODE") - err := configTLS(sslmode, &cc) + err := configTLS(configTLSArgs{sslMode: sslmode}, &cc) if err != nil { return cc, err } @@ -913,14 +891,27 @@ func ParseEnvLibpq() (ConnConfig, error) { return cc, nil } -func configTLS(sslmode string, cc *ConnConfig) error { +type configTLSArgs struct { + sslMode string + sslRootCert string + sslCert string + sslKey string +} + +// configTLS uses lib/pq's TLS parameters to reconstruct a coherent tls.Config. +// Inputs are parsed out and provided by ParseDSN() or ParseURI(). +func configTLS(args configTLSArgs, cc *ConnConfig) error { // Match libpq default behavior - if sslmode == "" { - sslmode = "prefer" + if args.sslMode == "" { + args.sslMode = "prefer" } - switch sslmode { + switch args.sslMode { case "disable": + cc.UseFallbackTLS = false + cc.TLSConfig = nil + cc.FallbackTLSConfig = nil + return nil case "allow": cc.UseFallbackTLS = true cc.FallbackTLSConfig = &tls.Config{InsecureSkipVerify: true} @@ -938,6 +929,37 @@ func configTLS(sslmode string, cc *ConnConfig) error { return errors.New("sslmode is invalid") } + { + caCertPool := x509.NewCertPool() + + caPath := args.sslRootCert + caCert, err := ioutil.ReadFile(caPath) + if err != nil { + return errors.Wrapf(err, "unable to read CA file %q", caPath) + } + + if !caCertPool.AppendCertsFromPEM(caCert) { + return errors.Wrap(err, "unable to add CA to cert pool") + } + + cc.TLSConfig.RootCAs = caCertPool + cc.TLSConfig.ClientCAs = caCertPool + } + + sslcert := args.sslCert + sslkey := args.sslKey + + if (sslcert != "" && sslkey == "") || (sslcert == "" && sslkey != "") { + return fmt.Errorf(`both "sslcert" and "sslkey" are required`) + } + + cert, err := tls.LoadX509KeyPair(sslcert, sslkey) + if err != nil { + return errors.Wrap(err, "unable to read cert") + } + + cc.TLSConfig.Certificates = []tls.Certificate{cert} + return nil } From 52bada3401ad89f239c40c329b6f25d2f6ac8bae Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Fri, 2 Feb 2018 00:18:06 -0800 Subject: [PATCH 4/6] Add a clientcert example to simplify future TLS testing. --- conn_config_test.go.example | 55 +++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/conn_config_test.go.example b/conn_config_test.go.example index 463c0841..096e1354 100644 --- a/conn_config_test.go.example +++ b/conn_config_test.go.example @@ -1,7 +1,14 @@ package pgx_test import ( - "github.com/jackc/pgx" + // "crypto/tls" + // "crypto/x509" + // "fmt" + // "go/build" + // "io/ioutil" + // "path" + + "github.com/jackc/pgx" ) var defaultConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "pgx_md5", Password: "secret", Database: "pgx_test"} @@ -22,7 +29,51 @@ var cratedbConnConfig *pgx.ConnConfig = nil // var md5ConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "pgx_md5", Password: "secret", Database: "pgx_test"} // var plainPasswordConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "pgx_pw", Password: "secret", Database: "pgx_test"} // var invalidUserConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "invalid", Database: "pgx_test"} -// var tlsConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "pgx_md5", Password: "secret", Database: "pgx_test", TLSConfig: &tls.Config{InsecureSkipVerify: true}} // var customDialerConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "pgx_md5", Password: "secret", Database: "pgx_test"} // var replicationConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "pgx_replication", Password: "secret", Database: "pgx_test"} +// var tlsConnConfig *pgx.ConnConfig = &pgx.ConnConfig{Host: "127.0.0.1", User: "pgx_md5", Password: "secret", Database: "pgx_test", TLSConfig: &tls.Config{InsecureSkipVerify: true}} +// +//// or to test client certs: +// +// var tlsConnConfig *pgx.ConnConfig +// +// func init() { +// homeDir := build.Default.GOPATH +// tlsConnConfig = &pgx.ConnConfig{ +// Host: "127.0.0.1", +// User: "pgx_md5", +// Password: "secret", +// Database: "pgx_test", +// TLSConfig: &tls.Config{ +// InsecureSkipVerify: true, +// }, +// } +// caCertPool := x509.NewCertPool() +// +// caPath := path.Join(homeDir, "/src/github.com/jackc/pgx/rootCA.pem") +// caCert, err := ioutil.ReadFile(caPath) +// if err != nil { +// panic(fmt.Sprintf("unable to read CA file: %v", err)) +// } +// +// if !caCertPool.AppendCertsFromPEM(caCert) { +// panic("unable to add CA to cert pool") +// } +// +// tlsConnConfig.TLSConfig.RootCAs = caCertPool +// tlsConnConfig.TLSConfig.ClientCAs = caCertPool +// +// sslCert := path.Join(homeDir, "/src/github.com/jackc/pgx/pg_md5.crt") +// sslKey := path.Join(homeDir, "/src/github.com/jackc/pgx/pg_md5.key") +// if (sslCert != "" && sslKey == "") || (sslCert == "" && sslKey != "") { +// panic(`both "sslcert" and "sslkey" are required`) +// } +// +// cert, err := tls.LoadX509KeyPair(sslCert, sslKey) +// if err != nil { +// panic(fmt.Sprintf("unable to read cert: %v", err)) +// } +// +// tlsConnConfig.TLSConfig.Certificates = []tls.Certificate{cert} +// } From f0dc593c2f9236e9d1c68728a73af27dcd989eb6 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Fri, 2 Feb 2018 08:24:41 -0800 Subject: [PATCH 5/6] Only initialize the CA if the path is not an empty string. --- conn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conn.go b/conn.go index 8064a8be..ee8d9c81 100644 --- a/conn.go +++ b/conn.go @@ -929,7 +929,7 @@ func configTLS(args configTLSArgs, cc *ConnConfig) error { return errors.New("sslmode is invalid") } - { + if args.sslRootCert != "" { caCertPool := x509.NewCertPool() caPath := args.sslRootCert From 6a4303120fbdcfd5ceb9438aaae62eacb8c41a73 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Fri, 2 Feb 2018 08:37:23 -0800 Subject: [PATCH 6/6] Only read in TLS certs when the key and cert are present. --- conn.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/conn.go b/conn.go index ee8d9c81..9509973b 100644 --- a/conn.go +++ b/conn.go @@ -953,12 +953,14 @@ func configTLS(args configTLSArgs, cc *ConnConfig) error { return fmt.Errorf(`both "sslcert" and "sslkey" are required`) } - cert, err := tls.LoadX509KeyPair(sslcert, sslkey) - if err != nil { - return errors.Wrap(err, "unable to read cert") - } + if sslcert != "" && sslkey != "" { + cert, err := tls.LoadX509KeyPair(sslcert, sslkey) + if err != nil { + return errors.Wrap(err, "unable to read cert") + } - cc.TLSConfig.Certificates = []tls.Certificate{cert} + cc.TLSConfig.Certificates = []tls.Certificate{cert} + } return nil }