[PATCHES v4] Fix GPO unapply log
Jeremy Allison
jra at samba.org
Fri Dec 15 19:00:18 UTC 2017
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
>
More information about the samba-technical
mailing list