thanks

Background for the policy list: see thread starting at
http://lists.debian.org/debian-devel/2011/03/msg01174.html
and continuing in April at
http://lists.debian.org/debian-devel/2011/04/msg00210.html

Ian and Tollef and Scott Kitterman are against removal of system users,
and nobody (except, very mildly, me) is for their removal, so I guess
the consensus on -devel is clear: we should not remove system users,
ever, in maintainer scripts. If an admin wants to do it manually, that
is, of course, OK.

Thus, I propose to change 9.2.2 "UID and GID classes", the paragraph on
uids in the range 100-999, to add the following sentence to the end of
the paragraph:

        Packages must not remove system users and groups they have
        created.

Not sure if a mass bug filing is warranted if this policy change is
accepted, but definitely a lintian check would be in order (I'm happy to
write it).

Adding a copy to the bug report.

Everyone please Cc 621833@bugs.debian.org if replying to this subhtread.
Thanks.

I agree that the accounts should not be deleted, but that the packages
should still be responsible for certain forms of cleanup:

 - removing the user home directory (on purge?)
 - locking the account
 - (optional) scanning the filesystem to clean up any other files owned by
   the user

This is the good kind of cleanup to do.  Deleting the account entirely is
the bad kind of cleanup, because you can never guarantee that you've gotten
*all* the files belonging to that user/group, thanks to removable media; so
if the UID is reused, some other account gets access to files it wasn't
meant to.

I don't think dpkg-maintscript-helper is the right layer of abstraction for
something like this; we already have an imperative interface for account
creation/deletion, which is adduser/deluser, and if that interface isn't
sufficiently straightforward we should remedy that directly.

I'm not sure if debhelper can help here.  I guess we would need a new config
file (debian/users?), but I'm not sure it could be done with a very
debhelper-like syntax.

Hi all,

For locking the account, I think it could be problematic if you have some
kind of central account management system (i.e. LDAP/AD), and you don't
want to lock it globally.


	sean

sean finney <seanius@seanius.net> writes:

Yeah, but adduser doesn't ever do anything with central account management
systems anyway, so far as I know, so you could tell adduser to lock it and
if adduser can't find it in the local /etc/passwd or /etc/shadow, it would
just give up.

I was always given the impression that adduser and friends "wanted" to be
able to handle non-local accounts, but nobody had ever extended it to do
so?  So I think it's a bit shaky to make that assumption.

But if we specifically limit the scope for users/groups being locked to
"only if they're in /etc/passwd,/etc/group" then yes I think that the
recommendation makes sense.  But then we probably ought to also have
some boilerplate examples of exactly how it should be done.

On that note, I just read over 9.2 and see we don't have anything about
the right behavior for adding users/groups there either, and you have
similar problems along those lines.  Actually it seems that 9.2 as a
whole could use a bit of a facelift :)



	sean

sean finney <seanius@seanius.net> writes:

If that's really a future intention, maybe add a no-op --local flag to
adduser that says not to do that, should it ever have been added?

Yes.

(Cc to the relevant bug added.)

I think that's a good idea. Steve Langasek in the bug (#621833) and
others agree, so I think there's a strong consensus on that.

Hi Lars,

I don't think I'd agree there, at least without also addressing:

 * It also needs to limit the scope to locally defined users (i.e. not
   fail when it is unable to lock an NIS/LDAP/etc account).
 * There needs to be a way to explicitly do that with adduser or a similar
   tool[1][2][3][4].

Also, we haven't discussed what should be done in the case of a user
account possibly shared between different packages, where any one of
them may create it and 1..N of them might be installed.  For example,
nagios/nrpe comes to mind, or maybe parallel installed postgres versions
and related tools.  Alternatively, if we strictly interpret what
Ian suggested to not include such packages, we should state that and/or
give alternate instructions for this case.


I second your original proposal though, that packages must not delete
system users that they have created.  I don't think anyone had objections
to that, and the question is whether things should be taken further.


	sean

[1] Assuming it must be done with one tool, it should also state "lock
    users with $thiscommand $theseoptions", and depend on "$thisversion"
	of the tool in question if the tool needs to be updated.
[2] And if "$theseoptions" are not specifically for the "local only case",
    we need a way to tell the difference between a "locking failed" error and
	a "user is remote" error.
