Fwd: [PATCHES] simple gpo patches fixes

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Wed Mar 28 22:07:00 UTC 2018


On 22/03/18 02:52, David Mulder via samba-technical wrote:
> 
> 
> From 89cef25f9343fad2d11b161bd89281f9633e1d08 Mon Sep 17 00:00:00 2001
> From: David Mulder <dmulder at suse.com>
> Date: Mon, 12 Mar 2018 09:44:38 -0600
> Subject: [PATCH 1/2] gpo: Separate logic from implementation in gpclass
> 
> This patch does not change functionality, it only
> rearranges sources to make it possible to add new
> gpo extensions. The previous logic has the
> gp_sec_ext tightly coupled with the parent gp_ext
> class. This patch decouples that relationship.
> Group Policy extensions can now be added as a
> single file which will import parent functionality
> from the samba.gpclass module. The gp_sec_ext has
> been modified this way.

In general it is better to structure these kinds of changes as a few
well defined steps. That makes it easier to see that each step is
leaving things logically as they were. By my count, you do six things
in this commit:

1. rename inf_to to file_to
2. shift an __init__ definition up the class hierarchy.
3. shift some classes into the new module
4. shift some functions from the script into the gpclass module
5. change formatting in one or two places.
6. something to do with a subclassable read method.

It would be better to do these in six commits.

> Signed-off-by: David Mulder <dmulder at suse.com>
> ---
>  python/samba/gp_sec_ext.py            | 136 +++++++++++++++++
>  python/samba/gpclass.py               | 277 ++++++++++++++--------------------
>  source4/scripting/bin/samba_gpoupdate |  79 +---------
>  3 files changed, 255 insertions(+), 237 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..b50f1fb3f3e
> --- /dev/null
> +++ b/python/samba/gp_sec_ext.py
> @@ -0,0 +1,136 @@
> +import os.path

This should start with a a copyright/GPL blurb, along the lines of
gpclass.py and all the others.

