[Freeipa-devel] [PATCH] 0001 ipa_kdb add krbPrincipalAuthInd handling

Matt Rogers mrogers at redhat.com
Tue Apr 26 18:02:04 UTC 2016


On 04/26, Sumit Bose wrote:
> On Thu, Apr 14, 2016 at 12:59:55PM -0400, Matt Rogers wrote:
> > 
> > 
> > ----- Original Message -----
> > > From: "Nathaniel McCallum" <npmccallum at redhat.com>
> > > To: "Matt Rogers" <mrogers at redhat.com>, freeipa-devel at redhat.com
> > > Sent: Thursday, April 14, 2016 10:32:15 AM
> > > Subject: Re: [Freeipa-devel] [PATCH] 0001 ipa_kdb add krbPrincipalAuthInd handling
> > > 
> > > On Mon, 2016-04-11 at 10:41 -0400, Matt Rogers wrote:
> > > > Hi,
> > > > 
> > > > The attached patch is a part of the authentication indicator
> > > > enhancements,
> > > > adding indicator value storage and retrieval for the KDB driver.
> > > > 
> > > > https://fedorahosted.org/freeipa/ticket/5782
> > > 
> > > Can you add some whitespace in next_attr()? The density of the code
> > > there hurts readability.
> > > 
> > Sure, I've attached the revised patch.
> 
> Hi Matt,
> 
> thank you for the patch. Currently I have the following question.
> 
> You call krb5_dbe_set_string to remove the 'require_auth' data before
> calling ipadb_get_ldap_mod_extra_data()
> 

> > +        /* Delete authinds from tl_data so it is not included in krbExtraData. */
> > +        kerr = krb5_dbe_set_string(kcontext, entry, "require_auth", NULL);
> > +        if (kerr) {
> > +            goto done;
> > +        }
> > +
> >          kerr = ipadb_get_ldap_mod_extra_data(imods,
> >                                               entry->tl_data,
> >                                               mod_op);
> > 
> 
> Why it is needed to filter this data again in
> ipadb_get_ldap_mod_extra_data()?
> 

Hi Sumit, thanks for looking. The above krb5_dbe_set_string() call I
believe I left there in error - We decided to operate on a filtered copy
of the tl_data (which filter_authind_str_attrs() handles) rather than
removing it completely with krb5_dbe_set_string(). I had tested with the
above code commented out but forgot to remove it with the submitted patch.

I've attached the updated patch. Thanks for spotting that.

-- 
Matt Rogers
Red Hat, Inc
-------------- next part --------------
>From a75ca3d391554c0254c1b2d206f96edc29f11149 Mon Sep 17 00:00:00 2001
From: Matt Rogers <mrogers at redhat.com>
Date: Fri, 25 Mar 2016 17:01:40 -0400
Subject: [PATCH] ipa_kdb: add krbPrincipalAuthInd handling

Store and retrieve the authentication indicator "require_auth" string in
the krbPrincipalAuthInd attribute. Skip storing auth indicators to
krbExtraData.
---
 daemons/ipa-kdb/ipa_kdb_principals.c | 278 +++++++++++++++++++++++++++++++++++
 install/share/60kerberos.ldif        |   6 +-
 2 files changed, 282 insertions(+), 2 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index 629f8193223c924267f6d5f39d258cfbc51c7f63..fbfb15d179790e30ea69e43f693bd90fdd0ff2ec 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -54,6 +54,7 @@ static char *std_principal_attrs[] = {
     "krbLastSuccessfulAuth",
     "krbLastFailedAuth",
     "krbLoginFailedCount",
+    "krbPrincipalAuthInd",
     "krbExtraData",
     "krbLastAdminUnlock",
     "krbObjectReferences",
@@ -428,6 +429,85 @@ done:
     return kerr;
 }
 
