Remove one use of IDENT_USERNAME_MAX

Started by Peter Eisentrautover 6 years ago6 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

It seems to me that using IDENT_USERNAME_MAX for peer authentication is
some kind of historical leftover and not really appropriate or useful,
so I propose the attached cleanup.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Remove-one-use-of-IDENT_USERNAME_MAX.patchtext/plain; charset=UTF-8; name=0001-Remove-one-use-of-IDENT_USERNAME_MAX.patch; x-mac-creator=0; x-mac-type=0Download+7-6
#2Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Remove one use of IDENT_USERNAME_MAX

Hello.

At Sat, 26 Oct 2019 08:55:03 +0200, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in

IDENT_USERNAME_MAX is the maximum length of the information returned
by an ident server, per RFC 1413. Using it as the buffer size in peer
authentication is inappropriate. It was done here because of the
historical relationship between peer and ident authentication. But
since it's also completely useless code-wise, remove it.

In think one of the reasons for the coding is the fact that *pw is
described to be placed in the static area, which can be overwritten by
succeeding calls to getpw*() functions. I think we can believe
check_usermap() never calls them but I suppose that some comments
needed..

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kyotaro Horiguchi (#2)
Re: Remove one use of IDENT_USERNAME_MAX

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

At Sat, 26 Oct 2019 08:55:03 +0200, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in

IDENT_USERNAME_MAX is the maximum length of the information returned
by an ident server, per RFC 1413. Using it as the buffer size in peer
authentication is inappropriate. It was done here because of the
historical relationship between peer and ident authentication. But
since it's also completely useless code-wise, remove it.

In think one of the reasons for the coding is the fact that *pw is
described to be placed in the static area, which can be overwritten by
succeeding calls to getpw*() functions.

Good point ... so maybe pstrdup instead of using a fixed-size buffer?

regards, tom lane

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#3)
Re: Remove one use of IDENT_USERNAME_MAX

On 2019-10-28 14:45, Tom Lane wrote:

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

At Sat, 26 Oct 2019 08:55:03 +0200, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in

IDENT_USERNAME_MAX is the maximum length of the information returned
by an ident server, per RFC 1413. Using it as the buffer size in peer
authentication is inappropriate. It was done here because of the
historical relationship between peer and ident authentication. But
since it's also completely useless code-wise, remove it.

In think one of the reasons for the coding is the fact that *pw is
described to be placed in the static area, which can be overwritten by
succeeding calls to getpw*() functions.

Good point ... so maybe pstrdup instead of using a fixed-size buffer?

Maybe. Or we just decide that check_usermap() is not allowed to call
getpw*(). It's just a string-matching routine, so it doesn't have any
such business anyway.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#4)
Re: Remove one use of IDENT_USERNAME_MAX

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-10-28 14:45, Tom Lane wrote:

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

In think one of the reasons for the coding is the fact that *pw is
described to be placed in the static area, which can be overwritten by
succeeding calls to getpw*() functions.

Good point ... so maybe pstrdup instead of using a fixed-size buffer?

Maybe. Or we just decide that check_usermap() is not allowed to call
getpw*(). It's just a string-matching routine, so it doesn't have any
such business anyway.

I'm okay with that as long as you add a comment describing this
assumption.

regards, tom lane

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#5)
Re: Remove one use of IDENT_USERNAME_MAX

On 2019-10-29 15:34, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-10-28 14:45, Tom Lane wrote:

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

In think one of the reasons for the coding is the fact that *pw is
described to be placed in the static area, which can be overwritten by
succeeding calls to getpw*() functions.

Good point ... so maybe pstrdup instead of using a fixed-size buffer?

Maybe. Or we just decide that check_usermap() is not allowed to call
getpw*(). It's just a string-matching routine, so it doesn't have any
such business anyway.

I'm okay with that as long as you add a comment describing this
assumption.

Committed with a pstrdup(). That seemed more consistent with other code
in that file.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services