[PATCHES v4] Fix GPO unapply log
David Mulder
dmulder at suse.com
Fri Dec 15 19:29:35 UTC 2017
Hmm, went to merge, but I don't see the push at https://git.samba.org/samba
On 12/15/2017 12:00 PM, Jeremy Allison wrote:
> On Fri, Dec 15, 2017 at 11:03:31AM +1300, Garming Sam via samba-technical wrote:
>> Hi,
>>
>> I've marked my reviews and also added some tidy-up patches for some of
>> the earlier patches.
>>
>> Can someone else please review and push?
> LGTM. RB+ and pushed ! Thanks Garming.
>
>> Garming
>>
>> On 14/12/17 08:00, David Mulder wrote:
>>> And this time I'll actually attach the patches. Sorry about that.
>>>
>> From de2d7fd4c39d2e154737b7e10466ad9bea116359 Mon Sep 17 00:00:00 2001
>> From: Garming Sam <garming at catalyst.net.nz>
>> Date: Wed, 22 Nov 2017 10:57:18 +1300
>> Subject: [PATCH 1/7] libgpo: Always check for ldap_server argument
>>
>> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
>> ---
>> libgpo/pygpo.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/libgpo/pygpo.c b/libgpo/pygpo.c
>> index d7bb173..fba0e34 100644
>> --- a/libgpo/pygpo.c
>> +++ b/libgpo/pygpo.c
>> @@ -192,9 +192,11 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds)
>> } else {
>> realm = lp_realm();
>> workgroup = lp_workgroup();
>> - if (!ldap_server) return -1;
>> }
>>
>> + if (ldap_server == NULL) {
>> + return -1;
>> + }
>> if ( !(self->ads_ptr = ads_init(realm, workgroup, ldap_server)) )
>> return -1;
>>
>> --
>> 1.9.1
>>
>>
>> From 02fcbacdc683229eefc8669cf4d0a3d1bb926a6e Mon Sep 17 00:00:00 2001
>> From: Garming Sam <garming at catalyst.net.nz>
>> Date: Wed, 22 Nov 2017 10:58:55 +1300
>> Subject: [PATCH 2/7] libgpo: typo credentaials -> credentials
>>
>> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
>> ---
>> libgpo/pygpo.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libgpo/pygpo.c b/libgpo/pygpo.c
>> index fba0e34..46063b4 100644
>> --- a/libgpo/pygpo.c
>> +++ b/libgpo/pygpo.c
>> @@ -166,7 +166,7 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds)
>> if (!py_check_dcerpc_type(py_creds, "samba.credentials",
>> "Credentials")) {
>> PyErr_Format(PyExc_TypeError,
>> - "Expected samba.credentaials "
>> + "Expected samba.credentials "
>> "for credentials argument");
>> return -1;
>> }
>> --
>> 1.9.1
>>
>>
>> From 448cfcc890b3caf717e5d60ca25ed4693df7fca1 Mon Sep 17 00:00:00 2001
>> From: Garming Sam <garming at catalyst.net.nz>
>> Date: Wed, 22 Nov 2017 11:00:35 +1300
>> Subject: [PATCH 3/7] libgpo: Tidy up some if statements
>>
>> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
>> ---
>> libgpo/pygpo.c | 48 ++++++++++++++++++++++++++++++++----------------
>> 1 file changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/libgpo/pygpo.c b/libgpo/pygpo.c
>> index 46063b4..e94f0f0 100644
>> --- a/libgpo/pygpo.c
>> +++ b/libgpo/pygpo.c
>> @@ -159,8 +159,9 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds)
>> "credentials", NULL};
>> if (!PyArg_ParseTupleAndKeywords(args, kwds, "sO|O",
>> discard_const_p(char *, kwlist),
>> - &ldap_server, &lp_obj, &py_creds))
>> + &ldap_server, &lp_obj, &py_creds)) {
>> return -1;
>> + }
>>
>> if (py_creds) {
>> if (!py_check_dcerpc_type(py_creds, "samba.credentials",
>> @@ -197,8 +198,11 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds)
>> if (ldap_server == NULL) {
>> return -1;
>> }
>> - if ( !(self->ads_ptr = ads_init(realm, workgroup, ldap_server)) )
>> +
>> + self->ads_ptr = ads_init(realm, workgroup, ldap_server);
>> + if (self->ads_ptr == NULL) {
>> return -1;
>> + }
>>
>> return 0;
>> }
>> @@ -223,23 +227,28 @@ static PyObject* py_ads_connect(ADS *self)
>> Py_RETURN_FALSE;
>> }
>> } else {
>> - char *passwd;
>> -
>> - if (asprintf(&(self->ads_ptr->auth.user_name), "%s$",
>> - lp_netbios_name()) == -1) {
>> - PyErr_SetString(PyExc_SystemError, "Failed to asprintf");
>> + char *passwd = NULL;
>> + int ret = asprintf(&(self->ads_ptr->auth.user_name), "%s$",
>> + lp_netbios_name());
>> + if (ret == -1) {
>> + PyErr_SetString(PyExc_SystemError,
>> + "Failed to asprintf");
>> TALLOC_FREE(frame);
>> Py_RETURN_FALSE;
>> - } else
>> + } else {
>> self->ads_ptr->auth.flags |= ADS_AUTH_USER_CREDS;
>> + }
>> +
>> if (!secrets_init()) {
>> - PyErr_SetString(PyExc_SystemError, "secrets_init() failed");
>> + PyErr_SetString(PyExc_SystemError,
>> + "secrets_init() failed");
>> TALLOC_FREE(frame);
>> Py_RETURN_FALSE;
>> }
>> - if (!(passwd =
>> - secrets_fetch_machine_password(self->ads_ptr->server.workgroup,
>> - NULL, NULL))) {
>> +
>> + passwd = secrets_fetch_machine_password(self->ads_ptr->server.workgroup,
>> + NULL, NULL);
>> + if (passwd == NULL) {
>> PyErr_SetString(PyExc_SystemError,
>> "Failed to fetch the machine account password");
>> TALLOC_FREE(frame);
>> @@ -346,7 +355,7 @@ static ADS_STATUS find_samaccount(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx,
>>
>> if (dn_ret) {
>> *dn_ret = talloc_strdup(mem_ctx, dn);
>> - if (!*dn_ret) {
>> + if (*dn_ret == NULL) {
>> status = ADS_ERROR(LDAP_NO_MEMORY);
>> goto out;
>> }
>> @@ -473,18 +482,25 @@ void initgpo(void)
>> PyObject *m;
>>
>> debug_setup_talloc_log();
>> +
>> /* Instantiate the types */
>> m = Py_InitModule3("gpo", py_gpo_methods, "libgpo python bindings");
>> - if (m == NULL) return;
>> + if (m == NULL) {
>> + return;
>> + }
>> +
>> PyModule_AddObject(m, "version",
>> PyString_FromString(SAMBA_VERSION_STRING));
>>
>> - if (PyType_Ready(&ads_ADSType) < 0)
>> + if (PyType_Ready(&ads_ADSType) < 0) {
>> return;
>> + }
>> +
>> PyModule_AddObject(m, "ADS_STRUCT", (PyObject *)&ads_ADSType);
>>
>> - if (pytalloc_BaseObject_PyType_Ready(&GPOType) < 0)
>> + if (pytalloc_BaseObject_PyType_Ready(&GPOType) < 0) {
>> return;
>> + }
>>
>> Py_INCREF((PyObject *)(void *)&GPOType);
>> PyModule_AddObject(m, "GROUP_POLICY_OBJECT",
>> --
>> 1.9.1
>>
>>
>> From 0510b319fe8ac26c8eedc17806b0ca4d97f068e9 Mon Sep 17 00:00:00 2001
>> From: Garming Sam <garming at catalyst.net.nz>
>> Date: Wed, 22 Nov 2017 11:00:56 +1300
>> Subject: [PATCH 4/7] libgpo: Remedy some longer lines
>>
>> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
>> ---
>> libgpo/pygpo.c | 64 +++++++++++++++++++++++++++++++++++++---------------------
>> 1 file changed, 41 insertions(+), 23 deletions(-)
>>
>> diff --git a/libgpo/pygpo.c b/libgpo/pygpo.c
>> index e94f0f0..7a02a0d 100644
>> --- a/libgpo/pygpo.c
>> +++ b/libgpo/pygpo.c
>> @@ -54,11 +54,14 @@ static PyGetSetDef GPO_setters[] = {
>> NULL},
>> {discard_const_p(char, "file_sys_path"), (getter)GPO_get_file_sys_path,
>> NULL, NULL, NULL},
>> - {discard_const_p(char, "display_name"), (getter)GPO_get_display_name, NULL,
>> - NULL, NULL},
>> - {discard_const_p(char, "name"), (getter)GPO_get_name, NULL, NULL, NULL},
>> - {discard_const_p(char, "link"), (getter)GPO_get_link, NULL, NULL, NULL},
>> - {discard_const_p(char, "user_extensions"), (getter)GPO_get_user_extensions,
>> + {discard_const_p(char, "display_name"), (getter)GPO_get_display_name,
>> + NULL, NULL, NULL},
>> + {discard_const_p(char, "name"), (getter)GPO_get_name, NULL, NULL,
>> + NULL},
>> + {discard_const_p(char, "link"), (getter)GPO_get_link, NULL, NULL,
>> + NULL},
>> + {discard_const_p(char, "user_extensions"),
>> + (getter)GPO_get_user_extensions,
>> NULL, NULL, NULL},
>> {discard_const_p(char, "machine_extensions"),
>> (getter)GPO_get_machine_extensions, NULL, NULL, NULL},
>> @@ -81,7 +84,8 @@ static PyObject *py_gpo_get_unix_path(PyObject *self, PyObject *args,
>> discard_const_p(char *, kwlist),
>> &cache_dir)) {
>> PyErr_SetString(PyExc_SystemError,
>> - "Failed to parse arguments to gpo_get_unix_path()");
>> + "Failed to parse arguments to "
>> + "gpo_get_unix_path()");
>> goto out;
>> }
>>
>> @@ -113,7 +117,8 @@ out:
>> }
>>
>> static PyMethodDef GPO_methods[] = {
>> - {"get_unix_path", (PyCFunction)py_gpo_get_unix_path, METH_KEYWORDS, NULL },
>> + {"get_unix_path", (PyCFunction)py_gpo_get_unix_path, METH_KEYWORDS,
>> + NULL },
>> {NULL}
>> };
>>
>> @@ -155,8 +160,9 @@ static int py_ads_init(ADS *self, PyObject *args, PyObject *kwds)
>> PyObject *lp_obj = NULL;
>> struct loadparm_context *lp_ctx = NULL;
>>
>> - static const char *kwlist[] = {"ldap_server", "loadparm_context",
>> - "credentials", NULL};
>> + static const char *kwlist[] = {
>> + "ldap_server", "loadparm_context", "credentials", NULL
>> + };
>> if (!PyArg_ParseTupleAndKeywords(args, kwds, "sO|O",
>> discard_const_p(char *, kwlist),
>> &ldap_server, &lp_obj, &py_creds)) {
>> @@ -222,7 +228,8 @@ static PyObject* py_ads_connect(ADS *self)
>>
>> status = ads_connect_user_creds(self->ads_ptr);
>> if (!ADS_ERR_OK(status)) {
>> - PyErr_SetString(PyExc_SystemError, "ads_connect() failed");
>> + PyErr_SetString(PyExc_SystemError,
>> + "ads_connect() failed");
>> TALLOC_FREE(frame);
>> Py_RETURN_FALSE;
>> }
>> @@ -250,12 +257,14 @@ static PyObject* py_ads_connect(ADS *self)
>> NULL, NULL);
>> if (passwd == NULL) {
>> PyErr_SetString(PyExc_SystemError,
>> - "Failed to fetch the machine account password");
>> + "Failed to fetch the machine account "
>> + "password");
>> TALLOC_FREE(frame);
>> Py_RETURN_FALSE;
>> }
>> self->ads_ptr->auth.password = smb_xstrdup(passwd);
>> - self->ads_ptr->auth.realm = smb_xstrdup(self->ads_ptr->server.realm);
>> + self->ads_ptr->auth.realm =
>> + smb_xstrdup(self->ads_ptr->server.realm);
>> if (!strupper_m(self->ads_ptr->auth.realm)) {
>> PyErr_SetString(PyExc_SystemError, "Failed to strdup");
>> TALLOC_FREE(frame);
>> @@ -265,7 +274,8 @@ static PyObject* py_ads_connect(ADS *self)
>>
>> status = ads_connect(self->ads_ptr);
>> if (!ADS_ERR_OK(status)) {
>> - PyErr_SetString(PyExc_SystemError, "ads_connect() failed");
>> + PyErr_SetString(PyExc_SystemError,
>> + "ads_connect() failed");
>> TALLOC_FREE(frame);
>> SAFE_FREE(passwd);
>> Py_RETURN_FALSE;
>> @@ -320,14 +330,15 @@ static ADS_STATUS find_samaccount(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx,
>> char *dn = NULL;
>> uint32_t uac = 0;
>>
>> - filter = talloc_asprintf(mem_ctx, "(sAMAccountName=%s)", samaccountname);
>> + filter = talloc_asprintf(mem_ctx, "(sAMAccountName=%s)",
>> + samaccountname);
>> if (filter == NULL) {
>> status = ADS_ERROR_NT(NT_STATUS_NO_MEMORY);
>> goto out;
>> }
>>
>> - status = ads_do_search_all(ads, ads->config.bind_path, LDAP_SCOPE_SUBTREE,
>> - filter, attrs, &res);
>> + status = ads_do_search_all(ads, ads->config.bind_path,
>> + LDAP_SCOPE_SUBTREE, filter, attrs, &res);
>>
>> if (!ADS_ERR_OK(status)) {
>> goto out;
>> @@ -387,22 +398,27 @@ static PyObject *py_ads_get_gpo_list(ADS *self, PyObject *args, PyObject *kwds)
>> discard_const_p(char *, kwlist),
>> &samaccountname)) {
>> PyErr_SetString(PyExc_SystemError,
>> - "Failed to parse arguments to py_ads_get_gpo_list()");
>> + "Failed to parse arguments to "
>> + "py_ads_get_gpo_list()");
>> goto out;
>> }
>>
>> frame = talloc_stackframe();
>>
>> - status = find_samaccount(self->ads_ptr, frame, samaccountname, &uac, &dn);
>> + status = find_samaccount(self->ads_ptr, frame,
>> + samaccountname, &uac, &dn);
>> if (!ADS_ERR_OK(status)) {
>> TALLOC_FREE(frame);
>> - PyErr_SetString(PyExc_SystemError, "Failed to find samAccountName");
>> + PyErr_SetString(PyExc_SystemError,
>> + "Failed to find samAccountName");
>> goto out;
>> }
>>
>> - if (uac & UF_WORKSTATION_TRUST_ACCOUNT || uac & UF_SERVER_TRUST_ACCOUNT) {
>> + if (uac & UF_WORKSTATION_TRUST_ACCOUNT ||
>> + uac & UF_SERVER_TRUST_ACCOUNT) {
>> flags |= GPO_LIST_FLAG_MACHINE;
>> - status = gp_get_machine_token(self->ads_ptr, frame, dn, &token);
>> + status = gp_get_machine_token(self->ads_ptr, frame, dn,
>> + &token);
>> } else {
>> status = ads_get_sid_token(self->ads_ptr, frame, dn, &token);
>> }
>> @@ -455,7 +471,8 @@ out:
>> static PyMethodDef ADS_methods[] = {
>> { "connect", (PyCFunction)py_ads_connect, METH_NOARGS,
>> "Connect to the LDAP server" },
>> - { "get_gpo_list", (PyCFunction)py_ads_get_gpo_list, METH_KEYWORDS, NULL },
>> + { "get_gpo_list", (PyCFunction)py_ads_get_gpo_list, METH_KEYWORDS,
>> + NULL },
>> { NULL }
>> };
>>
>> @@ -471,7 +488,8 @@ static PyTypeObject ads_ADSType = {
>> };
>>
>> static PyMethodDef py_gpo_methods[] = {
>> - {"gpo_get_sysvol_gpt_version", (PyCFunction) py_gpo_get_sysvol_gpt_version,
>> + {"gpo_get_sysvol_gpt_version",
>> + (PyCFunction)py_gpo_get_sysvol_gpt_version,
>> METH_VARARGS, NULL},
>> {NULL}
>> };
>> --
>> 1.9.1
>>
>>
>> From 52d380f8a09debf639573096ccadfdeb2a30adbd Mon Sep 17 00:00:00 2001
>> From: David Mulder <dmulder at suse.com>
>> Date: Mon, 20 Nov 2017 06:41:19 -0700
>> Subject: [PATCH 5/7] gpo: Fix the empty apply log
>>
>> The apply log wasn't being saved, apparently the pointers to elements
>> of the tree were getting lost.
>>
>> Signed-off-by: David Mulder <dmulder at suse.com>
>> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
>> ---
>> python/samba/gpclass.py | 65 ++++++++++++++++++++++++++++---------------------
>> 1 file changed, 37 insertions(+), 28 deletions(-)
>>
>> diff --git a/python/samba/gpclass.py b/python/samba/gpclass.py
>> index 5a0ca9f..780ef55 100644
>> --- a/python/samba/gpclass.py
>> +++ b/python/samba/gpclass.py
>> @@ -95,10 +95,11 @@ class gp_log:
>> self.gpdb = etree.fromstring(db_log)
>> else:
>> self.gpdb = etree.Element('gp')
>> - self.user = self.gpdb.find('user[@name="%s"]' % user)
>> - if self.user is None:
>> - self.user = etree.SubElement(self.gpdb, 'user')
>> - self.user.attrib['name'] = user
>> + self.user = user
>> + user_obj = self.gpdb.find('user[@name="%s"]' % user)
>> + if user_obj is None:
>> + user_obj = etree.SubElement(self.gpdb, 'user')
>> + user_obj.attrib['name'] = user
>>
>> def state(self, value):
>> ''' Policy application state
>> @@ -113,7 +114,8 @@ class gp_log:
>> '''
>> # If we're enforcing, but we've unapplied, apply instead
>> if value == GPOSTATE.ENFORCE:
>> - apply_log = self.user.find('applylog')
>> + user_obj = self.gpdb.find('user[@name="%s"]' % self.user)
>> + apply_log = user_obj.find('applylog')
>> if apply_log is None or len(apply_log) == 0:
>> self._state = GPOSTATE.APPLY
>> else:
>> @@ -126,14 +128,16 @@ class gp_log:
>> param guid - guid value of the GPO from which we're applying
>> policy
>> '''
>> - self.guid = self.user.find('guid[@value="%s"]' % guid)
>> - if self.guid is None:
>> - self.guid = etree.SubElement(self.user, 'guid')
>> - self.guid.attrib['value'] = guid
>> + self.guid = guid
>> + user_obj = self.gpdb.find('user[@name="%s"]' % self.user)
>> + obj = user_obj.find('guid[@value="%s"]' % guid)
>> + if obj is None:
>> + obj = etree.SubElement(user_obj, 'guid')
>> + obj.attrib['value'] = guid
>> if self._state == GPOSTATE.APPLY:
>> - apply_log = self.user.find('applylog')
>> + apply_log = user_obj.find('applylog')
>> if apply_log is None:
>> - apply_log = etree.SubElement(self.user, 'applylog')
>> + apply_log = etree.SubElement(user_obj, 'applylog')
>> item = etree.SubElement(apply_log, 'guid')
>> item.attrib['count'] = '%d' % (len(apply_log)-1)
>> item.attrib['value'] = guid
>> @@ -145,14 +149,15 @@ class gp_log:
>> Removes the GPO guid last added to the list, which is the most recently
>> applied GPO.
>> '''
>> - apply_log = self.user.find('applylog')
>> + user_obj = self.gpdb.find('user[@name="%s"]' % self.user)
>> + apply_log = user_obj.find('applylog')
>> if apply_log is not None:
>> ret = apply_log.find('guid[@count="%d"]' % (len(apply_log)-1))
>> if ret is not None:
>> apply_log.remove(ret)
>> return ret.attrib['value']
>> - if len(apply_log) == 0 and apply_log in self.user:
>> - self.user.remove(apply_log)
>> + if len(apply_log) == 0 and apply_log in user_obj:
>> + user_obj.remove(apply_log)
>> return None
>>
>> def store(self, gp_ext_name, attribute, old_val):
>> @@ -164,10 +169,12 @@ class gp_log:
>> '''
>> if self._state == GPOSTATE.UNAPPLY or self._state == GPOSTATE.ENFORCE:
>> return None
>> - assert self.guid is not None, "gpo guid was not set"
>> - ext = self.guid.find('gp_ext[@name="%s"]' % gp_ext_name)
>> + user_obj = self.gpdb.find('user[@name="%s"]' % self.user)
>> + guid_obj = user_obj.find('guid[@value="%s"]' % self.guid)
>> + assert guid_obj is not None, "gpo guid was not set"
>> + ext = guid_obj.find('gp_ext[@name="%s"]' % gp_ext_name)
>> if ext is None:
>> - ext = etree.SubElement(self.guid, 'gp_ext')
>> + ext = etree.SubElement(guid_obj, 'gp_ext')
>> ext.attrib['name'] = gp_ext_name
>> attr = ext.find('attribute[@name="%s"]' % attribute)
>> if attr is None:
>> @@ -182,8 +189,10 @@ class gp_log:
>> return - The value of the attribute prior to policy
>> application
>> '''
>> - assert self.guid is not None, "gpo guid was not set"
>> - ext = self.guid.find('gp_ext[@name="%s"]' % gp_ext_name)
>> + user_obj = self.gpdb.find('user[@name="%s"]' % self.user)
>> + guid_obj = user_obj.find('guid[@value="%s"]' % self.guid)
>> + assert guid_obj is not None, "gpo guid was not set"
>> + ext = guid_obj.find('gp_ext[@name="%s"]' % gp_ext_name)
>> if ext is not None:
>> attr = ext.find('attribute[@name="%s"]' % attribute)
>> if attr is not None:
>> @@ -198,12 +207,14 @@ class gp_log:
>> return - list of (attr, value, apply_func) tuples for
>> unapplying policy
>> '''
>> - assert self.guid is not None, "gpo guid was not set"
>> + user_obj = self.gpdb.find('user[@name="%s"]' % self.user)
>> + guid_obj = user_obj.find('guid[@value="%s"]' % self.guid)
>> + assert guid_obj is not None, "gpo guid was not set"
>> ret = []
>> data_maps = {}
>> for gp_ext in gp_extensions:
>> data_maps.update(gp_ext.apply_map())
>> - exts = self.guid.findall('gp_ext')
>> + exts = guid_obj.findall('gp_ext')
>> if exts is not None:
>> for ext in exts:
>> ext_map = {val[0]: val[1] for (key, val) in \
>> @@ -220,21 +231,19 @@ class gp_log:
>> attribute
>> param attribute - attribute to remove
>> '''
>> - assert self.guid is not None, "gpo guid was not set"
>> - ext = self.guid.find('gp_ext[@name="%s"]' % gp_ext_name)
>> + user_obj = self.gpdb.find('user[@name="%s"]' % self.user)
>> + guid_obj = user_obj.find('guid[@value="%s"]' % self.guid)
>> + assert guid_obj is not None, "gpo guid was not set"
>> + ext = guid_obj.find('gp_ext[@name="%s"]' % gp_ext_name)
>> if ext is not None:
>> attr = ext.find('attribute[@name="%s"]' % attribute)
>> if attr is not None:
>> ext.remove(attr)
>> if len(ext) == 0:
>> - self.guid.remove(ext)
>> + guid_obj.remove(ext)
>>
>> def commit(self):
>> ''' Write gp_log changes to disk '''
>> - if len(self.guid) == 0 and self.guid in self.user:
>> - self.user.remove(self.guid)
>> - if len(self.user) == 0 and self.user in self.gpdb:
>> - self.gpdb.remove(self.user)
>> self.gpostore.store(self.username, etree.tostring(self.gpdb, 'utf-8'))
>>
>> class GPOStorage:
>> --
>> 1.9.1
>>
>>
>> From c430d1207ca93fa29dcb1ca766a902bdfdd51322 Mon Sep 17 00:00:00 2001
>> From: David Mulder <dmulder at suse.com>
>> Date: Fri, 1 Dec 2017 11:18:55 -0700
>> Subject: [PATCH 6/7] gpo: Only commit the earliest change to the log
>>
>> Otherwise we overwrite the original value,
>> leaving the setting tattooed on unapplied
>>
>> Signed-off-by: David Mulder <dmulder at suse.com>
>> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
>> ---
>> python/samba/gpclass.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/python/samba/gpclass.py b/python/samba/gpclass.py
>> index 780ef55..00330eb 100644
>> --- a/python/samba/gpclass.py
>> +++ b/python/samba/gpclass.py
>> @@ -180,7 +180,7 @@ class gp_log:
>> if attr is None:
>> attr = etree.SubElement(ext, 'attribute')
>> attr.attrib['name'] = attribute
>> - attr.text = old_val
>> + attr.text = old_val
>>
>> def retrieve(self, gp_ext_name, attribute):
>> ''' Retrieve a stored attribute from the gp_log
>> --
>> 1.9.1
>>
>>
>> From aa012766749d89af6dd3b29ac8c9bb0fbb6579d7 Mon Sep 17 00:00:00 2001
>> From: David Mulder <dmulder at suse.com>
>> Date: Wed, 6 Dec 2017 10:16:11 -0700
>> Subject: [PATCH 7/7] gpo: Test that unapply works
>>
>> Signed-off-by: David Mulder <dmulder at suse.com>
>> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
>> ---
>> source4/scripting/bin/samba_gpoupdate | 4 +-
>> source4/torture/gpo/apply.c | 124 ++++++++++++++++++++++++++--------
>> 2 files changed, 99 insertions(+), 29 deletions(-)
>>
>> diff --git a/source4/scripting/bin/samba_gpoupdate b/source4/scripting/bin/samba_gpoupdate
>> index 4585297..f593e47 100755
>> --- a/source4/scripting/bin/samba_gpoupdate
>> +++ b/source4/scripting/bin/samba_gpoupdate
>> @@ -36,7 +36,7 @@ from samba import smb
>> import logging
>>
>> ''' Fetch the hostname of a writable DC '''
>> -def get_dc_hostname():
>> +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))
>> @@ -52,7 +52,7 @@ def get_gpo_list(dc_hostname, creds, lp):
>>
>> def apply_gp(lp, creds, test_ldb, logger, store, gp_extensions):
>> gp_db = store.get_gplog(creds.get_username())
>> - dc_hostname = get_dc_hostname()
>> + dc_hostname = get_dc_hostname(creds, lp)
>> try:
>> conn = smb.SMB(dc_hostname, 'sysvol', lp=lp, creds=creds)
>> except:
>> diff --git a/source4/torture/gpo/apply.c b/source4/torture/gpo/apply.c
>> index 1d16834..88da0b1 100644
>> --- a/source4/torture/gpo/apply.c
>> +++ b/source4/torture/gpo/apply.c
>> @@ -40,13 +40,13 @@ struct torture_suite *gpo_apply_suite(TALLOC_CTX *ctx)
>> return suite;
>> }
>>
>> -static int exec_wait(const char **cmd)
>> +static int exec_wait(char **cmd)
>> {
>> int ret;
>> pid_t pid = fork();
>> switch (pid) {
>> case 0:
>> - execv(cmd[0], discard_const_p(char *, &(cmd[1])));
>> + execv(cmd[0], &(cmd[1]));
>> ret = -1;
>> break;
>> case -1:
>> @@ -60,7 +60,7 @@ static int exec_wait(const char **cmd)
>> return ret;
>> }
>>
>> -static int unix2nttime(char *sval)
>> +static int unix2nttime(const char *sval)
>> {
>> return (strtoll(sval, NULL, 10) * -1 / 60 / 60 / 24 / 10000000);
>> }
>> @@ -80,6 +80,7 @@ PasswordComplexity = %d\n\
>>
>> bool torture_gpo_system_access_policies(struct torture_context *tctx)
>> {
>> + TALLOC_CTX *ctx = talloc_new(tctx);
>> int ret, vers = 0, i;
>> const char *sysvol_path = NULL, *gpo_dir = NULL;
>> const char *gpo_file = NULL, *gpt_file = NULL;
>> @@ -92,23 +93,25 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
>> "pwdProperties",
>> NULL
>> };
>> - const struct ldb_val *val;
>> 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);
>> torture_assert(tctx, sysvol_path, "Failed to fetch the sysvol path");
>>
>> /* Ensure the sysvol path exists */
>> - gpo_dir = talloc_asprintf(tctx, "%s/%s", sysvol_path, GPODIR);
>> + 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(tctx, "%s/%s", gpo_dir, GPOFILE);
>> + 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);
>> @@ -116,11 +119,11 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
>> "Failed to fetch the gpo update command");
>>
>> /* Open and read the samba db and store the initial password settings */
>> - samdb = samdb_connect(tctx, tctx->ev, tctx->lp_ctx,
>> + 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, tctx, &result, ldb_get_default_basedn(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");
>> @@ -130,14 +133,14 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
>> for (i = 0; i < 3; i++) {
>> /* Write out the sysvol */
>> if ( (fp = fopen(gpo_file, "w")) ) {
>> - fputs(talloc_asprintf(tctx, GPTTMPL, minpwdcases[i],
>> + fputs(talloc_asprintf(ctx, GPTTMPL, minpwdcases[i],
>> maxpwdcases[i], pwdlencases[i],
>> pwdpropcases[i]), fp);
>> fclose(fp);
>> }
>>
>> /* Update the version in the GPT.INI */
>> - gpt_file = talloc_asprintf(tctx, "%s/%s", sysvol_path, GPTINI);
>> + gpt_file = talloc_asprintf(ctx, "%s/%s", sysvol_path, GPTINI);
>> if ( (fp = fopen(gpt_file, "r")) ) {
>> char line[256];
>> while (fgets(line, 256, fp)) {
>> @@ -149,49 +152,116 @@ bool torture_gpo_system_access_policies(struct torture_context *tctx)
>> fclose(fp);
>> }
>> if ( (fp = fopen(gpt_file, "w")) ) {
>> - char *data = talloc_asprintf(tctx, "[General]\nVersion=%d\n",
>> + char *data = talloc_asprintf(ctx,
>> + "[General]\nVersion=%d\n",
>> ++vers);
>> fputs(data, fp);
>> fclose(fp);
>> }
>>
>> /* Run the gpo update command */
>> - ret = exec_wait(gpo_update_cmd);
>> + ret = exec_wait(discard_const_p(char *, gpo_update_cmd));
>> torture_assert(tctx, ret == 0,
>> "Failed to execute the gpo update command");
>>
>> - ret = ldb_search(samdb, tctx, &result, ldb_get_default_basedn(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");
>>
>> /* minPwdAge */
>> - val = ldb_msg_find_ldb_val(result->msgs[0], attrs[0]);
>> - torture_assert(tctx, unix2nttime((char*)val->data) == minpwdcases[i],
>> + torture_assert_int_equal(tctx, unix2nttime(
>> + ldb_msg_find_attr_as_string(
>> + result->msgs[0],
>> + attrs[0],
>> + "")), minpwdcases[i],
>> "The minPwdAge was not applied");
>>
>> /* maxPwdAge */
>> - val = ldb_msg_find_ldb_val(result->msgs[0], attrs[1]);
>> - torture_assert(tctx, unix2nttime((char*)val->data) == maxpwdcases[i],
>> + torture_assert_int_equal(tctx, unix2nttime(
>> + ldb_msg_find_attr_as_string(
>> + result->msgs[0],
>> + attrs[1],
>> + "")), maxpwdcases[i],
>> "The maxPwdAge was not applied");
>>
>> /* minPwdLength */
>> - val = ldb_msg_find_ldb_val(result->msgs[0], attrs[2]);
>> - torture_assert(tctx, atoi((char*)val->data) == pwdlencases[i],
>> - "The minPwdLength was not applied");
>> + torture_assert_int_equal(tctx, ldb_msg_find_attr_as_int(
>> + result->msgs[0],
>> + attrs[2],
>> + -1),
>> + pwdlencases[i],
>> + "The minPwdLength was not applied");
>>
>> /* pwdProperties */
>> - val = ldb_msg_find_ldb_val(result->msgs[0], attrs[3]);
>> - torture_assert(tctx, atoi((char*)val->data) == pwdpropcases[i],
>> + torture_assert_int_equal(tctx, ldb_msg_find_attr_as_int(
>> + result->msgs[0],
>> + attrs[3],
>> + -1),
>> + pwdpropcases[i],
>> "The pwdProperties were not applied");
>> }
>>
>> - for (i = 0; i < old_message->num_elements; i++) {
>> - old_message->elements[i].flags = LDB_FLAG_MOD_REPLACE;
>> + /* 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");
>> + 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 was not unapplied");
>> + /* 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 was not unapplied");
>> + /* 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 was not unapplied");
>> + /* 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 were not unapplied");
>>
>> - ret = ldb_modify(samdb, old_message);
>> - torture_assert(tctx, ret == 0, "Failed to reset password settings.");
>> -
>> + talloc_free(ctx);
>> return true;
>> }
>> --
>> 1.9.1
>>
>
--
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