Bug 9262 - useradd class doesn't clean properly
Summary: useradd class doesn't clean properly
Status: RESOLVED FIXED
Alias: None
Product: OE-Core
Classification: Build System, Metadata & Runtime
Component: core (show other bugs)
Version: unspecified
Hardware: All Multiple
: Medium+ normal
Target Milestone: 2.1 M4
Assignee: Maxin B. John
QA Contact: Valentin Hangan
URL:
Whiteboard:
Depends on:
Blocks:
 
Reported: 2016-03-15 13:43 UTC by Gary Thomas
Modified: 2016-05-04 11:50 UTC (History)
7 users (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: Don't know


Attachments
userdel_sysroot_sstate patch (2.12 KB, patch)
2016-03-21 15:18 UTC, Maxin B. John
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gary Thomas 2016-03-15 13:43:36 UTC
If a recipe is built that creates user(s) via useradd.bbclass, those users are not removed (at least from sysroot) when the recipe/package is cleaned, even using cleanall.

This can lead to confusion, in particular if the recipe is changed in a way that affects how the user(s) are created because they won't be recreated when the recipe is rebuilt.  For example if the recipe creates a user with no explicit --uid/--gid parameters, those parameters will be ignored if they are later added to the recipe and rebuilt.

Note: this may also be related to bug #7724
Comment 1 Jussi Kukkonen 2016-03-16 08:19:30 UTC
Maxin was just looking at this or something very much like it -- CCing just in case.
Comment 2 Maxin B. John 2016-03-16 11:12:40 UTC
Fedora User account handling policy says:
( https://fedoraproject.org/wiki/Packaging:UsersAndGroups )
"
Do not remove users or groups

We never remove users or groups created by packages. There's no sane way to check if files owned by those users/groups are left behind (and even if there would, what would we do with them?) and leaving those behind with ownerships pointing to now nonexistent users/groups may result in security issues when a semantically unrelated user/group is created later and reuses the UID/GID. Also, in some setups deleting the user/group might not be possible or/nor desirable (eg. when using a shared, remote user/group database). Cleanup of unused users/groups is left to the system administrators to take care of if they so desire.
"

Debian also has similar policy:
( https://wiki.debian.org/AccountHandlingInMaintainerScripts )

"
Technical Issues of account removal 

Deleting an account is significantly harder than creating it because of the following reasons:

at package purge time, a package can no longer rely on its dependencies being installed
the local admin might have decided to chown unrelated files to the package account
if we remove the account on purge, it is possible that the UID will be re-used at some later time
"

Also Yocto #8078 describes a use-case where "Package A" depends on user created by "Package B". If we remove the user along with "Package B", then, we might end up with a broken "Package A" in the device.
Comment 3 Gary Thomas 2016-03-16 11:27:30 UTC
(In reply to comment #2)
> Fedora User account handling policy says:
> ( https://fedoraproject.org/wiki/Packaging:UsersAndGroups )
> "
> Do not remove users or groups
> 
> We never remove users or groups created by packages. There's no sane way to
> check if files owned by those users/groups are left behind (and even if
> there would, what would we do with them?) and leaving those behind with
> ownerships pointing to now nonexistent users/groups may result in security
> issues when a semantically unrelated user/group is created later and reuses
> the UID/GID. Also, in some setups deleting the user/group might not be
> possible or/nor desirable (eg. when using a shared, remote user/group
> database). Cleanup of unused users/groups is left to the system
> administrators to take care of if they so desire.
> "
> 
> Debian also has similar policy:
> ( https://wiki.debian.org/AccountHandlingInMaintainerScripts )
> 
> "
> Technical Issues of account removal 
> 
> Deleting an account is significantly harder than creating it because of the
> following reasons:
> 
> at package purge time, a package can no longer rely on its dependencies
> being installed
> the local admin might have decided to chown unrelated files to the package
> account
> if we remove the account on purge, it is possible that the UID will be
> re-used at some later time
> "
> 
> Also Yocto #8078 describes a use-case where "Package A" depends on user
> created by "Package B". If we remove the user along with "Package B", then,
> we might end up with a broken "Package A" in the device.

So how does one handle the scenario as described?  It seems a bit harsh to have to wipe "tmp" and start over, if that even works...
Comment 4 Gary Thomas 2016-03-16 12:20:14 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Fedora User account handling policy says:
> > ( https://fedoraproject.org/wiki/Packaging:UsersAndGroups )
> > "
> > Do not remove users or groups
> > 
> > We never remove users or groups created by packages. There's no sane way to
> > check if files owned by those users/groups are left behind (and even if
> > there would, what would we do with them?) and leaving those behind with
> > ownerships pointing to now nonexistent users/groups may result in security
> > issues when a semantically unrelated user/group is created later and reuses
> > the UID/GID. Also, in some setups deleting the user/group might not be
> > possible or/nor desirable (eg. when using a shared, remote user/group
> > database). Cleanup of unused users/groups is left to the system
> > administrators to take care of if they so desire.
> > "
> > 
> > Debian also has similar policy:
> > ( https://wiki.debian.org/AccountHandlingInMaintainerScripts )
> > 
> > "
> > Technical Issues of account removal 
> > 
> > Deleting an account is significantly harder than creating it because of the
> > following reasons:
> > 
> > at package purge time, a package can no longer rely on its dependencies
> > being installed
> > the local admin might have decided to chown unrelated files to the package
> > account
> > if we remove the account on purge, it is possible that the UID will be
> > re-used at some later time
> > "
> > 
> > Also Yocto #8078 describes a use-case where "Package A" depends on user
> > created by "Package B". If we remove the user along with "Package B", then,
> > we might end up with a broken "Package A" in the device.
> 
> So how does one handle the scenario as described?  It seems a bit harsh to
> have to wipe "tmp" and start over, if that even works...

To follow up on this, OE is not Debian nor Fedora.  What I mean is that in those environments, single packages are built and the build system knows nothing of package interdependencies.  OE on the other hand knows all about such things and it should be no big problem that if a package causes the removal of a user/group ID then any dependent packages would be also marked appropriately so they will be rebuilt/reinstalled.  I don't know how "hard" this is to make happen, but the fact that it can be done and it makes the build system more "correct", then that's how it should behave.
Comment 5 Maxin B. John 2016-03-17 12:29:22 UTC
(In reply to comment #3)

> 
> So how does one handle the scenario as described?  It seems a bit harsh to
> have to wipe "tmp" and start over, if that even works...

We don't need to wipe "tmp" directory for that. 
# bitbake -c cleansstate base-passwd

should do it. However,that method also has some problems at the moment(Please see bug #8759)
Comment 6 Maxin B. John 2016-03-21 15:18:24 UTC
Created attachment 3049 [details]
userdel_sysroot_sstate patch

This patch provides a function which cleans-up users/groups during cleansstate/cleanall task.
Comment 7 Maxin B. John 2016-03-21 15:21:24 UTC
Please let me know if this is an acceptable fix.
Comment 8 Gary Thomas 2016-03-22 07:33:27 UTC
(In reply to comment #7)
> Please let me know if this is an acceptable fix.

Yes, this does seem to solve the issue I was having.  I created my package then reran bitbake after altering the useradd settings in the recipe and the changes worked as desired unlike before where nothing happened the second time around.
Comment 9 Maxin B. John (inactive) 2016-03-22 08:57:24 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > Please let me know if this is an acceptable fix.
> 
> Yes, this does seem to solve the issue I was having.  I created my package
> then reran bitbake after altering the useradd settings in the recipe and the
> changes worked as desired unlike before where nothing happened the second
> time around.

Thanks a lot!

Fix sent to oe-core mailing list:
http://lists.openembedded.org/pipermail/openembedded-core/2016-March/119264.html
Comment 10 Richard Purdie 2016-03-31 14:06:07 UTC
We really need to have this removal code run at any of the points sstate_clean() is called. This means the user would be removed for do_clean, when a version is upgraded and the sysroot is cleaned out during do_configure and so on. I talked with Maxin about this and have some pointers as to how we might be able to add some hooks to sstate_clean to make this work. If we don't do this, I worry we'll only fix the issue for some cases and not all.
Comment 11 Maxin B. John 2016-04-05 07:36:38 UTC
v2 sent to oe-core ML:
http://lists.openembedded.org/pipermail/openembedded-core/2016-April/119879.html