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

2023-04-24 Thread Daniel Gustafsson
> On 21 Apr 2023, at 18:56, Jacob Champion wrote: > > On Fri, Apr 21, 2023 at 12:55 AM Daniel Gustafsson wrote: >> This has been done and the open item marked as completed. > > Thanks! Now that the weirdness is handled by the tests, I think we can > remove the Cirrus workaround. Something like

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

2023-04-21 Thread Jacob Champion
On Fri, Apr 21, 2023 at 12:55 AM Daniel Gustafsson wrote: > This has been done and the open item marked as completed. Thanks! Now that the weirdness is handled by the tests, I think we can remove the Cirrus workaround. Something like the attached, which passes the macOS Meson suite for me.

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

2023-04-21 Thread Daniel Gustafsson
> On 18 Apr 2023, at 14:32, Daniel Gustafsson wrote: > >> On 17 Apr 2023, at 18:20, Jacob Champion wrote: > >> Note that it won't fix the >> (completely different) misleading error message for OpenSSL 3.0, but >> since that's an *actively* unhelpful error message coming back from >> OpenSSL, I

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

2023-04-18 Thread Daniel Gustafsson
> On 17 Apr 2023, at 18:20, Jacob Champion wrote: > Note that it won't fix the > (completely different) misleading error message for OpenSSL 3.0, but > since that's an *actively* unhelpful error message coming back from > OpenSSL, I don't think we want to override it. Agreed, the best we can do

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

2023-04-17 Thread Jacob Champion
On Fri, Apr 14, 2023 at 3:36 PM Daniel Gustafsson wrote: > This "error: Success" error has been reported to the list numerous times as > misleading, and I'd love to make progress on improving error reporting during > the v17 cycle. Agreed! > The attached checks for the specific known error, and

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

2023-04-14 Thread Daniel Gustafsson
> On 14 Apr 2023, at 19:34, Jacob Champion wrote: > > On Fri, Apr 14, 2023 at 7:20 AM Daniel Gustafsson wrote: >> And again with the attachment. > > After some sleep... From inspection I think the final EOF branch could > be masked by the new branch, if verification has failed but was already

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

2023-04-14 Thread Jacob Champion
On Fri, Apr 14, 2023 at 7:20 AM Daniel Gustafsson wrote: > And again with the attachment. After some sleep... From inspection I think the final EOF branch could be masked by the new branch, if verification has failed but was already ignored. To test that, I tried hanging up on the client

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

2023-04-14 Thread Daniel Gustafsson
> On 14 Apr 2023, at 16:20, Daniel Gustafsson wrote: > >> On 14 Apr 2023, at 15:51, Tom Lane wrote: >> >> Daniel Gustafsson writes: >>> I mainly put save_errno back into SOCK_ERRNO for greppability, I don't have >>> any >>> strong opinions either way so I went with the latter suggestion.

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

2023-04-14 Thread Daniel Gustafsson
> On 14 Apr 2023, at 15:51, Tom Lane wrote: > > Daniel Gustafsson writes: >> I mainly put save_errno back into SOCK_ERRNO for greppability, I don't have >> any >> strong opinions either way so I went with the latter suggestion. Attached v3 >> does the above change and passes the tests both

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

2023-04-14 Thread Tom Lane
Daniel Gustafsson writes: > I mainly put save_errno back into SOCK_ERRNO for greppability, I don't have > any > strong opinions either way so I went with the latter suggestion. Attached v3 > does the above change and passes the tests both with a broken and working > system CA pool. Unless

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

2023-04-14 Thread Daniel Gustafsson
> On 14 Apr 2023, at 01:27, Tom Lane wrote: > > Daniel Gustafsson writes: >> Good points, it should of course be SOCK_ERRNO. The attached saves off errno >> and reinstates it to avoid clobbering. Will test it on Windows in the >> morning >> as well. > > I think instead of this: > > +

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

2023-04-13 Thread Tom Lane
Daniel Gustafsson writes: > Good points, it should of course be SOCK_ERRNO. The attached saves off errno > and reinstates it to avoid clobbering. Will test it on Windows in the morning > as well. I think instead of this: +SOCK_ERRNO_SET(save_errno); you could just do

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

