Updated package uploaded for cauldron and Mageia 6.

Advisory:
========================

Updated libuser package fixes bugs.

When creating a new user with userdrake, if a directory matching the name
supplied for the new user already exists userdrake will segfault leaving the new user created but without a home directory or mail spool. This patch fixes that by appending an integer derived from the system time and then proceeding to create the new home directory in the usual manner and avoiding the segfault. This allows the new user to log in with a home directory while preserving the contents of the old directory in case data needs to be migrated.

========================

Updated packages in core/updates_testing:
========================
lib64user1-0.62-8.1.mga6
lib64user-devel-0.62-8.1.mga6
libuser-0.62-8.1.mga6
libuser-debuginfo-0.62-8.1.mga6
libuser-ldap-0.62-8.1.mga6
libuser-python-0.62-8.1.mga6
libuser-python3-0.62-8.1.mga6

from libuser-0.62-8.1.mga6.src.rpm

Note that userdrake outputs a gtk warning when ran from the console which already existed and is not the subject of this update. A description of how to reproduce the problem is in the description above or may also be caused by using mkdir to create a directory in /home such as /home/userabc and then using userdrake to add a new user userabc using /home/userabc as the home directory. The old pkg will segfault.

Source RPM: userdrake-2.16-1.mga6.src.rpm => userdrake-2.16-1.mga6.src.rpm libuser-0.62-8.mga6.src.rpm
CC: (none) => mrambo
Assignee: mageiatools => qa-bugs

MGA6-32 on Dell Latitude D600 MATE
First checked that I could replicate the problem before the update, which it did. I noticed also that the user was created as "Blocked" when the error occurred.
No installation issues
After the update, the operations were as can be expected normally.

CC: (none) => herman.viaene
Whiteboard: (none) => MGA6-32-OK

Testing MGA6 x64 real hardware
via MCC - System - Manage users
BTW What is the equivalent draksomething command? 'useradd' and 'userdel' seem to work correctly with respect to the error.

BEFORE update:
 lib64user1-0.62-8.mga6
 libuser-0.62-8.mga6
Created a new user with no existing home directory;
Returns to the list of users.

Deleted that user leaving the home directory; re-created the same user creating home directory:
Returns directly to the MCC 'System' screen. [Crash? The error]
Trying the same thing from a terminal is OK:
 # useradd -m -p rubbish rubbish
 useradd: warning: the home directory already exists.
 Not copying any file from skel directory into it.

AFTER the update:
- lib64user1-0.62-8.1.mga6.x86_64
- libuser-0.62-8.1.mga6.x86_64

Starting from scratch:
Add new user with non-existant home directory
Returns to the user list.
The new user can login, at correct home directory.
 # ls -l /home/
 drwxr-x--- 49 lewis lewis 4096 Ion   4 14:01 lewis/
 drwxr-x---  5 rubbish users 4096 Ion   4 14:36 rubbish/

Remove the user, leaving the home directory which remains as above.
Returns to the list of users.
Re-add the same user asking for a home directory.
Returns to the user list - the error corrected.
But not without a new fault:
 # ls -l /home/
 drwxr-x--- 49 lewis   lewis 4096 Ion   4 14:01 lewis/
 drwxr-x---  4 rubbish users 4096 Gor  12 08:47 rubbish/
 drwxr-x---  5 rubbish users 4096 Ion   4 14:36 rubbish.1515073238/

 # tree /home/rubbish.1515073238/
 /home/rubbish.1515073238/
 └── tmp

 # tail -3 /etc/passwd
 dhcpd:x:967:958:system user for dhcp:/dev/null:/bin/false
 powerdns:x:966:957:system user for pdns-recursor:/var/lib/powerdns:/bin/false
 rubbish:x:1000:100:rubbish:/home/rubbish:/bin/bash
shows the correct original home directory.
And logging into the user also gives the correct home directory.

Clearly the superflous home directory should *not* be there. Asking for feedback, and witholding the advisory in case a version update results.

Keywords: (none) => feedback
CC: (none) => lewyssmith

Hello Lewis, the bug is about userdrake, not useradd or userdel. You can run it from cli by just typing userdrake and answering the password prompt.

The superfluous home directory is intentional and explained in the advisory.

"When creating a new user with userdrake, if a directory matching the name
supplied for the new user already exists, userdrake will segfault leaving the new user created but *without* a home directory or mail spool. This patch fixes that by appending an integer derived from the system time (to the original directory entry) and then proceeding to create the new home directory in the usual manner thus avoiding the segfault. This allows the new user to log in *with* a home directory while preserving the contents of the old directory in case data needs to be migrated." (modified slightly in an attempt to improve clarity)

The goal is to: 1) not segfault userdrake 2) not create a user with no home directory when using userdrake 3) not remove, delete, or alter an old or existing directory of the same name when data may need to be migrated from that old directory - whether or not that old directory belonged to the same person.