[3] Or maybe instead of using a tool we have something like declarative
    users, where we state policy and farm it off to dpkg to implement?
[4] Whoa, footnotespam ftw, sorry.

Also, we need to provide a way for sysadmin to know they can safely remove a stale
system user.

Cheers,

If we could do that, we could just remove them automatically and not
bother the sysadmin.

Scott K

Yes, and these were already suggested in the bug log, if I've undertood
everyone correctly (not all those mails were on -devel, though).

In my opinion, those packages should arrange for things to work right
amongst themselves. The typical case would be to have a -common package,
which creates and locks down the user, and everything else depends on
it. But other options are also possible; I guess anything that achieves
the same effect should be OK by the policy manual.

Not necessarily. We can't be sure there aren't any files lying around
(at least not efficiently enough) to cause problems with UID reuse etc,
but we may inform the admin that at least from the packaging point of
view, the user/group isn't needed anymore. He can then decide to take
appropriate action if he feels the house-keeping is necessary.
It could simply be a matter of using the "User Name/Comment" field to
write something like "formerly used by package X; may be removed".
Admittedly not strictly necessary, but nice for those cases where you
check your /etc/passwd a few years later and ask yourself where that
user came from.


Cheers

I do object to telling maintainers they must not delete system users,
without also giving guidance on how and when to lock the accounts.

Sorry, no time at the moment to propose verbiage to reconcile this with your
concerns.

Steve Langasek writes ("Re: Bug#621833: System users: removing them"):

Yes, I agree with this.

I think the right thing to do would be to have deluser lock (rather
than delete) system users when invoked in the way currently used by
maintainer scripts.  Provided that doesn't make interactive use of
deluser break somehow.

Ian.

* Ian Jackson (ijackson@chiark.greenend.org.uk) [110501 16:39]:

Good idea.


I agree that system users should never be removed by maintainer
scripts, but as said: Someone would need to write that down before
starting to behave so.


Andi

I am managing a package that does 'userdel' in a purge. It removes the
home directory as that contains config files. I am a bit concerned about
disabling the account further, because then I will have to add more
logic about reenabling it in certain scenarios.

I've been looking at how this might be accomplished right now, and
have these observations to make.  (These are WRT my addition and
removal of the "sbuild" user in the sbuild package.)

1) Locking on removal.

  This is as simple as doing (in postrm)

    # Lock sbuild account.
    usermod -U -e 1 sbuild

However, one does now need to consider how "unlocking" will occur
if the package is reinstalled, which I don't think has been covered
as yet:

2) Reinstallation.

   I'm currently using this logic (in postinst)

     # Create dedicated sbuild user
     if ! getent passwd sbuild > /dev/null; then
         adduser --system --quiet --home /var/lib/sbuild --no-create-home \
         --shell /bin/bash --ingroup sbuild --gecos "Debian source builder" sbuild
     fi

  However, consider that if the account is locked, the user already
  exists and no unlocking will occur, leaving the reinstalled
  package broken.  This logic is common to many packages.

  I've added this after the above to unlock if locked:

    # Unlock account in case it was locked from previous purge.
    usermod -U -e '' sbuild

  This appears to reverse the locking, via inspection of the
  shadow record.  However, "" isn't documented as a valid
  value for EXPIRE_DATE (it's the default in /etc/default/useradd
  though).

Given the need to consider unlocking as well as locking, I'm not sure
it's worth adding special support to deluser: the typical logic used
to create the user will be insufficient to unlock, so it's no less
the effort to add an explict unlock/lock to the maintainer scripts,
dropping use of deluser entirely.

I do agree that a --local option would be a valuable and useful
addition to the adduser and deluser etc. tools, even if currently
a no-op.  However, due to the above I don't think that adding
special-case user locking to deluser is the correct course of action.


Regards,
Roger

Oops, should of course be "usermod -L -e 1 sbuild"

(culled cc list of a few people I know read -devel)
Roger Leigh wrote:

The obvious question then would be whether it's worth adding special
support to deluser *and* adduser, no?

I don't have an answer in mind, though the lazy person in me would
like it to work.

