On (22/07/14 03:13), Yassir Elley wrote:
> On Sun, Jul 20, 2014 at 05:48:45PM -0400, Yassir Elley wrote:
> > Hi,
> >
> > The attached patch adds support for gpo version checking and for
> > conditional downloading of policy files. In order to get that to work,
> > this patch also replaces the deprecated smb functions (e.g. smbc_open,
> > smbc_read, etc) I had been using (not knowing they were deprecated) with
> > the newer smb functions, such as "smbc_getFunctionOpen(smbc_ctx)(smbc_ctx,
> > smb_uri, O_RDONLY, 0755);".
> >
> > Before this patch, the policy files were being unconditionally downloaded
> > every time. After this patch, the backend sends the cached_gpt_version
> > (from a previous transaction, if any) to the gpo_child. The gpo_child
> > downloads the per-GPO GPT.INI file, from which it extracts the
> > sysvol_gpt_version. Only if the sysvol_gpt_version is greater than the
> > cached_gpt_version (or if there is no cached_gpt_version) does the
> > gpo_child download the policy files. The gpo_child returns the
> > sysvol_gpt_version to the backend, which stores it in the cache.
> >
> > Note that, while a GPO has both a SYSVOL-stored Group Policy Template (gpt)
> > version, as well as an LDAP-stored Group Policy Container (gpc) version
> > (which should be equal), it is best practice to use the gpt version for
> > version checking b/c SYSVOL replication is typically much faster than AD
> > replication. In other words, the gpt version is quickly up-to-date on all
> > DCs for a given domain, while the gpc version may be stale for some time
> > after a change is made.
> >
> > In order to keep the gpo_child as simple as possible, the gpo_child does
> > not interact with the sysdb cache at all (only the backend does). However,
> > the gpo_child does need to parse the GPT.INI file in order to determine
> > whether to download the policy files. As such, the gpo_child includes
> > GPT.INI file parsing functionality (which is per-GPO, and not
> > CSE-specific).
> >
> > Note that this patch does not add support for offline mode, which will be
> > implemented in a subsequent patch.
> >
> > Regards,
> > Yassir.
>
Hi Jakub,
I agree with all of your comments so far and have attached revised patches to this email.
I split the original patch into two patches, as you suggested, and I addressed your review
comments in the revised patches. Thanks for the review! Keep the comments coming! :)
Regards,
Yassir.
From f3f00a78102ad5e389266b28c43d0396caf57592 Mon Sep 17 00:00:00
2001
From: Yassir Elley <yelley(a)redhat.com>
Date: Tue, 22 Jul 2014 03:03:03 -0400
Subject: [PATCH 2/2] AD-GPO: only download policy files if gpo version changes
---
src/providers/ad/ad_gpo.c | 306 +++++++++++++++++++-----------
src/providers/ad/ad_gpo_child.c | 404 ++++++++++++++++++++++++++++++++--------
2 files changed, 517 insertions(+), 193 deletions(-)
I have few comments to the ad_gpo_child
diff --git a/src/providers/ad/ad_gpo_child.c
b/src/providers/ad/ad_gpo_child.c
index 22b8b52046b6298f2eeaf425cef7bf0271865fe2..93ae592e591eefe812212cc99ddbec70c4e0aba9
100644
--- a/src/providers/ad/ad_gpo_child.c
+++ b/src/providers/ad/ad_gpo_child.c
@@ -27,6 +27,7 @@
#include <sys/stat.h>
#include <popt.h>
#include <libsmbclient.h>
+#include <ini_configobj.h>
#include <security/pam_modules.h>
#include "util/util.h"
@@ -34,14 +35,17 @@
#include "providers/dp_backend.h"
#include "sss_cli.h"
-#define RIGHTS_SECTION "Privilege Rights"
-#define ALLOW_LOGON_LOCALLY "SeInteractiveLogonRight"
-#define DENY_LOGON_LOCALLY "SeDenyInteractiveLogonRight"
#define SMB_BUFFER_SIZE 65536
+#define GPT_INI "/GPT.INI"
+#define INI_GENERAL_SECTION "General"
+#define GPT_INI_VERSION "Version"
struct input_buffer {
- const char *smb_uri;
- const char *unix_path;
+ int cached_gpt_version;
+ const char *smb_server;
+ const char *smb_share;
+ const char *smb_path;
+ const char *smb_cse_suffix;
};
static errno_t
@@ -51,34 +55,62 @@ unpack_buffer(uint8_t *buf,
{
size_t p = 0;
uint32_t len;
+ uint32_t cached_gpt_version;
- /* smb_uri size and length */
- SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
-
- DEBUG(SSSDBG_TRACE_ALL, "smb_uri size: %d\n", len);
+ /* cached_gpt_version */
+ SAFEALIGN_COPY_UINT32_CHECK(&cached_gpt_version, buf + p, size, &p);
+ DEBUG(SSSDBG_TRACE_FUNC, "cached_gpt_version: %d\n", cached_gpt_version);
+ ibuf->cached_gpt_version = cached_gpt_version;
+ /* smb_server */
+ SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
+ DEBUG(SSSDBG_TRACE_ALL, "smb_server length: %d\n", len);
if (len == 0) {
return EINVAL;
} else {
if ((p + len ) > size) return EINVAL;
- ibuf->smb_uri = talloc_strndup(ibuf, (char *)(buf + p), len);
- if (ibuf->smb_uri == NULL) return ENOMEM;
- DEBUG(SSSDBG_TRACE_ALL, "got smb_uri: %s\n", ibuf->smb_uri);
+ ibuf->smb_server = talloc_strndup(ibuf, (char *)(buf + p), len);
+ if (ibuf->smb_server == NULL) return ENOMEM;
+ DEBUG(SSSDBG_TRACE_ALL, "smb_server: %s\n", ibuf->smb_server);
p += len;
}
- /* unix_path size and length */
+ /* smb_share */
SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
+ DEBUG(SSSDBG_TRACE_ALL, "smb_share length: %d\n", len);
+ if (len == 0) {
+ return EINVAL;
+ } else {
+ if ((p + len ) > size) return EINVAL;
+ ibuf->smb_share = talloc_strndup(ibuf, (char *)(buf + p), len);
+ if (ibuf->smb_share == NULL) return ENOMEM;
+ DEBUG(SSSDBG_TRACE_ALL, "smb_share: %s\n", ibuf->smb_share);
+ p += len;
+ }
- DEBUG(SSSDBG_TRACE_ALL, "unix_path size: %d\n", len);
+ /* smb_path */
+ SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
+ DEBUG(SSSDBG_TRACE_ALL, "smb_path length: %d\n", len);
+ if (len == 0) {
+ return EINVAL;
+ } else {
+ if ((p + len ) > size) return EINVAL;
+ ibuf->smb_path = talloc_strndup(ibuf, (char *)(buf + p), len);
+ if (ibuf->smb_path == NULL) return ENOMEM;
+ DEBUG(SSSDBG_TRACE_ALL, "smb_path: %s\n", ibuf->smb_path);
+ p += len;
+ }
+ /* smb_cse_suffix */
+ SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p);
+ DEBUG(SSSDBG_TRACE_ALL, "smb_cse_suffix length: %d\n", len);
if (len == 0) {
return EINVAL;
} else {
if ((p + len ) > size) return EINVAL;
- ibuf->unix_path = talloc_strndup(ibuf, (char *)(buf + p), len);
- if (ibuf->unix_path == NULL) return ENOMEM;
- DEBUG(SSSDBG_TRACE_ALL, "got unix_path: %s\n", ibuf->unix_path);
+ ibuf->smb_cse_suffix = talloc_strndup(ibuf, (char *)(buf + p), len);
+ if (ibuf->smb_cse_suffix == NULL) return ENOMEM;
+ DEBUG(SSSDBG_TRACE_ALL, "smb_cse_suffix: %s\n",
ibuf->smb_cse_suffix);
p += len;
}
@@ -88,14 +120,16 @@ unpack_buffer(uint8_t *buf,
static errno_t
pack_buffer(struct response *r,
+ int sysvol_gpt_version,
int result)
{
size_t p = 0;
/* A buffer with the following structure must be created:
+ * uint32_t sysvol_gpt_version (required)
* uint32_t status of the request (required)
*/
- r->size = sizeof(uint32_t);
+ r->size = 2 * sizeof(uint32_t);
r->buf = talloc_array(r, uint8_t, r->size);
if(r->buf == NULL) {
@@ -104,6 +138,9 @@ pack_buffer(struct response *r,
DEBUG(SSSDBG_TRACE_FUNC, "result [%d]\n", result);
+ /* sysvol_gpt_version */
+ SAFEALIGN_SET_UINT32(&r->buf[p], sysvol_gpt_version, &p);
+
/* result */
SAFEALIGN_SET_UINT32(&r->buf[p], result, &p);
@@ -112,6 +149,7 @@ pack_buffer(struct response *r,
static errno_t
prepare_response(TALLOC_CTX *mem_ctx,
+ int sysvol_gpt_version,
int result,
struct response **rsp)
{
@@ -126,14 +164,14 @@ prepare_response(TALLOC_CTX *mem_ctx,
r->buf = NULL;
r->size = 0;
- ret = pack_buffer(r, result);
+ ret = pack_buffer(r, sysvol_gpt_version, result);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "pack_buffer failed\n");
return ret;
}
*rsp = r;
- DEBUG(SSSDBG_TRACE_FUNC, "r->size: %zu\n", r->size);
+ DEBUG(SSSDBG_TRACE_ALL, "r->size: %zu\n", r->size);
return EOK;
}
@@ -153,12 +191,12 @@ sssd_krb_get_auth_data_fn(const char * pServer,
/*
* This function prepares the gpo_cache by:
- * - parsing the input_unix_path into its component directories
+ * - parsing the input_smb_path into its component directories
* - creating each component directory (if it doesn't already exist)
*/
static errno_t prepare_gpo_cache(TALLOC_CTX *mem_ctx,
const char *cache_dir,
- const char *input_unix_path)
+ const char *input_smb_path_with_suffix)
{
char *current_dir;
char *ptr;
@@ -167,25 +205,27 @@ static errno_t prepare_gpo_cache(TALLOC_CTX *mem_ctx,
int i;
char *first = NULL;
char *last = NULL;
- char *unix_path = NULL;
+ char *smb_path_with_suffix = NULL;
- unix_path = talloc_strdup(mem_ctx, input_unix_path);
- if (unix_path == NULL) {
+ smb_path_with_suffix = talloc_strdup(mem_ctx, input_smb_path_with_suffix);
+ if (smb_path_with_suffix == NULL) {
return ENOMEM;
}
+ DEBUG(SSSDBG_TRACE_ALL, "smb_path_with_suffix: %s\n",
smb_path_with_suffix);
+
current_dir = talloc_strdup(mem_ctx, cache_dir);
if (current_dir == NULL) {
return ENOMEM;
}
- ptr = unix_path + 1;
+ ptr = smb_path_with_suffix + 1;
while ((ptr = strchr(ptr, delim))) {
ptr++;
num_dirs++;
}
- ptr = unix_path + 1;
+ ptr = smb_path_with_suffix + 1;
for (i = 0; i < num_dirs; i++) {
first = ptr;
@@ -210,12 +250,16 @@ static errno_t prepare_gpo_cache(TALLOC_CTX *mem_ctx,
/*
* This function stores the input buf to a local file, whose file path
- * is constructed by concatenating GPO_CACHE_PATH to the input unix_path.
- * Note that the backend will later read the policy file from the same file path.
+ * is constructed by concatenating:
+ * GPO_CACHE_PATH,
+ * input smb_path,
+ * input smb_cse_suffix
+ * Note that the backend will later read the file from the same file path.
*/
-static errno_t store_bytes_in_gpo_cache(const char *unix_path,
- uint8_t *buf,
- int buflen)
+static errno_t gpo_cache_store_file(const char *smb_path,
+ const char *smb_cse_suffix,
+ uint8_t *buf,
+ int buflen)
{
int ret;
int fd = -1;
@@ -223,6 +267,7 @@ static errno_t store_bytes_in_gpo_cache(const char *unix_path,
ssize_t written;
mode_t old_umask;
char *filename = NULL;
+ char *smb_path_with_suffix = NULL;
TALLOC_CTX *tmp_ctx = NULL;
tmp_ctx = talloc_new(NULL);
@@ -231,10 +276,18 @@ static errno_t store_bytes_in_gpo_cache(const char *unix_path,
goto done;
}
- /* create component directories of unix_path, if needed */
- ret = prepare_gpo_cache(tmp_ctx, GPO_CACHE_PATH, unix_path);
+ smb_path_with_suffix =
+ talloc_asprintf(tmp_ctx, "%s%s", smb_path, smb_cse_suffix);
+ if (smb_path_with_suffix == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
- filename = talloc_asprintf(tmp_ctx, GPO_CACHE_PATH"%s", unix_path);
+ /* create component directories of smb_path, if needed */
+ ret = prepare_gpo_cache(tmp_ctx, GPO_CACHE_PATH, smb_path_with_suffix);
^^^
return code is ignored. It may cause problems if prepare_gpo_cache fail.
+
+ filename = talloc_asprintf(tmp_ctx, GPO_CACHE_PATH"%s",
smb_path_with_suffix);
tmp_name = talloc_asprintf(tmp_ctx, "%sXXXXXX", filename);
if (tmp_name == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf failed.\n");
@@ -303,21 +356,132 @@ static errno_t store_bytes_in_gpo_cache(const char *unix_path,
return ret;
}
+static errno_t
+parse_ini_file_with_libini(struct ini_cfgobj *ini_config,
+ int *_gpt_version)
+{
+ int ret = 0;
+ struct value_obj *vobj = NULL;
+ int gpt_version;
+
+ ret = ini_get_config_valueobj(INI_GENERAL_SECTION, GPT_INI_VERSION,
+ ini_config, INI_GET_FIRST_VALUE, &vobj);
+ if (ret != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "ini_get_config_valueobj failed [%d][%s]\n", ret,
strerror(ret));
+ goto done;
+ }
+ if (vobj == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "section/name not found: [%s][%s]\n",
+ INI_GENERAL_SECTION, GPT_INI_VERSION);
+ ret = EINVAL;
+ goto done;
+ }
+
+ gpt_version = ini_get_int32_config_value(vobj, 0, -1, &ret);
+ if (ret != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "ini_get_int32_config_value failed [%d][%s]\n",
+ ret, strerror(ret));
+ goto done;
+ }
+
+ *_gpt_version = gpt_version;
+
+ ret = EOK;
+
+ done:
+
+ return ret;
+}
+
+/*
+ * This function parses the GPT_INI file stored in the gpo_cache, and uses the
+ * results to populate the output parameters ...
+ */
+static errno_t
+ad_gpo_parse_ini_file(const char *smb_path,
+ int *_gpt_version)
+{
+ struct ini_cfgfile *file_ctx = NULL;
+ struct ini_cfgobj *ini_config = NULL;
+ const char *ini_filename;
+ int ret;
+ int gpt_version;
+ TALLOC_CTX *tmp_ctx = NULL;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ini_filename = talloc_asprintf(tmp_ctx, GPO_CACHE_PATH"%s%s",
+ smb_path, GPT_INI);
could you test if
allocation was successful?
+
+ DEBUG(SSSDBG_TRACE_FUNC, "ini_filename:%s\n", ini_filename);
+
+
+ ret = ini_config_create(&ini_config);
+ if (ret != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "ini_config_create failed [%d][%s]\n", ret, strerror(ret));
+ goto done;
+ }
+
+ ret = ini_config_file_open(ini_filename, 0, &file_ctx);
+ if (ret != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "ini_config_file_open failed [%d][%s]\n", ret, strerror(ret));
+ goto done;
+ }
+
+ ret = ini_config_parse(file_ctx, INI_STOP_ON_NONE, 0, 0, ini_config);
+ if (ret != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "ini_config_parse failed [%d][%s]\n", ret, strerror(ret));
+ goto done;
+ }
+
+ ret = parse_ini_file_with_libini(ini_config, &gpt_version);
+ if (ret != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "parse_ini_file_with_libini failed [%d][%s]\n",
+ ret, strerror(ret));
+ goto done;
+ }
+
+ *_gpt_version = gpt_version;
+
+ done:
+
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Error encountered: %d.\n", ret);
+ }
+
+ ini_config_file_destroy(file_ctx);
+ ini_config_destroy(ini_config);
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
/*
- * This cse-specific function (GP_EXT_GUID_SECURITY) opens an SMB connection,
- * retrieves the data referenced by the input smb_uri, and then closes the SMB
- * connection. The data is then written to a file in the GPO_CACHE directory,
- * to be read by the backend.
+ * This function uses the input smb uri components to download a sysvol file
+ * (e.g. INI file, policy file, etc) and store it to the GPO_CACHE directory.
*/
static errno_t
-process_security_settings_cse(TALLOC_CTX *mem_ctx,
- const char *smb_uri,
- const char *unix_path)
+copy_smb_file_to_gpo_cache(SMBCCTX *smbc_ctx,
+ const char *smb_server,
+ const char *smb_share,
+ const char *smb_path,
+ const char *smb_cse_suffix)
{
- SMBCCTX *context;
+ char *smb_uri = NULL;
+ SMBCFILE *file;
int ret = 0;
uint8_t *buf = NULL;
int buflen = 0;
+
TALLOC_CTX *tmp_ctx = NULL;
tmp_ctx = talloc_new(NULL);
@@ -325,59 +489,131 @@ process_security_settings_cse(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
- DEBUG(SSSDBG_TRACE_ALL, "%s\n", smb_uri);
- DEBUG(SSSDBG_TRACE_ALL, "%s\n", unix_path);
+ smb_uri = talloc_asprintf(tmp_ctx, "%s%s%s%s", smb_server,
+ smb_share, smb_path, smb_cse_suffix);
+ DEBUG(SSSDBG_TRACE_FUNC, "smb_uri: %s\n", smb_uri);
- context = smbc_new_context();
- if (context == NULL) {
+ errno = 0;
+ file = smbc_getFunctionOpen(smbc_ctx)(smbc_ctx, smb_uri, O_RDONLY, 0755);
+ if (file == NULL) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, "smbc_getFunctionOpen failed [%d][%s]\n",
+ ret, strerror(ret));
+ goto done;
+ }
+
+ buf = talloc_array(tmp_ctx, uint8_t, SMB_BUFFER_SIZE);
+
+ errno = 0;
+ buflen = smbc_getFunctionRead(smbc_ctx)(smbc_ctx, file, buf, SMB_BUFFER_SIZE);
+ if(buflen < 0) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, "smbc_getFunctionRead failed [%d][%s]\n",
+ ret, strerror(ret));
+ goto done;
+ }
+
+ DEBUG(SSSDBG_TRACE_ALL, "smb_buflen: %d\n", buflen);
+
+ ret = gpo_cache_store_file(smb_path, smb_cse_suffix, buf, buflen);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "gpo_cache_store_file failed [%d][%s]\n",
+ ret, strerror(ret));
+ goto done;
+ }
+
+ done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
+
+/*
+ * Using its smb_uri components and cached_gpt_version inputs, this function
+ * does several things:
+ * - it downloads the GPT_INI file to GPO_CACHE
+ * - it parses the sysvol_gpt_version field from the GPT_INI file
+ * - if the sysvol_gpt_version is greater than the cached_gpt_version
+ * - it downloads the policy file to GPO_CACHE
+ * - else
+ * - it doesn't retrieve the policy file
+ * - in this case, the backend will use the existing policy file in GPO_CACHE
+ * - it returns the sysvol_gpt_version in the _sysvol_gpt_version output param
+ *
+ * Note that if the cached_gpt_version sent by the backend is -1 (to indicate
+ * that no gpt_version has been set in the cache for the corresponding gpo_guid),
+ * then the parsed sysvol_gpt_version (which must be at least 0) will be greater
+ * than the cached_gpt_version, thereby triggering a fresh download.
+ *
+ * Note that the backend will later do the following:
+ * - backend will save the the sysvol_gpt_version to sysdb cache
+ * - backend will read the policy file from the GPO_CACHE
+ */
+static errno_t
+perform_smb_operations(int cached_gpt_version,
+ const char *smb_server,
+ const char *smb_share,
+ const char *smb_path,
+ const char *smb_cse_suffix,
+ int *_sysvol_gpt_version)
+{
+ SMBCCTX *smbc_ctx;
+ int ret = 0;
I don't think we need to initialise ret to zero. From
defensive point of
view it would be better to init with EINVAL. I think zero can be removed.
+ int sysvol_gpt_version;
+
+ smbc_ctx = smbc_new_context();
+ if (smbc_ctx == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "Could not allocate new smbc context\n");
ret = ENOMEM;
goto done;
}
- smbc_setFunctionAuthData(context, sssd_krb_get_auth_data_fn);
- smbc_setOptionUseKerberos(context, 1);
+ smbc_setFunctionAuthData(smbc_ctx, sssd_krb_get_auth_data_fn);
+ smbc_setOptionUseKerberos(smbc_ctx, 1);
/* Initialize the context using the previously specified options */
- if (smbc_init_context(context) == NULL) {
+ if (smbc_init_context(smbc_ctx) == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "Could not initialize smbc context\n");
ret = ENOMEM;
goto done;
}
- /* Tell the compatibility layer to use this context */
- smbc_set_context(context);
-
- int remotehandle = smbc_open(smb_uri, O_RDONLY, 0755);
- if (remotehandle < 0) {
- DEBUG(SSSDBG_CRIT_FAILURE, "smbc_open failed\n");
- ret = EPIPE;
- goto done;
- }
-
- buf = talloc_array(tmp_ctx, uint8_t, SMB_BUFFER_SIZE);
- buflen = smbc_read(remotehandle, buf, SMB_BUFFER_SIZE);
- if(buflen < 0) {
- DEBUG(SSSDBG_CRIT_FAILURE, "smbc_read failed\n");
- ret = EPIPE;
- goto done;
- }
-
- DEBUG(SSSDBG_CRIT_FAILURE, "buflen: %d\n", buflen);
-
- smbc_close(remotehandle);
-
- ret = store_bytes_in_gpo_cache(unix_path, buf, buflen);
+ /* download ini file */
+ ret = copy_smb_file_to_gpo_cache(smbc_ctx, smb_server, smb_share, smb_path,
+ GPT_INI);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
- "store_bytes_in_gpo_cache failed [%d][%s]\n",
+ "copy_smb_file_to_gpo_cache failed [%d][%s]\n",
ret, strerror(ret));
goto done;
}
+ ret = ad_gpo_parse_ini_file(smb_path, &sysvol_gpt_version);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Cannot parse ini file: [%d][%s]\n", ret, strerror(ret));
+ goto done;
+ }
+
+ DEBUG(SSSDBG_TRACE_FUNC, "sysvol_gpt_version: %d\n", sysvol_gpt_version);
+
+ if (sysvol_gpt_version > cached_gpt_version) {
+ /* download policy file */
+ ret = copy_smb_file_to_gpo_cache(smbc_ctx, smb_server, smb_share,
+ smb_path, smb_cse_suffix);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "copy_smb_file_to_gpo_cache failed [%d][%s]\n",
+ ret, strerror(ret));
+ goto done;
+ }
+ }
+
+ *_sysvol_gpt_version = sysvol_gpt_version;
+
done:
- smbc_free_context(context, 0);
- talloc_free(tmp_ctx);
+ smbc_free_context(smbc_ctx, 0);
return ret;
}
@@ -388,6 +624,7 @@ main(int argc, const char *argv[])
poptContext pc;
int debug_fd = -1;
errno_t ret;
+ int sysvol_gpt_version;
int result;
TALLOC_CTX *main_ctx = NULL;
uint8_t *buf = NULL;
@@ -484,18 +721,21 @@ main(int argc, const char *argv[])
goto fail;
}
- DEBUG(SSSDBG_TRACE_FUNC, "processing security settings\n");
+ DEBUG(SSSDBG_TRACE_FUNC, "performing smb operations\n");
- result = process_security_settings_cse(main_ctx,
- ibuf->smb_uri,
- ibuf->unix_path);
+ result = perform_smb_operations(ibuf->cached_gpt_version,
+ ibuf->smb_server,
+ ibuf->smb_share,
+ ibuf->smb_path,
+ ibuf->smb_cse_suffix,
+ &sysvol_gpt_version);
if (result != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
- "process_security_settings_cse failed.[%d][%s].\n",
+ "perform_smb_operations failed.[%d][%s].\n",
result, strerror(result));
missing goto fail.
This is a reason of 1st warning.
src/providers/ad/ad_gpo_child.c:142:34: error: ‘sysvol_gpt_version’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
SAFEALIGN_SET_UINT32(&r->buf[p], sysvol_gpt_version, &p);
^
src/providers/ad/ad_gpo_child.c:627:9: note: ‘sysvol_gpt_version’ was declared here
int sysvol_gpt_version;
}
There is also another gcc warning. It is only visible with -O1 or higher.
The same warning is with ferao 20/rawhide and rhel6
src/providers/ad/ad_gpo_child.c: In function ‘main’:
src/providers/ad/ad_gpo_child.c:142:34: error: ‘gpt_version’ may be used uninitialized
in this function [-Werror=maybe-uninitialized]
SAFEALIGN_SET_UINT32(&r->buf[p], sysvol_gpt_version, &p);
^
src/providers/ad/ad_gpo_child.c:413:9: note: ‘gpt_version’ was declared here
int gpt_version;
^
cc1: all warnings being treated as errors
There are two possibel solutions:
a) explicit initialisation to unexpected value.
--- a/src/providers/ad/ad_gpo_child.c
+++ b/src/providers/ad/ad_gpo_child.c
@@ -410,7 +410,7 @@ ad_gpo_parse_ini_file(const char *smb_path,
struct ini_cfgobj *ini_config = NULL;
const char *ini_filename;
int ret;
- int gpt_version;
+ int gpt_version = -1;
TALLOC_CTX *tmp_ctx = NULL;
b) disable inlining of parse_ini_file_with_libini.
--- a/src/providers/ad/ad_gpo_child.c
+++ b/src/providers/ad/ad_gpo_child.c
@@ -359,7 +359,7 @@ static errno_t gpo_cache_store_file(const char *smb_path,
return ret;
}
-static errno_t
+errno_t
parse_ini_file_with_libini(struct ini_cfgobj *ini_config,
int *_gpt_version)
{
LS