+static void strv_free(char **strv)
+{
+    int i;
+
+    if (strv == NULL) {
+        return;
+    }
+
+    for (i = 0; strv[i] != NULL; i++) {
+        free(strv[i]);
+    }
+
+    free(strv);
+}
+
+static krb5_error_code ipadb_get_ldap_auth_ind(krb5_context kcontext,
+                                               LDAP *lcontext,
+                                               LDAPMessage *lentry,
+                                               krb5_db_entry *entry)
+{
+    krb5_error_code ret = 0;
+    char **authinds = NULL;
+    char *aistr = NULL;
+    char *ap = NULL;
+    size_t len = 0;
+    size_t l = 0;
+    int count = 0;
+    int i = 0;
+
+    ret = ipadb_ldap_attr_to_strlist(lcontext, lentry, "krbPrincipalAuthInd",
+                                     &authinds);
+    switch (ret) {
+    case 0:
+        break;
+    case ENOENT:
+        return 0;
+    default:
+        return ret;
+    }
+
+    for (count = 0; authinds != NULL && authinds[count] != NULL; count++) {
+        len += strlen(authinds[count]) + 1;
+    }
+
+    if (len == 0) {
+        strv_free(authinds);
+        return 0;
+    }
+
+    aistr = malloc(len);
+    if (aistr == NULL) {
+        ret = errno;
+        goto cleanup;
+    }
+
+    /* Create a space-separated string of authinds. */
+    ap = aistr;
+    l = len;
+    for (i = 0; i < count; i++) {
+        ret = snprintf(ap, l, "%s ", authinds[i]);
+        if (ret <= 0 || ret > l) {
+            ret = ENOMEM;
+            goto cleanup;
+        }
+        ap += ret;
+        l -= ret;
+    }
+    aistr[len - 1] = '\0';
+
+    ret = krb5_dbe_set_string(kcontext, entry, "require_auth",
+                              aistr);
+
+cleanup:
+    strv_free(authinds);
+    free(aistr);
+
+    return ret;
+}
+
 static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
                                               char *principal,
                                               LDAPMessage *lentry,
@@ -611,6 +691,10 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
         goto done;
     }
 
+    ret = ipadb_get_ldap_auth_ind(kcontext, lcontext, lentry, entry);
+    if (ret)
+        goto done;
+
     ret = ipadb_ldap_attr_to_key_data(lcontext, lentry,
                                       "krbPrincipalKey",
                                       &res_key_data, &result, &mkvno);
@@ -1422,12 +1506,128 @@ done:
     return kerr;
 }
 
