On Wed, Aug 06, 2014 at 06:36:02PM +0200, Pavel Reichl wrote:
On 08/04/2014 02:37 PM, Pavel Březina wrote:
On 08/01/2014 07:48 PM, Pavel Reichl wrote:
Hello,
please see attached patches. Every patch is documented in its commit
message.
Generally speaking the first 6 patches are preparation for patch #8.
Thanks,
Pavel Reichl
PS: I also attached output of my testing to make it more obvious how the
patches are supposed to work.
------------------------------------------------------------------------
# john, people, example.com
dn: uid=john,ou=people,dc=example,dc=com
pwdAccountLockedTime: 000001010000Z
# max, people, example.com
dn: uid=max,ou=people,dc=example,dc=com
# dick, people, example.com
dn: uid=dick,ou=people,dc=example,dc=com
pwdAccountLockedTime: 20140801115742Z
--------------------------------------------------------------------------
$ ssh -l john@openldap `hostname`
Connection closed by UNKNOWN
$ ssh -l max@openldap `hostname`
Last login: Fri Aug 1 15:16:21 2014 from sssd.dev.work
$ ssh -l dick@openldap `hostname`
Last login: Fri Aug 1 12:57:33 2014
Patch #1:
Tentative ack, although you don't have to use tmp_ctx in
sdap_save_user_cache_bool(). You can just allocate attrs on NULL since it
is the only talloc context you use there.
Patch #2:
Nack. I see it is safe here but please avoid initializing ret to EOK. Set
it to EOK at the end of the function instead.
Patch #3:
Ack.
Patch #4:
Typo in commit message "Also remove *sdap_access_filter_recv()* as it is
replaced by *sdap_access_filter_recv()*."
But it's nack from me anyway.
Tevent naming convention seems to be broken all around this code and this
patch actually makes it a little bit better. But you shouldn't use
function named "sdap_access_recv" inside "sdap_access_done".
I see that sdap_access_send can invoke two different requests depending on
the configuration. Ideally both should have custom _recv and you should
use some kind of switch to determine which one should be called.
I would be willing to tentatively ack a patch with let's say
"sdap_access_check_recv" or "sdap_access_subreq_recv" or similar used in
sdap_access_done.
Patch #5:
Ack.
Patch #6:
Ack.
Patch #7:
Ack. But please swap this ticket with #8. The feature should be first
implemented then documented.
Patch #8:
I believe the following comment is copy & paste? It shouldn't say filter
request :)
+ /* Connection to LDAP succeeded
+ * Send filter request
+ */
+ if (strcasecmp(pwdAccountLockedTime,
+ PERMANENTLY_LOCKED_ACCOUNT) == 0) {
+ locked = true;
+ } else {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Account of: %s is beeing blocked by password
policy, "
+ "but value: [%s] value is ignored by SSSD.\n",
+ state->username, pwdAccountLockedTime);
+ }
Why do we ignore the value? Isn't the value a time? Shouldn't we check if
lock_time < machine_time?
Yes, it's time, but we care only if its the special value '000001010000Z'
otherwise we grant the access.
Also the ticket says that this policy is only enabled if pwdLockout is
true. Is it possible to acquire the attribute if pwdLockout is false? I
see you don't check it.
Yes it's possible to acquire, I have updated the patches.
Those are questions that should be answered in the code if they are not
bugs.
I haven't test the code yet - will it work also with 389ds or do you have
an OpenLDAP server I could use handy?
I'll try to grant you access to my VM as I find configuring ppolicy as very
painful experience.
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I hope I have addressed all you concerns and didn't produce many new bugs.
:-)
Mostly looks OK to me (although I just quickly read the patches..), some
questions inline.
From 96bb7442a9b9fc3f21e772e565fcf758b55c2dc9 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Fri, 1 Aug 2014 11:00:48 +0100
Subject: [PATCH 2/9] SDAP: refactor sdap_access_filter_send
As preparation for ticket #2364 separate code for parsing user basedn
to a new function sdap_get_basedn_user_entry().
---
src/providers/ldap/sdap_access.c | 52 ++++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c
index
0702c565a466ba067350595233f9746443d0864d..ddf623616f662d92fd751ff7952f08527c60be45
100644
--- a/src/providers/ldap/sdap_access.c
+++ b/src/providers/ldap/sdap_access.c
@@ -45,6 +45,11 @@ static errno_t sdap_save_user_cache_bool(struct
sss_domain_info *domain,
const char *attr_name,
bool value);
+static errno_t sdap_get_basedn_user_entry(TALLOC_CTX *mem_ctx,
+ struct ldb_message *user_entry,
+ const char *username,
+ char **_basedn);
+
static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct be_ctx *be_ctx,
@@ -666,7 +671,6 @@ static struct tevent_req
*sdap_access_filter_send(TALLOC_CTX *mem_ctx,
{
struct sdap_access_filter_req_ctx *state;
struct tevent_req *req;
- const char *basedn;
char *clean_username;
errno_t ret = ERR_INTERNAL;
char *name;
@@ -704,20 +708,9 @@ static struct tevent_req
*sdap_access_filter_send(TALLOC_CTX *mem_ctx,
goto done;
}
- /* Perform online operation */
- basedn = ldb_msg_find_attr_as_string(user_entry, SYSDB_ORIG_DN, NULL);
- if (basedn == NULL) {
- DEBUG(SSSDBG_CRIT_FAILURE,"Could not find originalDN for user [%s]\n",
- state->username);
- ret = EINVAL;
- goto done;
- }
-
- state->basedn = talloc_strdup(state, basedn);
- if (state->basedn == NULL) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "Could not allocate memory for originalDN\n");
- ret = ENOMEM;
+ ret = sdap_get_basedn_user_entry(state, user_entry, state->username,
+ &state->basedn);
+ if (ret != EOK) {
goto done;
}
@@ -1141,3 +1134,32 @@ static errno_t sdap_access_host(struct ldb_message *user_entry)
return ret;
}
+
+static errno_t sdap_get_basedn_user_entry(TALLOC_CTX *mem_ctx,
+ struct ldb_message *user_entry,
+ const char *username,
+ char **_basedn)
+{
+ const char *basedn;
+ errno_t ret;
+
+ basedn = ldb_msg_find_attr_as_string(user_entry, SYSDB_ORIG_DN, NULL);
+ if (basedn == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE,"Could not find originalDN for user [%s]\n",
+ username);
+ ret = EINVAL;
+ goto done;
+ }
+
+ *_basedn = talloc_strdup(mem_ctx, basedn);
+ if (*_basedn == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Could not allocate memory for originalDN\n");
+ ret = ENOMEM;
+ goto done;
+ }
Do we need to strdup the basedn here?
+ ret = EOK;
+
+done:
+ return ret;
+}
--
1.9.3
From e510dbbb83b75982ce755cc27d8754875832aa3a Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Fri, 1 Aug 2014 12:22:21 +0100
Subject: [PATCH 4/9] SDAP: refactor sdap_access_filter_done
As preparation for ticket #2364 move code from sdap_access_filter_done()
into sdap_access_done() to make its reuse possible and thus avoid code
duplication.
---
src/providers/ldap/sdap_access.c | 41 ++++++++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c
index
e54f4e0d5d13d72e8cb38bd1c827ea2f52e7dcef..68633643f7430c26885a78d2903e7a4dd636e1d5
100644
--- a/src/providers/ldap/sdap_access.c
+++ b/src/providers/ldap/sdap_access.c
@@ -60,8 +60,11 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX
*mem_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);
+static errno_t sdap_access_subreq_recv(struct tevent_req *req);
+
static errno_t sdap_account_expired(struct sdap_access_ctx *access_ctx,
struct pam_data *pd,
struct ldb_message *user_entry);
@@ -219,14 +222,14 @@ static errno_t check_next_rule(struct sdap_access_req_ctx
*state,
return ret;
}
-static void sdap_access_filter_done(struct tevent_req *subreq)
+static void sdap_access_done(struct tevent_req *subreq)
{
errno_t ret;
struct tevent_req *req = tevent_req_callback_data(subreq, struct
tevent_req);
struct sdap_access_req_ctx *state =
tevent_req_data(req, struct sdap_access_req_ctx);
- ret = sdap_access_filter_recv(subreq);
+ ret = sdap_access_subreq_recv(subreq);
talloc_zfree(subreq);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access check result.\n");
@@ -249,6 +252,11 @@ static void sdap_access_filter_done(struct tevent_req
*subreq)
}
}
+static void sdap_access_filter_done(struct tevent_req *subreq)
+{
+ sdap_access_done(subreq);
+}
+
errno_t sdap_access_recv(struct tevent_req *req)
{
TEVENT_REQ_RETURN_ON_ERROR(req);
@@ -256,7 +264,6 @@ errno_t sdap_access_recv(struct tevent_req *req)
return EOK;
}
-
#define SHADOW_EXPIRE_MSG "Account expired according to shadow attributes"
static errno_t sdap_account_expired_shadow(struct pam_data *pd,
@@ -644,6 +651,10 @@ static errno_t sdap_account_expired(struct sdap_access_ctx
*access_ctx,
return ret;
}
+enum sdap_access_control_type {
+ SDAP_ACCESS_CONTROL_FILTER,
+};
+
struct sdap_access_filter_req_ctx {
const char *username;
const char *filter;
@@ -656,6 +667,7 @@ struct sdap_access_filter_req_ctx {
struct sss_domain_info *domain;
bool cached_access;
char *basedn;
+ enum sdap_access_control_type ac_type;
};
static errno_t sdap_access_filter_decide_offline(struct tevent_req *req);
@@ -696,6 +708,7 @@ static struct tevent_req
*sdap_access_filter_send(TALLOC_CTX *mem_ctx,
state->ev = ev;
state->access_ctx = access_ctx;
state->domain = domain;
+ state->ac_type = SDAP_ACCESS_CONTROL_FILTER;
DEBUG(SSSDBG_TRACE_FUNC,
"Performing access filter check for user [%s]\n", username);
@@ -772,6 +785,27 @@ done:
return req;
}
+static errno_t sdap_access_subreq_recv(struct tevent_req *req)
+{
+ struct sdap_access_filter_req_ctx *state;
+ errno_t ret;
+
+ state = tevent_req_data(req, struct sdap_access_filter_req_ctx);
+
+ switch(state->ac_type) {
+ case SDAP_ACCESS_CONTROL_FILTER:
+ ret = sdap_access_filter_recv(req);
+ break;
+ default:
+ ret = EINVAL;
+ DEBUG(SSSDBG_MINOR_FAILURE, "Unknown access control type: %d.",
+ state->ac_type);
+ break;
+ }
+
+ return ret;
+}
+
This function is misplaced, it immediatelly follows _send() in the code.
_recv() should be the last in the flow.
From f6cdc0ce09645cae4254c4e6cb6dfc24de0545e1 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Fri, 1 Aug 2014 17:44:24 +0100
Subject: [PATCH 8/9] SDAP: account lock to restrict access via ssh key
Be able to configure sssd to honor openldap account lock to restrict
access via ssh key. Introduce new ldap_access_order value ('lock')
for enabling/disabling this feature.
Resolves:
https://fedorahosted.org/sssd/ticket/2364
---
src/providers/ldap/ldap_init.c | 2 +
src/providers/ldap/sdap_access.c | 439 +++++++++++++++++++++++++++++++++++++++
src/providers/ldap/sdap_access.h | 4 +
3 files changed, 445 insertions(+)
diff --git a/src/providers/ldap/ldap_init.c b/src/providers/ldap/ldap_init.c
index
9960fd3ab7a6e446e5cdc1e8fa4984aab812d980..85d9acef1d2dd55361cba2ae67376673fe67d021
100644
--- a/src/providers/ldap/ldap_init.c
+++ b/src/providers/ldap/ldap_init.c
@@ -421,6 +421,8 @@ int sssm_ldap_access_init(struct be_ctx *bectx,
access_ctx->access_rule[c] = LDAP_ACCESS_SERVICE;
} else if (strcasecmp(order_list[c], LDAP_ACCESS_HOST_NAME) == 0) {
access_ctx->access_rule[c] = LDAP_ACCESS_HOST;
+ } else if (strcasecmp(order_list[c], LDAP_ACCESS_LOCK_NAME) == 0) {
+ access_ctx->access_rule[c] = LDAP_ACCESS_LOCK;
} else {
DEBUG(SSSDBG_CRIT_FAILURE,
"Unexpected access rule name [%s].\n", order_list[c]);
diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c
index
248ffbad44d9aa4794b6042f9a825639277cc1e9..e1bc018835f98661683bb8971ff2c8be6b3bcf99
100644
--- a/src/providers/ldap/sdap_access.c
+++ b/src/providers/ldap/sdap_access.c
@@ -40,6 +40,7 @@
#include "providers/data_provider.h"
#include "providers/dp_backend.h"
+#define PERMANENTLY_LOCKED_ACCOUNT "000001010000Z"
#define MALFORMED_FILTER "Malformed access control filter [%s]\n"
static errno_t sdap_save_user_cache_bool(struct sss_domain_info *domain,
@@ -52,6 +53,18 @@ static errno_t sdap_get_basedn_user_entry(TALLOC_CTX
*mem_ctx,
const char *username,
char **_basedn);
+static struct tevent_req *
+sdap_access_lock_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ 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 void sdap_access_lock_done(struct tevent_req *subreq);
+
static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct be_ctx *be_ctx,
@@ -63,6 +76,8 @@ static struct tevent_req *sdap_access_filter_send(TALLOC_CTX
*mem_ctx,
static errno_t sdap_access_filter_recv(struct tevent_req *req);
+static errno_t sdap_access_lock_recv(struct tevent_req *req);
+
static errno_t sdap_access_subreq_recv(struct tevent_req *req);
static errno_t sdap_account_expired(struct sdap_access_ctx *access_ctx,
@@ -182,6 +197,24 @@ static errno_t check_next_rule(struct sdap_access_req_ctx
*state,
/* we are done with no errors */
return EOK;
+ case LDAP_ACCESS_LOCK:
+ ret = EOK;
+
+ subreq = sdap_access_lock_send(state, state->ev, state->be_ctx,
+ state->domain,
+ state->access_ctx,
+ state->conn,
+ state->pd->user,
+ state->user_entry);
+ if (subreq == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "sdap_access_lock_send failed.\n");
+ return ENOMEM;
+ }
+
+ tevent_req_set_callback(subreq, sdap_access_lock_done, req);
+ return EAGAIN;
+ break;
+
case LDAP_ACCESS_FILTER:
subreq = sdap_access_filter_send(state, state->ev, state->be_ctx,
state->domain,
@@ -658,6 +691,7 @@ static errno_t sdap_account_expired(struct sdap_access_ctx
*access_ctx,
enum sdap_access_control_type {
SDAP_ACCESS_CONTROL_FILTER,
+ SDAP_ACCESS_CONTROL_LOCK,
The code looks for a specific lock type, so the enum should be named
with a more specific name, too, maybe something like
SDAP_ACCESS_PPOLICY_LOCK?
};
struct sdap_access_filter_req_ctx {
@@ -678,8 +712,10 @@ struct sdap_access_filter_req_ctx {
static errno_t sdap_access_decide_offline(struct tevent_req *req);
static int sdap_access_filter_retry(struct tevent_req *req);
+static void sdap_access_lock_connect_done(struct tevent_req *subreq);
static void sdap_access_filter_connect_done(struct tevent_req *subreq);
static void sdap_access_filter_get_access_done(struct tevent_req *req);
+
static struct tevent_req *sdap_access_filter_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct be_ctx *be_ctx,
@@ -802,6 +838,9 @@ static errno_t sdap_access_subreq_recv(struct tevent_req
*req)
case SDAP_ACCESS_CONTROL_FILTER:
ret = sdap_access_filter_recv(req);
break;
+ case SDAP_ACCESS_CONTROL_LOCK:
+ ret = sdap_access_lock_recv(req);
+ break;
default:
ret = EINVAL;
DEBUG(SSSDBG_MINOR_FAILURE, "Unknown access control type: %d.",
@@ -996,6 +1035,13 @@ static errno_t sdap_access_filter_recv(struct tevent_req
*req)
return EOK;
}
+static errno_t sdap_access_lock_recv(struct tevent_req *req)
+{
+ TEVENT_REQ_RETURN_ON_ERROR(req);
+
+ return EOK;
+}
+
This function seems misplaced, the order of tevent code is
foo_send()
foo_step1()
foo_step2()
foo_recv();
Here you have recv() before send();
#define AUTHR_SRV_MISSING_MSG "Authorized service attribute missing, " \
"access denied"
#define AUTHR_SRV_DENY_MSG "Access denied by authorized service attribute"
@@ -1172,6 +1218,399 @@ static errno_t sdap_access_host(struct ldb_message
*user_entry)
return ret;
}
+static void sdap_access_lock_get_lockout_done(struct tevent_req *subreq);
+static int sdap_access_lock_retry(struct tevent_req *req);
+static struct tevent_req*
+sdap_access_locked_time_send(struct tevent_req *req);
+static void sdap_access_locked_time_done(struct tevent_req *subreq);
+
+static struct tevent_req *
+sdap_access_lock_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ 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)
+{
+ struct sdap_access_filter_req_ctx *state;
Please don't reuse one state for two requests, that's quite dangerous.
+ struct tevent_req *req;
+ errno_t ret = ERR_INTERNAL;
+
+ req = tevent_req_create(mem_ctx,
+ &state, struct sdap_access_filter_req_ctx);
+ if (req == NULL) {
+ return NULL;
+ }
+
+ state->filter = NULL;
+ state->username = username;
+ state->opts = access_ctx->id_ctx->opts;
+ state->conn = conn;
+ state->ev = ev;
+ state->access_ctx = access_ctx;
+ state->domain = domain;
+ state->ac_type = SDAP_ACCESS_CONTROL_LOCK;
+
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Performing access lock check for user [%s]\n", username);
+
+ state->cached_access = ldb_msg_find_attr_as_bool(user_entry,
+ SYSDB_LDAP_ACCESS_CACHED,
+ false);
+
+ /* Ok, we have one result, check if we are online or offline */
+ if (be_is_offline(be_ctx)) {
+ /* Ok, we're offline. Return from the cache */
+ ret = sdap_access_decide_offline(req);
+ goto done;
+ }
+
+ ret = sdap_get_basedn_user_entry(state, user_entry, state->username,
+ &state->basedn);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ DEBUG(SSSDBG_TRACE_FUNC, "Checking lock against LDAP\n");
+
+ state->sdap_op = sdap_id_op_create(state,
+ state->conn->conn_cache);
+ if (!state->sdap_op) {
+ DEBUG(SSSDBG_OP_FAILURE, "sdap_id_op_create failed\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sdap_access_lock_retry(req);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ return req;
+
+done:
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
+ tevent_req_error(req, ret);
+ }
+ tevent_req_post(req, ev);
+ return req;
+}
+
+static int sdap_access_lock_retry(struct tevent_req *req)
+{
+ struct sdap_access_filter_req_ctx *state;
+ struct tevent_req *subreq;
+ int ret;
+
+ state = tevent_req_data(req, struct sdap_access_filter_req_ctx);
+ subreq = sdap_id_op_connect_send(state->sdap_op, state, &ret);
+ if (!subreq) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "sdap_id_op_connect_send failed: %d (%s)\n", ret, strerror(ret));
+ return ret;
+ }
+
+ tevent_req_set_callback(subreq, sdap_access_lock_connect_done, req);
+ return EOK;
+}
+
+static void sdap_access_lock_connect_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req;
+ struct sdap_access_filter_req_ctx *state;
+ int ret, dp_error;
+ const char *attrs[] = { SYSDB_LDAP_ACCESS_LOCKOUT, NULL };
+ const char *ppolicy_dn;
+
+ req = tevent_req_callback_data(subreq, struct tevent_req);
+ state = tevent_req_data(req, struct sdap_access_filter_req_ctx);
+
+ ret = sdap_id_op_connect_recv(subreq, &dp_error);
+ talloc_zfree(subreq);
+
+ if (ret != EOK) {
+ if (dp_error == DP_ERR_OFFLINE) {
+ ret = sdap_access_decide_offline(req);
+ if (ret == EOK) {
+ tevent_req_done(req);
+ return;
+ }
+ }
+
+ tevent_req_error(req, ret);
+ return;
+ }
+
+ ppolicy_dn = dp_opt_get_string(state->opts->basic,
+ SDAP_PPOLICY_SEARCH_BASE);
+ /* We can't find out if ppolicy is enabled, so we deny access */
+ if (ppolicy_dn == NULL) {
+ DEBUG(SSSDBG_CONF_SETTINGS,
+ "ldap_ppolicy_search_base was not defined in configuration file."
+ " Access is denied!\n");
+ tevent_req_error(req, ERR_ACCESS_DENIED);
+ return;
+ }
+
+ /* Connection to LDAP succeeded
+ * Send 'pwdLockout' request
+ */
+ subreq = sdap_get_generic_send(state,
+ state->ev,
+ state->opts,
+ sdap_id_op_handle(state->sdap_op),
+ ppolicy_dn,
+ LDAP_SCOPE_BASE,
+ NULL, attrs,
+ NULL, 0,
+ dp_opt_get_int(state->opts->basic,
+ SDAP_SEARCH_TIMEOUT),
+ false);
+ if (subreq == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Could not start LDAP communication\n");
+ tevent_req_error(req, EIO);
+ return;
+ }
+
+ tevent_req_set_callback(subreq, sdap_access_lock_get_lockout_done, req);
+}
+
+static void sdap_access_lock_get_lockout_done(struct tevent_req *subreq)
+{
+ int ret, tret, dp_error;
+ size_t num_results;
+ bool pwdLockout = false;
+ struct sysdb_attrs **results;
+ struct tevent_req *req;
+ struct sdap_access_filter_req_ctx *state;
+
+ req = tevent_req_callback_data(subreq, struct tevent_req);
+ state = tevent_req_data(req, struct sdap_access_filter_req_ctx);
+
+ ret = sdap_get_generic_recv(subreq, state, &num_results, &results);
+ talloc_zfree(subreq);
+
+ /* Check the number of responses we got
+ * If it's exactly 1, we passed the check
+ * If it's < 1, we failed the check
+ * Anything else is an error
+ */
+ if (num_results < 1) {
+ // todo doplnit base
czech todo and a C++ comment...baad :-)
+ DEBUG(SSSDBG_CONF_SETTINGS,
+ "[%s] was not found."
+ "Granting access.\n", SYSDB_LDAP_ACCESS_LOCKOUT);
+ } else if (results == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "num_results > 0, but results is NULL\n");
+ ret = ERR_INTERNAL;
+ goto done;
+ } else if (num_results > 1) {
+ /* It should not be possible to get more than one reply
+ * here, since we're doing a base-scoped search
+ */
+ DEBUG(SSSDBG_CRIT_FAILURE, "Received multiple replies\n");
+ ret = ERR_INTERNAL;
+ goto done;
+ } else { /* Ok, we got a single reply */
+ ret = sysdb_attrs_get_bool(results[0], SYSDB_LDAP_ACCESS_LOCKOUT,
+ &pwdLockout);
You don't check ret here.
+ }
+
+ if (pwdLockout) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Password policy is enabled on LDAP server.\n");
+
+ subreq = sdap_access_locked_time_send(req);
This function name shouldn't end with _send, it's not a standalone
request. We tend to use _step for these. Also is there any reason why
you can't set the callback inside the function and return just errno?
Alternatively, don't wrap the search in a function and just put the
contents of sdap_access_locked_time_send() here.
+ if (subreq == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "sdap_access_lock_send failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
+ tevent_req_set_callback(subreq, sdap_access_locked_time_done, req);
+ ret = EAGAIN;
+ goto done;
+
+ } else {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Password policy is disabled on LDAP server "
+ "- storing 'access granted' in sysdb.\n");
+ tret = sdap_save_user_cache_bool(state->domain, state->username,
+ SYSDB_LDAP_ACCESS_CACHED, true);
+ if (tret != EOK) {
+ /* Failing to save to the cache is non-fatal.
+ * Just return the result.
+ */
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to set user locked attribute\n");
+ goto done;
+ }
+
+ ret = EOK;
+ goto done;
+ }
+
+done:
+ if (ret != EAGAIN) {
+ /* release connection */
+ tret = sdap_id_op_done(state->sdap_op, ret, &dp_error);
+ if (tret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "sdap_get_generic_send() returned error [%d][%s]\n",
+ ret, sss_strerror(ret));
+ }
+
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
+ tevent_req_error(req, ret);
+ }
+ }
+}
+
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
