Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/22/2010 05:08 AM, Ralf Haferkamp wrote: > Am Freitag 19 März 2010 17:40:31 schrieb Sumit Bose: >> On Thu, Mar 18, 2010 at 03:12:14PM +0100, Ralf Haferkamp wrote: > [..] >>> >>> Updated patch attached. I think I fixed the coding style issues. >>> Additionally I just noticed that my orginal patch broke password >>> resets via LDAP password policies. This should be fixed now as >>> well. >>> >>> regards, >>> >>> Ralf >> >> I have tested this new version and didn't find any issues. Password >> resets are working, too. >> >> It would be nice if you can fix the whitespace errors: > Done. > > Ack and pushed to master. - -- 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/ iEYEARECAAYFAkunZuEACgkQeiVVYja6o6NjrgCgpjkv2cs6EVYDeUh9Qq0xOoQN zXgAnjoa62Id0UlSlWGCVljuup5beqWZ =hdpb -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
Am Freitag 19 März 2010 17:40:31 schrieb Sumit Bose:
> On Thu, Mar 18, 2010 at 03:12:14PM +0100, Ralf Haferkamp wrote:
[..]
> >
> > Updated patch attached. I think I fixed the coding style issues.
> > Additionally I just noticed that my orginal patch broke password
> > resets via LDAP password policies. This should be fixed now as
> > well.
> >
> > regards,
> >
> > Ralf
>
> I have tested this new version and didn't find any issues. Password
> resets are working, too.
>
> It would be nice if you can fix the whitespace errors:
Done.
--
regards,
Ralf
From b54fc164cad14de4a713fe0c054ad8f7b718f105 Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp
Date: Fri, 12 Mar 2010 10:54:40 +0100
Subject: [PATCH] Improvements for LDAP Password Policy support
Display warnings about remaining grace logins and password
expiration to the user, when LDAP Password Policies are used.
Improved detection if LDAP Password policies are supported by
LDAP Server.
---
src/providers/ldap/ldap_auth.c | 52 +-
src/providers/ldap/sdap.h |5 ++
src/providers/ldap/sdap_async.h|6 ++-
src/providers/ldap/sdap_async_connection.c | 53 +++
src/sss_client/pam_sss.c | 82
src/sss_client/sss_cli.h | 23 ++---
6 files changed, 201 insertions(+), 20 deletions(-)
diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
index 5228703..8c77e3a 100644
--- a/src/providers/ldap/ldap_auth.c
+++ b/src/providers/ldap/ldap_auth.c
@@ -7,6 +7,7 @@
Sumit Bose
Copyright (C) 2008 Red Hat
+Copyright (C) 2010, rha...@suse.de, Novell Inc.
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
@@ -135,6 +136,39 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd, time_t now,
return EOK;
}
+static errno_t check_pwexpire_ldap(struct pam_data *pd,
+ struct sdap_ppolicy_data *ppolicy,
+ enum sdap_result *result)
+{
+if (ppolicy->grace > 0 || ppolicy->expire > 0) {
+uint32_t *data;
+uint32_t *ptr;
+
+data = talloc_size(pd, 2* sizeof(uint32_t));
+if (data == NULL) {
+DEBUG(1, ("talloc_size failed.\n"));
+return ENOMEM;
+}
+
+ptr = data;
+if (ppolicy->grace > 0) {
+*ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
+ptr++;
+*ptr = ppolicy->grace;
+} else if (ppolicy->expire > 0) {
+*ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
+ptr++;
+*ptr = ppolicy->expire;
+}
+
+pam_add_response(pd, SSS_PAM_USER_INFO, 2* sizeof(uint32_t),
+ (uint8_t*)data);
+}
+
+*result = SDAP_AUTH_SUCCESS;
+return EOK;
+}
+
static errno_t string_to_shadowpw_days(const char *s, long *d)
{
long l;
@@ -569,8 +603,15 @@ static void auth_bind_user_done(struct tevent_req *subreq)
struct auth_state *state = tevent_req_data(req,
struct auth_state);
int ret;
-
-ret = sdap_auth_recv(subreq, &state->result);
+struct sdap_ppolicy_data *ppolicy;
+
+ret = sdap_auth_recv(subreq, state, &state->result, &ppolicy);
+if (ppolicy != NULL) {
+DEBUG(9,("Found ppolicy data, "
+ "assuming LDAP password policies are active.\n"));
+state->pw_expire_type = PWEXPIRE_LDAP_PASSWORD_POLICY;
+state->pw_expire_data = ppolicy;
+}
talloc_zfree(subreq);
if (ret) {
tevent_req_error(req, ret);
@@ -960,6 +1001,13 @@ static void sdap_pam_auth_done(struct tevent_req *req)
}
break;
case PWEXPIRE_LDAP_PASSWORD_POLICY:
+ret = check_pwexpire_ldap(state->pd, pw_expire_data, &result);
+if (ret != EOK) {
+DEBUG(1, ("check_pwexpire_ldap failed.\n"));
+state->pd->pam_status = PAM_SYSTEM_ERR;
+goto done;
+}
+break;
case PWEXPIRE_NONE:
break;
default:
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 007185f..f0e345e 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -85,6 +85,11 @@ struct sdap_service {
char *uri;
};
+struct sdap_ppolicy_data {
+int grace;
+int expire;
+};
+
#define SYSDB_SHADOWPW_LASTCHANGE "shadowLastChange"
#define SYSDB_SHADOWPW_MIN "shadowMin"
#define SYSDB_SHADOWPW_MAX "shadowMax"
diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index 3c52d23..888df6b 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -76,7 +76,11 @@ struct tevent_req *sdap_auth_send(TALLOC_CTX *memctx,
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
On Thu, Mar 18, 2010 at 03:12:14PM +0100, Ralf Haferkamp wrote:
> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> > On Wed, 17 Mar 2010 15:33:38 +0100
> >
> > Ralf Haferkamp wrote:
> > > Hi,
> > >
> > > here's another set of enhancements to the LDAP Password Policy
> > > support in the PAM module and the LDAP backend. The PAM module now
> > > issues warning when either the grace counter or the expire counter
> > > of the LDAP Ppolicy Control in > 0.
> > >
> > > Addtionally I changed the detection for Ppolicy support of the LDAP
> > > Server a bit. If the Server returned the Ppolicy Control in the Bind
> > > Response ppolicy support is assumed.
> > >
> > > I left the original check for "pwdAttribute" intact, though I think
> > > it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
> > > usually not part of the user's entry, it's part of the Entry that
> > > contains the Policy. Addtionally it might be protected by ACLs and
> > > not be returned for anonymous (without losing any functionality).
> >
> > Ralf,
> > patch looks mostly good, but there are some heavy coding style
> > violations.
> >
> > if ( condition ){ is not good, it should be: is (condition) {
> >
> > same for some functions foo( ccc ); is not good, use foo(ccc);
> >
> > Ie, no space in parenthesis, a space only after keywords, and
> > always before the opening {
> >
> > Don't use ++x, but x++ if possible.
> >
> > Also there is at least one place where the return of talloc is not
> > checked.
> >
> > Finally please try to keep the 80 columns limit where possible.
>
> Updated patch attached. I think I fixed the coding style issues.
> Additionally I just noticed that my orginal patch broke password resets
> via LDAP password policies. This should be fixed now as well.
>
> regards,
> Ralf
I have tested this new version and didn't find any issues. Password
resets are working, too.
It would be nice if you can fix the whitespace errors:
Applying: Improvements for LDAP Password Policy support
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:187: trailing whitespace.
DEBUG(4,
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:291: trailing whitespace.
ret = snprintf(user_msg, sizeof(user_msg),
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:300: trailing whitespace.
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:337: trailing whitespace.
ret = snprintf(user_msg, sizeof(user_msg),
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:344: trailing whitespace.
warning: squelched 2 whitespace errors
warning: 7 lines add whitespace errors.
bye,
Sumit
> From 0b1f123f20f4944425d70b02bb346b78176d0abe Mon Sep 17 00:00:00 2001
> From: Ralf Haferkamp
> Date: Fri, 12 Mar 2010 10:54:40 +0100
> Subject: [PATCH] Improvements for LDAP Password Policy support
>
> Display warnings about remaining grace logins and password
> expiration to the user, when LDAP Password Policies are used.
>
> Improved detection if LDAP Password policies are supported by
> LDAP Server.
> ---
> src/providers/ldap/ldap_auth.c | 52 +-
> src/providers/ldap/sdap.h |5 ++
> src/providers/ldap/sdap_async.h|6 ++-
> src/providers/ldap/sdap_async_connection.c | 53 +++
> src/sss_client/pam_sss.c | 82
>
> src/sss_client/sss_cli.h | 23 ++---
> 6 files changed, 201 insertions(+), 20 deletions(-)
>
> diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
> index 5228703..b572bf2 100644
> --- a/src/providers/ldap/ldap_auth.c
> +++ b/src/providers/ldap/ldap_auth.c
> @@ -7,6 +7,7 @@
> Sumit Bose
>
> Copyright (C) 2008 Red Hat
> +Copyright (C) 2010, rha...@suse.de, Novell Inc.
>
> 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
> @@ -135,6 +136,39 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd,
> time_t now,
> return EOK;
> }
>
> +static errno_t check_pwexpire_ldap(struct pam_data *pd,
> + struct sdap_ppolicy_data *ppolicy,
> + enum sdap_result *result)
> +{
> +if (ppolicy->grace > 0 || ppolicy->expire > 0) {
> +uint32_t *data;
> +uint32_t *ptr;
> +
> +data = talloc_size(pd, 2* sizeof(uint32_t));
> +if (*data == NULL) {
> +DEBUG(1, ("talloc_size failed.\n"));
> +return ENOMEM;
> +}
> +
> +ptr = data;
> +if (ppolicy->grace > 0) {
> +*ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
> +ptr++;
> +*ptr = ppolicy->grace;
> +} else if (ppolicy->expire > 0) {
> +*ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
> +ptr++;
> +*ptr = ppolicy->expire;
> +
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
Am Donnerstag 18 März 2010 17:15:39 schrieb Dmitri Pal:
> Ralf Haferkamp wrote:
> > Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> >> Ralf Haferkamp wrote:
> >>> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> On Wed, 17 Mar 2010 15:33:38 +0100
>
> Ralf Haferkamp wrote:
> > Hi,
> >
> > here's another set of enhancements to the LDAP Password Policy
> > support in the PAM module and the LDAP backend. The PAM module
> > now issues warning when either the grace counter or the expire
> > counter of the LDAP Ppolicy Control in > 0.
> >
> > Addtionally I changed the detection for Ppolicy support of the
> > LDAP Server a bit. If the Server returned the Ppolicy Control in
> > the Bind Response ppolicy support is assumed.
> >
> > I left the original check for "pwdAttribute" intact, though I
> > think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> > Attribute is usually not part of the user's entry, it's part of
> > the Entry that contains the Policy. Addtionally it might be
> > protected by ACLs and not be returned for anonymous (without
> > losing any functionality).
>
> Ralf,
> patch looks mostly good, but there are some heavy coding style
> violations.
>
> if ( condition ){ is not good, it should be: is (condition) {
>
> same for some functions foo( ccc ); is not good, use foo(ccc);
>
> Ie, no space in parenthesis, a space only after keywords, and
> always before the opening {
>
> Don't use ++x, but x++ if possible.
>
> Also there is at least one place where the return of talloc is
> not checked.
>
> Finally please try to keep the 80 columns limit where possible.
> >>>
> >>> Updated patch attached. I think I fixed the coding style issues.
> >>> Additionally I just noticed that my orginal patch broke password
> >>> resets via LDAP password policies. This should be fixed now as
> >>> well.
> >>>
> >>> regards,
> >>>
> >>> Ralf
> >>
> >> +data = talloc_size(pd, 2* sizeof(uint32_t));
> >> +if (*data == NULL) {
> >> +DEBUG(1, ("talloc_size failed.\n"));
> >> +return ENOMEM;
> >> +}
> >>
> >>
> >> The returned value check does not look right.
> >> I do not know if there are other places with similar logic.
> >
> > There weren't (The one above was embarrasing enough. I must have
> > overlooked the compiler warning :| ). Updated patch attached.
>
> Shouldn't it be freed somewhere down within the same function?
As Sumit already pointed, that will happen when the associated
struct pam_data is talloc_free'd after the response was sent to the
client.
Ralf
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
On Thu, Mar 18, 2010 at 12:15:39PM -0400, Dmitri Pal wrote:
> Ralf Haferkamp wrote:
> > Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> >
> >> Ralf Haferkamp wrote:
> >>
> >>> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> >>>
> On Wed, 17 Mar 2010 15:33:38 +0100
>
> Ralf Haferkamp wrote:
>
> > Hi,
> >
> > here's another set of enhancements to the LDAP Password Policy
> > support in the PAM module and the LDAP backend. The PAM module now
> > issues warning when either the grace counter or the expire counter
> > of the LDAP Ppolicy Control in > 0.
> >
> > Addtionally I changed the detection for Ppolicy support of the
> > LDAP Server a bit. If the Server returned the Ppolicy Control in
> > the Bind Response ppolicy support is assumed.
> >
> > I left the original check for "pwdAttribute" intact, though I
> > think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> > Attribute is usually not part of the user's entry, it's part of
> > the Entry that contains the Policy. Addtionally it might be
> > protected by ACLs and not be returned for anonymous (without
> > losing any functionality).
> >
> Ralf,
> patch looks mostly good, but there are some heavy coding style
> violations.
>
> if ( condition ){ is not good, it should be: is (condition) {
>
> same for some functions foo( ccc ); is not good, use foo(ccc);
>
> Ie, no space in parenthesis, a space only after keywords, and
> always before the opening {
>
> Don't use ++x, but x++ if possible.
>
> Also there is at least one place where the return of talloc is not
> checked.
>
> Finally please try to keep the 80 columns limit where possible.
>
> >>> Updated patch attached. I think I fixed the coding style issues.
> >>> Additionally I just noticed that my orginal patch broke password
> >>> resets via LDAP password policies. This should be fixed now as
> >>> well.
> >>>
> >>> regards,
> >>>
> >>> Ralf
> >>>
> >> +data = talloc_size(pd, 2* sizeof(uint32_t));
> >> +if (*data == NULL) {
> >> +DEBUG(1, ("talloc_size failed.\n"));
> >> +return ENOMEM;
> >> +}
> >>
> >>
> >> The returned value check does not look right.
> >> I do not know if there are other places with similar logic.
> >>
> > There weren't (The one above was embarrasing enough. I must have
> > overlooked the compiler warning :| ). Updated patch attached.
> >
>
> Shouldn't it be freed somewhere down within the same function?
>
No, this data will be send to a client in a later call and freed by a
talloc_free at the end to the client request.
bye,
Sumit
> >
> >> My eye just caught it when I was scrolling through the patch...
> >>
> >
> > Ralf
> >
> >
> >
> > ___
> > sssd-devel mailing list
> > sssd-devel@lists.fedorahosted.org
> > https://fedorahosted.org/mailman/listinfo/sssd-devel
>
>
> --
> Thank you,
> Dmitri Pal
>
> Engineering Manager IPA project,
> Red Hat Inc.
>
>
> ---
> Looking to carve out IT costs?
> www.redhat.com/carveoutcosts/
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
Ralf Haferkamp wrote:
> Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
>
>> Ralf Haferkamp wrote:
>>
>>> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
>>>
On Wed, 17 Mar 2010 15:33:38 +0100
Ralf Haferkamp wrote:
> Hi,
>
> here's another set of enhancements to the LDAP Password Policy
> support in the PAM module and the LDAP backend. The PAM module now
> issues warning when either the grace counter or the expire counter
> of the LDAP Ppolicy Control in > 0.
>
> Addtionally I changed the detection for Ppolicy support of the
> LDAP Server a bit. If the Server returned the Ppolicy Control in
> the Bind Response ppolicy support is assumed.
>
> I left the original check for "pwdAttribute" intact, though I
> think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> Attribute is usually not part of the user's entry, it's part of
> the Entry that contains the Policy. Addtionally it might be
> protected by ACLs and not be returned for anonymous (without
> losing any functionality).
>
Ralf,
patch looks mostly good, but there are some heavy coding style
violations.
if ( condition ){ is not good, it should be: is (condition) {
same for some functions foo( ccc ); is not good, use foo(ccc);
Ie, no space in parenthesis, a space only after keywords, and
always before the opening {
Don't use ++x, but x++ if possible.
Also there is at least one place where the return of talloc is not
checked.
Finally please try to keep the 80 columns limit where possible.
>>> Updated patch attached. I think I fixed the coding style issues.
>>> Additionally I just noticed that my orginal patch broke password
>>> resets via LDAP password policies. This should be fixed now as
>>> well.
>>>
>>> regards,
>>>
>>> Ralf
>>>
>> +data = talloc_size(pd, 2* sizeof(uint32_t));
>> +if (*data == NULL) {
>> +DEBUG(1, ("talloc_size failed.\n"));
>> +return ENOMEM;
>> +}
>>
>>
>> The returned value check does not look right.
>> I do not know if there are other places with similar logic.
>>
> There weren't (The one above was embarrasing enough. I must have
> overlooked the compiler warning :| ). Updated patch attached.
>
Shouldn't it be freed somewhere down within the same function?
>
>> My eye just caught it when I was scrolling through the patch...
>>
>
> Ralf
>
>
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> Ralf Haferkamp wrote:
> > Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> >> On Wed, 17 Mar 2010 15:33:38 +0100
> >>
> >> Ralf Haferkamp wrote:
> >>> Hi,
> >>>
> >>> here's another set of enhancements to the LDAP Password Policy
> >>> support in the PAM module and the LDAP backend. The PAM module now
> >>> issues warning when either the grace counter or the expire counter
> >>> of the LDAP Ppolicy Control in > 0.
> >>>
> >>> Addtionally I changed the detection for Ppolicy support of the
> >>> LDAP Server a bit. If the Server returned the Ppolicy Control in
> >>> the Bind Response ppolicy support is assumed.
> >>>
> >>> I left the original check for "pwdAttribute" intact, though I
> >>> think it doesn't make a lot of sense. The "pwdAttribute" LDAP
> >>> Attribute is usually not part of the user's entry, it's part of
> >>> the Entry that contains the Policy. Addtionally it might be
> >>> protected by ACLs and not be returned for anonymous (without
> >>> losing any functionality).
> >>
> >> Ralf,
> >> patch looks mostly good, but there are some heavy coding style
> >> violations.
> >>
> >> if ( condition ){ is not good, it should be: is (condition) {
> >>
> >> same for some functions foo( ccc ); is not good, use foo(ccc);
> >>
> >> Ie, no space in parenthesis, a space only after keywords, and
> >> always before the opening {
> >>
> >> Don't use ++x, but x++ if possible.
> >>
> >> Also there is at least one place where the return of talloc is not
> >> checked.
> >>
> >> Finally please try to keep the 80 columns limit where possible.
> >
> > Updated patch attached. I think I fixed the coding style issues.
> > Additionally I just noticed that my orginal patch broke password
> > resets via LDAP password policies. This should be fixed now as
> > well.
> >
> > regards,
> >
> > Ralf
>
> +data = talloc_size(pd, 2* sizeof(uint32_t));
> +if (*data == NULL) {
> +DEBUG(1, ("talloc_size failed.\n"));
> +return ENOMEM;
> +}
>
>
> The returned value check does not look right.
> I do not know if there are other places with similar logic.
There weren't (The one above was embarrasing enough. I must have
overlooked the compiler warning :| ). Updated patch attached.
> My eye just caught it when I was scrolling through the patch...
Ralf
From 1bcc807693212861807849b582bae3bb75e889ca Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp
Date: Fri, 12 Mar 2010 10:54:40 +0100
Subject: [PATCH] Improvements for LDAP Password Policy support
Display warnings about remaining grace logins and password
expiration to the user, when LDAP Password Policies are used.
Improved detection if LDAP Password policies are supported by
LDAP Server.
---
src/providers/ldap/ldap_auth.c | 52 +-
src/providers/ldap/sdap.h |5 ++
src/providers/ldap/sdap_async.h|6 ++-
src/providers/ldap/sdap_async_connection.c | 53 +++
src/sss_client/pam_sss.c | 82
src/sss_client/sss_cli.h | 23 ++---
6 files changed, 201 insertions(+), 20 deletions(-)
diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
index 5228703..8c77e3a 100644
--- a/src/providers/ldap/ldap_auth.c
+++ b/src/providers/ldap/ldap_auth.c
@@ -7,6 +7,7 @@
Sumit Bose
Copyright (C) 2008 Red Hat
+Copyright (C) 2010, rha...@suse.de, Novell Inc.
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
@@ -135,6 +136,39 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd, time_t now,
return EOK;
}
+static errno_t check_pwexpire_ldap(struct pam_data *pd,
+ struct sdap_ppolicy_data *ppolicy,
+ enum sdap_result *result)
+{
+if (ppolicy->grace > 0 || ppolicy->expire > 0) {
+uint32_t *data;
+uint32_t *ptr;
+
+data = talloc_size(pd, 2* sizeof(uint32_t));
+if (data == NULL) {
+DEBUG(1, ("talloc_size failed.\n"));
+return ENOMEM;
+}
+
+ptr = data;
+if (ppolicy->grace > 0) {
+*ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
+ptr++;
+*ptr = ppolicy->grace;
+} else if (ppolicy->expire > 0) {
+*ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
+ptr++;
+*ptr = ppolicy->expire;
+}
+
+pam_add_response(pd, SSS_PAM_USER_INFO, 2* sizeof(uint32_t),
+ (uint8_t*)data);
+}
+
+*result = SDAP_AUTH_SUCCESS;
+return EOK;
+}
+
static errno_t string_to_shadowpw_days(const char *s, long *d)
{
long l;
@@ -569,8 +603,15 @@ static void auth_bind_user_done(struct tevent_req *subreq)
struct auth_state *state = tevent_req_data(req,
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal:
> Ralf Haferkamp wrote:
[..]
> >
> > Updated patch attached. I think I fixed the coding style issues.
> > Additionally I just noticed that my orginal patch broke password
> > resets via LDAP password policies. This should be fixed now as
> > well.
> >
> > regards,
> >
> > Ralf
>
> +data = talloc_size(pd, 2* sizeof(uint32_t));
> +if (*data == NULL) {
> +DEBUG(1, ("talloc_size failed.\n"));
> +return ENOMEM;
> +}
>
>
> The returned value check does not look right.
> I do not know if there are other places with similar logic.
> My eye just caught it when I was scrolling through the patch...
Oops, thanks for catching that. I'll update the patch once more.
--
Ralf
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
Ralf Haferkamp wrote:
> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
>
>> On Wed, 17 Mar 2010 15:33:38 +0100
>>
>> Ralf Haferkamp wrote:
>>
>>> Hi,
>>>
>>> here's another set of enhancements to the LDAP Password Policy
>>> support in the PAM module and the LDAP backend. The PAM module now
>>> issues warning when either the grace counter or the expire counter
>>> of the LDAP Ppolicy Control in > 0.
>>>
>>> Addtionally I changed the detection for Ppolicy support of the LDAP
>>> Server a bit. If the Server returned the Ppolicy Control in the Bind
>>> Response ppolicy support is assumed.
>>>
>>> I left the original check for "pwdAttribute" intact, though I think
>>> it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
>>> usually not part of the user's entry, it's part of the Entry that
>>> contains the Policy. Addtionally it might be protected by ACLs and
>>> not be returned for anonymous (without losing any functionality).
>>>
>> Ralf,
>> patch looks mostly good, but there are some heavy coding style
>> violations.
>>
>> if ( condition ){ is not good, it should be: is (condition) {
>>
>> same for some functions foo( ccc ); is not good, use foo(ccc);
>>
>> Ie, no space in parenthesis, a space only after keywords, and
>> always before the opening {
>>
>> Don't use ++x, but x++ if possible.
>>
>> Also there is at least one place where the return of talloc is not
>> checked.
>>
>> Finally please try to keep the 80 columns limit where possible.
>>
>
> Updated patch attached. I think I fixed the coding style issues.
> Additionally I just noticed that my orginal patch broke password resets
> via LDAP password policies. This should be fixed now as well.
>
> regards,
> Ralf
>
+data = talloc_size(pd, 2* sizeof(uint32_t));
+if (*data == NULL) {
+DEBUG(1, ("talloc_size failed.\n"));
+return ENOMEM;
+}
The returned value check does not look right.
I do not know if there are other places with similar logic.
My eye just caught it when I was scrolling through the patch...
>
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> On Wed, 17 Mar 2010 15:33:38 +0100
>
> Ralf Haferkamp wrote:
> > Hi,
> >
> > here's another set of enhancements to the LDAP Password Policy
> > support in the PAM module and the LDAP backend. The PAM module now
> > issues warning when either the grace counter or the expire counter
> > of the LDAP Ppolicy Control in > 0.
> >
> > Addtionally I changed the detection for Ppolicy support of the LDAP
> > Server a bit. If the Server returned the Ppolicy Control in the Bind
> > Response ppolicy support is assumed.
> >
> > I left the original check for "pwdAttribute" intact, though I think
> > it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
> > usually not part of the user's entry, it's part of the Entry that
> > contains the Policy. Addtionally it might be protected by ACLs and
> > not be returned for anonymous (without losing any functionality).
>
> Ralf,
> patch looks mostly good, but there are some heavy coding style
> violations.
>
> if ( condition ){ is not good, it should be: is (condition) {
>
> same for some functions foo( ccc ); is not good, use foo(ccc);
>
> Ie, no space in parenthesis, a space only after keywords, and
> always before the opening {
>
> Don't use ++x, but x++ if possible.
>
> Also there is at least one place where the return of talloc is not
> checked.
>
> Finally please try to keep the 80 columns limit where possible.
Updated patch attached. I think I fixed the coding style issues.
Additionally I just noticed that my orginal patch broke password resets
via LDAP password policies. This should be fixed now as well.
regards,
Ralf
From 0b1f123f20f4944425d70b02bb346b78176d0abe Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp
Date: Fri, 12 Mar 2010 10:54:40 +0100
Subject: [PATCH] Improvements for LDAP Password Policy support
Display warnings about remaining grace logins and password
expiration to the user, when LDAP Password Policies are used.
Improved detection if LDAP Password policies are supported by
LDAP Server.
---
src/providers/ldap/ldap_auth.c | 52 +-
src/providers/ldap/sdap.h |5 ++
src/providers/ldap/sdap_async.h|6 ++-
src/providers/ldap/sdap_async_connection.c | 53 +++
src/sss_client/pam_sss.c | 82
src/sss_client/sss_cli.h | 23 ++---
6 files changed, 201 insertions(+), 20 deletions(-)
diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
index 5228703..b572bf2 100644
--- a/src/providers/ldap/ldap_auth.c
+++ b/src/providers/ldap/ldap_auth.c
@@ -7,6 +7,7 @@
Sumit Bose
Copyright (C) 2008 Red Hat
+Copyright (C) 2010, rha...@suse.de, Novell Inc.
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
@@ -135,6 +136,39 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd, time_t now,
return EOK;
}
+static errno_t check_pwexpire_ldap(struct pam_data *pd,
+ struct sdap_ppolicy_data *ppolicy,
+ enum sdap_result *result)
+{
+if (ppolicy->grace > 0 || ppolicy->expire > 0) {
+uint32_t *data;
+uint32_t *ptr;
+
+data = talloc_size(pd, 2* sizeof(uint32_t));
+if (*data == NULL) {
+DEBUG(1, ("talloc_size failed.\n"));
+return ENOMEM;
+}
+
+ptr = data;
+if (ppolicy->grace > 0) {
+*ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
+ptr++;
+*ptr = ppolicy->grace;
+} else if (ppolicy->expire > 0) {
+*ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
+ptr++;
+*ptr = ppolicy->expire;
+}
+
+pam_add_response(pd, SSS_PAM_USER_INFO, 2* sizeof(uint32_t),
+ (uint8_t*)data);
+}
+
+*result = SDAP_AUTH_SUCCESS;
+return EOK;
+}
+
static errno_t string_to_shadowpw_days(const char *s, long *d)
{
long l;
@@ -569,8 +603,15 @@ static void auth_bind_user_done(struct tevent_req *subreq)
struct auth_state *state = tevent_req_data(req,
struct auth_state);
int ret;
-
-ret = sdap_auth_recv(subreq, &state->result);
+struct sdap_ppolicy_data *ppolicy;
+
+ret = sdap_auth_recv(subreq, state, &state->result, &ppolicy);
+if (ppolicy != NULL) {
+DEBUG(9,("Found ppolicy data, "
+ "assuming LDAP password policies are active.\n"));
+state->pw_expire_type = PWEXPIRE_LDAP_PASSWORD_POLICY;
+state->pw_expire_data = ppolicy;
+}
talloc_zfree(subreq);
if (ret) {
tevent_req_error(req, ret);
@@ -960,6 +1001,13 @@ static void sdap_pam_auth_done(struct tevent_req *req)
}
break;
case PWEXPI
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 03/18/2010 07:42 AM, Simo Sorce wrote:
> On Wed, 17 Mar 2010 15:33:38 +0100
> Ralf Haferkamp wrote:
>
>> Hi,
>>
>> here's another set of enhancements to the LDAP Password Policy
>> support in the PAM module and the LDAP backend. The PAM module now
>> issues warning when either the grace counter or the expire counter of
>> the LDAP Ppolicy Control in > 0.
>>
>> Addtionally I changed the detection for Ppolicy support of the LDAP
>> Server a bit. If the Server returned the Ppolicy Control in the Bind
>> Response ppolicy support is assumed.
>>
>> I left the original check for "pwdAttribute" intact, though I think
>> it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
>> usually not part of the user's entry, it's part of the Entry that
>> contains the Policy. Addtionally it might be protected by ACLs and
>> not be returned for anonymous (without losing any functionality).
>>
>
> Ralf,
> patch looks mostly good, but there are some heavy coding style
> violations.
>
> if ( condition ){ is not good, it should be: is (condition) {
>
> same for some functions foo( ccc ); is not good, use foo(ccc);
>
> Ie, no space in parenthesis, a space only after keywords, and
> always before the opening {
>
> Don't use ++x, but x++ if possible.
>
> Also there is at least one place where the return of talloc is not
> checked.
>
> Finally please try to keep the 80 columns limit where possible.
>
>
> Simo.
>
Ralf, for details, please see our style guide at
http://www.freeipa.org/page/Coding_Style
- --
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/
iEYEARECAAYFAkuiJCAACgkQeiVVYja6o6MW3QCeImzq9lNDhilHdYqSWXdeoXSX
hEAAnioECFUSxvbn6eb1jYTs4Pn5jTgi
=qGfe
-END PGP SIGNATURE-
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Improvements for LDAP Password Policy support
On Wed, 17 Mar 2010 15:33:38 +0100
Ralf Haferkamp wrote:
> Hi,
>
> here's another set of enhancements to the LDAP Password Policy
> support in the PAM module and the LDAP backend. The PAM module now
> issues warning when either the grace counter or the expire counter of
> the LDAP Ppolicy Control in > 0.
>
> Addtionally I changed the detection for Ppolicy support of the LDAP
> Server a bit. If the Server returned the Ppolicy Control in the Bind
> Response ppolicy support is assumed.
>
> I left the original check for "pwdAttribute" intact, though I think
> it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
> usually not part of the user's entry, it's part of the Entry that
> contains the Policy. Addtionally it might be protected by ACLs and
> not be returned for anonymous (without losing any functionality).
>
Ralf,
patch looks mostly good, but there are some heavy coding style
violations.
if ( condition ){ is not good, it should be: is (condition) {
same for some functions foo( ccc ); is not good, use foo(ccc);
Ie, no space in parenthesis, a space only after keywords, and
always before the opening {
Don't use ++x, but x++ if possible.
Also there is at least one place where the return of talloc is not
checked.
Finally please try to keep the 80 columns limit where possible.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] Improvements for LDAP Password Policy support
Hi,
here's another set of enhancements to the LDAP Password Policy support in
the PAM module and the LDAP backend. The PAM module now issues warning
when either the grace counter or the expire counter of the LDAP Ppolicy
Control in > 0.
Addtionally I changed the detection for Ppolicy support of the LDAP
Server a bit. If the Server returned the Ppolicy Control in the Bind
Response ppolicy support is assumed.
I left the original check for "pwdAttribute" intact, though I think it
doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is usually
not part of the user's entry, it's part of the Entry that contains the
Policy. Addtionally it might be protected by ACLs and not be returned for
anonymous (without losing any functionality).
--
Ralf
From 0b06bdc110a489802e359ceea3b890cf84524491 Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp
Date: Fri, 12 Mar 2010 10:54:40 +0100
Subject: [PATCH] Improvements for LDAP Password Policy support
Display warnings about remaining grace logins and password
expiration to the user, when LDAP Password Policies are used.
Improved detection if LDAP Password policies are supported by
LDAP Server.
---
src/providers/ldap/ldap_auth.c | 44 +++-
src/providers/ldap/sdap.h |5 ++
src/providers/ldap/sdap_async.h|6 ++-
src/providers/ldap/sdap_async_connection.c | 43
src/sss_client/pam_sss.c | 79
src/sss_client/sss_cli.h | 23 ++---
6 files changed, 180 insertions(+), 20 deletions(-)
diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
index 5228703..aec31dd 100644
--- a/src/providers/ldap/ldap_auth.c
+++ b/src/providers/ldap/ldap_auth.c
@@ -7,6 +7,7 @@
Sumit Bose
Copyright (C) 2008 Red Hat
+Copyright (C) 2010, rha...@suse.de, Novell Inc.
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
@@ -135,6 +136,31 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd, time_t now,
return EOK;
}
+static errno_t check_pwexpire_ldap(struct pam_data *pd,
+ struct sdap_ppolicy_data *ppolicy,
+ enum sdap_result *result)
+{
+if ( ppolicy->grace > 0 || ppolicy->expire > 0 ){
+uint32_t *data = talloc_size(pd, 2* sizeof(uint32_t));
+uint32_t *ptr = data;
+if ( ppolicy->grace > 0 ){
+*ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
+++ptr;
+*ptr = ppolicy->grace;
+} else if ( ppolicy->expire > 0 ) {
+*ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
+++ptr;
+*ptr = ppolicy->expire;
+}
+
+pam_add_response( pd, SSS_PAM_USER_INFO, 2*sizeof(uint32_t),
+ (uint8_t*)data );
+}
+
+*result = SDAP_AUTH_SUCCESS;
+return EOK;
+}
+
static errno_t string_to_shadowpw_days(const char *s, long *d)
{
long l;
@@ -569,8 +595,15 @@ static void auth_bind_user_done(struct tevent_req *subreq)
struct auth_state *state = tevent_req_data(req,
struct auth_state);
int ret;
-
-ret = sdap_auth_recv(subreq, &state->result);
+struct sdap_ppolicy_data *ppolicy;
+
+ret = sdap_auth_recv(subreq, state, &state->result, &ppolicy);
+if ( ppolicy ){
+DEBUG(9,("Found ppolicy data, "
+ "assuming LDAP password policies are active.\n"));
+state->pw_expire_type = PWEXPIRE_LDAP_PASSWORD_POLICY;
+state->pw_expire_data = ppolicy;
+}
talloc_zfree(subreq);
if (ret) {
tevent_req_error(req, ret);
@@ -960,6 +993,13 @@ static void sdap_pam_auth_done(struct tevent_req *req)
}
break;
case PWEXPIRE_LDAP_PASSWORD_POLICY:
+ret = check_pwexpire_ldap(state->pd, pw_expire_data, &result);
+if (ret != EOK) {
+DEBUG(1, ("check_pwexpire_ldap failed.\n"));
+state->pd->pam_status = PAM_SYSTEM_ERR;
+goto done;
+}
+break;
case PWEXPIRE_NONE:
break;
default:
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 007185f..f0e345e 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -85,6 +85,11 @@ struct sdap_service {
char *uri;
};
+struct sdap_ppolicy_data {
+int grace;
+int expire;
+};
+
#define SYSDB_SHADOWPW_LASTCHANGE "shadowLastChange"
#define SYSDB_SHADOWPW_MIN "shadowMin"
#define SYSDB_SHADOWPW_MAX "shadowMax"
diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index 3c52d23..888df6b 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -76,7 +76,11 @@ struct tevent_
