sslsni default support in 1.10.7 breaks verify-ca option

lib / pq

Pure Go Postgres driver for database/sql

https://pkg.go.dev/github.com/lib/pq

MIT License

8.86k stars 908 forks source link

sslsni default support in 1.10.7 breaks verify-ca option #1106

Open mszlgr opened 1 year ago

mszlgr commented 1 year ago

After updating from 1.10.6 to 1.10.7 I found that when using sslmode=verify-ca mode:

client.ConnectionState().ServerName in this part of code has value of host parameter from db connection string
few lines below it is used to verify leaf server cert - https://github.com/lib/pq/blob/master/ssl.go#L202 breaking verify-ca option, throwing x509: “*.<host>” certificate name does not match input

In version 1.10.6 sslVerifyCertificateAuthority function was using empty string in its place, ignoring verification of leaf certificate.

I would like to be able to use sslsni option and on same time use verify-ca option.

cbandy commented 1 year ago

The verify-ca option has always set DNSName during verify, but I agree, we don't want name verification in this function.

cbandy commented 1 year ago

The behavior of DNSName wasn't documented until Go 1.15, so I suspect it was required at some point.

It looks like another case for sslsni=1 might cover this at the end of TestSSLVerifyCA. https://github.com/lib/pq/blob/922c00e176fb3960d912dc2c7f67ea2cf18d27b0/ssl_test.go#L158-L159

johto commented 1 year ago

The verify-ca option has always set DNSName during verify, but I agree, we don't want name verification in this function.

I was just hit by this. Looks like verify-ca requires the CN to match now. What's the correct fix here? Not setting DNSName at all? Do you want me to open a PR? (Though I don't feel like I have enough context here to write a good commit message.)