Re: [PATCHES] [BUGS] BUG #1134: ALTER USER ... RENAME breaks md5 passwords

2004-05-06 Thread Bruce Momjian
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <[EMAIL PROTECTED]> writes:
> > > The attached patch clears the password field on rename:
> > 
> > I think you should clear the password field *only* if it's
> > MD5-encrypted.
> 
> Patch attached and applied.

Oh, I forgot to display the new behavior:

test=> CREATE USER test;
CREATE USER
test=> ALTER USER test RENAME TO test2;
ALTER USER
test=> ALTER USER test2 UNENCRYPTED PASSWORD 'x';
ALTER USER
test=> ALTER USER test2 RENAME TO test4;
ALTER USER
test=> ALTER USER test4 PASSWORD 'x';
ALTER USER
test=> ALTER USER test4 RENAME TO test8;
NOTICE:  MD5 password cleared because of user rename
ALTER USER
test=> SELECT * FROM pg_shadow WHERE usename = 'test8';
 usename | usesysid | usecreatedb | usesuper | usecatupd | passwd |
valuntil | useconfig

-+--+-+--+---++--+---

 test8   |  100 | f   | f| f || 
|
(1 row)

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])


Re: [PATCHES] [BUGS] BUG #1134: ALTER USER ... RENAME breaks md5 passwords

2004-05-06 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > The attached patch clears the password field on rename:
> 
> I think you should clear the password field *only* if it's
> MD5-encrypted.

Patch attached and applied.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/alter_user.sgml
===
RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/alter_user.sgml,v
retrieving revision 1.32
diff -c -c -r1.32 alter_user.sgml
*** doc/src/sgml/ref/alter_user.sgml29 Nov 2003 19:51:38 -  1.32
--- doc/src/sgml/ref/alter_user.sgml5 May 2004 03:06:44 -
***
*** 57,62 
--- 57,65 
 The second variant changes the name of the user.  Only a database
 superuser can rename user accounts.  The session user cannot be
 renamed.  (Connect as a different user if you need to do that.)
+Because MD5-encrypted passwords use the username as
+cryptographic salt, renaming a user clears their MD5
+password.

  

Index: src/backend/commands/user.c
===
RCS file: /cvsroot/pgsql-server/src/backend/commands/user.c,v
retrieving revision 1.139
diff -c -c -r1.139 user.c
*** src/backend/commands/user.c 16 Mar 2004 05:05:57 -  1.139
--- src/backend/commands/user.c 5 May 2004 03:06:46 -
***
*** 959,966 
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg("user \"%s\" does not exist", stmt->user)));
  
!   if (!(superuser()
!|| ((Form_pg_shadow) GETSTRUCT(oldtuple))->usesysid == GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 errmsg("permission denied")));
--- 959,966 
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg("user \"%s\" does not exist", stmt->user)));
  
!   if (!(superuser() ||
!   ((Form_pg_shadow) GETSTRUCT(oldtuple))->usesysid == GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 errmsg("permission denied")));
***
*** 1157,1172 
  void
  RenameUser(const char *oldname, const char *newname)
  {
!   HeapTuple   tup;
Relationrel;
! 
/* ExclusiveLock because we need to update the password file */
rel = heap_openr(ShadowRelationName, ExclusiveLock);
  
!   tup = SearchSysCacheCopy(SHADOWNAME,
 CStringGetDatum(oldname),
 0, 0, 0);
!   if (!HeapTupleIsValid(tup))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg("user \"%s\" does not exist", oldname)));
--- 1157,1181 
  void
  RenameUser(const char *oldname, const char *newname)
  {
!   HeapTuple   oldtuple,
!   newtuple;
!   TupleDesc   dsc;
Relationrel;
!   Datum   datum;
!   boolisnull;
!   Datum   repl_val[Natts_pg_shadow];
!   charrepl_null[Natts_pg_shadow];
!   charrepl_repl[Natts_pg_shadow];
!   int i;
!   
/* ExclusiveLock because we need to update the password file */
rel = heap_openr(ShadowRelationName, ExclusiveLock);
+   dsc = RelationGetDescr(rel);
  
!   oldtuple = SearchSysCache(SHADOWNAME,
 CStringGetDatum(oldname),
 0, 0, 0);
!   if (!HeapTupleIsValid(oldtuple))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg("user \"%s\" does not exist", oldname)));
***
*** 1177,1183 
 * not be an actual problem besides a little confusion, so think about
 * this and decide.
 */
!   if (((Form_pg_shadow) GETSTRUCT(tup))->usesysid == GetSessionUserId())
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 errmsg("session user may not be renamed")));
--- 1186,1192 
 * not be an actual problem besides a little confusion, so think about
 * this and decide.
 */
