[PATCH] Fix GPOs by fixing 'vfs objects' handling in loadparm (this time for sure...)
Michael Adam
obnox at samba.org
Thu Nov 15 15:39:58 MST 2012
Hi Andrew,
as noted, on irc, I am reviewing this patchset.
I hope that this fixes it for good!
cheers - Michael
PS: if all goes well, this mail will also set the
patchwork.samba.org state to under review
On 2012-11-14 at 12:55 +1100, Andrew Bartlett wrote:
> The attached patch fixes up GPO handling for me. This is what I've been
> able to do, with a Windows 7 client running GPMC:
>
> - as a "Domain Admin" who isn't actually "administrator"
> - create a new group policy
> - assign it to the domain
> - use it to remove the "games" menu and the recycle bin
> - select the default domain policy, and not get any errors
>
> Then I logged in as an unprivileged user, and the policy was correctly
> applied.
>
> Attached is also a patch to add a "samba-tool gpo aclcheck" tool, which
> does much the same ACL check that GPMC does, and can be run remotely.
>
> As you will see in the patch, and particularly if you run testparm
> before and after the change to loadparm ensures the [netlogon] and
> [sysvol] shares actually use the required VFS modules.
>
> Hopefully this really is the final fix - and it is nice because it shows
> at at least some of this was working before August, when I 'tidied this
> up', and broke it. However, I've felt like I was at the end of this
> road many times before, and so we will persevere.
>
> Thanks,
>
> Andrew Bartlett
> --
> Andrew Bartlett http://samba.org/~abartlet/
> Authentication Developer, Samba Team http://samba.org
> >From 54df9fbf38b1739c68bbfa09bcf59e60c93f7521 Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Wed, 14 Nov 2012 12:30:54 +1100
> Subject: [PATCH 2/2] s3-param: Handle setting default "vfs objects" in
> init_locals()
>
> This function is helpfully called between when we finish processing
> the globals and when we start processing the individual shares. This
> means that the "vfs objects" we specify here become the defaults for
> (eg) [netlogon] and [sysvol] but the admin can override these on a
> per-share basis or (as we must in make test) for the whole server.
>
> This broke setting and fetching of group policy objects from Windows
> clients, since this setting was moved from fileserver.conf in
> 8518dd6406c0132dfd8c44e084c2b39792974f2c, and wasn't found in 'make
> test' because we have to override the vfs objects to insert the
> xattr_tdb and fake_acl modules.
>
> Andrew Bartlett
> ---
> source3/param/loadparm.c | 101 +++++++++++++++++++++++-----------------------
> 1 files changed, 51 insertions(+), 50 deletions(-)
>
> diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
> index 12cb8db..d8648d1 100644
> --- a/source3/param/loadparm.c
> +++ b/source3/param/loadparm.c
> @@ -319,6 +319,7 @@ static void set_allowed_client_auth(void);
> static void add_to_file_list(const char *fname, const char *subfname);
> static bool lp_set_cmdline_helper(const char *pszParmName, const char *pszParmValue, bool store_values);
> static void free_param_opts(struct parmlist_entry **popts);
> +static bool lp_is_in_client(void);
>
> #include "lib/param/param_table.c"
>
> @@ -3469,12 +3470,60 @@ static bool equal_parameter(parm_type type, void *ptr1, void *ptr2)
> }
>
> /***************************************************************************
> - Initialize any local varients in the sDefault table.
> + Initialize any local variables in the sDefault table that depend on
> + items in [globals] and do any other work needed now that we just
> + finished processing the [globals].
> ***************************************************************************/
>
> void init_locals(void)
> {
> - /* None as yet. */
> + if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC) {
> + const char **vfs_objects = lp_vfs_objects(-1);
> + if (!vfs_objects || !vfs_objects[0]) {
> + if (lp_parm_const_string(-1, "xattr_tdb", "file", NULL)) {
> + lp_do_parameter(-1, "vfs objects", "dfs_samba4 acl_xattr xattr_tdb");
> + } else if (lp_parm_const_string(-1, "posix", "eadb", NULL)) {
> + lp_do_parameter(-1, "vfs objects", "dfs_samba4 acl_xattr posix_eadb");
> + } else {
> + lp_do_parameter(-1, "vfs objects", "dfs_samba4 acl_xattr");
> + }
> + }
> + lp_do_parameter(-1, "passdb backend", "samba_dsdb");
> +
> + lp_do_parameter(-1, "rpc_server:default", "external");
> + lp_do_parameter(-1, "rpc_server:svcctl", "embedded");
> + lp_do_parameter(-1, "rpc_server:srvsvc", "embedded");
> + lp_do_parameter(-1, "rpc_server:eventlog", "embedded");
> + lp_do_parameter(-1, "rpc_server:ntsvcs", "embedded");
> + lp_do_parameter(-1, "rpc_server:winreg", "embedded");
> + lp_do_parameter(-1, "rpc_server:spoolss", "embedded");
> + lp_do_parameter(-1, "rpc_daemon:spoolssd", "embedded");
> + lp_do_parameter(-1, "rpc_server:tcpip", "no");
> +
> + lp_do_parameter(-1, "map hidden", "no");
> + lp_do_parameter(-1, "map system", "no");
> + lp_do_parameter(-1, "map readonly", "no");
> + lp_do_parameter(-1, "store dos attributes", "yes");
> + lp_do_parameter(-1, "create mask", "0777");
> + lp_do_parameter(-1, "directory mask", "0777");
> + }
> +
> + set_allowed_client_auth();
> +
> + if (lp_security() == SEC_ADS && strchr(lp_passwordserver(), ':')) {
> + DEBUG(1, ("WARNING: The optional ':port' in password server = %s is deprecated\n",
> + lp_passwordserver()));
> + }
> +
> + /* Now we check bWINSsupport and set szWINSserver to 127.0.0.1 */
> + /* if bWINSsupport is true and we are in the client */
> + if (lp_is_in_client() && Globals.bWINSsupport) {
> + lp_do_parameter(GLOBAL_SECTION_SNUM, "wins server", "127.0.0.1");
> + }
> +
> + init_iconv();
> +
> + fault_configure(smb_panic_s3);
> }
>
> /***************************************************************************
> @@ -4868,56 +4917,8 @@ static bool lp_load_ex(const char *pszFname,
> }
> }
>
> - set_allowed_client_auth();
> -
> - if (lp_security() == SEC_ADS && strchr(lp_passwordserver(), ':')) {
> - DEBUG(1, ("WARNING: The optional ':port' in password server = %s is deprecated\n",
> - lp_passwordserver()));
> - }
> -
> bLoaded = true;
>
> - /* Now we check bWINSsupport and set szWINSserver to 127.0.0.1 */
> - /* if bWINSsupport is true and we are in the client */
> - if (lp_is_in_client() && Globals.bWINSsupport) {
> - lp_do_parameter(GLOBAL_SECTION_SNUM, "wins server", "127.0.0.1");
> - }
> -
> - init_iconv();
> -
> - fault_configure(smb_panic_s3);
> -
> - if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC) {
> - const char **vfs_objects = lp_vfs_objects(-1);
> - if (!vfs_objects || !vfs_objects[0]) {
> - if (lp_parm_const_string(-1, "xattr_tdb", "file", NULL)) {
> - lp_do_parameter(-1, "vfs objects", "dfs_samba4 acl_xattr xattr_tdb");
> - } else if (lp_parm_const_string(-1, "posix", "eadb", NULL)) {
> - lp_do_parameter(-1, "vfs objects", "dfs_samba4 acl_xattr posix_eadb");
> - } else {
> - lp_do_parameter(-1, "vfs objects", "dfs_samba4 acl_xattr");
> - }
> - }
> - lp_do_parameter(-1, "passdb backend", "samba_dsdb");
> -
> - lp_do_parameter(-1, "rpc_server:default", "external");
> - lp_do_parameter(-1, "rpc_server:svcctl", "embedded");
> - lp_do_parameter(-1, "rpc_server:srvsvc", "embedded");
> - lp_do_parameter(-1, "rpc_server:eventlog", "embedded");
> - lp_do_parameter(-1, "rpc_server:ntsvcs", "embedded");
> - lp_do_parameter(-1, "rpc_server:winreg", "embedded");
> - lp_do_parameter(-1, "rpc_server:spoolss", "embedded");
> - lp_do_parameter(-1, "rpc_daemon:spoolssd", "embedded");
> - lp_do_parameter(-1, "rpc_server:tcpip", "no");
> -
> - lp_do_parameter(-1, "map hidden", "no");
> - lp_do_parameter(-1, "map system", "no");
> - lp_do_parameter(-1, "map readonly", "no");
> - lp_do_parameter(-1, "store dos attributes", "yes");
> - lp_do_parameter(-1, "create mask", "0777");
> - lp_do_parameter(-1, "directory mask", "0777");
> - }
> -
> bAllowIncludeRegistry = true;
>
> return (bRetval);
> --
> 1.7.7.6
>
> >From 14df72d4cb6ab9719dab59a6a160f4083477d69a Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Mon, 5 Nov 2012 19:36:28 +1100
> Subject: [PATCH 1/2] samba-tool: Add new samba-tool gpo aclcheck and test
>
> ---
> source4/scripting/python/samba/netcmd/gpo.py | 63 ++++++++++++++++++++
> .../scripting/python/samba/tests/samba_tool/gpo.py | 10 +++
> 2 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/source4/scripting/python/samba/netcmd/gpo.py b/source4/scripting/python/samba/netcmd/gpo.py
> index 347231b..f70317a 100644
> --- a/source4/scripting/python/samba/netcmd/gpo.py
> +++ b/source4/scripting/python/samba/netcmd/gpo.py
> @@ -1072,6 +1072,68 @@ class cmd_del(Command):
> self.outf.write("GPO %s deleted.\n" % gpo)
>
>
> +class cmd_aclcheck(Command):
> + """Check all GPOs have matching LDAP and DS ACLs."""
> +
> + synopsis = "%prog [options]"
> +
> + takes_optiongroups = {
> + "sambaopts": options.SambaOptions,
> + "versionopts": options.VersionOptions,
> + "credopts": options.CredentialsOptions,
> + }
> +
> + takes_options = [
> + Option("-H", "--URL", help="LDB URL for database or target server", type=str,
> + metavar="URL", dest="H")
> + ]
> +
> + def run(self, H=None, sambaopts=None, credopts=None, versionopts=None):
> +
> + self.lp = sambaopts.get_loadparm()
> + self.creds = credopts.get_credentials(self.lp, fallback_machine=True)
> +
> + self.url = dc_url(self.lp, self.creds, H)
> +
> + # We need to know writable DC to setup SMB connection
> + if H and H.startswith('ldap://'):
> + dc_hostname = H[7:]
> + self.url = H
> + else:
> + dc_hostname = netcmd_finddc(self.lp, self.creds)
> + self.url = dc_url(self.lp, self.creds, dc=dc_hostname)
> +
> + samdb_connect(self)
> +
> + msg = get_gpo_info(self.samdb, None)
> +
> + for m in msg:
> + # verify UNC path
> + unc = m['gPCFileSysPath'][0]
> + try:
> + [dom_name, service, sharepath] = parse_unc(unc)
> + except ValueError:
> + raise CommandError("Invalid GPO path (%s)" % unc)
> +
> + # SMB connect to DC
> + try:
> + conn = smb.SMB(dc_hostname, service, lp=self.lp, creds=self.creds)
> + except Exception:
> + raise CommandError("Error connecting to '%s' using SMB" % dc_hostname)
> +
> + fs_sd = conn.get_acl(sharepath, security.SECINFO_OWNER | security.SECINFO_GROUP | security.SECINFO_DACL, security.SEC_FLAG_MAXIMUM_ALLOWED)
> +
> + ds_sd_ndr = m['ntSecurityDescriptor'][0]
> + ds_sd = ndr_unpack(security.descriptor, ds_sd_ndr).as_sddl()
> +
> + # Create a file system security descriptor
> + domain_sid = security.dom_sid(self.samdb.get_domain_sid())
> + expected_fs_sddl = dsacl2fsacl(ds_sd, domain_sid)
> +
> + if (fs_sd.as_sddl(domain_sid) != expected_fs_sddl):
> + raise CommandError("Invalid GPO ACL %s on path (%s), should be %s" % (fs_sd.as_sddl(domain_sid), sharepath, expected_fs_sddl))
> +
> +
> class cmd_gpo(SuperCommand):
> """Group Policy Object (GPO) management."""
>
> @@ -1088,3 +1150,4 @@ class cmd_gpo(SuperCommand):
> subcommands["fetch"] = cmd_fetch()
> subcommands["create"] = cmd_create()
> subcommands["del"] = cmd_del()
> + subcommands["aclcheck"] = cmd_aclcheck()
> diff --git a/source4/scripting/python/samba/tests/samba_tool/gpo.py b/source4/scripting/python/samba/tests/samba_tool/gpo.py
> index 7ada91f..82e7268 100644
> --- a/source4/scripting/python/samba/tests/samba_tool/gpo.py
> +++ b/source4/scripting/python/samba/tests/samba_tool/gpo.py
> @@ -44,6 +44,16 @@ os.environ["SERVER"])
> self.assertCmdSuccess(result, "Ensuring gpo fetched successfully")
> shutil.rmtree(os.path.join(self.tempdir, "policy"))
>
> + def test_show(self):
> + """Show a real GPO, and make sure it passes"""
> + (result, out, err) = self.runsubcmd("gpo", "show", self.gpo_guid, "-H", "ldap://%s" % os.environ["SERVER"])
> + self.assertCmdSuccess(result, "Ensuring gpo fetched successfully")
> +
> + def test_aclcheck(self):
> + """Check all the GPOs on the remote server have correct ACLs"""
> + (result, out, err) = self.runsubcmd("gpo", "aclcheck", "-H", "ldap://%s" % os.environ["SERVER"], "-U%s%%%s" % (os.environ["USERNAME"], os.environ["PASSWORD"]))
> + self.assertCmdSuccess(result, "Ensuring gpo checked successfully")
> +
> def setUp(self):
> """set up a temporary GPO to work with"""
> super(GpoCmdTestCase, self).setUp()
> --
> 1.7.7.6
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20121115/a043ef8b/attachment.pgp>
More information about the samba-technical
mailing list