[PATCHES] Make gpo extensible

David Mulder dmulder at suse.com
Fri Jan 19 17:03:45 UTC 2018


This adds the ability to add 3rd party kdc and machine policy extensions
(user policy to follow/nearly finished). Extensions are loaded
dynamically as long as they are installed in the correct sub-directory
of /var/lib/samba/gp_exts/. Examples and instructions are included.
This also includes the patches for adding the gpo disable feature (which
now makes a little more sense, because extensions are located in
/var/lib/samba/gp_exts/ instead of python lib directories).
Another patch (read gpo versions from cache) is included, which for some
reason didn't make it into 4.8 (I'm not sure why this one patch got
missed from the series I submitted).

Making gpo extensible I think is a good compromise, since now vendors
can add their own extensions, without the extensions needing to be
committed to the samba tree. For example, a extension for distributing
RPMs would be useful for certain distros, but doesn't necessarily make
sense committed to samba.

 buildtools/wafsamba/wafsamba.py       |   8 +-
 python/gp_exts/__init__.py            |  37 +++++++++
 python/gp_exts/gp_example_ex.py       | 109 +++++++++++++++++++++++++
 python/gp_exts/kdc/__init__.py        |   4 +
 python/gp_exts/kdc/gp_sec_ext.py      | 140
++++++++++++++++++++++++++++++++
 python/gp_exts/machine/__init__.py    |   4 +
 python/samba/gpclass.py               | 209
+++++++++++------------------------------------
 python/wscript                        |   6 ++
 source4/scripting/bin/samba_gpoupdate |  21 ++++-
 source4/torture/gpo/apply.c           | 258
++++++++++++++++++++++++++++++++++++++++++++++++-----------
 10 files changed, 584 insertions(+), 212 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 55349deb2739742c6b9d26e877c907d629bb08a1 Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Mon, 8 Jan 2018 07:17:29 -0700
Subject: [PATCH 1/3] gpo: Read GPO versions from a cache, not sysvol

Signed-off-by: David Mulder <dmulder at suse.com>
---
 source4/scripting/bin/samba_gpoupdate | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/source4/scripting/bin/samba_gpoupdate b/source4/scripting/bin/samba_gpoupdate
index 26e0984413e..d6c0ecc5db5 100755
--- a/source4/scripting/bin/samba_gpoupdate
+++ b/source4/scripting/bin/samba_gpoupdate
@@ -57,6 +57,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)
@@ -72,8 +81,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)
-- 
2.13.6


From f0ebe032e18a20a7e3ad885da1f2b3dfb02539fd Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Mon, 8 Jan 2018 07:39:49 -0700
Subject: [PATCH 2/3] gpo: provide a means for disabling gpo exts

Signed-off-by: David Mulder <dmulder at suse.com>
---
 python/samba/gp_sec_ext.py            | 140 ++++++++++++++++++
 python/samba/gpclass.py               | 209 +++++++--------------------
 source4/scripting/bin/samba_gpoupdate |   4 +-
 source4/torture/gpo/apply.c           | 258 ++++++++++++++++++++++++++++------
 4 files changed, 402 insertions(+), 209 deletions(-)
 create mode 100644 python/samba/gp_sec_ext.py

