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