On Mon, 2012-05-21 at 13:04 +0200, Jakub Hrozek wrote: > On Mon, May 14, 2012 at 11:04:00AM -0400, Stephen Gallagher wrote: > > Joshua Roys submitted a patch in > > https://bugzilla.redhat.com/show_bug.cgi?id=726456 to add support for > > the Netscape password warning expiration control. I've made some > > modifications to his original patch to clean it up, so now I'm sending > > it to the list for further code-review. > > > > Fixes https://fedorahosted.org/sssd/ticket/984 > > > + if (response_controls[c]->ldctl_value.bv_len > 32) > > + continue; > > + if (!state->ppolicy) > > + state->ppolicy = talloc(state, struct > > sdap_ppolicy_data); > > I would prefer to enclose even single-line statements (well, perhaps the > continue is OK as well as return <code> would be..). I've been hit > myself by adding a new statement after if without noticing there's no > brackets.. >
Yeah, I usually do the same. I forgot to change that from the
originally-submitted patch. Fixed.
> > + /* ensure that bv_val is a null-terminated string */
> > + nval = talloc_strndup(NULL,
> > +
> > response_controls[c]->ldctl_value.bv_val,
> > +
> > response_controls[c]->ldctl_value.bv_len);
> > + if (nval == NULL) {
> > + ret = ENOMEM;
> > + goto done;
> > + }
> > + state->ppolicy->expire = strtouint32(nval, NULL, 10);
> > + if (errno != 0) {
> > + ret = errno;
> > + DEBUG(SSSDBG_MINOR_FAILURE,
> > + ("Could not convert control response to an
> > integer. ",
> > + "[%s]\n", strerror(ret)));
>
> nval should be freed here (or perhaps allocated on state..)
>
Strictly speaking, it would have been freed in the done: label. I
originally did that because I thought I was going to hang onto nval
longer. Since I clearly do not need to, I rearranged that so that it's
freed right after the conversion.
I made one additional change. I realized that we were overloading 'ret'
with both errno and ldap return values. I created a new variable 'lret'
to differentiate them and avoid polluting the values.
From c93bcdc53646dabdb7066978e92b4ea8ade9b4be Mon Sep 17 00:00:00 2001
From: Joshua Roys <roysj...@gmail.com>
Date: Mon, 14 May 2012 10:23:34 -0400
Subject: [PATCH] Simple implementation of Netscape password warning
expiration control
---
src/providers/ldap/sdap_async_connection.c | 96 +++++++++++++++++++++-------
src/util/sss_ldap.h | 8 +++
2 files changed, 82 insertions(+), 22 deletions(-)
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index e933e296b7df20ff8d034c2a11745b5c68b25e65..efd9cd8cc7205e4cb838523b0311ffd50805d590 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -26,6 +26,7 @@
#include "util/util.h"
#include "util/sss_krb5.h"
#include "util/sss_ldap.h"
+#include "util/strtonum.h"
#include "providers/ldap/sdap_async_private.h"
#include "providers/ldap/ldap_common.h"
@@ -541,7 +542,9 @@ static void simple_bind_done(struct sdap_op *op,
struct simple_bind_state *state = tevent_req_data(req,
struct simple_bind_state);
char *errmsg = NULL;
- int ret;
+ char *nval;
+ errno_t ret;
+ int lret;
LDAPControl **response_controls;
int c;
ber_int_t pp_grace;
@@ -555,30 +558,33 @@ static void simple_bind_done(struct sdap_op *op,
state->reply = talloc_steal(state, reply);
- ret = ldap_parse_result(state->sh->ldap, state->reply->msg,
+ lret = ldap_parse_result(state->sh->ldap, state->reply->msg,
&state->result, NULL, &errmsg, NULL,
&response_controls, 0);
- if (ret != LDAP_SUCCESS) {
- DEBUG(2, ("ldap_parse_result failed (%d)\n", state->op->msgid));
+ if (lret != LDAP_SUCCESS) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("ldap_parse_result failed (%d)\n", state->op->msgid));
ret = EIO;
goto done;
}
if (response_controls == NULL) {
- DEBUG(5, ("Server returned no controls.\n"));
+ DEBUG(SSSDBG_TRACE_LIBS, ("Server returned no controls.\n"));
state->ppolicy = NULL;
} else {
for (c = 0; response_controls[c] != NULL; c++) {
- DEBUG(9, ("Server returned control [%s].\n",
- response_controls[c]->ldctl_oid));
+ DEBUG(SSSDBG_TRACE_INTERNAL,
+ ("Server returned control [%s].\n",
+ response_controls[c]->ldctl_oid));
if (strcmp(response_controls[c]->ldctl_oid,
LDAP_CONTROL_PASSWORDPOLICYRESPONSE) == 0) {
- ret = ldap_parse_passwordpolicy_control(state->sh->ldap,
+ lret = ldap_parse_passwordpolicy_control(state->sh->ldap,
response_controls[c],
&pp_expire, &pp_grace,
&pp_error);
- if (ret != LDAP_SUCCESS) {
- DEBUG(1, ("ldap_parse_passwordpolicy_control failed.\n"));
+ if (lret != LDAP_SUCCESS) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("ldap_parse_passwordpolicy_control failed.\n"));
ret = EIO;
goto done;
}
@@ -586,9 +592,10 @@ static void simple_bind_done(struct sdap_op *op,
DEBUG(7, ("Password Policy Response: expire [%d] grace [%d] "
"error [%s].\n", pp_expire, pp_grace,
ldap_passwordpolicy_err2txt(pp_error)));
- state->ppolicy = talloc(state, struct sdap_ppolicy_data);
+ if (!state->ppolicy)
+ state->ppolicy = talloc_zero(state,
+ struct sdap_ppolicy_data);
if (state->ppolicy == NULL) {
- DEBUG(1, ("talloc failed.\n"));
ret = ENOMEM;
goto done;
}
@@ -596,36 +603,81 @@ static void simple_bind_done(struct sdap_op *op,
state->ppolicy->expire = pp_expire;
if (state->result == LDAP_SUCCESS) {
if (pp_error == PP_changeAfterReset) {
- DEBUG(4, ("Password was reset. "
- "User must set a new password.\n"));
+ DEBUG(SSSDBG_TRACE_LIBS,
+ ("Password was reset. "
+ "User must set a new password.\n"));
state->result = LDAP_X_SSSD_PASSWORD_EXPIRED;
} else if (pp_grace > 0) {
- DEBUG(4, ("Password expired. "
- "[%d] grace logins remaining.\n", pp_grace));
+ DEBUG(SSSDBG_TRACE_LIBS,
+ ("Password expired. "
+ "[%d] grace logins remaining.\n",
+ pp_grace));
} else if (pp_expire > 0) {
- DEBUG(4, ("Password will expire in [%d] seconds.\n",
- pp_expire));
+ DEBUG(SSSDBG_TRACE_LIBS,
+ ("Password will expire in [%d] seconds.\n",
+ pp_expire));
}
} else if (state->result == LDAP_INVALID_CREDENTIALS &&
pp_error == PP_passwordExpired) {
- DEBUG(4,
+ DEBUG(SSSDBG_TRACE_LIBS,
("Password expired user must set a new password.\n"));
state->result = LDAP_X_SSSD_PASSWORD_EXPIRED;
}
+ } else if (strcmp(response_controls[c]->ldctl_oid,
+ LDAP_CONTROL_PWEXPIRED) == 0) {
+ DEBUG(SSSDBG_TRACE_LIBS,
+ ("Password expired user must set a new password.\n"));
+ state->result = LDAP_X_SSSD_PASSWORD_EXPIRED;
+ } else if (strcmp(response_controls[c]->ldctl_oid,
+ LDAP_CONTROL_PWEXPIRING) == 0) {
+ /* ignore controls with suspiciously long values */
+ if (response_controls[c]->ldctl_value.bv_len > 32) {
+ continue;
+ }
+
+ if (!state->ppolicy) {
+ state->ppolicy = talloc(state, struct sdap_ppolicy_data);
+ }
+
+ if (state->ppolicy == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ /* ensure that bv_val is a null-terminated string */
+ nval = talloc_strndup(NULL,
+ response_controls[c]->ldctl_value.bv_val,
+ response_controls[c]->ldctl_value.bv_len);
+ if (nval == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ state->ppolicy->expire = strtouint32(nval, NULL, 10);
+ ret = errno;
+ talloc_zfree(nval);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("Could not convert control response to an integer. ",
+ "[%s]\n", strerror(ret)));
+ goto done;
+ }
+
+ DEBUG(SSSDBG_TRACE_LIBS,
+ ("Password will expire in [%d] seconds.\n",
+ state->ppolicy->expire));
}
}
}
- DEBUG(3, ("Bind result: %s(%d), %s\n",
+ DEBUG(SSSDBG_TRACE_FUNC, ("Bind result: %s(%d), %s\n",
sss_ldap_err2string(state->result), state->result,
errmsg ? errmsg : "no errmsg set"));
- ret = LDAP_SUCCESS;
+ ret = EOK;
done:
ldap_controls_free(response_controls);
ldap_memfree(errmsg);
- if (ret == LDAP_SUCCESS) {
+ if (ret == EOK) {
tevent_req_done(req);
} else {
tevent_req_error(req, ret);
diff --git a/src/util/sss_ldap.h b/src/util/sss_ldap.h
index 8a69b832965bf5ad23986a9b64cb5252cc3b1999..46829259aedcf4a4f2ba3f94fc059c343c0e9ba6 100644
--- a/src/util/sss_ldap.h
+++ b/src/util/sss_ldap.h
@@ -29,6 +29,14 @@
#define LDAP_X_SSSD_PASSWORD_EXPIRED 0x555D
+#ifndef LDAP_CONTROL_PWEXPIRED
+#define LDAP_CONTROL_PWEXPIRED "2.16.840.1.113730.3.4.4"
+#endif
+
+#ifndef LDAP_CONTROL_PWEXPIRING
+#define LDAP_CONTROL_PWEXPIRING "2.16.840.1.113730.3.4.5"
+#endif
+
#ifdef LDAP_OPT_DIAGNOSTIC_MESSAGE
#define SDAP_DIAGNOSTIC_MESSAGE LDAP_OPT_DIAGNOSTIC_MESSAGE
#else
--
1.7.10.1
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
