[SSSD] [PATCH] Add a new option ad_access_filter
Jakub Hrozek
jhrozek at redhat.com
Fri Oct 25 17:52:23 UTC 2013
On Fri, Oct 25, 2013 at 07:44:47PM +0200, Lukas Slebodnik wrote:
> On (25/10/13 17:45), Jakub Hrozek wrote:
> >On Thu, Oct 10, 2013 at 01:49:40PM +0200, Jakub Hrozek wrote:
> >> Hi,
> >>
> >> the attached patches implement the new option ad_access_control as
> >> designed in
> >> https://fedorahosted.org/sssd/wiki/DesignDocs/ActiveDirectoryAccessControl
> >>
> >> The only part not implemented exactly as per the design page is changing
> >> the default. I will write an e-mail about the issue into the thread with
> >> the design decision so that also people who filter out patches can
> >> participate.
> >
> >Attached are patches rebased on current master.
>
> >From 3f115bad21ce47bb7dee4541982613c0212ed229 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Sun, 6 Oct 2013 20:23:07 +0200
> >Subject: [PATCH 1/6] LDAP: Amend sdap_access_check to allow any connection
> ACK
>
> >From ffeaf255263671f8cc43bab3aec7b1304e076705 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Tue, 8 Oct 2013 19:10:41 +0200
> >Subject: [PATCH 2/6] LDAP: Parse FQDN into name/domain for subdomain users
> ACK
>
> >From 7b7532f68058dfad4122890743041973bb6bdfc6 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Wed, 2 Oct 2013 17:48:49 +0200
> >Subject: [PATCH 3/6] AD: Add a new option ad_access_filter
> ACK
>
> >From 1daec9096cc5ae8690b3fe567713cf9ef23096e7 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Mon, 7 Oct 2013 18:02:04 +0200
> >Subject: [PATCH 4/6] AD: Use the ad_access_filter if it's set
> ACK
>
> >From e4b1e0ca7d4078e787ccdab5d87b73aa5c8a6ab7 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Tue, 8 Oct 2013 17:50:56 +0200
> >Subject: [PATCH 5/6] AD: Search GC by default during access control, fall back
> > to LDAP
> >
> >Resolves:
> >https://fedorahosted.org/sssd/ticket/2082
> >
> >In order to allow the ad_access_filter option to work for subdomain
> >users as well, the Global Catalog must be searched. This patch adds a
> >wrapper request atop sdap_access_send that selects the right connection
> >(GC or LDAP) and optionally falls back to LDAP.
> >---
> > src/providers/ad/ad_access.c | 160 +++++++++++++++++++++++++++++++++++++++++--
> > src/providers/ad/ad_access.h | 4 +-
> > src/providers/ad/ad_init.c | 5 +-
> > 3 files changed, 159 insertions(+), 10 deletions(-)
> >
> >diff --git a/src/providers/ad/ad_access.c b/src/providers/ad/ad_access.c
> >index cf6412f22c6146b26090646b5b8d060efa43365f..59e2f2b81a3f9d1850d673551be079f42a6a2ac5 100644
> >--- a/src/providers/ad/ad_access.c
> >+++ b/src/providers/ad/ad_access.c
> >@@ -25,10 +25,158 @@
> > #include "src/providers/data_provider.h"
> > #include "src/providers/dp_backend.h"
> > #include "src/providers/ad/ad_access.h"
> >+#include "src/providers/ad/ad_common.h"
> > #include "src/providers/ldap/sdap_access.h"
> >
> > static void
> > ad_access_done(struct tevent_req *req);
> >+static errno_t
> >+ad_access_step(struct tevent_req *req, struct sdap_id_conn_ctx *conn);
> >+
> >+struct ad_access_state {
> >+ struct tevent_context *ev;
> >+ struct ad_access_ctx *ctx;
> >+ struct pam_data *pd;
> >+ struct be_ctx *be_ctx;
> >+ struct sss_domain_info *domain;
> >+
> >+ struct sdap_id_conn_ctx **clist;
> >+ int cindex;
> >+};
> >+
> >+static struct tevent_req *
> >+ad_access_send(TALLOC_CTX *mem_ctx,
> >+ struct tevent_context *ev,
> >+ struct be_ctx *be_ctx,
> >+ struct sss_domain_info *domain,
> >+ struct ad_access_ctx *ctx,
> >+ struct pam_data *pd)
> >+{
> >+ struct tevent_req *req;
> >+ struct ad_access_state *state;
> >+ errno_t ret;
> >+
> >+ req = tevent_req_create(mem_ctx, &state, struct ad_access_state);
> >+ if (req == NULL) {
> >+ return NULL;
> >+ }
> >+
> >+ state->ev = ev;
> >+ state->ctx = ctx;
> >+ state->pd = pd;
> >+ state->be_ctx = be_ctx;
> >+ state->domain = domain;
> >+
> >+ state->clist = talloc_zero_array(state, struct sdap_id_conn_ctx *, 3);
> >+ if (state->clist == NULL) {
> >+ ret = ENOMEM;
> >+ goto done;
> >+ }
> >+
> >+ /* Always try GC first */
> >+ state->clist[0] = ctx->gc_ctx;
> >+ if (IS_SUBDOMAIN(domain) == false) {
> This is the same situation like in file src/providers/ad/ad_id.c
> If GC is misconfigured, we should not go offline.
> SSSD should try to fall back to LDAP
> >+ /* With root domain users we have the option to
> >+ * fall back to LDAP in case ie POSIX attributes
> >+ * are used but not replicated to GC
> >+ */
> >+ state->clist[1] = ctx->ldap_ctx;
> >+ }
> >+
> >+ ret = ad_access_step(req, state->clist[state->cindex]);
> >+ if (ret != EOK) {
> >+ goto done;
> >+ }
> >
>
> >From 8d2cfbcc0d992b26f90c9494b10f3bf666498b66 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Tue, 8 Oct 2013 20:59:22 +0200
> >Subject: [PATCH 6/6] AD: Add extended access filter
> >
> >https://fedorahosted.org/sssd/ticket/2082
> >
> >Adds a new option that allows the admin to specify a LDAP access filter
> >that can be applied globally, per-domain or per-forest.
> >---
> > Makefile.am | 29 ++-
> > src/man/sssd-ad.5.xml | 41 +++-
> > src/providers/ad/ad_access.c | 213 ++++++++++++++++++-
> > src/providers/ad/ad_init.c | 5 +-
> > src/tests/cmocka/test_ad_access_filter.c | 341 +++++++++++++++++++++++++++++++
> > 5 files changed, 623 insertions(+), 6 deletions(-)
> > create mode 100644 src/tests/cmocka/test_ad_access_filter.c
> >
> >diff --git a/Makefile.am b/Makefile.am
> >index 8d548e2bd7f5ffa1d9d7a46dc08026213e8e80bc..bb5136e24cdbee48d9d356ef2f2849334f397d48 100644
> >--- a/Makefile.am
> >+++ b/Makefile.am
> >@@ -153,7 +153,8 @@ if HAVE_CMOCKA
> > dyndns-tests \
> > fqnames-tests \
> > test_sss_idmap \
> >- test_utils
> >+ test_utils \
> >+ ad_access_filter_tests
> > endif
> >
> > check_PROGRAMS = \
> >@@ -1389,6 +1390,32 @@ test_utils_LDADD = \
> > $(CMOCKA_LIBS) \
> > $(SSSD_INTERNAL_LTLIBS) \
> > libsss_test_common.la
> >+
> >+ad_access_filter_tests_SOURCES = \
> >+ $(TEST_MOCK_OBJ) \
> >+ $(sssd_be_SOURCES) \
> >+ src/util/sss_ldap.c \
> >+ src/util/sss_krb5.c \
> >+ src/util/find_uid.c \
> >+ src/util/user_info_msg.c \
> >+ src/tests/cmocka/test_ad_access_filter.c
> >+ad_access_filter_tests_CFLAGS = \
> >+ $(AM_CFLAGS) \
> >+ $(SYSTEMD_LOGIN_CFLAGS) \
> >+ -DUNIT_TESTING
> >+ad_access_filter_tests_LDADD = \
> >+ $(PAM_LIBS) \
> >+ $(CMOCKA_LIBS) \
> >+ $(SSSD_LIBS) \
> >+ $(CARES_LIBS) \
> >+ $(KRB5_LIBS) \
> >+ $(SSSD_INTERNAL_LTLIBS) \
> >+ $(SYSTEMD_LOGIN_LIBS) \
> >+ libsss_idmap.la \
> >+ libsss_ldap_common.la \
> >+ libsss_krb5_common.la \
> >+ libsss_test_common.la
> Please change the order of libs.
> libsss_idmap.la should be after libsss_ldap_common.la, because linking will
> fail with LDFLAGS=-Wl,--as-needed
>
> ./.libs/libsss_ldap_common.so: undefined reference to `is_domain_sid'
> ./.libs/libsss_ldap_common.so: undefined reference to
> `sss_idmap_bin_sid_to_sid'
> ./.libs/libsss_ldap_common.so: undefined reference to `idmap_error_string'
> ./.libs/libsss_ldap_common.so: undefined reference to
> `sss_idmap_ctx_set_autorid'
> ./.libs/libsss_ldap_common.so: undefined reference to
> `sss_idmap_ctx_set_rangesize'
> ./.libs/libsss_ldap_common.so: undefined reference to
> `sss_idmap_domain_by_name_has_algorithmic_mapping'
> ./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_init'
> ./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_sid_to_unix'
> ./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_set_upper'
> ./.libs/libsss_ldap_common.so: undefined reference to
> `sss_idmap_calculate_range'
> ./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_set_lower'
> ./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_add_domain_ex'
> ./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_get_upper'
> ./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_unix_to_sid'
> ./.libs/libsss_ldap_common.so: undefined reference to
> `sss_idmap_domain_has_algorithmic_mapping'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
>
> LS
Thank you for the review. I squashed the patches in.
-------------- next part --------------
>From 3f115bad21ce47bb7dee4541982613c0212ed229 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 6 Oct 2013 20:23:07 +0200
Subject: [PATCH 1/6] LDAP: Amend sdap_access_check to allow any connection
Related:
https://fedorahosted.org/sssd/ticket/2082
Also move the check for subdomain to the handler. I think it is the job
of the handler to decide which domain the request belongs to, not the
request itself.
---
src/providers/ad/ad_access.c | 4 +++-
src/providers/ipa/ipa_access.c | 13 +++++++++++--
src/providers/ldap/ldap_access.c | 11 ++++++++++-
src/providers/ldap/sdap_access.c | 40 ++++++++++++++++++----------------------
src/providers/ldap/sdap_access.h | 2 ++
5 files changed, 44 insertions(+), 26 deletions(-)
diff --git a/src/providers/ad/ad_access.c b/src/providers/ad/ad_access.c
index 746017dc5f6caa29128cc7404bcaa93359ef5766..cf6412f22c6146b26090646b5b8d060efa43365f 100644
--- a/src/providers/ad/ad_access.c
+++ b/src/providers/ad/ad_access.c
@@ -56,7 +56,9 @@ ad_access_handler(struct be_req *breq)
/* Verify that the account is not locked */
req = sdap_access_send(breq, be_ctx->ev, be_ctx, domain,
- access_ctx->sdap_access_ctx, pd);
+ access_ctx->sdap_access_ctx,
+ access_ctx->sdap_access_ctx->id_ctx->conn,
+ pd);
if (!req) {
be_req_terminate(breq, DP_ERR_FATAL, PAM_SYSTEM_ERR, NULL);
return;
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 7d44788af98350f47358488fc3840e675b2827d8..f067b7021d8afc483385ef9b8176242eca42988a 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -78,6 +78,7 @@ void ipa_access_handler(struct be_req *be_req)
struct pam_data *pd;
struct ipa_access_ctx *ipa_access_ctx;
struct tevent_req *req;
+ struct sss_domain_info *dom;
struct be_ctx *be_ctx = be_req_get_be_ctx(be_req);
pd = talloc_get_type(be_req_get_data(be_req), struct pam_data);
@@ -85,13 +86,21 @@ void ipa_access_handler(struct be_req *be_req)
ipa_access_ctx = talloc_get_type(be_ctx->bet_info[BET_ACCESS].pvt_bet_data,
struct ipa_access_ctx);
+ dom = be_ctx->domain;
+ if (strcasecmp(pd->domain, be_ctx->domain->name) != 0) {
+ /* Subdomain request, verify subdomain */
+ dom = find_subdomain_by_name(be_ctx->domain, pd->domain, true);
+ }
+
/* First, verify that this account isn't locked.
* We need to do this in case the auth phase was
* skipped (such as during GSSAPI single-sign-on
* or SSH public key exchange.
*/
- req = sdap_access_send(be_req, be_ctx->ev, be_ctx, be_ctx->domain,
- ipa_access_ctx->sdap_access_ctx, pd);
+ req = sdap_access_send(be_req, be_ctx->ev, be_ctx, dom,
+ ipa_access_ctx->sdap_access_ctx,
+ ipa_access_ctx->sdap_access_ctx->id_ctx->conn,
+ pd);
if (!req) {
be_req_terminate(be_req, DP_ERR_FATAL, PAM_SYSTEM_ERR, NULL);
return;
diff --git a/src/providers/ldap/ldap_access.c b/src/providers/ldap/ldap_access.c
index 8d5b5e22865219e13d0a090f0da4bce8ae06a234..c468a1cd53a607dc5b1ebe3635ae24286b7d6604 100644
--- a/src/providers/ldap/ldap_access.c
+++ b/src/providers/ldap/ldap_access.c
@@ -50,6 +50,7 @@ void sdap_pam_access_handler(struct be_req *breq)
struct pam_data *pd;
struct tevent_req *req;
struct sdap_access_ctx *access_ctx;
+ struct sss_domain_info *dom;
pd = talloc_get_type(be_req_get_data(breq), struct pam_data);
@@ -57,8 +58,16 @@ void sdap_pam_access_handler(struct be_req *breq)
talloc_get_type(be_ctx->bet_info[BET_ACCESS].pvt_bet_data,
struct sdap_access_ctx);
+ dom = be_ctx->domain;
+ if (strcasecmp(pd->domain, be_ctx->domain->name) != 0) {
+ /* Subdomain request, verify subdomain */
+ dom = find_subdomain_by_name(be_ctx->domain, pd->domain, true);
+ }
+
req = sdap_access_send(breq, be_ctx->ev, be_ctx,
- be_ctx->domain, access_ctx, pd);
+ dom, access_ctx,
+ access_ctx->id_ctx->conn,
+ pd);
if (req == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to start sdap_access request\n"));
sdap_access_reply(breq, PAM_SYSTEM_ERR);
diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c
index 9d1315fab9b838e3f955e82c38ce88a97cb9cd0a..267a2b863cc1e944998ee53ca985d17d2cef715d 100644
--- a/src/providers/ldap/sdap_access.c
+++ b/src/providers/ldap/sdap_access.c
@@ -45,6 +45,7 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
struct be_ctx *be_ctx,
struct sss_domain_info *domain,
struct sdap_access_ctx *access_ctx,
+ struct sdap_id_conn_ctx *conn,
const char *username,
struct ldb_message *user_entry);
static errno_t sdap_access_filter_recv(struct tevent_req *req);
@@ -62,6 +63,7 @@ struct sdap_access_req_ctx {
struct pam_data *pd;
struct tevent_context *ev;
struct sdap_access_ctx *access_ctx;
+ struct sdap_id_conn_ctx *conn;
struct be_ctx *be_ctx;
struct sss_domain_info *domain;
struct ldb_message *user_entry;
@@ -78,6 +80,7 @@ sdap_access_send(TALLOC_CTX *mem_ctx,
struct be_ctx *be_ctx,
struct sss_domain_info *domain,
struct sdap_access_ctx *access_ctx,
+ struct sdap_id_conn_ctx *conn,
struct pam_data *pd)
{
errno_t ret;
@@ -85,7 +88,6 @@ sdap_access_send(TALLOC_CTX *mem_ctx,
struct tevent_req *req;
struct ldb_result *res;
const char *attrs[] = { "*", NULL };
- struct sss_domain_info *user_dom;
req = tevent_req_create(mem_ctx, &state, struct sdap_access_req_ctx);
if (req == NULL) {
@@ -98,6 +100,7 @@ sdap_access_send(TALLOC_CTX *mem_ctx,
state->pd = pd;
state->ev = ev;
state->access_ctx = access_ctx;
+ state->conn = conn;
state->current_rule = 0;
DEBUG(6, ("Performing access check for user [%s]\n", pd->user));
@@ -108,20 +111,9 @@ sdap_access_send(TALLOC_CTX *mem_ctx,
goto done;
}
- /* Get original user DN, take care of subdomain users as well */
- if (strcasecmp(pd->domain, be_ctx->domain->name) != 0) {
- user_dom = find_subdomain_by_name(be_ctx->domain, pd->domain, true);
- if (user_dom == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, ("find_subdomain_by_name failed.\n"));
- ret = ENOMEM;
- goto done;
- }
- ret = sysdb_get_user_attr(state, user_dom->sysdb, user_dom,
- pd->user, attrs, &res);
- } else {
- ret = sysdb_get_user_attr(state, domain->sysdb, domain,
- pd->user, attrs, &res);
- }
+ /* Get original user DN, domain already points to the right (sub)domain */
+ ret = sysdb_get_user_attr(state, domain->sysdb, domain,
+ pd->user, attrs, &res);
if (ret != EOK) {
if (ret == ENOENT) {
/* If we can't find the user, return access denied */
@@ -177,6 +169,7 @@ static errno_t check_next_rule(struct sdap_access_req_ctx *state,
subreq = sdap_access_filter_send(state, state->ev, state->be_ctx,
state->domain,
state->access_ctx,
+ state->conn,
state->pd->user,
state->user_entry);
if (subreq == NULL) {
@@ -624,7 +617,8 @@ struct sdap_access_filter_req_ctx {
const char *filter;
struct tevent_context *ev;
struct sdap_access_ctx *access_ctx;
- struct sdap_id_ctx *sdap_ctx;
+ struct sdap_options *opts;
+ struct sdap_id_conn_ctx *conn;
struct sdap_id_op *sdap_op;
struct sysdb_handle *handle;
struct sss_domain_info *domain;
@@ -641,6 +635,7 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
struct be_ctx *be_ctx,
struct sss_domain_info *domain,
struct sdap_access_ctx *access_ctx,
+ struct sdap_id_conn_ctx *conn,
const char *username,
struct ldb_message *user_entry)
{
@@ -664,7 +659,8 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
state->filter = NULL;
state->username = username;
- state->sdap_ctx = access_ctx->id_ctx;
+ state->opts = access_ctx->id_ctx->opts;
+ state->conn = conn;
state->ev = ev;
state->access_ctx = access_ctx;
state->domain = domain;
@@ -707,9 +703,9 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
state->filter = talloc_asprintf(
state,
"(&(%s=%s)(objectclass=%s)%s)",
- state->sdap_ctx->opts->user_map[SDAP_AT_USER_NAME].name,
+ state->opts->user_map[SDAP_AT_USER_NAME].name,
clean_username,
- state->sdap_ctx->opts->user_map[SDAP_OC_USER].name,
+ state->opts->user_map[SDAP_OC_USER].name,
state->access_ctx->filter);
if (state->filter == NULL) {
DEBUG(0, ("Could not construct access filter\n"));
@@ -721,7 +717,7 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
DEBUG(6, ("Checking filter against LDAP\n"));
state->sdap_op = sdap_id_op_create(state,
- state->sdap_ctx->conn->conn_cache);
+ state->conn->conn_cache);
if (!state->sdap_op) {
DEBUG(2, ("sdap_id_op_create failed\n"));
ret = ENOMEM;
@@ -805,13 +801,13 @@ static void sdap_access_filter_connect_done(struct tevent_req *subreq)
*/
subreq = sdap_get_generic_send(state,
state->ev,
- state->sdap_ctx->opts,
+ state->opts,
sdap_id_op_handle(state->sdap_op),
state->basedn,
LDAP_SCOPE_BASE,
state->filter, NULL,
NULL, 0,
- dp_opt_get_int(state->sdap_ctx->opts->basic,
+ dp_opt_get_int(state->opts->basic,
SDAP_SEARCH_TIMEOUT),
false);
if (subreq == NULL) {
diff --git a/src/providers/ldap/sdap_access.h b/src/providers/ldap/sdap_access.h
index cda078688b19182112d27e605008f6406a604daa..30097e21f59abe19fc14d052edb0a6343c67f892 100644
--- a/src/providers/ldap/sdap_access.h
+++ b/src/providers/ldap/sdap_access.h
@@ -26,6 +26,7 @@
#define SDAP_ACCESS_H_
#include "providers/dp_backend.h"
+#include "providers/ldap/ldap_common.h"
#define SYSDB_LDAP_ACCESS_FILTER "ldap_access_filter_allow"
@@ -62,6 +63,7 @@ sdap_access_send(TALLOC_CTX *mem_ctx,
struct be_ctx *be_ctx,
struct sss_domain_info *domain,
struct sdap_access_ctx *access_ctx,
+ struct sdap_id_conn_ctx *conn,
struct pam_data *pd);
errno_t sdap_access_recv(struct tevent_req *req);
--
1.8.3.1
-------------- next part --------------
>From ffeaf255263671f8cc43bab3aec7b1304e076705 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 8 Oct 2013 19:10:41 +0200
Subject: [PATCH 2/6] LDAP: Parse FQDN into name/domain for subdomain users
Related:
https://fedorahosted.org/sssd/ticket/2082
When a subdomain user logs in, the username the account request receives
is a FQDN. This hackish patch parses the FQDN and only uses the name to
search the LDAP.
---
src/providers/ldap/sdap_access.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c
index 267a2b863cc1e944998ee53ca985d17d2cef715d..6b387271a229668ddfa5d67143a585e667a16ddd 100644
--- a/src/providers/ldap/sdap_access.c
+++ b/src/providers/ldap/sdap_access.c
@@ -644,6 +644,8 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
const char *basedn;
char *clean_username;
errno_t ret = ERR_INTERNAL;
+ char *name;
+ char *domname;
req = tevent_req_create(mem_ctx, &state, struct sdap_access_filter_req_ctx);
if (req == NULL) {
@@ -694,8 +696,16 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
}
/* Construct the filter */
+ /* Subdomain users are identified by FQDN. We need to use just the username */
+ ret = sss_parse_name(state, domain->names, username, &domname, &name);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Could not parse [%s] into name and "
+ "domain components, access might fail\n", username));
+ name = discard_const(username);
+ }
- ret = sss_filter_sanitize(state, state->username, &clean_username);
+ ret = sss_filter_sanitize(state, name, &clean_username);
if (ret != EOK) {
goto done;
}
--
1.8.3.1
-------------- next part --------------
>From 7b7532f68058dfad4122890743041973bb6bdfc6 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 2 Oct 2013 17:48:49 +0200
Subject: [PATCH 3/6] AD: Add a new option ad_access_filter
This patch just adds the option, it doesn't do anything useful yet.
Related:
https://fedorahosted.org/sssd/ticket/2082
---
src/config/etc/sssd.api.d/sssd-ad.conf | 1 +
src/man/sssd-ad.5.xml | 24 ++++++++++++++++++++++++
src/providers/ad/ad_common.h | 1 +
src/providers/ad/ad_opts.h | 1 +
4 files changed, 27 insertions(+)
diff --git a/src/config/etc/sssd.api.d/sssd-ad.conf b/src/config/etc/sssd.api.d/sssd-ad.conf
index 120c827523d5b103396b3a38bca8cc75b25d0cc2..9f606f6c4da65d4bfb20a97ee27801dac9307868 100644
--- a/src/config/etc/sssd.api.d/sssd-ad.conf
+++ b/src/config/etc/sssd.api.d/sssd-ad.conf
@@ -4,6 +4,7 @@ ad_server = str, None, false
ad_backup_server = str, None, false
ad_hostname = str, None, false
ad_enable_dns_sites = bool, None, false
+ad_access_filter = str, None, false
ldap_uri = str, None, false
ldap_backup_uri = str, None, false
ldap_search_base = str, None, false
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index 3aa865ab110bf93afb08ccf2ee8ca7e62a86a3f7..41bd3ba0536bcc3038eea6befecf84f882aa5d30 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -169,6 +169,30 @@ ldap_id_mapping = False
</varlistentry>
<varlistentry>
+ <term>ad_access_filter (boolean)</term>
+ <listitem>
+ <para>
+ This option specifies LDAP access control
+ filter that the user must match in order
+ to be allowed access. Please note that the
+ <quote>access_filter</quote> option must be
+ explicitly set to <quote>ad</quote> in order
+ for this option to have an effect.
+ </para>
+ <para>
+ Example:
+ </para>
+ <programlisting>
+access_provider = ad
+ad_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com
+ </programlisting>
+ <para>
+ Default: Not set
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>dyndns_update (boolean)</term>
<listitem>
<para>
diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index 98aeb2165273b5dbfafd3b8757180d74b4504c98..b8b73c042b8a5433f720c89c04447c07cd3eac43 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -41,6 +41,7 @@ enum ad_basic_opt {
AD_KEYTAB,
AD_KRB5_REALM,
AD_ENABLE_DNS_SITES,
+ AD_ACCESS_FILTER,
AD_OPTS_BASIC /* opts counter */
};
diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h
index f3b6cd61632601ec92a408a6a73cff6446fd48bf..8022a16274a04389b7a64b491ec28a0c3c55aaef 100644
--- a/src/providers/ad/ad_opts.h
+++ b/src/providers/ad/ad_opts.h
@@ -35,6 +35,7 @@ struct dp_option ad_basic_opts[] = {
{ "krb5_keytab", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "krb5_realm", DP_OPT_STRING, NULL_STRING, NULL_STRING},
{ "ad_enable_dns_sites", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
+ { "ad_access_filter", DP_OPT_STRING, NULL_STRING, NULL_STRING},
DP_OPTION_TERMINATOR
};
--
1.8.3.1
-------------- next part --------------
>From 1daec9096cc5ae8690b3fe567713cf9ef23096e7 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 7 Oct 2013 18:02:04 +0200
Subject: [PATCH 4/6] AD: Use the ad_access_filter if it's set
Related:
https://fedorahosted.org/sssd/ticket/2082
Currently the AD access control only checks if an account has been
expired. This patch amends the logic so that if ad_access_filter is set,
it is used automatically.
---
src/providers/ad/ad_init.c | 22 ++++++++++++++++++++--
src/providers/ldap/ldap_common.c | 19 +++++++++++++++++++
src/providers/ldap/ldap_common.h | 3 +++
src/providers/ldap/ldap_init.c | 24 +++++++-----------------
4 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index c829cc861a421db35b1e5f7a91941e20a3684ee6..d744c2a05cffa93a02a2c771ebd80725b0220a5b 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -366,6 +366,7 @@ sssm_ad_access_init(struct be_ctx *bectx,
errno_t ret;
struct ad_access_ctx *access_ctx;
struct ad_id_ctx *ad_id_ctx;
+ const char *filter;
access_ctx = talloc_zero(bectx, struct ad_access_ctx);
if (!access_ctx) return ENOMEM;
@@ -392,10 +393,27 @@ sssm_ad_access_init(struct be_ctx *bectx,
ret = ENOMEM;
goto fail;
}
-
access_ctx->sdap_access_ctx->id_ctx = access_ctx->sdap_ctx;
+
+ /* If ad_access_filter is set, the value of ldap_acess_order is
+ * expire, filter, otherwise only expire
+ */
access_ctx->sdap_access_ctx->access_rule[0] = LDAP_ACCESS_EXPIRE;
- access_ctx->sdap_access_ctx->access_rule[1] = LDAP_ACCESS_EMPTY;
+ filter = dp_opt_get_cstring(access_ctx->ad_options, AD_ACCESS_FILTER);
+ if (filter != NULL) {
+ access_ctx->sdap_access_ctx->filter = sdap_get_access_filter(
+ access_ctx->sdap_access_ctx,
+ filter);
+ if (access_ctx->sdap_access_ctx->filter == NULL) {
+ ret = ENOMEM;
+ goto fail;
+ }
+
+ access_ctx->sdap_access_ctx->access_rule[1] = LDAP_ACCESS_FILTER;
+ access_ctx->sdap_access_ctx->access_rule[2] = LDAP_ACCESS_EMPTY;
+ } else {
+ access_ctx->sdap_access_ctx->access_rule[1] = LDAP_ACCESS_EMPTY;
+ }
*ops = &ad_access_ops;
*pvt_data = access_ctx;
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index b3a048c1cb7336cec0a5bbfebd3509c56637bed3..e5c457c24514444553f407c1de32d77cab3f7231 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -1757,6 +1757,25 @@ char *sdap_get_id_specific_filter(TALLOC_CTX *mem_ctx,
return filter; /* NULL or not */
}
+char *sdap_get_access_filter(TALLOC_CTX *mem_ctx,
+ const char *base_filter)
+{
+ char *filter = NULL;
+
+ if (base_filter == NULL) return NULL;
+
+ if (base_filter[0] == '(') {
+ /* This filter is wrapped in parentheses.
+ * Pass it as-is to the openldap libraries.
+ */
+ filter = talloc_strdup(mem_ctx, base_filter);
+ } else {
+ filter = talloc_asprintf(mem_ctx, "(%s)", base_filter);
+ }
+
+ return filter;
+}
+
errno_t
sdap_attrs_get_sid_str(TALLOC_CTX *mem_ctx,
struct sdap_idmap_ctx *idmap_ctx,
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index fb9a34c6067521f8d28fb403b97ca6394918f303..0bd6c9cc2116db3f7c024c0d3cfc0035eee32357 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -261,6 +261,9 @@ char *sdap_get_id_specific_filter(TALLOC_CTX *mem_ctx,
const char *base_filter,
const char *extra_filter);
+char *sdap_get_access_filter(TALLOC_CTX *mem_ctx,
+ const char *base_filter);
+
errno_t msgs2attrs_array(TALLOC_CTX *mem_ctx, size_t count,
struct ldb_message **msgs,
struct sysdb_attrs ***attrs);
diff --git a/src/providers/ldap/ldap_init.c b/src/providers/ldap/ldap_init.c
index 341338ca491593d906cff1ee71b040c2d0dea38b..17874b1325d3e5bf2eded62c6f022364b84a5c2a 100644
--- a/src/providers/ldap/ldap_init.c
+++ b/src/providers/ldap/ldap_init.c
@@ -341,7 +341,7 @@ int sssm_ldap_access_init(struct be_ctx *bectx,
access_ctx->access_rule[c] = LDAP_ACCESS_FILTER;
filter = dp_opt_get_cstring(access_ctx->id_ctx->opts->basic,
- SDAP_ACCESS_FILTER);
+ SDAP_ACCESS_FILTER);
if (filter == NULL) {
/* It's okay if this is NULL. In that case we will simply act
* like the 'deny' provider.
@@ -349,24 +349,14 @@ int sssm_ldap_access_init(struct be_ctx *bectx,
DEBUG(0, ("Warning: LDAP access rule 'filter' is set, "
"but no ldap_access_filter configured. "
"All domain users will be denied access.\n"));
- }
- else {
- if (filter[0] == '(') {
- /* This filter is wrapped in parentheses.
- * Pass it as-is to the openldap libraries.
- */
- access_ctx->filter = filter;
- }
- else {
- /* Add parentheses around the filter */
- access_ctx->filter = talloc_asprintf(access_ctx, "(%s)", filter);
- if (access_ctx->filter == NULL) {
- ret = ENOMEM;
- goto done;
- }
+ } else {
+ access_ctx->filter = sdap_get_access_filter(access_ctx,
+ filter);
+ if (access_ctx->filter == NULL) {
+ ret = ENOMEM;
+ goto done;
}
}
-
} else if (strcasecmp(order_list[c], LDAP_ACCESS_EXPIRE_NAME) == 0) {
access_ctx->access_rule[c] = LDAP_ACCESS_EXPIRE;
--
1.8.3.1
-------------- next part --------------
>From edf02a62ff95f58005d230a27d8c34424416ab20 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 8 Oct 2013 17:50:56 +0200
Subject: [PATCH 5/6] AD: Search GC by default during access control, fall back
to LDAP
Resolves:
https://fedorahosted.org/sssd/ticket/2082
In order to allow the ad_access_filter option to work for subdomain
users as well, the Global Catalog must be searched. This patch adds a
wrapper request atop sdap_access_send that selects the right connection
(GC or LDAP) and optionally falls back to LDAP.
---
src/providers/ad/ad_access.c | 164 +++++++++++++++++++++++++++++++++++++++++--
src/providers/ad/ad_access.h | 4 +-
src/providers/ad/ad_init.c | 5 +-
3 files changed, 163 insertions(+), 10 deletions(-)
diff --git a/src/providers/ad/ad_access.c b/src/providers/ad/ad_access.c
index cf6412f22c6146b26090646b5b8d060efa43365f..51c872c3be83a5ed2b3bb79f174e5402907372cb 100644
--- a/src/providers/ad/ad_access.c
+++ b/src/providers/ad/ad_access.c
@@ -25,10 +25,162 @@
#include "src/providers/data_provider.h"
#include "src/providers/dp_backend.h"
#include "src/providers/ad/ad_access.h"
+#include "src/providers/ad/ad_common.h"
#include "src/providers/ldap/sdap_access.h"
static void
ad_access_done(struct tevent_req *req);
+static errno_t
+ad_access_step(struct tevent_req *req, struct sdap_id_conn_ctx *conn);
+
+struct ad_access_state {
+ struct tevent_context *ev;
+ struct ad_access_ctx *ctx;
+ struct pam_data *pd;
+ struct be_ctx *be_ctx;
+ struct sss_domain_info *domain;
+
+ struct sdap_id_conn_ctx **clist;
+ int cindex;
+};
+
+static struct tevent_req *
+ad_access_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct be_ctx *be_ctx,
+ struct sss_domain_info *domain,
+ struct ad_access_ctx *ctx,
+ struct pam_data *pd)
+{
+ struct tevent_req *req;
+ struct ad_access_state *state;
+ errno_t ret;
+
+ req = tevent_req_create(mem_ctx, &state, struct ad_access_state);
+ if (req == NULL) {
+ return NULL;
+ }
+
+ state->ev = ev;
+ state->ctx = ctx;
+ state->pd = pd;
+ state->be_ctx = be_ctx;
+ state->domain = domain;
+
+ state->clist = talloc_zero_array(state, struct sdap_id_conn_ctx *, 3);
+ if (state->clist == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ /* Always try GC first */
+ ctx->gc_ctx->ignore_mark_offline = false;
+ state->clist[0] = ctx->gc_ctx;
+ if (IS_SUBDOMAIN(domain) == false) {
+ /* fall back to ldap if gc is not available */
+ state->clist[0]->ignore_mark_offline = true;
+
+ /* With root domain users we have the option to
+ * fall back to LDAP in case ie POSIX attributes
+ * are used but not replicated to GC
+ */
+ state->clist[1] = ctx->ldap_ctx;
+ }
+
+ ret = ad_access_step(req, state->clist[state->cindex]);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ ret = EOK;
+done:
+ if (ret != EOK) {
+ tevent_req_error(req, ret);
+ tevent_req_post(req, ev);
+ }
+ return req;
+}
+
+static errno_t
+ad_access_step(struct tevent_req *req, struct sdap_id_conn_ctx *conn)
+{
+ struct tevent_req *subreq;
+ struct ad_access_state *state;
+
+ state = tevent_req_data(req, struct ad_access_state);
+
+ subreq = sdap_access_send(req, state->ev, state->be_ctx,
+ state->domain, state->ctx->sdap_access_ctx,
+ conn, state->pd);
+ if (req == NULL) {
+ return ENOMEM;
+ }
+ tevent_req_set_callback(subreq, ad_access_done, req);
+ return EOK;
+}
+
+static void
+ad_access_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req;
+ struct ad_access_state *state;
+ errno_t ret;
+
+ req = tevent_req_callback_data(subreq, struct tevent_req);
+ state = tevent_req_data(req, struct ad_access_state);
+
+ ret = sdap_access_recv(subreq);
+ talloc_zfree(subreq);
+ switch (ret) {
+ case EOK:
+ tevent_req_done(req);
+ return;
+
+ case ERR_ACCOUNT_EXPIRED:
+ tevent_req_error(req, ret);
+ return;
+
+ case ERR_ACCESS_DENIED:
+ /* Retry on ACCESS_DENIED, too, to make sure that we don't
+ * miss out any attributes not present in GC
+ * FIXME - this is slow. We should retry only if GC failed
+ * and LDAP succeeded after the first ACCESS_DENIED
+ */
+ break;
+
+ default:
+ break;
+ }
+
+ /* If possible, retry with LDAP */
+ state->cindex++;
+ if (state->clist[state->cindex] == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Error retrieving access check result: %s\n",
+ sss_strerror(ret)));
+ tevent_req_error(req, ret);
+ return;
+ }
+
+ ret = ad_access_step(req, state->clist[state->cindex]);
+ if (ret != EOK) {
+ tevent_req_error(req, ret);
+ return;
+ }
+
+ /* Another check in progress */
+}
+
+static errno_t
+ad_access_recv(struct tevent_req *req)
+{
+ TEVENT_REQ_RETURN_ON_ERROR(req);
+
+ return EOK;
+}
+
+static void
+ad_access_check_done(struct tevent_req *req);
void
ad_access_handler(struct be_req *breq)
@@ -55,19 +207,17 @@ ad_access_handler(struct be_req *breq)
}
/* Verify that the account is not locked */
- req = sdap_access_send(breq, be_ctx->ev, be_ctx, domain,
- access_ctx->sdap_access_ctx,
- access_ctx->sdap_access_ctx->id_ctx->conn,
- pd);
+ req = ad_access_send(breq, be_ctx->ev, be_ctx, domain,
+ access_ctx, pd);
if (!req) {
be_req_terminate(breq, DP_ERR_FATAL, PAM_SYSTEM_ERR, NULL);
return;
}
- tevent_req_set_callback(req, ad_access_done, breq);
+ tevent_req_set_callback(req, ad_access_check_done, breq);
}
static void
-ad_access_done(struct tevent_req *req)
+ad_access_check_done(struct tevent_req *req)
{
errno_t ret;
struct be_req *breq =
@@ -75,7 +225,7 @@ ad_access_done(struct tevent_req *req)
struct pam_data *pd =
talloc_get_type(be_req_get_data(breq), struct pam_data);
- ret = sdap_access_recv(req);
+ ret = ad_access_recv(req);
talloc_zfree(req);
switch (ret) {
case EOK:
diff --git a/src/providers/ad/ad_access.h b/src/providers/ad/ad_access.h
index 62259265dfea8083dbb0a98f64e424bb819c6d04..ca5e69729c574be53b7da04df0ff89446da04c58 100644
--- a/src/providers/ad/ad_access.h
+++ b/src/providers/ad/ad_access.h
@@ -24,9 +24,11 @@
#define AD_ACCESS_H_
struct ad_access_ctx {
- struct sdap_id_ctx *sdap_ctx;
struct dp_option *ad_options;
struct sdap_access_ctx *sdap_access_ctx;
+
+ struct sdap_id_conn_ctx *ldap_ctx;
+ struct sdap_id_conn_ctx *gc_ctx;
};
void
diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index d744c2a05cffa93a02a2c771ebd80725b0220a5b..d7f41a563442332743b21f1e1cebdd0ee4abf5eb 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -375,7 +375,8 @@ sssm_ad_access_init(struct be_ctx *bectx,
if (ret != EOK) {
goto fail;
}
- access_ctx->sdap_ctx = ad_id_ctx->sdap_id_ctx;
+ access_ctx->ldap_ctx = ad_id_ctx->ldap_ctx;
+ access_ctx->gc_ctx = ad_id_ctx->gc_ctx;
ret = dp_copy_options(access_ctx, ad_options->basic, AD_OPTS_BASIC,
&access_ctx->ad_options);
@@ -393,7 +394,7 @@ sssm_ad_access_init(struct be_ctx *bectx,
ret = ENOMEM;
goto fail;
}
- access_ctx->sdap_access_ctx->id_ctx = access_ctx->sdap_ctx;
+ access_ctx->sdap_access_ctx->id_ctx = ad_id_ctx->sdap_id_ctx;
/* If ad_access_filter is set, the value of ldap_acess_order is
* expire, filter, otherwise only expire
--
1.8.3.1
-------------- next part --------------
>From f247804c292bb3004a00859e460e4240fbc4c0a8 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 8 Oct 2013 20:59:22 +0200
Subject: [PATCH 6/6] AD: Add extended access filter
https://fedorahosted.org/sssd/ticket/2082
Adds a new option that allows the admin to specify a LDAP access filter
that can be applied globally, per-domain or per-forest.
---
Makefile.am | 29 ++-
src/man/sssd-ad.5.xml | 41 +++-
src/providers/ad/ad_access.c | 213 ++++++++++++++++++-
src/providers/ad/ad_init.c | 5 +-
src/tests/cmocka/test_ad_access_filter.c | 341 +++++++++++++++++++++++++++++++
5 files changed, 623 insertions(+), 6 deletions(-)
create mode 100644 src/tests/cmocka/test_ad_access_filter.c
diff --git a/Makefile.am b/Makefile.am
index 8d548e2bd7f5ffa1d9d7a46dc08026213e8e80bc..f12e44495c2be855e8b6f815961c7eb77b812fdf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -153,7 +153,8 @@ if HAVE_CMOCKA
dyndns-tests \
fqnames-tests \
test_sss_idmap \
- test_utils
+ test_utils \
+ ad_access_filter_tests
endif
check_PROGRAMS = \
@@ -1389,6 +1390,32 @@ test_utils_LDADD = \
$(CMOCKA_LIBS) \
$(SSSD_INTERNAL_LTLIBS) \
libsss_test_common.la
+
+ad_access_filter_tests_SOURCES = \
+ $(TEST_MOCK_OBJ) \
+ $(sssd_be_SOURCES) \
+ src/util/sss_ldap.c \
+ src/util/sss_krb5.c \
+ src/util/find_uid.c \
+ src/util/user_info_msg.c \
+ src/tests/cmocka/test_ad_access_filter.c
+ad_access_filter_tests_CFLAGS = \
+ $(AM_CFLAGS) \
+ $(SYSTEMD_LOGIN_CFLAGS) \
+ -DUNIT_TESTING
+ad_access_filter_tests_LDADD = \
+ $(PAM_LIBS) \
+ $(CMOCKA_LIBS) \
+ $(SSSD_LIBS) \
+ $(CARES_LIBS) \
+ $(KRB5_LIBS) \
+ $(SSSD_INTERNAL_LTLIBS) \
+ $(SYSTEMD_LOGIN_LIBS) \
+ libsss_ldap_common.la \
+ libsss_idmap.la \
+ libsss_krb5_common.la \
+ libsss_test_common.la
+
endif
noinst_PROGRAMS = pam_test_client
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index 41bd3ba0536bcc3038eea6befecf84f882aa5d30..64231cc450ab8b0fb81decfcf27d50ad5e6b9155 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -180,11 +180,46 @@ ldap_id_mapping = False
for this option to have an effect.
</para>
<para>
- Example:
+ The option also supports specifying different
+ filters per domain or forest. This
+ extended filter would consist of:
+ <quote>KEYWORD:NAME:FILTER</quote>.
+ The keyword can be either <quote>DOM</quote>,
+ <quote>FOREST</quote> or missing.
+ </para>
+ <para>
+ If the keyword equals to <quote>DOM</quote>
+ or is missing, then <quote>NAME</quote> specifies
+ the domain or subdomain the filter applies to.
+ If the keyword equals to <quote>FOREST</quote>,
+ then the filter equals to all domains from the
+ forest specified by <quote>NAME</quote>.
+ </para>
+ <para>
+ Multiple filters can be separated with the
+ <quote>?</quote> character, similarly to how
+ search bases work.
+ </para>
+ <para>
+ The most specific match is always used. For
+ example, if the option specified filter
+ for a domain the user is a member of and a
+ global filter, the per-domain filter would
+ be applied. If there are more matches with
+ the same specification, the first one is used.
+ </para>
+ <para>
+ Examples:
</para>
<programlisting>
-access_provider = ad
-ad_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com
+# apply filter on domain called dom1 only:
+dom1:(memberOf=cn=admins,ou=groups,dc=dom1,dc=com)
+
+# apply filter on domain called dom2 only:
+DOM:dom2:(memberOf=cn=admins,ou=groups,dc=dom2,dc=com)
+
+# apply filter on forest called EXAMPLE.COM only:
+FOREST:EXAMPLE.COM:(memberOf=cn=admins,ou=groups,dc=example,dc=com)
</programlisting>
<para>
Default: Not set
diff --git a/src/providers/ad/ad_access.c b/src/providers/ad/ad_access.c
index 51c872c3be83a5ed2b3bb79f174e5402907372cb..c16f049424d65b50c306fad7e782570868a664ea 100644
--- a/src/providers/ad/ad_access.c
+++ b/src/providers/ad/ad_access.c
@@ -33,6 +33,196 @@ ad_access_done(struct tevent_req *req);
static errno_t
ad_access_step(struct tevent_req *req, struct sdap_id_conn_ctx *conn);
+/*
+ * More advanced format can be used to restrict the filter to a specific
+ * domain or a specific forest. This format is KEYWORD:NAME:FILTER
+ *
+ * KEYWORD can be one of DOM or FOREST
+ * KEYWORD can be missing
+ * NAME is a label.
+ * - if KEYWORD equals DOM or missing completely, the filter is applied
+ * for users from domain named NAME only
+ * - if KEYWORD equals FOREST, the filter is applied on users from
+ * forest named NAME only
+ * examples of valid filters are:
+ * apply filter on domain called dom1 only:
+ * dom1:(memberOf=cn=admins,ou=groups,dc=dom1,dc=com)
+ * apply filter on domain called dom2 only:
+ * DOM:dom2:(memberOf=cn=admins,ou=groups,dc=dom2,dc=com)
+ * apply filter on forest called EXAMPLE.COM only:
+ * FOREST:EXAMPLE.COM:(memberOf=cn=admins,ou=groups,dc=example,dc=com)
+ *
+ * If any of the extended formats are used, the filter MUST be enclosed
+ * already.
+ */
+
+/* From least specific */
+#define AD_FILTER_GENERIC 0x01
+#define AD_FILTER_FOREST 0x02
+#define AD_FILTER_DOMAIN 0x04
+
+#define KW_FOREST "FOREST"
+#define KW_DOMAIN "DOM"
+
+/* parse filter in the format domain_name:filter */
+static errno_t
+parse_sub_filter(TALLOC_CTX *mem_ctx, const char *full_filter,
+ char **filter, char **sub_name, int *flags,
+ const int flagconst)
+{
+ char *specdelim;
+
+ specdelim = strchr(full_filter, ':');
+ if (specdelim == NULL) return EINVAL;
+
+ /* Make sure the filter is already enclosed in brackets */
+ if (*(specdelim+1) != '(') return EINVAL;
+
+ *sub_name = talloc_strndup(mem_ctx, full_filter, specdelim - full_filter);
+ *filter = talloc_strdup(mem_ctx, specdelim+1);
+ if (*sub_name == NULL || *filter == NULL) return ENOMEM;
+
+ *flags = flagconst;
+ return EOK;
+}
+
+static inline errno_t
+parse_dom_filter(TALLOC_CTX *mem_ctx, const char *dom_filter,
+ char **filter, char **domname, int *flags)
+{
+ return parse_sub_filter(mem_ctx, dom_filter, filter, domname,
+ flags, AD_FILTER_DOMAIN);
+}
+
+static inline errno_t
+parse_forest_filter(TALLOC_CTX *mem_ctx, const char *forest_filter,
+ char **filter, char **forest_name, int *flags)
+{
+ return parse_sub_filter(mem_ctx, forest_filter, filter, forest_name,
+ flags, AD_FILTER_FOREST);
+}
+
+
+static errno_t
+parse_filter(TALLOC_CTX *mem_ctx, const char *full_filter,
+ char **filter, char **spec, int *flags)
+{
+ char *kwdelim, *specdelim;
+
+ if (filter == NULL || spec == NULL || flags == NULL) return EINVAL;
+
+ kwdelim = strchr(full_filter, ':');
+ if (kwdelim != NULL) {
+ specdelim = strchr(kwdelim+1, ':');
+
+ if (specdelim == NULL) {
+ /* There is a single keyword. Treat it as a domain name */
+ return parse_dom_filter(mem_ctx, full_filter, filter, spec, flags);
+ } else if (strncmp(full_filter, "DOM", kwdelim-full_filter) == 0) {
+ /* The format must be DOM:domain_name:filter */
+ if (specdelim && specdelim-kwdelim <= 1) {
+ /* Check if there is some domain_name */
+ return EINVAL;
+ }
+
+ return parse_dom_filter(mem_ctx, kwdelim + 1, filter, spec, flags);
+ } else if (strncmp(full_filter, "FOREST", kwdelim-full_filter) == 0) {
+ /* The format must be FOREST:forest_name:filter */
+ if (specdelim && specdelim-kwdelim <= 1) {
+ /* Check if there is some domain_name */
+ return EINVAL;
+ }
+
+ return parse_forest_filter(mem_ctx, kwdelim + 1,
+ filter, spec, flags);
+ }
+
+ /* Malformed option */
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Keyword in filter [%s] did not match expected format\n",
+ full_filter));
+ return EINVAL;
+ }
+
+ /* No keyword. Easy. */
+ *filter = talloc_strdup(mem_ctx, full_filter);
+ if (*filter == NULL) return ENOMEM;
+
+ *spec = NULL;
+ *flags = AD_FILTER_GENERIC;
+ return EOK;
+}
+
+static errno_t
+ad_parse_access_filter(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *dom,
+ const char *filter_list,
+ char **_filter)
+{
+ char **filters;
+ int nfilters;
+ errno_t ret;
+ char *best_match;
+ int best_flags;
+ char *filter;
+ char *spec;
+ int flags;
+ TALLOC_CTX *tmp_ctx;
+
+ tmp_ctx = talloc_new(mem_ctx);
+ if (tmp_ctx == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = split_on_separator(tmp_ctx, filter_list, '?', true, true,
+ &filters, &nfilters);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Cannot parse the list of ad_access_filters\n"));
+ goto done;
+ }
+
+ best_match = NULL;
+ best_flags = 0;
+ for (int i=0; i < nfilters; i++) {
+ ret = parse_filter(tmp_ctx, filters[i], &filter, &spec, &flags);
+ if (ret != EOK) {
+ /* Skip the faulty filter. At worst, the user won't be
+ * allowed access */
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Access filter [%s] could not be "
+ "parsed, skipping\n", filters[i]));
+ continue;
+ }
+
+ if (flags & AD_FILTER_DOMAIN && strcasecmp(spec, dom->name) != 0) {
+ /* If the filter specifies a domain, it must match the
+ * domain the user comes from
+ */
+ continue;
+ }
+
+ if (flags & AD_FILTER_FOREST && strcasecmp(spec, dom->forest) != 0) {
+ /* If the filter specifies a forest, it must match the
+ * forest the user comes from
+ */
+ continue;
+ }
+
+ if (flags > best_flags) {
+ best_flags = flags;
+ best_match = filter;
+ }
+ }
+
+ ret = EOK;
+ /* Make sure the result is enclosed in brackets */
+ *_filter = sdap_get_access_filter(mem_ctx, best_match);
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
struct ad_access_state {
struct tevent_context *ev;
struct ad_access_ctx *ctx;
@@ -40,6 +230,7 @@ struct ad_access_state {
struct be_ctx *be_ctx;
struct sss_domain_info *domain;
+ char *filter;
struct sdap_id_conn_ctx **clist;
int cindex;
};
@@ -67,6 +258,14 @@ ad_access_send(TALLOC_CTX *mem_ctx,
state->be_ctx = be_ctx;
state->domain = domain;
+ ret = ad_parse_access_filter(state, domain, ctx->sdap_access_ctx->filter,
+ &state->filter);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Could not determine the best filter\n"));
+ ret = ERR_ACCESS_DENIED;
+ goto done;
+ }
+
state->clist = talloc_zero_array(state, struct sdap_id_conn_ctx *, 3);
if (state->clist == NULL) {
ret = ENOMEM;
@@ -106,13 +305,25 @@ ad_access_step(struct tevent_req *req, struct sdap_id_conn_ctx *conn)
{
struct tevent_req *subreq;
struct ad_access_state *state;
+ struct sdap_access_ctx *req_ctx;
state = tevent_req_data(req, struct ad_access_state);
+ req_ctx = talloc(state, struct sdap_access_ctx);
+ if (req_ctx == NULL) {
+ return ENOMEM;
+ }
+ req_ctx->id_ctx = state->ctx->sdap_access_ctx->id_ctx;
+ req_ctx->filter = state->filter;
+ memcpy(&req_ctx->access_rule,
+ state->ctx->sdap_access_ctx->access_rule,
+ sizeof(int) * LDAP_ACCESS_LAST);
+
subreq = sdap_access_send(req, state->ev, state->be_ctx,
- state->domain, state->ctx->sdap_access_ctx,
+ state->domain, req_ctx,
conn, state->pd);
if (req == NULL) {
+ talloc_free(req_ctx);
return ENOMEM;
}
tevent_req_set_callback(subreq, ad_access_done, req);
diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index d7f41a563442332743b21f1e1cebdd0ee4abf5eb..d06efbd082bd6bba74fb6616c7dd722c99244988 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -402,7 +402,10 @@ sssm_ad_access_init(struct be_ctx *bectx,
access_ctx->sdap_access_ctx->access_rule[0] = LDAP_ACCESS_EXPIRE;
filter = dp_opt_get_cstring(access_ctx->ad_options, AD_ACCESS_FILTER);
if (filter != NULL) {
- access_ctx->sdap_access_ctx->filter = sdap_get_access_filter(
+ /* The processing of the extended filter is performed during the access
+ * check itself
+ */
+ access_ctx->sdap_access_ctx->filter = talloc_strdup(
access_ctx->sdap_access_ctx,
filter);
if (access_ctx->sdap_access_ctx->filter == NULL) {
diff --git a/src/tests/cmocka/test_ad_access_filter.c b/src/tests/cmocka/test_ad_access_filter.c
new file mode 100644
index 0000000000000000000000000000000000000000..e31f2dc37d7f78b0fa871839a57bd62a09c53944
--- /dev/null
+++ b/src/tests/cmocka/test_ad_access_filter.c
@@ -0,0 +1,341 @@
+/*
+ Authors:
+ Jakub Hrozek <jhrozek at redhat.com>
+
+ Copyright (C) 2013 Red Hat
+
+ SSSD tests: AD access control filter tests
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <talloc.h>
+#include <tevent.h>
+#include <errno.h>
+#include <popt.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <ifaddrs.h>
+#include <arpa/inet.h>
+
+/* In order to access opaque types */
+#include "providers/ad/ad_access.c"
+
+#include "tests/cmocka/common_mock.h"
+
+#define DOM_NAME "parent_dom"
+
+struct ad_access_test_ctx {
+ struct sss_domain_info *dom;
+};
+
+static struct ad_access_test_ctx *test_ctx;
+
+void ad_access_filter_test_setup(void **state)
+{
+ assert_true(leak_check_setup());
+ test_ctx = talloc_zero(global_talloc_context,
+ struct ad_access_test_ctx);
+ assert_non_null(test_ctx);
+
+ test_ctx->dom = talloc_zero(test_ctx, struct sss_domain_info);
+ assert_non_null(test_ctx->dom);
+
+ test_ctx->dom->name = talloc_strdup(test_ctx->dom, DOM_NAME);
+ assert_non_null(test_ctx->dom->name);
+}
+
+void ad_access_filter_test_teardown(void **state)
+{
+ talloc_free(test_ctx);
+ assert_true(leak_check_teardown());
+}
+
+struct filter_parse_result {
+ const int result;
+ const char *best_match;
+};
+
+static void test_parse_filter_generic(const char *filter_in,
+ struct filter_parse_result *expected)
+{
+ errno_t ret;
+ TALLOC_CTX *tmp_ctx;
+ char *best_match;
+
+ assert_non_null(expected);
+
+ tmp_ctx = talloc_new(global_talloc_context);
+ assert_non_null(tmp_ctx);
+ check_leaks_push(tmp_ctx);
+
+ ret = ad_parse_access_filter(tmp_ctx, test_ctx->dom, filter_in,
+ &best_match);
+ assert_int_equal(ret, expected->result);
+ if (expected->result != EOK) {
+ goto done;
+ }
+
+ if (expected->best_match != NULL) {
+ assert_string_equal(best_match, expected->best_match);
+ } else {
+ assert_true(best_match == NULL);
+ }
+ talloc_free(best_match);
+
+done:
+ assert_true(check_leaks_pop(tmp_ctx) == true);
+ talloc_free(tmp_ctx);
+}
+
+/* Test that if one filter is provided, it is returned as-is
+ */
+void test_single_filter(void **state)
+{
+ struct filter_parse_result expected = {
+ .result = EOK,
+ .best_match = "(name=foo)"
+ };
+
+ test_parse_filter_generic("name=foo", &expected);
+ test_parse_filter_generic("(name=foo)", &expected);
+ test_parse_filter_generic(DOM_NAME":(name=foo)", &expected);
+ test_parse_filter_generic("DOM:"DOM_NAME":(name=foo)", &expected);
+}
+
+/* Test that if more filters are provided, the best match is returned */
+void test_filter_order(void **state)
+{
+ struct filter_parse_result expected = {
+ .result = EOK,
+ .best_match = "(name=foo)"
+ };
+
+ test_parse_filter_generic("name=foo?name=bar", &expected);
+ test_parse_filter_generic(DOM_NAME":(name=foo)?name=bar", &expected);
+ test_parse_filter_generic("name=bla?"DOM_NAME":(name=foo)?name=bar", &expected);
+ /* Test that another foreign domain wouldn't match */
+ test_parse_filter_generic("anotherdom:(name=bla)?"DOM_NAME":(name=foo)", &expected);
+ test_parse_filter_generic("anotherdom:(name=bla)?(name=foo)", &expected);
+}
+
+void test_filter_no_match(void **state)
+{
+ struct filter_parse_result expected = {
+ .result = EOK,
+ .best_match = NULL
+ };
+
+ test_parse_filter_generic("anotherdom:(name=bla)?yetanother:(name=foo)", &expected);
+}
+
+
+void parse_test_setup(void **state)
+{
+ assert_true(leak_check_setup());
+}
+
+void parse_test_teardown(void **state)
+{
+ assert_true(leak_check_teardown());
+}
+
+struct parse_result {
+ const int result;
+ const char *filter;
+ const char *spec;
+ const int flags;
+};
+
+static void test_parse_generic(const char *filter_in, struct parse_result *expected)
+{
+ errno_t ret;
+ TALLOC_CTX *tmp_ctx;
+ char *filter;
+ char *spec;
+ int flags;
+
+ assert_non_null(expected);
+
+ tmp_ctx = talloc_new(global_talloc_context);
+ assert_non_null(tmp_ctx);
+ check_leaks_push(tmp_ctx);
+
+ ret = parse_filter(tmp_ctx, filter_in, &filter, &spec, &flags);
+
+ assert_int_equal(ret, expected->result);
+ if (expected->result != EOK) {
+ goto done;
+ }
+
+ if (expected->filter != NULL) {
+ assert_string_equal(filter, expected->filter);
+ } else {
+ assert_true(filter == NULL);
+ }
+ talloc_free(filter);
+
+ if (expected->spec != NULL) {
+ assert_string_equal(spec, expected->spec);
+ } else {
+ assert_true(spec == NULL);
+ }
+ talloc_free(spec);
+
+ assert_int_equal(flags, expected->flags);
+
+done:
+ assert_true(check_leaks_pop(tmp_ctx) == true);
+ talloc_free(tmp_ctx);
+}
+
+void test_parse_plain(void **state)
+{
+ struct parse_result expected = {
+ .result = EOK,
+ .filter = "name=foo",
+ .spec = NULL,
+ .flags = AD_FILTER_GENERIC
+ };
+
+ test_parse_generic("name=foo", &expected);
+}
+
+void test_parse_dom_without_kw(void **state)
+{
+ struct parse_result expected = {
+ .result = EOK,
+ .filter = "(name=foo)",
+ .spec = "mydom",
+ .flags = AD_FILTER_DOMAIN
+ };
+
+ test_parse_generic("mydom:(name=foo)", &expected);
+
+ /* Check we can handle domain called DOM */
+ struct parse_result expected2 = {
+ .result = EOK,
+ .filter = "(name=foo)",
+ .spec = "DOM",
+ .flags = AD_FILTER_DOMAIN
+ };
+
+ test_parse_generic("DOM:(name=foo)", &expected2);
+}
+
+void test_parse_dom_kw(void **state)
+{
+ struct parse_result expected = {
+ .result = EOK,
+ .filter = "(name=foo)",
+ .spec = "mydom",
+ .flags = AD_FILTER_DOMAIN
+ };
+
+ test_parse_generic("DOM:mydom:(name=foo)", &expected);
+}
+
+void test_parse_forest_kw(void **state)
+{
+ struct parse_result expected = {
+ .result = EOK,
+ .filter = "(name=foo)",
+ .spec = "myforest",
+ .flags = AD_FILTER_FOREST
+ };
+
+ test_parse_generic("FOREST:myforest:(name=foo)", &expected);
+}
+
+
+void test_parse_malformed(void **state)
+{
+ struct parse_result expected = {
+ .result = EINVAL,
+ };
+
+ test_parse_generic("DOM:", &expected);
+ test_parse_generic("DOM::", &expected);
+ test_parse_generic("DOM:mydom:", &expected);
+ test_parse_generic("DOM:mydom:name=foo", &expected);
+ test_parse_generic("DOM::(name=foo)", &expected);
+ test_parse_generic("BLABLABLA:mydom:name=foo", &expected);
+}
+
+int main(int argc, const char *argv[])
+{
+ poptContext pc;
+ int opt;
+ struct poptOption long_options[] = {
+ POPT_AUTOHELP
+ SSSD_DEBUG_OPTS
+ POPT_TABLEEND
+ };
+
+ const UnitTest tests[] = {
+ unit_test_setup_teardown(test_parse_plain,
+ parse_test_setup,
+ parse_test_teardown),
+
+ unit_test_setup_teardown(test_parse_dom_without_kw,
+ parse_test_setup,
+ parse_test_teardown),
+
+ unit_test_setup_teardown(test_parse_dom_kw,
+ parse_test_setup,
+ parse_test_teardown),
+
+ unit_test_setup_teardown(test_parse_forest_kw,
+ parse_test_setup,
+ parse_test_teardown),
+
+ unit_test_setup_teardown(test_parse_malformed,
+ parse_test_setup,
+ parse_test_teardown),
+
+ unit_test_setup_teardown(test_single_filter,
+ ad_access_filter_test_setup,
+ ad_access_filter_test_teardown),
+
+ unit_test_setup_teardown(test_filter_order,
+ ad_access_filter_test_setup,
+ ad_access_filter_test_teardown),
+
+ unit_test_setup_teardown(test_filter_no_match,
+ ad_access_filter_test_setup,
+ ad_access_filter_test_teardown),
+
+ };
+
+ /* Set debug level to invalid value so we can deside if -d 0 was used. */
+ debug_level = SSSDBG_INVALID;
+
+ pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+ while((opt = poptGetNextOpt(pc)) != -1) {
+ switch(opt) {
+ default:
+ fprintf(stderr, "\nInvalid option %s: %s\n\n",
+ poptBadOption(pc, 0), poptStrerror(opt));
+ poptPrintUsage(pc, stderr, 0);
+ return 1;
+ }
+ }
+ poptFreeContext(pc);
+
+ DEBUG_INIT(debug_level);
+
+ tests_set_cwd();
+
+ return run_tests(tests);
+}
--
1.8.3.1
More information about the sssd-devel
mailing list