+/* Find the next key and value pair in *pos and update *pos. */
+static bool next_attr(const char **pos, const char *end,
+                      const char **key_out, const char **val_out)
+{
+    const char *key, *key_end, *val, *val_end;
+
+    *key_out = *val_out = NULL;
+
+    if (*pos == end) {
+        return false;
+    }
+
+    key = *pos;
+    key_end = memchr(key, '\0', end - key);
+    if (key_end == NULL) {      /* Malformed representation; give up. */
+        return false;
+    }
+
+    val = key_end + 1;
+    val_end = memchr(val, '\0', end - val);
+    if (val_end == NULL) {      /* Malformed representation; give up. */
+        return false;
+    }
+
+    *key_out = key;
+    *val_out = val;
+    *pos = val_end + 1;
+
+    return true;
+}
+
+static krb5_error_code filter_authind_str_attrs(krb5_tl_data *data,
+                                                krb5_tl_data **data_out)
+{
+    krb5_error_code ret = 0;
+    krb5_tl_data *new_data = NULL;
+    char *new_content = NULL;
+    char *new_cp = NULL;
+    const char *pos = NULL;
+    const char *end = NULL;
+    const char *key = NULL;
+    const char *val = NULL;
+    size_t data_len = 0;
+    size_t new_len = 0;
+
+    *data_out = NULL;
+
+    if (data->tl_data_type != KRB5_TL_STRING_ATTRS) {
+        return 0;
+    }
+
+    pos = (const char *)data->tl_data_contents;
+    data_len = data->tl_data_length;
+    end = pos + data_len;
+
+    new_data = malloc(sizeof(*new_data));
+    if (new_data == NULL) {
+        return errno;
+    }
+    new_content = malloc(data_len);
+    if (new_content == NULL) {
+        ret = errno;
+        goto done;
+    }
+
+    new_cp = new_content;
+    while (next_attr(&pos, end, &key, &val)) {
+        if (strcmp(key, "require_auth") == 0) {
+            continue;
+        }
+
+        ret = snprintf(new_cp, data_len, "%s", key);
+        if (ret <= 0 || ret > data_len) {
+            ret = ENOMEM;
+            goto done;
+        }
+        new_cp += (ret + 1);
+        data_len -= (ret + 1);
+
+        ret = snprintf(new_cp, data_len, "%s", val);
+        if (ret <= 0 || ret > data_len) {
+            ret = ENOMEM;
+            goto done;
+        }
+        new_cp += (ret + 1);
+        data_len -= (ret + 1);
+    }
+
+    if (data_len > 0) {
+        new_len = data->tl_data_length - data_len;
+    } else {
+        new_len = data->tl_data_length;
+    }
+
+    /* "require_auth" was the only entry, return empty contents */
+    if (new_len == 0) {
+        free(new_content);
+        new_content = NULL;
+    }
+
+    new_data->tl_data_type = data->tl_data_type;
+    new_data->tl_data_next = data->tl_data_next;
+    new_data->tl_data_contents = new_content;
+    new_data->tl_data_length = new_len;
+    *data_out = new_data;
+    ret = 0;
+
+done:
+    if (ret) {
+        free(new_content);
+        free(new_data);
+    }
+    return ret;
+}
+
 static krb5_error_code ipadb_get_ldap_mod_extra_data(struct ipadb_mods *imods,
                                                      krb5_tl_data *tl_data,
                                                      int mod_op)
 {
     krb5_error_code kerr;
     krb5_tl_data *data;
+    krb5_tl_data *data_tmp = NULL;
     struct berval **bvs = NULL;
     krb5_int16 be_type;
     int n, i;
@@ -1463,6 +1663,20 @@ static krb5_error_code ipadb_get_ldap_mod_extra_data(struct ipadb_mods *imods,
             continue;
         }
 
+        /* Exclude any auth indicators from krbExtraData */
+        kerr = filter_authind_str_attrs(data, &data_tmp);
+        if (kerr) {
+            goto done;
+        }
+        if (data_tmp != NULL) {
+            if (data_tmp->tl_data_contents == NULL) {
+                free(data_tmp);
+                data_tmp = NULL;
+                continue;
+            }
+            data = data_tmp;
+        }
+
         be_type = htons(data->tl_data_type);
 
         bvs[i] = calloc(1, sizeof(struct berval));
@@ -1497,6 +1711,10 @@ done:
             free(bvs[i]);
         }
     }
+    if (data_tmp != NULL) {
+        free(data_tmp->tl_data_contents);
+        free(data_tmp);
+    }
     free(bvs);
     return kerr;
 }
@@ -1649,6 +1867,62 @@ done:
     return kerr;
 }
 
+static krb5_error_code ipadb_get_ldap_mod_auth_ind(krb5_context kcontext,
+                                                   struct ipadb_mods *imods,
+                                                   krb5_db_entry *entry,
+                                                   int mod_op)
+{
+    krb5_error_code ret = 0;
+    char **strlist = NULL;
+    char *ais = NULL;
+    char *ai = NULL;
+    char *s = NULL;
+    size_t ai_size = 0;
+    int cnt = 0;
+    int i = 0;
+
+    ret = krb5_dbe_get_string(kcontext, entry, "require_auth", &ais);
+    if (ret) {
+        return ret;
+    }
+    if (ais == NULL) {
+        return 0;
+    }
+
+    ai_size = strlen(ais) + 1;
+
+    for (i = 0; i < ai_size; i++) {
+        if (ais[i] != ' ') {
+            continue;
+        }
+        if (i > 0 && ais[i - 1] != ' ') {
+            cnt++;
+        }
+    }
+
+    strlist = calloc(cnt + 2, sizeof(*strlist));
+    if (strlist == NULL) {
+        free(ais);
+        return errno;
+    }
+
+    cnt = 0;
+    ai = strtok_r(ais, " ", &s);
+    while (ai != NULL) {
+        if (ai[0] != '\0') {
+            strlist[cnt++] = ai;
+        }
+        ai = strtok_r(NULL, " ", &s);
+    }
+
+    ret = ipadb_get_ldap_mod_str_list(imods, "krbPrincipalAuthInd",
+                                      strlist, cnt, mod_op);
+
+    free(ais);
+    free(strlist);
+    return ret;
+}
+
 static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
                                            struct ipadb_mods *imods,
                                            krb5_db_entry *entry,
@@ -1835,6 +2109,10 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
         }
     }
 
+    kerr = ipadb_get_ldap_mod_auth_ind(kcontext, imods, entry, mod_op);
+    if (kerr)
+        goto done;
+
     /* KADM5_TL_DATA */
     if (entry->mask & KMASK_TL_DATA) {
         kerr = ipadb_get_tl_data(entry,
diff --git a/install/share/60kerberos.ldif b/install/share/60kerberos.ldif
index 8698e3a0538fd42443f4a3221fccfb739d4c104d..45bea2aefd3993a521e8ea732f902279622f8cb8 100644
--- a/install/share/60kerberos.ldif
+++ b/install/share/60kerberos.ldif
@@ -266,7 +266,9 @@ attributetypes: ( 2.16.840.1.113719.1.301.4.53.1 NAME 'krbPrincContainerRef' EQU
 attributetypes: ( 1.3.6.1.4.1.5322.21.2.5 NAME 'krbLastAdminUnlock' EQUALITY generalizedTimeMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.24 SINGLE-VALUE)
 ##### A list of services to which a service principal can delegate.
 attributetypes: ( 1.3.6.1.4.1.5322.21.2.4 NAME 'krbAllowedToDelegateTo' EQUALITY caseExactIA5Match SUBSTR caseExactSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26)
-########################################################################
+##### A list of authentication indicator strings, one of which must be satisfied
+##### to authenticate to the principal as a service.
+attributetypes: ( 2.16.840.1.113730.3.8.15.2.1 NAME 'krbPrincipalAuthInd' EQUALITY caseExactMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15)
 ########################################################################
 # 		        Object Class Definitions                       #
 ########################################################################
@@ -294,7 +296,7 @@ objectClasses: ( 2.16.840.1.113719.1.301.6.4.1 NAME 'krbKdcService' SUP ( krbSer
 objectClasses: ( 2.16.840.1.113719.1.301.6.5.1 NAME 'krbPwdService' SUP ( krbService ) )
 ###### The principal data auxiliary class. Holds principal information
 ###### and is used to store principal information for Person, Service objects.
-objectClasses: ( 2.16.840.1.113719.1.301.6.8.1 NAME 'krbPrincipalAux' AUXILIARY MAY ( krbPrincipalName $ krbCanonicalName $ krbUPEnabled $ krbPrincipalKey $ krbTicketPolicyReference $ krbPrincipalExpiration $ krbPasswordExpiration $ krbPwdPolicyReference $ krbPrincipalType $ krbPwdHistory $ krbLastPwdChange $ krbPrincipalAliases $ krbLastSuccessfulAuth $ krbLastFailedAuth $ krbLoginFailedCount $ krbExtraData $ krbLastAdminUnlock $ krbAllowedToDelegateTo ) )
+objectClasses: ( 2.16.840.1.113719.1.301.6.8.1 NAME 'krbPrincipalAux' AUXILIARY MAY ( krbPrincipalName $ krbCanonicalName $ krbUPEnabled $ krbPrincipalKey $ krbTicketPolicyReference $ krbPrincipalExpiration $ krbPasswordExpiration $ krbPwdPolicyReference $ krbPrincipalType $ krbPwdHistory $ krbLastPwdChange $ krbPrincipalAliases $ krbLastSuccessfulAuth $ krbLastFailedAuth $ krbLoginFailedCount $ krbExtraData $ krbLastAdminUnlock $ krbAllowedToDelegateTo $ krbPrincipalAuthInd ) )
 ###### This class is used to create additional principals and stand alone principals.
 objectClasses: ( 2.16.840.1.113719.1.301.6.9.1 NAME 'krbPrincipal' SUP ( top ) MUST ( krbPrincipalName ) MAY ( krbObjectReferences ) )
 ###### The principal references auxiliary class. Holds all principals referred
-- 
2.4.3



More information about the Freeipa-devel mailing list