[Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
Sumit Bose
sbose at redhat.com
Thu Mar 7 17:32:27 UTC 2013
On Wed, Mar 06, 2013 at 05:33:43PM +0100, Sumit Bose wrote:
> On Wed, Mar 06, 2013 at 08:51:47AM -0500, Simo Sorce wrote:
> > On Wed, 2013-03-06 at 14:49 +0100, Martin Kosek wrote:
> > > On 03/06/2013 10:41 AM, Sumit Bose wrote:
> > > > On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote:
> > > >> On 03/04/2013 04:22 PM, Sumit Bose wrote:
> > > >>> On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote:
> > > >>>> On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote:
> > > >>>>> On 03/01/2013 09:20 AM, Sumit Bose wrote:
> > > >>>>>> On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote:
> > > >>>>>>> On 02/28/2013 03:28 PM, Simo Sorce wrote:
> > > >>>>>>>> On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote:
> > > >>>>>>>>> On 02/28/2013 12:42 PM, Sumit Bose wrote:
> > > >>>>>>>>>> On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote:
> > > >>>>>>>>>>> On 02/27/2013 06:48 PM, Sumit Bose wrote:
> > > >>>>>>>>
> > > >>>>>>>>>>> Hi Sumit,
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> This looks like a good idea and would prevent the magic default PAC type, yes.
> > > >>>>>>>>>>> Though I would not add this service-specific setting to global IPA config object.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> I would rather like to see that in the service tree, for example as a
> > > >>>>>>>>>>> configuration option of the service root which could be controlled with
> > > >>>>>>>>>>> serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g:
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE
> > > >>>>>>>>>>> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD
> > > >>>>>>>>>>> # ipa serviceconfig-show
> > > >>>>>>>>>>> Default PAC Map: nfs:NONE, cifs:PAD
> > > >>>>>>>>>>
> > > >>>>>>>>>> Are you thinking of having this in addition to the for-all-services
> > > >>>>>>>>>> default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't
> > > >>>>>>>>>> like the first case because then three different objects needs to be
> > > >>>>>>>>>> consulted to find out which is the right type. This wouldn't be an issue
> > > >>>>>>>>>> for the plugin, but I think it is hard for the user/admin to follow.
> > > >>>>>>>>>
> > > >>>>>>>>> Hm, you are right.
> > > >>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>> If the current defaults shall be dropped I think this is a major change
> > > >>>>>>>>>> because it will require changes in the current CLI and WebUI which will
> > > >>>>>>>>>> be visible to the users. I'm not against this change, I'm just wondering
> > > >>>>>>>>>> if it is worth the effort for the next release?
> > > >>>>>>>>>>
> > > >>>>>>>>>> Maybe an argument to keep this is in global default is that the settings
> > > >>>>>>>>>> are used for the host/*.* services as well which are in a different
> > > >>>>>>>>>> sub-tree of the cn=accounts container. Additionally in future we might
> > > >>>>>>>>>> want apply those setting to the user TGTs as well?
> > > >>>>>>>>>
> > > >>>>>>>>> Yeah, that was actually my point. That we are mixing service-specific PAC
> > > >>>>>>>>> "rules" to the global setting. Which may be shared with host/*.* principals and
> > > >>>>>>>>> user principals. This automatic PAC rules may require some designing so that is
> > > >>>>>>>>> is generally usable.
> > > >>>>>>>>
> > > >>>>>>>> I think putting everything in the general config is more understandable
> > > >>>>>>>> and discoverable. These per-service defaults are basically exceptions to
> > > >>>>>>>> the general rule so it make sense to keep everything together.
> > > >>>>>>>>
> > > >>>>>>>> Simo.
> > > >>>>>>>>
> > > >>>>>>>
> > > >>>>>>> Ok, if these are really just an exceptions to the general rule (and there will
> > > >>>>>>> not be too many of them), I think we can leave it in config entry. But if we
> > > >>>>>>> expect to have exceptions for other types of entries (hosts, users), I think we
> > > >>>>>>> should rather use something like "service:nfs:NONE" do distinguish this exception.
> > > >>>>>>>
> > > >>>>>>> Question is, do we want to implement the interface and processing for that in
> > > >>>>>>> current Sumit's patches or do we use that is they are?
> > > >>>>>>
> > > >>>>>> I would like to update the patches so that they can handle the
> > > >>>>>> service:TYPE style entry and replace the current update code with just
> > > >>>>>> adding nfs:NONE to the global options. I will update the design page
> > > >>>>>> accordingly, too.
> > > >>>>>
> > > >>>>> Ok. If the update procedure shrinks just to adding service:nfs:NONE then it'd
> > > >>>>> be great.
> > > >>>>
> > > >>>> If we need to distinguish between service principals and user principals
> > > >>>> I would prefer rather use a special keyword for upns
> > > >>>>
> > > >>>> service: is redundant and I do not want here to be able to say
> > > >>>> upn:martin:NONE because per principal options are available on the
> > > >>>> principal object.
> > > >>>>
> > > >>>> I actually really do not see the need for changing the default just for
> > > >>>> user principals. If we are worried that one day we might want to really
> > > >>>> have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we
> > > >>>> might add upn:NONE and the lack of / will tell us this is not a service
> > > >>>> named upn/foo.bar.baz but rather it means user principal names.
> > > >>>>
> > > >>>> However I do not see us ever really needing upn:NONE
> > > >>>>
> > > >>>>>> I would prefer if the enhancements needed for the CLI and WebUI can be
> > > >>>>>> covered by other/new tickets, but I'm happy to add the needed
> > > >>>>>> information to the design page too.
> > > >>>>>>
> > > >>>>>> bye,
> > > >>>>>> Sumit
> > > >>>>>
> > > >>>>> I am OK with adding the interface for this special exception later. In that
> > > >>>>> case, a ticket + note in the design as you mentioned would be enough.
> > > >>>>
> > > >>>> Ack.
> > > >>>>
> > > >>>> Simo.
> > > >>>>
> > > >>>
> > > >>> Please find attached a new version of the patches. 0095 i(updating) is
> > > >>> renamed and much simpler now. I opened
> > > >>> https://fedorahosted.org/freeipa/ticket/3484 to added the needed change
> > > >>> for 'service:TYPE' to CLI and WebUI. For the time being I've added
> > > >>> patch 0108 which simply allows 'nfs:NONE' as a type to make sure that it
> > > >>> is not deleted accidentally when e.g. using the WebUI. If you do not
> > > >>> like it it can simply be dropped, everything is working fine without it.
> > > >>>
> > > >>> bye,
> > > >>> Sumit
> > > >>>
> > > >>
> > > >> Patch 0098:
> > > >>
> > > >> If this part does not match (and it will not for all non-nfs service principals):
> > > >>
> > > >> + if (service_type->length == (sep - authz_data_list[c]) &&
> > > >> + strncmp(authz_data_list[c], service_type->data,
> > > >> + service_type->length) == 0) {
> > > >> + authz_data_type = sep + 1;
> > > >> + }
> > > >>
> > > >> krb5kdc.log will contain an error:
> > > >>
> > > >> Mar 05 10:48:50 ipa.linux.ad.test krb5kdc[26703](Error): Ignoring unsupported
> > > >> authorization data type [nfs:NONE].
> > > >> Mar 05 10:48:50 ipa.linux.ad.test krb5kdc[26703](info): TGS_REQ (4 etypes {18
> > > >> 17 16 23}) 10.16.78.33: ISSUE: authtime 1362482261, etypes {rep=18 tkt=18
> > > >> ses=18}, HTTP/ipa.linux.ad.test at LINUX.AD.TEST for
> > > >> ldap/ipa.linux.ad.test at LINUX.AD.TEST
> > > >>
> > > >> I think we should just "continue" in this case.
> > > >
> > > > good catch, fixed
> > >
> > > Ok, thanks.
> > >
> > > >
> > > >>
> > > >> Otherwise, this looks and works fine.
> > > >
> > > > Thank you for the review, new version attached.
> > > >
> > > > I have an additional question about processing the service specific
> > > > defaults. Using 'service:NONE' is unambiguous because NONE trumps all.
> > > > But what do we expect if e.g. the defaults are MS-PAC and ldap:PAD for a
> > > > LDAP service ticket. Shall it contain PAC and PAD or only a PAD? I think
> > > > the first one where all defaults which apply to a service are added up
> > > > is more clear, and this is also the way the current code works. But I
> > > > wouldn't mind to change the logic if you think it makes more sense the
> > > > other way round, i.e. if there is a service specific default matching
> > > > the requested service only the service specific default are accounted.
> > >
> > > Hmm, that's a good question. I understand service:PACTYPE as an exclusion to
> > > the general setting, so for me it would make sense to override the global
> > > config - i.e. tje second case.
> > >
> > > Thus, if global config is MS-PAC, ldap:PAD, I think that ldap should have just
> > > PAD. If one also want MS-PAC, it should be set like "MS-PAC, ldap:PAD,
> > > ldap:MS-PAC". Otherwise you would not be able to configure that you want MS-PAC
> > > for all services and _only_ PAD for ldap...
> > >
> > > Does it make sense? We should also make sure that we update the RFE page to
> > > whatever we decide to do.
> >
> > +1 to Martin's choice.
>
> Martin, Simo, thank you for the feedback. I will send an new version of
> the patches, add a section in the design page explaining the details and
> add some tests for this logic.
>
> bye,
> Sumit
Design pages is updated and new patches attached.
bye,
Sumit
-------------- next part --------------
From 50c64c4db3601bba96e7aeadf6003628eea3282e 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/101] 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 5071348d90dedf60f8140e0a53b07d09a947d31f..eafba97393cdda43d8a2a4616f65f2291ba08aa7 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 01b674384d3ac9fbfa652b9d9ed8d72cc5c9e946 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 4 Mar 2013 14:25:43 +0100
Subject: [PATCH 095/101] Add NFS specific default for authorization data type
Since the hardcoded default fpr the NFS service was removed the default
authorization data type is now set in the global server configuration.
---
install/updates/60-trusts.update | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/install/updates/60-trusts.update b/install/updates/60-trusts.update
index cd7a7e355230168f6ed9333505469a0aa7498ba9..454ecb745a9cf266a200f64cdea0b11872858113 100644
--- a/install/updates/60-trusts.update
+++ b/install/updates/60-trusts.update
@@ -71,6 +71,11 @@ replace:aci:'(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword |
dn: cn=ipaConfig,cn=etc,$SUFFIX
addifnew: ipaKrbAuthzData: MS-PAC
+# Add authorization data type NONE for NFS because the hardcoded default was
+# removed.
+dn: cn=ipaConfig,cn=etc,$SUFFIX
+add: ipaKrbAuthzData: nfs:NONE
+
# Fix typo in some installs in the spelling of ORDERING. They were added
# with a typo which was silently dropped by 389-ds-base, so add in the
# proper ordering syntax now.
--
1.8.1.2
-------------- next part --------------
From 22d554cd6a2ea59229206e0eed60c7ebf631f8ad 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/101] 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 3fa054b938196925d9ce0849a02ecb26d5aa4719 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/101] 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 13f6a21f1556668e8e9acd54f3d0e6a345eb00dc..11c155e64e20d16da98158eb4d7b8034803bf1de 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 50ec2a090f55c76610b7cafa0addd8ce1c02d37e 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] 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 | 142 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 140 insertions(+), 2 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index eafba97393cdda43d8a2a4616f65f2291ba08aa7..2662b947bd589074faa1acf6a9bb97748161cb05 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,127 @@ 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;
+ bool srv_none_found = false;
+ char **authz_data_list;
+ bool with_pac = false;
+ bool srv_with_pac = false;
+ bool with_pad = false;
+ bool srv_with_pad = false;
+ char *sep;
+ krb5_data *service_type;
+ char *authz_data_type;
+ bool service_specific;
+
+ 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++) {
+ service_specific = false;
+ authz_data_type = authz_data_list[c];
+ sep = strchr(authz_data_list[c], ':');
+ if (sep != NULL) {
+ if (entry->princ == NULL) {
+ krb5_klog_syslog(LOG_ERR, "Missing principal in database "
+ "entry, no authorization data will " \
+ "be added.");
+ goto done;
+ }
+
+ service_type = krb5_princ_component(context, entry->princ, 0);
+ if (service_type == NULL) {
+ krb5_klog_syslog(LOG_ERR, "Missing service type in database "
+ "entry, no authorization data will " \
+ "be added.");
+ goto done;
+ }
+
+ if (service_type->length == (sep - authz_data_list[c]) &&
+ strncmp(authz_data_list[c], service_type->data,
+ service_type->length) == 0) {
+ service_specific = true;
+ authz_data_type = sep + 1;
+ } else {
+ /* Service specific default does not apply, skipping this
+ * entry. */
+ continue;
+ }
+ }
+
+ if (strcmp(authz_data_type, AUTHZ_DATA_TYPE_PAC) == 0) {
+ if (service_specific) {
+ srv_with_pac = true;
+ } else {
+ with_pac = true;
+ }
+ } else if (strcmp(authz_data_type, AUTHZ_DATA_TYPE_PAD) == 0) {
+ if (service_specific) {
+ srv_with_pad = true;
+ } else {
+ with_pad = true;
+ }
+ } else if (strcmp(authz_data_type, AUTHZ_DATA_TYPE_NONE) == 0) {
+ if (service_specific) {
+ srv_none_found = true;
+ } else {
+ none_found = true;
+ }
+ } else {
+ krb5_klog_syslog(LOG_ERR, "Ignoring unsupported " \
+ "authorization data type [%s].",
+ authz_data_list[c]);
+ }
+ }
+
+done:
+ if (srv_none_found || srv_with_pac || srv_with_pad) {
+ none_found = srv_none_found;
+ with_pac = srv_with_pac;
+ with_pad = srv_with_pad;
+ }
+
+ 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 +1662,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 +1674,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 +1695,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 89485808e16696aaea8cd10c49837caa7d57adda 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] Add unit test for get_authz_data_types()
---
daemons/ipa-kdb/Makefile.am | 29 +++++
daemons/ipa-kdb/tests/ipa_kdb_tests.c | 217 ++++++++++++++++++++++++++++++++++
2 files changed, 246 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..fbee4acdbe8d6426664aef8e2d826ef6c065a514
--- /dev/null
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -0,0 +1,217 @@
+/** 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"
+
+#define NFS_PRINC_STRING "nfs/fully.qualified.host.name at REALM.NAME"
+#define NON_NFS_PRINC_STRING "abcdef/fully.qualified.host.name at REALM.NAME"
+
+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_global_pac_nfs_none[] = {"MS-PAC", "nfs:NONE", NULL};
+ char *ad_global_pac_nfs_pad[] = {"MS-PAC", "nfs:PAD", NULL};
+ krb5_context krb5_ctx;
+ krb5_error_code kerr;
+ struct ipadb_context *ipa_ctx;
+ krb5_principal nfs_princ;
+ krb5_principal non_nfs_princ;
+
+ 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");
+
+ kerr = krb5_parse_name(krb5_ctx, NFS_PRINC_STRING, &nfs_princ);
+ fail_unless(kerr == 0, "krb5_parse_name failed.");
+
+ kerr = krb5_parse_name(krb5_ctx, NON_NFS_PRINC_STRING, &non_nfs_princ);
+ fail_unless(kerr == 0, "krb5_parse_name failed.");
+
+ struct test_set {
+ char **authz_data;
+ char **global_authz_data;
+ krb5_principal princ;
+ bool exp_with_pac;
+ bool exp_with_pad;
+ const char *err_msg;
+ } test_set[] = {
+ {ad_none_only, NULL, NULL, false, false, "with only NONE in entry"},
+ {ad_pac_only, NULL, NULL, true, false, "with only MS-PAC in entry"},
+ {ad_pad_only, NULL, NULL, false, true, "with only PAD in entry"},
+ {ad_illegal_only, NULL, NULL, false, false, "with only an invalid value in entry"},
+ {ad_pac_and_pad, NULL, NULL, true, true, "with MS-PAC and PAD in entry"},
+ {ad_pac_and_none, NULL, NULL, false, false, "with MS-PAC and NONE in entry"},
+ {ad_none_and_pad, NULL, NULL, false, false, "with NONE and PAD in entry"},
+ {NULL, ad_none_only, NULL, false, false, "with only NONE in global config"},
+ {NULL, ad_pac_only, NULL, true, false, "with only MS-PAC in global config"},
+ {NULL, ad_pad_only, NULL, false, true, "with only PAD in global config"},
+ {NULL, ad_illegal_only, NULL, false, false, "with only an invalid value in global config"},
+ {NULL, ad_pac_and_pad, NULL, true, true, "with MS-PAC and PAD in global config"},
+ {NULL, ad_pac_and_none, NULL, false, false, "with MS-PAC and NONE in global config"},
+ {NULL, ad_none_and_pad, NULL, false, false, "with NONE and PAD in global entry"},
+ {NULL, ad_global_pac_nfs_none, NULL, true, false, "with NULL principal and PAC and nfs:NONE in global entry"},
+ {NULL, ad_global_pac_nfs_none, nfs_princ, false, false, "with nfs principal and PAC and nfs:NONE in global entry"},
+ {NULL, ad_global_pac_nfs_none, non_nfs_princ, true, false, "with non-nfs principal and PAC and nfs:NONE in global entry"},
+ {NULL, ad_global_pac_nfs_pad, NULL, true, false, "with NULL principal and PAC and nfs:PAD in global entry"},
+ {NULL, ad_global_pac_nfs_pad, nfs_princ, false, true, "with nfs principal and PAC and nfs:PAD in global entry"},
+ {NULL, ad_global_pac_nfs_pad, non_nfs_princ, true, false, "with non-nfs principal and PAC and nfs:PAD in global entry"},
+ {ad_none_only, ad_pac_only, NULL, false, false, "with NONE overriding PAC in global entry"},
+ {ad_pad_only, ad_pac_only, NULL, false, true, "with PAC overriding PAC in global entry"},
+ {ad_illegal_only, ad_pac_only, NULL, false, false, "with invalid value overriding PAC in global entry"},
+ {ad_pac_and_pad, ad_pac_only, NULL, true, true, "with PAC and PAD overriding PAC in global entry"},
+ {ad_none_and_pad, ad_pac_only, NULL, false, false, "with NONE and PAD overriding PAC in global entry"},
+ {NULL, NULL, NULL, false, false, NULL}
+ };
+
+ 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;
+ entry->princ = test_set[c].princ;
+ 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_free_principal(krb5_ctx, nfs_princ);
+ krb5_free_principal(krb5_ctx, non_nfs_princ);
+ 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 82cb66279454e6c625ca409235e6b8e631eeb973 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/101] 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
-------------- next part --------------
From 3b61193647a1ac1dce24832260edc31f638a368b Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 4 Mar 2013 14:52:10 +0100
Subject: [PATCH 101/101] Allow 'nfs:NONE' in global configuration
This patch adds 'nfs:NONE' as an allowed entry for the global
authorization data type in the CLI and WebUI. This is an ad-hoc solution
to make sure that the new default value for the NFS service is not
removed by chance.
This patch should be removed if a more generic solution is implemented
to modify service:TYP style values of the authorization data type.
---
API.txt | 2 +-
install/ui/src/freeipa/serverconfig.js | 4 ++--
ipalib/plugins/config.py | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/API.txt b/API.txt
index a43fce596a6e01ba1fc709242ede0303994a255d..5ddda637175184d115396b1f394db1b45ae57de9 100644
--- a/API.txt
+++ b/API.txt
@@ -498,7 +498,7 @@ option: Str('ipadefaultprimarygroup', attribute=True, autofill=False, cli_name='
option: Str('ipagroupobjectclasses', attribute=True, autofill=False, cli_name='groupobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipagroupsearchfields', attribute=True, autofill=False, cli_name='groupsearch', multivalue=False, required=False)
option: IA5Str('ipahomesrootdir', attribute=True, autofill=False, cli_name='homedirectory', multivalue=False, required=False)
-option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD'))
+option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'nfs:NONE'))
option: Int('ipamaxusernamelength', attribute=True, autofill=False, cli_name='maxusername', minvalue=1, multivalue=False, required=False)
option: Bool('ipamigrationenabled', attribute=True, autofill=False, cli_name='enable_migration', multivalue=False, required=False)
option: Int('ipapwdexpadvnotify', attribute=True, autofill=False, cli_name='pwdexpnotify', minvalue=0, multivalue=False, required=False)
diff --git a/install/ui/src/freeipa/serverconfig.js b/install/ui/src/freeipa/serverconfig.js
index 347c46c1cb4daa94555c5a3272010d8a57e308de..1c6dd219bce5ba5ee010930b7e508c071680446f 100644
--- a/install/ui/src/freeipa/serverconfig.js
+++ b/install/ui/src/freeipa/serverconfig.js
@@ -102,7 +102,7 @@ IPA.serverconfig.entity = function(spec) {
{
name: 'ipakrbauthzdata',
type: 'checkboxes',
- options: IPA.create_options(['MS-PAC', 'PAD'])
+ options: IPA.create_options(['MS-PAC', 'PAD', 'nfs:NONE'])
}
]
}
@@ -117,4 +117,4 @@ IPA.serverconfig.entity = function(spec) {
IPA.register('config', IPA.serverconfig.entity);
return {};
-});
\ No newline at end of file
+});
diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py
index db7fce7cb6ddea5e7364baf50eaa2e1a5b9f8021..33eb174ecc2c6af0463b475f3b87a734dd06f988 100644
--- a/ipalib/plugins/config.py
+++ b/ipalib/plugins/config.py
@@ -194,7 +194,7 @@ class config(LDAPObject):
cli_name='pac_type',
label=_('Default PAC types'),
doc=_('Default types of PAC supported for services'),
- values=(u'MS-PAC', u'PAD'),
+ values=(u'MS-PAC', u'PAD', u'nfs:NONE'),
csv=True,
),
)
--
1.8.1.2
More information about the Freeipa-devel
mailing list