[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Best practices on system users and groups

hi javier,

On Mon, Oct 31, 2005 at 10:03:01PM +0100, Javier Fernández-Sanguino Peña wrote:
> I would like developers to review and provide feedback for that section,

thanks for actually putting this into a document, however, i notice
two problems:

- the addgroup/adduser functions mask the error status, yet do not
  later check to see if the group was actually created.   this is doubly
  bad, because not only will the package continue to install, but no
  error message or warning is even printed.  here's a patch for adding a group:

--- foo 2005-11-01 10:29:15.000000000 +0100
+++ bar 2005-11-01 10:32:27.000000000 +0100
@@ -28,8 +28,13 @@
             # 1. create group if not existing
             if ! getent group | grep -q "^$SERVER_GROUP:" ; then
                   echo -n "Adding group $SERVER_GROUP.."
-                  addgroup --quiet --system $SERVER_GROUP 2>/dev/null ||true
-                 echo "..done"
+                  addgroup --quiet --system $SERVER_GROUP ||true
+                  if ! getent group | grep -q "^$SERVER_GROUP:"; then
+                      echo "...error encountered"
+                      exit 1
+                  else
+                      echo "...done"
+                  fi

  ... and the same for adding users.

  the second thing i notice is that you arbitrarily modify the user in
  question via usermod, which would override the local admin's changes.
  i wonder whether it is even good to recommend this at all.

On Tue, Nov 01, 2005 at 11:14:59AM +0200, Lars Wirzenius wrote:
> Also, sticking all the tens of lines of boilerplate code into the
> postinst of every package that needs a system user is a good way to
> invite trouble. When the boilerplate has a bug (possibly because things
> change in the future), it will have to be fixed in dozes on of packages.
> It's oh so much more sensible to create a tool that postinsts can call:
> if boilerplate code is good enough, then it can easily be abstracted
> away.

i'd like to double-ack this remark.  even if the oft-mentioned "dh_user"
were implemented, if there were a bug in this implementation, every
affected package would have to be /rebuilt/ if the buggy code were
actually in the postinsts.

if you're going to do this, it would be better to provide a program
or a shell library that is sourced in the postinst, and then
awrapper function which does all of this.  



Attachment: signature.asc
Description: Digital signature

Reply to: