Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert

On Fri, Jan 6, 2023 at 2:18 AM Jelte Fennema <postgres(at)jeltef(dot)nl> wrote:
>
> Huge +1 from me. On Azure we're already using public CAs to sign certificates for our managed postgres offerings[1][2]. Right now, our customers have to go to the hassle of downloading a specific root cert or finding their OS default location. Neither of these allow us to give users a simple copy-pastable connection string that uses secure settings. This would change this and make it much easier for our customers to use secure connections to their database.

Thanks! Timescale Cloud is in the same boat.

> I have two main questions:
> 1. From the rest of the thread it's not entirely clear to me why this patch goes for the sslrootcert=system approach, instead of changing what sslrootcert='' means when using verify-full. Like Tom Lane suggested, we could change it to try ~/.postgresql/root.crt and if that doesn't exist make it try the system store, instead of erroring out like it does now when ~/.postgresql/root.crt doesn't exist.

I mentioned it briefly upthread, but to expand: For something this
critical to security, I don't like solutions that don't say exactly
what they do. What does the following connection string mean?

$ psql 'host=example.org sslmode=verify-full'

If it sometimes means to use root.crt (if one exists) and sometimes to
use the system store, then
1) it's hard to audit the actual behavior without knowing the state of
the filesystem, and
2) if I want to connect to a server using the system store, and *only*
the system store, then I have to make sure that the default root.crt
never exists. But what if other software on my system relies on it?

It also provides a bigger target for exploit chains, because I can
remove somebody's root.crt file and their connections may try to
continue with an effectively weaker verification level instead of
erroring out. I realize that for many people this is a nonissue ("if
you can delete the root cert, you can probably do much worse") but IME
arbitrary file deletion vulnerabilities are treated with less concern
than arbitrary file writes.

By contrast,

$ psql 'host=example.org sslrootcert=system sslmode=verify-full'

has a clear meaning. We'll never use a root.crt.

(A downside of reusing sslrootcert like this is the cross-version
hazard. The connection string 'host=example.org sslrootcert=system'
means something strong with this patchset, but something very weak to
libpq 15 and prior. So clients should probably continue to specify
sslmode=verify-full explicitly for the foreseeable future.)

> This approach seems nicer to me, as it doesn't require introducing another special keyword.

Maybe I'm being overly aspirational, but one upside to that special
keyword is that it's a clear signal that the user wants to use the
public CA model. So we have the opportunity to remove footguns
aggressively when we see this mode. In the future we may have further
opportunities to strengthen sslrootcert=system (OCSP and/or
must-staple support?) that would be harder to roll out by default if
we're just trying to guess what the user wants.

> It would also remove the need for the changing of defaults depending on the value of sslrootcert.

Agreed. Personally I think the benefit of 0002 outweighs its cost, but
maybe there's a better way to implement it.

> 2. Should we allow the same approach with ssl_ca_file on the server side, for client cert validation?

I don't know enough about this use case to implement it safely. We'd
have to make sure the HBA entry is checking the hostname (so that we
do the reverse DNS dance), and I guess we'd need to introduce a new
clientcert verify-* mode? Also, it seems like server operators are
more likely to know exactly which roots they need, at least compared
to clients. I agree the feature is useful, but I'm not excited about
attaching it to this patchset.

--Jacob