From 053dfa2799f11fcc49bd353abfaf0bc4d981011c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 8 Mar 2013 16:49:21 +1100 Subject: [PATCH 1/3] param: Remove incorrectly added defaults in AD DC allowing WORLD WRITABLE files These defaults were incorrectly added in fc5caffbc139d63cab1ec105884863f73772586f in what turns out to be an incorrect fix for bug #9462, which was in turn introduced by the swapping of security mask (default 0777) for create mask (0755) in 6adc7dad96b8c7366da042f0d93b28c1ecb092eb. While the permissions on sysvol and netlogon (the default shares) were fixed by provision, any additional shares that did not yet have an explit ACL set would create world-writable files by default. Administrators will need to manually correct the file permissions on any additional shares that were created after installation of the AD DC. Andrew Bartlett Reviewed-by: Michael Adam Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Sun Mar 10 12:00:31 CET 2013 on sn-devel-104 (cherry picked from commit 287b5f6c0f40d3e3d09bc2ce80f5fee02cbae40f) --- source3/param/loadparm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 0e1b019..007b418 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -3502,8 +3502,6 @@ static void init_locals(void) lp_do_parameter(-1, "map readonly", "no"); lp_do_parameter(-1, "map archive", "no"); lp_do_parameter(-1, "store dos attributes", "yes"); - lp_do_parameter(-1, "create mask", "0777"); - lp_do_parameter(-1, "directory mask", "0777"); } } -- 1.7.10.4 From fc19aafa6e3e1dd1bed121c1527c52519614429f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 8 Mar 2013 16:15:37 +1100 Subject: [PATCH 2/3] smbd:posix_acls Remove incorrectly added lp_create_mask() and lp_dir_mask() calls When 6adc7dad96b8c7366da042f0d93b28c1ecb092eb removed the calls to lp_security_mask/lp_force_security_mode/lp_dir_security_mask/lp_force_dir_security_mode these calls were replaced with lp_create_mask() and lp_dir_mask() The issue is that while lp_security_mask() and lp_dir_security_mask defaulted to 0777, the replacement calls did not. This changes behaviour, and incorrectly prevents a posix mode being specified by the client from being applied to the disk in the non-ACL enabled case. Andrew Bartlett Reviewed-by: Jeremy Allison Reviewed-by: Michael Adam (cherry picked from commit fc496ef323c908a6b621198d9dc8076f6857385e) --- source3/smbd/posix_acls.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index bbc1eed..3ff34fc 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -3083,14 +3083,11 @@ SMB_ACL_T free_empty_sys_acl(connection_struct *conn, SMB_ACL_T the_acl) static bool convert_canon_ace_to_posix_perms( files_struct *fsp, canon_ace *file_ace_list, mode_t *posix_perms) { - int snum = SNUM(fsp->conn); size_t ace_count = count_canon_ace_list(file_ace_list); canon_ace *ace_p; canon_ace *owner_ace = NULL; canon_ace *group_ace = NULL; canon_ace *other_ace = NULL; - mode_t and_bits; - mode_t or_bits; if (ace_count != 3) { DEBUG(3,("convert_canon_ace_to_posix_perms: Too many ACE " @@ -3130,20 +3127,6 @@ static bool convert_canon_ace_to_posix_perms( files_struct *fsp, canon_ace *file if (fsp->is_directory) *posix_perms |= (S_IWUSR|S_IXUSR); - /* If requested apply the masks. */ - - /* Get the initial bits to apply. */ - - if (fsp->is_directory) { - and_bits = lp_dir_mask(snum); - or_bits = lp_force_dir_mode(snum); - } else { - and_bits = lp_create_mask(snum); - or_bits = lp_force_create_mode(snum); - } - - *posix_perms = (((*posix_perms) & and_bits)|or_bits); - DEBUG(10,("convert_canon_ace_to_posix_perms: converted u=%o,g=%o,w=%o " "to perm=0%o for file %s.\n", (int)owner_ace->perms, (int)group_ace->perms, (int)other_ace->perms, -- 1.7.10.4 From ee3ac648598f7de27dea565381faebb313dba471 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sun, 10 Mar 2013 20:25:53 +1100 Subject: [PATCH 3/3] Revert "Ensure the masks don't conflict with the ACL checks." This reverts commit 78594909b8b22bd07978922b1c85dfd6f6456963 which was needed by 7622aa16adeb00bf161a6dd07664c37125391272. This change masked bug #9462 which was fixed by 2013bb9b4dbed747921df2591068e2765428f57d. The issue was that the defaults for the substituted parameters did not match the old parameter. Changing the values in our test suite hid the issue, but did not fix the issue. (Additional change in the revert is to correct the expected ACL value in posixacl.py due to changed implied inherited permissions). Andrew Bartlett Reviewed-by: Michael Adam Reviewed-by: Jeremy Allison Autobuild-User(master): Michael Adam Autobuild-Date(master): Mon Mar 11 19:46:24 CET 2013 on sn-devel-104 (cherry picked from commit 58e385a5ac37c072a4eef3baa7926b799a732e94) The last 3 patches address bug #Bug 9709 - CVE-2013-1863; Remove forced set of 'create mask' to 0777. CVE-2013-1863: World-writeable files may be created in additional shares on a Samba 4.0 AD DC. --- selftest/target/Samba3.pm | 3 +-- selftest/target/Samba4.pm | 3 +-- source4/scripting/python/samba/tests/posixacl.py | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 6c63413..70304fe 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -897,8 +897,7 @@ sub provision($$$$$$) map system = no map readonly = no store dos attributes = yes - create mask = 0777 - directory mask = 0777 + create mask = 755 dos filemode = yes vfs objects = $vfs_modulesdir_abs/acl_xattr.so $vfs_modulesdir_abs/fake_acls.so $vfs_modulesdir_abs/xattr_tdb.so $vfs_modulesdir_abs/streams_depot.so diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm index 5f1c907..d17a37c3 100644 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -1387,8 +1387,7 @@ sub provision_plugin_s4_dc($$) smbd:sharedelay = 100000 smbd:writetimeupdatedelay = 500000 - create mask = 0777 - directory mask = 0777 + create mask = 755 dos filemode = yes dcerpc endpoint servers = -winreg -srvsvc diff --git a/source4/scripting/python/samba/tests/posixacl.py b/source4/scripting/python/samba/tests/posixacl.py index 652721f..6a234e4 100644 --- a/source4/scripting/python/samba/tests/posixacl.py +++ b/source4/scripting/python/samba/tests/posixacl.py @@ -210,7 +210,7 @@ class PosixAclMappingTests(TestCaseInTempDir): smbd.chown(self.tempdir, BA_id, SO_id) smbd.set_simple_acl(self.tempdir, 0750) facl = getntacl(self.lp, self.tempdir, direct_db_access=False) - acl = "O:BAG:SOD:(A;;0x001f01ff;;;BA)(A;;0x001200a9;;;SO)(A;;;;;WD)(A;OICIIO;0x001f01ff;;;CO)(A;OICIIO;0x001f01ff;;;CG)(A;OICIIO;0x001f01ff;;;WD)" + acl = "O:BAG:SOD:(A;;0x001f01ff;;;BA)(A;;0x001200a9;;;SO)(A;;;;;WD)(A;OICIIO;0x001f01ff;;;CO)(A;OICIIO;0x001200a9;;;CG)(A;OICIIO;0x001200a9;;;WD)" anysid = security.dom_sid(security.SID_NT_SELF) self.assertEquals(acl, facl.as_sddl(anysid)) -- 1.7.10.4