Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-02-20 Thread Jim Jones
I'm withdrawing this patch, as the same feature was already implemented 
in a different patch written by Jacob[1]


Thanks everyone!

Best, Jim

1- 
https://www.postgresql.org/message-id/flat/caawbhmi4v9zeavfuscdfx1por3zwrv9fuxkv_2marqvyc-m...@mail.gmail.com#199c1f49fbefa6be401db35f5cfa7742




smime.p7s
Description: S/MIME Cryptographic Signature


Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-30 Thread Jacob Champion
On Sun, Jan 29, 2023 at 5:02 AM Jim Jones  wrote:
> On 27.01.23 21:13, Cary Huang wrote:
>  > But, if the server does request clientcert but client uses
> "sslcertmode=disable" to connect and not give a certificate, it would
> also result in authentication failure. In this case, we actually would
> want to ignore "sslcertmode=disable" and send default certificates if
> found.
>
> I'm just wondering if this is really necessary. If the server asks for a
> certificate and the user explicitly says "I don't want to send it",
> shouldn't it be ok for the server return an authentication failure? I
> mean, wouldn't it defeat the purpose of "sslcertmode=disable"?

+1. In my opinion, if I tell libpq not to share my certificate with
the server, and it then fails to authenticate, that's intended and
useful behavior. (I don't really want libpq to try to find more ways
to authenticate me; that causes other security issues [1, 2].)

--Jacob

[1] 
https://www.postgresql.org/message-id/0adf992619e7bf138eb4119622d37e3efb6515d5.camel%40j-davis.com
[2] https://www.postgresql.org/message-id/46562.1637695110%40sss.pgh.pa.us




Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-30 Thread Jacob Champion
On Fri, Jan 27, 2023 at 12:13 PM Cary Huang  wrote:
> > (Eventually I'd like to teach the server not to ask for a client
> >  certificate if it's not going to use it.)
>
> If clientcert is not requested by the server, but yet the client still
> sends the certificate, the server will still verify it. This is the case
> in this discussion.

I think this is maybe conflating the application-level behavior with
the protocol-level behavior. A client certificate is requested by the
server if ssl_ca_file is set, whether clientcert is set in the HBA or
not. It's this disconnect between the intuitive behavior and the
actual behavior that I'd like to eventually improve.

--Jacob




Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-29 Thread Jim Jones


On 27.01.23 21:13, Cary Huang wrote:

I agree that it is a more elegant approach to add 
"sslcertmode=disable" on the client side to prevent sending default 
certificate.


But, if the server does request clientcert but client uses 
"sslcertmode=disable" to connect and not give a certificate, it would 
also result in authentication failure. In this case, we actually would 
want to ignore "sslcertmode=disable" and send default certificates if 
found.


Those are all very good points.

> But, if the server does request clientcert but client uses 
"sslcertmode=disable" to connect and not give a certificate, it would 
also result in authentication failure. In this case, we actually would 
want to ignore "sslcertmode=disable" and send default certificates if 
found.


I'm just wondering if this is really necessary. If the server asks for a 
certificate and the user explicitly says "I don't want to send it", 
shouldn't it be ok for the server return an authentication failure? I 
mean, wouldn't it defeat the purpose of "sslcertmode=disable"? Although 
it might be indeed quite handy I'm not sure how I feel about explicitly 
telling the client to not send a certificate and having it being sent 
anyway :)


Best, Jim



smime.p7s
Description: S/MIME Cryptographic Signature


Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-27 Thread Cary Huang

> I think the sslcertmode=disable option that I introduced in [1] solves
> this issue too; would it work for your case? That whole patchset is
> meant to tackle the general case of the problem you've described.
>
> (Eventually I'd like to teach the server not to ask for a client
>  certificate if it's not going to use it.)

there is an option in pg_hba.conf on the server side called "clientcert" 
that can be specified besides the auth method that controls if certain 
client connections are required to send client certificate for 
additional verification. The value of "clientcert" can be "verify-ca" or 
"verify-full". For example:


hostssl    all all 127.0.0.1/32 md5 
clientcert=verify-full


If clientcert is not requested by the server, but yet the client still 
sends the certificate, the server will still verify it. This is the case 
in this discussion.


I agree that it is a more elegant approach to add "sslcertmode=disable" 
on the client side to prevent sending default certificate.


But, if the server does request clientcert but client uses 
"sslcertmode=disable" to connect and not give a certificate, it would 
also result in authentication failure. In this case, we actually would 
want to ignore "sslcertmode=disable" and send default certificates if found.


It would perhaps to better change the parameter to 
"defaultclientcert=on-demand" on the client side that will:


1. not send the existing default certificate if server does not request 
a certificate
2. send the existing default certificate if server does request a 
certificate while the client does not use "sslcert" parameter to specify 
another non-default certificate


I put "default" in the parameter name to indicate that it only applies 
to default certificate. If user specifies a non-default certificate 
using "sslcert" parameter, "defaultclientcert" should not be used and 
client should give error if both exists.



Cary Huang

HighGo Software Canada
www.highgo.ca






Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-25 Thread Israel Barth Rubio
Hello Jacob,

> I'm not sure how helpful it is to assign "blame" here. I think the
> requested improvement is reasonable -- it should be possible to
> override the default for a particular connection, without having to
> pick a junk value that you hope doesn't match up with an actual file
> on the disk.

Right, I agree we can look for improvements. "blame" was likely
not the best word to express myself in that message.

> sslmode=disable isn't used in either of our proposals, though. Unless
> I'm missing what you mean?

Sorry about the noise, I misread the code snippet shared earlier
(sslmode x sslcertmode). I just took a closer read at the previously
mentioned patch about sslcertmode and it seems a bit
more elegant way of achieving something similar to what has
been proposed here.

Best regards,
Israel.

Em qua., 25 de jan. de 2023 às 14:09, Jacob Champion <
jchamp...@timescale.com> escreveu:

> On Wed, Jan 25, 2023 at 7:47 AM Israel Barth Rubio
>  wrote:
> > I imagine more people might have already hit a similar situation too.
> While the
> > workaround can seem a bit weird, in my very humble opinion the
> user/client is
> > somehow still the one to blame in this case as it is providing the
> "wrong" file in
> > a path that is checked by libpq. With that in mind I would be inclined
> to say it is
> > an acceptable workaround.
>
> I'm not sure how helpful it is to assign "blame" here. I think the
> requested improvement is reasonable -- it should be possible to
> override the default for a particular connection, without having to
> pick a junk value that you hope doesn't match up with an actual file
> on the disk.
>
> > Although both patches achieve a similar goal regarding not sending the
> > client certificate there is still a slight but in my opinion important
> difference
> > between them: sslmode=disable will also disable channel encryption. It
> > may or may not be acceptable depending on how the connection is between
> > your client and the server.
>
> sslmode=disable isn't used in either of our proposals, though. Unless
> I'm missing what you mean?
>
> --Jacob
>


Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-25 Thread Jacob Champion
On Wed, Jan 25, 2023 at 7:47 AM Israel Barth Rubio
 wrote:
> I imagine more people might have already hit a similar situation too. While 
> the
> workaround can seem a bit weird, in my very humble opinion the user/client is
> somehow still the one to blame in this case as it is providing the "wrong" 
> file in
> a path that is checked by libpq. With that in mind I would be inclined to say 
> it is
> an acceptable workaround.

I'm not sure how helpful it is to assign "blame" here. I think the
requested improvement is reasonable -- it should be possible to
override the default for a particular connection, without having to
pick a junk value that you hope doesn't match up with an actual file
on the disk.

> Although both patches achieve a similar goal regarding not sending the
> client certificate there is still a slight but in my opinion important 
> difference
> between them: sslmode=disable will also disable channel encryption. It
> may or may not be acceptable depending on how the connection is between
> your client and the server.

sslmode=disable isn't used in either of our proposals, though. Unless
I'm missing what you mean?

--Jacob




Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-25 Thread Israel Barth Rubio
Hello Jim/Jacob,

> > I do not think it is worth it to change the current behavior of
> PostgreSQL
> > in that sense.
>
> Well, I am not suggesting to change the current behavior of PostgreSQL in
> that matter. Quite the contrary, I find this feature very convenient,
> specially when you need to deal with many different clusters. What I am
> proposing is rather the possibility to disable it on demand :) I mean,
> in case I do not want libpq to try to authenticate using the certificates
> in `~/.postgresql`.
>
> > PostgreSQL looks for the cert and key under `~/.postgresql` as a
> facility.
> > These files do not exist by default, so if PostgreSQL finds something in
> > there it assumes you want to use it.
>
> Yes. I'm just trying to find an elegant way to disable this assumption
> on demand.

Right, I do understand your proposal. I was just thinking out loud and
wondering about the broad audience of such a mode in the sslmode
argument.