We could add special behaviour to adduser to unlock the account
if it already exists when run in the postinst.  However, most
postinsts wrap the call to adduser with a check for whether the
account already exists, so it would not be called without an
update to every preinst employing this strategy.  It would also
alter the existing behaviour of adduser, which is to return nonzero
if the user already exists, which could cause breakage.

I dislike the fact that the behaviour of adduser and deluser would,
in effect, /not/ add or delete users as intended, which is rather
counter-intuitive.  Providing that we have consensus on a recommended
strategy for locking and unlocking accounts which can go into policy,
I think all we need are examples for how maintainer scripts are
expected to handle account creation and locking/unlocking.


Regards,
Roger

Cheking for the account already being present in postinst should not
be necessary. Adduser is designed to do the right thing without
additional logic in maintainer scripts. If it doesn't, please file a
bug.

If people find it desireable to automatically unlock an existing
account on adduser --system <name>, this could easily be implemented
in adduser, doing the right thing to locked accounts. If that may be
necessary, please file a bug against adduser.

What should the --local option do? If you want adduser to grow this
option, please file a bug.

Greetings
Marc

Yes.

Which would be a bug in the maintainer scripts.

adduser --system is designed (and, IIRC, documented) to have the
effect of "after the call to adduser --system, the account will exist
and is useable. The only case when adduser --system really errors out
is when the account already exists but is not a system account."

The would be rather easy. Account creation/unlocking would happen with
an unwrapped call to adduser --system, account locking with a call to
the appropriate back-end command, or we could add an lockuser command
to the adduser package. I think, the latter would be preferable since
we would then be able to add sugar to the locking process. A wishlist
bug against adduser is in order.

Greetings
Marc, with a rather worn and dusty adduser hat on

I've seen that cause data loss.  You must make sure the homedir is
exactly as you set it when you created the system user.  If it isn't,
abort.

If you want to know how bad it can be, we've had once packages that had
/ set as the home dir of system users they created.

I've just reviewed the discussion so far, here's my best attempt
at a summary of the current status:

* To create an user, a maintainer script should call
  "adduser --system foo". It is not necessary to wrap this in
  a check for whether the user exists.
* When the package is removed, the user should be locked:
  "lockuser foo".
* lockuser is a still-hypothetical tool, which needs to be added
  to the adduser package. It is a wrapper around "usermod -L -e 1 foo".
* Similarly, adduser needs to be changed to unlock:
  "usermod -U -e '' foo".
* Policy 9.2.2, the description of the 100-999 UID range for system
  users, should be changed to mention when and how users need to
  be locked. Perhaps by adding the following sentence to the end of
  the paragraph: "When the package is removed, it should lock the
  user it created using 'lockuser'."
* We need a lintian check to verify that packages create and lock
  users properly.
* Once the lintian check is done, bugs on all packages that fail it
  should be filed.

Have I understood the discussion correctly? Any corrections or
objections to the above?

Unclear to me are the following two points:

* Should packages also remove the contents of the system account's
  home directory? Should this be done upon package remove or purge?
  If this is to be done, should we also provide a tool for it, to
  make sure everyone does it the right way? "clearuserhome foo"
  would essentially be "find ~foo -mindepth 1 -exec rm '{}' +",
  except it needs to delete directories as well, and should
  possibly have protection against crossing mount points,
  and perhaps verifying ownership of files before removing, etc.

* Is there consensus that adduser should get a --local option,
  and if so, what should its semantics be, and should packages
  start using it now? Or can this wait until there's an actual
  need for --local, so that the precise semantics can be defined?
  There's a fairly few packages that create users, so we should
  be able to deal with them fairly easily later.

Would "lockuser" need to be in the adduser package?  Given that
adduser is only priority:important, it's not guaranteed to be present
when postrm is run, so the operation could fail.  Maybe passwd is a
better place for it, given that it contains useradd etc., and is
priority:required.


Regards,
Roger

Maybe also a piuparts check.

I think this should be done is the content is exactly the same as when the
package was just installed. But this might be too hard to check.

Cheers,

Wouldn't lockuser be called on package removal, instead of purge?  In
which case a dependency on adduser would cover it?

Cheers,
Julien