> +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
> +                                    ),
> +                                   }
> +               }
> +
> diff --git a/python/samba/gpclass.py b/python/samba/gpclass.py
> index 33c9001cb6d..6888a18999a 100644
> --- a/python/samba/gpclass.py
> +++ b/python/samba/gpclass.py
> @@ -25,6 +25,11 @@ from StringIO import StringIO
>  from abc import ABCMeta, abstractmethod
>  import xml.etree.ElementTree as etree
>  import re
> +from samba.net import Net
> +from samba.dcerpc import nbt
> +from samba import smb
> +import samba.gpo as gpo
> +import chardet
>  
>  try:
>      from enum import Enum
> @@ -286,6 +291,9 @@ class GPOStorage:
>  class gp_ext(object):
>      __metaclass__ = ABCMeta
>  
> +    def __init__(self, logger):
> +        self.logger = logger
> +
>      @abstractmethod
>      def list(self, rootpath):
>          pass
> @@ -295,14 +303,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 +352,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 +363,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 +405,78 @@ 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
> +    @abstractmethod
> +    def __str__(self):
> +        pass
>  
> -        # Fixing the bug where only some Linux Boxes capitalize MACHINE
> -        if afile.endswith('inf'):
> +''' Fetch the hostname of a writable DC '''
> +def get_dc_hostname(creds, lp):
> +    net = Net(creds=creds, lp=lp)
> +    cldap_ret = net.finddc(domain=lp.get('realm'), flags=(nbt.NBT_SERVER_LDAP |
> +        nbt.NBT_SERVER_DS))
> +    return cldap_ret.pdc_dns_name
> +
> +''' Fetch a list of GUIDs for applicable GPOs '''
> +def get_gpo_list(dc_hostname, creds, lp):
> +    gpos = []
> +    ads = gpo.ADS_STRUCT(dc_hostname, lp, creds)
> +    if ads.connect():
> +        gpos = ads.get_gpo_list(creds.get_username())
> +    return gpos
> +
> +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)
> +    try:
> +        conn =  smb.SMB(dc_hostname, 'sysvol', lp=lp, creds=creds)
> +    except:
> +        logger.error('Error connecting to \'%s\' using SMB' % dc_hostname)
> +        raise
> +    gpos = get_gpo_list(dc_hostname, creds, lp)
> +
> +    for gpo_obj in gpos:
> +        guid = gpo_obj.name
> +        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])
> +        if version != store.get_int(guid):
> +            logger.info('GPO %s has changed' % guid)
> +            gp_db.state(GPOSTATE.APPLY)
> +        else:
> +            gp_db.state(GPOSTATE.ENFORCE)
> +        gp_db.set_guid(guid)
> +        store.start()
> +        for ext in gp_extensions:
>              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
> +                ext.parse(ext.list(path), test_ldb, conn, gp_db, lp)
> +            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()
> +
> +def unapply_log(gp_db):
> +    while True:
> +        item = gp_db.apply_log_pop()
> +        if item:
> +            yield item
> +        else:
> +            break
> +
> +def unapply_gp(lp, creds, test_ldb, logger, store, gp_extensions):
> +    gp_db = store.get_gplog(creds.get_username())
> +    gp_db.state(GPOSTATE.UNAPPLY)
> +    for gpo_guid in unapply_log(gp_db):
> +        gp_db.set_guid(gpo_guid)
> +        unapply_attributes = gp_db.list(gp_extensions)
> +        for attr in unapply_attributes:
> +            attr_obj = attr[-1](logger, test_ldb, gp_db, lp, attr[0], attr[1])
> +            attr_obj.mapper()[attr[0]][0](attr[1]) # Set the old value
> +            gp_db.delete(str(attr_obj), attr[0])
> +        gp_db.commit()
>  
> diff --git a/source4/scripting/bin/samba_gpoupdate b/source4/scripting/bin/samba_gpoupdate
> index 26e0984413e..89b3ed77616 100755
> --- a/source4/scripting/bin/samba_gpoupdate
> +++ b/source4/scripting/bin/samba_gpoupdate
> @@ -34,84 +34,9 @@ try:
>      from samba.samdb import SamDB
>  except:
>      SamDB = None
> -from samba.gpclass import *
> -from samba.net import Net
> -from samba.dcerpc import nbt
> -from samba import smb
> -import samba.gpo as gpo
> +from samba.gpclass import apply_gp, unapply_gp, GPOStorage
> +from samba.gp_sec_ext import gp_sec_ext
>  import logging
> -import chardet
> -
> -''' Fetch the hostname of a writable DC '''
> -def get_dc_hostname(creds, lp):
> -    net = Net(creds=creds, lp=lp)
> -    cldap_ret = net.finddc(domain=lp.get('realm'), flags=(nbt.NBT_SERVER_LDAP |
> -        nbt.NBT_SERVER_DS))
> -    return cldap_ret.pdc_dns_name
> -
> -''' Fetch a list of GUIDs for applicable GPOs '''
> -def get_gpo_list(dc_hostname, creds, lp):
> -    gpos = []
> -    ads = gpo.ADS_STRUCT(dc_hostname, lp, creds)
> -    if ads.connect():
> -        gpos = ads.get_gpo_list(creds.get_username())
> -    return gpos
> -
> -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)
> -    try:
> -        conn =  smb.SMB(dc_hostname, 'sysvol', lp=lp, creds=creds)
> -    except:
> -        logger.error('Error connecting to \'%s\' using SMB' % dc_hostname)
> -        raise
> -    gpos = get_gpo_list(dc_hostname, creds, lp)
> -
> -    for gpo_obj in gpos:
> -        guid = gpo_obj.name
> -        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])
> -        if version != store.get_int(guid):
> -            logger.info('GPO %s has changed' % guid)
> -            gp_db.state(GPOSTATE.APPLY)
> -        else:
> -            gp_db.state(GPOSTATE.ENFORCE)
> -        gp_db.set_guid(guid)
> -        store.start()
> -        for ext in gp_extensions:
> -            try:
> -                ext.parse(ext.list(path), test_ldb, conn, gp_db, lp)
> -            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()
> -
> -def unapply_log(gp_db):
> -    while True:
> -        item = gp_db.apply_log_pop()
> -        if item:
> -            yield item
> -        else:
> -            break
> -
> -def unapply_gp(lp, creds, test_ldb, logger, store, gp_extensions):
> -    gp_db = store.get_gplog(creds.get_username())
> -    gp_db.state(GPOSTATE.UNAPPLY)
> -    for gpo_guid in unapply_log(gp_db):
> -        gp_db.set_guid(gpo_guid)
> -        unapply_attributes = gp_db.list(gp_extensions)
> -        for attr in unapply_attributes:
> -            attr_obj = attr[-1](logger, test_ldb, gp_db, lp, attr[0], attr[1])
> -            attr_obj.mapper()[attr[0]][0](attr[1]) # Set the old value
> -            gp_db.delete(str(attr_obj), attr[0])
> -        gp_db.commit()
>  
>  if __name__ == "__main__":
>      parser = optparse.OptionParser('samba_gpoupdate [options]')
> -- 
> 2.13.6
> 
> 
> From 47e58fcf5b97dcb7c1004420af11231b1220cc99 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 2/2] gpo: Read GPO versions locally, not from sysvol
> 
> This patch does not change current functionality
> for the kdc. Non-kdc clients cannot read directly
> from the sysvol, so we need to store the GPT.INI
> file locally to read each gpo version.
> 
> Signed-off-by: David Mulder <dmulder at suse.com>
> ---
>  python/samba/gpclass.py | 20 ++++++++++++++++++--
>  source4/param/pyparam.c | 19 +++++++++++++++++++
>  2 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/python/samba/gpclass.py b/python/samba/gpclass.py
> index 6888a18999a..2678069090a 100644
> --- a/python/samba/gpclass.py
> +++ b/python/samba/gpclass.py
> @@ -424,6 +424,23 @@ def get_gpo_list(dc_hostname, creds, lp):
>          gpos = ads.get_gpo_list(creds.get_username())
>      return gpos
>  
> +def gpo_version(lp, conn, path):
> +    # gpo.gpo_get_sysvol_gpt_version() reads the GPT.INI from a local file.
> +    # If we don't have a sysvol path locally (if we're not a kdc), then
> +    # store the file locally here so it can be read on a client.
> +    sysvol = lp.get("path", "sysvol")
> +    if sysvol:
> +        local_path = os.path.join(sysvol, path, 'GPT.INI')
> +    else:
> +        gpt_path = lp.cache_path(os.path.join('gpt', path))
> +        local_path = os.path.join(gpt_path, 'GPT.INI')
> +        if not os.path.exists(os.path.dirname(local_path)):
> +            os.makedirs(os.path.dirname(local_path), 0o700)
> +        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)
> @@ -439,8 +456,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(lp, conn, path)
>          if version != store.get_int(guid):
>              logger.info('GPO %s has changed' % guid)
>              gp_db.state(GPOSTATE.APPLY)
> diff --git a/source4/param/pyparam.c b/source4/param/pyparam.c
> index f16c2c0b227..1d99ada09da 100644
> --- a/source4/param/pyparam.c
> +++ b/source4/param/pyparam.c
> @@ -358,6 +358,22 @@ static PyObject *py_samdb_url(PyObject *self, PyObject *unused)
>  	return PyStr_FromFormat("tdb://%s/sam.ldb", lpcfg_private_dir(lp_ctx));
>  }
>  
> +static PyObject *py_cache_path(PyObject *self, PyObject *args)
> +{
> +	struct loadparm_context *lp_ctx = PyLoadparmContext_AsLoadparmContext(self);
> +	char *name, *path;
> +	PyObject *ret;
> +
> +	if (!PyArg_ParseTuple(args, "s", &name)) {
> +		return NULL;
> +	}
> +
> +	path = lpcfg_cache_path(NULL, lp_ctx, name);
> +	ret = PyStr_FromString(path);

There are many reasons lpcfg_cache_path() can return NULL, which
PyString_FromString() can't cope with, so you need to check.

Also, in Python 3, is a string what you want? It means the path has to
be utf-8 -- is that something we know? I don't know what we normally
do here.

cheers,
Douglas

> +	talloc_free(path);
> +
> +	return ret;
> +}
>  
>  static PyMethodDef py_lp_ctx_methods[] = {
>  	{ "load", py_lp_ctx_load, METH_VARARGS,
> @@ -394,6 +410,9 @@ static PyMethodDef py_lp_ctx_methods[] = {
>  	{ "samdb_url", py_samdb_url, METH_NOARGS,
>  	        "S.samdb_url() -> string\n"
>  	        "Returns the current URL for sam.ldb." },
> +	{ "cache_path", py_cache_path, METH_VARARGS,
> +		"S.cache_path(name) -> string\n"
> +		"Returns a path in the Samba cache directory." },
>  	{ NULL }
>  };
>  
> -- 
> 2.13.6






More information about the samba-technical mailing list