userdel: --force: Allow the flag to be specified twice, for more granularity

[alejandro-colomar]

Previously, with a single --force, one could only chose between not forcing, and forcing (which could destroy one's system entirely).

Now one can pass --force, with only a small danger, to skip some checks, and a double --force --force with the meaning of the previous --force, which will possibly destroy the entire system.

Closes: #1050
Reported-by: @wolfsage
Cc: @poettering
Cc: @hallyn

---

I have NOT tested this. Please test.

---

Revisions:

v2

• Reword documentation

$ git range-diff shadow/master gh/ff ff 
1:  5d8ddfa5 ! 1:  11c0e8b5 userdel: --force: Allow the flag to be specified twice, for more granularity
    @@ man/userdel.8.xml
     +    </para>
     +    <para>
     +      If specified once,
    -+      it doesn't check if the user is still logged in.
    ++      users are removed
    ++      even if they're still logged in,
    ++      and groups are removed
    ++      even if they are the primary group of a user.
     +      If specified twice,
     +      it skips all safety checks.
          </para>

v2b

• Reword paragraph.

$ git range-diff shadow/master gh/ff ff 
1:  11c0e8b5 ! 1:  cb580580 userdel: --force: Allow the flag to be specified twice, for more granularity
    @@ man/userdel.8.xml
     +    </para>
     +    <para>
     +      If specified once,
    -+      users are removed
    -+      even if they're still logged in,
    -+      and groups are removed
    -+      even if they are the primary group of a user.
    ++      a user is removed
    ++      even if it's still logged in,
    ++      and its primary group is removed
    ++      even if it's the primary group of another user.
     +      If specified twice,
     +      it skips all safety checks.
          </para>

v2c

• Rebase

$ git range-diff gh/master..gh/ff master..ff 
1:  cb580580 = 1:  72af7d32 userdel: --force: Allow the flag to be specified twice, for more granularity

v2d

• Rebase

$ git range-diff master..gh/ff shadow/master..ff 
1:  72af7d32 = 1:  372fdbe2 userdel: --force: Allow the flag to be specified twice, for more granularity

[alejandro-colomar]

Does this distinction make sense? Should the group thing go with -ff?

[alejandro-colomar]

I think it's ok, but I ask just in case.

[alejandro-colomar]

@wolfsage, can you please check if this looks good to you? Also, would you please test it?

[ikerexxe]

I don't like this change and I think it'll be problematic to some users.

[alejandro-colomar]

I don't like this change and I think it'll be problematic to some users.

Yep, we have problems either way. This is half broken, so users will complain about it being quite unsafe. Yet, scripts may rely on that brokenness, so fixing it may break scripts.

I'm in favour of fixing it and potentially breaking some random scripts. Any other opinions?

[ikerexxe]

From my perspective, the force' option implies deleting the user regardless of the consequences. This option should be used with care, because as you mention, it can have fatal consequences for the system. The double use of the force' option is problematic and will cause more problems than it intends to solve. I would try to keep it as simple as possible and avoid such a complication.

[alejandro-colomar]

From my perspective, the force' option implies deleting the user regardless of the consequences. This option should be used with care, because as you mention, it can have fatal consequences for the system. The double use of the force' option is problematic and will cause more problems than it intends to solve. I would try to keep it as simple as possible and avoid such a complication.

There's prior art in other software for this schema of asking for double -ff for more dangerous stuff.

This is for example from git-clean(1):

     -f, --force
         If the Git configuration variable clean.requireForce is not
         set to false, git clean will refuse to delete files or
         directories unless given -f. Git will refuse to modify
         untracked nested git repositories (directories with a .git
         subdirectory) unless a second -f is given.

The only thing that might be a problem is existing scripts that use this (or interactive users that might not read the new docs if they remember well the old docs; but if I were them, I'd probably re-read the docs just in case). But for interactive use, or for new scripts, I don't think requiring the option twice would be problematic.

[hallyn]

Alex, thank you for continuing to try to make this better.

Please don't make any changes yet based on what I say here, I'm just thinking aloud :)

First, there's a difference between "home is not owned by the user" and "home is / and we're all gonna die!". Shadow is not that "generic" of a package. I think it would be ok to add a list of "do not touch" paths, and always refuse to delete those no matter what: /, /usr, /etc, and /var at least. I suppose we could even make the list configurable through /etc/login.defs, so distros can add their own entries.

Then, for the "home not owned by user" case, we just need to draw line between not breaking any current users and complicating code. The main options are: 1. change nothing (which if we implement the never-delete list above becomes far less scary), 2. add a default-off DELETE_UNOWNED_HOME option to login.defs, 3. add -ff like you have, and 4. both 2 and 3.

[alejandro-colomar]

Shadow is not that "generic" of a package. I think it would be ok to add a list of "do not touch" paths, and always refuse to delete those no matter what: /, /usr, /etc, and /var at least.

How about refusing to add users with such homedirs in the first place?

(Just thinking aloud too.)

[alejandro-colomar]

Then, for the "home not owned by user" case, we just need to draw line between not breaking any current users and complicating code. The main options are: [...] 2. add a default-off DELETE_UNOWNED_HOME option to login.defs. [...]

I tend to dislike configurability of programs. It adds complexity to both sysadmins, and the program itself. I think I would prefer a hardcoded list of directories that should not be removed.

[hallyn]

Shadow is not that "generic" of a package. I think it would be ok to add a list of "do not touch" paths, and always refuse to delete those no matter what: /, /usr, /etc, and /var at least.

How about refusing to add users with such homedirs in the first place?

(Just thinking aloud too.)

The problem with that is that a mistaken or malicious /etc/passwd edit could easily change it afterwards.

[alejandro-colomar]

Shadow is not that "generic" of a package. I think it would be ok to add a list of "do not touch" paths, and always refuse to delete those no matter what: /, /usr, /etc, and /var at least.

How about refusing to add users with such homedirs in the first place?
(Just thinking aloud too.)

The problem with that is that a mistaken or malicious /etc/passwd edit could easily change it afterwards.

I think -ff is good in the long term (even if we may want to do other changes too); even if we want to forbid removing /, I'd still hide removing other non-owned dirs under -ff.

What's your opinion about breaking existing scripts (or hardcoded brains)? I don't know how much people would rely on -f for removing non-owned dirs, and other stuff we're hiding under -ff.

[hallyn]

What's your opinion about breaking existing scripts (or hardcoded brains)? I don't know how much people would rely on -f for removing non-owned dirs, and other stuff we're hiding under -ff.

Well actually, I would imagine there are a lot of places that have end-of-semester or whatever scripts that nuke the out-going students' accounts. For those, having -f return error and -ff succeed would be a problem - if they ever have a situation like this. It might be better if -f just skipped the deletion and returned success. But that has its own problems.

[alejandro-colomar]

What's your opinion about breaking existing scripts (or hardcoded brains)? I don't know how much people would rely on -f for removing non-owned dirs, and other stuff we're hiding under -ff.

Well actually, I would imagine there are a lot of places that have end-of-semester or whatever scripts that nuke the out-going students' accounts. For those, having -f return error and -ff succeed would be a problem - if they ever have a situation like this. It might be better if -f just skipped the deletion and returned success. But that has its own problems.

But I expect such user accounts to either have an owned dir, or have something like /nonexistent that shouldn't be removed. In what case would one have a directory that isn't owned but they want to remove it anyway?