2023-04-13 Thread Daniel Gustafsson
> On 14 Apr 2023, at 00:52, Tom Lane wrote: > > Daniel Gustafsson writes: >> The attached diff passes the tests on OpenSSL 1.0.1 through 3.1 as well as on >> LibreSSL. Thoughts? > > 1. You can't assume that errno starts out zero, unless you zero it > right before SSL_connect. Maybe we should

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

2023-04-13 Thread Tom Lane
Daniel Gustafsson writes: > The attached diff passes the tests on OpenSSL 1.0.1 through 3.1 as well as on > LibreSSL. Thoughts? 1. You can't assume that errno starts out zero, unless you zero it right before SSL_connect. 2. I wonder whether it's safe to assume that errno (a/k/a SOCK_ERRNO)

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

2023-04-13 Thread Daniel Gustafsson
> On 13 Apr 2023, at 18:42, Daniel Gustafsson wrote: > Regarding the thread; I hope to have a suggestion for a way forward regarding > the open issue later tonight. After reading OpenSSL code and documentation, I think the simplest solution is to explicitly check for X509 errors when OpenSSL

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

2023-04-13 Thread Daniel Gustafsson
> On 13 Apr 2023, at 18:39, Tom Lane wrote: > > Daniel Gustafsson writes: >>> On 12 Apr 2023, at 22:23, Tom Lane wrote: >>> 2. Run a second BF animal that's intentionally pointed at the MacPorts >>> environment, in hopes of testing what MacPorts users would see. > >> I think #2 would be a

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

2023-04-13 Thread Tom Lane
Daniel Gustafsson writes: >> On 12 Apr 2023, at 22:23, Tom Lane wrote: >> 2. Run a second BF animal that's intentionally pointed at the MacPorts >> environment, in hopes of testing what MacPorts users would see. > I think #2 would be a good addition. Most won't build OpenSSL themselves so >

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

2023-04-12 Thread Tom Lane
Daniel Gustafsson writes: > On 12 Apr 2023, at 22:23, Tom Lane wrote: >> Plausible alternatives include: >> 2. Run a second BF animal that's intentionally pointed at the MacPorts >> environment, in hopes of testing what MacPorts users would see. >> >> #2 feels like it might not be a waste of

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

2023-04-12 Thread Greg Stark
On Wed, Apr 12, 2023, 19:30 Daniel Gustafsson wrote: > > The issue we have is that we cannot get PGconn in > verify_cb so logging an error is tricky. Hm, the man page talks about a "ex_data mechanism" which seems to be referring to this Rube Goldberg device

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

2023-04-12 Thread Daniel Gustafsson
> On 12 Apr 2023, at 23:46, Daniel Gustafsson wrote: >> On 12 Apr 2023, at 23:40, Tom Lane wrote: >> So this smells to me like a new OpenSSL bug: they should tolerate >> a missing certs dir like they used to. Who wants to file it? > > They are specifying that: "A missing default location is

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

2023-04-12 Thread Daniel Gustafsson
> On 12 Apr 2023, at 23:40, Tom Lane wrote: > > Peter Eisentraut writes: >> On 12.04.23 22:52, Jacob Champion wrote: >>> Does the test start passing if you create an empty certs directory? It >>> still wouldn't explain why Daniel's setup is succeeding... > >> After >> mkdir

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

2023-04-12 Thread Tom Lane
Peter Eisentraut writes: > On 12.04.23 22:52, Jacob Champion wrote: >> Does the test start passing if you create an empty certs directory? It >> still wouldn't explain why Daniel's setup is succeeding... > After > mkdir /usr/local/etc/openssl@3/certs > the tests pass! Likewise, though MacPorts

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

2023-04-12 Thread Peter Eisentraut
On 12.04.23 22:52, Jacob Champion wrote: It surprises me that you can get a successful test with a missing certs directory. If I remove the workaround in Cirrus, I get the following error, which looks the same to me: [20:40:00.253](0.000s) not ok 121 - sslrootcert=system does not connect

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

2023-04-12 Thread Tom Lane
Oh! I was a little behind on MacPorts updates, and after pulling the latest (taking their openssl from 3.0.8 to 3.1.0) I can duplicate Peter's problem: # +++ tap check in src/test/ssl +++ t/001_ssltests.pl .. 120/? # Failed test 'sslrootcert=system does not connect with private CA: matches' #

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

