Thread

Commits

Spell the values of libpq's gssdelegation parameter as "0" and "1".
- 1f9f6aa49198 16.0 landed
Expand some more uses of "deleg" to "delegation" or "delegated".
- a2eb99a01e01 16.0 landed
Fix remaining references to gss_accept_deleg.
- f4001a553770 16.0 landed
rename "gss_accept_deleg" to "gss_accept_delegation".
- 9c0a0e2ed92a 16.0 landed

Naming of gss_accept_deleg

Bruce Momjian <bruce@momjian.us> — 2023-05-19T12:09:54Z

Why is the new PG 16 GUC called "gss_accept_deleg" and not
"gss_accept_delegation"?  The abbreviation here seems atypical.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Re: Naming of gss_accept_deleg

Stephen Frost <sfrost@snowman.net> — 2023-05-19T13:07:26Z

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:
> Why is the new PG 16 GUC called "gss_accept_deleg" and not
> "gss_accept_delegation"?  The abbreviation here seems atypical.

At the time it felt natural to me but I don't feel strongly about it,
happy to change it if folks would prefer it spelled out.

Thanks,

Stephen

Re: Naming of gss_accept_deleg

Bruce Momjian <bruce@momjian.us> — 2023-05-19T13:16:09Z

On Fri, May 19, 2023 at 09:07:26AM -0400, Stephen Frost wrote:
> Greetings,
> 
> * Bruce Momjian (bruce@momjian.us) wrote:
> > Why is the new PG 16 GUC called "gss_accept_deleg" and not
> > "gss_accept_delegation"?  The abbreviation here seems atypical.
> 
> At the time it felt natural to me but I don't feel strongly about it,
> happy to change it if folks would prefer it spelled out.

Yes, please do spell it out, thanks.  The fact "deleg" looks similar to
"debug" also doesn't help.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Re: Naming of gss_accept_deleg

Abhijit Menon-Sen <ams@toroid.org> — 2023-05-19T13:35:19Z

At 2023-05-19 09:16:09 -0400, bruce@momjian.us wrote:
>
> On Fri, May 19, 2023 at 09:07:26AM -0400, Stephen Frost wrote:
> > 
> > > Why is the new PG 16 GUC called "gss_accept_deleg" and not
> > > "gss_accept_delegation"?  The abbreviation here seems atypical.
> > 
> > At the time it felt natural to me but I don't feel strongly about it,
> > happy to change it if folks would prefer it spelled out.
> 
> Yes, please do spell it out, thanks.  The fact "deleg" looks similar to
> "debug" also doesn't help.

Note that GSS-API itself calls it the "DELEG" flag:

    if (conn->gcred != GSS_C_NO_CREDENTIAL)
            gss_flags |= GSS_C_DELEG_FLAG;

I would also prefer a GUC named gss_accept_delegation, but the current
name matches the libpq gssdeleg connection parameter and the PGSSDELEG
environment variable. Maybe there's something to be said for keeping
those three things alike?

-- Abhijit

Re: Naming of gss_accept_deleg

Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-19T13:42:00Z

Abhijit Menon-Sen <ams@toroid.org> writes:
> I would also prefer a GUC named gss_accept_delegation, but the current
> name matches the libpq gssdeleg connection parameter and the PGSSDELEG
> environment variable. Maybe there's something to be said for keeping
> those three things alike?

+1 for spelling it out in all user-visible names.  I do not think
that that GSS-API C symbol is a good precedent to follow.

			regards, tom lane

Re: Naming of gss_accept_deleg

Bruce Momjian <bruce@momjian.us> — 2023-05-19T13:44:36Z

On Fri, May 19, 2023 at 09:42:00AM -0400, Tom Lane wrote:
> Abhijit Menon-Sen <ams@toroid.org> writes:
> > I would also prefer a GUC named gss_accept_delegation, but the current
> > name matches the libpq gssdeleg connection parameter and the PGSSDELEG
> > environment variable. Maybe there's something to be said for keeping
> > those three things alike?
> 
> +1 for spelling it out in all user-visible names.  I do not think
> that that GSS-API C symbol is a good precedent to follow.

