On Thu, Mar 18, 2010 at 05:51:13PM -0400, Simo Sorce wrote:
Some time ago I added code to fetch the rootdse on connection, but didn't publicize it too much.
Attached find 2 patches.
- Rework the way we store data fetched from the rootdse so the it is
more useful and is actually attached to the ldap handle.
I'm in general fine with this one, although I haven't tested it throughly yet.
- Check controls are supported before using them.
I have two comments here:
- sdap_is_control_supported() should be integrated in sss_ldap_control_create() or a new call should be created which calls sdap_is_control_supported() and then sss_ldap_control_create(). - the debug message 'Server does not support the Password Policy control' should have a much lower level than 7. Because if the control is missing the user might not see the expected behaviour/performance.
bye, Sumit
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From be64c4245db58200db783623a6eaa791fe4339d6 Mon Sep 17 00:00:00 2001
From: Simo Sorce ssorce@redhat.com Date: Thu, 18 Mar 2010 16:58:02 -0400 Subject: [PATCH 1/2] Store rootdse supported features in sdap_handler
src/providers/ipa/ipa_access.c | 10 +-- src/providers/ldap/ldap_common.h | 3 - src/providers/ldap/ldap_id.c | 15 ++--- src/providers/ldap/ldap_id_enum.c | 10 +-- src/providers/ldap/sdap.c | 84 ++++++++++++++++++++++------ src/providers/ldap/sdap.h | 23 +++++++- src/providers/ldap/sdap_async.h | 5 +- src/providers/ldap/sdap_async_connection.c | 45 ++++++--------- 8 files changed, 120 insertions(+), 75 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c index 7dfe1fd..6740f09 100644 --- a/src/providers/ipa/ipa_access.c +++ b/src/providers/ipa/ipa_access.c @@ -437,7 +437,7 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx, }
subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
sdap_ctx->be, sdap_ctx->service, NULL);
sdap_ctx->be, sdap_ctx->service, false); if (!subreq) { DEBUG(1, ("sdap_cli_connect_send failed.\n")); ret = ENOMEM;@@ -482,8 +482,7 @@ static void hbac_get_host_info_connect_done(struct tevent_req *subreq) struct hbac_get_host_info_state); int ret;
- ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh,
NULL);
- ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh); talloc_zfree(subreq); if (ret) { tevent_req_error(req, ret);
@@ -946,7 +945,7 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx, }
subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
sdap_ctx->be, sdap_ctx->service, NULL);
sdap_ctx->be, sdap_ctx->service, false); if (!subreq) { DEBUG(1, ("sdap_cli_connect_send failed.\n")); ret = ENOMEM;@@ -991,8 +990,7 @@ static void hbac_get_rules_connect_done(struct tevent_req *subreq) struct hbac_get_rules_state); int ret;
- ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh,
NULL);
- ret = sdap_cli_connect_recv(subreq, state->sdap_ctx, &state->sdap_ctx->gsh); talloc_zfree(subreq); if (ret) { tevent_req_error(req, ret);
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index ff1ffb7..cbacf83 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -39,9 +39,6 @@ struct sdap_id_ctx { struct fo_service *fo_service; struct sdap_service *service;
- /* what rootDSE returns */
- struct sysdb_attrs *rootDSE;
- /* global sdap handler */ struct sdap_handle *gsh;
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index 4bbc07a..7a646a3 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -106,7 +106,7 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx, * or SASL/GSSAPI, etc ... */ subreq = sdap_cli_connect_send(state, ev, ctx->opts, ctx->be, ctx->service,
&ctx->rootDSE);
false); if (!subreq) { ret = ENOMEM; goto fail;@@ -143,8 +143,7 @@ static void users_get_connect_done(struct tevent_req *subreq) struct users_get_state); int ret;
- ret = sdap_cli_connect_recv(subreq, state->ctx,
&state->ctx->gsh, &state->ctx->rootDSE);
- ret = sdap_cli_connect_recv(subreq, state->ctx, &state->ctx->gsh); talloc_zfree(subreq); if (ret) { if (ret == ENOTSUP) {
@@ -329,7 +328,7 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx, * or SASL/GSSAPI, etc ... */ subreq = sdap_cli_connect_send(state, ev, ctx->opts, ctx->be, ctx->service,
&ctx->rootDSE);
false); if (!subreq) { ret = ENOMEM; goto fail;@@ -366,8 +365,7 @@ static void groups_get_connect_done(struct tevent_req *subreq) struct groups_get_state); int ret;
- ret = sdap_cli_connect_recv(subreq, state->ctx,
&state->ctx->gsh, &state->ctx->rootDSE);
- ret = sdap_cli_connect_recv(subreq, state->ctx, &state->ctx->gsh); talloc_zfree(subreq); if (ret) { if (ret == ENOTSUP) {
@@ -517,7 +515,7 @@ static struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx, * or SASL/GSSAPI, etc ... */ subreq = sdap_cli_connect_send(state, ev, ctx->opts, ctx->be, ctx->service,
&ctx->rootDSE);
false); if (!subreq) { ret = ENOMEM; goto fail;@@ -555,8 +553,7 @@ static void groups_by_user_connect_done(struct tevent_req *subreq) struct groups_by_user_state); int ret;
- ret = sdap_cli_connect_recv(subreq, state->ctx,
&state->ctx->gsh, &state->ctx->rootDSE);
- ret = sdap_cli_connect_recv(subreq, state->ctx, &state->ctx->gsh); talloc_zfree(subreq); if (ret) { if (ret == ENOTSUP) {
diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c index bc06e8b..cf01a5b 100644 --- a/src/providers/ldap/ldap_id_enum.c +++ b/src/providers/ldap/ldap_id_enum.c @@ -363,7 +363,7 @@ static struct tevent_req *enum_users_send(TALLOC_CTX *memctx, * or SASL/GSSAPI, etc ... */ subreq = sdap_cli_connect_send(state, ev, ctx->opts, ctx->be, ctx->service,
&ctx->rootDSE);
false); if (!subreq) { ret = ENOMEM; goto fail;@@ -402,8 +402,7 @@ static void enum_users_connect_done(struct tevent_req *subreq) struct enum_users_state); int ret;
- ret = sdap_cli_connect_recv(subreq, state->ctx,
&state->ctx->gsh, &state->ctx->rootDSE);
- ret = sdap_cli_connect_recv(subreq, state->ctx, &state->ctx->gsh); talloc_zfree(subreq); if (ret) { if (ret == ENOTSUP) {
@@ -518,7 +517,7 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx, * or SASL/GSSAPI, etc ... */ subreq = sdap_cli_connect_send(state, ev, ctx->opts, ctx->be, ctx->service,
&ctx->rootDSE);
false); if (!subreq) { ret = ENOMEM; goto fail;@@ -556,8 +555,7 @@ static void enum_groups_connect_done(struct tevent_req *subreq) struct enum_groups_state); int ret;
- ret = sdap_cli_connect_recv(subreq, state->ctx,
&state->ctx->gsh, &state->ctx->rootDSE);
- ret = sdap_cli_connect_recv(subreq, state->ctx, &state->ctx->gsh); talloc_zfree(subreq); if (ret) { if (ret == ENOTSUP) {
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c index 39c67cc..e0f5038 100644 --- a/src/providers/ldap/sdap.c +++ b/src/providers/ldap/sdap.c @@ -325,37 +325,85 @@ errno_t setup_tls_config(struct dp_option *basic_opts) }
-bool sdap_rootdse_sasl_mech_is_supported(struct sysdb_attrs *rootdse,
const char *sasl_mech)+bool sdap_check_sup_list(struct sup_list *l, const char *val) {
struct ldb_message_element *el = NULL;
struct ldb_val *val; int i;
if (!sasl_mech) return false;
- if (!val) {
return false;- }
- for (i = 0; i < rootdse->num; i++) {
if (strcasecmp(rootdse->a[i].name, "supportedSASLMechanisms")) {
- for (i = 0; i < l->num_vals; i++) {
if (strcasecmp(val, (char *)l->vals[i])) { continue; }
el = &rootdse->a[i];break;
return true;
- if (!el) {
/* no supported SASL Mechanism at all ? */return false;
- return false;
+}
+static int sdap_init_sup_list(TALLOC_CTX *memctx,
struct sup_list *list,int num, struct ldb_val *vals)+{
- int i;
- list->vals = talloc_array(memctx, char *, num);
- if (!list->vals) {
return ENOMEM;
- for (i = 0; i < el->num_values; i++) {
val = &el->values[i];if (strncasecmp(sasl_mech, (const char *)val->data, val->length)) {continue;
- for (i = 0; i < num; i++) {
list->vals[i] = talloc_strndup(list->vals,(char *)vals[i].data, vals[i].length);if (!list->vals[i]) {return ENOMEM; }
return true;}
return false;
- list->num_vals = num;
- return EOK;
+}
+int sdap_set_rootdse_supported_lists(struct sysdb_attrs *rootdse,
struct sdap_handle *sh)+{
- struct ldb_message_element *el = NULL;
- int ret;
- int i;
- for (i = 0; i < rootdse->num; i++) {
el = &rootdse->a[i];if (strcasecmp(el->name, "supportedControl") == 0) {ret = sdap_init_sup_list(sh, &sh->supported_controls,el->num_values, el->values);if (ret) {return ret;}}if (strcasecmp(el->name, "supportedExtension") == 0) {ret = sdap_init_sup_list(sh, &sh->supported_extensions,el->num_values, el->values);if (ret) {return ret;}}if (strcasecmp(el->name, "supportedSASLMechanisms") == 0) {ret = sdap_init_sup_list(sh, &sh->supported_saslmechs,el->num_values, el->values);if (ret) {return ret;}}- }
- return EOK;
}
int build_attrs_from_map(TALLOC_CTX *memctx, diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index 007185f..a5574fc 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -67,6 +67,11 @@ struct ldap_cb_data { struct fd_event_item *fd_list; };
+struct sup_list {
- int num_vals;
- char **vals;
+};
struct sdap_handle { LDAP *ldap; bool connected; @@ -77,6 +82,10 @@ struct sdap_handle { struct tevent_fd *fde; #endif
- struct sup_list supported_saslmechs;
- struct sup_list supported_controls;
- struct sup_list supported_extensions;
- struct sdap_op *ops;
};
@@ -250,8 +259,18 @@ int sdap_get_msg_dn(TALLOC_CTX *memctx, struct sdap_handle *sh,
errno_t setup_tls_config(struct dp_option *basic_opts);
-bool sdap_rootdse_sasl_mech_is_supported(struct sysdb_attrs *rootdse,
const char *sasl_mech);+int sdap_set_rootdse_supported_lists(struct sysdb_attrs *rootdse,
struct sdap_handle *sh);+bool sdap_check_sup_list(struct sup_list *l, const char *val);
+#define sdap_is_sasl_mech_supported(sh, sasl_mech) \
- sdap_check_sup_list(&((sh)->supported_saslmechs), sasl_mech)
+#define sdap_is_control_supported(sh, ctrl_oid) \
- sdap_check_sup_list(&((sh)->supported_controls), ctrl_oid)
+#define sdap_is_extension_supported(sh, ext_oid) \
- sdap_check_sup_list(&((sh)->supported_extensions), ext_oid)
int build_attrs_from_map(TALLOC_CTX *memctx, struct sdap_attr_map *map, diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index 3c52d23..1139d46 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -103,11 +103,10 @@ struct tevent_req *sdap_cli_connect_send(TALLOC_CTX *memctx, struct sdap_options *opts, struct be_ctx *be, struct sdap_service *service,
struct sysdb_attrs **rootdse);
bool skip_rootdse);int sdap_cli_connect_recv(struct tevent_req *req, TALLOC_CTX *memctx,
struct sdap_handle **gsh,struct sysdb_attrs **rootdse);
struct sdap_handle **gsh);struct tevent_req *sdap_get_generic_send(TALLOC_CTX *memctx, struct tevent_context *ev, diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c index 586733f..f0b8c2e 100644 --- a/src/providers/ldap/sdap_async_connection.c +++ b/src/providers/ldap/sdap_async_connection.c @@ -810,7 +810,6 @@ struct sdap_cli_connect_state { struct sdap_service *service;
bool use_rootdse;
struct sysdb_attrs *rootdse;
struct sdap_handle *sh;
@@ -831,7 +830,7 @@ struct tevent_req *sdap_cli_connect_send(TALLOC_CTX *memctx, struct sdap_options *opts, struct be_ctx *be, struct sdap_service *service,
struct sysdb_attrs **rootdse)
bool skip_rootdse){ struct tevent_req *req, *subreq; struct sdap_cli_connect_state *state; @@ -844,12 +843,10 @@ struct tevent_req *sdap_cli_connect_send(TALLOC_CTX *memctx, state->service = service; state->srv = NULL;
- if (rootdse) {
state->use_rootdse = true;state->rootdse = *rootdse;- } else {
- if (skip_rootdse) { state->use_rootdse = false;
state->rootdse = NULL;
} else {
state->use_rootdse = true;}
/* NOTE: this call may cause service->uri to be refreshed
@@ -908,7 +905,7 @@ static void sdap_cli_connect_done(struct tevent_req *subreq) return; }
- if (state->use_rootdse && !state->rootdse) {
- if (state->use_rootdse) { /* fetch the rootDSE this time */ sdap_cli_rootdse_step(req); return;
@@ -918,8 +915,7 @@ static void sdap_cli_connect_done(struct tevent_req *subreq)
if (sasl_mech && state->use_rootdse) { /* check if server claims to support GSSAPI */
if (!sdap_rootdse_sasl_mech_is_supported(state->rootdse,sasl_mech)) {
if (!sdap_is_sasl_mech_supported(state->sh, sasl_mech)) { tevent_req_error(req, ENOTSUP); return; }@@ -970,22 +966,29 @@ static void sdap_cli_rootdse_done(struct tevent_req *subreq) struct tevent_req); struct sdap_cli_connect_state *state = tevent_req_data(req, struct sdap_cli_connect_state);
- struct sysdb_attrs *rootdse; const char *sasl_mech; int ret;
- ret = sdap_get_rootdse_recv(subreq, state, &state->rootdse);
ret = sdap_get_rootdse_recv(subreq, state, &rootdse); talloc_zfree(subreq); if (ret) { tevent_req_error(req, ret); return; }
/* save rootdse data about supported features */
ret = sdap_set_rootdse_supported_lists(rootdse, state->sh);
if (ret) {
tevent_req_error(req, ret);return;}
sasl_mech = dp_opt_get_string(state->opts->basic, SDAP_SASL_MECH);
if (sasl_mech && state->use_rootdse) { /* check if server claims to support GSSAPI */
if (!sdap_rootdse_sasl_mech_is_supported(state->rootdse,sasl_mech)) {
if (!sdap_is_sasl_mech_supported(state->sh, sasl_mech)) { tevent_req_error(req, ENOTSUP); return; }@@ -1094,8 +1097,7 @@ static void sdap_cli_auth_done(struct tevent_req *subreq)
int sdap_cli_connect_recv(struct tevent_req *req, TALLOC_CTX *memctx,
struct sdap_handle **gsh,struct sysdb_attrs **rootdse)
struct sdap_handle **gsh){ struct sdap_cli_connect_state *state = tevent_req_data(req, struct sdap_cli_connect_state); @@ -1125,19 +1127,6 @@ int sdap_cli_connect_recv(struct tevent_req *req, talloc_zfree(state->sh); }
- if (rootdse) {
if (state->use_rootdse) {*rootdse = talloc_steal(memctx, state->rootdse);if (!*rootdse) {return ENOMEM;}} else {*rootdse = NULL;}- } else {
talloc_zfree(rootdse);- }
- return EOK;
}
-- 1.6.6.1
From d8981c63d348ee53c017c82c476ee35bf82e38d3 Mon Sep 17 00:00:00 2001
From: Simo Sorce ssorce@redhat.com Date: Thu, 18 Mar 2010 17:24:00 -0400 Subject: [PATCH 2/2] Check if control is supported before using it.
src/providers/ldap/sdap_async.c | 22 ++++++++++++++-------- src/providers/ldap/sdap_async_connection.c | 22 ++++++++++++++-------- 2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c index 959c08a..030bf8d 100644 --- a/src/providers/ldap/sdap_async.c +++ b/src/providers/ldap/sdap_async.c @@ -550,7 +550,8 @@ struct tevent_req *sdap_exop_modify_passwd_send(TALLOC_CTX *memctx, BerElement *ber = NULL; struct berval *bv = NULL; int msgid;
- LDAPControl *request_controls[2];
LDAPControl **request_controls = NULL;
LDAPControl *ctrls[2] = { NULL, NULL };
req = tevent_req_create(memctx, &state, struct sdap_exop_modify_passwd_state);
@@ -585,20 +586,25 @@ struct tevent_req *sdap_exop_modify_passwd_send(TALLOC_CTX *memctx, return NULL; }
- ret = sss_ldap_control_create(LDAP_CONTROL_PASSWORDPOLICYREQUEST,
0, NULL, 0, &request_controls[0]);- if (ret != LDAP_SUCCESS) {
DEBUG(1, ("sss_ldap_control_create failed.\n"));goto fail;
- if (sdap_is_control_supported(state->sh,
LDAP_CONTROL_PASSWORDPOLICYREQUEST)) {ret = sss_ldap_control_create(LDAP_CONTROL_PASSWORDPOLICYREQUEST,0, NULL, 0, &ctrls[0]);if (ret != LDAP_SUCCESS) {DEBUG(1, ("sss_ldap_control_create failed.\n"));goto fail;}request_controls = ctrls;- } else {
DEBUG(7, ("Server does not support the Password Policy control.\n"));
request_controls[1] = NULL;
DEBUG(4, ("Executing extended operation\n"));
ret = ldap_extended_operation(state->sh->ldap, LDAP_EXOP_MODIFY_PASSWD, bv, request_controls, NULL, &msgid); ber_bvfree(bv);
ldap_control_free(request_controls[0]);
- if (ctrls[0]) ldap_control_free(ctrls[0]); if (ret == -1 || msgid == -1) { DEBUG(1, ("ldap_extended_operation failed\n")); goto fail;
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c index f0b8c2e..d86e4cc 100644 --- a/src/providers/ldap/sdap_async_connection.c +++ b/src/providers/ldap/sdap_async_connection.c @@ -296,7 +296,8 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx, int ret = EOK; int msgid; int ldap_err;
- LDAPControl *request_controls[2];
LDAPControl **request_controls = NULL;
LDAPControl *ctrls[2] = { NULL, NULL };
req = tevent_req_create(memctx, &state, struct simple_bind_state); if (!req) return NULL;
@@ -312,19 +313,24 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx, state->user_dn = user_dn; state->pw = pw;
- ret = sss_ldap_control_create(LDAP_CONTROL_PASSWORDPOLICYREQUEST,
0, NULL, 0, &request_controls[0]);- if (ret != LDAP_SUCCESS) {
DEBUG(1, ("sss_ldap_control_create failed.\n"));goto fail;
- if (sdap_is_control_supported(state->sh,
LDAP_CONTROL_PASSWORDPOLICYREQUEST)) {ret = sss_ldap_control_create(LDAP_CONTROL_PASSWORDPOLICYREQUEST,0, NULL, 0, &ctrls[0]);if (ret != LDAP_SUCCESS) {DEBUG(1, ("sss_ldap_control_create failed.\n"));goto fail;}request_controls = ctrls;- } else {
DEBUG(7, ("Server does not support the Password Policy control.\n"));
request_controls[1] = NULL;
DEBUG(4, ("Executing simple bind as: %s\n", state->user_dn));
ret = ldap_sasl_bind(state->sh->ldap, state->user_dn, LDAP_SASL_SIMPLE, state->pw, request_controls, NULL, &msgid);
ldap_control_free(request_controls[0]);
- if (ctrls[0]) ldap_control_free(ctrls[0]); if (ret == -1 || msgid == -1) { ret = ldap_get_option(state->sh->ldap, LDAP_OPT_RESULT_CODE, &ldap_err);
-- 1.6.6.1
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel