From 0454708f0317c8de64ee0d1e84280af31e386d87 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 5 Dec 2012 15:04:01 +0100 Subject: [PATCH 1/2] s3:smbd: don't apply create/directory mask and modes in apply_default_perms() The mask/mode parameters should only apply to a situation with only pure posix permissions. Once we are dealing with ACLs and inheritance, we need to do it correctly. This fixes bug #9462: Users can not be given write permissions any more by default Signed-off-by: Michael Adam --- source3/smbd/posix_acls.c | 88 ++++++--------------------------------------- 1 file changed, 11 insertions(+), 77 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index ef0a084..a6a8c3d 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1236,48 +1236,19 @@ NTSTATUS unpack_nt_owners(struct connection_struct *conn, return NT_STATUS_OK; } -/**************************************************************************** - Ensure the enforced permissions for this share apply. -****************************************************************************/ -static void apply_default_perms(const struct share_params *params, - const bool is_directory, canon_ace *pace, - mode_t type) +static void trim_ace_perms(canon_ace *pace) { - mode_t and_bits = (mode_t)0; - mode_t or_bits = (mode_t)0; - - /* Get the initial bits to apply. */ + pace->perms = pace->perms & (S_IXUSR|S_IWUSR|S_IRUSR); +} +static void ensure_minimal_owner_ace_perms(const bool is_directory, + canon_ace *pace) +{ + pace->perms |= S_IRUSR; if (is_directory) { - and_bits = lp_dir_mask(params->service); - or_bits = lp_force_dir_mode(params->service); - } else { - and_bits = lp_create_mask(params->service); - or_bits = lp_force_create_mode(params->service); - } - - /* Now bounce them into the S_USR space. */ - switch(type) { - case S_IRUSR: - /* Ensure owner has read access. */ - pace->perms |= S_IRUSR; - if (is_directory) - pace->perms |= (S_IWUSR|S_IXUSR); - and_bits = unix_perms_to_acl_perms(and_bits, S_IRUSR, S_IWUSR, S_IXUSR); - or_bits = unix_perms_to_acl_perms(or_bits, S_IRUSR, S_IWUSR, S_IXUSR); - break; - case S_IRGRP: - and_bits = unix_perms_to_acl_perms(and_bits, S_IRGRP, S_IWGRP, S_IXGRP); - or_bits = unix_perms_to_acl_perms(or_bits, S_IRGRP, S_IWGRP, S_IXGRP); - break; - case S_IROTH: - and_bits = unix_perms_to_acl_perms(and_bits, S_IROTH, S_IWOTH, S_IXOTH); - or_bits = unix_perms_to_acl_perms(or_bits, S_IROTH, S_IWOTH, S_IXOTH); - break; + pace->perms |= (S_IWUSR|S_IXUSR); } - - pace->perms = ((pace->perms & and_bits)|or_bits); } /**************************************************************************** @@ -1430,45 +1401,14 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, bool got_duplicate_group = false; for (pace = *pp_ace; pace; pace = pace->next) { + trim_ace_perms(pace); if (pace->type == SMB_ACL_USER_OBJ) { - /* - * Ensure we have default parameters for the - * user (owner) even on default ACLs. - */ - apply_default_perms(params, is_directory, pace, S_IRUSR); + ensure_minimal_owner_ace_perms(is_directory, pace); pace_user = pace; - } else if (pace->type == SMB_ACL_GROUP_OBJ) { - - /* - * Ensure create mask/force create mode is respected on set. - */ - - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRGRP); - } pace_group = pace; - } else if (pace->type == SMB_ACL_OTHER) { - - /* - * Ensure create mask/force create mode is respected on set. - */ - - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IROTH); - } pace_other = pace; - - } else if (pace->type == SMB_ACL_USER || pace->type == SMB_ACL_GROUP) { - - /* - * Ensure create mask/force create mode is respected on set. - */ - - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRGRP); - } } } @@ -1520,7 +1460,7 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, * Ensure we have default parameters for the * user (owner) even on default ACLs. */ - apply_default_perms(params, is_directory, pace, S_IRUSR); + ensure_minimal_owner_ace_perms(is_directory, pace); DLIST_ADD(*pp_ace, pace); pace_user = pace; @@ -1546,9 +1486,6 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, } else { pace->perms = 0; } - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRGRP); - } DLIST_ADD(*pp_ace, pace); pace_group = pace; @@ -1568,9 +1505,6 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, pace->trustee = global_sid_World; pace->attr = ALLOW_ACE; pace->perms = 0; - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IROTH); - } DLIST_ADD(*pp_ace, pace); pace_other = pace; -- 1.7.9.5 From 3b34376fe4efa8fbc1cd92724209ec90eeddf631 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 4 Dec 2012 15:47:06 -0800 Subject: [PATCH 2/2] Documentation fixes for bug #9462 - Users can not be given write permissions any more by default Ensure we don't apply the masks + force modes on security setting changes, only on create. Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam --- docs-xml/smbdotconf/security/createmask.xml | 5 ----- docs-xml/smbdotconf/security/directorymask.xml | 5 ----- .../smbdotconf/security/directorysecuritymask.xml | 4 +--- docs-xml/smbdotconf/security/forcecreatemode.xml | 6 ------ .../smbdotconf/security/forcedirectorymode.xml | 6 ------ .../security/forcedirectorysecuritymode.xml | 5 +---- docs-xml/smbdotconf/security/forcesecuritymode.xml | 5 +---- docs-xml/smbdotconf/security/securitymask.xml | 4 +--- 8 files changed, 4 insertions(+), 36 deletions(-) diff --git a/docs-xml/smbdotconf/security/createmask.xml b/docs-xml/smbdotconf/security/createmask.xml index 59e208d..5df0718 100644 --- a/docs-xml/smbdotconf/security/createmask.xml +++ b/docs-xml/smbdotconf/security/createmask.xml @@ -26,11 +26,6 @@ This parameter does not affect directory masks. See the parameter for details. - - - New in Samba 4.0.0. This mask is applied whenever permissions are changed on a file. To allow clients full control - over permission changes it should be set to 0777. - force create mode diff --git a/docs-xml/smbdotconf/security/directorymask.xml b/docs-xml/smbdotconf/security/directorymask.xml index 2ebfc16..b17625c 100644 --- a/docs-xml/smbdotconf/security/directorymask.xml +++ b/docs-xml/smbdotconf/security/directorymask.xml @@ -23,11 +23,6 @@ Following this Samba will bit-wise 'OR' the UNIX mode created from this parameter with the value of the parameter. This parameter is set to 000 by default (i.e. no extra mode bits are added). - - - New in Samba 4.0.0. This mask is applied whenever permissions are changed on a directory. To allow clients full control - over permission changes it should be set to 0777. - force directory mode diff --git a/docs-xml/smbdotconf/security/directorysecuritymask.xml b/docs-xml/smbdotconf/security/directorysecuritymask.xml index c5c8c65..ad208f4 100644 --- a/docs-xml/smbdotconf/security/directorysecuritymask.xml +++ b/docs-xml/smbdotconf/security/directorysecuritymask.xml @@ -5,9 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to mask - any permission bit changes on directories. + This parameter has been removed for Samba 4.0.0. diff --git a/docs-xml/smbdotconf/security/forcecreatemode.xml b/docs-xml/smbdotconf/security/forcecreatemode.xml index 5a57a29..a3f1c2c 100644 --- a/docs-xml/smbdotconf/security/forcecreatemode.xml +++ b/docs-xml/smbdotconf/security/forcecreatemode.xml @@ -10,12 +10,6 @@ mode after the mask set in the create mask parameter is applied. - - New in Samba 4.0.0. This mode is also 'OR'ed into the mode bits whenever - permissions are changed on a file, not just when the file is created. - This replaces the now removed force security mode. - - The example below would force all newly created files to have read and execute permissions set for 'group' and 'other' as well as the read/write/execute bits set for the 'user'. diff --git a/docs-xml/smbdotconf/security/forcedirectorymode.xml b/docs-xml/smbdotconf/security/forcedirectorymode.xml index e5b37ea..7effc0e 100644 --- a/docs-xml/smbdotconf/security/forcedirectorymode.xml +++ b/docs-xml/smbdotconf/security/forcedirectorymode.xml @@ -12,12 +12,6 @@ mask in the parameter directory mask is applied. - - New in Samba 4.0.0. This mode is also 'OR'ed into the mode bits whenever - permissions are changed on a directory, not just when the file is created. - This replaces the now removed force directory security mode. - - The example below would force all created directories to have read and execute permissions set for 'group' and 'other' as well as the read/write/execute bits set for the 'user'. diff --git a/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml b/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml index 3ea3b5c..a45395d 100644 --- a/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml +++ b/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml @@ -5,10 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to - force any permission changes on directories to include specific UNIX - permission bits. + This parameter has been removed for Samba 4.0.0. diff --git a/docs-xml/smbdotconf/security/forcesecuritymode.xml b/docs-xml/smbdotconf/security/forcesecuritymode.xml index 2568bcc..5a9479e 100644 --- a/docs-xml/smbdotconf/security/forcesecuritymode.xml +++ b/docs-xml/smbdotconf/security/forcesecuritymode.xml @@ -5,10 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to - force any permission changes on files to include specific UNIX - permission bits. + This parameter has been removed for Samba 4.0.0. diff --git a/docs-xml/smbdotconf/security/securitymask.xml b/docs-xml/smbdotconf/security/securitymask.xml index cb7fcfa..e535d32 100644 --- a/docs-xml/smbdotconf/security/securitymask.xml +++ b/docs-xml/smbdotconf/security/securitymask.xml @@ -5,9 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to mask - any permission bit changes on files. + This parameter has been removed for Samba 4.0.0. -- 1.7.9.5