Bug 724986

Summary: system-config-user ignores S_ISUID and S_ISGID bit from /etc/skel
Product: [Fedora] Fedora Reporter: ritz <rkhadgar>
Component: libuserAssignee: Miloslav Trmač <mitr>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mitr
Target Milestone: ---Keywords: Patch
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 724987 (view as bug list) Environment:
Last Closed: 2011-10-04 11:30:17 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Attachments:
Description Flags
proposed patch rkhadgar: review?

Description ritz 2011-07-22 14:04:12 UTC
Created attachment 514709 [details]
proposed patch

Description of problem:
system-config-user ignores S_ISUID and S_ISGID  bit from /etc/skel

Version-Release number of selected component (if applicable):
libuser-0.57.2-1.fc15.x86_64

How reproducible:
always

Steps to Reproduce:
1. mkdir /etc/skel/xyz
2. chmod +s /etc/skel/xyz
3. system-config-user ( create user abc )
  
Actual results:
stat /etc/abc/xyz does not show gid bit

Expected results:
stat /etc/abc/xyz should show gid bit


Additional info:

from libuser/apps/lnewusers.c

297                         /* Unless the nocreatehomedirs flag was given, attempt
298                          * to create the user's home directory. */
299                         if (!nocreatehome) {
300                                 if (lu_homedir_populate(ctx, NULL, homedir,
301                                                         uid, gid, 0700, 
...



from libuser/apps/apputil.c

320 /* Populate a user's home directory, copying data from a named skeleton
321    directory (or default if skeleton is NULL), setting all ownerships as
322    given, and setting the mode of the top-level directory as given. */
323 gboolean
324 lu_homedir_populate(struct lu_context *ctx, const char *skeleton,
325                     const char *directory, uid_t owner, gid_t group,
326                     mode_t mode, struct lu_error **error)
327 {
328         lu_security_context_t fscreate;
...
339         ret = lu_homedir_copy(skeleton, directory, owner, group, mode, 0,
340                               error);


 87 /* Copy the "src" directory to "dest", setting all ownerships as given, and
 88    setting the mode of the top-level directory as given.  The group ID of the
 89    copied files is preserved if it is nonzero.  If keep_contexts, preserve
 90    SELinux contexts in files under dest; use matchpathcon otherwise.
 91 
 92    Note that keep_contexts does NOT affect the context of dest; the caller must
 93    perform an explicit setfscreatecon() before calling lu_homedir_copy() to set
 94    the context of dest.  The SELinux fscreate context is on return from this
 95    function is unspecified. */
 96 static gboolean
 97 lu_homedir_copy(const char *src, const char *dest, uid_t owner, gid_t group,
 98                 mode_t mode, gboolean keep_contexts, struct lu_error **error)
 99 {
...
122         /* Create the top-level directory. */
123         if (mkdir(dest, mode) == -1 && errno != EEXIST) {
124                 lu_error_new(error, lu_error_generic,
125                              _("Error creating `%s': %s"), dest,
126                              strerror(errno));
127                 goto err_dir;
128         }
<--- add chmod here
...
173                 /* If it's a directory, descend into it. */
174                 if (S_ISDIR(st.st_mode)) {
175                         if (!lu_homedir_copy(srcpath, path, owner,
176                                              st.st_gid ?: group, st.st_mode,
177                                              keep_contexts, error))
 

from man 3p mkdir

APPLICATION USAGE
       In order to ensure that the S_ISUID and S_ISGID bits are set, an application requiring this should use stat() after a successful chmod() to verify this.

Comment 1 Miloslav Trmač 2011-10-04 11:30:17 UTC
Thanks for your report.  Fixed in rawhide and F16 libuser-0.57.2-1 based on your patch, but with some changes (e.g. to keep handling of umask unchanged).