Re: [BUGS] BUG #1134: ALTER USER ... RENAME breaks md5 passwords - Mailing list pgsql-patches

PostgreSQL Bugs List wrote:
>
> The following bug has been logged online:
>
> Bug reference:      1134
> Logged by:          Fabien COELHO
>
> Email address:      coelho@cri.ensmp.fr
>
> 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
  pgman@candle.pha.pa.us               |  (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.sgml    29 Nov 2003 19:51:38 -0000    1.32
--- doc/src/sgml/ref/alter_user.sgml    27 Apr 2004 00:29:56 -0000
***************
*** 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 <literal>MD5</>-encrypted passwords use the username as
+    cryptographic salt, renaming a user clears their password.
    </para>

    <para>
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 -0000    1.139
--- src/backend/commands/user.c    27 Apr 2004 00:29:57 -0000
***************
*** 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;
      Relation    rel;
!
      /* 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;
      Relation    rel;
!     Datum        repl_val[Natts_pg_shadow];
!     char        repl_null[Natts_pg_shadow];
!     char        repl_repl[Natts_pg_shadow];
!     int            i;
!
      /* ExclusiveLock because we need to update the password file */
      rel = heap_openr(ShadowRelationName, ExclusiveLock);

!     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")));
--- 1182,1188 ----
       * 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,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                   errmsg("session user may not be renamed")));
***************
*** 1196,1208 ****
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("must be superuser to rename users")));

!     /* rename */
!     namestrcpy(&(((Form_pg_shadow) GETSTRUCT(tup))->usename), newname);
!     simple_heap_update(rel, &tup->t_self, tup);
!     CatalogUpdateIndexes(rel, tup);

      heap_close(rel, NoLock);
-     heap_freetuple(tup);

      user_file_update_needed = true;
  }
--- 1201,1231 ----
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   errmsg("must be superuser to rename users")));

!     for (i = 0; i < Natts_pg_shadow; i++)
!         repl_repl[i] = ' ';
!
!     repl_repl[Anum_pg_shadow_usename - 1] = 'r';
!     repl_val[Anum_pg_shadow_usename - 1] = DirectFunctionCall1(namein,
!                                             CStringGetDatum(newname));
!     repl_null[Anum_pg_shadow_usename - 1] = ' ';

+     if (!heap_attisnull(oldtuple, Anum_pg_shadow_passwd))
+     {
+         /* MD5 uses the username as salt, so just clear it on a rename */
+         repl_repl[Anum_pg_shadow_passwd - 1] = 'r';
+         repl_null[Anum_pg_shadow_passwd - 1] = 'n';
+
+         ereport(NOTICE,
+             (errmsg("password cleared because of user rename")));
+     }
+
+     newtuple = heap_modifytuple(oldtuple, rel, repl_val, repl_null, repl_repl);
+     simple_heap_update(rel, &oldtuple->t_self, newtuple);
+
+     CatalogUpdateIndexes(rel, newtuple);
+
+     ReleaseSysCache(oldtuple);
      heap_close(rel, NoLock);

      user_file_update_needed = true;
  }