[Freeipa-devel] [PATCH] 0001 ipa_kdb add krbPrincipalAuthInd handling
Matt Rogers
mrogers at redhat.com
Thu Apr 28 18:58:07 UTC 2016
On 04/27, Matt Rogers wrote:
> On 04/27, Sumit Bose wrote:
> > On Tue, Apr 26, 2016 at 02:02:04PM -0400, Matt Rogers wrote:
> > > 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.
> >
> > ok, makes sense.
> >
> > Nevertheless, comments in kdb.h like:
> >
> > /* String attributes (currently stored inside tl-data) map C string keys to
> > * values. They can be set via kadmin and consumed by KDC plugins. */
> >
> > and
> >
> > /* String attributes may not always be represented in tl-data. kadmin clients
> > * must use the get_strings and set_string RPCs. */
> >
> > make me wonder if it is a good idea to operate on the tl-data of type
> > KRB5_TL_STRING_ATTRS directly? I know we do this in other places as well
> > so I'm not insisting to change it, I'm just wondering about the reasons.
> >
> > Would something like (error checks are missing)
> >
> > kerr = krb5_dbe_get_string(kcontext, entry, "require_auth",
> > &require_auth_str);
> >
> > if (require_auth_str != NULL) {
> > kerr = krb5_dbe_set_string(kcontext, entry, "require_auth", NULL);
> > }
> >
> > kerr = ipadb_get_ldap_mod_extra_data(imods,
> > entry->tl_data,
> > mod_op);
> >
> > if (require_auth_str != NULL) {
> > kerr = krb5_dbe_set_string(kcontext, entry, "require_auth",
> > require_auth_str);
> > }
> >
> > have the same effect with only using the recommended API (and making the
> > patch smaller)?
> >
>
> I believe that would be the same effect, just less efficient. But
> sticking to the API is probably safer in the long run. I am okay
> with changing it if you would prefer.
>
Here's the updated patch. Thanks again for the review!
--
Matt Rogers
Red Hat, Inc
-------------- next part --------------
>From c08bb73685b2f7a17a22e5ea066ad9b60b7c4163 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 | 171 +++++++++++++++++++++++++++++++++++
install/share/60kerberos.ldif | 6 +-
2 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index 629f8193223c924267f6d5f39d258cfbc51c7f63..837b4ad37f25aebbe61623e156544cf01d1c0c9e 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);
@@ -1428,6 +1512,7 @@ static krb5_error_code ipadb_get_ldap_mod_extra_data(struct ipadb_mods *imods,
{
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;
@@ -1649,6 +1734,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,
@@ -1657,6 +1798,7 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
krb5_error_code kerr;
krb5_int32 time32le;
int mkvno;
+ char *req_auth_str = NULL;
/* check each mask flag in order */
@@ -1835,6 +1977,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,
@@ -1854,12 +2000,36 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
}
}
+ kerr = krb5_dbe_get_string(kcontext, entry, "require_auth",
+ &req_auth_str);
+ if (kerr) {
+ goto done;
+ }
+
+ /* Do not store auth indicators from the string attribute in
+ * krbExtraData. Remove require_auth value from the entry temporarily. */
+ if (req_auth_str != NULL) {
+ 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);
if (kerr && kerr != ENOENT) {
goto done;
}
+
+ /* Restore require_auth value */
+ if (req_auth_str != NULL) {
+ kerr = krb5_dbe_set_string(kcontext, entry, "require_auth",
+ req_auth_str);
+ if (kerr) {
+ goto done;
+ }
+ }
}
/* KADM5_LOAD */
@@ -1937,6 +2107,7 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
kerr = 0;
done:
+ free(req_auth_str);
return kerr;
}
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