Keywords: feedback => (none)

M6/x64 continued

Thank you Mike for your explanation; question of "Read the ... manual"!
And knowing about 'userdrake' was a help rather than wading through MCC.

Accepting what you say, there remains an ambiguity - just a question of *wording* - before uploading the Advisory. You imply ? intend ? that the new user will have the 'duplicate' home directory when he logs in. In fact, he has the *old* one (all examples after re-creating the user with an existing home directory):
 # ls -l /home
 drwxr-x---  4 rubbish users 4096 Gor  12 08:47 rubbish/
 drwxr-x---  4 rubbish users 4096 Gor  12 08:47 rubbish.1515142624/

 # tail -1 /etc/passwd
 rubbish:x:1000:100:rubbish:/home/rubbish:/bin/bash
verified by logging in as that user:
 login: rubbish
 [password]
 $ pwd
 /home/rubbish
Of course the new duplicate is available to him, equally. But he would have to fiddle things to make it his home directory. As things stand, the duplicate home directory seems to serve nothing beyond saving userdrake from crashing.

So I am OKing this, but saving validation & Advisory upload for this point to be clarified.

Whiteboard: MGA6-32-OK => MGA6-32-OK MGA6-64-OK

Hi Lewis, I'm open to suggestions on the wording - I do want it clear. My thinking was not that this would be a duplicate at all. Given a user of fred userdrake has no idea if /home/fred is fred.smith or fred.walser or some other fred that used to exist on a *multiuser* system. I just want the fred being added to have *his* own home directory at first logon while preserving whatever fred was there in case it needs to be migrated (no lost data).