2023-04-12 Thread Jacob Champion
(Peter, your emails are being redirected to spam for me, FYI. Something about messagingengine.) On Wed, Apr 12, 2023 at 12:57 PM Daniel Gustafsson wrote: > > On 12 Apr 2023, at 21:43, Peter Eisentraut > > wrote: > > On 12.04.23 18:54, Jacob Champion wrote: > >> Peter, you should have a

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

2023-04-12 Thread Daniel Gustafsson
> On 12 Apr 2023, at 22:23, Tom Lane wrote: > > Daniel Gustafsson writes: >> We don't have great coverage of macOS in the buildfarm sadly, I wonder if can >> get sifaka to run the SSL tests if we ask nicely? > > I was just looking into that, but it seems like it'd be a mess. > > I have a

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

2023-04-12 Thread Tom Lane
Daniel Gustafsson writes: > We don't have great coverage of macOS in the buildfarm sadly, I wonder if can > get sifaka to run the SSL tests if we ask nicely? I was just looking into that, but it seems like it'd be a mess. I have a modern openssl installation from MacPorts, but if I try to

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

2023-04-12 Thread Daniel Gustafsson
> On 12 Apr 2023, at 21:43, Peter Eisentraut > wrote: > > On 12.04.23 18:54, Jacob Champion wrote: >> Peter, you should have a .../etc/openssl@3/certs directory somewhere >> in your Homebrew installation prefix -- do you, or has Homebrew >> removed it by mistake? > > I don't have that, but I

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

2023-04-12 Thread Peter Eisentraut
On 12.04.23 18:54, Jacob Champion wrote: Peter, you should have a .../etc/openssl@3/certs directory somewhere in your Homebrew installation prefix -- do you, or has Homebrew removed it by mistake? I don't have that, but I don't have it for openssl@1.1 either. I have ~$ ll

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

2023-04-12 Thread Jacob Champion
On Wed, Apr 12, 2023 at 2:24 AM Daniel Gustafsson wrote: > > On 12 Apr 2023, at 09:11, Peter Eisentraut > > wrote: > > # Failed test 'sslrootcert=system does not connect with private CA: > > matches' > > # at t/001_ssltests.pl line 479. > > # 'psql: error: connection to

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

2023-04-12 Thread Daniel Gustafsson
> On 12 Apr 2023, at 09:11, Peter Eisentraut > wrote: > > On 05.04.23 23:29, Jacob Champion wrote: >> On Wed, Apr 5, 2023 at 2:27 PM Daniel Gustafsson wrote: >>> I squashed and pushed v10 with a few small comment tweaks for typos and some >>> pgindenting. Thanks! >> Thank you very much! > >

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

2023-04-12 Thread Daniel Gustafsson
> On 12 Apr 2023, at 09:11, Peter Eisentraut > wrote: > > On 05.04.23 23:29, Jacob Champion wrote: >> On Wed, Apr 5, 2023 at 2:27 PM Daniel Gustafsson wrote: >>> I squashed and pushed v10 with a few small comment tweaks for typos and some >>> pgindenting. Thanks! >> Thank you very much! > >

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

2023-04-12 Thread Peter Eisentraut
On 05.04.23 23:29, Jacob Champion wrote: On Wed, Apr 5, 2023 at 2:27 PM Daniel Gustafsson wrote: I squashed and pushed v10 with a few small comment tweaks for typos and some pgindenting. Thanks! Thank you very much! This patch (8eda731465) makes the ssl tests fail for me: not ok 121 -

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

2023-04-05 Thread Jacob Champion
On Wed, Apr 5, 2023 at 2:27 PM Daniel Gustafsson wrote: > I squashed and pushed v10 with a few small comment tweaks for typos and some > pgindenting. Thanks! Thank you very much! --Jacob

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

2023-04-05 Thread Daniel Gustafsson
> On 3 Apr 2023, at 23:09, Jacob Champion wrote: > > On Mon, Apr 3, 2023 at 12:40 PM Daniel Gustafsson wrote: >> Doh, sorry, my bad. I read and wrote 1.0.1 but was thinking about 1.0.2. >> You >> are right, in 1.0.1 that API does not exist. I'm not all too concerned with >> skipping this

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

