URL: https://github.com/SSSD/sssd/pull/412 Author: mzidek-rh Title: #412: Gpo contributed patches Action: opened
PR body: """ Hi, I am moving the patches from this PR: https://pagure.io/SSSD/sssd/pull-request/3320 Here on GH. The issue was stalled for a long time, and I do not think it is good idea to ask for changes in the patches after 7 months. So I decided to immediately change all the problems I found and squshed them into the patches in this PR. There were no big issues with the patches: - I rebased it on top of the current master - added missing include for toupper function - the debug message format was not the same as we use on other places (note that this change was difficult to squash for each patch, so not all patches are 'clean', because I squashed all the debug message issues to the GPO: Improve logging of GPO... patch, however the result after applying all patches is as expected) - I added some comments - there were some memory leaks (missing frees) Notice that I am not the author of the patches, only reviewer. The patches as they are in this PR LGTM. I tested them to check if something is broken (manually.. sigh) and now I am waiting for the CI results. There was one issue occuring when I tested these patches and I originally though the is caused by these patches, but now I know it is not. The issue is that I did not have GPOs distributed to all DCs and it looks like SSSD randomly picks one DC and contacts it (even though it was DC from subdomain) so it could not always find all the GPOs (it looked for them on wrong DCs, where they were not). Other then that I encountered no issues with the patches, so it is ACK from me (tentative). However since I am not native speaker and the last patch is MAN page change, I would appreciate second pair of eyes for that patch. Second pair of eyes for the rest of the patches would also be appreciated :) However if nobody raises any issues I will ACK it soon (given the tests will pass). """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/412/head:pr412 git checkout pr412
From 8af12e988a54813d940fd4a85e8f4a9f13095b00 Mon Sep 17 00:00:00 2001 From: REIM THOMAS <rei...@gmail.com> Date: Mon, 27 Feb 2017 11:35:19 +0100 Subject: [PATCH 1/6] GPO: Grant access if DACL is not present We falsely stopped GPO processing when Group Policy Container in AD did not contain a DACL or "DACL Present" bit was not set. Such GPOs are considered to be applicable according to MS-ADTS: https://msdn.microsoft.com/en-us/library/cc223518.aspx. Resolves: https://pagure.io/SSSD/sssd/issue/3324 Signed-off-by: REIM THOMAS <rei...@gmail.com> --- src/providers/ad/ad_gpo.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index a5237f6fa..bdfffe6b2 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -893,23 +893,22 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, continue; } - /* - * [MS-ADTS] 5.1.3.3.4: - * If the security descriptor has no DACL or its "DACL Present" bit - * is not set, then grant requester the requested control access right. - */ + if (((sd->type & SEC_DESC_DACL_PRESENT)) && (dacl != NULL)) { + ret = ad_gpo_evaluate_dacl(dacl, idmap_ctx, user_sid, group_sids, + group_size, &access_allowed); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, "Could not determine if GPO is applicable\n"); + continue; + } + } else { + /* + * [MS-ADTS] 5.1.3.3.4: + * If the security descriptor has no DACL or its "DACL Present" bit + * is not set, then grant requester the requested control access right. + */ - if ((!(sd->type & SEC_DESC_DACL_PRESENT)) || (dacl == NULL)) { DEBUG(SSSDBG_TRACE_ALL, "DACL is not present\n"); access_allowed = true; - break; - } - - ret = ad_gpo_evaluate_dacl(dacl, idmap_ctx, user_sid, group_sids, - group_size, &access_allowed); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, "Could not determine if GPO is applicable\n"); - continue; } if (access_allowed) { From ff66079de193a0043528c067905e89a11d49ef5b Mon Sep 17 00:00:00 2001 From: REIM THOMAS <rei...@gmail.com> Date: Fri, 3 Mar 2017 12:40:56 +0100 Subject: [PATCH 2/6] GPO: Support group policy file main folders with upper case name There are AD domain controller implementations that use upper case names for the main folder on SYSVOL under which group policy files and templates are stored. E. g. 'MACHINE' instead of 'Machine'. gpo_child uses library libsmbclient to copy group policy files from the AD domain controller into a local GPO cache directory. libsmbclient does not allow to request the domain controller to perform case insensitive SMB URI lookups, if SYSVOL is located on a case sensitive file system. If a group policy template is stored under main folder 'MACHINE' gpo_child cannot retrieve the policy data and exits with error code 2 (No such file or directory). GPO based access control fails with error 22 (Invalid argument) and users may not be able to login. GP_EXT_GUID_SECURITY_SUFFIX constant defines a case sensitive main folder name (/Machine/Microsoft/Windows NT/SecEdit/GptTmpl.inf) for the policy template to retrieve. If the group policy file cannot be retrieved, gpo_child will now also try to retrieve the file using an upper case main folder name, i. e. /MACHINE/Microsoft/Windows NT/SecEdit/GptTmpl.inf. Resolves: https://pagure.io/SSSD/sssd/issue/3324 Signed-off-by: REIM THOMAS <rei...@gmail.com> --- src/providers/ad/ad_gpo_child.c | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/src/providers/ad/ad_gpo_child.c b/src/providers/ad/ad_gpo_child.c index 8e5e06254..40fee6579 100644 --- a/src/providers/ad/ad_gpo_child.c +++ b/src/providers/ad/ad_gpo_child.c @@ -23,6 +23,7 @@ */ #include <sys/types.h> +#include <ctype.h> #include <unistd.h> #include <sys/stat.h> #include <popt.h> @@ -529,6 +530,7 @@ copy_smb_file_to_gpo_cache(SMBCCTX *smbc_ctx, const char *smb_cse_suffix) { char *smb_uri = NULL; + char *gpt_main_folder = NULL; SMBCFILE *file; int ret; uint8_t *buf = NULL; @@ -554,10 +556,38 @@ copy_smb_file_to_gpo_cache(SMBCCTX *smbc_ctx, 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; + /* + * DCs may use upper case names for the main folder, where GPTs are + * stored. libsmbclient does not allow us to request case insensitive + * file name lookups on DCs with case sensitive file systems. + */ + gpt_main_folder = strstr(smb_uri, "/Machine/"); + if (gpt_main_folder == NULL) { + /* At this moment we do not use any GPO from user settings, + * but it can change in the furute so let's keep the follwing + * line around to make this part of the code 'just work' also + * with the user GPO settings. */ + gpt_main_folder = strstr(smb_uri, "/User/"); + } + if (gpt_main_folder != NULL) { + ++gpt_main_folder; + while (gpt_main_folder != NULL && *gpt_main_folder != '/') { + *gpt_main_folder = toupper(*gpt_main_folder); + ++gpt_main_folder; + } + + DEBUG(SSSDBG_TRACE_FUNC, "smb_uri: %s\n", smb_uri); + + 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); From 4ab1cc729ea674d1c31674d8b3edf29dd7fe8140 Mon Sep 17 00:00:00 2001 From: REIM THOMAS <rei...@gmail.com> Date: Fri, 3 Mar 2017 12:40:56 +0100 Subject: [PATCH 3/6] GPO: Close group policy file after copying The SMB protocol sequence for copying the content of group policy files should be: - smbc_getFunctionOpen() - smbc_getFunctionRead() - smbc_getFunctionClose(). Inform the AD server, that we do not need further access to a policy file after we have copied its content. Resolves: https://pagure.io/SSSD/sssd/issue/3324 Signed-off-by: REIM THOMAS <rei...@gmail.com> --- src/providers/ad/ad_gpo_child.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/providers/ad/ad_gpo_child.c b/src/providers/ad/ad_gpo_child.c index 40fee6579..40442997b 100644 --- a/src/providers/ad/ad_gpo_child.c +++ b/src/providers/ad/ad_gpo_child.c @@ -608,6 +608,8 @@ copy_smb_file_to_gpo_cache(SMBCCTX *smbc_ctx, DEBUG(SSSDBG_TRACE_ALL, "smb_buflen: %d\n", buflen); + smbc_getFunctionClose(smbc_ctx)(smbc_ctx, file); + ret = gpo_cache_store_file(smb_path, smb_cse_suffix, buf, buflen); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, From 91840f17555dd351b23d44c2aecdc95e08db98c3 Mon Sep 17 00:00:00 2001 From: REIM THOMAS <rei...@gmail.com> Date: Fri, 3 Mar 2017 13:46:36 +0100 Subject: [PATCH 4/6] GPO: Group policy access evaluation not in line with [MS-ADTS] The implemented security ACE evaluation algorithm is too strict and does not meet Microsoft technical specifications: Security access rights for a group policy object may be split into several access control entries (ACE). The implemented algorithm does not consider this and denies access to GPOs, where the "ApplyGroupPolicy" (AGP) ACE is preceded by a standard access rights ACE. The algorithm also denies access, if the AGP ACE is preceded by other extended object ACEs. Update security access right evaluation algorithms to be in line with the applicable Microsoft technical specifications: - Add a simple evaluation algorithm to check standard access rights for the complete GPO ([MS-ADTS] 5.1.3.3.2 and [MS-GOPD] 2.4): The requester must have been granted read access (RIGHT_DS_READ_PROPERTY) to the properties of the GPO - Fix the "ApplyGroupPolicy" evaluation algorithm to be in line with [MS-ADTS] 5.1.3.3.4 Further improve debug messages during security filtering for administrators to figure out why access to a GPO was denied: - Inform administrators when a GPO with applicable AGP access right has not been evaluated due to missing or denied read access. - Show the trustee's SID that specifies the particular user or group for which GPO access has been denied - Align message content to Microsoft tool like Gpresult Resolves: https://pagure.io/SSSD/sssd/issue/3324 Signed-off-by: REIM THOMAS <rei...@gmail.com> --- src/providers/ad/ad_gpo.c | 253 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 200 insertions(+), 53 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index bdfffe6b2..c9a98060d 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -134,7 +134,7 @@ struct gp_gpo { const char *policy_filename; }; -enum ace_eval_status { +enum ace_eval_agp_status { AD_GPO_ACE_DENIED, AD_GPO_ACE_ALLOWED, AD_GPO_ACE_NEUTRAL @@ -676,40 +676,55 @@ ad_gpo_ace_includes_client_sid(const char *user_sid, } /* - * This function determines whether use of the extended right - * named "ApplyGroupPolicy" (AGP) is allowed, by comparing the specified - * user_sid and group_sids against the specified access control entry (ACE). + * This function determines whether use of the extended right named + * "ApplyGroupPolicy" (AGP) is allowed for the GPO, by comparing the + * specified user_sid and group_sids against the passed access control + * entry (ACE). * This function returns ALLOWED, DENIED, or NEUTRAL depending on whether - * the ACE explictly allows, explicitly denies, or does neither. + * the ACE explicitly allows, explicitly denies, or does neither. * - * Note that the 'M' abbreviation used in the evaluation algorithm stands for - * "access_mask", which represents the set of access rights associated with an - * individual ACE. The access right of interest to the GPO code is + * Notes: + * (1) Abbreviation 'M' used in the evaluation algorithm stands for + * "access_mask", which represents the set of access rights associated with + * the passed ACE. The access right of interest to the GPO code is * RIGHT_DS_CONTROL_ACCESS, which serves as a container for all control access * rights. The specific control access right is identified by a GUID in the * ACE's ObjectType. In our case, this is the GUID corresponding to AGP. + * (2) ACE that require an evaluation algorithm different from [MS-ADTS] + * 5.1.3.3.4, e. g. RIGHT_DS_CONTROL_ACCESS (CR) is not present in M, are + * ignored. * * The ACE evaluation algorithm is specified in [MS-ADTS] 5.1.3.3.4: - * - Deny access by default - * - If the "Inherit Only" (IO) flag is set in the ACE, skip the ACE. - * - If the SID in the ACE does not match any SID in the requester's - * security context, skip the ACE - * - If the ACE type is "Object Access Allowed", the access right - * RIGHT_DS_CONTROL_ACCESS (CR) is present in M, and the ObjectType - * field in the ACE is either not present OR contains a GUID value equal - * to AGP, then grant requested control access right. Stop access checking. - * - If the ACE type is "Object Access Denied", the access right - * RIGHT_DS_CONTROL_ACCESS (CR) is present in M, and the ObjectType - * field in the ACE is either not present OR contains a GUID value equal to - * AGP, then deny the requested control access right. Stop access checking. + * Evaluate the DACL by examining each ACE in sequence, starting with the first + * ACE. Perform the following sequence of actions for each ACE in the order as + * shown: + * 1. If the "Inherit Only" (IO) flag is set in the ACE, skip the ACE. + * 2. If the SID in the ACE does not match any SID in the requester's + * security context, skip the ACE. + * 3. If the ACE type is "Object Access Allowed", the access right + * RIGHT_DS_CONTROL_ACCESS (CR) is present in M, and the ObjectType + * field in the ACE is not present, then grant the requested control + * access right. Stop any further access checks. + * 4. If the ACE type is "Object Access Allowed" the access right + * RIGHT_DS_CONTROL_ACCESS (CR) is present in M, and the ObjectType + * field in the ACE contains a GUID value equal to AGP, then grant + * the requested control access right. Stop any further access checks. + * 5. If the ACE type is "Object Access Denied", the access right + * RIGHT_DS_CONTROL_ACCESS (CR) is present in M, and the ObjectType + * field in the ACE is not present, then deny the requested control + * access right. Stop any further access checks. + * 6. If the ACE type is "Object Access Denied" the access right + * RIGHT_DS_CONTROL_ACCESS (CR) is present in M, and the ObjectType + * field in the ACE contains a GUID value equal to AGP, then deny + * the requested control access right. Stop any further access checks. */ -static enum ace_eval_status ad_gpo_evaluate_ace(struct security_ace *ace, - struct sss_idmap_ctx *idmap_ctx, - const char *user_sid, - const char **group_sids, - int group_size) +static enum ace_eval_agp_status +ad_gpo_evaluate_agp_ace(struct security_ace *ace, + struct sss_idmap_ctx *idmap_ctx, + const char *user_sid, + const char **group_sids, + int group_size) { - bool agp_included = false; bool included = false; int ret = 0; struct security_ace_object object; @@ -730,36 +745,100 @@ static enum ace_eval_status ad_gpo_evaluate_ace(struct security_ace *ace, return AD_GPO_ACE_NEUTRAL; } - object = ace->object.object; - GUID_from_string(AD_AGP_GUID, &ext_right_agp_guid); - - if (object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) { - if (GUID_equal(&object.type.type, &ext_right_agp_guid)) { - agp_included = true; - } - } else { - agp_included = false; - } - if (ace->access_mask & SEC_ADS_CONTROL_ACCESS) { - if (agp_included) { - if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { - return AD_GPO_ACE_ALLOWED; - } else if (ace->type == SEC_ACE_TYPE_ACCESS_DENIED_OBJECT) { - return AD_GPO_ACE_DENIED; + object = ace->object.object; + if (object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) { + GUID_from_string(AD_AGP_GUID, &ext_right_agp_guid); + if (!GUID_equal(&object.type.type, &ext_right_agp_guid)) { + return AD_GPO_ACE_NEUTRAL; } } + if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) { + return AD_GPO_ACE_ALLOWED; + } else if (ace->type == SEC_ACE_TYPE_ACCESS_DENIED_OBJECT) { + return AD_GPO_ACE_DENIED; + } + } + + return AD_GPO_ACE_NEUTRAL; +} + +/* + * This function evaluates, which standard access rights the passed access + * control entry (ACE) allows or denies for the entire GPO. + * + * Notes: + * (1) Abbreviation 'M' used in the evaluation algorithm stands for + * "access_mask", which represents the set of access rights associated with + * the passed ACE. + * (2) Abbreviation 'G' used in the evaluation algorithm stands for + * "granted rights", which represents the set of access rights, that + * have already been granted by previously evaluated ACEs. + * (3) Abbreviation 'D' used in the evaluation algorithm stands for + * "denied rights", which represents the set of access rights, that + * have already been explicitly denied by previously evaluated ACEs. + * + * The simple ACE evaluation algorithm is specified in [MS-ADTS] 5.1.3.3.2: + * Evaluate the DACL by examining each ACE in sequence, starting with the first + * ACE. Perform the following sequence of actions for each ACE in the order as + * shown: + * 1. If the "Inherit Only" (IO) flag is set in the ACE, skip the ACE. + * 2. If the SID in the ACE does not match any SID in the requester's + * security context, skip the ACE. + * 3. If the ACE type is "Access Denied" and the access rights in M + * are not in G, then add the rights in M to D. + * 4. If the ACE type is "Access Allowed" and the access rights in M + * are not in D, then add the rights in M to G. + */ +static errno_t ad_gpo_simple_evaluate_ace(struct security_ace *ace, + struct sss_idmap_ctx *idmap_ctx, + const char *user_sid, + const char **group_sids, + int group_size, + uint32_t *_gpo_access_granted_status, + uint32_t *_gpo_access_denied_status) +{ + bool included = false; + uint32_t filtered_access_rights = 0; + int ret = 0; + + if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) { + return EOK; } - return AD_GPO_ACE_DENIED; + ret = ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size, + ace->trustee, idmap_ctx, &included); + + if (ret != EOK || !included) { + return ret; + } + + if (ace->type == SEC_ACE_TYPE_ACCESS_DENIED) { + filtered_access_rights = ace->access_mask & ~*_gpo_access_granted_status; + *_gpo_access_denied_status |= filtered_access_rights; + } else if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED) { + filtered_access_rights = ace->access_mask & ~*_gpo_access_denied_status; + *_gpo_access_granted_status |= filtered_access_rights; + } + + return ret; } + /* * This function extracts the GPO's DACL (discretionary access control list) * from the GPO's specified security descriptor, and determines whether * the GPO is applicable to the policy target, by comparing the specified * user_sid and group_sids against each access control entry (ACE) in the DACL. - * The boolean result is assigned to the _access_allowed output parameter. + * The GPO is only applicable to the target, if the requester has been granted + * read access (RIGHT_DS_READ_PROPERTY) to the properties of the GPO and + * control access (RIGHT_DS_CONTROL_ACCESS) to apply the GPO (AGP). + * The required read and control access rights for a particular trustee are + * usually located in different ACEs, i. e. one ACE for control of read access + * and one for control access. + * If it comes to the end of the DACL, and the required access is still not + * explicitly allowed or denied, SSSD denies access to the object as specified + * in [MS-ADTS] 5.1.3.1. */ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, struct sss_idmap_ctx *idmap_ctx, @@ -769,14 +848,19 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, bool *_dacl_access_allowed) { uint32_t num_aces = 0; - enum ace_eval_status ace_status; - int i = 0; + uint32_t access_granted_status = 0; + uint32_t access_denied_status = 0; + enum ace_eval_agp_status ace_status; struct security_ace *ace = NULL; + int i = 0; + int ret = 0; + enum idmap_error_code err; + char *trustee_dom_sid_str = NULL; num_aces = dacl->num_aces; /* - * [MS-ADTS] 5.1.3.3.4: + * [MS-ADTS] 5.1.3.3.2. and 5.1.3.3.4: * If the DACL does not have any ACE, then deny the requester the * requested control access right. */ @@ -785,21 +869,84 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, return EOK; } + /* + * [MS-GOPD] 2.4: + * To process a policy that applies to a Group Policy client, the core + * Group Policy engine must be able to read the policy data from the + * directory service so that the policy settings can be applied to the + * Group Policy client or the interactive user. + */ + for (i = 0; i < dacl->num_aces; i++) { + ace = &dacl->aces[i]; + + ret = ad_gpo_simple_evaluate_ace(ace, idmap_ctx, user_sid, + group_sids, group_size, + &access_granted_status, + &access_denied_status); + + if (ret != EOK) { + err = sss_idmap_smb_sid_to_sid(idmap_ctx, &ace->trustee, + &trustee_dom_sid_str); + if (err != IDMAP_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, + " sss_idmap_smb_sid_to_sid failed.\n"); + return EFAULT; + } + + DEBUG(SSSDBG_MINOR_FAILURE, + " Could not determine if ACE is applicable; " + " Trustee: %s\n", trustee_dom_sid_str); + free(trustee_dom_sid_str); + trustee_dom_sid_str = NULL; + continue; + } + } + for (i = 0; i < dacl->num_aces; i ++) { ace = &dacl->aces[i]; - ace_status = ad_gpo_evaluate_ace(ace, idmap_ctx, user_sid, - group_sids, group_size); + err = sss_idmap_smb_sid_to_sid(idmap_ctx, &ace->trustee, + &trustee_dom_sid_str); + if (err != IDMAP_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, " sss_idmap_smb_sid_to_sid failed.\n"); + return EFAULT; + } + + ace_status = ad_gpo_evaluate_agp_ace(ace, idmap_ctx, user_sid, + group_sids, group_size); switch (ace_status) { case AD_GPO_ACE_NEUTRAL: continue; case AD_GPO_ACE_ALLOWED: - *_dacl_access_allowed = true; - return EOK; + if (access_granted_status & SEC_ADS_READ_PROP) { + *_dacl_access_allowed = true; + return EOK; + } else { + DEBUG(SSSDBG_TRACE_ALL, + " GPO read properties access denied (security); " + " Trustee: %s\n", trustee_dom_sid_str); + free(trustee_dom_sid_str); + trustee_dom_sid_str = NULL; + continue; + } case AD_GPO_ACE_DENIED: - *_dacl_access_allowed = false; - return EOK; + if (access_granted_status & SEC_ADS_READ_PROP) { + DEBUG(SSSDBG_TRACE_ALL, + " GPO denied (security); " + " Trustee: %s\n", trustee_dom_sid_str); + free(trustee_dom_sid_str); + trustee_dom_sid_str = NULL; + *_dacl_access_allowed = false; + return EOK; + } else { + DEBUG(SSSDBG_TRACE_ALL, + " GPO read properties access denied (security); " + " Trustee: %s\n", trustee_dom_sid_str); + free(trustee_dom_sid_str); + trustee_dom_sid_str = NULL; + continue; + } } } From 2e2f9dc66c73b0ce35d97ffc8c2410b225533c19 Mon Sep 17 00:00:00 2001 From: REIM THOMAS <rei...@gmail.com> Date: Sat, 4 Mar 2017 20:01:49 +0100 Subject: [PATCH 5/6] GPO: Improve logging of GPO security filtering GPO security filtering is as critical as the actual logon policy rights checking. Administrators should not only be able to figure out, why GPO access check granted or denied a user login, but also why a GPO access check was not performed due to security filtering. GPO access check can be logged using debug level Function Data, whereas GPO security filtering can only be logged with lowest level tracing. - Debug the main security filtering activities on level Function Data - Debug missing security descriptor as minor failure, because it terminates GPO security filtering. Resolves: https://pagure.io/SSSD/sssd/issue/3324 Signed-off-by: REIM THOMAS <rei...@gmail.com> --- src/providers/ad/ad_gpo.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index c9a98060d..2ff027569 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -889,12 +889,12 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, &trustee_dom_sid_str); if (err != IDMAP_SUCCESS) { DEBUG(SSSDBG_OP_FAILURE, - " sss_idmap_smb_sid_to_sid failed.\n"); + "sss_idmap_smb_sid_to_sid failed.\n"); return EFAULT; } DEBUG(SSSDBG_MINOR_FAILURE, - " Could not determine if ACE is applicable; " + "Could not determine if ACE is applicable; " " Trustee: %s\n", trustee_dom_sid_str); free(trustee_dom_sid_str); trustee_dom_sid_str = NULL; @@ -908,7 +908,7 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, err = sss_idmap_smb_sid_to_sid(idmap_ctx, &ace->trustee, &trustee_dom_sid_str); if (err != IDMAP_SUCCESS) { - DEBUG(SSSDBG_OP_FAILURE, " sss_idmap_smb_sid_to_sid failed.\n"); + DEBUG(SSSDBG_OP_FAILURE, "sss_idmap_smb_sid_to_sid failed.\n"); return EFAULT; } @@ -923,8 +923,8 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, *_dacl_access_allowed = true; return EOK; } else { - DEBUG(SSSDBG_TRACE_ALL, - " GPO read properties access denied (security); " + DEBUG(SSSDBG_TRACE_FUNC, + "GPO read properties access denied (security); " " Trustee: %s\n", trustee_dom_sid_str); free(trustee_dom_sid_str); trustee_dom_sid_str = NULL; @@ -932,16 +932,16 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, } case AD_GPO_ACE_DENIED: if (access_granted_status & SEC_ADS_READ_PROP) { - DEBUG(SSSDBG_TRACE_ALL, - " GPO denied (security); " + DEBUG(SSSDBG_TRACE_FUNC, + "GPO denied (security); " " Trustee: %s\n", trustee_dom_sid_str); free(trustee_dom_sid_str); trustee_dom_sid_str = NULL; *_dacl_access_allowed = false; return EOK; } else { - DEBUG(SSSDBG_TRACE_ALL, - " GPO read properties access denied (security); " + DEBUG(SSSDBG_TRACE_FUNC, + "GPO read properties access denied (security); " " Trustee: %s\n", trustee_dom_sid_str); free(trustee_dom_sid_str); trustee_dom_sid_str = NULL; @@ -950,6 +950,11 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, } } + if (access_granted_status & SEC_ADS_READ_PROP) { + DEBUG(SSSDBG_TRACE_FUNC, + "GPO apply group policy access denied (security)\n"); + } + *_dacl_access_allowed = false; return EOK; } @@ -1012,12 +1017,12 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, access_allowed = false; candidate_gpo = candidate_gpos[i]; - DEBUG(SSSDBG_TRACE_ALL, "examining dacl candidate_gpo_guid:%s\n", - candidate_gpo->gpo_guid); + DEBUG(SSSDBG_TRACE_FUNC, "examining dacl candidate_gpo_guid:%s\n", + candidate_gpo->gpo_guid); /* gpo_func_version must be set to version 2 */ if (candidate_gpo->gpo_func_version != 2) { - DEBUG(SSSDBG_TRACE_ALL, + DEBUG(SSSDBG_TRACE_FUNC, "GPO not applicable to target per security filtering: " "gPCFunctionalityVersion is not 2\n"); continue; @@ -1025,7 +1030,7 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, sd = candidate_gpo->gpo_sd; if (sd == NULL) { - DEBUG(SSSDBG_TRACE_ALL, "Security descriptor is missing\n"); + DEBUG(SSSDBG_MINOR_FAILURE, "Security descriptor is missing\n"); ret = EINVAL; goto done; } @@ -1034,7 +1039,7 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, /* gpo_flags value of 2 means that GPO's computer portion is disabled */ if (candidate_gpo->gpo_flags == 2) { - DEBUG(SSSDBG_TRACE_ALL, + DEBUG(SSSDBG_TRACE_FUNC, "GPO not applicable to target per security filtering: " "GPO's computer portion is disabled\n"); continue; @@ -1044,7 +1049,8 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, ret = ad_gpo_evaluate_dacl(dacl, idmap_ctx, user_sid, group_sids, group_size, &access_allowed); if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, "Could not determine if GPO is applicable\n"); + DEBUG(SSSDBG_MINOR_FAILURE, + "Could not determine if GPO is applicable\n"); continue; } } else { @@ -1059,13 +1065,13 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, } if (access_allowed) { - DEBUG(SSSDBG_TRACE_ALL, + DEBUG(SSSDBG_TRACE_FUNC, "GPO applicable to target per security filtering\n"); dacl_filtered_gpos[gpo_dn_idx] = talloc_steal(dacl_filtered_gpos, candidate_gpo); gpo_dn_idx++; } else { - DEBUG(SSSDBG_TRACE_ALL, + DEBUG(SSSDBG_TRACE_FUNC, "GPO not applicable to target per security filtering: " "result of DACL evaluation\n"); continue; From 5245ee473adfb726c5e47c7f8d9b30974d9deda4 Mon Sep 17 00:00:00 2001 From: REIM THOMAS <rei...@gmail.com> Date: Sun, 5 Mar 2017 13:04:41 +0100 Subject: [PATCH 6/6] MAN: Provide minimum information on GPO access control Update sssd-ad man page to give administrators the minimum required information how SSSD performs GPO based access control. Also added a hint how to configure logging to get sufficient GPO troubleshooting information by examining the logs. Resolves: https://pagure.io/SSSD/sssd/issue/3324 Signed-off-by: REIM THOMAS <rei...@gmail.com> --- src/man/sssd-ad.5.xml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 3 deletions(-) diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml index 08c1dd09f..284530365 100644 --- a/src/man/sssd-ad.5.xml +++ b/src/man/sssd-ad.5.xml @@ -342,8 +342,44 @@ DOM:dom1:(memberOf:1.2.840.113556.1.4.1941:=cn=nestedgroup,ou=groups,dc=example, <para> GPO-based access control functionality uses GPO policy settings to determine whether or not a - particular user is allowed to logon to a particular - host. + particular user is allowed to logon to the host. + For more information on the supported policy + settings please refer to the + <quote>ad_gpo_map</quote> options. + </para> + <para> + Before performing access control SSSD applies group + policy security filtering on the GPOs. For every + single user login, the applicability of the GPOs + that are linked to the host is checked. In order for + a GPO to apply to a user, the user or at least one + of the groups to which it belongs must have + following permissions on the GPO: + <itemizedlist> + <listitem> + <para> + Read: The user or one of its groups must + have read access to the properties of the + GPO (RIGHT_DS_READ_PROPERTY) + </para> + </listitem> + <listitem> + <para> + Apply Group Policy: The user or at least + one of its groups must be allowed to + apply the GPO (RIGHT_DS_CONTROL_ACCESS). + </para> + </listitem> + </itemizedlist> + </para> + <para> + By default, the Authenticated Users group is present + on a GPO and this group has both Read and Apply Group + Policy access rights. Since authentication of a user + must have been completed successfully before GPO + security filtering and access control are started, + the Authenticated Users group permissions on the GPO + always apply also to the user. </para> <para> NOTE: If the operation mode is set to enforcing, it @@ -356,7 +392,13 @@ DOM:dom1:(memberOf:1.2.840.113556.1.4.1941:=cn=nestedgroup,ou=groups,dc=example, will output a syslog message if access would have been denied. By examining the logs, administrators can then make the necessary changes before setting - the mode to enforcing. + the mode to enforcing. For logging GPO-based access + control debug level 'function data' is required (see + <citerefentry> + <refentrytitle>sss_debuglevel</refentrytitle> + <manvolnum>8</manvolnum> + </citerefentry> + manual page). </para> <para> There are three supported values for this option: @@ -418,6 +460,18 @@ DOM:dom1:(memberOf:1.2.840.113556.1.4.1941:=cn=nestedgroup,ou=groups,dc=example, which GPO-based access control is evaluated based on the InteractiveLogonRight and DenyInteractiveLogonRight policy settings. + Only those GPOs are evaluated for which the user has + Read and Apply Group Policy permission (see option + <quote>ad_gpo_access_control</quote>). + If an evaluated GPO contains the deny interactive + logon setting for the user or one of its groups, the + user is denied local access. + If none of the evaluated GPOs has an interactive + logon right defined, the user is granted local + access. If at least one evaluated GPO contains + interactive logon right settings, the user is + granted local access only, if it or at least one of + its groups is part of the policy settings. </para> <para> Note: Using the Group Policy Management Editor @@ -513,6 +567,19 @@ ad_gpo_map_interactive = +my_pam_service, -login which GPO-based access control is evaluated based on the RemoteInteractiveLogonRight and DenyRemoteInteractiveLogonRight policy settings. + Only those GPOs are evaluated for which the user has + Read and Apply Group Policy permission (see option + <quote>ad_gpo_access_control</quote>). + If an evaluated GPO contains the deny remote + logon setting for the user or one of its groups, the + user is denied remote interactive access. + If none of the evaluated GPOs has a remote + interactive logon right defined, the user is granted + remote access. If at least one evaluated GPO + contains remote interactive logon right settings, + the user is granted remote access only, if it or at + least one of its groups is part of the policy + settings. </para> <para> Note: Using the Group Policy Management Editor this @@ -559,6 +626,18 @@ ad_gpo_map_remote_interactive = +my_pam_service, -sshd which GPO-based access control is evaluated based on the NetworkLogonRight and DenyNetworkLogonRight policy settings. + Only those GPOs are evaluated for which the user has + Read and Apply Group Policy permission (see option + <quote>ad_gpo_access_control</quote>). + If an evaluated GPO contains the deny network + logon setting for the user or one of its groups, the + user is denied network logon access. + If none of the evaluated GPOs has a network + logon right defined, the user is granted logon + access. If at least one evaluated GPO contains + network logon right settings, the user is + granted logon access only, if it or at least one of + its groups is part of the policy settings. </para> <para> Note: Using the Group Policy Management Editor @@ -605,6 +684,18 @@ ad_gpo_map_network = +my_pam_service, -ftp which GPO-based access control is evaluated based on the BatchLogonRight and DenyBatchLogonRight policy settings. + Only those GPOs are evaluated for which the user has + Read and Apply Group Policy permission (see option + <quote>ad_gpo_access_control</quote>). + If an evaluated GPO contains the deny batch + logon setting for the user or one of its groups, the + user is denied batch logon access. + If none of the evaluated GPOs has a batch + logon right defined, the user is granted logon + access. If at least one evaluated GPO contains + batch logon right settings, the user is + granted logon access only, if it or at least one of + its groups is part of the policy settings. </para> <para> Note: Using the Group Policy Management Editor @@ -645,6 +736,18 @@ ad_gpo_map_batch = +my_pam_service, -crond which GPO-based access control is evaluated based on the ServiceLogonRight and DenyServiceLogonRight policy settings. + Only those GPOs are evaluated for which the user has + Read and Apply Group Policy permission (see option + <quote>ad_gpo_access_control</quote>). + If an evaluated GPO contains the deny service + logon setting for the user or one of its groups, the + user is denied service logon access. + If none of the evaluated GPOs has a service + logon right defined, the user is granted logon + access. If at least one evaluated GPO contains + service logon right settings, the user is + granted logon access only, if it or at least one of + its groups is part of the policy settings. </para> <para> Note: Using the Group Policy Management Editor
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