Once nice bonus of the release notes is that it allows us to see the new
API in one place to check for consistency.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Re: Naming of gss_accept_deleg

Nathan Bossart <nathandbossart@gmail.com> — 2023-05-19T14:58:34Z

On Fri, May 19, 2023 at 09:42:00AM -0400, Tom Lane wrote:
> Abhijit Menon-Sen <ams@toroid.org> writes:
>> I would also prefer a GUC named gss_accept_delegation, but the current
>> name matches the libpq gssdeleg connection parameter and the PGSSDELEG
>> environment variable. Maybe there's something to be said for keeping
>> those three things alike?
> 
> +1 for spelling it out in all user-visible names.  I do not think
> that that GSS-API C symbol is a good precedent to follow.

+1

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Re: Naming of gss_accept_deleg

Bruce Momjian <bruce@momjian.us> — 2023-05-21T01:33:44Z

On Fri, May 19, 2023 at 07:58:34AM -0700, Nathan Bossart wrote:
> On Fri, May 19, 2023 at 09:42:00AM -0400, Tom Lane wrote:
> > Abhijit Menon-Sen <ams@toroid.org> writes:
> >> I would also prefer a GUC named gss_accept_delegation, but the current
> >> name matches the libpq gssdeleg connection parameter and the PGSSDELEG
> >> environment variable. Maybe there's something to be said for keeping
> >> those three things alike?
> > 
> > +1 for spelling it out in all user-visible names.  I do not think
> > that that GSS-API C symbol is a good precedent to follow.
> 
> +1

With less then 48 hours to beta 1 packaging, I have made this change and
adjusted internal variable to match.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Re: Naming of gss_accept_deleg

Nathan Bossart <nathandbossart@gmail.com> — 2023-05-21T03:17:57Z

On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote:
> With less then 48 hours to beta 1 packaging, I have made this change and
> adjusted internal variable to match.

The buildfarm and cfbot seem unhappy with 9c0a0e2.  It looks like there are
a few remaining uses of gss_accept_deleg to rename.  I'm planning to commit
the attached patch shortly.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Re: Naming of gss_accept_deleg

Bruce Momjian <bruce@momjian.us> — 2023-05-21T03:20:50Z

On Sat, May 20, 2023 at 08:17:57PM -0700, Nathan Bossart wrote:
> On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote:
> > With less then 48 hours to beta 1 packaging, I have made this change and
> > adjusted internal variable to match.
> 
> The buildfarm and cfbot seem unhappy with 9c0a0e2.  It looks like there are
> a few remaining uses of gss_accept_deleg to rename.  I'm planning to commit
> the attached patch shortly.

Yes, please do.  I saw some matches in the tests but was confused since
my tests passed.  I now realize I wasn't testing those.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Re: Naming of gss_accept_deleg

Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-21T03:21:57Z

Nathan Bossart <nathandbossart@gmail.com> writes:
> On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote:
>> With less then 48 hours to beta 1 packaging, I have made this change and
>> adjusted internal variable to match.

> The buildfarm and cfbot seem unhappy with 9c0a0e2.  It looks like there are
> a few remaining uses of gss_accept_deleg to rename.  I'm planning to commit
> the attached patch shortly.

I thought the plan was to also rename the libpq "gssdeleg" connection
parameter and so on?  I can look into that tomorrow, if nobody beats
me to it.

			regards, tom lane

Re: Naming of gss_accept_deleg

Bruce Momjian <bruce@momjian.us> — 2023-05-21T03:27:29Z

On Sat, May 20, 2023 at 11:21:57PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
> > On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote:
> >> With less then 48 hours to beta 1 packaging, I have made this change and
> >> adjusted internal variable to match.
> 
> > The buildfarm and cfbot seem unhappy with 9c0a0e2.  It looks like there are
> > a few remaining uses of gss_accept_deleg to rename.  I'm planning to commit
> > the attached patch shortly.
> 
> I thought the plan was to also rename the libpq "gssdeleg" connection
> parameter and so on?  I can look into that tomorrow, if nobody beats
> me to it.

