On 02/12/2013 06:23 PM, Simo Sorce wrote:
On Tue, 2013-02-12 at 18:03 +0100, Tomas Babej wrote:
On 02/12/2013 05:50 PM, Tomas Babej wrote:
Hi,
This patch adds a check for krbprincipalexpiration attribute to
pre_bind operation
in ipa-pwd-extop dirsrv plugin. If the principal is expired, auth is
denied and LDAP_INVALID_CREDENTIALS along with the error message is
sent back to the client. Since krbprincipalexpiration attribute is
not
mandatory, if there is no value set, the check is passed.
https://fedorahosted.org/freeipa/ticket/3305
Comments inline.
@@ -1166,6 +1173,29 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
goto done;
}
+ /* check the krbPrincipalExpiration attribute is present */
+ ret = slapi_entry_attr_find(entry, "krbPrincipalExpiration",
&attr);
+ if (!ret) {
ret is not a boolean so probably better ti use (ret != 0)
+ /* if it is, check whether the principal has not expired */
+
+ principal_expire = slapi_entry_attr_get_charptr(entry,
+ "krbprincipalexpiration");
+ memset(&expire_tm, 0, sizeof (expire_tm));
+
+ if (strptime(principal_expire, "%Y%m%d%H%M%SZ", &expire_tm)){
style issue missing space between ) and {
+ expire_time = mktime(&expire_tm);
+ /* this might have overflown on 32-bit system */
This comment does not help to point out what you want to put in
evidence.
if there is an overflow what is the consequence ? Or does it mean the
next condition is taking that into account ?
Yeah, this was rather ill-worded. Replaced by a rather verbose
comment that hopefully clears things out.
+ if (current_time > expire_time && expire_time > 0){
style issue missing space between ) and {
+ LOG_FATAL("kerberos principal has expired in user
entry: %s\n",
+ dn);
I think a better phrasing would be: "The kerberos principal on %s is
expired\n"
+ errMesg = "Kerberos principal has expired.";
s/has/is/
The rest looks good to me.
Simo.
Styling issues fixed and updated patch attached :)
Tomas
>From 38cd879aac269b9a6ba506c7386eb8cd495ad6ac Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Mon, 11 Feb 2013 15:33:12 +0100
Subject: [PATCH] Deny LDAP binds for user accounts with expired principal
Adds a check for krbprincipalexpiration attribute to pre_bind operation
in ipa-pwd-extop dirsrv plugin. If the principal is expired, auth is
denied and LDAP_INVALID_CREDENTIALS along with the error message is
sent back to the client. Since krbprincipalexpiration attribute is not
mandatory, if there is no value set, the check is passed.
https://fedorahosted.org/freeipa/ticket/3305
---
.../ipa-pwd-extop/ipapwd_prepost.c | 42 +++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c
index 3b512a4744d3edddc52e224c11aaa93388d06b75..8404e92b6fcdbfe2dc18903502f33e8ae2bf37bb 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c
@@ -1123,16 +1123,23 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
Slapi_Value *value = NULL;
Slapi_Attr *attr = NULL;
struct tm expire_tm;
+ time_t current_time;
+ time_t expire_time;
char *errMesg = "Internal operations error\n"; /* error message */
char *expire = NULL; /* passwordExpirationTime attribute value */
+ char *principal_expire = NULL; /* krbPrincipalExpiration attribute value */
char *dn = NULL; /* bind DN */
Slapi_Value *objectclass;
int method; /* authentication method */
int ret = 0;
char *principal = NULL;
+ bool auth_failed = false;
LOG_TRACE("=>\n");
+ /* get current time*/
+ current_time = time(NULL);
+
/* get BIND parameters */
ret |= slapi_pblock_get(pb, SLAPI_BIND_TARGET, &dn);
ret |= slapi_pblock_get(pb, SLAPI_BIND_METHOD, &method);
@@ -1150,7 +1157,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
const char *attrs_list[] = {SLAPI_USERPWD_ATTR, "krbprincipalkey", "uid",
"krbprincipalname", "objectclass",
"passwordexpirationtime", "passwordhistory",
- NULL};
+ "krbprincipalexpiration", NULL};
/* retrieve user entry */
ret = ipapwd_getEntry(dn, &entry, (char **) attrs_list);
@@ -1166,6 +1173,33 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
goto done;
}
+ /* check the krbPrincipalExpiration attribute is present */
+ ret = slapi_entry_attr_find(entry, "krbPrincipalExpiration", &attr);
+ if (ret != 0) {
+ /* if it is, check whether the principal has not expired */
+
+ principal_expire = slapi_entry_attr_get_charptr(entry,
+ "krbprincipalexpiration");
+ memset(&expire_tm, 0, sizeof (expire_tm));
+
+ if (strptime(principal_expire, "%Y%m%d%H%M%SZ", &expire_tm)) {
+ expire_time = mktime(&expire_tm);
+
+ /* mktime returns -1 if the tm struct cannot be represented as
+ * as calendar time (seconds since the Epoch). This might happen
+ * with tm structs that are ill-formated or on 32-bit platforms
+ * with dates that would cause overflow (year 2038 and later).
+ * In such cases, skip the expiration check. */
+
+ if (current_time > expire_time && expire_time > 0) {
+ LOG_FATAL("kerberos principal on %s is expired\n", dn);
+ errMesg = "Kerberos principal is expired.";
+ auth_failed = true;
+ goto done;
+ }
+ }
+ }
+
/* we aren't interested in host principals */
objectclass = slapi_value_new_string("ipaHost");
if ((slapi_entry_attr_has_syntax_value(entry, SLAPI_ATTR_OBJECTCLASS, objectclass)) == 1) {
@@ -1284,10 +1318,16 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
done:
slapi_ch_free_string(&principal);
slapi_ch_free_string(&expire);
+ slapi_ch_free_string(&principal_expire);
if (entry)
slapi_entry_free(entry);
free_ipapwd_krbcfg(&krbcfg);
+ if (auth_failed){
+ slapi_send_ldap_result(pb, LDAP_INVALID_CREDENTIALS, NULL, errMesg,
+ 0, NULL);
+ }
+
return 0;
}
--
1.8.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
