On 03/03/2015 03:28 PM, Jakub Hrozek wrote:
> On Mon, Mar 02, 2015 at 11:42:06PM +0100, Lukas Slebodnik wrote:
>> On (22/01/15 17:06), Pavel Reichl wrote:
>>> Second patch now contains unit test.
>> >From f8686fbb988d29309a41064a678c0daf4aa40b6b Mon Sep 17 00:00:00 2001
>>> From: Pavel Reichl <preichl(a)redhat.com>
>>> Date: Mon, 19 Jan 2015 03:24:09 -0500
>>> Subject: [PATCH 1/3] SDAP: new option - pwdlocking natural/only by
>>> admin
>>>
>>> This is a follow up to #2364.
>>> To distinguish user locked out from accessing machine via SSH if an
>>> account was administratively locked (pwdAccountLockedTime set to
>>> 000001010000Z) in the OpenLDAP Password Policy overlay or if user
>>> password is locked out from natural reasons (too many attempts,
>>> expired
>>> password).
>>>
>>> Part of solution for:
>>>
https://fedorahosted.org/sssd/ticket/2534
>> You wrote in description of this ticket that it it is a follow up to
>> #2364.
>> from my point of view it is an extension.
>>
>> Do we really need an option to enable this feature?
>>
>> The current implementation locks account with pwdAccountLockedTime
>> set to
>> 000001010000Z. The other values(dates) are ignored.
> I would prefer if we didn't add a new option as well, but since we
> released
> a version that only supported the lockout and not any other semantics,
> I don't think we can get away with just changing the functionality. A
> minor version can break functionality. But a major version can :-)
>
> So I propose the following:
> 1) Add a new value for ldap_access_order called "ppolicy" that would
> evaluate the pwdAccountLockedTime fully, including the new
> functionality in this patchset
> 2) In 1.12, deprecate the "lockout" option and log a warning that it
> will be removed in future relase and users should migrate to "ppolicy"
> option
> 3) In master (1.13), remove the "lockout" ldap_access_order value
>
> That way, we won't add a new option and I think the "ppolicy" option
is
> better anyway.
>
>> >From 942f9ab711ac787f10bcc18bd8b638e6f8a540b9 Mon Sep 17 00:00:00 2001
>>> From: Pavel Reichl <preichl(a)redhat.com>
>>> Date: Tue, 20 Jan 2015 16:27:41 -0500
>>> Subject: [PATCH 2/3] UTIL: convert GeneralizedTime to unix time
>>>
>>> New utility function *convert_time* to convert GeneralizedTime to
>>> unix time.
> According to the slapo-ppolicy man page, the pwdAccountLockedTime is
> just generalizedTime and according to RFC 4517, it's possible to use any
> timezone, but the UTC zone is preferred:
> ~~~~~~~~~~~~~~~~~~~~~
> The time value represents coordinated universal time (equivalent to
> Greenwich
> Mean Time) if the "Z" form of <g-time-zone> is used; otherwise, the
> value
> represents a local time in the time zone indicated by <g-differential>.
> In the latter case, coordinated universal time can be calculated by
> subtracting the differential from the local time. The "Z" form of
> <g-time-zone> SHOULD be used in preference to <g-differential>.
> ~~~~~~~~~~~~~~~~~~~~~
>
> I would suggest that the utility function would only support the "Z"
> form and return a special error code if another timezone is specified.
> In that case, the sssd access code would return ACCESS_DENIED and log a
> failure saying that the timezone specifier in ppolicy is not supported.
>
> The reason is that when it comes to access control, I strongly prefer
> using non-ambiguous formats. We'll see if there are any users out there
> who actually use a non-Zulu time in ppolicy and can improve the support
> later.
>
> Of course, the name of the function must make it clear that only time
> specification with a trailing "Z" is supported, also the man page should
> make it clear in description of "ppolicy" ldap_access_order that only
> "Z"
> is supported.
>
>>> ---
>>> Makefile.am | 3 ++-
>>> src/tests/util-tests.c | 47
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>> src/util/util.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> src/util/util.h | 3 +++
>>> 4 files changed, 91 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index
>>>
ca8921bb132db6b7a57d9436a3219057d08b64c6..2da86572752d0702b8983f4cc647188c8cc81382
>>> 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -1535,7 +1535,8 @@ simple_access_tests_LDADD = \
>>> libsss_test_common.la
>>>
>>> util_tests_SOURCES = \
>>> - src/tests/util-tests.c
>>> + src/tests/util-tests.c \
>>> + src/util/util.c
>> It's better to link test with internal library (libsss_util.so) rather
>> then add source file to the test. It's not necessary to change it now.
>> Please try to ammend cmocka test in future. (test_utils)
>>
>> It's not necessary to change it now. It's just info for the future.
>>
>> BTW. You didn't add $(NULL) to the end of aotomake variable.
>>
>>> util_tests_CFLAGS = \
>>> $(AM_CFLAGS) \
>>> $(CHECK_CFLAGS)
>>> diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
>>> index
>>>
94015d8e1e0efb143a4fea998f1b16db1e63365e..ebfae703ff7962c12b9eddf83d2c2e61db29b6eb
>>> 100644
>>> --- a/src/tests/util-tests.c
>>> +++ b/src/tests/util-tests.c
>>> @@ -28,6 +28,8 @@
>>> #include <sys/types.h>
>>> #include <sys/stat.h>
>>> #include <fcntl.h>
>>> +#include <stdlib.h>
>>> +
>>> #include "util/util.h"
>>> #include "util/sss_utf8.h"
>>> #include "util/murmurhash3.h"
>>> @@ -1020,6 +1022,44 @@ START_TEST(test_known_service)
>>> }
>>> END_TEST
>>>
>>> +static void convert_time_tz(void)
>> Why do wee need a static wunction which does not accept any argument?
>> It's better to inline it or add argument timezone and change
>> timezone(setenv)
>> inside function.
>>
>>> +{
>>> + errno_t ret;
>>> + time_t unix_time;
>>> +
>>> + ret = convert_time("20140801115742Z",
"%Y%m%d%H%M%SZ",
>>> &unix_time);
>>> + fail_unless(ret == EOK && difftime(1406894262, unix_time) ==
0);
>>> +}
>>> +
>>> +START_TEST(test_convert_time)
>>> +{
>>> + const char *format = "%Y%m%d%H%M%SZ";
>>> + time_t unix_time;
>>> + errno_t ret;
>>> + const char *orig_tz;
>>> +
>>> + ret = convert_time("0", format, &unix_time);
>>> + fail_unless(ret == EINVAL);
>>> + ret = convert_time("000001010000Z", format, &unix_time);
>>> + fail_unless(ret == EINVAL);
>>> +
>>> + /* test that results are still same no matter what timezone is
>>> set */
>>> + convert_time_tz();
>>> + orig_tz = getenv("TZ");
>>> +
>>> + ret = setenv("TZ", "GST-1", 1);
>>> + fail_if(ret == -1);
>>> + convert_time_tz();
>>> +
>>> + ret = setenv("TZ", "GST-2", 1);
>>> + fail_if(ret == -1);
>>> + convert_time_tz();
>>> +
>>> + ret = setenv("TZ", orig_tz, 1);
>>> + fail_if(ret == -1);
>>> +}
>>> +END_TEST
>>> +
>>> Suite *util_suite(void)
>>> {
>>> Suite *s = suite_create("util");
>>> @@ -1067,10 +1107,17 @@ Suite *util_suite(void)
>>> tcase_add_test(tc_atomicio, test_atomicio_read_exact_sized_file);
>>> tcase_add_test(tc_atomicio, test_atomicio_read_from_empty_file);
>>>
>>> + TCase *tc_convert_time = tcase_create("convert_time");
>>> + tcase_add_checked_fixture (tc_convert_time,
>> ^
>> extra space
>>
>>> + ck_leak_check_setup,
>>> + ck_leak_check_teardown);
>>> + tcase_add_test(tc_convert_time, test_convert_time);
>>> +
>>> suite_add_tcase (s, tc_util);
>>> suite_add_tcase (s, tc_utf8);
>>> suite_add_tcase (s, tc_mh3);
>>> suite_add_tcase (s, tc_atomicio);
>>> + suite_add_tcase (s, tc_convert_time);
>>>
>>> return s;
>>> }
>>> diff --git a/src/util/util.c b/src/util/util.c
>>> index
>>>
613c559bb2002686c7833642d0946e46e5a9b5d6..eb3002fd4dde5b88a6595aee7186bae8d7d773d2
>>> 100644
>>> --- a/src/util/util.c
>>> +++ b/src/util/util.c
>>> @@ -18,6 +18,7 @@
>>> along with this program. If not, see
>>> <
http://www.gnu.org/licenses/>.
>>> */
>>>
>>> +#include "config.h"
>>> #include <ctype.h>
>>> #include <netdb.h>
>>> #include <poll.h>
>>> @@ -26,6 +27,7 @@
>>> #include <arpa/inet.h>
>>> #include <talloc.h>
>>> #include <dhash.h>
>>> +#include <time.h>
>>>
>>> #include "util/util.h"
>>> #include "util/sss_utf8.h"
>>> @@ -904,3 +906,40 @@ errno_t sss_fd_nonblocking(int fd)
>>>
>>> return EOK;
>>> }
>>> +
>>> +/* Convert GeneralizedTime
>>> (
http://en.wikipedia.org/wiki/GeneralizedTime)
>>> + * to unix time (seconds since epoch). Use UTC time zone.
>>> + */
>>> +errno_t convert_time(const char *str, const char *format, time_t
>>> *unix_time)
>>> +{
>>> + char *end;
>>> + struct tm tm;
>>> +
>>> + memset(&tm, 0, sizeof(tm));
>>> +
>>> + end = strptime(str, format, &tm);
>>> + /* not all characters from format were matched */
>>> + if (end == NULL) {
>>> + DEBUG(SSSDBG_TRACE_INTERNAL,
>>> + "String [%s] failed to match format [%s].\n", str,
>>> format);
>>> + return EINVAL;
>>> + }
>>> +
>>> + /* str is 'longer' than format */
>>> + if (*end != '\0') {
>>> + DEBUG(SSSDBG_TRACE_INTERNAL,
>>> + "String [%s] is longer than format [%s].\n", str,
>>> format);
>>> + return EINVAL;
>>> + }
>>> +
>>> + *unix_time = mktime(&tm);
>>> + if (*unix_time == -1) {
>>> + DEBUG(SSSDBG_TRACE_INTERNAL,
>>> + "mktime failed to convert [%s].\n", str);
>>> + return EINVAL;
>>> + }
>>> +
>>> + tzset();
>>> + *unix_time -= timezone;
>> We prefer to touch output argument just once. (just set output
>> argumetn with
>> local variable.
>>
>> The 1st benefit is that function is not poluted with
>> dereferencing of pointer
>> (code is nicer). The second benefit is that you set default vale
>> just once.
>> It is not case in this function.
>>
>>
>>> + return EOK;
>>> +}
>>> diff --git a/src/util/util.h b/src/util/util.h
>>> index
>>>
60dbf9381c5fe26e7d42d51c6f9c7df6e3ebc4be..bc23d7bc5d6307f63a480c5faaeaa98ca921bd46
>>> 100644
>>> --- a/src/util/util.h
>>> +++ b/src/util/util.h
>>> @@ -641,4 +641,7 @@ int set_seuser(const char *login_name, const
>>> char *seuser_name,
>>> const char *mlsrange);
>>> int del_seuser(const char *login_name);
>>>
>>> +/* convert time from generalized form to unix time */
>>> +errno_t convert_time(const char *str, const char *format, time_t
>>> *unix_time);
>> Please try to prefix this function. I know it's part of internal
>> library,
>> but the name is very general and in theory can conflic with
>> function in other
>> libraries/ (prefix sss_ or s3_ is fine)
> Prefix and rename, please :-)
>
> errno_t sss_utc_to_time_t() ?
>
> btw do we need to have this function in src/util? Is it ever going to be
> used anywhere else?
>
>>
>> The output argumet shout be prefixed with underscore (_).
>> So it is obviout which pointer is input and which is output argument.
>>
>>
>>> #endif /* __SSSD_UTIL_H__ */
>>> --
>>> 2.1.0
>>>
>> >From 9bb77a8cb8501d7827da8457d434dc31321aca50 Mon Sep 17 00:00:00 2001
>>> From: Pavel Reichl <preichl(a)redhat.com>
>>> Date: Tue, 20 Jan 2015 18:34:44 -0500
>>> Subject: [PATCH 3/3] SDAP: Lock out ssh keys when account naturally
>>> expires
>>>
>>> Resolves:
>>>
https://fedorahosted.org/sssd/ticket/2534
>>> ---
>>> Makefile.am | 3 +-
>>> src/providers/ldap/sdap_access.c | 106
>>> ++++++++++++++++++++++++++++++++++-----
>>> src/providers/ldap/sdap_access.h | 1 +
>>> 3 files changed, 96 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index
>>>
2da86572752d0702b8983f4cc647188c8cc81382..e780b7098cc853dfaaa4ae4d830cc410fa94eb80
>>> 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -2383,7 +2383,8 @@ libsss_ldap_common_la_SOURCES = \
>>> src/providers/ldap/sdap_domain.c \
>>> src/providers/ldap/sdap.c \
>>> src/util/user_info_msg.c \
>>> - src/util/sss_ldap.c
>>> + src/util/sss_ldap.c \
>>> + src/util/util.c
>> The file src/util/util.c is part of internal library libsss_util.so
>> It's better to link this module with library rather then add it to
>> source file.
>>
>>> libsss_ldap_common_la_CFLAGS = \
>>> $(KRB5_CFLAGS)
>>> libsss_ldap_common_la_LIBADD = \
>>> diff --git a/src/providers/ldap/sdap_access.c
>>> b/src/providers/ldap/sdap_access.c
>>> index
>>>
e09e6b8fe55e8e8bae9ff7f46e815595cb938971..39a6f4397f83b2ca560d0fb883002a97caf73c74
>>> 100644
>>> --- a/src/providers/ldap/sdap_access.c
>>> +++ b/src/providers/ldap/sdap_access.c
>>> @@ -1568,7 +1568,9 @@ errno_t sdap_access_lock_step(struct
>>> tevent_req *req)
>>> errno_t ret;
>>> struct tevent_req *subreq;
>>> struct sdap_access_lock_req_ctx *state;
>>> - const char *attrs[] = { SYSDB_LDAP_ACCESS_LOCKED_TIME, NULL };
>>> + const char *attrs[] = { SYSDB_LDAP_ACCESS_LOCKED_TIME,
>>> + SYSDB_LDAP_ACESS_LOCKOUT_DURATION,
>>> + NULL };
>>>
>>> state = tevent_req_data(req, struct sdap_access_lock_req_ctx);
>>>
>>> @@ -1597,15 +1599,86 @@ done:
>>> return ret;
>>> }
>>>
>>> +static errno_t
>>> +decide_if_account_is_locked(bool locked_only_by_admin,
>>> + const char *pwdAccountLockedTime,
>>> + const char *pwdAccountLockedDurationTime,
>>> + bool *locked)
>> output argumet should be prefixed
>> with "_"
> Also is_account_locked() would be a shorter name :-)
>
>>> +{
>>> + errno_t ret;
>>> + time_t lock_time;
>>> + time_t duration;
>>> + time_t now;
>>> +
>>> + /* Default action is to consider account to be locked. */
>>> + *locked = true;
>> It's more readable if code does not touch output argument very often.
>> Functions is nicer if stars "*" are not used very often.
>>
>>
>>> +
>>> + /* account is permanently locked */
>>> + if (strcasecmp(pwdAccountLockedTime,
>>> + PERMANENTLY_LOCKED_ACCOUNT) == 0) {
>>> + goto done;
>> return variable ret is not initialized.
>>
>> @see warnigs
>> Error: UNINIT (CWE-457): [#def1]
>> sssd-1.12.90/src/providers/ldap/sdap_access.c:1620: var_decl:
>> Declaring variable "ret" without initializer.
>> sssd-1.12.90/src/providers/ldap/sdap_access.c:1680: uninit_use:
>> Using uninitialized value "ret".
>>
>> Error: CLANG_WARNING: [#def2]
>> sssd-1.12.90/src/providers/ldap/sdap_access.c:1680:5: warning:
>> Undefined or garbage value returned to caller
>> # return ret;
>> # ^~~~~~~~~~
>>
>> Error: COMPILER_WARNING: [#def4]
>> sssd-1.12.90/src/providers/ldap/sdap_access.c: scope_hint: In
>> function 'sdap_access_lock_step_done'
>> sssd-1.12.90/src/providers/ldap/sdap_access.c:1758:16: warning:
>> 'ret' may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>> # if (ret != EOK) {
>> #
>>
>>> + }
>>> +
>>> + if (locked_only_by_admin) {
>>> + /* We do *not* care about exact value of account locked
>>> time, we
>>> + * only *do* care if the value is equal to
>>> + * PERMANENTLY_LOCKED_ACCOUNT, which means that account is
>>> locked
>>> + * permanently.
>>> + */
>>> + DEBUG(SSSDBG_TRACE_FUNC,
>>> + "Account is beeing blocked by password policy, "
>>> + "but value: [%s] value is ignored by SSSD.\n",
>>> + pwdAccountLockedTime);
>>> + *locked = false;
>>> + } else {
>>> + /* Account may be locked out from natural reasons (too
>>> many attempts,
>>> + * expired password). In this case, pwdAccountLockedTime
>>> is also set,
>>> + * to the time of lock out.
>>> + */
>>> + ret = convert_time(pwdAccountLockedTime, "%Y%m%d%H%M%SZ",
>>> + &lock_time);
>>> + if (ret != EOK) {
>>> + DEBUG(SSSDBG_TRACE_FUNC, "convert_time() failed with
>>> %d:%s.\n",
>>> + ret, sss_strerror(ret));
>>> + goto done;
>>> + }
>>> +
>>> + now = time(NULL);
>>> +
>>> + /* Account was NOT locked in past. */
>>> + if (difftime(lock_time, now) > 0.0) {
>>> + *locked = false;
>>> + } else if (pwdAccountLockedDurationTime != NULL) {
>>> + errno = 0;
>>> + duration = strtol(pwdAccountLockedDurationTime, NULL, 0);
> I think we already have a utility function for this in
> src/util/strtonum.c, please use it.
>
>>> + if (errno) {
>>> + ret = errno;
>>> + goto done;
>>> + }
>>> + /* Lockout has expired */
>>> + if (duration != 0 && difftime(now, lock_time) >
>>> duration) {
>>> + *locked = false;
>>> + }
>>> + }
>>> + }
>>> +
>>> + ret = EOK;
>>> +
>>> +done:
>>> + return ret;
>>> +}
>>> +
>>> static void sdap_access_lock_step_done(struct tevent_req *subreq)
>>> {
>>> int ret, tret, dp_error;
>>> size_t num_results;
>>> bool locked = false;
>>> const char *pwdAccountLockedTime;
>>> + const char *pwdAccountLockedDurationTime;
>>> struct sysdb_attrs **results;
>>> struct tevent_req *req;
>>> struct sdap_access_lock_req_ctx *state;
>>> + bool pwd_admin_locked_only;
>>>
>>> req = tevent_req_callback_data(subreq, struct tevent_req);
>>> state = tevent_req_data(req, struct sdap_access_lock_req_ctx);
>>> @@ -1613,6 +1686,9 @@ static void sdap_access_lock_step_done(struct
>>> tevent_req *subreq)
>>> ret = sdap_get_generic_recv(subreq, state, &num_results,
>>> &results);
>>> talloc_zfree(subreq);
>>>
>>> + pwd_admin_locked_only = dp_opt_get_bool(state->opts->basic,
>>> + SDAP_PWDLOCKOUT_ADMIN_LOCKED_ONLY);
>>> +
>> I have already commented it in 1st patch that IMHO we do not need
>> this option
>> at all.
>>
>> If other developers agree we need that option then please move it to
>> the place
>> where we DO NEED it. Because it is used just once but initialized
>> far far away
>> from place where it is used.
>>
>> There is another warning from static analyser.
>> I think iit's false possitive because we needn't test output
>> variable of
>> dp_opt_get_bool.
>>
>>
>> Error: CHECKED_RETURN (CWE-252): [#def3]
>> sssd-1.12.90/src/providers/ldap/sdap_access.c:1701: check_return:
>> Calling "_dp_opt_get_bool" without checking return value (as is done
>> elsewhere 48 out of 67 times).
>> sssd-1.12.90/src/providers/ad/ad_common.c:1170: example_checked:
>> Example 1: "_dp_opt_get_bool(ad_ctx->ad_options->basic,
>> AD_ENABLE_GC, <anonymous>)" has its value checked in
>> "_dp_opt_get_bool(ad_ctx->ad_options->basic, AD_ENABLE_GC,
>> <anonymous>)".
>> sssd-1.12.90/src/providers/ad/ad_dyndns.c:48: example_checked:
>> Example 2: "_dp_opt_get_bool(ad_opts->dyndns_ctx->opts,
>> DP_OPT_DYNDNS_UPDATE, <anonymous>)" has its value checked in
>> "_dp_opt_get_bool(ad_opts->dyndns_ctx->opts, DP_OPT_DYNDNS_UPDATE,
>> <anonymous>) == 0".
>> sssd-1.12.90/src/providers/ad/ad_id.c:230: example_checked: Example
>> 3: "_dp_opt_get_bool(ad_ctx->ad_options->basic, AD_ENABLE_GC,
>> <anonymous>)" has its value checked in
>> "_dp_opt_get_bool(ad_ctx->ad_options->basic, AD_ENABLE_GC,
>> <anonymous>)".
>> sssd-1.12.90/src/providers/ad/ad_init.c:235: example_checked:
>> Example 4: "_dp_opt_get_bool(ad_options->basic, AD_ENABLE_DNS_SITES,
>> <anonymous>)" has its value checked in
>> "_dp_opt_get_bool(ad_options->basic, AD_ENABLE_DNS_SITES,
>> <anonymous>)".
>> sssd-1.12.90/src/providers/ipa/ipa_init.c:332: example_checked:
>> Example 5: "_dp_opt_get_bool(ipa_options->basic,
>> IPA_ENABLE_DNS_SITES, <anonymous>)" has its value checked in
>> "_dp_opt_get_bool(ipa_options->basic, IPA_ENABLE_DNS_SITES,
>> <anonymous>)".
>> sssd-1.12.90/src/providers/ldap/sdap_access.c:1754: unchecked_value:
>> Using value "pwd_admin_locked_only" as a function argument without
>> checking appropriately.
>>
>>
>>> ret = sdap_id_op_done(state->sdap_op, ret, &dp_error);
>>> if (ret != EOK) {
>>> if (dp_error == DP_ERR_OK) {
>>> @@ -1653,22 +1729,26 @@ static void
>>> sdap_access_lock_step_done(struct tevent_req *subreq)
>>> ret = ERR_INTERNAL;
>>> goto done;
>>> } else { /* Ok, we got a single reply */
>>> + ret = sysdb_attrs_get_string(results[0],
>>> SYSDB_LDAP_ACESS_LOCKOUT_DURATION,
>>> + &pwdAccountLockedDurationTime);
>>> + if (ret != EOK) {
>>> + /* This attribute might not be set even if account is
>>> locked */
>>> + pwdAccountLockedDurationTime = NULL;
>>> + }
>>> +
>>> ret = sysdb_attrs_get_string(results[0],
>>> SYSDB_LDAP_ACCESS_LOCKED_TIME,
>>> &pwdAccountLockedTime);
>>> if (ret == EOK) {
>>> - /* We do *not* care about exact value of account
>>> locked time, we
>>> - * only *do* care if the value is equal to
>>> - * PERMANENTLY_LOCKED_ACCOUNT, which means that
>>> account is locked
>>> - * permanently.
>>> - */
>>> - if (strcasecmp(pwdAccountLockedTime,
>>> - PERMANENTLY_LOCKED_ACCOUNT) == 0) {
>>> + ret = decide_if_account_is_locked(pwd_admin_locked_only,
>>> + pwdAccountLockedTime,
>>> + pwdAccountLockedDurationTime,
>>> + &locked);
>>> + if (ret != EOK) {
>>> + DEBUG(SSSDBG_MINOR_FAILURE,
>>> + "decide_if_account_is_locked failed: %d:[%s].
"
>>> + "Account will be considered to be
locked.\n",
>>> + ret, sss_strerror(ret));
>>> 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);
>>> }
>>> } else {
>>> /* Attribute SYSDB_LDAP_ACCESS_LOCKED_TIME in not be
>>> present unless
>>> diff --git a/src/providers/ldap/sdap_access.h
>>> b/src/providers/ldap/sdap_access.h
>>> index
>>>
f085e619961198b887d65ed5ee0bc5cdd90d1b20..3e59b9b2f4851a87c01f3e353c11b3bdf9bb4ff7
>>> 100644
>>> --- a/src/providers/ldap/sdap_access.h
>>> +++ b/src/providers/ldap/sdap_access.h
>>> @@ -35,6 +35,7 @@
>>> #define SYSDB_LDAP_ACCESS_CACHED_LOCKOUT
"ldap_access_lockout_allow"
>>> /* names of ppolicy attributes */
>>> #define SYSDB_LDAP_ACCESS_LOCKED_TIME "pwdAccountLockedTime"
>>> +#define SYSDB_LDAP_ACESS_LOCKOUT_DURATION "pwdLockoutDuration"
>>> #define SYSDB_LDAP_ACCESS_LOCKOUT "pwdLockout"
>>>
>>> #define LDAP_ACCESS_FILTER_NAME "filter"
>>> --
>>> 2.1.0
>> Patch works as expexted.
> Thank you for testing, I only read the patches, didn't do any testing
> yet.
>
>> Tested with 3 different values of pwdAccountLockedTime
>>
>> dn: uid=testuser1,ou=Users,dc=example,dc=com
>> changetype: modify
>> replace: pwdAccountLockedTime
>> pwdAccountLockedTime: 000001010000Z
>>
>> dn: uid=testuser1,ou=Users,dc=example,dc=com
>> changetype: modify
>> replace: pwdAccountLockedTime
>> pwdAccountLockedTime: 20150310003750Z
>>
>> dn: uid=testuser1,ou=Users,dc=example,dc=com
>> changetype: modify
>> replace: pwdAccountLockedTime
>> pwdAccountLockedTime: 20150301003750Z
>>
>> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks for comments, please see updated patchset.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel