Re: [HACKERS] rolcanlogin vs. the flat password file
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Heikki Linnakangas [EMAIL PROTECTED] writes: At least if we think it's more than a very narrow legitimate use, compared to the number of ppl making the mistake. Did we ever come to a conclusion on this or not? I've changed my patch per the suggestions in the thread, but I've held back on committing it to hear arguments... Go or no-go? I'm inclined to vote no-go on the message. AFAIR we've only heard the one complaint about this, so I'm not convinced there's a lot of people making such a mistake. We did make the logic change to deal with the underlying problem of a misleading error message after you'd done it, and I think that might be enough. Ok. I'm dropping it for now. If someone wants it later, the patch is in the archives... //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] rolcanlogin vs. the flat password file
Magnus Hagander wrote: Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Heikki Linnakangas [EMAIL PROTECTED] writes: At least if we think it's more than a very narrow legitimate use, compared to the number of ppl making the mistake. Did we ever come to a conclusion on this or not? I've changed my patch per the suggestions in the thread, but I've held back on committing it to hear arguments... Go or no-go? I'm inclined to vote no-go on the message. AFAIR we've only heard the one complaint about this, so I'm not convinced there's a lot of people making such a mistake. We did make the logic change to deal with the underlying problem of a misleading error message after you'd done it, and I think that might be enough. Ok. I'm dropping it for now. If someone wants it later, the patch is in the archives... Indulge me while I say that it's pretty useless there. The archiver mangles it pretty badly -- I have never found a patch you could actually use in the archives (or on Bruce's queues for that matter). What I have had to do was log into the majordomo page and have it send the email to me. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] rolcanlogin vs. the flat password file
Magnus Hagander wrote: On Wed, Oct 17, 2007 at 11:27:10AM -0400, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: There's legitimate use for creating a role with NOLOGIN and a password. If we think that, then we shouldn't have a message at all. At least if we think it's more than a very narrow legitimate use, compared to the number of ppl making the mistake. I agree with making it a NOTICE instead of WARNING though. Did we ever come to a conclusion on this or not? I've changed my patch per the suggestions in the thread, but I've held back on committing it to hear arguments... Go or no-go? //Magnus ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] rolcanlogin vs. the flat password file
Magnus Hagander [EMAIL PROTECTED] writes: Heikki Linnakangas [EMAIL PROTECTED] writes: At least if we think it's more than a very narrow legitimate use, compared to the number of ppl making the mistake. Did we ever come to a conclusion on this or not? I've changed my patch per the suggestions in the thread, but I've held back on committing it to hear arguments... Go or no-go? I'm inclined to vote no-go on the message. AFAIR we've only heard the one complaint about this, so I'm not convinced there's a lot of people making such a mistake. We did make the logic change to deal with the underlying problem of a misleading error message after you'd done it, and I think that might be enough. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] rolcanlogin vs. the flat password file
On Sun, Oct 14, 2007 at 06:16:04PM -0400, Stephen Frost wrote:
* Tom Lane ([EMAIL PROTECTED]) wrote:
Stephen Frost [EMAIL PROTECTED] writes:
I wonder if the OP was unhappy because he created a role w/ a pw and
then couldn't figure out why the user couldn't log in?
Hm, maybe. In that case just not filtering the entry out of the flat
file would be good enough.
I've confirmed the confusing behavior in CVS HEAD. With password auth
selected in pg_hba.conf:
[...]
Should we just do this, or is it worth working harder?
I certainly like this. Honestly, I'd also like the warning when doing a
'create role'/'alter role' that sets/changes the pw on an account that
doesn't have 'rolcanlogin'. Much better to have me notice that I goof'd
the command and fix it before telling the user 'go ahead and log in'
than to have the user complain that it's not working. :)
Just my 2c.
I think that's a good idea. Attached is a patch that implements this (I
think - haven't messed around in that area of the code before). Thoughts?
//Magnus
Index: src/backend/commands/user.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/user.c,v
retrieving revision 1.177
diff -c -r1.177 user.c
*** src/backend/commands/user.c 3 Sep 2007 18:46:30 - 1.177
--- src/backend/commands/user.c 17 Oct 2007 14:45:33 -
***
*** 250,255
--- 250,260
if (dvalidUntil)
validUntil = strVal(dvalidUntil-arg);
+ /* Warn about combination that's likely incorrect */
+ if (password !canlogin)
+ ereport(WARNING,
+ (errmsg(created user with password that cannot
log in)));
+
/* Check some permissions first */
if (issuper)
{
***
*** 649,654
--- 654,664
DirectFunctionCall1(textin,
CStringGetDatum(encrypted_password));
}
new_record_repl[Anum_pg_authid_rolpassword - 1] = 'r';
+ /* Warn about combination that's likely incorrect */
+ if (canlogin == 0 ||
+ Form_pg_authid) GETSTRUCT(tuple))-rolcanlogin == 0)
canlogin != 1))
+ ereport(WARNING,
+ (errmsg(set password for a user that
cannot log in)));
}
/* unset password */
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] rolcanlogin vs. the flat password file
* Magnus Hagander ([EMAIL PROTECTED]) wrote: I think that's a good idea. Attached is a patch that implements this (I think - haven't messed around in that area of the code before). Thoughts? Cool, thanks! My only comment is that you should probably stick to one 'zero' convention- either '!canlogin' or 'canlogin == 0'. I prefer the former, but the inconsistancy in a single patch is kind of odd. I'm not sure if there's an overall PG preference. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] rolcanlogin vs. the flat password file
Magnus Hagander wrote: On Sun, Oct 14, 2007 at 06:16:04PM -0400, Stephen Frost wrote: * Tom Lane ([EMAIL PROTECTED]) wrote: Stephen Frost [EMAIL PROTECTED] writes: I wonder if the OP was unhappy because he created a role w/ a pw and then couldn't figure out why the user couldn't log in? Hm, maybe. In that case just not filtering the entry out of the flat file would be good enough. I've confirmed the confusing behavior in CVS HEAD. With password auth selected in pg_hba.conf: [...] Should we just do this, or is it worth working harder? I certainly like this. Honestly, I'd also like the warning when doing a 'create role'/'alter role' that sets/changes the pw on an account that doesn't have 'rolcanlogin'. Much better to have me notice that I goof'd the command and fix it before telling the user 'go ahead and log in' than to have the user complain that it's not working. :) Just my 2c. I think that's a good idea. Attached is a patch that implements this (I think - haven't messed around in that area of the code before). Thoughts? Is WARNING an appropriate level for this? I think NOTICE is enough, it's not like something bad is going to happen if you do that, it just means that you've likely screwed up. There's legitimate use for creating a role with NOLOGIN and a password. Maybe you're going to give login privilege later on. It wouldn't be nice to get WARNINGs in that case, even NOTICEs would be sligthly annoying. Note that per-role guc variables will also have no effect on a role with no login privilege. How about connection limit, is that inherited? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] rolcanlogin vs. the flat password file
* Tom Lane ([EMAIL PROTECTED]) wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: There's legitimate use for creating a role with NOLOGIN and a password. If we think that, then we shouldn't have a message at all. I'm not sure I agree with that. I don't agree that there's really a legitimate use for creating a role w/ NOLOGIN and a password either, for that matter. A 'NOTICE' level message would be fine with me. We have NOTICE messages for when we create an index for a PK. I find a message about an entirely unexpected and unworkable configuration alot more useful than those. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] rolcanlogin vs. the flat password file
Heikki Linnakangas [EMAIL PROTECTED] writes: There's legitimate use for creating a role with NOLOGIN and a password. If we think that, then we shouldn't have a message at all. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] rolcanlogin vs. the flat password file
Stephen Frost wrote: * Tom Lane ([EMAIL PROTECTED]) wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: There's legitimate use for creating a role with NOLOGIN and a password. If we think that, then we shouldn't have a message at all. I'm not sure I agree with that. I don't agree that there's really a legitimate use for creating a role w/ NOLOGIN and a password either, for that matter. Preparing a new user account prior to an employee starting? In my last post we would do that regularly - setup all the accounts etc for the new user, but disable them all until the start date. /D ---(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
Re: [HACKERS] rolcanlogin vs. the flat password file
On Wed, Oct 17, 2007 at 05:09:25PM +0100, Dave Page wrote: Stephen Frost wrote: * Tom Lane ([EMAIL PROTECTED]) wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: There's legitimate use for creating a role with NOLOGIN and a password. If we think that, then we shouldn't have a message at all. I'm not sure I agree with that. I don't agree that there's really a legitimate use for creating a role w/ NOLOGIN and a password either, for that matter. Preparing a new user account prior to an employee starting? In my last post we would do that regularly - setup all the accounts etc for the new user, but disable them all until the start date. Yeah, but did you actually set a password for them? We do that all the time here, but we don't set the passwords until they show up. //Magnus ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] rolcanlogin vs. the flat password file
On Wed, Oct 17, 2007 at 11:27:10AM -0400, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: There's legitimate use for creating a role with NOLOGIN and a password. If we think that, then we shouldn't have a message at all. At least if we think it's more than a very narrow legitimate use, compared to the number of ppl making the mistake. I agree with making it a NOTICE instead of WARNING though. //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] rolcanlogin vs. the flat password file
Magnus Hagander wrote: On Wed, Oct 17, 2007 at 05:09:25PM +0100, Dave Page wrote: Stephen Frost wrote: * Tom Lane ([EMAIL PROTECTED]) wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: There's legitimate use for creating a role with NOLOGIN and a password. If we think that, then we shouldn't have a message at all. I'm not sure I agree with that. I don't agree that there's really a legitimate use for creating a role w/ NOLOGIN and a password either, for that matter. Preparing a new user account prior to an employee starting? In my last post we would do that regularly - setup all the accounts etc for the new user, but disable them all until the start date. Yeah, but did you actually set a password for them? Yeah, then have them change them all during day 1 IT induction training. We had a much smaller team that I know you do, and the staff that would do the account setup would often be busy first thing on Monday morning when new starters might often arrive - so we would just 'flip the switch' on the pre-configured accounts. /D ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] rolcanlogin vs. the flat password file
Tom Lane wrote: With the attached patch to not drop nologin roles from the flat password file, it acts more sanely: postgres=# create user foo nologin; CREATE ROLE postgres=# \c - foo Password for user foo: FATAL: password authentication failed for user foo Previous connection kept postgres=# alter user foo password 'foo'; ALTER ROLE postgres=# \c - foo Password for user foo: correct password entered here FATAL: role foo is not permitted to log in Previous connection kept Should we just do this, or is it worth working harder? IMHO this is exactly what we want. It does only offer more information when you already got authentication right and therefore doesn't open an information leak. Not sure about the warning when creating a role with a password but nologin. Could be useful. Best Regards Michael Paesold ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] rolcanlogin vs. the flat password file
On Oct 14, 2007, at 14:34 , Tom Lane wrote: I am not entirely convinced whether we should do anything about this: the general theory on authentication failures is that you don't say much about exactly why it failed, so as to not give a brute-force attacker any info about whether he gave a valid userid or not. So there's an argument to be made that the current behavior is what we want. But I'm pretty sure that it wasn't intentionally designed to act this way. Would there be a difference in how this is logged and how it's reported to the user? I can see where an admin (having access to logs) would want to have additional information such as whether a role login has failed due to not having login privileges or whether the failure was due to an incorrect role/password pair. I lean towards less information back to the user as to the nature of the failure. If the general consensus is to leave the current behavior, a comment should probably be included to note that the behavior is intentional. Michael Glaesemann grzm seespotcode net ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] rolcanlogin vs. the flat password file
* Tom Lane ([EMAIL PROTECTED]) wrote: We could certainly change flatfiles.c to disregard rolcanlogin, which'd actually make the code simpler. However, that in itself wouldn't change the behavior, unless you were to assign a password to the NOLOGIN role which seems a fairly strange thing to do. I think what the OP wishes is that not permitted to log in would be checked before checking password validity, and to do that we'd have to add rolcanlogin to the flat password file and put the check somewhere upstream of the authentication process. I wonder if the OP was unhappy because he created a role w/ a pw and then couldn't figure out why the user couldn't log in? I've run into that in the past and it takes some leg-work to figure out what's going on. A warning on a 'create role' or 'alter role' command which sets a password when 'rolcanlogin' is false might be an alternative way to 'fix' this. In general, I would say that it's correct to say 'invalid authentication'/'bad pw' until the user is authenticated and then say 'not permitted to log in' if they're not authorized (don't have rolcanlogin), which is I think what we do. That combined with the warning above would, I think, cover most of problem cases. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] rolcanlogin vs. the flat password file
Michael Glaesemann [EMAIL PROTECTED] writes: Would there be a difference in how this is logged and how it's reported to the user? Not without making all the same infrastructure changes that would be needed to tell the user something different than now. As things stand, the password auth code can't tell the difference between a nonexistent role and a nologin role; neither one has an entry in the flat file. If we dropped the filtering in flatfiles.c, then a nologin role would have an entry, but most likely without a password, so you'd still just see password auth failed. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] rolcanlogin vs. the flat password file
Stephen Frost [EMAIL PROTECTED] writes: * Tom Lane ([EMAIL PROTECTED]) wrote: ... I think what the OP wishes is that not permitted to log in would be checked before checking password validity, and to do that we'd have to add rolcanlogin to the flat password file and put the check somewhere upstream of the authentication process. I wonder if the OP was unhappy because he created a role w/ a pw and then couldn't figure out why the user couldn't log in? Hm, maybe. In that case just not filtering the entry out of the flat file would be good enough. In hindsight I'm not sure why we indulged in that bit of complication anyway --- it seems unlikely that an installation would have so many nologin roles, compared to regular ones, that the increase in size of the flat file would be objectionable. In general, I would say that it's correct to say 'invalid authentication'/'bad pw' until the user is authenticated and then say 'not permitted to log in' if they're not authorized (don't have rolcanlogin), which is I think what we do. That *would* be the behavior if we removed the filtering. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] rolcanlogin vs. the flat password file
I wrote:
Stephen Frost [EMAIL PROTECTED] writes:
I wonder if the OP was unhappy because he created a role w/ a pw and
then couldn't figure out why the user couldn't log in?
Hm, maybe. In that case just not filtering the entry out of the flat
file would be good enough.
I've confirmed the confusing behavior in CVS HEAD. With password auth
selected in pg_hba.conf:
postgres=# create user foo nologin;
CREATE ROLE
postgres=# \c - foo
Password for user foo:
FATAL: password authentication failed for user foo
Previous connection kept
postgres=# alter user foo password 'foo';
ALTER ROLE
postgres=# \c - foo
Password for user foo: correct password entered here
FATAL: password authentication failed for user foo
Previous connection kept
With the attached patch to not drop nologin roles from the flat password
file, it acts more sanely:
postgres=# create user foo nologin;
CREATE ROLE
postgres=# \c - foo
Password for user foo:
FATAL: password authentication failed for user foo
Previous connection kept
postgres=# alter user foo password 'foo';
ALTER ROLE
postgres=# \c - foo
Password for user foo: correct password entered here
FATAL: role foo is not permitted to log in
Previous connection kept
Should we just do this, or is it worth working harder?
regards, tom lane
*** src/backend/utils/init/flatfiles.c.orig Wed Aug 1 18:45:08 2007
--- src/backend/utils/init/flatfiles.c Sun Oct 14 17:14:27 2007
***
*** 298,304
*
* The format for the flat auth file is
*rolename password validuntil memberof memberof ...
- * Only roles that are marked rolcanlogin are entered into the auth file.
* Each role's line lists all the roles (groups) of which it is directly
* or indirectly a member, except for itself.
*
--- 298,303
***
*** 312,318
typedef struct
{
Oid roleid;
- boolrolcanlogin;
char *rolname;
char *rolpassword;
char *rolvaliduntil;
--- 311,316
***
*** 407,414
tempname)));
/*
!* Read pg_authid and fill temporary data structures. Note we must read
!* all roles, even those without rolcanlogin.
*/
totalblocks = RelationGetNumberOfBlocks(rel_authid);
totalblocks = totalblocks ? totalblocks : 1;
--- 405,411
tempname)));
/*
!* Read pg_authid and fill temporary data structures.
*/
totalblocks = RelationGetNumberOfBlocks(rel_authid);
totalblocks = totalblocks ? totalblocks : 1;
***
*** 433,439
}
auth_info[curr_role].roleid = HeapTupleGetOid(tuple);
- auth_info[curr_role].rolcanlogin = aform-rolcanlogin;
auth_info[curr_role].rolname = pstrdup(NameStr(aform-rolname));
auth_info[curr_role].member_of = NIL;
--- 430,435
***
*** 565,574
List *roles_names_list = NIL;
ListCell *mem;
- /* We can skip this for non-login roles */
- if (!auth_info[curr_role].rolcanlogin)
- continue;
-
/*
* This search algorithm is the same as in
is_member_of_role; we
* are just working with a different input data
structure.
--- 561,566
***
*** 642,650
for (curr_role = 0; curr_role total_roles; curr_role++)
{
auth_entry *arole = auth_info[curr_role];
-
- if (arole-rolcanlogin)
- {
ListCell *mem;
fputs_quote(arole-rolname, fp);
--- 634,639
***
*** 660,666
}
fputs(\n, fp);
- }
}
if (FreeFile(fp))
--- 649,654
---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
Re: [HACKERS] rolcanlogin vs. the flat password file
Tom Lane wrote: Should we just do this, or is it worth working harder? Not worth more, IMNSHO. cheers andrew ---(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
Re: [HACKERS] rolcanlogin vs. the flat password file
* Tom Lane ([EMAIL PROTECTED]) wrote: Stephen Frost [EMAIL PROTECTED] writes: I wonder if the OP was unhappy because he created a role w/ a pw and then couldn't figure out why the user couldn't log in? Hm, maybe. In that case just not filtering the entry out of the flat file would be good enough. I've confirmed the confusing behavior in CVS HEAD. With password auth selected in pg_hba.conf: [...] Should we just do this, or is it worth working harder? I certainly like this. Honestly, I'd also like the warning when doing a 'create role'/'alter role' that sets/changes the pw on an account that doesn't have 'rolcanlogin'. Much better to have me notice that I goof'd the command and fix it before telling the user 'go ahead and log in' than to have the user complain that it's not working. :) Just my 2c. Thanks, Stephen signature.asc Description: Digital signature
