[PATCHES v3] GPO support for client machine policy

David Mulder dmulder at suse.com
Wed Dec 13 19:04:43 UTC 2017


And here are the patches after rebasing the unapply fix patches.

 docs-xml/smbdotconf/domain/gpoupdatecommand.xml    |  11 +-
 docs-xml/smbdotconf/winbind/applygrouppolicies.xml |  19 +++
 lib/param/loadparm.c                               |   1 +
 python/samba/gp_env_var_ext.py                     | 111 +++++++++++++++++
 python/samba/gp_file_append.py                     |  97 +++++++++++++++
 python/samba/gpclass.py                            | 174
++++++++++++++------------
 selftest/target/Samba4.pm                          |   2 +-
 source3/param/loadparm.c                           |   2 +
 source3/winbindd/winbindd.c                        |   2 +
 source3/winbindd/winbindd_gpupdate.c               | 116 ++++++++++++++++++
 source3/winbindd/winbindd_proto.h                  |   3 +
 source3/winbindd/wscript_build                     |   3 +-
 source4/dsdb/gpo/gpo_update.c                      | 193
-----------------------------
 source4/dsdb/wscript_build                         |   9 --
 source4/scripting/bin/samba_gpoupdate              |  52 ++++++--
 source4/scripting/bin/wscript_build                |   2 +-
 source4/scripting/wscript_build                    |   7 +-
 source4/torture/gpo/apply.c                        | 302
+++++++++++++++++++++++++++++++++++++++-------
 18 files changed, 758 insertions(+), 348 deletions(-)

On 12/13/2017 10:53 AM, David Mulder via samba-technical wrote:
> These patches add Group Policy support for client machines. Adds a
> winbind event that calls samba_gpoupdate to apply local machine
> policies. Adds the option "apply group policies" to smb.conf, which
> determines whether group policy will be applied to the client.
> To start off, we only have Environment Variable policies.
>
> Fixes in this patch version:
> * changed "winbind gpupdate" smb.conf option to "apply group policies".
> * Checks for windows paths in the PATH variable, and ignores them.
> * Also adds testing for the windows path ignore.
>
>  docs-xml/smbdotconf/domain/gpoupdatecommand.xml    |  11 +-
>  docs-xml/smbdotconf/winbind/applygrouppolicies.xml |  19 +++
>  lib/param/loadparm.c                               |   1 +
>  python/samba/gp_env_var_ext.py                     | 111 +++++++++++++++++
>  python/samba/gp_file_append.py                     |  97 +++++++++++++++
>  python/samba/gpclass.py                            | 174 ++++++++++++++------------
>  selftest/target/Samba4.pm                          |   2 +-
>  source3/param/loadparm.c                           |   2 +
>  source3/winbindd/winbindd.c                        |   2 +
>  source3/winbindd/winbindd_gpupdate.c               | 116 ++++++++++++++++++
>  source3/winbindd/winbindd_proto.h                  |   3 +
>  source3/winbindd/wscript_build                     |   3 +-
>  source4/dsdb/gpo/gpo_update.c                      | 193 -----------------------------
>  source4/dsdb/wscript_build                         |   9 --
>  source4/scripting/bin/samba_gpoupdate              |  52 ++++++--
>  source4/scripting/bin/wscript_build                |   2 +-
>  source4/scripting/wscript_build                    |   7 +-
>  source4/torture/gpo/apply.c                        | 302 +++++++++++++++++++++++++++++++++++++++-------
>  18 files changed, 758 insertions(+), 348 deletions(-)
>

-- 
David Mulder
SUSE Labs Software Engineer - Samba
dmulder at suse.com
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

-------------- next part --------------
From 95ffa180791f9e6060cac146acd735192a360e6f Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Wed, 9 Aug 2017 09:25:53 -0600
Subject: [PATCH 1/6] gpo: Add machine policies

Add environment variable machine policies

Signed-off-by: David Mulder <dmulder at suse.com>
---
 python/samba/gp_env_var_ext.py        |  98 +++++++++++++++++++
 python/samba/gp_file_append.py        |  97 +++++++++++++++++++
 python/samba/gpclass.py               | 174 ++++++++++++++++++----------------
 source4/scripting/bin/samba_gpoupdate |  52 ++++++++--
 source4/scripting/bin/wscript_build   |   2 +-
 source4/scripting/wscript_build       |   7 +-
 6 files changed, 336 insertions(+), 94 deletions(-)
 create mode 100644 python/samba/gp_env_var_ext.py
 create mode 100644 python/samba/gp_file_append.py

diff --git a/python/samba/gp_env_var_ext.py b/python/samba/gp_env_var_ext.py
new file mode 100644
index 00000000000..ef60ae77c6e
--- /dev/null
+++ b/python/samba/gp_env_var_ext.py
@@ -0,0 +1,98 @@
+from gpclass import gp_ext, file_to
+from gp_file_append import ini_file_append
+import xml.etree.ElementTree as etree
+import re, os, os.path
+
+sysconfdir = '/etc'
+
+class xml_to_env(file_to):
+    @staticmethod
+    def get_local_env():
+        global sysconfdir
+        return ini_file_append(os.path.join(sysconfdir, 'profile'))
+
+    def set_env_var(self, val):
+        global sysconfdir
+        if type(val) is not dict:
+            val = { 'value': val, 'action': 'U' }
+        profile = ini_file_append(os.path.join(sysconfdir, 'profile'))
+
+        old_val = profile[self.attribute]
+        self.logger.info('Environment Variable %s was changed from %s to %s' \
+                         % (self.attribute, old_val, val['value']))
+
+        if val['value'] == None:
+            del profile[self.attribute]
+        # Update, Replace, and Create all really do the same thing
+        elif val['action'] == 'U' or \
+             val['action'] == 'R' or \
+             val['action'] == 'C':
+            if self.attribute == 'PATH' and \
+               'partial' in val and \
+               val['partial'] == '1':
+                profile[self.attribute] = '$PATH:%s' % val['value']
+            else:
+                profile[self.attribute] = val['value']
+        # Remove should blank the variable
+        elif val['action'] == 'D':
+            profile[self.attribute] = ''
+
+        self.gp_db.store(str(self), self.attribute, old_val)
+
+    def mapper(self):
+        return self
+
+    def __getitem__(self, key):
+        return (self.set_env_var, self.explicit)
+
+    def __str__(self):
+        return 'Environment'
+
+class env_var_unapply_map:
+    def __getitem__(self, key):
+        return (key, xml_to_env)
+
+    def __contains__(self, key):
+        return True
+
+class gp_environment_variable_ext(gp_ext):
+    def __init__(self, logger, sysconfdir_in):
+        global sysconfdir
+        super(gp_environment_variable_ext, self).__init__(logger)
+        sysconfdir = sysconfdir_in
+
+    def list(self, rootpath):
+        return os.path.join(rootpath,
+           'MACHINE/Preferences/EnvironmentVariables/EnvironmentVariables.xml')
+
+    def apply_map(self):
+        if not hasattr(self, 'xml_conf'):
+            return { 'Environment' : env_var_unapply_map() }
+        else:
+            return {
+                'Environment' : {
+                    a.attrib['name']: (a.attrib['name'], xml_to_env) \
+                        for a in self.xml_conf.findall('EnvironmentVariable')
+                }
+            }
+
+    def read(self, policy):
+        self.xml_conf = etree.fromstring(policy)
+        apply_map = self.apply_map()['Environment']
+        ret = False
+
+        for env_var in self.xml_conf.findall('EnvironmentVariable'):
+            (att, setter) = apply_map.get(env_var.attrib['name'])
+            ret = True
+            setter(self.logger,
+                   self.ldb,
+                   self.gp_db,
+                   self.lp,
+                   att,
+                   env_var.find('Properties').attrib).update_samba()
+            self.gp_db.commit()
+        return ret
+
+    def __str__(self):
+        return 'Environment GPO extension'
+
diff --git a/python/samba/gp_file_append.py b/python/samba/gp_file_append.py
new file mode 100644
index 00000000000..69baeef701f
--- /dev/null
+++ b/python/samba/gp_file_append.py
@@ -0,0 +1,97 @@
+import re, os.path, hashlib
+
+class gp_file_append:
+    def __init__(self, filename):
+        self.filename = filename
+        self.section = \
+            '#\n# Samba GPO Section\n# These settings are applied via GPO\n#'
+        self.section_end = '#\n# End Samba GPO Section\n#'
+        self.checksum = None
+
+    def __get_file_data(self):
+        if not os.path.exists(os.path.dirname(self.filename)):
+            raise Exception, 'path %s not found' % \
+                os.path.dirname(self.filename)
+        if os.path.exists(self.filename):
+            data = open(self.filename, 'r').read()
+        else:
+            data = ''
+        checksum = hashlib.sha256(data).digest()
+        return (data, checksum)
+
+    def get_section(self):
+        self.data, self.checksum = self.__get_file_data()
+
+        # Get the current applied contents (if any)
+        current = ''
+        try:
+            self.start = re.finditer(self.section, self.data).next()
+            self.end = re.finditer(self.section_end, self.data).next()
+        except:
+            self.start = None
+            self.end = None
+        if self.start and self.end:
+            current = self.data[self.start.end():self.end.start()]
+        return current
+
+    def set_section(self, results):
+        _, checksum = self.__get_file_data()
+        if checksum != self.checksum:
+            raise Exception, '%s was modified elsewhere, checksum failed' % \
+                self.filename
+        if results.strip():
+            results = self.section + '\n' + results + '\n' + self.section_end
+            if self.start and self.end:
+                ndata = self.data[:self.start.start()] + results + \
+                    self.data[self.end.end():]
+            else:
+                ndata = self.data + results
+        else: # Remove the GPO section if all settings are removed
+            if self.start and self.end:
+                ndata = self.data[:self.start.start()] + \
+                    self.data[self.end.end():]
+            else:
+                ndata = self.data
+        open(self.filename, 'w').write(ndata)
+
+class ini_file_append:
+    def __init__(self, filename):
+        self.modified = False
+        self.append = gp_file_append(filename)
+        self.vals = self.__get_section()
+
+    def keys(self):
+        return self.vals.keys()
+
+    def __get_section(self):
+        return {
+            l.split('=')[0]: l.split('=')[-1] \
+                for l in self.append.get_section().strip().split('\n')
+        }
+
+    def __set_section(self):
+        results = '\n'.join([
+            '%s=%s' % (key, val) \
+                for key, val in self.vals.iteritems() if key
+        ])
+        self.append.set_section(results)
+
+    def __getitem__(self, key):
+        if key in self.vals.keys():
+            return self.vals[key]
+        else:
+            return None
+
+    def __setitem__(self, key, val):
+        self.modified = True
+        self.vals[key] = val
+
+    def __delitem__(self, key):
+        self.modified = True
+        if key in self.vals.keys():
+            del self.vals[key]
+
+    def __del__(self):
+        if self.modified:
+            self.__set_section()
+
diff --git a/python/samba/gpclass.py b/python/samba/gpclass.py
index 00330eb5ecb..02750105067 100644
--- a/python/samba/gpclass.py
+++ b/python/samba/gpclass.py
@@ -19,19 +19,12 @@ import sys
 import os
 import tdb
 sys.path.insert(0, "bin/python")
-import samba.gpo as gpo
-import optparse
-import ldb
-from samba.auth import system_session
-import samba.getopt as options
-from samba.samdb import SamDB
-from samba.netcmd import gpo as gpo_user
-import codecs
 from samba import NTSTATUSError
 from ConfigParser import ConfigParser
 from StringIO import StringIO
 from abc import ABCMeta, abstractmethod
 import xml.etree.ElementTree as etree
+import re
 
 try:
     from enum import Enum
@@ -217,12 +210,19 @@ class gp_log:
         exts = guid_obj.findall('gp_ext')
         if exts is not None:
             for ext in exts:
-                ext_map = {val[0]: val[1] for (key, val) in \
-                    data_maps[ext.attrib['name']].items()}
                 attrs = ext.findall('attribute')
                 for attr in attrs:
-                    ret.append((attr.attrib['name'], attr.text,
-                                ext_map[attr.attrib['name']]))
+                    func = None
+                    if attr.attrib['name'] in data_maps[ext.attrib['name']]:
+                        func = data_maps[ext.attrib['name']]\
+                               [attr.attrib['name']][-1]
+                    else:
+                        for dmap in data_maps[ext.attrib['name']].keys():
+                            if data_maps[ext.attrib['name']][dmap][0] == \
+                               attr.attrib['name']:
+                                func = data_maps[ext.attrib['name']][dmap][-1]
+                                break
+                    ret.append((attr.attrib['name'], attr.text, func))
         return ret
 
     def delete(self, gp_ext_name, attribute):
@@ -286,6 +286,9 @@ class GPOStorage:
 class gp_ext(object):
     __metaclass__ = ABCMeta
 
+    def __init__(self, logger):
+        self.logger = logger
+
     @abstractmethod
     def list(self, rootpath):
         pass
@@ -295,14 +298,41 @@ class gp_ext(object):
         pass
 
     @abstractmethod
-    def parse(self, afile, ldb, conn, gp_db, lp):
+    def read(self, policy):
         pass
 
+    def parse(self, afile, ldb, conn, gp_db, lp):
+        self.ldb = ldb
+        self.gp_db = gp_db
+        self.lp = lp
+
+        # Fixing the bug where only some Linux Boxes capitalize MACHINE
+        try:
+            blist = afile.split('/')
+            idx = afile.lower().split('/').index('machine')
+            for case in [
+                            blist[idx].upper(),
+                            blist[idx].capitalize(),
+                            blist[idx].lower()
+                        ]:
+                bfile = '/'.join(blist[:idx]) + '/' + case + '/' + \
+                    '/'.join(blist[idx+1:])
+                try:
+                    return self.read(conn.loadfile(bfile.replace('/', '\\')))
+                except NTSTATUSError:
+                    continue
+        except ValueError:
+            try:
+                return self.read(conn.loadfile(afile.replace('/', '\\')))
+            except Exception as e:
+                self.logger.error(str(e))
+                return None
+
     @abstractmethod
     def __str__(self):
         pass
 
-class inf_to():
+class file_to():
     __metaclass__ = ABCMeta
 
     def __init__(self, logger, ldb, gp_db, lp, attribute, val):
@@ -317,7 +347,7 @@ class inf_to():
         return self.val
 
     def update_samba(self):
-        (upd_sam, value) = self.mapper().get(self.attribute)
+        (upd_sam, value) = self.mapper()[self.attribute]
         upd_sam(value())
 
     @abstractmethod
@@ -328,7 +358,7 @@ class inf_to():
     def __str__(self):
         pass
 
-class inf_to_kdc_tdb(inf_to):
+class inf_to_kdc_tdb(file_to):
     def mins_to_hours(self):
         return '%d' % (int(self.val)/60)
 
@@ -357,7 +387,7 @@ class inf_to_kdc_tdb(inf_to):
     def __str__(self):
         return 'Kerberos Policy'
 
-class inf_to_ldb(inf_to):
+class inf_to_ldb(file_to):
     '''This class takes the .inf file parameter (essentially a GPO file mapped
     to a GUID), hashmaps it to the Samba parameter, which then uses an ldb
     object to update the parameter to Samba4. Not registry oriented whatsoever.
@@ -415,8 +445,53 @@ class inf_to_ldb(inf_to):
     def __str__(self):
         return 'System Access'
 
+class gp_inf_ext(gp_ext):
+    @abstractmethod
+    def list(self, rootpath):
+        pass
+
+    @abstractmethod
+    def apply_map(self):
+        pass
+
+    def read(self, policy):
+        ret = False
+        inftable = self.apply_map()
+
+        current_section = None
 
-class gp_sec_ext(gp_ext):
+        # So here we would declare a boolean,
+        # that would get changed to TRUE.
+        #
+        # If at any point in time a GPO was applied,
+        # then we return that boolean at the end.
+
+        inf_conf = ConfigParser()
+        inf_conf.optionxform=str
+        try:
+            inf_conf.readfp(StringIO(policy))
+        except:
+            inf_conf.readfp(StringIO(policy.decode('utf-16')))
+
+        for section in inf_conf.sections():
+            current_section = inftable.get(section)
+            if not current_section:
+                continue
+            for key, value in inf_conf.items(section):
+                if current_section.get(key):
+                    (att, setter) = current_section.get(key)
+                    value = value.encode('ascii', 'ignore')
+                    ret = True
+                    setter(self.logger, self.ldb, self.gp_db, self.lp, att,
+                           value).update_samba()
+                    self.gp_db.commit()
+        return ret
+
+    @abstractmethod
+    def __str__(self):
+        pass
+
+class gp_sec_ext(gp_inf_ext):
     '''This class does the following two things:
         1) Identifies the GPO if it has a certain kind of filepath,
         2) Finally parses it.
@@ -424,15 +499,12 @@ class gp_sec_ext(gp_ext):
 
     count = 0
 
-    def __init__(self, logger):
-        self.logger = logger
-
     def __str__(self):
         return "Security GPO extension"
 
     def list(self, rootpath):
         return os.path.join(rootpath,
-                            "MACHINE/Microsoft/Windows NT/SecEdit/GptTmpl.inf")
+            "MACHINE/Microsoft/Windows NT/SecEdit/GptTmpl.inf")
 
     def listmachpol(self, rootpath):
         return os.path.join(rootpath, "Machine/Registry.pol")
@@ -465,61 +537,3 @@ class gp_sec_ext(gp_ext):
                                    }
                }
 
-    def read_inf(self, path, conn):
-        ret = False
-        inftable = self.apply_map()
-
-        policy = conn.loadfile(path.replace('/', '\\'))
-        current_section = None
-
-        # So here we would declare a boolean,
-        # that would get changed to TRUE.
-        #
-        # If at any point in time a GPO was applied,
-        # then we return that boolean at the end.
-
-        inf_conf = ConfigParser()
-        inf_conf.optionxform=str
-        try:
-            inf_conf.readfp(StringIO(policy))
-        except:
-            inf_conf.readfp(StringIO(policy.decode('utf-16')))
-
-        for section in inf_conf.sections():
-            current_section = inftable.get(section)
-            if not current_section:
-                continue
-            for key, value in inf_conf.items(section):
-                if current_section.get(key):
-                    (att, setter) = current_section.get(key)
-                    value = value.encode('ascii', 'ignore')
-                    ret = True
-                    setter(self.logger, self.ldb, self.gp_db, self.lp, att,
-                           value).update_samba()
-                    self.gp_db.commit()
-        return ret
-
-    def parse(self, afile, ldb, conn, gp_db, lp):
-        self.ldb = ldb
-        self.gp_db = gp_db
-        self.lp = lp
-
-        # Fixing the bug where only some Linux Boxes capitalize MACHINE
-        if afile.endswith('inf'):
-            try:
-                blist = afile.split('/')
-                idx = afile.lower().split('/').index('machine')
-                for case in [blist[idx].upper(), blist[idx].capitalize(),
-                             blist[idx].lower()]:
-                    bfile = '/'.join(blist[:idx]) + '/' + case + '/' + \
-                            '/'.join(blist[idx+1:])
-                    try:
-                        return self.read_inf(bfile, conn)
-                    except NTSTATUSError:
-                        continue
-            except ValueError:
-                try:
-                    return self.read_inf(afile, conn)
-                except:
-                    return None
-
diff --git a/source4/scripting/bin/samba_gpoupdate b/source4/scripting/bin/samba_gpoupdate
index f593e473fb1..d84594a6701 100755
--- a/source4/scripting/bin/samba_gpoupdate
+++ b/source4/scripting/bin/samba_gpoupdate
@@ -29,11 +29,19 @@ sys.path.insert(0, "bin/python")
 
 import optparse
 from samba import getopt as options
+from samba.auth import system_session
+try:
+    from samba.samdb import SamDB
+except:
+    SamDB = None
 from samba.gpclass import *
+from samba.gp_env_var_ext import gp_environment_variable_ext
 from samba.net import Net
 from samba.dcerpc import nbt
 from samba import smb
+import samba.gpo as gpo
 import logging
+import chardet
 
 ''' Fetch the hostname of a writable DC '''
 def get_dc_hostname(creds, lp):
@@ -50,6 +58,15 @@ def get_gpo_list(dc_hostname, creds, lp):
         gpos = ads.get_gpo_list(creds.get_username())
     return gpos
 
+def gpo_version(conn, path):
+    local_path = os.path.join(lp.get('cache directory'), 'gpt', path, 'GPT.INI')
+    if not os.path.exists(os.path.dirname(local_path)):
+        os.makedirs(os.path.dirname(local_path))
+    data = conn.loadfile(os.path.join(path, 'GPT.INI').replace('/', '\\'))
+    encoding = chardet.detect(data)
+    open(local_path, 'w').write(data.decode(encoding['encoding']))
+    return int(gpo.gpo_get_sysvol_gpt_version(os.path.dirname(local_path))[1])
+
 def apply_gp(lp, creds, test_ldb, logger, store, gp_extensions):
     gp_db = store.get_gplog(creds.get_username())
     dc_hostname = get_dc_hostname(creds, lp)
@@ -65,8 +82,7 @@ def apply_gp(lp, creds, test_ldb, logger, store, gp_extensions):
         if guid == 'Local Policy':
             continue
         path = os.path.join(lp.get('realm').lower(), 'Policies', guid)
-        local_path = os.path.join(lp.get("path", "sysvol"), path)
-        version = int(gpo.gpo_get_sysvol_gpt_version(local_path)[1])
+        version = gpo_version(conn, path)
         if version != store.get_int(guid):
             logger.info('GPO %s has changed' % guid)
             gp_db.state(GPOSTATE.APPLY)
@@ -74,13 +90,15 @@ def apply_gp(lp, creds, test_ldb, logger, store, gp_extensions):
             gp_db.state(GPOSTATE.ENFORCE)
         gp_db.set_guid(guid)
         store.start()
-        try:
-            for ext in gp_extensions:
+        for ext in gp_extensions:
+            try:
                 ext.parse(ext.list(path), test_ldb, conn, gp_db, lp)
-        except:
-            logger.error('Failed to parse gpo %s' % guid)
-            store.cancel()
-            continue
+            except Exception as e:
+                logger.error('Failed to parse gpo %s for extension %s' % \
+                    (guid, str(ext)))
+                logger.error('Message was: ' + str(e))
+                store.cancel()
+                continue
         store.store(guid, '%i' % version)
         store.commit()
 
@@ -115,6 +133,10 @@ if __name__ == "__main__":
     parser.add_option('-H', '--url', dest='url', help='URL for the samdb')
     parser.add_option('-X', '--unapply', help='Unapply Group Policy',
                       action='store_true')
+    parser.add_option('-M', '--machine', help='Apply machine policy',
+                      action='store_true', default=False)
+    parser.add_option('--sysconfdir', help='Location of sysconfdir: ex. /etc/',
+                      default='/etc/')
     parser.add_option_group(credopts)
 
     # Set the options and the arguments
@@ -148,10 +170,20 @@ if __name__ == "__main__":
     cache_dir = lp.get('cache directory')
     store = GPOStorage(os.path.join(cache_dir, 'gpo.tdb'))
 
-    gp_extensions = [gp_sec_ext(logger)]
+    gp_extensions = []
+    if opts.machine:
+        if lp.get('server role') == 'active directory domain controller':
+            gp_extensions.append(gp_sec_ext(logger))
+        gp_extensions.append(gp_environment_variable_ext(logger,
+                                                         opts.sysconfdir))
+    else:
+        pass # User extensions
 
     # Get a live instance of Samba
-    test_ldb = SamDB(url, session_info=session, credentials=creds, lp=lp)
+    if SamDB:
+        test_ldb = SamDB(url, session_info=session, credentials=creds, lp=lp)
+    else:
+        test_ldb = None
 
     if not opts.unapply:
         apply_gp(lp, creds, test_ldb, logger, store, gp_extensions)
diff --git a/source4/scripting/bin/wscript_build b/source4/scripting/bin/wscript_build
index 737f1bce411..043442b3407 100644
--- a/source4/scripting/bin/wscript_build
+++ b/source4/scripting/bin/wscript_build
@@ -7,6 +7,6 @@ if bld.CONFIG_SET('AD_DC_BUILD_IS_ENABLED'):
                    'samba_kcc',
                    'samba_upgradeprovision',
                    'samba_upgradedns',
-                   'samba_gpoupdate',
                    'gen_output.py']:
         bld.SAMBA_SCRIPT(script, pattern=script, installdir='.')
+bld.SAMBA_SCRIPT('samba_gpoupdate', pattern='samba_gpoupdate', installdir='.')
diff --git a/source4/scripting/wscript_build b/source4/scripting/wscript_build
index eb11c4202fe..2f53cce12b7 100644
--- a/source4/scripting/wscript_build
+++ b/source4/scripting/wscript_build
@@ -2,10 +2,11 @@
 
 from samba_utils import MODE_755
 
-sbin_files = None
+sbin_files = ''
 if bld.CONFIG_SET('AD_DC_BUILD_IS_ENABLED'):
-    sbin_files = 'bin/samba_dnsupdate bin/samba_spnupdate bin/samba_upgradedns bin/samba_kcc bin/samba_gpoupdate'
-    man_files = 'man/samba_gpoupdate.8'
+    sbin_files = 'bin/samba_dnsupdate bin/samba_spnupdate bin/samba_upgradedns bin/samba_kcc '
+sbin_files += 'bin/samba_gpoupdate'
+man_files = 'man/samba_gpoupdate.8'
 
 if sbin_files:
     bld.INSTALL_FILES('${SBINDIR}',
-- 
2.13.6


From 2557f8e9e6bafdf9f08d1a96a6da518d15bc3514 Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Wed, 6 Dec 2017 12:51:22 -0700
Subject: [PATCH 2/6] Revert "gpo: Create the gpo update service"

This reverts commit 5662e49b49f6557c80f216f510f224bbf800f40a.
---
 source4/dsdb/gpo/gpo_update.c | 193 ------------------------------------------
 source4/dsdb/wscript_build    |   9 --
 2 files changed, 202 deletions(-)
 delete mode 100644 source4/dsdb/gpo/gpo_update.c

diff --git a/source4/dsdb/gpo/gpo_update.c b/source4/dsdb/gpo/gpo_update.c
deleted file mode 100644
index 997e97ec14d..00000000000
--- a/source4/dsdb/gpo/gpo_update.c
+++ /dev/null
@@ -1,193 +0,0 @@
-/*
-   Unix SMB/CIFS mplementation.
-   GPO update service
-
-   Copyright (C) Luke Morrison 2013
-
-   Inspired by dns_updates.c written by Andrew Trigell 2009
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/
-
-*/
-
-#include "includes.h"
-#include "dsdb/samdb/samdb.h"
-#include "auth/auth.h"
-#include "smbd/service.h"
-#include "lib/messaging/irpc.h"
-#include "param/param.h"
-#include "system/filesys.h"
-#include "dsdb/common/util.h"
-#include "libcli/composite/composite.h"
-#include "libcli/security/dom_sid.h"
-#include "librpc/gen_ndr/ndr_irpc.h"
-#include "libds/common/roles.h"
-
-struct gpoupdate_service {
-	struct auth_session_info *system_session_info;
-	struct task_server *task;
-
-	/* status for periodic sysvol/GPO scan update - >sysvscan */
-	struct {
-		uint32_t interval;
-		struct tevent_timer *te;
-		struct tevent_req *subreq;
-		NTSTATUS status;
-	} sysvscan;
-};
-
-/*
-Called when the sysvol scan has finished
-*/
-static void gpoupdate_sysvscan_done(struct tevent_req *subreq)
-{
-	struct gpoupdate_service *service = tevent_req_callback_data(subreq,
-								     struct
-								     gpoupdate_service);
-	int ret;
-	int sys_errno;
-
-	service->sysvscan.subreq = NULL;
-
-	ret = samba_runcmd_recv(subreq, &sys_errno);
-	TALLOC_FREE(subreq);
-	if (ret != 0) {
-		service->sysvscan.status =
-		    map_nt_error_from_unix_common(sys_errno);
-	} else {
-		service->sysvscan.status = NT_STATUS_OK;
-	}
-
-	if (!NT_STATUS_IS_OK(service->sysvscan.status)) {
-		DEBUG(0, (__location__ ": Failed GPO update - %s\n",
-			  nt_errstr(service->sysvscan.status)));
-	} else {
-		DEBUG(3, ("Completed GPO update check OK\n"));
-	}
-}
-
-static NTSTATUS gpoupdate_sysvscan_schedule(struct gpoupdate_service *service);
-
-static void gpoupdate_scan_apply(struct gpoupdate_service *service);
-
-static void gpoupdate_sysvscan_handler_te(struct tevent_context *ev,
-					  struct tevent_timer *te,
-					  struct timeval t, void *ptr)
-{
-	struct gpoupdate_service *service =
-	    talloc_get_type(ptr, struct gpoupdate_service);
-
-	gpoupdate_scan_apply(service);
-	gpoupdate_sysvscan_schedule(service);
-}
-
-static NTSTATUS gpoupdate_sysvscan_schedule(struct gpoupdate_service *service)
-{
-	/*
-	 * This is configured, default to 900 sec (15 mins) in
-	 * gpoupdate_task_init via gpoupdate:config interval
-	 */
-	service->sysvscan.te =
-	    tevent_add_timer(service->task->event_ctx, service,
-			     timeval_current_ofs(service->sysvscan.interval, 0),
-			     gpoupdate_sysvscan_handler_te, service);
-	NT_STATUS_HAVE_NO_MEMORY(service->sysvscan.te);
-	return NT_STATUS_OK;
-}
-
-static void gpoupdate_scan_apply(struct gpoupdate_service *service)
-{
-	const char *const *gpo_update_command =
-	    lpcfg_gpo_update_command(service->task->lp_ctx);
-	const char *smbconf = lpcfg_configfile(service->task->lp_ctx);
-	TALLOC_FREE(service->sysvscan.subreq);
-	DEBUG(3, ("Calling GPO update script\n"));
-	service->sysvscan.subreq = samba_runcmd_send(service,
-						     service->task->event_ctx,
-						     timeval_current_ofs(20, 0),
-						     2, 0,
-						     gpo_update_command,
-						     smbconf, NULL);
-	if (service->sysvscan.subreq == NULL) {
-		DEBUG(0,
-		      (__location__
-		       ": samba_runcmd_send() failed with no memory\n"));
-		return;
-	}
-	tevent_req_set_callback(service->sysvscan.subreq,
-				gpoupdate_sysvscan_done, service);
-}
-
-static void gpoupdate_task_init(struct task_server *task)
-{
-	NTSTATUS status;
-	struct gpoupdate_service *service;
-
-	if (lpcfg_server_role(task->lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC) {
-		/* not useful for non-DC */
-		return;
-	}
-
-	task_server_set_title(task, "task[gpoupdate]");
-
-	service = talloc_zero(task, struct gpoupdate_service);
-	if (!service) {
-		task_server_terminate(task,
-				      "gpoupdate_task_init: out of memory",
-				      true);
-		return;
-	}
-	service->task = task;
-	task->private_data = service;
-
-	service->system_session_info = system_session(service->task->lp_ctx);
-	if (!service->system_session_info) {
-		task_server_terminate(task,
-				      "gpoupdate: Failed to obtain server "
-				      "credentials\n",
-				      true);
-		return;
-	}
-
-	service->sysvscan.interval = lpcfg_parm_int(task->lp_ctx, NULL,
-						    "gpoupdate",
-						    "config interval",
-						    900); /* in seconds */
-	status = gpoupdate_sysvscan_schedule(service);
-	if (!NT_STATUS_IS_OK(status)) {
-		task_server_terminate(task,
-				      talloc_asprintf(task,
-						      "gpoupdate: Failed to update "
-						      "sysvol scan schedule: %s\n",
-						      nt_errstr(status)),
-				      true);
-		return;
-	}
-}
-
-NTSTATUS server_service_gpoupdate_init(TALLOC_CTX *ctx);
-
-/*
-  register ourselves as a available server
-*/
-NTSTATUS server_service_gpoupdate_init(TALLOC_CTX *ctx)
-{
-	struct service_details details = {
-		.inhibit_fork_on_accept = true,
-		.inhibit_pre_fork = true
-	};
-	return register_server_service(ctx, "gpoupdate",
-				       gpoupdate_task_init,
-				       &details);
-}
diff --git a/source4/dsdb/wscript_build b/source4/dsdb/wscript_build
index 328497c8590..29c6f0e4a70 100644
--- a/source4/dsdb/wscript_build
+++ b/source4/dsdb/wscript_build
@@ -62,15 +62,6 @@ bld.SAMBA_MODULE('service_dns_update',
 	enabled=bld.AD_DC_BUILD_IS_ENABLED()
 	)
 
-bld.SAMBA_MODULE('service_gpo_update',
-	source='gpo/gpo_update.c',
-	subsystem='service',
-	init_function='server_service_gpoupdate_init',
-	deps='samdb UTIL_RUNCMD samba-util ldb samdb-common samba-errors talloc auth_system_session samba-hostconfig',
-	internal_module=False,
-	enabled=bld.AD_DC_BUILD_IS_ENABLED()
-	)
-
 bld.SAMBA_PYTHON('python_dsdb',
 	source='pydsdb.c',
 	# the dependency on dcerpc here is because gensec
-- 
2.13.6


From 03eef3ec6cc7c1529c65af5bbeb4e6d9a12840b3 Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Tue, 21 Nov 2017 03:44:12 -0700
Subject: [PATCH 3/6] gpo: Add the winbind call to gpupdate

Signed-off-by: David Mulder <dmulder at suse.com>
---
 docs-xml/smbdotconf/domain/gpoupdatecommand.xml    |  11 +-
 docs-xml/smbdotconf/winbind/applygrouppolicies.xml |  19 ++++
 lib/param/loadparm.c                               |   1 +
 source3/param/loadparm.c                           |   2 +
 source3/winbindd/winbindd.c                        |   2 +
 source3/winbindd/winbindd_gpupdate.c               | 116 +++++++++++++++++++++
 source3/winbindd/winbindd_proto.h                  |   3 +
 source3/winbindd/wscript_build                     |   3 +-
 8 files changed, 152 insertions(+), 5 deletions(-)
 create mode 100644 docs-xml/smbdotconf/winbind/applygrouppolicies.xml
 create mode 100644 source3/winbindd/winbindd_gpupdate.c

diff --git a/docs-xml/smbdotconf/domain/gpoupdatecommand.xml b/docs-xml/smbdotconf/domain/gpoupdatecommand.xml
index 22a42163f27..77b596051b7 100644
--- a/docs-xml/smbdotconf/domain/gpoupdatecommand.xml
+++ b/docs-xml/smbdotconf/domain/gpoupdatecommand.xml
@@ -5,10 +5,13 @@
                  xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
 <description>
 	<para>This option sets the command that is called to apply GPO policies.
-        The samba_gpoupdate script applies System Access and Kerberos Policies.
-        System Access policies set minPwdAge, maxPwdAge, minPwdLength, and
-        pwdProperties in the samdb. Kerberos Policies set kdc:service ticket lifetime,
-        kdc:user ticket lifetime, and kdc:renewal lifetime in smb.conf.
+        The samba_gpoupdate script applies System Access and Kerberos Policies
+	to the KDC, or Environment Variable policies to client machines. System
+	Access policies set minPwdAge, maxPwdAge, minPwdLength, and
+	pwdProperties in the samdb. Kerberos Policies set kdc:service ticket
+	lifetime, kdc:user ticket lifetime, and kdc:renewal lifetime in
+	smb.conf. Environment Variable policies apply environment variables,
+	such as PATH, to /etc/profile.
 	</para>
 </description>
 
diff --git a/docs-xml/smbdotconf/winbind/applygrouppolicies.xml b/docs-xml/smbdotconf/winbind/applygrouppolicies.xml
new file mode 100644
index 00000000000..67baa0d1426
--- /dev/null
+++ b/docs-xml/smbdotconf/winbind/applygrouppolicies.xml
@@ -0,0 +1,19 @@
+<samba:parameter name="apply group policies"
+                 context="G"
+                 type="boolean"
+                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
+<description>
+
+	<para>This option controls whether winbind will execute the gpupdate
+	command defined in <smbconfoption name="gpo update command"/> on the
+	Group Policy update interval. The Group	Policy update interval is
+	defined as every 90 minutes, plus a random offset between 0 and	30
+	minutes. This applies Group Policy Machine polices to the client or
+	KDC and machine policies to a server.
+	</para>
+
+</description>
+
+<value type="default">no</value>
+<value type="example">yes</value>
+</samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index d788ffbe36f..73a6eac293d 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2731,6 +2731,7 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
 	lpcfg_do_global_parameter(lp_ctx, "winbindd socket directory", dyn_WINBINDD_SOCKET_DIR);
 	lpcfg_do_global_parameter(lp_ctx, "ntp signd socket directory", dyn_NTP_SIGND_SOCKET_DIR);
 	lpcfg_do_global_parameter_var(lp_ctx, "gpo update command", "%s/samba_gpoupdate", dyn_SCRIPTSBINDIR);
+	lpcfg_do_global_parameter_var(lp_ctx, "apply group policies", "False");
 	lpcfg_do_global_parameter_var(lp_ctx, "dns update command", "%s/samba_dnsupdate", dyn_SCRIPTSBINDIR);
 	lpcfg_do_global_parameter_var(lp_ctx, "spn update command", "%s/samba_spnupdate", dyn_SCRIPTSBINDIR);
 	lpcfg_do_global_parameter_var(lp_ctx, "samba kcc command",
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 01c022e2889..11bf9993c19 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -926,6 +926,8 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
 	Globals.gpo_update_command = str_list_make_v3_const(NULL, s, NULL);
 	TALLOC_FREE(s);
 
+	Globals.apply_group_policies = false;
+
 	s = talloc_asprintf(talloc_tos(), "%s/samba_spnupdate", get_dyn_SCRIPTSBINDIR());
 	if (s == NULL) {
 		smb_panic("init_globals: ENOMEM");
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index ceb131e9b32..2e0a45c82b5 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -1786,6 +1786,8 @@ int main(int argc, const char **argv)
 		daemon_ready("winbindd");
 	}
 
+	gpupdate_init();
+
 	/* Loop waiting for requests */
 	while (1) {
 		frame = talloc_stackframe();
diff --git a/source3/winbindd/winbindd_gpupdate.c b/source3/winbindd/winbindd_gpupdate.c
new file mode 100644
index 00000000000..48ebb5501aa
--- /dev/null
+++ b/source3/winbindd/winbindd_gpupdate.c
@@ -0,0 +1,116 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Group Policy Update event for winbindd
+ * Copyright (C) David Mulder 2017
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "includes.h"
+#include "param/param.h"
+#include "param/loadparm.h"
+#include "winbindd.h"
+
+/*
+ * gpupdate_interval()
+ * return   Random integer between 5400 and 7200, the group policy update
+ *          interval in seconds
+ *
+ * Group Policy should be updated every 90 minutes in the background,
+ * with a random offset between 0 and 30 minutes. This ensures mutiple
+ * clients will not update at the same time.
+ */
+#define GPUPDATE_INTERVAL       (90*60)
+#define GPUPDATE_RAND_OFFSET    (30*60)
+static uint32_t gpupdate_interval(void)
+{
+	int rand_int_offset = rand() % GPUPDATE_RAND_OFFSET;
+	return GPUPDATE_INTERVAL+rand_int_offset;
+}
+
+struct gpupdate_state {
+	TALLOC_CTX *ctx;
+	struct loadparm_context *lp_ctx;
+};
+
+static void gpupdate_callback(struct tevent_context *ev,
+			      struct tevent_timer *tim,
+			      struct timeval current_time,
+			      void *private_data)
+{
+	struct tevent_timer *time_event;
+	struct timeval schedule;
+	struct tevent_req *req = NULL;
+	struct gpupdate_state *data =
+		talloc_get_type_abort(private_data, struct gpupdate_state);
+	const char *const *gpupdate_cmd =
+		lpcfg_gpo_update_command(data->lp_ctx);
+	const char *smbconf = lp_default_path();
+
+	/* Execute gpupdate */
+	req = samba_runcmd_send(data->ctx, ev, timeval_zero(), 2, 0,
+				gpupdate_cmd,
+				"-s",
+				smbconf,
+				"--machine",
+				"--machine-pass",
+				NULL);
+	if (req == NULL) {
+		DEBUG(0, ("Failed to execute the gpupdate command\n"));
+		return;
+	}
+
+	/* Schedule the next event */
+	schedule = tevent_timeval_current_ofs(gpupdate_interval(), 0);
+	time_event = tevent_add_timer(ev, data->ctx, schedule,
+				      gpupdate_callback, data);
+	if (time_event == NULL) {
+		DEBUG(0, ("Failed scheduling the next gpupdate event\n"));
+	}
+}
+
+void gpupdate_init(void)
+{
+	struct tevent_timer *time_event;
+	struct timeval schedule;
+	TALLOC_CTX * ctx = talloc_new(server_event_context());
+	struct gpupdate_state *data = talloc(ctx, struct gpupdate_state);
+	struct loadparm_context *lp_ctx =
+		loadparm_init_s3(NULL, loadparm_s3_helpers());
+
+	/*
+	 * Check if gpupdate is enabled for winbind, if not
+	 * return without scheduling any events.
+	 */
+	if (!lpcfg_apply_group_policies(lp_ctx)) {
+		return;
+	}
+
+	/*
+	 * Execute the first event immediately, future events
+	 * will execute on the gpupdate interval, which is every
+	 * 90 to 120 minutes (at random).
+	 */
+	schedule = tevent_timeval_current_ofs(0, 0);
+	data->ctx = ctx;
+	data->lp_ctx = lp_ctx;
+	if (data->lp_ctx == NULL) {
+		smb_panic("Could not load smb.conf\n");
+	}
+	time_event = tevent_add_timer(server_event_context(), data->ctx,
+				      schedule, gpupdate_callback, data);
+	if (time_event == NULL) {
+		DEBUG(0, ("Failed scheduling the gpupdate event\n"));
+	}
+}
+
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index cf01337aaad..0e59a51ba54 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -932,4 +932,7 @@ NTSTATUS wb_irpc_register(void);
 /* The following definitions come from winbindd/winbindd_reconnect.c  */
 bool reconnect_need_retry(NTSTATUS status, struct winbindd_domain *domain);
 
+/* The following definitions come from winbindd/winbindd_gpupdate.c  */
+void gpupdate_init(void);
+
 #endif /*  _WINBINDD_PROTO_H_  */
diff --git a/source3/winbindd/wscript_build b/source3/winbindd/wscript_build
index 51264e9e365..48250ea565e 100644
--- a/source3/winbindd/wscript_build
+++ b/source3/winbindd/wscript_build
@@ -254,7 +254,8 @@ bld.SAMBA3_BINARY('winbindd',
                  winbindd_pam_logoff.c
                  winbindd_pam_chauthtok.c
                  winbindd_pam_auth_crap.c
-                 winbindd_pam_chng_pswd_auth_crap.c''',
+                 winbindd_pam_chng_pswd_auth_crap.c
+                 winbindd_gpupdate.c''',
                  deps='''
                  talloc
                  tevent
-- 
2.13.6


From cf26aafdbd62c82bb5b2ce16b2da2a94470a0109 Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Wed, 6 Dec 2017 14:29:23 -0700
Subject: [PATCH 4/6] gpo: test that machine policies are working

Signed-off-by: David Mulder <dmulder at suse.com>
---
 selftest/target/Samba4.pm   |   2 +-
 source4/torture/gpo/apply.c | 239 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 194 insertions(+), 47 deletions(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 68feb6b4673..45ace7dc5f9 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -616,7 +616,7 @@ sub provision_raw_step1($$)
 	rndc command = true
 	dns update command = $ctx->{samba_dnsupdate}
 	spn update command = $ENV{SRCDIR_ABS}/source4/scripting/bin/samba_spnupdate -s $ctx->{smb_conf}
-	gpo update command = $ENV{SRCDIR_ABS}/source4/scripting/bin/samba_gpoupdate -s $ctx->{smb_conf} -H $ctx->{privatedir}/sam.ldb
+	gpo update command = $ENV{SRCDIR_ABS}/source4/scripting/bin/samba_gpoupdate -s $ctx->{smb_conf} -H $ctx->{privatedir}/sam.ldb --sysconfdir=$ctx->{etcdir} --machine
 	dreplsrv:periodic_startup_interval = 0
 	dsdb:schema update allowed = yes
 
diff --git a/source4/torture/gpo/apply.c b/source4/torture/gpo/apply.c
index 88da0b1e5fc..6a5de3ba39c 100644
--- a/source4/torture/gpo/apply.c
+++ b/source4/torture/gpo/apply.c
@@ -27,13 +27,17 @@
 #include "lib/ldb/include/ldb.h"
 #include "torture/gpo/proto.h"
 #include <unistd.h>
+#include <fcntl.h>
 
 struct torture_suite *gpo_apply_suite(TALLOC_CTX *ctx)
 {
 	struct torture_suite *suite = torture_suite_create(ctx, "apply");
 
-	torture_suite_add_simple_test(suite, "gpo_param_from_gpo",
+	torture_suite_add_simple_test(suite, "gpo_system_access_policies",
 				      torture_gpo_system_access_policies);
+	torture_suite_add_simple_test(suite,
+				      "gpo_environment_variables_policies",
+				   torture_gpo_environment_variables_policies);
 
 	suite->description = talloc_strdup(suite, "Group Policy apply tests");
 
@@ -78,10 +82,109 @@ PasswordComplexity = %d\n\
 #define GPTINI "addom.samba.example.com/Policies/"\
 	       "{31B2F340-016D-11D2-945F-00C04FB984F9}/GPT.INI"
 
+#define ENVPATH "addom.samba.example.com/Policies/"\
+		"{31B2F340-016D-11D2-945F-00C04FB984F9}/MACHINE/"\
+		"Preferences/EnvironmentVariables"
+#define ENVXML "EnvironmentVariables.xml"
+#define ENVTMPL "\
+<?xml version=\"1.0\" encoding=\"utf-8\"?>\
+<EnvironmentVariables clsid=\"{BF141A63-327B-438a-B9BF-2C188F13B7AD}\">\
+<EnvironmentVariable clsid=\"{78570023-8373-4a19-BA80-2F150738EA19}\"\
+	name=\"PATH\" status=\"PATH = %s\" image=\"2\"\
+	uid=\"{B048DE7E-9B24-497C-B798-17708F7B33C5}\">\
+<Properties action=\"%s\" name=\"PATH\" value=\"%s\" user=\"0\"\
+	partial=\"%s\"/>\
+</EnvironmentVariable>\
+</EnvironmentVariables>\
+"
+#define RESENV "#\n\
+# Samba GPO Section\n\
+# These settings are applied via GPO\n\
+#\n\
+PATH=%s\n\
+#\n\
+# End Samba GPO Section\n\
+#"
+
+static void increment_gpt_ini(TALLOC_CTX *ctx, const char *gpt_file)
+{
+	FILE *fp = NULL;
+	int vers = 0;
+
+	/* Update the version in the GPT.INI */
+	if ( (fp = fopen(gpt_file, "r")) ) {
+		char line[256];
+		while (fgets(line, 256, fp)) {
+			if (strncasecmp(line, "Version=", 8) == 0) {
+				vers = atoi(line+8);
+				break;
+			}
+		}
+		fclose(fp);
+	}
+	if ( (fp = fopen(gpt_file, "w")) ) {
+		char *data = talloc_asprintf(ctx,
+					     "[General]\nVersion=%d\n",
+					     ++vers);
+		fputs(data, fp);
+		fclose(fp);
+	}
+}
+
+static bool exec_gpo_update_command(struct torture_context *tctx)
+{
+	int ret = 0;
+	const char **gpo_update_cmd;
+
+	/* Get the gpo update command */
+	gpo_update_cmd = lpcfg_gpo_update_command(tctx->lp_ctx);
+	torture_assert(tctx, gpo_update_cmd && gpo_update_cmd[0],
+		       "Failed to fetch the gpo update command");
+
+	/* Run the gpo update command */
+	ret = exec_wait(discard_const_p(char *, gpo_update_cmd));
+	torture_assert(tctx, ret == 0,
+		       "Failed to execute the gpo update command");
+
+	return true;
+}
+
+static bool exec_gpo_unapply_command(struct torture_context *tctx)
+{
+	TALLOC_CTX *ctx = talloc_new(tctx);
+	char **gpo_unapply_cmd;
+	const char **gpo_update_cmd;
+	int gpo_update_len = 0;
+	const char **itr;
+	int ret = 0, i;
+
+	/* Get the gpo update command */
+	gpo_update_cmd = lpcfg_gpo_update_command(tctx->lp_ctx);
+	torture_assert(tctx, gpo_update_cmd && gpo_update_cmd[0],
+		       "Failed to fetch the gpo update command");
+
+	for (itr = gpo_update_cmd; *itr != NULL; itr++) {
+		gpo_update_len++;
+	}
+	gpo_unapply_cmd = talloc_array(ctx, char*, gpo_update_len+2);
+	for (i = 0; i < gpo_update_len; i++) {
+		gpo_unapply_cmd[i] = talloc_strdup(gpo_unapply_cmd,
+						   gpo_update_cmd[i]);
+	}
+	gpo_unapply_cmd[i] = talloc_asprintf(gpo_unapply_cmd, "--unapply");
+	gpo_unapply_cmd[i+1] = NULL;
+	ret = exec_wait(gpo_unapply_cmd);
+	torture_assert(tctx, ret == 0,
+		       "Failed to execute the gpo unapply command");
+
+	talloc_free(ctx);
+	return true;
+}
+
 bool torture_gpo_system_access_policies(struct torture_context *tctx)
 {
 	TALLOC_CTX *ctx = talloc_new(tctx);
-	int ret, vers = 0, i;
+	int ret, i;
 	const char *sysvol_path = NULL, *gpo_dir = NULL;
 	const char *gpo_file = NULL, *gpt_file = NULL;
 	struct ldb_context *samdb = NULL;
@@ -94,15 +197,11 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
 		NULL
 	};
 	FILE *fp = NULL;
-	const char **gpo_update_cmd;
-	char **gpo_unapply_cmd;
 	int minpwdcases[] = { 0, 1, 998 };
 	int maxpwdcases[] = { 0, 1, 999 };
 	int pwdlencases[] = { 0, 1, 14 };
 	int pwdpropcases[] = { 0, 1, 1 };
 	struct ldb_message *old_message = NULL;
-	const char **itr;
-	int gpo_update_len = 0;
 
 	sysvol_path = lpcfg_path(lpcfg_service(tctx->lp_ctx, "sysvol"),
 				 lpcfg_default_service(tctx->lp_ctx), tctx);
@@ -113,11 +212,6 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
 	mkdir_p(gpo_dir, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
 	gpo_file = talloc_asprintf(ctx, "%s/%s", gpo_dir, GPOFILE);
 
-	/* Get the gpo update command */
-	gpo_update_cmd = lpcfg_gpo_update_command(tctx->lp_ctx);
-	torture_assert(tctx, gpo_update_cmd && gpo_update_cmd[0],
-		       "Failed to fetch the gpo update command");
-
 	/* Open and read the samba db and store the initial password settings */
 	samdb = samdb_connect(ctx, tctx->ev, tctx->lp_ctx,
 			      system_session(tctx->lp_ctx), 0);
@@ -139,30 +233,10 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
 			fclose(fp);
 		}
 
-		/* Update the version in the GPT.INI */
 		gpt_file = talloc_asprintf(ctx, "%s/%s", sysvol_path, GPTINI);
-		if ( (fp = fopen(gpt_file, "r")) ) {
-			char line[256];
-			while (fgets(line, 256, fp)) {
-				if (strncasecmp(line, "Version=", 8) == 0) {
-					vers = atoi(line+8);
-					break;
-				}
-			}
-			fclose(fp);
-		}
-		if ( (fp = fopen(gpt_file, "w")) ) {
-			char *data = talloc_asprintf(ctx,
-						     "[General]\nVersion=%d\n",
-						     ++vers);
-			fputs(data, fp);
-			fclose(fp);
-		}
+		increment_gpt_ini(ctx, gpt_file);
 
-		/* Run the gpo update command */
-		ret = exec_wait(discard_const_p(char *, gpo_update_cmd));
-		torture_assert(tctx, ret == 0,
-			       "Failed to execute the gpo update command");
+		exec_gpo_update_command(tctx);
 
 		ret = ldb_search(samdb, ctx, &result,
 				 ldb_get_default_basedn(samdb),
@@ -204,19 +278,8 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
 	}
 
 	/* Unapply the settings and verify they are removed */
-	for (itr = gpo_update_cmd; *itr != NULL; itr++) {
-		gpo_update_len++;
-	}
-	gpo_unapply_cmd = talloc_array(ctx, char*, gpo_update_len+2);
-	for (i = 0; i < gpo_update_len; i++) {
-		gpo_unapply_cmd[i] = talloc_strdup(gpo_unapply_cmd,
-						   gpo_update_cmd[i]);
-	}
-	gpo_unapply_cmd[i] = talloc_asprintf(gpo_unapply_cmd, "--unapply");
-	gpo_unapply_cmd[i+1] = NULL;
-	ret = exec_wait(gpo_unapply_cmd);
-	torture_assert(tctx, ret == 0,
-		       "Failed to execute the gpo unapply command");
+	exec_gpo_unapply_command(tctx);
+
 	ret = ldb_search(samdb, ctx, &result, ldb_get_default_basedn(samdb),
 			 LDB_SCOPE_BASE, attrs, NULL);
 	torture_assert(tctx, ret == LDB_SUCCESS && result->count == 1,
@@ -265,3 +328,87 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
 	talloc_free(ctx);
 	return true;
 }
+
+bool torture_gpo_environment_variables_policies(struct torture_context *tctx)
+{
+	TALLOC_CTX *ctx = talloc_new(tctx);
+	const char **gpo_update_cmd;
+	const char *env_dir = NULL, *env_xml = NULL, *smbconf = NULL;
+	const char *sysvol_path = NULL, *gpt_file = NULL;
+	char *profile = NULL;
+	const char *envcases[][4] = {
+		{ "", "D", "0" },
+		{ "/bin", "U", "0" },
+		{ "/bin", "U", "1" },
+	};
+	const char *envres[] = {
+		"",
+		"/bin",
+		"$PATH:/bin",
+		NULL,
+	};
+	char *envexpected = NULL, *envreturned = NULL;
+	size_t envlen = 0;
+	int fd, i;
+	FILE *fp = NULL;
+
+	/* Ensure the sysvol path exists */
+	sysvol_path = lpcfg_path(lpcfg_service(tctx->lp_ctx, "sysvol"),
+				 lpcfg_default_service(tctx->lp_ctx), tctx);
+	torture_assert(tctx, sysvol_path, "Failed to fetch the sysvol path");
+	env_dir = talloc_asprintf(ctx, "%s/%s", sysvol_path, ENVPATH);
+	mkdir_p(env_dir, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
+	env_xml = talloc_asprintf(ctx, "%s/%s", env_dir, ENVXML);
+
+	smbconf = lpcfg_configfile(tctx->lp_ctx);
+	profile = talloc_strndup(ctx, smbconf, strlen(smbconf)-8);
+	profile = talloc_strdup_append(profile, "profile");
+
+	/* Get the gpo update command */
+	gpo_update_cmd = lpcfg_gpo_update_command(tctx->lp_ctx);
+	torture_assert(tctx, gpo_update_cmd && gpo_update_cmd[0],
+		       "Failed to fetch the gpo update command");
+
+	for (i = 0; i < 3; i++) {
+		if ( (fp = fopen(env_xml, "w")) ) {
+			fputs(talloc_asprintf(ctx, ENVTMPL, envcases[i][0],
+					      envcases[i][1], envcases[i][0],
+					      envcases[i][2]), fp);
+			fclose(fp);
+		}
+		gpt_file = talloc_asprintf(ctx, "%s/%s", sysvol_path, GPTINI);
+		increment_gpt_ini(ctx, gpt_file);
+
+		exec_gpo_update_command(tctx);
+
+		/* Machine env var policy */
+		envexpected = talloc_asprintf(ctx, RESENV, envres[i]);
+		fd = open(profile, O_RDONLY);
+		envlen = strlen(envexpected);
+		torture_assert(tctx, lseek(fd, -envlen,
+					   SEEK_END) != -1,
+			      "Failed to seek to start position in profile");
+		envreturned = talloc_array(ctx, char, envlen);
+		torture_assert(tctx, read(fd, envreturned, envlen) == envlen,
+				"Failed to read from profile");
+		torture_assert(tctx, strncmp(envexpected, envreturned, envlen)\
+				     == 0,
+				"Environment variable policy was not applied");
+	}
+
+	/* Unapply the settings and verify they are removed */
+	exec_gpo_unapply_command(tctx);
+
+	/* Machine env var policy */
+	fd = open(profile, O_RDONLY);
+	if (lseek(fd, -envlen, SEEK_END) != -1) {
+		if (read(fd, envreturned, envlen) == envlen) {
+			torture_assert(tctx, strncmp(envexpected, envreturned,
+						     envlen) != 0,
+			    "Environment variable policy was not unapplied");
+		}
+	}
+
+	talloc_free(ctx);
+	return true;
+}
-- 
2.13.6


From 803b012e92be0002e94ea4707083310d1fce43dd Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Fri, 8 Dec 2017 14:12:02 -0700
Subject: [PATCH 5/6] gpo: enable group policy apply from winbind as default

Signed-off-by: David Mulder <dmulder at suse.com>
---
 docs-xml/smbdotconf/winbind/applygrouppolicies.xml | 4 ++--
 lib/param/loadparm.c                               | 2 +-
 source3/param/loadparm.c                           | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs-xml/smbdotconf/winbind/applygrouppolicies.xml b/docs-xml/smbdotconf/winbind/applygrouppolicies.xml
index 67baa0d1426..750d2d74ae1 100644
--- a/docs-xml/smbdotconf/winbind/applygrouppolicies.xml
+++ b/docs-xml/smbdotconf/winbind/applygrouppolicies.xml
@@ -14,6 +14,6 @@
 
 </description>
 
-<value type="default">no</value>
-<value type="example">yes</value>
+<value type="default">yes</value>
+<value type="example">no</value>
 </samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 73a6eac293d..611b6a33905 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2731,7 +2731,7 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
 	lpcfg_do_global_parameter(lp_ctx, "winbindd socket directory", dyn_WINBINDD_SOCKET_DIR);
 	lpcfg_do_global_parameter(lp_ctx, "ntp signd socket directory", dyn_NTP_SIGND_SOCKET_DIR);
 	lpcfg_do_global_parameter_var(lp_ctx, "gpo update command", "%s/samba_gpoupdate", dyn_SCRIPTSBINDIR);
-	lpcfg_do_global_parameter_var(lp_ctx, "apply group policies", "False");
+	lpcfg_do_global_parameter_var(lp_ctx, "apply group policies", "True");
 	lpcfg_do_global_parameter_var(lp_ctx, "dns update command", "%s/samba_dnsupdate", dyn_SCRIPTSBINDIR);
 	lpcfg_do_global_parameter_var(lp_ctx, "spn update command", "%s/samba_spnupdate", dyn_SCRIPTSBINDIR);
 	lpcfg_do_global_parameter_var(lp_ctx, "samba kcc command",
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 11bf9993c19..20a8e364b59 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -926,7 +926,7 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
 	Globals.gpo_update_command = str_list_make_v3_const(NULL, s, NULL);
 	TALLOC_FREE(s);
 
-	Globals.apply_group_policies = false;
+	Globals.apply_group_policies = true;
 
 	s = talloc_asprintf(talloc_tos(), "%s/samba_spnupdate", get_dyn_SCRIPTSBINDIR());
 	if (s == NULL) {
-- 
2.13.6


From 8a57430543d45ca05fa4e32ed6c5954acc36248e Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Tue, 12 Dec 2017 10:32:41 -0700
Subject: [PATCH 6/6] gpo: Don't apply MS PATH environment variables

Signed-off-by: David Mulder <dmulder at suse.com>
---
 python/samba/gp_env_var_ext.py | 13 +++++++++
 source4/torture/gpo/apply.c    | 63 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/python/samba/gp_env_var_ext.py b/python/samba/gp_env_var_ext.py
index ef60ae77c6e..7655d90e684 100644
--- a/python/samba/gp_env_var_ext.py
+++ b/python/samba/gp_env_var_ext.py
@@ -15,6 +15,19 @@ class xml_to_env(file_to):
         global sysconfdir
         if type(val) is not dict:
             val = { 'value': val, 'action': 'U' }
+
+        # MS PATH variable is seperated by ';', which if placed in a posix
+        # PATH causes everything after the ';' to be executed, which is not
+        # what we intend. So, ignore PATH variables that contain ';'. Also
+        # ensure the first character in the PATH is something valid (rather
+        # than C:\ or something).
+        if val['value'] and self.attribute == 'PATH' and \
+           (';' in val['value'] or \
+           val['value'][0] not in ['/', '$', '~', '.']):
+            self.logger.info('Ignored PATH variable assignment due to it\'s' +\
+                ' similarity to a Windows PATH: %s' % val['value'])
+            return
+
         profile = ini_file_append(os.path.join(sysconfdir, 'profile'))
 
         old_val = profile[self.attribute]
diff --git a/source4/torture/gpo/apply.c b/source4/torture/gpo/apply.c
index 6a5de3ba39c..30a04787969 100644
--- a/source4/torture/gpo/apply.c
+++ b/source4/torture/gpo/apply.c
@@ -38,6 +38,8 @@ struct torture_suite *gpo_apply_suite(TALLOC_CTX *ctx)
 	torture_suite_add_simple_test(suite,
 				      "gpo_environment_variables_policies",
 				   torture_gpo_environment_variables_policies);
+	torture_suite_add_simple_test(suite, "gpo_bad_env_var",
+				      torture_gpo_bad_env_var);
 
 	suite->description = talloc_strdup(suite, "Group Policy apply tests");
 
@@ -412,3 +414,64 @@ bool torture_gpo_environment_variables_policies(struct torture_context *tctx)
 	talloc_free(ctx);
 	return true;
 }
+
+bool torture_gpo_bad_env_var(struct torture_context *tctx)
+{
+	TALLOC_CTX *ctx = talloc_new(tctx);
+	const char **gpo_update_cmd;
+	const char *env_dir = NULL, *env_xml = NULL, *smbconf = NULL;
+	const char *sysvol_path = NULL, *gpt_file = NULL;
+	char *profile = NULL;
+	const char *envcases[][4] = {
+		{ "C:\\WINDOWS\\system32", "U", "0" },
+		{ "\%USERPROFILE\%\\bin", "U", "0" },
+		{ "/foo;/bar", "U", "0" },
+	};
+	int i;
+	FILE *fp = NULL;
+	struct stat *finfo = talloc_zero(ctx, struct stat);
+
+	/* Ensure the sysvol path exists */
+	sysvol_path = lpcfg_path(lpcfg_service(tctx->lp_ctx, "sysvol"),
+				 lpcfg_default_service(tctx->lp_ctx), tctx);
+	torture_assert(tctx, sysvol_path, "Failed to fetch the sysvol path");
+	env_dir = talloc_asprintf(ctx, "%s/%s", sysvol_path, ENVPATH);
+	mkdir_p(env_dir, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
+	env_xml = talloc_asprintf(ctx, "%s/%s", env_dir, ENVXML);
+
+	smbconf = lpcfg_configfile(tctx->lp_ctx);
+	profile = talloc_strndup(ctx, smbconf, strlen(smbconf)-8);
+	profile = talloc_strdup_append(profile, "profile");
+
+	/* Get the gpo update command */
+	gpo_update_cmd = lpcfg_gpo_update_command(tctx->lp_ctx);
+	torture_assert(tctx, gpo_update_cmd && gpo_update_cmd[0],
+		       "Failed to fetch the gpo update command");
+
+	for (i = 0; i < 3; i++) {
+		if ( (fp = fopen(env_xml, "w")) ) {
+			fputs(talloc_asprintf(ctx, ENVTMPL, envcases[i][0],
+					      envcases[i][1], envcases[i][0],
+					      envcases[i][2]), fp);
+			fclose(fp);
+		}
+		gpt_file = talloc_asprintf(ctx, "%s/%s", sysvol_path, GPTINI);
+		increment_gpt_ini(ctx, gpt_file);
+
+		exec_gpo_update_command(tctx);
+
+		/* Make sure the profile is either not there, or empty */
+		if (access(profile, F_OK) == 0) {
+			torture_assert(tctx, stat(profile, finfo) == 0,
+				       "Failed to stat the profile");
+			torture_assert_int_equal(tctx, finfo->st_size, 0,
+					"The profile was not zero bytes");
+		}
+	}
+
+	/* Unapply the settings */
+	exec_gpo_unapply_command(tctx);
+
+	talloc_free(ctx);
+	return true;
+}
-- 
2.13.6



More information about the samba-technical mailing list