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

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> 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
  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    5 May 2004 03:06:44 -0000
***************
*** 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 <literal>MD5</>-encrypted passwords use the username as
+    cryptographic salt, renaming a user clears their <literal>MD5</>
+    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    5 May 2004 03:06:46 -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,1181 ----
  void
  RenameUser(const char *oldname, const char *newname)
  {
!     HeapTuple    oldtuple,
!                 newtuple;
!     TupleDesc    dsc;
      Relation    rel;
!     Datum        datum;
!     bool        isnull;
!     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);
+     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,
                  (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;
  }
--- 1205,1237 ----
                  (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] = ' ';
!
!     datum = heap_getattr(oldtuple, Anum_pg_shadow_passwd, dsc, &isnull);
!
!     if (!isnull && isMD5(DatumGetCString(DirectFunctionCall1(textout, datum))))
!     {
!         /* 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("MD5 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;
  }