Some time ago I added code to fetch the rootdse on connection, but didn't publicize it too much.
Attached find 2 patches.
1) Rework the way we store data fetched from the rootdse so the it is more useful and is actually attached to the ldap handle.
2) Check controls are supported before using them.
Simo.
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
On Fri, 19 Mar 2010 12:00:47 +0100 Sumit Bose sbose@redhat.com wrote:
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.
Sumit, do you want to take over the second patch ? I don't have a way to thoroughly test it anyway and I am a bit short on time.
Simo.
On Fri, Mar 19, 2010 at 08:48:49AM -0400, Simo Sorce wrote:
On Fri, 19 Mar 2010 12:00:47 +0100 Sumit Bose sbose@redhat.com wrote:
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.
Sumit, do you want to take over the second patch ? I don't have a way to thoroughly test it anyway and I am a bit short on time.
sure
bye, Sumit
Simo.
-- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
On Fri, Mar 19, 2010 at 02:15:06PM +0100, Sumit Bose wrote:
On Fri, Mar 19, 2010 at 08:48:49AM -0400, Simo Sorce wrote:
On Fri, 19 Mar 2010 12:00:47 +0100 Sumit Bose sbose@redhat.com wrote:
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.
Sumit, do you want to take over the second patch ? I don't have a way to thoroughly test it anyway and I am a bit short on time.
sure
Hi,
not even 4 months later here are the revised versions of these patches. I had it add two more changes to the first patch to make it build on master because of the change to sdap_cli_connect_recv().
In the second patch if have created a new call sdap_control_create() which checks if the control is supported with sdap_is_control_supported() and calls sss_ldap_control_create() on success. I think this approach is more clean, because it leaves sss_ldap_control_create() as a pure replacement function.
bye, Sumit
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/09/2010 05:59 AM, Sumit Bose wrote:
On Fri, Mar 19, 2010 at 02:15:06PM +0100, Sumit Bose wrote:
On Fri, Mar 19, 2010 at 08:48:49AM -0400, Simo Sorce wrote:
On Fri, 19 Mar 2010 12:00:47 +0100 Sumit Bose sbose@redhat.com wrote:
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.
Sumit, do you want to take over the second patch ? I don't have a way to thoroughly test it anyway and I am a bit short on time.
sure
Hi,
not even 4 months later here are the revised versions of these patches. I had it add two more changes to the first patch to make it build on master because of the change to sdap_cli_connect_recv().
In the second patch if have created a new call sdap_control_create() which checks if the control is supported with sdap_is_control_supported() and calls sss_ldap_control_create() on success. I think this approach is more clean, because it leaves sss_ldap_control_create() as a pure replacement function.
Naturally, we only got around to reviewing this two months after this revision :(
Unfortunately, the changes made to the creation of LDAP operations in SSSD 1.3.0 conflict madly with these changes. Would it be possible for you to rebase this patch atop the current master?
If not, I'll add it to my todo list post-1.4.0.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
On Thu, Sep 02, 2010 at 09:44:02AM -0400, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/09/2010 05:59 AM, Sumit Bose wrote:
On Fri, Mar 19, 2010 at 02:15:06PM +0100, Sumit Bose wrote:
On Fri, Mar 19, 2010 at 08:48:49AM -0400, Simo Sorce wrote:
On Fri, 19 Mar 2010 12:00:47 +0100 Sumit Bose sbose@redhat.com wrote:
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.
Sumit, do you want to take over the second patch ? I don't have a way to thoroughly test it anyway and I am a bit short on time.
sure
Hi,
not even 4 months later here are the revised versions of these patches. I had it add two more changes to the first patch to make it build on master because of the change to sdap_cli_connect_recv().
In the second patch if have created a new call sdap_control_create() which checks if the control is supported with sdap_is_control_supported() and calls sss_ldap_control_create() on success. I think this approach is more clean, because it leaves sss_ldap_control_create() as a pure replacement function.
Naturally, we only got around to reviewing this two months after this revision :(
Unfortunately, the changes made to the creation of LDAP operations in SSSD 1.3.0 conflict madly with these changes. Would it be possible for you to rebase this patch atop the current master?
If not, I'll add it to my todo list post-1.4.0.
rebased versions attached.
bye, Sumit
Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkx/qiIACgkQeiVVYja6o6MFHQCeIhpJQ4Ttk0+76fKo6uAsVHhB cy8AnRsnD3Fy736/lAJVEc7BioJGpYI8 =KNjZ -----END PGP SIGNATURE----- _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
On Fri, Sep 10, 2010 at 05:05:21PM +0200, Jakub Hrozek wrote:
On 09/10/2010 11:16 AM, Sumit Bose wrote:
rebased versions attached.
bye, Sumit
Ack to both patches, but may I suggest that the attached patch be squashed in? Perhaps it would make for a little more readable code.
yes, please do, ACK to both changes.
bye, Sumit
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/10/2010 05:40 PM, Sumit Bose wrote:
On Fri, Sep 10, 2010 at 05:05:21PM +0200, Jakub Hrozek wrote:
On 09/10/2010 11:16 AM, Sumit Bose wrote:
rebased versions attached.
bye, Sumit
Ack to both patches, but may I suggest that the attached patch be squashed in? Perhaps it would make for a little more readable code.
yes, please do, ACK to both changes.
bye, Sumit
Attaching both patches, the first one contains my squashed mini-patch acked by Sumit.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/12/2010 12:34 PM, Jakub Hrozek wrote:
On 09/10/2010 05:40 PM, Sumit Bose wrote:
On Fri, Sep 10, 2010 at 05:05:21PM +0200, Jakub Hrozek wrote:
On 09/10/2010 11:16 AM, Sumit Bose wrote:
rebased versions attached.
bye, Sumit
Ack to both patches, but may I suggest that the attached patch be squashed in? Perhaps it would make for a little more readable code.
yes, please do, ACK to both changes.
bye, Sumit
Attaching both patches, the first one contains my squashed mini-patch acked by Sumit.
Pushed both patches to master.
Patch 0002 needed to be rebased (again) because of the async bind patch being reverted. However, the patch from earlier in this thread applied cleanly now :)
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Simo Sorce -
Stephen Gallagher -
Sumit Bose