!   if (((Form_pg_shadow) GETSTRUCT(oldtuple))->usesysid == GetSessionUserId())
ereport(ERROR,
  

Re: [PATCHES] [BUGS] BUG #1134: ALTER USER ... RENAME breaks md5 passwords

2004-04-30 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > The attached patch clears the password field on rename:
> 
> I think you should clear the password field *only* if it's
> MD5-encrypted.

I thought about that but it seems strange to conditionally do the
clearing, but if you think we should, I can do it.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]


Re: [PATCHES] [BUGS] BUG #1134: ALTER USER ... RENAME breaks md5 passwords

2004-04-30 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes:
> The attached patch clears the password field on rename:

I think you should clear the password field *only* if it's
MD5-encrypted.

regards, tom lane

---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster


Re: [PATCHES] [BUGS] BUG #1134: ALTER USER ... RENAME breaks md5 passwords

2004-04-26 Thread Bruce Momjian
PostgreSQL Bugs List wrote:
> 
> The following bug has been logged online:
> 
> Bug reference:  1134
> Logged by:  Fabien COELHO
> 
> Email address:  [EMAIL PROTECTED]
> 
> PostgreSQL version: 7.5 Dev
> 
> Operating system:   any
> 
> Description:ALTER USER ... RENAME breaks md5 passwords
> 
> Details: 
> 
> If you rename a user with a md5 password, the
> password is broken. md5 passwords are the default,
> so it means that renaming a user with a password
> does not work by default.
> 
> This is because the username is used implicitly as salt. This was a bad idea 
> (tm). 
> 
> Fixing this has implications on the client/server
> protocol for md5 authentication. If you're going
> to fix it some day, consider also adding more
> characters to the server nonce used in the protocol.

Yes, the problem is that we used the username for the salt, just like
FreeBSD does for its MD5 passwords.  Of course, you can't rename unix
users, while PostgreSQL allows user renaming.

The attached patch clears the password field on rename:

test=> CREATE USER pass password 'aa';
CREATE USER
test=> ALTER USER pass RENAME TO pass2;
NOTICE:  password cleared because OF USER RENAME
ALTER USER
test=> ALTER USER pass2 RENAME TO pass3;
ALTER USER

and adds documention explaining this behavior.  I can't think of a
better solution.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/alter_user.sgml
===
RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/alter_user.sgml,v
retrieving revision 1.32
diff -c -c -r1.32 alter_user.sgml
*** doc/src/sgml/ref/alter_user.sgml29 Nov 2003 19:51:38 -  1.32
--- doc/src/sgml/ref/alter_user.sgml27 Apr 2004 00:29:56 -
***
*** 57,62 
--- 57,64 
 The second variant changes the name of the user.  Only a database
 superuser can rename user accounts.  The session user cannot be
 renamed.  (Connect as a different user if you need to do that.)
+Because MD5-encrypted passwords use the username as
+cryptographic salt, renaming a user clears their password.

  

Index: src/backend/commands/user.c
===
RCS file: /cvsroot/pgsql-server/src/backend/commands/user.c,v
retrieving revision 1.139
diff -c -c -r1.139 user.c
*** src/backend/commands/user.c 16 Mar 2004 05:05:57 -  1.139
--- src/backend/commands/user.c 27 Apr 2004 00:29:57 -
***
*** 959,966 
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg("user \"%s\" does not exist", stmt->user)));
  
!   if (!(superuser()
!|| ((Form_pg_shadow) GETSTRUCT(oldtuple))->usesysid == GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 errmsg("permission denied")));
--- 959,966 
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg("user \"%s\" does not exist", stmt->user)));
  
!   if (!(superuser() ||
!   ((Form_pg_shadow) GETSTRUCT(oldtuple))->usesysid == GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 errmsg("permission denied")));
***
*** 1157,1172 
  void
  RenameUser(const char *oldname, const char *newname)
  {
!   HeapTuple   tup;
Relationrel;
! 
/* ExclusiveLock because we need to update the password file */
rel = heap_openr(ShadowRelationName, ExclusiveLock);
  
!   tup = SearchSysCacheCopy(SHADOWNAME,
 CStringGetDatum(oldname),
 0, 0, 0);
!   if (!HeapTupleIsValid(tup))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg("user \"%s\" does not exist", oldname)));
--- 1157,1177 
  void
  RenameUser(const char *oldname, const char *newname)
  {
!   HeapTuple   oldtuple,
!   newtuple;
Relationrel;
!   Datum   repl_val[Natts_pg_shadow];
!   charrepl_null[Natts_pg_shadow];
!   charrepl_repl[Natts_pg_shadow];
!   int i;
!   
/* ExclusiveLock because we need to update the password file */
rel = heap_openr(ShadowRelationName, ExclusiveLock);
  
!   oldtuple = SearchSysCache(SHADOWNAME,