Something else that came to my mind is that sslmode itself seems more
like an argument covering the client expectations regarding the connection
to the server, I mean, if it expects channel encryption and/or validation
of the
server identity.

I wonder if we are willing to add some functionality around the expectations
regarding the client certificate, if it wouldn't make more sense to be
controlled
through something like the clientcert option of pg_hba? If so, the downside
of
that is the fact that the client would still send the certificate even if
it would not
be used at all by the server. Again, just thinking out loud about what your
goal
is and possible ways of accomplishing that:)

> > I do realize that this patch is a big ask, since probably nobody except
> > me "needs it" :D
>
> I'd imagine other people have run into it too; it's just a matter of
> how palatable the workarounds were to them. :)

I imagine more people might have already hit a similar situation too. While
the
workaround can seem a bit weird, in my very humble opinion the user/client
is
somehow still the one to blame in this case as it is providing the "wrong"
file in
a path that is checked by libpq. With that in mind I would be inclined to
say it is
an acceptable workaround.

> > I think the sslcertmode=disable option that I introduced in [1]
solves this issue too;
>
> Well, I see there is indeed a significant overlap between our patches -
> but yours has a much more comprehensive approach! If I got it right,
> the new slcertmode=disable would indeed cancel the existing certs in
> '~/.postgresql/ in case they exist. Right?
>
> +if (conn->sslcertmode[0] == 'd') /* disable */
> +{
> +/* don't send a client cert even if we have one */
> +have_cert = false;
> +}
> +else if (fnbuf[0] == '\0')
>
> My idea was rather to use the existing sslmode with a new option
> "no-clientcert" that does actually the same:
>
>  /* sslmode no-clientcert */
>  if (conn->sslmode[0] == 'n')
>  {
>  fnbuf[0] = '\0';
>  }
>
>  ...
>
>  if (fnbuf[0] == '\0')
>  {
>  /* no home directory, proceed without a client cert */
>  have_cert = false;
>  }
>
> I wish I had found your patchset some months ago. Now I hate myself
> for the duplication of efforts :D

Although both patches achieve a similar goal regarding not sending the
client certificate there is still a slight but in my opinion important
difference
between them: sslmode=disable will also disable channel encryption. It
may or may not be acceptable depending on how the connection is between
your client and the server.

Kind regards,
Israel.


Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-23 Thread Jacob Champion
On Sat, Jan 21, 2023 at 4:35 AM Jim Jones  wrote:
> Well, I see there is indeed a significant overlap between our patches -
> but yours has a much more comprehensive approach! If I got it right,
> the new slcertmode=disable would indeed cancel the existing certs in
> '~/.postgresql/ in case they exist. Right?

Right!

> I wish I had found your patchset some months ago. Now I hate myself
> for the duplication of efforts :D

It's a big list... I missed your thread back when you first posted it.

> What is the status of your patchset?

Currently waiting for a committer to sign on. But it's now being
discussed in another feature thread [1], coincidentally, so I think
the odds are fairly good.

--Jacob

[1] 
https://www.postgresql.org/message-id/CA%2BTgmoZbFiYJWqxakw0fcNrPSPCqc_QnF8iCdXZqyM%3Dd5jA-KA%40mail.gmail.com




Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-21 Thread Jim Jones

Hi Jacob,

> I think the sslcertmode=disable option that I introduced in [1] 
solves this issue too;


Well, I see there is indeed a significant overlap between our patches -
but yours has a much more comprehensive approach! If I got it right,
the new slcertmode=disable would indeed cancel the existing certs in
'~/.postgresql/ in case they exist. Right?

+    if (conn->sslcertmode[0] == 'd') /* disable */
+    {
+        /* don't send a client cert even if we have one */
+        have_cert = false;
+    }
+    else if (fnbuf[0] == '\0')

My idea was rather to use the existing sslmode with a new option
"no-clientcert" that does actually the same:

    /* sslmode no-clientcert */
    if (conn->sslmode[0] == 'n')
    {

    fnbuf[0] = '\0';

    }

    ...

    if (fnbuf[0] == '\0')
    {
    /* no home directory, proceed without a client cert */
    have_cert = false;
    }

I wish I had found your patchset some months ago. Now I hate myself
for the duplication of efforts :D

What is the status of your patchset?

