Horton

Thread

Commits

  1. libpq: Remove code for SCM credential authentication

  1. Remove last traces of SCM credential auth from libpq?

    Michael Paquier <michael@paquier.xyz> — 2023-03-16T07:40:33Z

    Hi all,

libpq has kept some code related to the support of authentication with
SCM credentials for some time now, code dead in the backend since
9.1.  Wouldn't it be time to let it go and remove this code entirely,
erroring in libpq if attempting to connect to a server that supports
that?

Hard to say if this is actually working these days.

Opinions or thoughts?
--
Michael

  2. Re: Remove last traces of SCM credential auth from libpq?

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-16T14:49:45Z

    Michael Paquier <michael@paquier.xyz> writes:
> libpq has kept some code related to the support of authentication with
> SCM credentials for some time now, code dead in the backend since
> 9.1.  Wouldn't it be time to let it go and remove this code entirely,
> erroring in libpq if attempting to connect to a server that supports
> that?

+1.  Since that's only used on Unix-domain sockets, it could only be
useful if you were using current libpq while talking to a pre-9.1
server on the same machine.  That seems fairly unlikely --- and if
you did have to do that, you could still connect, just not with peer
auth.  You'd be suffering other quality-of-life problems too,
because we removed support for such old servers from psql and pg_dump
awhile ago.

> Hard to say if this is actually working these days.

I didn't trace the old discussions, but the commit that removed the
server-side support (be4585b1c) mentions something about portability
issues with that code ... so it's rather likely that it didn't work
anyway.

In addition to the changes here, it looks like you could drop the
configure/meson probes that set HAVE_STRUCT_CMSGCRED.

Also, in pg_fe_sendauth, couldn't you just let the default: case
handle it instead of adding a bespoke error message?  We're not
really expecting that anyone is ever going to hit this, so I'm
not convinced it's worth the translation burden.

			regards, tom lane

  3. Re: Remove last traces of SCM credential auth from libpq?

    Jonathan S. Katz <jkatz@postgresql.org> — 2023-03-16T17:28:51Z

    On 3/16/23 10:49 AM, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> libpq has kept some code related to the support of authentication with
>> SCM credentials for some time now, code dead in the backend since
>> 9.1.  Wouldn't it be time to let it go and remove this code entirely,
>> erroring in libpq if attempting to connect to a server that supports
>> that?
> 
> +1.  Since that's only used on Unix-domain sockets, it could only be
> useful if you were using current libpq while talking to a pre-9.1
> server on the same machine.

+1.

> Also, in pg_fe_sendauth, couldn't you just let the default: case
> handle it instead of adding a bespoke error message?  We're not
> really expecting that anyone is ever going to hit this, so I'm
> not convinced it's worth the translation burden.

+1 to this, that was my thought as well. That would let us remove the 
"AUTH_REQ_SCM_CREDS" constant too.

It looks like in the po files there are a bunch of "SCM_CRED 
authentication method not supported" messages that can also be removed.

Thanks,

Jonathan

  4. Re: Remove last traces of SCM credential auth from libpq?

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-16T17:50:09Z

    "Jonathan S. Katz" <jkatz@postgresql.org> writes:
> It looks like in the po files there are a bunch of "SCM_CRED 
> authentication method not supported" messages that can also be removed.

Those will go away in the normal course of translation maintenance,
there's no need to remove them by hand.  (Generally speaking, there
is no need to ever touch the .po files except when new versions get
imported from the translation repo.)

			regards, tom lane

  5. Re: Remove last traces of SCM credential auth from libpq?

    Michael Paquier <michael@paquier.xyz> — 2023-03-17T00:04:49Z

    On Thu, Mar 16, 2023 at 10:49:45AM -0400, Tom Lane wrote:
> In addition to the changes here, it looks like you could drop the
> configure/meson probes that set HAVE_STRUCT_CMSGCRED.

Right, done.

> Also, in pg_fe_sendauth, couldn't you just let the default: case
> handle it instead of adding a bespoke error message?  We're not
> really expecting that anyone is ever going to hit this, so I'm
> not convinced it's worth the translation burden.

Yes, I was wondering if that's worth keeping or not, so I chose
consistency with AUTH_REQ_KRB4 and AUTH_REQ_KRB5.

Would it be better to hold on this patch for 17~?  I have just noticed
that while looking at Jacob's patch for require_auth, so the timing is
not good.  Honestly, I don't see a reason to wait a few extra month to
remove that, particularly now that pg_dump and pg_upgrade go down to
9.2..
--
Michael

  6. Re: Remove last traces of SCM credential auth from libpq?

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-17T00:10:12Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Thu, Mar 16, 2023 at 10:49:45AM -0400, Tom Lane wrote:
>> Also, in pg_fe_sendauth, couldn't you just let the default: case
>> handle it instead of adding a bespoke error message?  We're not
>> really expecting that anyone is ever going to hit this, so I'm
>> not convinced it's worth the translation burden.

> Yes, I was wondering if that's worth keeping or not, so I chose
> consistency with AUTH_REQ_KRB4 and AUTH_REQ_KRB5.

Maybe flush those special messages too?  I'm not sure how long
they've been obsolete, though.

> Would it be better to hold on this patch for 17~?

Nah, I see no reason to wait.  We already dropped the higher-level
client support (psql/pg_dump) for these server versions in v15.

			regards, tom lane

  7. Re: Remove last traces of SCM credential auth from libpq?

    Michael Paquier <michael@paquier.xyz> — 2023-03-17T00:30:32Z

    On Thu, Mar 16, 2023 at 08:10:12PM -0400, Tom Lane wrote:
> Maybe flush those special messages too?  I'm not sure how long
> they've been obsolete, though.

KRB4 was switched in a159ad3 back in 2005, and KRB5 in 98de86e back in
2014 (deprecated in 8.3, so that's even older than creds).  So yes,
that could be removed as well, I guess, falling back to the default
error message.

> Nah, I see no reason to wait.  We already dropped the higher-level
> client support (psql/pg_dump) for these server versions in v15.

Okay.  I'll clean up this part today, then.
--
Michael

  8. Re: Remove last traces of SCM credential auth from libpq?

    Michael Paquier <michael@paquier.xyz> — 2023-03-17T23:13:10Z

    On Fri, Mar 17, 2023 at 09:30:32AM +0900, Michael Paquier wrote:
> KRB4 was switched in a159ad3 back in 2005, and KRB5 in 98de86e back in
> 2014 (deprecated in 8.3, so that's even older than creds).  So yes,
> that could be removed as well, I guess, falling back to the default
> error message.

This seems like something worth a thread of its own, will send a
patch.

>> Nah, I see no reason to wait.  We already dropped the higher-level
>> client support (psql/pg_dump) for these server versions in v15.
> 
> Okay.  I'll clean up this part today, then.

I got around to do that with 98ae2c8.
--
Michael