2023-04-03 Thread Jacob Champion
On Mon, Apr 3, 2023 at 12:40 PM Daniel Gustafsson wrote: > Doh, sorry, my bad. I read and wrote 1.0.1 but was thinking about 1.0.2. You > are right, in 1.0.1 that API does not exist. I'm not all too concerned with > skipping this tests on OpenSSL versions that by the time 16 ships are 6 years

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

2023-04-03 Thread Daniel Gustafsson
> On 3 Apr 2023, at 21:04, Jacob Champion wrote: > > On Sun, Apr 2, 2023 at 1:36 PM Daniel Gustafsson wrote: >>> On 31 Mar 2023, at 19:59, Jacob Champion wrote: >>> I can make that change; note that it'll also skip some of the new tests >>> with OpenSSL 1.0.1, where there's no

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

2023-04-03 Thread Jacob Champion
On Sun, Apr 2, 2023 at 1:36 PM Daniel Gustafsson wrote: > > On 31 Mar 2023, at 19:59, Jacob Champion wrote: > > I can make that change; note that it'll also skip some of the new tests > > with OpenSSL 1.0.1, where there's no SSL_CTX_set_cert_cb. If that's > > acceptable, it should be an easy

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

2023-04-02 Thread Daniel Gustafsson
> On 31 Mar 2023, at 19:59, Jacob Champion wrote: >> + # Let tests differentiate between vanilla OpenSSL and LibreSSL. >> + AC_CHECK_DECLS([LIBRESSL_VERSION_NUMBER], [], [], [#include >> ]) >> We have a check for SSL_CTX_set_cert_cb which is specifically done since it's >> not present in

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

2023-03-31 Thread Jacob Champion
On 3/31/23 02:14, Daniel Gustafsson wrote: >> On 14 Mar 2023, at 20:20, Jacob Champion wrote: > >> Rebased over yesterday's Meson changes in v8. > > I had a look at this and agree that it's something we should do. Great, thanks for the review! > + # Let tests differentiate between vanilla

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

2023-03-31 Thread Daniel Gustafsson
> On 14 Mar 2023, at 20:20, Jacob Champion wrote: > Rebased over yesterday's Meson changes in v8. I had a look at this and agree that it's something we should do. The patch seems quite close to committable, I just have a few comments on it: + # Let tests differentiate between vanilla OpenSSL

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

2023-03-14 Thread Jacob Champion
On Tue, Mar 14, 2023 at 11:01 AM Gregory Stark (as CFM) wrote: > It does look like a rebase for meson.build would be helpful. I'm not > marking it waiting on author just for meson.build conflicts but it > would be perhaps more likely to be picked up by a committer if it's > showing green in

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

2023-03-14 Thread Gregory Stark (as CFM)
It does look like a rebase for meson.build would be helpful. I'm not marking it waiting on author just for meson.build conflicts but it would be perhaps more likely to be picked up by a committer if it's showing green in cfbot.

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

2023-02-28 Thread Jacob Champion
On 2/16/23 10:38, Jacob Champion wrote: > Thanks for the nudge, v7 rebases over the configure conflict from 9244c11afe. I think/hope this is well-baked enough for a potential commit this CF, so I've adjusted the target version. Let me know if there are any concerns about the approach. Thanks!

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

2023-02-16 Thread Jacob Champion
On Thu, Feb 16, 2023 at 1:35 AM Jelte Fennema wrote: > > FYI the last patch does not apply cleanly anymore. So a rebase is needed. Thanks for the nudge, v7 rebases over the configure conflict from 9244c11afe. --Jacob 1: e7e2d43b18 ! 1: b07af1c564 libpq: add sslrootcert=system to use default

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

2023-02-16 Thread Jelte Fennema
FYI the last patch does not apply cleanly anymore. So a rebase is needed.

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

2023-01-11 Thread Magnus Hagander
On Wed, Jan 11, 2023 at 8:06 PM Jacob Champion wrote: > On Wed, Jan 11, 2023 at 10:23 AM Magnus Hagander > wrote: > > Sorry to jump in (very) late in this game. So first, I like this general > approach :) > > Thanks! > > > It feels icky to have to add configure tests just to make a test work. >

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

2023-01-11 Thread Jacob Champion
On Wed, Jan 11, 2023 at 10:23 AM Magnus Hagander wrote: > Sorry to jump in (very) late in this game. So first, I like this general > approach :) Thanks! > It feels icky to have to add configure tests just to make a test work. But I > guess there isn't really a way around that if we want to

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

2023-01-11 Thread Magnus Hagander
On Wed, Jan 11, 2023 at 6:27 PM Jacob Champion wrote: > On Wed, Jan 11, 2023 at 6:37 AM Jelte Fennema wrote: > > > > LGTM. As far as I can tell this is ready for a committer. > > Thanks for the reviews! > Sorry to jump in (very) late in this game. So first, I like this general approach :) It

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

2023-01-11 Thread Jacob Champion
On Wed, Jan 11, 2023 at 6:37 AM Jelte Fennema wrote: > > LGTM. As far as I can tell this is ready for a committer. Thanks for the reviews! --Jacob

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

2023-01-11 Thread Jelte Fennema
LGTM. As far as I can tell this is ready for a committer. On Wed, 11 Jan 2023 at 00:15, Jacob Champion wrote: > > On Mon, Jan 9, 2023 at 7:07 AM Jelte Fennema wrote: > > I also took a closer look at the code, and the only comment I have is: > > > > > appendPQExpBuffer(>errorMessage, > > > >

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

2023-01-10 Thread Jacob Champion
On Mon, Jan 9, 2023 at 7:07 AM Jelte Fennema wrote: > I also took a closer look at the code, and the only comment I have is: > > > appendPQExpBuffer(>errorMessage, > > These calls can all be replaced by the recently added libpq_append_conn_error Argh, thanks for the catch. Fixed. > Finally I

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

2023-01-10 Thread Jacob Champion
On Mon, Jan 9, 2023 at 7:40 AM Andrew Dunstan wrote: > I'm confused. A client cert might not have a hostname at all, and isn't > used to verify the connecting address, but to verify the username. It > needs to have a CN/DN equal to the user name of the connection, or that > maps to that name via

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

2023-01-09 Thread Andrew Dunstan
On 2023-01-09 Mo 10:07, Jelte Fennema wrote: > Thanks for clarifying your reasoning. I now agree that ssrootcert=system > is now the best option. Cool, that looks like a consensus. > >>> 2. Should we allow the same approach with ssl_ca_file on the server side, >>> for client cert

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

2023-01-09 Thread Jelte Fennema
Thanks for clarifying your reasoning. I now agree that ssrootcert=system is now the best option. > > 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

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

2023-01-06 Thread Jacob Champion
On Fri, Jan 6, 2023 at 2:18 AM Jelte Fennema 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

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

2023-01-06 Thread Andrew Dunstan
On 2023-01-06 Fr 09:28, Jelte Fennema wrote: >> One reason might be that it doesn't give you any way not to fall back on >> the system store. > To not fall back to the system store you could still provide the exact path > to the CA cert file. I guess. I don't have strong feelings one way or

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

2023-01-06 Thread Jelte Fennema
> One reason might be that it doesn't give you any way not to fall back on > the system store. To not fall back to the system store you could still provide the exact path to the CA cert file. > +1 for doing this, although I think client certs are less likely to have > been issued by a public CA.

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

2023-01-06 Thread Andrew Dunstan
On 2023-01-06 Fr 05:17, Jelte Fennema 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.

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

2023-01-06 Thread Jelte Fennema
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

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

2023-01-03 Thread Jacob Champion
On Thu, Dec 8, 2022 at 3:10 PM Jacob Champion wrote: > For now, it's worked around in v4. This should finally get the cfbot > fully green. Cirrus's switch to M1 Macs changed the Homebrew installation path, so v5 adjusts the workaround accordingly. --Jacob 1: b01812f604 ! 1: 7618037c86 libpq:

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

2022-12-08 Thread Jacob Champion
On Mon, Dec 5, 2022 at 10:53 AM Jacob Champion wrote: > We are not the first using Homebrew to run into this, and best I can > tell, it is a brew-specific bug. The certificate directory that's been > configured isn't actually installed by the formula. (A colleague here > was able to verify the

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

2022-12-05 Thread Jacob Champion
On Fri, Dec 2, 2022 at 9:58 AM Jacob Champion wrote: > Thanks for the nudge -- running with OpenSSL 3.0.7 in CI did not fix > the issue. I suspect a problem with our error stack handling... It is a problem with the error queue, but *whose* problem is probably up for debate. The queue looks like

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

2022-12-02 Thread Jacob Champion
On Thu, Dec 1, 2022 at 9:26 PM Michael Paquier wrote: > On Mon, Nov 07, 2022 at 05:04:14PM -0800, Jacob Champion wrote: > > The macOS/OpenSSL 3.0.0 failure is still unfixed. > > Err, could you look at that? I am switching the patch as waiting on > author. Thanks for the nudge -- running with

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

2022-12-01 Thread Michael Paquier
On Mon, Nov 07, 2022 at 05:04:14PM -0800, Jacob Champion wrote: > Done. sslrootcert=system now prevents you from explicitly setting a > weaker sslmode, to try to cement it as a Do What I Mean sort of > feature. If you need something weird then you can still jump through > the hoops by setting

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

2022-11-07 Thread Jacob Champion
On Thu, Nov 3, 2022 at 4:39 PM Jacob Champion wrote: > There is an additional test failure with LibreSSL, which doesn't appear > to honor the SSL_CERT_FILE environment variable. This isn't a problem in > production -- if you're using LibreSSL, you'd presumably understand that > you can't use that

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

2022-11-03 Thread Jacob Champion
On Tue, Nov 1, 2022 at 10:55 AM Jacob Champion wrote: > On Tue, Nov 1, 2022 at 10:03 AM Jacob Champion > wrote: > > I'm not familiar with "unregistered scheme" in this context and will > > need to dig in. > > Unfortunately I can't reproduce with 3.0.0 on Ubuntu :( Sorry, when rereading my own

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

2022-11-01 Thread Jacob Champion
On Tue, Nov 1, 2022 at 10:03 AM Jacob Champion wrote: > I'm not familiar with "unregistered scheme" in this context and will > need to dig in. Unfortunately I can't reproduce with 3.0.0 on Ubuntu :( I'm suspicious that it may be related to [1], in which case the problem might be fixed by

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

2022-11-01 Thread Jacob Champion
On Tue, Nov 1, 2022 at 5:30 AM wrote: > Sweet. I just created an account with username `habets`. Added! OpenSSL 3.0.0 doesn't get along with one of my new tests: # Failed test 'sslrootcert=system does not connect with private CA: matches' # at

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

2022-11-01 Thread thomas
On Mon, 31 Oct 2022 23:36:16 +, Jacob Champion said: >> I wanted to get feedback on the approach before wordsmithing too >> much. > > I've added this to tomorrow's CF [1]. Thomas, if you get (or already > have) a PG community username, I can add you as an author. Sweet. I just created an

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

2022-10-31 Thread Jacob Champion
On Tue, Oct 25, 2022 at 1:20 PM Jacob Champion wrote: > I wanted to get feedback on the approach before wordsmithing too > much. I've added this to tomorrow's CF [1]. Thomas, if you get (or already have) a PG community username, I can add you as an author. Thanks, --Jacob [1]

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

2022-10-25 Thread Jacob Champion
On Tue, Oct 25, 2022 at 7:26 AM Andrew Dunstan wrote: > I don't find too much difficulty in having one option's default depend > on another's value, as long as it's documented. My patch is definitely missing the documentation for that part right now; I wanted to get feedback on the approach

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

2022-10-25 Thread Jacob Champion
On Tue, Oct 25, 2022 at 4:01 AM wrote: > Yeah I agree that not forcing verify-full when using system CAs is a > giant foot-gun, and many will stop configuring just until it works. > > Is there any argument for not checking hostname when using a CA pool > for which literally anyone can create a

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

2022-10-25 Thread Andrew Dunstan
On 2022-10-25 Tu 07:01, tho...@habets.se wrote: > On Tue, 25 Oct 2022 01:03:23 +0100, Jacob Champion > said: >> I'd like to try to get this conversation started again. To pique >> interest I've attached a new version of 0001, which implements >> `sslrootcert=system` instead as suggested

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

2022-10-25 Thread thomas
On Tue, 25 Oct 2022 01:03:23 +0100, Jacob Champion said: > I'd like to try to get this conversation started again. To pique > interest I've attached a new version of 0001, which implements > `sslrootcert=system` instead as suggested upthread. In 0002 I went > further and switched the default

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

2022-10-24 Thread Jacob Champion
On Mon, Oct 4, 2021 at 9:14 PM Bruce Momjian wrote: > On Tue, Sep 28, 2021 at 02:54:39AM -0700, tho...@habets.se wrote: > > And you say for complex setups. Fair enough. But currently I'd say the > > default is wrong, and what should be default is not configurable. > > Agreed, I think this needs

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

2021-10-04 Thread Bruce Momjian
On Tue, Sep 28, 2021 at 02:54:39AM -0700, tho...@habets.se wrote: > On Tue, 28 Sep 2021 02:09:11 +0100, Bruce Momjian said: > > I don't think public CA's are not a good idea for complex setups since > > they open the ability for an external party to create certificates that > > are trusted by

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

2021-09-28 Thread thomas
On Tue, 28 Sep 2021 02:09:11 +0100, Bruce Momjian said: > I don't think public CA's are not a good idea for complex setups since > they open the ability for an external party to create certificates that > are trusted by your server's CA, e.g., certificate authentication. I'm not arguing for, and

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

2021-09-27 Thread Bruce Momjian
On Tue, Sep 7, 2021 at 12:58:44PM -0400, Tom Lane wrote: > Yeah, that would mostly fix the usability concern. I guess what it > comes down to is whether you think that public or private certs are > likely to be the majority use-case in the long run. The shortage of > previous requests for this

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

2021-09-22 Thread Daniel Gustafsson
> On 22 Sep 2021, at 20:59, Andrew Dunstan wrote: > I think we need to be consistent on this. NSS builds and OpenSSL builds > should act the same, mutatis mutandis. I 100% agree. Different TLS backends should be able use different truststores etc but once the server is running they must be

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

2021-09-22 Thread Andrew Dunstan
On 9/22/21 2:36 PM, Jacob Champion wrote: > On Sat, 2021-09-18 at 14:20 +0200, Cameron Murdoch wrote: >> Having sslrootcert use the system trust store if >> ~/.postgresql/root.crt doesn’t exist would seem like a good change. > Fallback behavior can almost always be exploited given the right >

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

2021-09-22 Thread Jacob Champion
On Sat, 2021-09-18 at 14:20 +0200, Cameron Murdoch wrote: > Having sslrootcert use the system trust store if > ~/.postgresql/root.crt doesn’t exist would seem like a good change. Fallback behavior can almost always be exploited given the right circumstances. IMO, if I've told psql to use a root

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

2021-09-19 Thread Andrew Dunstan
On 9/17/21 5:35 PM, Greg Stark wrote: > Hm. Let's Encrypt's FAQ tells me I'm on the right track with that > question but the distinctinos are far more coarse than I was worried > about: > > > Does Let’s Encrypt issue certificates for anything other than SSL/TLS > for websites? > > Let’s Encrypt

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

2021-09-18 Thread Cameron Murdoch
On Sat, 18 Sep 2021 at 12:57, Thomas Habets wrote: > > But these are two changes: > 1. Actually verify against a CA > 2. Actually check the CN/altnames > > Anything short of "verify-full" is in my view "not checking". Even with a > private CA this allows for a lot of lateral movement in an org,

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

2021-09-18 Thread Thomas Habets
On Sat, 18 Sept 2021 at 00:10, Cameron Murdoch wrote: > I also agree that the proposed patch is not the right way to go as it is > essentially the same as verify-full, and I think that the correct fix would > be to change the default. > But these are two changes: 1. Actually verify against a CA

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

2021-09-17 Thread Cameron Murdoch
Hi, I manage a bunch of Postgres servers at Oslo University and we use real ssl certs on all our servers. I was actually really surprised to discover that the libpq default is sslmode=require and that the root cert defaults to a file under the user’s home directory. I have been planning to use

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

2021-09-17 Thread Greg Stark
Hm. Let's Encrypt's FAQ tells me I'm on the right track with that question but the distinctinos are far more coarse than I was worried about: Does Let’s Encrypt issue certificates for anything other than SSL/TLS for websites? Let’s Encrypt certificates are standard Domain Validation

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

2021-09-17 Thread Tom Lane
Greg Stark writes: > However I have a different question. Are the system certificates > intended or general purpose certificates? Do they have their intended > uses annotated on the certificates? Does SSL Verification have any > logic deciding which certificates are appropriate for signing

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

2021-09-17 Thread Greg Stark
On Tue, 7 Sept 2021 at 12:59, Tom Lane wrote: > > I guess what it > comes down to is whether you think that public or private certs are > likely to be the majority use-case in the long run. The shortage of > previous requests for this feature says that right now, just about > everyone is using

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

2021-09-07 Thread Tom Lane
Andrew Dunstan writes: > On 9/7/21 11:47 AM, Tom Lane wrote: >> so I'm coming around to the idea >> that we need to do something. I don't like the details of Thomas' >> proposal though; specifically I don't see a need to invent a new sslmode >> value. I think it should just be "if

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

2021-09-07 Thread Andrew Dunstan
On 9/7/21 11:47 AM, Tom Lane wrote: > > This is not how I supposed it worked, That happens to me more than I usually admit -) > so I'm coming around to the idea > that we need to do something. I don't like the details of Thomas' > proposal though; specifically I don't see a need to invent

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

2021-09-07 Thread Tom Lane
Andrew Dunstan writes: > You don't have to copy anything to achieve what you want. Just set the > sslrootcert parameter of your connection to point to the system file. e.g. > psql "sslmode=verify-full > sslrootcert=/etc/pki/ca-trust/extracted/openssl/ca-bundle.trust.crt ..." While that does

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

2021-09-07 Thread Andrew Dunstan
On 9/7/21 10:57 AM, tho...@habets.se wrote: > On Tue, 7 Sep 2021 15:16:51 +0100, Andrew Dunstan said: >> can't you specify a CA cert in the system's >> CA store if necessary? e.g. on my Fedora system I think it's >> /etc/pki/ca-trust/extracted/openssl/ca-bundle.trust.crt > I could, but that

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

2021-09-07 Thread Tom Lane
tho...@habets.se writes: > On Tue, 7 Sep 2021 15:16:51 +0100, Andrew Dunstan said: >> can't you specify a CA cert in the system's >> CA store if necessary? e.g. on my Fedora system I think it's >> /etc/pki/ca-trust/extracted/openssl/ca-bundle.trust.crt > I could, but that seems more like a

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

2021-09-07 Thread thomas
On Tue, 7 Sep 2021 15:16:51 +0100, Andrew Dunstan said: > can't you specify a CA cert in the system's > CA store if necessary? e.g. on my Fedora system I think it's > /etc/pki/ca-trust/extracted/openssl/ca-bundle.trust.crt I could, but that seems more like a workaround, where I have to change

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

2021-09-07 Thread Andrew Dunstan
On 9/6/21 6:21 PM, tho...@habets.se wrote: > On Mon, 6 Sep 2021 20:47:37 +0100, Tom Lane said: >> I'm confused by your description of this patch. AFAIK, OpenSSL verifies >> against the system-wide CA pool by default. Why do we need to do >> anything? > Experimentally, no it doesn't. Or if it

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

2021-09-06 Thread thomas
On Mon, 6 Sep 2021 20:47:37 +0100, Tom Lane said: > I'm confused by your description of this patch. AFAIK, OpenSSL verifies > against the system-wide CA pool by default. Why do we need to do > anything? Experimentally, no it doesn't. Or if it does, then it doesn't verify the CN/altnames of the

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

2021-09-06 Thread Tom Lane
Thomas Habets writes: > With Letsencrypt now protecting web servers left and right, and it makes > sense to me to just re-use the cert that the server may already have > installed. I'm confused by your description of this patch. AFAIK, OpenSSL verifies against the system-wide CA pool by

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

2021-09-06 Thread Thomas Habets
With Letsencrypt now protecting web servers left and right, and it makes sense to me to just re-use the cert that the server may already have installed. I've tested this on debian with the client compiled from the master branch, against a 13.3 server. This is my first patch to postgresql, so I