Oh, I didn't consider those.  I thought we would leave libpq alone since
those are often supplied on the command line and there are existing
short-style libpq options, e.g., gssencmode, krbsrvname, sslcrl.

I am fine with such a change though.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Re: Naming of gss_accept_deleg

Nathan Bossart <nathandbossart@gmail.com> — 2023-05-21T03:41:22Z

On Sat, May 20, 2023 at 11:21:57PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> The buildfarm and cfbot seem unhappy with 9c0a0e2.  It looks like there are
>> a few remaining uses of gss_accept_deleg to rename.  I'm planning to commit
>> the attached patch shortly.

Done.

> I thought the plan was to also rename the libpq "gssdeleg" connection
> parameter and so on?  I can look into that tomorrow, if nobody beats
> me to it.

Please do.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Re: Naming of gss_accept_deleg

Abhijit Menon-Sen <ams@toroid.org> — 2023-05-21T06:46:04Z

At 2023-05-20 23:21:57 -0400, tgl@sss.pgh.pa.us wrote:
>
> Nathan Bossart <nathandbossart@gmail.com> writes:
> > On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote:
> >> With less then 48 hours to beta 1 packaging, I have made this change and
> >> adjusted internal variable to match.
> 
> > The buildfarm and cfbot seem unhappy with 9c0a0e2.  It looks like there are
> > a few remaining uses of gss_accept_deleg to rename.  I'm planning to commit
> > the attached patch shortly.
> 
> I thought the plan was to also rename the libpq "gssdeleg" connection
> parameter and so on?  I can look into that tomorrow, if nobody beats
> me to it.

I was trying the change to see if it would be better to name it
"gssdelegate" instead (as in delegate on one side, and accept the
delegation on the other), but decided that "gssdelegation=enable"
reads better than "gssdelegate=enable".

Here's the diff.

-- Abhijit

Re: Naming of gss_accept_deleg

Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-21T14:57:38Z

Abhijit Menon-Sen <ams@toroid.org> writes:
> At 2023-05-20 23:21:57 -0400, tgl@sss.pgh.pa.us wrote:
>> I thought the plan was to also rename the libpq "gssdeleg" connection
>> parameter and so on?  I can look into that tomorrow, if nobody beats
>> me to it.

> I was trying the change to see if it would be better to name it
> "gssdelegate" instead (as in delegate on one side, and accept the
> delegation on the other), but decided that "gssdelegation=enable"
> reads better than "gssdelegate=enable".

Yeah, agreed.

> Here's the diff.

Thanks for doing that legwork!  I found a couple other places where
"deleg" had escaped notice, and changed the lot.  Watching the
buildfarm now ...

			regards, tom lane

Re: Naming of gss_accept_deleg

Stephen Frost <sfrost@snowman.net> — 2023-05-21T15:53:04Z

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Abhijit Menon-Sen <ams@toroid.org> writes:
> > At 2023-05-20 23:21:57 -0400, tgl@sss.pgh.pa.us wrote:
> >> I thought the plan was to also rename the libpq "gssdeleg" connection
> >> parameter and so on?  I can look into that tomorrow, if nobody beats
> >> me to it.
> 
> > I was trying the change to see if it would be better to name it
> > "gssdelegate" instead (as in delegate on one side, and accept the
> > delegation on the other), but decided that "gssdelegation=enable"
> > reads better than "gssdelegate=enable".
> 
> Yeah, agreed.
> 
> > Here's the diff.
> 
> Thanks for doing that legwork!  I found a couple other places where
> "deleg" had escaped notice, and changed the lot.  Watching the
> buildfarm now ...

Thanks all for taking this up over a weekend.

Stephen

Re: Naming of gss_accept_deleg

Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-05-22T09:16:09Z