Cheers
Jim





Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-20 Thread Jacob Champion
On Fri, Jan 20, 2023 at 11:09 AM Jim Jones  wrote:
> Well, I am not suggesting to change the current behavior of PostgreSQL in
> that matter. Quite the contrary, I find this feature very convenient,
> specially when you need to deal with many different clusters. What I am
> proposing is rather the possibility to disable it on demand :) I mean,
> in case I do not want libpq to try to authenticate using the certificates
> in `~/.postgresql`.

I think the sslcertmode=disable option that I introduced in [1] solves
this issue too; would it work for your case? That whole patchset is
meant to tackle the general case of the problem you've described.

(Eventually I'd like to teach the server not to ask for a client
certificate if it's not going to use it.)

> I do realize that this patch is a big ask, since probably nobody except
> me "needs it" :D

I'd imagine other people have run into it too; it's just a matter of
how palatable the workarounds were to them. :)

--Jacob

[1] 
https://www.postgresql.org/message-id/flat/CAAWbhmi4V9zEAvfUSCDFx1pOr3ZWrV9fuxkv_2maRqvyc-m9PQ%40mail.gmail.com#199c1f49fbefa6be401db35f5cfa7742




Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-20 Thread Jim Jones

Hello Israel,

Thanks a lot for the suggestion!

> I do not think it is worth it to change the current behavior of 
PostgreSQL

> in that sense.

Well, I am not suggesting to change the current behavior of PostgreSQL in
that matter. Quite the contrary, I find this feature very convenient,
specially when you need to deal with many different clusters. What I am
proposing is rather the possibility to disable it on demand :) I mean,
in case I do not want libpq to try to authenticate using the certificates
in `~/.postgresql`.

> PostgreSQL looks for the cert and key under `~/.postgresql` as a 
facility.

> These files do not exist by default, so if PostgreSQL finds something in
> there it assumes you want to use it.

Yes. I'm just trying to find an elegant way to disable this assumption 
on demand.


> I also think it is correct in the sense of choosing the certificate over
> a password based authentication when it finds a certificate as the cert
> based would provide you with stronger checks.

I couldn't agree more.

> It would require that you move the SSL cert and key from 
`~/.postgresql` to
> somewhere else and specify `sslcert` and `sslkey` in the expected 
service in the

> `~/.pg_service.conf` file.

That's exactly what I am trying to avoid. IOW, I want to avoid having to 
move

the cert files to another path and consequently having to configure 30
different entries in the pg_service.conf because of a single server that
does not support ssl authentication.

I do realize that this patch is a big ask, since probably nobody except 
me "needs it" :D


Thanks again for the message. Much appreciated!

Best,

Jim


smime.p7s
Description: S/MIME Cryptographic Signature


Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-19 Thread Israel Barth Rubio
Hello Jim,

> Hi Jelte, thanks for the message. You're right, an invalid cert path
> does solve the issue - I even use it for tests. Although it solves the
> authentication issue it still looks in my eyes like a non intuitive
> workaround/hack. Perhaps a new sslmode isn't the right place for this
> "feature"? Thanks again for the suggestion!

I do not think it is worth it to change the current behavior of PostgreSQL
in that sense.

PostgreSQL looks for the cert and key under `~/.postgresql` as a facility.
These files do not exist by default, so if PostgreSQL finds something in
there it assumes you want to use it.

I also think it is correct in the sense of choosing the certificate over
a password based authentication when it finds a certificate as the cert
based would provide you with stronger checks.

I believe that using libpq services would be a better approach if you
want to connect to several PostgreSQL clusters from the very same
source machine. That way you would specify whatever is specific to each
target cluster in a centralized configuration file and just reference each
target cluster by its service name in the connection string. It would
require that you move the SSL cert and key from `~/.postgresql` to somewhere
else and specify `sslcert` and `sslkey` in the expected service in the
`~/.pg_service.conf` file.

More info about that can be found at:

https://www.postgresql.org/docs/current/libpq-pgservice.html

Best regards,
Israel.

>


Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-06 Thread Jim Jones
Hi Jelte, thanks for the message. You're right, an invalid cert path 
does solve the issue - I even use it for tests. Although it solves the 
authentication issue it still looks in my eyes like a non intuitive 
workaround/hack. Perhaps a new sslmode isn't the right place for this 
"feature"? Thanks again for the suggestion!


Jim

On 06.01.23 09:32, Jelte Fennema wrote:
The easiest way to achieve the same (without patching libpq) is by 
setting sslcert to something non-existent. While maybe not the most 
obvious way, I would consider this the recommended approach.


On Fri, 6 Jan 2023 at 09:15, Jim Jones  wrote:

