Thread: elog(FATAL)ing non-existent roles during client authentication
Hi all, I noticed that during client authentication by HBA, some times we will necessarily determine whether or not a role exists. For example, password, crypt and md5 auth methods call get_role_line() which tells the caller whether the role exists. If it doesn't (or if the authentication fails due to a password mismatch) we error out. I wonder if we should check if the role exists for the other authentication methods too? get_role_line() should be very cheap and it would prevent unnecessary authentication work if we did it before contacting, for example, the client ident server. Even with trust, it would save work because otherwise we do not check if the user exists until InitializeSessionUserId(), at which time we're set up our proc entry etc. This might seem overly pessimistic, I know, but it seemed to me that a malicious user on a local network might be able to tie up a system in interesting ways by launching lots of connections without necessarily knowing any usernames/passwords. Thanks, Gavin
Gavin Sherry <swm@linuxworld.com.au> writes:
> I wonder if we should check if the role exists for the other
> authentication methods too? get_role_line() should be very cheap and it
> would prevent unnecessary authentication work if we did it before
> contacting, for example, the client ident server. Even with trust, it
> would save work because otherwise we do not check if the user exists until
> InitializeSessionUserId(), at which time we're set up our proc entry etc.
This only saves work if the supplied ID is in fact invalid, which one
would surely think isn't the normal case; otherwise it costs more.
I could see doing this in the ident path, because contacting a remote
ident server is certainly expensive on both sides. I doubt it's a good
idea in the trust case.
regards, tom lane
On Thu, 30 Nov 2006, Tom Lane wrote: > Gavin Sherry <swm@linuxworld.com.au> writes: > > I wonder if we should check if the role exists for the other > > authentication methods too? get_role_line() should be very cheap and it > > would prevent unnecessary authentication work if we did it before > > contacting, for example, the client ident server. Even with trust, it > > would save work because otherwise we do not check if the user exists until > > InitializeSessionUserId(), at which time we're set up our proc entry etc. > > This only saves work if the supplied ID is in fact invalid, which one > would surely think isn't the normal case; otherwise it costs more. Yes. > I could see doing this in the ident path, because contacting a remote > ident server is certainly expensive on both sides. I doubt it's a good > idea in the trust case. Agreed. How about Kerberos too, applying the same logic? Gavin
On Tue, 5 Dec 2006, Gavin Sherry wrote: > On Thu, 30 Nov 2006, Tom Lane wrote: > > > Gavin Sherry <swm@linuxworld.com.au> writes: > > > I wonder if we should check if the role exists for the other > > > authentication methods too? get_role_line() should be very cheap and it > > > would prevent unnecessary authentication work if we did it before > > > contacting, for example, the client ident server. Even with trust, it > > > would save work because otherwise we do not check if the user exists until > > > InitializeSessionUserId(), at which time we're set up our proc entry etc. > > > > This only saves work if the supplied ID is in fact invalid, which one > > would surely think isn't the normal case; otherwise it costs more. > > Yes. > > > I could see doing this in the ident path, because contacting a remote > > ident server is certainly expensive on both sides. I doubt it's a good > > idea in the trust case. > > Agreed. How about Kerberos too, applying the same logic? Attached is a patch check adds the checks. Gavin
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Gavin Sherry wrote:
> On Tue, 5 Dec 2006, Gavin Sherry wrote:
>
> > On Thu, 30 Nov 2006, Tom Lane wrote:
> >
> > > Gavin Sherry <swm@linuxworld.com.au> writes:
> > > > I wonder if we should check if the role exists for the other
> > > > authentication methods too? get_role_line() should be very cheap and it
> > > > would prevent unnecessary authentication work if we did it before
> > > > contacting, for example, the client ident server. Even with trust, it
> > > > would save work because otherwise we do not check if the user exists until
> > > > InitializeSessionUserId(), at which time we're set up our proc entry etc.
> > >
> > > This only saves work if the supplied ID is in fact invalid, which one
> > > would surely think isn't the normal case; otherwise it costs more.
> >
> > Yes.
> >
> > > I could see doing this in the ident path, because contacting a remote
> > > ident server is certainly expensive on both sides. I doubt it's a good
> > > idea in the trust case.
> >
> > Agreed. How about Kerberos too, applying the same logic?
>
> Attached is a patch check adds the checks.
>
> Gavin
Content-Description:
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Patch applied. Thanks. --------------------------------------------------------------------------- Gavin Sherry wrote: > On Tue, 5 Dec 2006, Gavin Sherry wrote: > > > On Thu, 30 Nov 2006, Tom Lane wrote: > > > > > Gavin Sherry <swm@linuxworld.com.au> writes: > > > > I wonder if we should check if the role exists for the other > > > > authentication methods too? get_role_line() should be very cheap and it > > > > would prevent unnecessary authentication work if we did it before > > > > contacting, for example, the client ident server. Even with trust, it > > > > would save work because otherwise we do not check if the user exists until > > > > InitializeSessionUserId(), at which time we're set up our proc entry etc. > > > > > > This only saves work if the supplied ID is in fact invalid, which one > > > would surely think isn't the normal case; otherwise it costs more. > > > > Yes. > > > > > I could see doing this in the ident path, because contacting a remote > > > ident server is certainly expensive on both sides. I doubt it's a good > > > idea in the trust case. > > > > Agreed. How about Kerberos too, applying the same logic? > > Attached is a patch check adds the checks. > > Gavin Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 9: In versions below 8.0, the planner will ignore your desire to > choose an index scan if your joining column's datatypes do not > match -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +