[Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type

Sumit Bose sbose at redhat.com
Wed Feb 27 10:58:31 UTC 2013


On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote:
> On 02/21/2013 04:24 PM, Sumit Bose wrote:
> > Hi,
> > 
> > this series of patches fix https://fedorahosted.org/freeipa/ticket/2960
> > The related design page is
> > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type .
> > 
> > It was agreed while discussing the design page that the currently
> > hardcoded value for the NFS service will be dropped completely, hence
> > the first patch reverts to patch which added the hardcoded value.
> > 
> > The second patch adds the needed attribute to compensate the now missing
> > hardcoded value.
> > 
> > In the following three patches the PAC type is read and used
> > accordingly. The last patch adds a unit test for a new function.
> > 
> > bye,
> > Sumit
> 
> I did only sanity testing to the C part of the RFE so far, but by reading it it
> looks ok. It may be beneficial if Simo or Alexander checked if the ipa-kdb part
> is OK.

Alexander asked in irc to change strcmp() to strcasecmp() so that
options like 'Ms-Pac' or 'none' are accepted as well.

> 
> I will comment on the Python parts:
> 
> 1) Your update check testing if there is any NFS service with ipakrbauthzdata
> looks like a good heuristics to prevent admin frustration. Though an ideal
> solution would require
> https://fedorahosted.org/freeipa/ticket/2961
> to prevent multiple updates when admin purposefully removes this attribute. We
> may want to reference the ticket in a comment in the update plugin...

I added a comment in the code and in #2961.

> 
> 
> 2) The upgrade plugin may get into infinite loop if ldap.find_entries returns
> truncated result. As you do not update entries directly but only return update
> instructions when you have no truncated results, you will loop.
> 
> To simulate this, I just created 2 NFS principals and set size_limit=1 in your
> find_entries call.

Thanks for catching this, I removed the truncated logic and added a
messages if truncated was set.

> 
> 
> 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS
> service principals added by service-add? Shouldn't we set ipakrbauthzdata for
> new services too? As a default when no user ipakrhbauthzdata is set...
> Otherwise admins could be surprised why their new NFS services do not work
> while the others do.
> 
> Also, I think we should have this NFS culprit documented somewhere (i.e. why we
> set ipakrbauthzdata to NONE by default). At least as a small section in the
> online help for services plugin...

I added comments to the help and the doc string in patch #100. If you
think it is necessary to implicitly set PAC type to 'NONE' if NFS
services are added and no type is given explicitly, I would prefer if
you can open a new tickets so that it can be discussed independently.

Thank you for the review. New versions attached.

bye,
Sumit

> 
> Martin
-------------- next part --------------
From 8eb34dd3190853bd3fa400434be4f7c720bf1354 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 12 Feb 2013 09:59:00 +0100
Subject: [PATCH 094/100] Revert "MS-PAC: Special case NFS services"

This reverts commit 5269458f552380759c86018cd1f30b64761be92e.

With the implementation of https://fedorahosted.org/freeipa/ticket/2960
a special hardcoded handling of NFS service tickets is not needed
anymore.
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 36 +-----------------------------------
 1 file changed, 1 insertion(+), 35 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 950000349702038e1d55bd257816f7f61e9557a5..f05c552fc18eb9b15ee3e39b513184589bc8c468 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -743,24 +743,6 @@ static bool is_cross_realm_krbtgt(krb5_const_principal princ)
     return true;
 }
 