I noticed that the value that enables this feature at libpq client side
is 'enable'.  However, for other Boolean settings like sslsni,
keepalives, requiressl, sslcompression, the value that enables feature
is '1' -- we use strings only for "enum" type of settings.

Also, it looks like connectOptions2() doesn't validate the string value.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
<inflex> really, I see PHP as like a strange amalgamation of C, Perl, Shell
<crab> inflex: you know that "amalgam" means "mixture with mercury",
       more or less, right?
<crab> i.e., "deadly poison"

Re: Naming of gss_accept_deleg

Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-22T13:42:44Z

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> I noticed that the value that enables this feature at libpq client side
> is 'enable'.  However, for other Boolean settings like sslsni,
> keepalives, requiressl, sslcompression, the value that enables feature
> is '1' -- we use strings only for "enum" type of settings.

> Also, it looks like connectOptions2() doesn't validate the string value.

Hmm, it certainly seems like this ought to accept exactly the
same inputs as other libpq boolean settings.  I can take a look
unless somebody else is already on it.

			regards, tom lane

Re: Naming of gss_accept_deleg

Abhijit Menon-Sen <ams@toroid.org> — 2023-05-22T13:44:40Z

At 2023-05-22 09:42:44 -0400, tgl@sss.pgh.pa.us wrote:
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > I noticed that the value that enables this feature at libpq client side
> > is 'enable'.  However, for other Boolean settings like sslsni,
> > keepalives, requiressl, sslcompression, the value that enables feature
> > is '1' -- we use strings only for "enum" type of settings.
> 
> > Also, it looks like connectOptions2() doesn't validate the string value.
> 
> Hmm, it certainly seems like this ought to accept exactly the
> same inputs as other libpq boolean settings.  I can take a look
> unless somebody else is already on it.

I'm working on it.

-- Abhijit

Re: Naming of gss_accept_deleg

Abhijit Menon-Sen <ams@toroid.org> — 2023-05-22T15:03:34Z

At 2023-05-22 09:42:44 -0400, tgl@sss.pgh.pa.us wrote:
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > I noticed that the value that enables this feature at libpq client side
> > is 'enable'.  However, for other Boolean settings like sslsni,
> > keepalives, requiressl, sslcompression, the value that enables feature
> > is '1' -- we use strings only for "enum" type of settings.
> 
> > Also, it looks like connectOptions2() doesn't validate the string value.
> 
> Hmm, it certainly seems like this ought to accept exactly the
> same inputs as other libpq boolean settings.  I can take a look
> unless somebody else is already on it.

Here's the diff, but the 0/1 values of settings like sslsni and
sslcompression don't seem to be validated anywhere, unlike the string
options in connectOptions2, so I didn't do anything for gssdelegation.

(I've never run the Kerberos tests before, but I changed one
"gssdelegation=disable" to "gssdelegation=1" and got a test failure, so
they're probably working as expected.)

-- Abhijit

Re: Naming of gss_accept_deleg

Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-22T15:08:16Z

Abhijit Menon-Sen <ams@toroid.org> writes:
> Here's the diff, but the 0/1 values of settings like sslsni and
> sslcompression don't seem to be validated anywhere, unlike the string
> options in connectOptions2, so I didn't do anything for gssdelegation.

Thanks!  I'll set to work on this.  I assume we want to squeeze
it into beta1, so there's not much time.

			regards, tom lane

Re: Naming of gss_accept_deleg

Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-22T15:54:15Z

Abhijit Menon-Sen <ams@toroid.org> writes:
> Here's the diff,

Pushed, thanks.

> but the 0/1 values of settings like sslsni and
> sslcompression don't seem to be validated anywhere, unlike the string
> options in connectOptions2, so I didn't do anything for gssdelegation.

Yeah.  Perhaps it's worth adding code to validate boolean options,
but since nobody has noticed the lack of that for decades, I'm not
in a hurry to (especially not in a last-minute patch).

Also, I noticed that PGGSSDELEGATION had not been added to the lists of
environment variables to unset in pg_regress.c and Test/Utils.pm.
Dealt with that in the same commit.

			regards, tom lane