I don't know the exact use case the OP had in discovering the segfault but I had something happen once so let me explain it this way. On my workstation I once had the circumstance where I had created a /home/install for some odd project which had some shell scripts. I later tried to add an install user for use with g4l and ran across this. I needed the new 'install' user to have a home directory as I was saving images there but the data in the existing /home/install did not have any relevance to that user. I also didn't want to lose the scripts (and some wouldn't want to have unauthorized access). This is probably something that seldom happens, but at the same time is is not impossible - plus the segfault that the OP found is real.

"Of course the new duplicate is available to him..."

I will add that the existing '/home/rubbish' would be available to the new added user only under limited circumstances. It would depend upon how the original /home/rubbish was created as to whether ownership uid/gid would line up to allow access (see my example above). libuser and userdrake in this case would not change any ownership so that may or may not be true. Only when you have just added, deleted, and readded the user in a testing environment is there great likelihood that things will line up to allow access. Many other circumstances would require admin access to retrieve the data in the 'duplicate'.

(In reply to Mike Rambo from comment #4)

> The superfluous home directory is intentional and explained in the advisory.
> 
> "When creating a new user with userdrake, if a directory matching the name
> supplied for the new user already exists, userdrake will segfault leaving
> the new user created but *without* a home directory or mail spool. This
> patch fixes that by appending an integer derived from the system time (to
> the original directory entry) and then proceeding to create the new home
> directory in the usual manner thus avoiding the segfault. This allows the
> new user to log in *with* a home directory while preserving the contents 

Would it be more logical to change the "integer derived from the system time" to something more "human logic" like

rename "user" to "user.renamed_duplicate_date_time" or something so people would see why/when it changed..

So something like 

/home/rubbish

would become

/home/rubbish.renamed_duplicate_20180105_1456"

of course the proper fix would probably be to check for /home/rubbish and give feedback to user that "/home/rubbish already exists, user not created"
or something like that (or ask "do you want to rename the existing directory" and go from there) and maybe even forcing privilegies to root:root on the moved dir to always ensure no unauthorized access to the renamed stuff

CC: (none) => tmb

(In reply to Thomas Backlund from comment #7)
> (In reply to Mike Rambo from comment #4)
> 
> > the new user created but *without* a home directory or mail spool. This
> > patch fixes that by appending an integer derived from the system time (to
> 
> Would it be more logical to change the "integer derived from the system
> time" to something more "human logic" like
> 
> rename "user" to "user.renamed_duplicate_date_time" or something so people
> would see why/when it changed..
> 

I'm open to whatever folks would prefer. I knew that whatever it was changed to should have a reasonable expectation of being unique but the details don't matter to me. I don't actually code in C but I can ask my son to whip something up if that is the preference. He wrote the existing patch anyway.

I don't know if the 'proper fix' is within his capabilities or not. He is getting pretty good but he is still only 16 and doesn't know the interaction between all the pieces very well. We were shooting for a minimally invasive patch that fixed the core problem and didn't cause others.

I'll check with him.

Yeah, I think we can go with the minimal fix for now for mga6 and then to a more extensive fix in cauldron for mga7

Thanks Mike & Thomas for your remarks.

The nub of my doubts seems to have been inverting the roles of the re-introduced virgin home directory and the retained original one. I had wrongly assumed that the the 'duplicate' was the *new* one, and that the correctly named home directory was the original one. Getting hold of the wrong end of the stick.

I see now that when a user is added with an existing home directory, it is *that* one that is *renamed* to the 'duplicate'; and the normally named home directory  which is the *new* one. So I propose the following Advisory wording (the vital sentance only):
...
"This patch fixes that by preserving the existing directory by re-naming it (appending an integer), and then proceeding to create the new home directory in the usual manner and avoiding the segfault."
...
Sorry for all the noise. A question of words indeed.
I am validating this, and if you agree the sentance above, will do the Advisory.

Keywords: (none) => validated_update
CC: (none) => sysadmin-bugs

Looks great to me. Thanks Lewis.

Thank you Mike. Advisory uploaded from Comment 1, tweaked slightly as per C10.

> He [your son] wrote the existing patch anyway
And deserves our felicitations!

Keywords: (none) => advisory

An update for this issue has been pushed to the Mageia Updates repository.

https://advisories.mageia.org/MGAA-2018-0009.html

Status: NEW => RESOLVED
Resolution: (none) => FIXED

I want to note that new userdrake and libuser packages have been uploaded for cauldron incorporating the suggestions Thomas made in Comment 7.

Specifically, userdrake will now ask whether to rename any already existing directory. If the user chooses yes the directory will be renamed by libuser as Thomas suggested, which is indeed much more intuitive than a random integer. The renamed directory entry is chown'd to root.root to leave the security implications to an admin. If they answer no they are left at the new user screen to make their own changes as they see fit.

Once we looked, it turned out that making these changes was not very difficult. My son created the patches, but not until after this update had been pushed for Mageia 6. I'm willing to push these to 6 also if you all believe the additional churn is warranted. The libuser update is just a subrel increase but the userdrake change will be a version change if we want it.

Mike
If you want to release this improvement, I guess it needs a sub-version bump and a new bugfix bug. No problem for QA because (without my ill-conceived diversions) it is quick & easy to test.

commit eea83398deb651edac83b481393f441484c21b53
Author: Nicolas Lécureuil <neoclust@...>
Date:   Thu Jan 11 18:19:26 2018 +0100

    add patch to notify of existing home directory (mga#21333)
    From Mike Rambo
---
 Commit Link:
   http://gitweb.mageia.org/software/userdrake/commit/?id=eea83398deb651edac83b481393f441484c21b53

commit f2d2ab69591a47380fbdcc3c226d9386d1a95764
Author: Mike Rambo <mrambo3501@...>
Date:   Wed Mar 7 13:09:04 2018 -0500

    - version 2.17 Notify user when home directory name already exists (mga#21333)
---
 Commit Link:
   http://gitweb.mageia.org/software/userdrake/commit/?id=f2d2ab69591a47380fbdcc3c226d9386d1a95764

Is this a good fix? Doesn't it create the possibility to eg. create a new user with homedir as /home and effectively lock other users out?

Wouldn't it be better to modify the name of the new user home directory rather than alter the existing one?

Just going by comment 14 btw

No, this is about /home/xxxxxxxx where the xxxxxxxx already exists. On a single user system you might assume it was the same user but that isn't true in other environments. The update handles the conflict by making sure the new user can log in with a fresh home directory in the normal location and leaves the admin to sort out what should happen with the existing xxxxxxxx which would be important in a multi-user or corporate environment.

So altering directory names is actually restricted to existing directories under /home?

Yes.

Isn't userdrake used in the installer too? If so, it sounds like this change would prevent people from installing whilst retaining their current /home.

Sorry to give you a grilling over this Mike. Final thought for now.

Has this change already been made/commented/tested in Cauldron?
Stable updates_testing is not the correct way to develop our tools.

The installer may use separate logic because the segfault in the description AFAIK only happens on an installed system. When the segfault happens you end up with no home directory created at all. Now, if the stars are lined up and your new user happens to have the same uid/gid as before you will still have access to the existing directory but there is no guarantee of this as the segfault prevents userdrake from getting to the code section which sets ownership.

But in answer to your specific question about the installer, I don't know. This update is intended to address the reported segfault and leave the new user able to log in with an intact new home directory and any existing directory, whether his/hers or someone else's, intact and available for an admin to fix up as needed.

Yes, the change has been in cauldron since Dec 31st 2017. I had thought about leaving it at that but after looking closer we found that Thomas' suggestions for a proper fix in Comment 7 were not that hard to do. Thus 22373 exists for the mga6 update.

I don't have any problem with being 'grilled'. I know I'm new and have made mistakes along with way. As my goal is to be an asset rather than a liability it isn't a problem if folks keep an eye on what I do. I likely need it.

Thanks for your responses. 

Speaking of mistakes, mine, let's move discussion over to the new bug 22373. :D