Dear PostgreSQL Hackers,

Some time ago we faced a small issue in libpq regarding
connections configured in the pg_hba.conf as type *hostssl* and
using *md5* as authentication method.

One of our users placed the client certificates in ~/.postgresql/
(*postgresql.crt,**postgresql.key*), so that libpq sends them to
the server without having to manually set *sslcert* and *sslkey* -
which is quite convenient. However, there are other servers where
the same user authenticates with password (md5), but libpq still
sends the client certificates for authentication by default. This
causes the authentication to fail even before the user has the
chance to enter his password, since he has no certificate
registered in the server.

To make it clearer:

Although the connection is configured as ...

*host  all  dummyuser 192.168.178.42/32
  md5
*

... and the client uses the following connection string ...

*psql "host=myserver dbname=db user=***dummyuser*" *

... the server tries to authenticate the user using the client
certificates in *~/.postgresql/* and, as expected, the
authentication fails:

*psql: error: connection to server at "myserver" (xx.xx.xx.xx),
port 5432 failed: SSL error: tlsv1 alert unknown ca*

Server log:
**

*2022-12-09 10:50:59.376 UTC [13896] LOG:  could not accept SSL
connection: certificate verify failed
*

Am I missing something?**

Obviously it would suffice to just remove or rename
*~/.postgresql/**postgresql.{crt,key}*, but the user needs them to
authenticate in other servers. So we came up with the workaround
to create a new sslmode (no-clientcert) to make libpq explicitly
ignore the client certificates, so that we can avoid ssl
authentication errors. These small changes can be seen in the
patch file attached.

*psql "host=myserver dbname=db user=dummyuser**
sslrootcert=server.crt sslmode=no-clientcert"*

Any better ideas to make libpq ignore
*~/.postgresql/**postgresql.{crt,key}*? Preferably without having
to change the source code :) Thanks in advance!

Best,

Jim


Re: Authentication fails for md5 connections if ~/.postgresql/postgresql.{crt and key} exist

2023-01-06 Thread Jelte Fennema
 The easiest way to achieve the same (without patching libpq) is by setting
sslcert to something non-existent. While maybe not the most obvious way, I
would consider this the recommended approach.

(sorry for the resend Jim, my original message got blocked to the wider
mailing list)

On Fri, 6 Jan 2023 at 09:15, Jim Jones  wrote:

> Dear PostgreSQL Hackers,
>
> Some time ago we faced a small issue in libpq regarding connections
> configured in the pg_hba.conf as type *hostssl* and using *md5* as
> authentication method.
>
> One of our users placed the client certificates in ~/.postgresql/ (
> *postgresql.crt,**postgresql.key*), so that libpq sends them to the
> server without having to manually set *sslcert* and *sslkey* - which is
> quite convenient. However, there are other servers where the same user
> authenticates with password (md5), but libpq still sends the client
> certificates for authentication by default. This causes the authentication
> to fail even before the user has the chance to enter his password, since he
> has no certificate registered in the server.
>
> To make it clearer:
>
> Although the connection is configured as ...
>
>
> *host  all  dummyuser  192.168.178.42/32   md5 *
>
> ... and the client uses the following connection string ...
>
> *psql "host=myserver dbname=db user=**dummyuser" *
>
> ... the server tries to authenticate the user using the client
> certificates in *~/.postgresql/* and, as expected, the authentication
> fails:
>
> *psql: error: connection to server at "myserver" (xx.xx.xx.xx), port 5432
> failed: SSL error: tlsv1 alert unknown ca*
>
> Server log:
>
>
> *2022-12-09 10:50:59.376 UTC [13896] LOG:  could not accept SSL
> connection: certificate verify failed *
>
> Am I missing something?
>
> Obviously it would suffice to just remove or rename *~/.postgresql/*
> *postgresql.{crt,key}*, but the user needs them to authenticate in other
> servers. So we came up with the workaround to create a new sslmode
> (no-clientcert) to make libpq explicitly ignore the client certificates, so
> that we can avoid ssl authentication errors. These small changes can be
> seen in the patch file attached.
>
> *psql "host=myserver dbname=db user=**dummyuser sslrootcert=server.crt
> sslmode=no-clientcert"*
>
> Any better ideas to make libpq ignore *~/.postgresql/*
> *postgresql.{crt,key}*? Preferably without having to change the source
> code :) Thanks in advance!
>
> Best,
>
> Jim
>
>