I guess since you're not removing the user after any files they owned
are purged (which necessitates doing it in at purge) then doing it in
at remove time is just fine.  If the depends on adduser is still
valid at remove time (I'm certain it is), then I agree it lockuser
could be in the adduser package.


Regards,
Roger

Hi all,

the discussion on handling of system users on package removal became silent
again, let me try to summarize ...:

* To avoid reusing uids, system users created by packages should not be
removed by the package. There may be still files owned by that user (think
also about removable media, backups, ...) that a different user (with
recycled uid) should not get access to. Of course the sysadmin may manually
remove such obsolete users and groups.
* A 'lockuser' tool should be added to the adduser package to properly disable
an unused system account, to be called from postrm remove. adduser needs to
properly unlock a previously locked account.


... and revive the discussion with some new points:

Concerning removing the home directory ... I think there are three groups of
files:
1) the home directory itself
2) files created by running the daemon
   (or whatever the package was supposed to do)
3) files created by the sysadmin by doing something like
     su - $pkgsysuser
   in order to debug things, ...

I don't think maintainer scripts should care for group 3 files as they don't
result from proper operation of the package (and will cause group 1 to be
left over). These files could be covered by
  rm -rf $(getent passwd $pkgsysuser | cut -d: -f6)/
which could be harmful if the sysadmin modified $pkgsysuser.
  rm -rf /var/lib/$pkgsysuser/
could be problematic as well.
Group 2 (probably state files) should be taken care of by postrm purge.
For the homedirectory itself (usually /var/lib/$whatever) I would suggest to
ship this as an empty directory in the package, and let the postinst script
set proper ownership and permissions after creating the user.
That way dpkg should take care and remove it if it's empty. I don't think
leaving a (locked) system user with a nonexisting home is a problem.


There may be problems if the user/homedir already exists. These need to be
addressed by adduser or the maintainer script. Following is a list I could
think of, but I didn't check the behavior of adduser in these cases:

The user exists
* but is not a system user
* but remotely (nis, ldap, ...) and modification may not be possible
* but the group is different
* but the gecos is different
* but the homedir is different
* but the shell is different

The homedirectory exists
* but is a file
* but is owned by someone else
* but has weird permissions (including 777, 000)
* but is a symbolic link (+ combine with above cases)

The homedirectory cannot be created.

Potentially different handling of these discrepancies is needed during new
installations and upgrades.


I still think that using dpkg-maintscript-helper may be helpful for adding
system users. A system_user sub-command that would take arguments like
adduser (and might add some defaults if not specified, e.g. gecos). Some
possible extensions could be --home-owner, --home-group, --home-permissions.
A proper dependency on adduser should be added to the misc:Depends substvar
(at least if debhelper is used, e.g. via debian/package.maintscript)
In postinst configure this would produce in an adduser call (with some
chown/chmod following).
In postrm remove this would produce a lockuser call (or could even implement a
lockuser equivalent) and in postrm purge (and remove?) it should try a rmdir
on the homedir of the system user (unless --no-create-home is given). If this
fails (and the directory still exists), a diagnostic should be emitted (only
in purge because any state files etc. in the homedir are expected to be
removed by the postrm purge first).


Concerning piuparts, we should probably have an install-purge-install test for
packages that create system users and also an install-purge-mangle-install
test that mangles the passwd entries of added system users before the package
gets installed a second time.

Andreas