diff --git a/python/samba/gp_sec_ext.py b/python/samba/gp_sec_ext.py
new file mode 100644
index 00000000000..b32793e1043
--- /dev/null
+++ b/python/samba/gp_sec_ext.py
@@ -0,0 +1,140 @@
+import os.path
+from gpclass import file_to, gp_inf_ext
+
+class inf_to_kdc_tdb(file_to):
+    def mins_to_hours(self):
+        return '%d' % (int(self.val)/60)
+
+    def days_to_hours(self):
+        return '%d' % (int(self.val)*24)
+
+    def set_kdc_tdb(self, val):
+        old_val = self.gp_db.gpostore.get(self.attribute)
+        self.logger.info('%s was changed from %s to %s' % (self.attribute,
+                                                           old_val, val))
+        if val is not None:
+            self.gp_db.gpostore.store(self.attribute, val)
+            self.gp_db.store(str(self), self.attribute, old_val)
+        else:
+            self.gp_db.gpostore.delete(self.attribute)
+            self.gp_db.delete(str(self), self.attribute)
+
+    def mapper(self):
+        return { 'kdc:user_ticket_lifetime': (self.set_kdc_tdb, self.explicit),
+                 'kdc:service_ticket_lifetime': (self.set_kdc_tdb,
+                                                 self.mins_to_hours),
+                 'kdc:renewal_lifetime': (self.set_kdc_tdb,
+                                          self.days_to_hours),
+               }
+
+    def __str__(self):
+        return 'Kerberos Policy'
+
+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.
+    '''
+
+    def ch_minPwdAge(self, val):
+        old_val = self.ldb.get_minPwdAge()
+        self.logger.info('KDC Minimum Password age was changed from %s to %s' \
+                         % (old_val, val))
+        self.gp_db.store(str(self), self.attribute, old_val)
+        self.ldb.set_minPwdAge(val)
+
+    def ch_maxPwdAge(self, val):
+        old_val = self.ldb.get_maxPwdAge()
+        self.logger.info('KDC Maximum Password age was changed from %s to %s' \
+                         % (old_val, val))
+        self.gp_db.store(str(self), self.attribute, old_val)
+        self.ldb.set_maxPwdAge(val)
+
+    def ch_minPwdLength(self, val):
+        old_val = self.ldb.get_minPwdLength()
+        self.logger.info(
+            'KDC Minimum Password length was changed from %s to %s' \
+             % (old_val, val))
+        self.gp_db.store(str(self), self.attribute, old_val)
+        self.ldb.set_minPwdLength(val)
+
+    def ch_pwdProperties(self, val):
+        old_val = self.ldb.get_pwdProperties()
+        self.logger.info('KDC Password Properties were changed from %s to %s' \
+                         % (old_val, val))
+        self.gp_db.store(str(self), self.attribute, old_val)
+        self.ldb.set_pwdProperties(val)
+
+    def days2rel_nttime(self):
+        seconds = 60
+        minutes = 60
+        hours = 24
+        sam_add = 10000000
+        val = (self.val)
+        val = int(val)
+        return  str(-(val * seconds * minutes * hours * sam_add))
+
+    def mapper(self):
+        '''ldap value : samba setter'''
+        return { "minPwdAge" : (self.ch_minPwdAge, self.days2rel_nttime),
+                 "maxPwdAge" : (self.ch_maxPwdAge, self.days2rel_nttime),
+                 # Could be none, but I like the method assignment in
+                 # update_samba
+                 "minPwdLength" : (self.ch_minPwdLength, self.explicit),
+                 "pwdProperties" : (self.ch_pwdProperties, self.explicit),
+
+               }
+
+    def __str__(self):
+        return 'System Access'
+
+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.
+    '''
+
+    count = 0
+
+    def __str__(self):
+        return "Security GPO extension"
+
+    def list(self, rootpath):
+        return os.path.join(rootpath,
+            "MACHINE/Microsoft/Windows NT/SecEdit/GptTmpl.inf")
+
+    def listmachpol(self, rootpath):
+        return os.path.join(rootpath, "Machine/Registry.pol")
+
+    def listuserpol(self, rootpath):
+        return os.path.join(rootpath, "User/Registry.pol")
+
+    def apply_map(self):
+        return {"System Access": {"MinimumPasswordAge": ("minPwdAge",
+                                                         inf_to_ldb),
+                                  "MaximumPasswordAge": ("maxPwdAge",
+                                                         inf_to_ldb),
+                                  "MinimumPasswordLength": ("minPwdLength",
+                                                            inf_to_ldb),
+                                  "PasswordComplexity": ("pwdProperties",
+                                                         inf_to_ldb),
+                                 },
+                "Kerberos Policy": {"MaxTicketAge": (
+                                        "kdc:user_ticket_lifetime",
+                                        inf_to_kdc_tdb
+                                    ),
+                                    "MaxServiceAge": (
+                                        "kdc:service_ticket_lifetime",
+                                        inf_to_kdc_tdb
+                                    ),
+                                    "MaxRenewAge": (
+                                        "kdc:renewal_lifetime",
+                                        inf_to_kdc_tdb
+                                    ),
+                                   }
+               }
+
+    @staticmethod
+    def disabled_file():
+        return os.path.splitext(os.path.abspath(__file__))[0] + '.py.disabled'
+
diff --git a/python/samba/gpclass.py b/python/samba/gpclass.py
index 33c9001cb6d..2ec00d5f69b 100644
--- a/python/samba/gpclass.py
+++ b/python/samba/gpclass.py
@@ -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,45 @@ class gp_ext(object):
         pass
 
     @abstractmethod
-    def parse(self, afile, ldb, conn, gp_db, lp):
+    def read(self, policy):
         pass
 
+    @classmethod
+    def enabled(cls):
+        return not os.path.exists(cls.disabled_file())
+
+    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 +351,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,148 +362,19 @@ class inf_to():
     def __str__(self):
         pass
 
-class inf_to_kdc_tdb(inf_to):
-    def mins_to_hours(self):
-        return '%d' % (int(self.val)/60)
-
-    def days_to_hours(self):
-        return '%d' % (int(self.val)*24)
-
-    def set_kdc_tdb(self, val):
-        old_val = self.gp_db.gpostore.get(self.attribute)
-        self.logger.info('%s was changed from %s to %s' % (self.attribute,
-                                                           old_val, val))
-        if val is not None:
-            self.gp_db.gpostore.store(self.attribute, val)
-            self.gp_db.store(str(self), self.attribute, old_val)
-        else:
-            self.gp_db.gpostore.delete(self.attribute)
-            self.gp_db.delete(str(self), self.attribute)
-
-    def mapper(self):
-        return { 'kdc:user_ticket_lifetime': (self.set_kdc_tdb, self.explicit),
-                 'kdc:service_ticket_lifetime': (self.set_kdc_tdb,
-                                                 self.mins_to_hours),
-                 'kdc:renewal_lifetime': (self.set_kdc_tdb,
-                                          self.days_to_hours),
-               }
-
-    def __str__(self):
-        return 'Kerberos Policy'
-
-class inf_to_ldb(inf_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.
-    '''
-
-    def ch_minPwdAge(self, val):
-        old_val = self.ldb.get_minPwdAge()
-        self.logger.info('KDC Minimum Password age was changed from %s to %s' \
-                         % (old_val, val))
-        self.gp_db.store(str(self), self.attribute, old_val)
-        self.ldb.set_minPwdAge(val)
-
-    def ch_maxPwdAge(self, val):
-        old_val = self.ldb.get_maxPwdAge()
-        self.logger.info('KDC Maximum Password age was changed from %s to %s' \
-                         % (old_val, val))
-        self.gp_db.store(str(self), self.attribute, old_val)
-        self.ldb.set_maxPwdAge(val)
-
-    def ch_minPwdLength(self, val):
-        old_val = self.ldb.get_minPwdLength()
-        self.logger.info(
-            'KDC Minimum Password length was changed from %s to %s' \
-             % (old_val, val))
-        self.gp_db.store(str(self), self.attribute, old_val)
-        self.ldb.set_minPwdLength(val)
-
-    def ch_pwdProperties(self, val):
-        old_val = self.ldb.get_pwdProperties()
-        self.logger.info('KDC Password Properties were changed from %s to %s' \
-                         % (old_val, val))
-        self.gp_db.store(str(self), self.attribute, old_val)
-        self.ldb.set_pwdProperties(val)
-
-    def days2rel_nttime(self):
-        seconds = 60
-        minutes = 60
-        hours = 24
-        sam_add = 10000000
-        val = (self.val)
-        val = int(val)
-        return  str(-(val * seconds * minutes * hours * sam_add))
-
-    def mapper(self):
-        '''ldap value : samba setter'''
-        return { "minPwdAge" : (self.ch_minPwdAge, self.days2rel_nttime),
-                 "maxPwdAge" : (self.ch_maxPwdAge, self.days2rel_nttime),
-                 # Could be none, but I like the method assignment in
-                 # update_samba
-                 "minPwdLength" : (self.ch_minPwdLength, self.explicit),
-                 "pwdProperties" : (self.ch_pwdProperties, self.explicit),
-
-               }
-
-    def __str__(self):
-        return 'System Access'
-
-
-class gp_sec_ext(gp_ext):
-    '''This class does the following two things:
-        1) Identifies the GPO if it has a certain kind of filepath,
-        2) Finally parses it.
-    '''
-
-    count = 0
-
-    def __init__(self, logger):
-        self.logger = logger
-
-    def __str__(self):
-        return "Security GPO extension"
-
+class gp_inf_ext(gp_ext):
+    @abstractmethod
     def list(self, rootpath):
-        return os.path.join(rootpath,
-                            "MACHINE/Microsoft/Windows NT/SecEdit/GptTmpl.inf")
-
-    def listmachpol(self, rootpath):
-        return os.path.join(rootpath, "Machine/Registry.pol")
-
-    def listuserpol(self, rootpath):
-        return os.path.join(rootpath, "User/Registry.pol")
+        pass
 
+    @abstractmethod
     def apply_map(self):
-        return {"System Access": {"MinimumPasswordAge": ("minPwdAge",
-                                                         inf_to_ldb),
-                                  "MaximumPasswordAge": ("maxPwdAge",
-                                                         inf_to_ldb),
-                                  "MinimumPasswordLength": ("minPwdLength",
-                                                            inf_to_ldb),
-                                  "PasswordComplexity": ("pwdProperties",
-                                                         inf_to_ldb),
-                                 },
-                "Kerberos Policy": {"MaxTicketAge": (
-                                        "kdc:user_ticket_lifetime",
-                                        inf_to_kdc_tdb
-                                    ),
-                                    "MaxServiceAge": (
-                                        "kdc:service_ticket_lifetime",
-                                        inf_to_kdc_tdb
-                                    ),
-                                    "MaxRenewAge": (
-                                        "kdc:renewal_lifetime",
-                                        inf_to_kdc_tdb
-                                    ),
-                                   }
-               }
-
-    def read_inf(self, path, conn):
+        pass
+
+    def read(self, policy):
         ret = False
         inftable = self.apply_map()
 
-        policy = conn.loadfile(path.replace('/', '\\'))
         current_section = None
 
         # So here we would declare a boolean,
@@ -499,27 +404,7 @@ class gp_sec_ext(gp_ext):
                     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
+    @abstractmethod
+    def __str__(self):
+        pass
 
diff --git a/source4/scripting/bin/samba_gpoupdate b/source4/scripting/bin/samba_gpoupdate
index d6c0ecc5db5..b8b2bb464a0 100755
--- a/source4/scripting/bin/samba_gpoupdate
+++ b/source4/scripting/bin/samba_gpoupdate
@@ -35,6 +35,7 @@ try:
 except:
     SamDB = None
 from samba.gpclass import *
+from samba.gp_sec_ext import gp_sec_ext
 from samba.net import Net
 from samba.dcerpc import nbt
 from samba import smb
@@ -170,7 +171,8 @@ if __name__ == "__main__":
     gp_extensions = []
     if opts.machine:
         if lp.get('server role') == 'active directory domain controller':
-            gp_extensions.append(gp_sec_ext(logger))
+            if gp_sec_ext.enabled():
+                gp_extensions.append(gp_sec_ext(logger))
     else:
         pass # User extensions
 
diff --git a/source4/torture/gpo/apply.c b/source4/torture/gpo/apply.c
index 88da0b1e5fc..dcfdd7a5dbe 100644
--- a/source4/torture/gpo/apply.c
+++ b/source4/torture/gpo/apply.c
@@ -27,13 +27,16 @@
 #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_disable_policies",
+				      torture_gpo_disable_policies);
 
 	suite->description = talloc_strdup(suite, "Group Policy apply tests");
 
@@ -78,10 +81,85 @@ PasswordComplexity = %d\n\
 #define GPTINI "addom.samba.example.com/Policies/"\
 	       "{31B2F340-016D-11D2-945F-00C04FB984F9}/GPT.INI"
 
+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 +172,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 +187,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 +208,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 +253,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 +303,131 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
 	talloc_free(ctx);
 	return true;
 }
+
+bool torture_gpo_disable_policies(struct torture_context *tctx)
+{
+	TALLOC_CTX *ctx = talloc_new(tctx);
+	int ret, i;
+	const char *sysvol_path = NULL, *gpo_dir = NULL;
+	const char *gpo_file = NULL, *gpt_file = NULL;
+	struct ldb_context *samdb = NULL;
+	struct ldb_result *result;
+	const char *attrs[] = {
+		"minPwdAge",
+		"maxPwdAge",
+		"minPwdLength",
+		"pwdProperties",
+		NULL
+	};
+	FILE *fp = NULL;
+	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 *disable_file = "bin/python/samba/gp_sec_ext.py.disabled";
+
+	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");
+
+	/* Ensure the sysvol path exists */
+	gpo_dir = talloc_asprintf(ctx, "%s/%s", sysvol_path, GPODIR);
+	mkdir_p(gpo_dir, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
+	gpo_file = talloc_asprintf(ctx, "%s/%s", gpo_dir, GPOFILE);
+
+	/* 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);
+	torture_assert(tctx, samdb, "Failed to connect to the samdb");
+
+	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,
+		       "Searching the samdb failed");
+
+	old_message = result->msgs[0];
+
+	/* Disable the policy */
+	open(disable_file, O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666);
+
+	for (i = 0; i < 3; i++) {
+		/* Write out the sysvol */
+		if ( (fp = fopen(gpo_file, "w")) ) {
+			fputs(talloc_asprintf(ctx, GPTTMPL, minpwdcases[i],
+					      maxpwdcases[i], pwdlencases[i],
+					      pwdpropcases[i]), fp);
+			fclose(fp);
+		}
+
+		gpt_file = talloc_asprintf(ctx, "%s/%s", sysvol_path, GPTINI);
+		increment_gpt_ini(ctx, gpt_file);
+
+		exec_gpo_update_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,
+			       "Searching the samdb failed");
+		/* minPwdAge */
+		torture_assert_int_equal(tctx, unix2nttime(
+			ldb_msg_find_attr_as_string(
+				result->msgs[0],
+				attrs[0],
+				"")
+			),
+			unix2nttime(ldb_msg_find_attr_as_string(old_message,
+				attrs[0],
+				"")
+			),
+			"The minPwdAge should not have been applied");
+		/* maxPwdAge */
+		torture_assert_int_equal(tctx, unix2nttime(
+			ldb_msg_find_attr_as_string(
+				result->msgs[0],
+				attrs[1],
+				"")
+			),
+			unix2nttime(ldb_msg_find_attr_as_string(old_message,
+				attrs[1],
+				"")
+			),
+			"The maxPwdAge should not have been applied");
+		/* minPwdLength */
+		torture_assert_int_equal(tctx,
+			ldb_msg_find_attr_as_int(
+				result->msgs[0],
+				attrs[2],
+				-1
+			),
+			ldb_msg_find_attr_as_int(
+				old_message,
+				attrs[2],
+				-2
+			),
+			"The minPwdLength should not have been applied");
+		/* pwdProperties */
+		torture_assert_int_equal(tctx,
+			ldb_msg_find_attr_as_int(
+				result->msgs[0],
+				attrs[3],
+				-1
+			),
+			ldb_msg_find_attr_as_int(
+				old_message,
+				attrs[3],
+				-2
+			),
+			"The pwdProperties should not have been applied");
+	}
+
+	/* Unapply the settings and verify they are removed */
+	exec_gpo_unapply_command(tctx);
+
+	/* Re-enable the policy */
+	remove(disable_file);
+
+	talloc_free(ctx);
+	return true;
+}
-- 
2.13.6


From cf592fd6730fc8e5ea27a6be2db6a84da36874ca Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder at suse.com>
Date: Mon, 15 Jan 2018 13:38:59 -0700
Subject: [PATCH 3/3] gpo: make gpo extensible

Make gpo easily extensible by automatically importing
extensions based on their location, ex.
Python scripts in /var/lib/samba/gp_exts/kdc/ will
only apply to a kdc, while scripts in
/var/lib/samba/gp_exts/machine/ will only apply machine
policies (which apply to both client machines and kdc).
Extensions must inherit from the class gp_ext(), and
these child classes are automatically detected.

Signed-off-by: David Mulder <dmulder at suse.com>
---
 buildtools/wafsamba/wafsamba.py             |   8 +-
 python/gp_exts/__init__.py                  |  37 ++++++++++
 python/gp_exts/gp_example_ex.py             | 109 ++++++++++++++++++++++++++++
 python/gp_exts/kdc/__init__.py              |   4 +
 python/{samba => gp_exts/kdc}/gp_sec_ext.py |   2 +-
 python/gp_exts/machine/__init__.py          |   4 +
 python/wscript                              |   6 ++
 source4/scripting/bin/samba_gpoupdate       |  11 ++-
 source4/torture/gpo/apply.c                 |   2 +-
 9 files changed, 177 insertions(+), 6 deletions(-)
 create mode 100644 python/gp_exts/__init__.py
 create mode 100644 python/gp_exts/gp_example_ex.py
 create mode 100644 python/gp_exts/kdc/__init__.py
 rename python/{samba => gp_exts/kdc}/gp_sec_ext.py (99%)
 create mode 100644 python/gp_exts/machine/__init__.py

diff --git a/buildtools/wafsamba/wafsamba.py b/buildtools/wafsamba/wafsamba.py
index 4bb19d070e2..ee0eba725bf 100644
--- a/buildtools/wafsamba/wafsamba.py
+++ b/buildtools/wafsamba/wafsamba.py
@@ -779,6 +779,9 @@ sys.path.insert(1, "%s")""" % (task.env["PYTHONARCHDIR"], task.env["PYTHONDIR"])
     else:
         replacement_shebang = "#!/usr/bin/env %s\n" % task.env["PYTHON"]
 
+    gp_pattern='sys.path.insert(0, "bin/py_gp")'
+    gp_replacement="""sys.path.insert(0, "%s")""" % task.env["STATEDIR"]
+
     installed_location=task.outputs[0].bldpath(task.env)
     source_file = open(task.inputs[0].srcpath(task.env))
     installed_file = open(installed_location, 'w')
@@ -790,6 +793,8 @@ sys.path.insert(1, "%s")""" % (task.env["PYTHONARCHDIR"], task.env["PYTHONDIR"])
             newline = replacement_shebang
         elif pattern in line:
             newline = line.replace(pattern, replacement)
+        elif gp_pattern and gp_pattern in line:
+            newline = line.replace(gp_pattern, gp_replacement)
         installed_file.write(newline)
         lineno = lineno + 1
     installed_file.close()
@@ -840,7 +845,8 @@ def install_file(bld, destdir, file, chmod=MODE_644, flat=False,
         inst_file = file + '.inst'
         bld.SAMBA_GENERATOR('python_%s' % destname,
                             rule=copy_and_fix_python_path,
-                            dep_vars=["PYTHON","PYTHON_SPECIFIED","PYTHONDIR","PYTHONARCHDIR"],
+                            dep_vars=["PYTHON","PYTHON_SPECIFIED",
+                                      "PYTHONDIR","PYTHONARCHDIR","STATEDIR"],
                             source=file,
                             target=inst_file)
         file = inst_file
diff --git a/python/gp_exts/__init__.py b/python/gp_exts/__init__.py
new file mode 100644
index 00000000000..d82298c79f3
--- /dev/null
+++ b/python/gp_exts/__init__.py
@@ -0,0 +1,37 @@
+# Get a list of modules names
+def list_modules(filename):
+    from os import listdir
+    from os.path import dirname, abspath, splitext
+    module_names = []
+    for f in listdir(dirname(abspath(filename))):
+        split = splitext(f)
+        if not '__init__' in f and (split[-1] == '.py' or split[-1] == '.pyc'):
+            module_names.append(split[0])
+    return list(set(module_names))
+
+# Find the top base class of a class
+# doesn't work with multiple base classes
+def get_base(cls):
+    base = None
+    bases = cls.__bases__
+    while len(bases) == 1 and bases[-1].__name__ != 'object':
+	base = bases[0]
+        bases = base.__bases__
+    return base
+
+def get_gp_exts_from_module(parent):
+    import inspect
+    parent_gp_exts = []
+    for mod_name in parent.modules:
+        mod = getattr(parent, mod_name)
+        clses = inspect.getmembers(mod, inspect.isclass)
+        for cls in clses:
+            base = get_base(cls[-1])
+            if base and base.__name__ == 'gp_ext' and cls[-1].__module__ == mod.__name__:
+                parent_gp_exts.append(cls[-1])
+    return parent_gp_exts
+
+from machine import *
+machine_gp_exts = get_gp_exts_from_module(machine)
+from kdc import *
+kdc_gp_exts = get_gp_exts_from_module(kdc)
diff --git a/python/gp_exts/gp_example_ex.py b/python/gp_exts/gp_example_ex.py
new file mode 100644
index 00000000000..283a40ff43f
--- /dev/null
+++ b/python/gp_exts/gp_example_ex.py
@@ -0,0 +1,109 @@
+from samba.gpclass import gp_ext, file_to
+
+# When placed in one of the respective kdc, machine, or user sub-directoriers,
+# a python file defining a class inheriting from gp_ext() will be automatically
+# imported and loaded by the samba_gpoupdate script and applied on the gpupdate
+# interval.
+
+class example_setter(file_to):
+    '''An example setter class, which must inherit from file_to
+    The mapper() and __str__() functions are mandatory, the rest of the
+    implementation is arbitrary.
+    '''
+
+    def set_int(self, val):
+        example = open('/etc/example.conf', 'w')
+        example.write('%s = %d\n' % (self.attr, val))
+        example.close()
+
+    def set_str(self, val):
+        example = open('/etc/example.conf', 'w')
+        example.write('%s = %s\n' % (self.attr, val))
+        example.close()
+
+    def to_int(self):
+        return int(self.val)
+
+    def mapper(self):
+        '''
+        Maps local setting names to an apply function, and a value converter.
+        The self.explicit converter causes the original value to be used,
+        without conversion.
+        '''
+        return { "LinuxSettingInt" : (self.set_int, self.to_int),
+                 "LinuxSettingStr" : (self.set_str, self.explicit),
+               }
+
+    def __str__(self):
+        '''
+        The name of the setter, as seen in the apply_map() keys.
+        '''
+        return "Example"
+
+class gp_example_ex(gp_ext):
+    '''An example group policy extension, which must inhert from gp_ext
+    A group policy extension reads policies from the sysvol, and applies them
+    as settings to the local machine.
+    '''
+
+    def __str__(self):
+        '''
+        Must return a unique extension name for identifying the extension
+        '''
+        return "Example extension"
+
+    def read(self, policy):
+        '''
+        Receives the policy file as a string, and must parse and apply the
+        policy using the setters returned by the apply_map() function.
+        Alteratively, your class could inherit from gp_inf_ext() if reading a
+        ini/inf file (common for gpos) and this function is implemented for
+        you.
+        '''
+        mappings = self.apply_map()
+
+        # Policies are all on seperate lines, space delimited
+        for line in policy.split('\n'):
+            (key, value) = line.split()
+            (att, setter) = mappings['Example'].get(key)
+            setter(self.logger,
+                   self.ldb,
+                   self.gp_db,
+                   self.lp,
+                   self.creds,
+                   att,
+                   value).update_samba()
+            # gp_db.commit() saves the unapply log, this is mandatory
+            self.gp_db.commit()
+
+    def list(self, rootpath):
+        '''
+        This function must return the sysvol path to a policy file to be read
+        '''
+        return os.path.join(rootpath, "MACHINE/SomeGPfile.txt")
+
+    def apply_map(self):
+        '''
+        The apply_map must return a dictionary of dictionaries. The first key
+        "Example" is the name of the setter class. The inner keys
+        "MSSettingName1", etc are the gpo value of the setting being applied.
+        The value tuple contains the name of the local setting, and the
+        setter class. The setter class is used within the read() function to
+        convert and apply settings.
+        '''
+        return { "Example" : { "MSSettingName1":
+                                    ("LinuxSettingInt", example_setter),
+                               "MSSettingName2":
+                                    ("LinuxSettingStr", example_setter),
+                             }
+               }
+
+    @staticmethod
+    def disabled_file():
+        '''
+        This function, exactly as written, must be included in every extension.
+        It returns a file name that, when existing, disables this policy
+        extension.
+        '''
+        return os.path.splitext(os.path.abspath(__file__))[0] + '.py.disabled'
+
diff --git a/python/gp_exts/kdc/__init__.py b/python/gp_exts/kdc/__init__.py
new file mode 100644
index 00000000000..f88f7798389
--- /dev/null
+++ b/python/gp_exts/kdc/__init__.py
@@ -0,0 +1,4 @@
+from gp_exts import list_modules
+modules = list_modules(__file__)
+__all__ = modules
+del list_modules
diff --git a/python/samba/gp_sec_ext.py b/python/gp_exts/kdc/gp_sec_ext.py
similarity index 99%
rename from python/samba/gp_sec_ext.py
rename to python/gp_exts/kdc/gp_sec_ext.py
index b32793e1043..85a9a2d3d0b 100644
--- a/python/samba/gp_sec_ext.py
+++ b/python/gp_exts/kdc/gp_sec_ext.py
@@ -1,5 +1,5 @@
 import os.path
-from gpclass import file_to, gp_inf_ext
+from samba.gpclass import file_to, gp_inf_ext
 
 class inf_to_kdc_tdb(file_to):
     def mins_to_hours(self):
diff --git a/python/gp_exts/machine/__init__.py b/python/gp_exts/machine/__init__.py
new file mode 100644
index 00000000000..f88f7798389
--- /dev/null
+++ b/python/gp_exts/machine/__init__.py
@@ -0,0 +1,4 @@
+from gp_exts import list_modules
+modules = list_modules(__file__)
+__all__ = modules
+del list_modules
diff --git a/python/wscript b/python/wscript
index 211fac4de62..9bfff7fb991 100644
--- a/python/wscript
+++ b/python/wscript
@@ -82,3 +82,9 @@ def build(bld):
                              installdir='python')
 
             bld.INSTALL_WILDCARD('${PYTHONARCHDIR}', 'samba/**/*.py', flat=False)
+        for env in bld.gen_python_environments():
+            bld.SAMBA_SCRIPT('samba_python_files',
+                             pattern='gp_exts/**/*.py',
+                             installdir='py_gp')
+
+            bld.INSTALL_WILDCARD('${STATEDIR}', 'gp_exts/**/*.py', flat=False)
diff --git a/source4/scripting/bin/samba_gpoupdate b/source4/scripting/bin/samba_gpoupdate
index b8b2bb464a0..b566b0a20be 100755
--- a/source4/scripting/bin/samba_gpoupdate
+++ b/source4/scripting/bin/samba_gpoupdate
@@ -26,7 +26,9 @@ import os
 import sys
 
 sys.path.insert(0, "bin/python")
+sys.path.insert(0, "bin/py_gp")
 
+import gp_exts
 import optparse
 from samba import getopt as options
 from samba.auth import system_session
@@ -35,7 +37,6 @@ try:
 except:
     SamDB = None
 from samba.gpclass import *
-from samba.gp_sec_ext import gp_sec_ext
 from samba.net import Net
 from samba.dcerpc import nbt
 from samba import smb
@@ -171,8 +172,12 @@ if __name__ == "__main__":
     gp_extensions = []
     if opts.machine:
         if lp.get('server role') == 'active directory domain controller':
-            if gp_sec_ext.enabled():
-                gp_extensions.append(gp_sec_ext(logger))
+            for ext in gp_exts.kdc_gp_exts:
+                if ext.enabled():
+                    gp_extensions.append(ext(logger))
+        for ext in gp_exts.machine_gp_exts:
+            if ext.enabled():
+                gp_extensions.append(ext(logger))
     else:
         pass # User extensions
 
diff --git a/source4/torture/gpo/apply.c b/source4/torture/gpo/apply.c
index dcfdd7a5dbe..7c43ce94b7f 100644
--- a/source4/torture/gpo/apply.c
+++ b/source4/torture/gpo/apply.c
@@ -325,7 +325,7 @@ bool torture_gpo_disable_policies(struct torture_context *tctx)
 	int pwdlencases[] = { 0, 1, 14 };
 	int pwdpropcases[] = { 0, 1, 1 };
 	struct ldb_message *old_message = NULL;
-	const char *disable_file = "bin/python/samba/gp_sec_ext.py.disabled";
+	const char *disable_file = "bin/py_gp/gp_exts/kdc/gp_sec_ext.py.disabled";
 
 	sysvol_path = lpcfg_path(lpcfg_service(tctx->lp_ctx, "sysvol"),
 				 lpcfg_default_service(tctx->lp_ctx), tctx);
-- 
2.13.6



More information about the samba-technical mailing list