-static bool is_service_of_type(krb5_const_principal princ, const char *type)
-{
-    size_t len;
-
-    if (princ->length < 2) {
-        return false;
-    }
-
-    len = strlen(type);
-
-    if ((princ->data[0].length == len) ||
-        (strncasecmp(princ->data[0].data, type, len) == 0)) {
-        return true;
-    }
-
-    return false;
-}
-
 static char *gen_sid_string(TALLOC_CTX *memctx, struct dom_sid *dom_sid,
                             uint32_t rid)
 {
@@ -1555,7 +1537,6 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
     krb5_error_code kerr;
     krb5_pac pac = NULL;
     krb5_data pac_data;
-    bool is_nfs = false;
 
     /* When using s4u2proxy client_princ actually refers to the proxied user
      * while client->princ to the proxy service asking for the TGS on behalf
@@ -1566,32 +1547,17 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
         ks_client_princ = client->princ;
     }
 
-    /* NFS Server on Linux is limited and will choke on big tickets.
-     * So avoid attachnig the PAC to nfs/ tickets for now.
-     * FIXME: remove this when we have interface to support disabling
-     * PACs on arbitrary services */
-    if (is_service_of_type(ks_client_princ, "nfs") ||
-        is_service_of_type(server->princ, "nfs")) {
-        is_nfs = true;
-    }
-
     is_as_req = ((flags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) != 0);
 
     if (is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) {
 
-        if (is_nfs) {
-            *signed_auth_data = NULL;
-            kerr = 0;
-            goto done;
-        }
-
         kerr = ipadb_get_pac(context, client, &pac);
         if (kerr != 0 && kerr != ENOENT) {
             goto done;
         }
     }
 
-    if (!is_as_req & !is_nfs) {
+    if (!is_as_req) {
         /* find the existing PAC, if present */
         kerr = krb5_find_authdata(context, tgt_auth_data, NULL,
                                   KRB5_AUTHDATA_WIN2K_PAC, &pac_auth_data);
-- 
1.8.1.2

-------------- next part --------------
From 6eaa83868c1c80c8eeac5029176cb6d1b3ce03a6 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 21 Feb 2013 14:27:12 +0100
Subject: [PATCH 095/100] Add ipakrbauthzdata to NFS services during upgrades

This update became necessary because instead of hardcoding a value of
'NONE' for ipakrbauthzdata for all NFS services ipakrbauthzdata can now
be specified per service. To avoid issues during upgrades
ipakrbauthzdata must be set to NONE for all NFS services where this
attribute isn't set, because an unset attribute means using the default
which might be different from 'NONE'.

Another issue might arise on systems where this update is already run
and for some NFS services the ipakrbauthzdata attribute is removed
manually on purpose. Another update might set it back to 'NONE' which
might be irritating. For this reason this plugin only update entries if
none of the existing NFS service entries has ipakrbauthzdata set.
---
 ipaserver/install/plugins/update_services.py | 82 +++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/plugins/update_services.py b/ipaserver/install/plugins/update_services.py
index c384af52fb91692c7f339c5fb2da92ae45984482..22c847767983945354bee5dfa4f246c4ef720e4b 100644
--- a/ipaserver/install/plugins/update_services.py
+++ b/ipaserver/install/plugins/update_services.py
@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from ipaserver.install.plugins import MIDDLE
+from ipaserver.install.plugins import MIDDLE, LAST
 from ipaserver.install.plugins.baseupdate import PostUpdate
 from ipalib import api, errors
 from ipapython.dn import DN
@@ -93,3 +93,83 @@ class update_service_principalalias(PostUpdate):
         return (False, False, [])
 
 api.register(update_service_principalalias)
+
+class update_service_nfs_ipakrbauthzdata(PostUpdate):
+    """
+    Update all NFS services which do not have ipakrbauthzdata attribute set.
+
+    Since this plugin uses ipakrbprincipalalias update_service_principalalias
+    must be run before, 'order' is set accordingly.
+
+    This update became necessary because instead of hardcoding a value of
+    'NONE' for ipakrbauthzdata for all NFS services ipakrbauthzdata can now be
+    specified per service. To avoid issues during upgrades ipakrbauthzdata
+    must be set to NONE for all NFS services where this attribute isn't set,
+    because an unset attribute means using the default which might be
+    different from 'NONE'.
+
+    Another issue might arise on systems where this update is already run and
+    for some NFS services the ipakrbauthzdata attribute is removed manually on
+    purpose. Another update might it set back to 'NONE' which might be
+    irritating. For this reason this plugin only update entries if none of the
+    existing NFS service entries has ipakrbauthzdata set. If a solution for
+    https://fedorahosted.org/freeipa/ticket/2961 is implemented this heuristic
+    should be dropped.
+    """
+    order = LAST
+
+    def execute(self, **options):
+        ldap = self.obj.backend
+
+        base_dn = DN(api.env.container_service, api.env.basedn)
+        search_filter = ("(&(objectclass=ipakrbprincipal)"
+                           "(objectclass=ipaservice)"
+                           "(ipakrbprincipalalias=nfs*))")
+        root_logger.debug("update_service_nfs_ipakrbauthzdata: search for "
+                          "affected services")
+
+        try:
+            (entries, truncated) = ldap.find_entries(search_filter,
+                ['objectclass', 'ipakrbauthzdata'], base_dn,
+                time_limit=0, size_limit=0)
+        except errors.NotFound:
+            root_logger.debug("update_service_nfs_ipakrbauthzdata: "
+                              "no NFS service to update found")
+            return (False, False, [])
+        except errors.ExecutionError, err:
+            root_logger.error("update_service_nfs_ipakrbauthzdata: cannot "
+                              "retrieve list of NFS services: %s", err)
+            return (False, False, [])
+        if not entries:
+            root_logger.debug("update_service_nfs_ipakrbauthzdata: "
+                              "no NFS service was returned")
+            return (False, False, [])
+        root_logger.debug("update_service_nfs_ipakrbauthzdata: found %d "
+                          "NFS services, truncated: %s",
+                          len(entries), truncated)
+
+        for nfs_dn, entry in entries:
+            if entry.has_key('ipakrbauthzdata'):
+                root_logger.info("update_service_nfs_ipakrbauthzdata: "
+                                 "found a NFS service where "
+                                 "ipakrbauthzdata is already set, "
+                                 "nothing to do")
+                return (False, False, [])
+
+        nfs_updates = {}
+        for nfs_dn, entry in entries:
+            nfs_updates[nfs_dn] = {'dn': nfs_dn,
+                                   'updates': ['add:ipakrbauthzdata:NONE']}
+
+        if not truncated:
+            root_logger.debug("update_service_nfs_ipakrbauthzdata: "
+                              "all affected services updated")
+        else:
+            root_logger.info("search results for NFS services have been "
+                             "truncated by the server; please check manually "
+                              "if the PAC type is set correctly for all NFS "
+                              "services")
+
+        return (False, True, [nfs_updates])
+
+api.register(update_service_nfs_ipakrbauthzdata)
-- 
1.8.1.2

-------------- next part --------------
From 0b154a1f4929062ddec79ace9ca116ade54879d1 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 12 Feb 2013 11:01:11 +0100
Subject: [PATCH 096/100] ipa-kdb: Read global defaul ipaKrbAuthzData

The ipaKrbAuthzData LDAP attribute is read from the ipaConfig object
and the read value(s) are stored in the ipadb context.
---
 daemons/ipa-kdb/ipa_kdb.c | 27 ++++++++++++++++++++++++++-
 daemons/ipa-kdb/ipa_kdb.h |  3 +++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index 2a344dc69158dbc3f6d0207ab0bb781676240ed9..e5c718ea9ffd38dbd49026e825d4a7f920638181 100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -40,6 +40,8 @@ struct ipadb_context *ipadb_get_context(krb5_context kcontext)
 static void ipadb_context_free(krb5_context kcontext,
                                struct ipadb_context **ctx)
 {
+    size_t c;
+
     if (*ctx != NULL) {
         free((*ctx)->uri);
         free((*ctx)->base);
@@ -51,6 +53,12 @@ static void ipadb_context_free(krb5_context kcontext,
         free((*ctx)->supp_encs);
         ipadb_mspac_struct_free(&(*ctx)->mspac);
         krb5_free_default_realm(kcontext, (*ctx)->realm);
+
+        for (c = 0; (*ctx)->authz_data && (*ctx)->authz_data[c]; c++) {
+            free((*ctx)->authz_data[c]);
+        }
+        free((*ctx)->authz_data);
+
         free(*ctx);
         *ctx = NULL;
     }
@@ -167,13 +175,14 @@ done:
 
 int ipadb_get_global_configs(struct ipadb_context *ipactx)
 {
-    char *attrs[] = { "ipaConfigString", NULL };
+    char *attrs[] = { "ipaConfigString", IPA_KRB_AUTHZ_DATA_ATTR, NULL };
     struct berval **vals = NULL;
     LDAPMessage *res = NULL;
     LDAPMessage *first;
     char *base = NULL;
     int i;
     int ret;
+    char **authz_data_list;
 
     ret = asprintf(&base, "cn=ipaConfig,cn=etc,%s", ipactx->base);
     if (ret == -1) {
@@ -215,6 +224,22 @@ int ipadb_get_global_configs(struct ipadb_context *ipactx)
         }
     }
 
+    ret = ipadb_ldap_attr_to_strlist(ipactx->lcontext, first,
+                                     IPA_KRB_AUTHZ_DATA_ATTR, &authz_data_list);
+    if (ret != 0 && ret != ENOENT) {
+        goto done;
+    }
+    if (ret == 0) {
+        if (ipactx->authz_data != NULL) {
+            for (i = 0; ipactx->authz_data[i]; i++) {
+                free(ipactx->authz_data[i]);
+            }
+            free(ipactx->authz_data);
+        }
+
+        ipactx->authz_data = authz_data_list;
+    }
+
     ret = 0;
 
 done:
diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index f472f02458e040b921b9f3f85bcc36fa954785d5..7b1576124140ae53cf28a1ed47bfa1acf31cdd59 100644
--- a/daemons/ipa-kdb/ipa_kdb.h
+++ b/daemons/ipa-kdb/ipa_kdb.h
@@ -74,6 +74,8 @@
 
 #define IPA_SETUP "ipa-setup-override-restrictions"
 
+#define IPA_KRB_AUTHZ_DATA_ATTR "ipaKrbAuthzData"
+
 struct ipadb_mspac;
 
 struct ipadb_context {
@@ -89,6 +91,7 @@ struct ipadb_context {
     struct ipadb_mspac *mspac;
     bool disable_last_success;
     bool disable_lockout;
+    char **authz_data;
 };
 
 #define IPA_E_DATA_MAGIC 0x0eda7a
-- 
1.8.1.2

-------------- next part --------------
From 23d5892f70df57581157ba115016b6debfb0774f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 12 Feb 2013 09:44:32 +0100
Subject: [PATCH 097/100] ipa-kdb: Read ipaKrbAuthzData with other principal
 data

The ipaKrbAuthzData LDAP attribute is read together with the other data
of the requestedprincipal and the read value(s) are stored in the e-data
of the entry for later use.
---
 daemons/ipa-kdb/ipa_kdb.h            |  1 +
 daemons/ipa-kdb/ipa_kdb_principals.c | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index 7b1576124140ae53cf28a1ed47bfa1acf31cdd59..9daaab80dc514dd1bb3e85775c1e284d19dac0cd 100644
--- a/daemons/ipa-kdb/ipa_kdb.h
+++ b/daemons/ipa-kdb/ipa_kdb.h
@@ -105,6 +105,7 @@ struct ipadb_e_data {
     char **pw_history;
     struct ipapwd_policy *pol;
     time_t last_admin_unlock;
+    char **authz_data;
 };
 
 struct ipadb_context *ipadb_get_context(krb5_context kcontext);
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index bb2074bf9c85b0bdcf228696cf8391c11d2384c4..46a921cb2f267b977df4a0042d81a49beccb5a91 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -63,6 +63,7 @@ static char *std_principal_attrs[] = {
     /* IPA SPECIFIC ATTRIBUTES */
     "nsaccountlock",
     "passwordHistory",
+    IPA_KRB_AUTHZ_DATA_ATTR,
 
     "objectClass",
     NULL
@@ -237,6 +238,7 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
     krb5_kvno mkvno = 0;
     char **restrlist;
     char *restring;
+    char **authz_data_list;
     krb5_timestamp restime;
     bool resbool;
     int result;
@@ -503,6 +505,17 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
         ied->last_admin_unlock = restime;
     }
 
+    ret = ipadb_ldap_attr_to_strlist(lcontext, lentry,
+                                     IPA_KRB_AUTHZ_DATA_ATTR, &authz_data_list);
+    if (ret != 0 && ret != ENOENT) {
+        kerr = KRB5_KDB_INTERNAL_ERROR;
+        goto done;
+    }
+    if (ret == 0) {
+        ied->authz_data = authz_data_list;
+    }
+
+
     kerr = 0;
 
 done:
@@ -831,6 +844,10 @@ void ipadb_free_principal(krb5_context kcontext, krb5_db_entry *entry)
                     free(ied->pw_history[i]);
                 }
                 free(ied->pw_history);
+                for (i = 0; ied->authz_data && ied->authz_data[i]; i++) {
+                    free(ied->authz_data[i]);
+                }
+                free(ied->authz_data);
                 free(ied->pol);
                 free(ied);
             }
-- 
1.8.1.2

-------------- next part --------------
From 157093b95b65e6e1bd1e906c4de12b6ff4e57d21 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 12 Feb 2013 14:02:27 +0100
Subject: [PATCH 098/100] ipa-kdb: add PAC only if requested

Instead of always adding a PAC to the Kerberos ticket the global default
for the authorization data and the authorization data of the service
entry is evaluated and the PAC is added accordingly.
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 86 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index f05c552fc18eb9b15ee3e39b513184589bc8c468..c72e762ca4c680def9ed97e492f6edfcc5e0677f 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -96,6 +96,10 @@ static char *memberof_pac_attrs[] = {
 #define SID_SUB_AUTHS 15
 #define MAX(a,b) (((a)>(b))?(a):(b))
 
+#define AUTHZ_DATA_TYPE_PAC "MS-PAC"
+#define AUTHZ_DATA_TYPE_PAD "PAD"
+#define AUTHZ_DATA_TYPE_NONE "NONE"
+
 static int string_to_sid(char *str, struct dom_sid *sid)
 {
     unsigned long val;
@@ -1515,6 +1519,71 @@ done:
     return kerr;
 }
 
+void get_authz_data_types(krb5_context context, krb5_db_entry *entry,
+                          bool *_with_pac, bool *_with_pad)
+{
+    struct ipadb_e_data *ied = NULL;
+    struct ipadb_context *ipactx;
+    size_t c;
+    bool none_found = false;
+    char **authz_data_list;
+    bool with_pac = false;
+    bool with_pad = false;
+
+    if (entry != NULL) {
+        ied = (struct ipadb_e_data *) entry->e_data;
+    }
+
+    if (ied == NULL || ied->authz_data == NULL) {
+        if (context == NULL) {
+            krb5_klog_syslog(LOG_ERR, "Missing Kerberos context, no " \
+                                      "authorization data will be added.");
+            goto done;
+        }
+
+        ipactx = ipadb_get_context(context);
+        if (ipactx == NULL || ipactx->authz_data == NULL) {
+            krb5_klog_syslog(LOG_ERR, "No default authorization data types " \
+                                      "available, no authorization data will " \
+                                      "be added.");
+            goto done;
+        }
+
+        authz_data_list = ipactx->authz_data;
+    } else {
+        authz_data_list = ied->authz_data;
+    }
+
+
+    for (c = 0; authz_data_list[c]; c++) {
+        if (strcasecmp(authz_data_list[c], AUTHZ_DATA_TYPE_PAC) == 0) {
+            with_pac = true;
+        } else if (strcasecmp(authz_data_list[c], AUTHZ_DATA_TYPE_PAD) == 0) {
+            with_pad = true;
+        } else if (strcasecmp(authz_data_list[c], AUTHZ_DATA_TYPE_NONE) == 0) {
+            none_found = true;
+        } else {
+            krb5_klog_syslog(LOG_ERR, "Ignoring unsupported " \
+                                      "authorization data type [%s].",
+                                      authz_data_list[c]);
+        }
+    }
+
+done:
+    if (none_found) {
+        with_pac = false;
+        with_pad = false;
+    }
+
+    if (_with_pac != NULL) {
+        *_with_pac = with_pac;
+    }
+    if (_with_pad != NULL) {
+        *_with_pad = with_pad;
+    }
+
+}
+
 krb5_error_code ipadb_sign_authdata(krb5_context context,
                                     unsigned int flags,
                                     krb5_const_principal client_princ,
@@ -1537,6 +1606,8 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
     krb5_error_code kerr;
     krb5_pac pac = NULL;
     krb5_data pac_data;
+    bool with_pac;
+    bool with_pad;
 
     /* When using s4u2proxy client_princ actually refers to the proxied user
      * while client->princ to the proxy service asking for the TGS on behalf
@@ -1547,9 +1618,20 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
         ks_client_princ = client->princ;
     }
 
+    /* We only need to check the server entry here, because even if the client
+     * is a service with a valid authorization data it will result to NONE
+     * because ipadb_get_pac() can only generate a pac for 'real' IPA users.
+     * (I assume this will be the same for PAD.) */
+    get_authz_data_types(context, server, &with_pac, &with_pad);
+
+    if (with_pad) {
+        krb5_klog_syslog(LOG_ERR, "PAD authorization data is requested but " \
+                                  "currently not supported.");
+    }
+
     is_as_req = ((flags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) != 0);
 
-    if (is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) {
+    if (is_as_req && with_pac && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) {
 
         kerr = ipadb_get_pac(context, client, &pac);
         if (kerr != 0 && kerr != ENOENT) {
@@ -1557,7 +1639,7 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
         }
     }
 
-    if (!is_as_req) {
+    if (!is_as_req && with_pac) {
         /* find the existing PAC, if present */
         kerr = krb5_find_authdata(context, tgt_auth_data, NULL,
                                   KRB5_AUTHDATA_WIN2K_PAC, &pac_auth_data);
-- 
1.8.1.2

-------------- next part --------------
From e263d27ab1363bd55ba7122daee95bf9cd32b4aa Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 19 Feb 2013 12:16:37 +0100
Subject: [PATCH 099/100] Add unit test for get_authz_data_types()

---
 daemons/ipa-kdb/Makefile.am           |  29 +++++
 daemons/ipa-kdb/tests/ipa_kdb_tests.c | 197 ++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+)
 create mode 100644 daemons/ipa-kdb/tests/ipa_kdb_tests.c

diff --git a/daemons/ipa-kdb/Makefile.am b/daemons/ipa-kdb/Makefile.am
index 5f4e6e2a6a940486a0c904f737f28c476df98773..23ba1cc05ec157a0f4d9b594350ebaf10b2098dc 100644
--- a/daemons/ipa-kdb/Makefile.am
+++ b/daemons/ipa-kdb/Makefile.am
@@ -52,6 +52,35 @@ ipadb_la_LIBADD = 		\
 	$(NDRPAC_LIBS)		\
 	$(NULL)
 
+if HAVE_CHECK
+TESTS = ipa_kdb_tests
+check_PROGRAMS = ipa_kdb_tests
+endif
+
+ipa_kdb_tests_SOURCES =        \
+       tests/ipa_kdb_tests.c   \
+       ipa_kdb.c               \
+       ipa_kdb_common.c        \
+       ipa_kdb_mkey.c          \
+       ipa_kdb_passwords.c     \
+       ipa_kdb_principals.c    \
+       ipa_kdb_pwdpolicy.c     \
+       ipa_kdb_mspac.c         \
+       ipa_kdb_delegation.c    \
+       ipa_kdb_audit_as.c      \
+       $(KRB5_UTIL_SRCS)       \
+       $(NULL)
+ipa_kdb_tests_CFLAGS = $(CHECK_CFLAGS)
+ipa_kdb_tests_LDADD =          \
+       $(CHECK_LIBS)           \
+       $(KRB5_LIBS)            \
+       $(LDAP_LIBS)            \
+       $(NDRPAC_LIBS)          \
+       -lnss3                  \
+       -lkdb5                  \
+       -lsss_idmap             \
+       $(NULL)
+
 dist_noinst_DATA = ipa_kdb.exports
 
 EXTRA_DIST =			\
diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
new file mode 100644
index 0000000000000000000000000000000000000000..77c5ed896bc1b94d9757007d5f0ee816ba5c6675
--- /dev/null
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -0,0 +1,197 @@
+/** BEGIN COPYRIGHT BLOCK
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Additional permission under GPLv3 section 7:
+ *
+ * In the following paragraph, "GPL" means the GNU General Public
+ * License, version 3 or any later version, and "Non-GPL Code" means
+ * code that is governed neither by the GPL nor a license
+ * compatible with the GPL.
+ *
+ * You may link the code of this Program with Non-GPL Code and convey
+ * linked combinations including the two, provided that such Non-GPL
+ * Code only links to the code of this Program through those well
+ * defined interfaces identified in the file named EXCEPTION found in
+ * the source code files (the "Approved Interfaces"). The files of
+ * Non-GPL Code may instantiate templates or use macros or inline
+ * functions from the Approved Interfaces without causing the resulting
+ * work to be covered by the GPL. Only the copyright holders of this
+ * Program may make changes or additions to the list of Approved
+ * Interfaces.
+ *
+ * Authors:
+ * Sumit Bose <sbose at redhat.com>
+ *
+ * Copyright (C) 2013 Red Hat, Inc.
+ * All rights reserved.
+ * END COPYRIGHT BLOCK **/
+
+#include <check.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <krb5/krb5.h>
+#include <kdb.h>
+
+#include "ipa-kdb/ipa_kdb.h"
+
+int krb5_klog_syslog(int l, const char *format, ...)
+{
+    va_list ap;
+    char *s = NULL;
+    int ret;
+
+    va_start(ap, format);
+
+    ret = vasprintf(&s, format, ap);
+    va_end(ap);
+    if (ret < 0) {
+        /* ENOMEM */
+        return -1;
+    }
+
+    fprintf(stderr, "%s\n", s);
+    free(s);
+
+    return 0;
+}
+
+extern void get_authz_data_types(krb5_context context, krb5_db_entry *entry,
+                                 bool *with_pac, bool *with_pad);
+
+START_TEST(test_get_authz_data_types)
+{
+    bool with_pac;
+    bool with_pad;
+    krb5_db_entry *entry;
+    struct ipadb_e_data *ied;
+    size_t c;
+    char *ad_none_only[] = {"NONE", NULL};
+    char *ad_pad_only[] = {"PAD", NULL};
+    char *ad_pac_only[] = {"MS-PAC", NULL};
+    char *ad_illegal_only[] = {"abc", NULL};
+    char *ad_pac_and_pad[] = {"MS-PAC", "PAD", NULL};
+    char *ad_pac_and_none[] = {"MS-PAC", "NONE", NULL};
+    char *ad_none_and_pad[] = {"NONE", "PAD", NULL};
+    char *ad_pac_and_pad_case[] = {"Ms-Pac", "PaD", NULL};
+    krb5_context krb5_ctx;
+    krb5_error_code kerr;
+    struct ipadb_context *ipa_ctx;
+
+    struct test_set {
+        char **authz_data;
+        char **global_authz_data;
+        bool exp_with_pac;
+        bool exp_with_pad;
+        const char *err_msg;
+    } test_set[] = {
+        {ad_none_only, NULL, false, false, "with only NONE in entry"},
+        {ad_pac_only, NULL, true, false, "with only MS-PAC in entry"},
+        {ad_pad_only, NULL, false, true, "with only PAD in entry"},
+        {ad_illegal_only, NULL, false, false, "with only an invalid value in entry"},
+        {ad_pac_and_pad, NULL, true, true, "with MS-PAC and PAD in entry"},
+        {ad_pac_and_none, NULL, false, false, "with MS-PAC and NONE in entry"},
+        {ad_none_and_pad, NULL, false, false, "with NONE and PAD in entry"},
+        {ad_pac_and_pad_case, NULL, true, true, "with MS-PAC and PAD in entry in different case"},
+        {NULL, ad_none_only, false, false, "with only NONE in global config"},
+        {NULL, ad_pac_only, true, false, "with only MS-PAC in global config"},
+        {NULL, ad_pad_only, false, true, "with only PAD in global config"},
+        {NULL, ad_illegal_only, false, false, "with only an invalid value in global config"},
+        {NULL, ad_pac_and_pad, true, true, "with MS-PAC and PAD in global config"},
+        {NULL, ad_pac_and_none, false, false, "with MS-PAC and NONE in global config"},
+        {NULL, ad_none_and_pad, false, false, "with NONE and PAD in global entry"},
+        {NULL, ad_pac_and_pad_case, true, true, "with MS-PAC and PAD in global entry in different case"},
+        {ad_none_only, ad_pac_only, false, false, "with NONE overriding PAC in global entry"},
+        {ad_pad_only, ad_pac_only, false, true, "with PAC overriding PAC in global entry"},
+        {ad_illegal_only, ad_pac_only, false, false, "with invalid value overriding PAC in global entry"},
+        {ad_pac_and_pad, ad_pac_only, true, true, "with PAC and PAD overriding PAC in global entry"},
+        {ad_none_and_pad, ad_pac_only, false, false, "with NONE and PAD overriding PAC in global entry"},
+        {NULL, NULL, false, false, NULL}
+    };
+
+    get_authz_data_types(NULL, NULL, NULL, NULL);
+
+    with_pad = true;
+    get_authz_data_types(NULL, NULL, NULL, &with_pad);
+    fail_unless(!with_pad, "with_pad not false with NULL inuput.");
+
+    with_pac = true;
+    get_authz_data_types(NULL, NULL, &with_pac, NULL);
+    fail_unless(!with_pac, "with_pac not false with NULL inuput.");
+
+    with_pad = true;
+    with_pac = true;
+    get_authz_data_types(NULL, NULL, &with_pac, &with_pad);
+    fail_unless(!with_pad, "with_pad not false with NULL inuput.");
+    fail_unless(!with_pac, "with_pac not false with NULL inuput.");
+
+    entry = calloc(1, sizeof(krb5_db_entry));
+    fail_unless(entry != NULL, "calloc krb5_db_entry failed.");
+
+    ied = calloc(1, sizeof(struct ipadb_e_data));
+    fail_unless(ied != NULL, "calloc struct ipadb_e_data failed.");
+    entry->e_data = (void *) ied;
+
+    kerr = krb5_init_context(&krb5_ctx);
+    fail_unless(kerr == 0, "krb5_init_context failed.");
+    kerr = krb5_db_setup_lib_handle(krb5_ctx);
+    fail_unless(kerr == 0, "krb5_db_setup_lib_handle failed.\n");
+    ipa_ctx = calloc(1, sizeof(struct ipadb_context));
+    fail_unless(ipa_ctx != NULL, "calloc failed.\n");
+    ipa_ctx->kcontext = krb5_ctx;
+    kerr = krb5_db_set_context(krb5_ctx, ipa_ctx);
+    fail_unless(kerr == 0, "krb5_db_set_context failed.\n");
+
+    for (c = 0; test_set[c].authz_data != NULL ||
+                test_set[c].global_authz_data != NULL; c++) {
+        ied->authz_data = test_set[c].authz_data;
+        ipa_ctx->authz_data = test_set[c].global_authz_data;
+        get_authz_data_types(krb5_ctx, entry, &with_pac, &with_pad);
+        fail_unless(with_pad == test_set[c].exp_with_pad, "with_pad not %s %s.",
+                    test_set[c].exp_with_pad ? "true" : "false",
+                    test_set[c].err_msg);
+        fail_unless(with_pac == test_set[c].exp_with_pac, "with_pac not %s %s.",
+                    test_set[c].exp_with_pac ? "true" : "false",
+                    test_set[c].err_msg);
+    }
+
+    krb5_db_fini(krb5_ctx);
+    krb5_free_context(krb5_ctx);
+}
+END_TEST
+
+Suite * ipa_kdb_suite(void)
+{
+    Suite *s = suite_create("IPA kdb");
+
+    TCase *tc_helper = tcase_create("Helper functions");
+    tcase_add_test(tc_helper, test_get_authz_data_types);
+    suite_add_tcase(s, tc_helper);
+
+    return s;
+}
+
+int main(void)
+{
+    int number_failed;
+
+    Suite *s = ipa_kdb_suite ();
+    SRunner *sr = srunner_create (s);
+    srunner_run_all (sr, CK_VERBOSE);
+    number_failed = srunner_ntests_failed (sr);
+    srunner_free (sr);
+
+    return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
-- 
1.8.1.2

-------------- next part --------------
From 6bec0c6de14fbcaf731eed479a73c23073e7c018 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 27 Feb 2013 10:32:40 +0100
Subject: [PATCH 100/100] Mention PAC issue with NFS in service plugin doc

---
 ipalib/plugins/service.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index a3d436e61eb7b7ef2958188a83055b53f52e2562..6b6634458a5980ed263e929365034a7d90bc5bb3 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -66,6 +66,11 @@ EXAMPLES:
  Override a default list of supported PAC types for the service:
    ipa service-mod HTTP/web.example.com --pac-type=MS-PAC
 
+   A typical use case where overriding the PAC type is needed is NFS.
+   Currently the related code in the Linux kernel can only handle Kerberos
+   tickets up to a maximal size. Since the PAC data can become quite large it
+   is recommended to set --pac-type=NONE for NFS services.
+
  Delete an IPA service:
    ipa service-del HTTP/web.example.com
 
@@ -258,7 +263,8 @@ class service(LDAPObject):
             cli_name='pac_type',
             label=_('PAC type'),
             doc=_("Override default list of supported PAC types."
-                  " Use 'NONE' to disable PAC support for this service"),
+                  " Use 'NONE' to disable PAC support for this service,"
+                  " e.g. this might be necessary for NFS services."),
             values=(u'MS-PAC', u'PAD', u'NONE'),
             csv=True,
         ),
-- 
1.8.1.2



More information about the Freeipa-devel mailing list