Pgx: Why sslmode = require checks SSL certs.

[zargex]

Hello, I am wondering about this.

Why pgx treats require like verify-full ?

Would you accept a pull request with the changes ?

Thanks.

[jackc]

It's been quite a while, so I'm a bit fuzzy on the details. But if I recall correctly the issue was that there wasn't a way to exactly replicate the properties of require. That is, the Go TLS library either makes stronger or weaker security guarantees but not exactly the same. That said, I may have been mistaken at the time or the Go TLS system may be more full-featured now.

If it can be implemented correctly now then a PR would be welcome.

[jacobmarble]

I'm bumping into this issue using Amazon RedShift. Our situation is such that we want SSL, but don't need to verify the certificate, so I can't use pgx to connect to our cluster without the unneeded hassle of installing their public key.

This diff makes required work for me. Notice that I had to remove ssl_renegotiation_limit, because RedShift doesn't support it. Any red flags?

$ git diff
diff --git a/conn.go b/conn.go
index f549e03..d0b4f4b 100644
--- a/conn.go
+++ b/conn.go
@@ -321,7 +321,7 @@ func (c *Conn) connect(config ConnConfig, network, address string, tlsConfig *tl
        // Go does not support (https://github.com/golang/go/issues/5742)
        // PostgreSQL recommends disabling (http://www.postgresql.org/docs/9.4/static/runtime-config-connection.html#GUC-SSL-RENEGOTIATION-LIMIT)
        if tlsConfig != nil {
-               startupMsg.Parameters["ssl_renegotiation_limit"] = "0"
+               //startupMsg.Parameters["ssl_renegotiation_limit"] = "0"
        }

        // Copy default run-time params
@@ -711,7 +711,12 @@ func configSSL(sslmode string, cc *ConnConfig) error {
                cc.TLSConfig = &tls.Config{InsecureSkipVerify: true}
                cc.UseFallbackTLS = true
                cc.FallbackTLSConfig = nil
-       case "require", "verify-ca", "verify-full":
+       case "require":
+               cc.TLSConfig = &tls.Config{
+                       InsecureSkipVerify: true,
+                       ServerName:         cc.Host,
+               }
+       case "verify-ca", "verify-full":
                cc.TLSConfig = &tls.Config{
                        ServerName: cc.Host,
                }

[jackc]

I remember ssl_renegotiation_limit being an issue with Redshift in the past, but I thought they had added support at some point. Maybe the Redshift instance needs to be upgraded? Also Go has some support for renegotiation now in the tls.Config struct. The error that manifested when this was wrong was a broken connection after 512MB of transfer. That can be tested with a simple select statement like select generate_series(1, 100000000) whose result will cause greater than 512MB of traffic.

As far as adjusting the meaning of require, what you have should work. However, setting ServerName is superfluous because InsecureSkipVerify causes it to be ignored.

Another option for handling TLS is to configure the tls.Config struct directly and tell pgx to use that. That gives the highest level of TLS control.

[johanbrandhorst]

I'm also running into this issue, and would appreciate it being solved. @jacobmarble do you intend to submit a PR to get this in?

[jacobmarble]

@johanbrandhorst I don't have cycles for a PR right now, so you go ahead. I have created a local modified copy of v3.0.1

Thanks for the context, @jackc, I'll see if we can upgrade RedShift.

[jacobmarble]

The issue has not been resolved in RedShift.

https://forums.aws.amazon.com/thread.jspa?threadID=229990

[johanbrandhorst]

I submitted #319 which contains the minimal changes discussed.

[jackc]

The recent merge resolves the sslmode issue. It still leaves the SSL renegotiation issue with RedShift. Not sure if/what should be done for it, but regardless, that should be a separate issue.