Fwd: [PATCHES] simple gpo patches fixes

David Mulder dmulder at suse.com
Thu Mar 29 18:44:13 UTC 2018


Thanks for the review!

On 03/28/2018 04:07 PM, Douglas Bagnall via samba-technical wrote:
> 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.
I've broken down that patch into sizable chunks. I've pushed them to the
git repo: https://github.com/samba-team/samba/pull/154
>> 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.
Added the copyright notice.
>
>> +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.
Added a check for this.
> 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.
None of this code works in python3 yet, afaik. The libgpo python code
was only just barely ported to python3.
>
> 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
>
>
>
>

-- 
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)




More information about the samba-technical mailing list