From 3637db8a5a6957285455367e63cdbd8f32365afa Mon Sep 17 00:00:00 2001 From: Ondrej Valousek Date: Wed, 11 Sep 2024 11:55:51 +0200 Subject: [PATCH 1/3] Nested groups: Handle Foreign Security Principals correctly (i.e. ignore for now) --- src/providers/ldap/sdap_async_nested_groups.c | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c index 2e3b0c434f3..ef995697dea 100644 --- a/src/providers/ldap/sdap_async_nested_groups.c +++ b/src/providers/ldap/sdap_async_nested_groups.c @@ -47,6 +47,7 @@ enum sdap_nested_group_dn_type { SDAP_NESTED_GROUP_DN_USER, SDAP_NESTED_GROUP_DN_GROUP, + SDAP_NESTED_GROUP_DN_FSP, SDAP_NESTED_GROUP_DN_UNKNOWN }; @@ -526,6 +527,41 @@ sdap_nested_member_is_group(struct sdap_nested_group_ctx *group_ctx, return sdap_nested_member_is_ent(group_ctx, dn, filter, false); } +static bool +sdap_nested_member_is_fsp(struct sdap_nested_group_ctx *group_ctx, + const char *dn) +{ + char *fspdn, *basedn; + int fspdn_len, dn_len, len_diff; + int reti; + bool ret = false; + + reti = domain_to_basedn(group_ctx, group_ctx->domain->realm, &basedn); + if (reti != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to obtain basedn\n"); + return false; + } + + fspdn = talloc_asprintf(group_ctx, "%s,%s", + "CN=ForeignSecurityPrincipals", basedn); + if (fspdn == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to run talloc_asprintf\n"); + return false; + } + + talloc_free(basedn); + fspdn_len = strlen(fspdn); + dn_len = strlen(dn); + len_diff = dn_len - fspdn_len; + if (len_diff < 0) { + talloc_free(fspdn); + return false; + } + ret = strncasecmp(&dn[len_diff], fspdn, fspdn_len) == 0; + talloc_free(fspdn); + return ret; +} + static errno_t sdap_nested_group_split_members(TALLOC_CTX *mem_ctx, struct sdap_nested_group_ctx *group_ctx, @@ -548,6 +584,7 @@ sdap_nested_group_split_members(TALLOC_CTX *mem_ctx, bool bret; bool is_user; bool is_group; + bool is_fsp; errno_t ret; int i; @@ -625,7 +662,12 @@ sdap_nested_group_split_members(TALLOC_CTX *mem_ctx, is_group = sdap_nested_member_is_group(group_ctx, dn, &group_filter); - if (is_user && is_group) { + is_fsp = sdap_nested_member_is_fsp(group_ctx, dn); + + if (is_fsp) { + DEBUG(SSSDBG_TRACE_ALL, "[%s] is Foreign Security principal\n", dn); + type = SDAP_NESTED_GROUP_DN_FSP; + } else if (is_user && is_group) { /* search bases overlap */ DEBUG(SSSDBG_TRACE_ALL, "[%s] is unknown object\n", dn); type = SDAP_NESTED_GROUP_DN_UNKNOWN; @@ -1500,6 +1542,10 @@ static errno_t sdap_nested_group_single_step(struct tevent_req *req) state->group_ctx, state->current_member); break; + case SDAP_NESTED_GROUP_DN_FSP: /* TODO: Process FSPs */ + DEBUG(SSSDBG_TRACE_ALL, "Ignoring Foreign Security Principal [%s]\n", + state->current_member->dn); + return EOK; } if (subreq == NULL) { @@ -1625,6 +1671,9 @@ sdap_nested_group_single_step_process(struct tevent_req *subreq) state->nested_groups[state->num_groups] = entry; state->num_groups++; + break; + case SDAP_NESTED_GROUP_DN_FSP: /* TODO: Handle FSPs */ + DEBUG(SSSDBG_TRACE_ALL, "BUG!!! We should never get here\n"); break; case SDAP_NESTED_GROUP_DN_UNKNOWN: if (state->ignore_unreadable_references) { From 6afcc0b94604a9428fda3169902b8e8bc3dc2057 Mon Sep 17 00:00:00 2001 From: Ondrej Valousek Date: Tue, 17 Sep 2024 11:32:56 +0200 Subject: [PATCH 2/3] Precision FSP detection update (optional) --- src/lib/idmap/sss_idmap.c | 16 +++++- src/lib/idmap/sss_idmap.exports | 1 + src/lib/idmap/sss_idmap.h | 11 ++++ src/providers/ldap/sdap.h | 1 + src/providers/ldap/sdap_async_nested_groups.c | 56 ++++++++++++------- 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c index 7ad05659a76..43dd5028fdb 100644 --- a/src/lib/idmap/sss_idmap.c +++ b/src/lib/idmap/sss_idmap.c @@ -201,7 +201,7 @@ const char *idmap_error_string(enum idmap_error_code err) } } -bool is_domain_sid(const char *sid) +bool is_str_sid(const char *sid, int count) { const char *p; long long a; @@ -228,15 +228,25 @@ bool is_domain_sid(const char *sid) return false; } c++; - } while(c < 3 && *endptr != '\0'); + } while(c < count && *endptr != '\0'); - if (c != 3 || *endptr != '\0') { + if (c != count || *endptr != '\0') { return false; } return true; } +bool is_principal_sid(const char *str) +{ + return is_str_sid(str, 4); +} + +bool is_domain_sid(const char *str) +{ + return is_str_sid(str, 3); +} + enum idmap_error_code sss_idmap_init(idmap_alloc_func *alloc_func, void *alloc_pvt, idmap_free_func *free_func, diff --git a/src/lib/idmap/sss_idmap.exports b/src/lib/idmap/sss_idmap.exports index 840677794bd..813987ebf86 100644 --- a/src/lib/idmap/sss_idmap.exports +++ b/src/lib/idmap/sss_idmap.exports @@ -62,5 +62,6 @@ SSS_IDMAP_0.5 { sss_idmap_ctx_set_extra_slice_init; sss_idmap_add_auto_domain_ex; + is_principal_sid; } SSS_IDMAP_0.4; diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h index 9c27a160004..a544e8625b0 100644 --- a/src/lib/idmap/sss_idmap.h +++ b/src/lib/idmap/sss_idmap.h @@ -706,6 +706,17 @@ const char *idmap_error_string(enum idmap_error_code err); */ bool is_domain_sid(const char *str); +/** + * @brief Check if given string can be used as principal SID + * + * @param[in] str String to check + * + * @return + * - true: String can be used as principal SID + * - false: String can not be used as principal SID + */ +bool is_principal_sid(const char *str); + /** * @brief Check if a domain is configured with algorithmic mapping * diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index d66ca156afe..6ddfc4c4613 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -467,6 +467,7 @@ struct sdap_domain { * the base DN which will not be based on the DNS domain of the LDAP * server. naming_context might be NULL even after connection to an LDAP * server. */ + char *fspdn; /* Foreign security principal search base */ char *naming_context; struct sdap_search_base **search_bases; diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c index ef995697dea..9b4ab31c9ae 100644 --- a/src/providers/ldap/sdap_async_nested_groups.c +++ b/src/providers/ldap/sdap_async_nested_groups.c @@ -531,34 +531,50 @@ static bool sdap_nested_member_is_fsp(struct sdap_nested_group_ctx *group_ctx, const char *dn) { - char *fspdn, *basedn; - int fspdn_len, dn_len, len_diff; - int reti; - bool ret = false; - - reti = domain_to_basedn(group_ctx, group_ctx->domain->realm, &basedn); - if (reti != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "Unable to obtain basedn\n"); - return false; - } + char *fspdn; + size_t fspdn_len, dn_len; + int reti, len_diff; + bool ret = false; + + if (group_ctx->opts->sdom->fspdn == NULL) { + char *basedn; + + reti = domain_to_basedn(group_ctx, group_ctx->domain->realm, &basedn); + if (reti != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to obtain basedn\n"); + return false; + } - fspdn = talloc_asprintf(group_ctx, "%s,%s", - "CN=ForeignSecurityPrincipals", basedn); - if (fspdn == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "Unable to run talloc_asprintf\n"); - return false; + fspdn = talloc_asprintf(group_ctx->opts->sdom, "%s,%s", + "CN=ForeignSecurityPrincipals", basedn); + if (fspdn == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to run talloc_asprintf\n"); + return false; + } + talloc_free(basedn); + group_ctx->opts->sdom->fspdn = fspdn; + } else { + fspdn = group_ctx->opts->sdom->fspdn; } - talloc_free(basedn); fspdn_len = strlen(fspdn); dn_len = strlen(dn); len_diff = dn_len - fspdn_len; - if (len_diff < 0) { - talloc_free(fspdn); + if (len_diff < sizeof("CN=S-")) { return false; } - ret = strncasecmp(&dn[len_diff], fspdn, fspdn_len) == 0; - talloc_free(fspdn); + ret = (strncasecmp(&dn[len_diff], fspdn, fspdn_len) == 0); + + if (ret) { /* looks like FSP, so just double check to be 100% sure */ + char *fsp_str = talloc_strdup(group_ctx, dn); + + if (fsp_str == NULL) + return false; + fsp_str[len_diff - 1] = '\0'; /* replace comma with NULL */ + ret = is_principal_sid(&fsp_str[3]); + talloc_free(fsp_str); + } + return ret; } From ad457a97f101e23c669170553129854eb98ed1ce Mon Sep 17 00:00:00 2001 From: Ondrej Valousek Date: Mon, 28 Oct 2024 10:06:24 +0100 Subject: [PATCH 3/3] AD: Detect and ignore (for now) Foreign Security Principals --- src/db/sysdb.h | 1 + src/providers/ldap/sdap_async_nested_groups.c | 34 ++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 047a4877fed..ea9fecdf012 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -64,6 +64,7 @@ #define SYSDB_DOMAIN_ID_RANGE_CLASS "domainIDRange" #define SYSDB_TRUSTED_AD_DOMAIN_RANGE_CLASS "TrustedADDomainRange" #define SYSDB_CERTMAP_CLASS "certificateMappingRule" +#define SYSDB_AD_FSP_CLASS "foreignSecurityPrincipal" #define SYSDB_DN "dn" #define SYSDB_NAME "name" diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c index 9b4ab31c9ae..01545803d79 100644 --- a/src/providers/ldap/sdap_async_nested_groups.c +++ b/src/providers/ldap/sdap_async_nested_groups.c @@ -252,6 +252,33 @@ static errno_t sdap_nested_group_hash_user(struct sdap_nested_group_ctx *group_ctx, struct sysdb_attrs *user) { + errno_t ret; + const char *val = NULL; + const char **val_list = NULL; + + ret = sysdb_attrs_get_string_array(user, SYSDB_OBJECTCLASS, group_ctx, &val_list); + if (ret == EOK) { + /* if called from sdap_nested_group_single_step_process(), then we have objectclass + * and uid attrs so we can test for Foreign Security principals */ + if (string_in_list(SYSDB_AD_FSP_CLASS, discard_const(val_list), false)) { + /* TODO: handle Foreign Security Principal here + * since we don't know how to do it now, then we skip them for now */ + sysdb_attrs_get_string(user, SYSDB_ORIG_DN, &val); + DEBUG(SSSDBG_TRACE_ALL, "Ignoring Foreign Principal %s\n",val); + talloc_free(val_list); + return EOK; + } + talloc_free(val_list); + + ret = sysdb_attrs_get_string(user, group_ctx->opts->user_map[SDAP_AT_USER_NAME].name, &val); + if (ret != EOK) { + DEBUG(SSSDBG_TRACE_ALL, "Unable to get username for %s\n",val); + return ret; + } + /* we need to populate the sys_name in the user map so the user is recognized later on */ + sysdb_attrs_add_string(user, group_ctx->opts->user_map[SDAP_AT_USER_NAME].sys_name, val); + } + return sdap_nested_group_hash_entry(group_ctx->users, user, "users"); } @@ -1895,8 +1922,8 @@ sdap_nested_group_lookup_user_send(TALLOC_CTX *mem_ctx, attrs[2] = NULL; /* create filter */ - base_filter = talloc_asprintf(state, "(objectclass=%s)", - group_ctx->opts->user_map[SDAP_OC_USER].name); + base_filter = talloc_asprintf(state, "(|(objectclass=%s)(objectclass=%s))", + group_ctx->opts->user_map[SDAP_OC_USER].name,SYSDB_AD_FSP_CLASS); if (base_filter == NULL) { ret = ENOMEM; goto immediately; @@ -1912,8 +1939,7 @@ sdap_nested_group_lookup_user_send(TALLOC_CTX *mem_ctx, /* search */ subreq = sdap_get_generic_send(state, ev, group_ctx->opts, group_ctx->sh, member->dn, LDAP_SCOPE_BASE, filter, attrs, - group_ctx->opts->user_map, - group_ctx->opts->user_map_cnt, + NULL, 0, dp_opt_get_int(group_ctx->opts->basic, SDAP_SEARCH_TIMEOUT), false);