On 30/06/12 13:24, Stephan Springl wrote (on Bug #679642):
Policy bug #621833 (where it was originally suggested by Roger Leigh),
so this potentially affects quite a few packages.

Stephan's proposed patch (below) makes me think we really need a script
(or dpkg-maintscript-helper subcommand) that locks and unlocks system
users, in which we can make changes like this once and have them affect
every relevant package, rather than individually patching every
maintainer script.

Roger: does the change below look appropriate?

Regards,
    S

[in the preinst]
[in the postrm]

It looks sane to me.  Having a dh_ command or some other dpkg
maintscript helper shell function to do this automatically would
IMO be a very nice improvement.


Regards,
Roger

On Sat, 30 Jun 2012 14:36:47 +0100, Roger Leigh <rleigh@codelibre.net> wrote:

Given the amount of code lines that were spent in adduser to allow its
transparent usage in maintainer scripts, I would prefer having that
code in adduser. with adduser --lock locking an account and adduser
--system unlocking a locked user that is present but locked.

Having debhelper code for that is wrong since it means rebuilding
packages to fix bugs in that code.

Greetings
Marc

That's a bug in a lot of packages, yes. adduser has been designed to
allow adduser --system to be called without that logic:

       If  called  with one non-option argument and the --system option, adduser
       will add a system user. If a user with the same name  already  exists  in
       the  system  uid  range (or, if the uid is specified, if a user with that
       uid already exists), adduser will exit with a warning. This  warning  can
       be suppressed by adding "--quiet".

So, just remove the extra getent passwd check and you should be fine.

Greetings
Marc

That's a bug in a lot of packages, yes. adduser has been designed to
allow adduser --system to be called without that logic:

       If  called  with one non-option argument and the --system option, adduser
       will add a system user. If a user with the same name  already  exists  in
       the  system  uid  range (or, if the uid is specified, if a user with that
       uid already exists), adduser will exit with a warning. This  warning  can
       be suppressed by adding "--quiet".

So, just remove the extra getent passwd check and you should be fine.

Greetings
Marc

Yes, that would be the way to go for adduser --system

Yes, packages having used that approached are buggy in the first place.

NACK, adduser --system does return zero if the user already exists and
its parameters are sufficiently similiar to the parameters requested
by the maintainer script.

NACK, don't put the same logic into a hundred maintainer scripts where
they'll have two hundred different bugs. Put the logic into a central
place where bugs can be handled centrally.

Greetings
Marc

Yes, that would be the way to go for adduser --system

Yes, packages having used that approached are buggy in the first place.

NACK, adduser --system does return zero if the user already exists and
its parameters are sufficiently similiar to the parameters requested
by the maintainer script.

NACK, don't put the same logic into a hundred maintainer scripts where
they'll have two hundred different bugs. Put the logic into a central
place where bugs can be handled centrally.

Greetings
Marc

Don't do that, use deluser, if you insist. And even that is dangerous.

Greetings
Marc

It would be a bug to do so. Add --quiet to the adduser call if you
don't want to show the resulting warning to your users, but I'd
recommend to leave the warning active.

Why not extending deluser to not delete the user if it is a system
account?

No, the local admin might have put important additional data in there.
It may be an idea to remove all files that the _package_ has put
there, but that would be a _significant_ burden IMO.

Purge, of course. When you remove and reinstall, you should be exactly
where you were before.

Actually --system was meant for that.

Greetings
Marc, who has for quite some time taken care of adduser but has lost
touch to the package recently

adduser should be elevated to priority:required then. adduser contains
all Debian logic for account handling, while passwd doesn't. adduser
is the logical place for Debianisms.

Greetings
Marc

It would be a bug to do so. Add --quiet to the adduser call if you
don't want to show the resulting warning to your users, but I'd
recommend to leave the warning active.

Why not extending deluser to not delete the user if it is a system
account?

No, the local admin might have put important additional data in there.
It may be an idea to remove all files that the _package_ has put
there, but that would be a _significant_ burden IMO.

Purge, of course. When you remove and reinstall, you should be exactly
where you were before.

Actually --system was meant for that.

Greetings
Marc, who has for quite some time taken care of adduser but has lost
touch to the package recently

adduser should be elevated to priority:required then. adduser contains
all Debian logic for account handling, while passwd doesn't. adduser
is the logical place for Debianisms.

Greetings
Marc

Marc,
	I inherited it. I had the feeling things were going to be clarified so
I was waiting on that clarification.  Also if I recall I was trying to
raise the issue that half the issue was missed.

That is of course acceptable. Don't break things until Policy forces
you to do so ,-)

Greetings
Marc

Yeah and next time I'll make sure I really understand policy before
implementing it. ;-)

Actually  surely a good first step, perhaps even before fixing policy,
would be to get some sort of experimental check in lintian for any sort
of attempt to delete a system user in a maintenance script. Then we
could get a handle on how big the problem is.

How is "sufficiently similar" defined, and where is it documented?  It's not
in policy, and I don't see anything in the adduser manpage that explains
this.

Because that's contrary to the obvious meaning of 'deluser' and will be
confusing to maintainers, if it doesn't actually result in the user being
deleted.  It's much better to have an interface that does what it says.

This should be configurable by the package maintainer using a --remove-home
flag.  In the general case, admins should not use per-package directories
under /var/lib as a dumping ground for arbitrary files and then expect these
files to be retained when the package is purged.

Because that's contrary to the obvious meaning of 'deluser' and will be
confusing to maintainers, if it doesn't actually result in the user being
deleted.  It's much better to have an interface that does what it says.

This should be configurable by the package maintainer using a --remove-home
flag.  In the general case, admins should not use per-package directories
under /var/lib as a dumping ground for arbitrary files and then expect these
files to be retained when the package is purged.

*should* be in the adduser package, not in the passwd package; but that's
not a sound reason to raise the priority of adduser, and raising the
priority doesn't guarantee usability in the postrm anyway.

*should* be in the adduser package, not in the passwd package; but that's
not a sound reason to raise the priority of adduser, and raising the
priority doesn't guarantee usability in the postrm anyway.

Add a system user
       If called with one non-option  argument  and  the  --system  option,
       adduser will add a system user. If a user with the same name already
       exists in the system uid range (or, if the uid is  specified,  if  a
       user  with  that uid already exists), adduser will exit with a warn‐
       ing. This warning can be suppressed by adding "--quiet".

If that's not enough, a bug report against adduser is in order.

Greetings
Marc

That would mean changing probably thousands of packages.

If that behavior is documented (in Policy?), I am fine with zapping
user data.

Greetings
Marc

That would mean changing probably thousands of packages.

If that behavior is documented (in Policy?), I am fine with zapping
user data.

Greetings
Marc

Back in 2011-04-04 (see first mail in the bug report you're cc'ing to)
I did some greps, and found 103 packages that mention deluser in their
maintainer scripts. How did you come to the conclusion of "thousands
of packages"? If you're just guessing wildly, please stop: Debian
development is hard enough, let's try to stick to facts and when they're
not available, investigate rather than assume.

Back in 2011-04-04 (see first mail in the bug report you're cc'ing to)
I did some greps, and found 103 packages that mention deluser in their
maintainer scripts. How did you come to the conclusion of "thousands
of packages"? If you're just guessing wildly, please stop: Debian
development is hard enough, let's try to stick to facts and when they're
not available, investigate rather than assume.

I was extrapolating from my own packages to Debian proper, which was
probably skewed. I apologize.

Greetings
Marc

I was extrapolating from my own packages to Debian proper, which was
probably skewed. I apologize.

Greetings
Marc

Hey.

Wouldn't something like the following be a solution:

Apart from some "static" system users/groups which every system has,
let system users be in a certain reserved range, which is not the
normal 1-1000 range but neither a range where normal users can be
created.

When packages try to add their system user/group, they check whether it
already exists, if it does not or if it's on the reserved range - fine.

If it does but is in another range, one must presume that the local
admin accidentally used the same name.
Package installation should then fail. What we have right now, that
many packages simply check whether the group exists and create it only
if not, is IMO a security hole.


On purge, the user/group shall be removed, but again, only if it's from
the reserved range.


Existing packages should be mandated to migrate.



Maybe it would additionally make sense to come up with some pseudo-
reserved namespace for user/group names, too.
Like some packages already use debian-<foo> ... or _<foo>.

It's not perfect and doesn't guarantee that there are no collisions,
but better than nothing.



With respect to files owned by some system user/group and wich are left
over after package removal...
Well, normal removal shouldn't remove the user/group anyway.
And on purge, any files "owned" by the package should be properly
deleted anyway.


If there are nevertheless files which aren't deleted, like e.g. if
stuff in /srv/www would be owned by some daemon user/group, one could
do the following to provide at least some better protection:

When new system users/groups are created, their IDs are recorded and
after deleting them these IDs will only be reused, if otherwise the
reserved range would be empty (starting with the first one deleted).


Cheers,
Chris.

Oh and such creation/deletion of system users/groups should then
definitely done by some centrally managed code.

This would also allow to easily update things like home-dir, shell or
the GECOS field.

Right now most installations quickly run out of sync, e.g. many legacy
installations will have system users with /bin/false as shell, while
apparently at some point this was changed to /usr/sbin/nologin .


Cheers,
Chris.