[Freeipa-devel] Handling of multiple krbPrincipalNames and of krbCanonicalNames

[Nalin Dahyabhai]

Comparing master's ipa-kdb's handling of krbPrincipalName and
krbCanonicalName attributes with that of the upstream kldap driver,
there are a few differences which I'm thinking are bugs.

* If an entry has multiple krbPrincipalName values, the name which
  was used to look it up is required to match only the last value of the
  attribute that we read, not any of them.

* If an entry has a krbCanonicalName value, and the name which we used
  to look it up doesn't match it, if database aliases are allowed, we
  return an error instead of using it to populate the returned entry.

I'm attaching patches for both of these, though the second still doesn't
quite match the behavior of kldap.so, in that we don't preserve the
requested name if it differs from the canonical name only in case.  I
don't know that it matters, but I'm mentioning here just in case.

Cheers,

Nalin
-------------- next part --------------
>From d4330cd204757bdbbcb50164d03fedf864d6b736 Mon Sep 17 00:00:00 2001
From: Nalin Dahyabhai <nalin at dahyabhai.net>
Date: Mon, 7 Oct 2013 15:24:29 -0400
Subject: [PATCH 1/4] Accept any alias, not just the last value

If the entry's krbPrincipalName attribute is multi-valued, accept any of
the values, not just the last one we happen to examine.
---
 daemons/ipa-kdb/ipa_kdb_principals.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index 38059d2..d3b2820 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -653,6 +653,9 @@ static krb5_error_code ipadb_find_principal(krb5_context kcontext,
             } else {
                 found = (strcmp(vals[i]->bv_val, (*principal)) == 0);
             }
+            if (found) {
+                break;
+            }
         }
 
         ldap_value_free_len(vals);
-- 
1.8.3.1

-------------- next part --------------
>From 59c38ecfe3786c72b7fea9aeba2118f1d07f3235 Mon Sep 17 00:00:00 2001
From: Nalin Dahyabhai <nalin at dahyabhai.net>
Date: Mon, 7 Oct 2013 15:26:21 -0400
Subject: [PATCH 2/4] Restore krbCanonicalName handling

When an entry has a krbCanonicalName, if KRB5_KDB_FLAG_ALIAS_OK is set,
rewrite the principal name to the canonical value, else error out,
instead of always returning an error if the requested name doesn't look
like the canonical one.
---
 daemons/ipa-kdb/ipa_kdb_principals.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index d3b2820..766aa92 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -672,11 +672,7 @@ static krb5_error_code ipadb_find_principal(krb5_context kcontext,
 
         /* Again, if aliases are accepted by KDC, use case-insensitive comparison */
         if ((flags & KRB5_KDB_FLAG_ALIAS_OK) != 0) {
-            if (ulc_casecmp(vals[0]->bv_val, vals[0]->bv_len,
-                            (*principal), strlen(*principal),
-                            NULL, NULL, &result) != 0)
-                return KRB5_KDB_INTERNAL_ERROR;
-            found = (result == 0);
+            found = true;
         } else {
             found = (strcmp(vals[0]->bv_val, (*principal)) == 0);
         }
-- 
1.8.3.1