Re: [Freeipa-devel] [PATCHES 0031-0032] Improve HBAC rule handling in selinuxusermap-add/mod/find
On 02/20/2013 12:31 PM, Tomas Babej wrote: On 02/19/2013 10:33 PM, Rob Crittenden wrote: Tomas Babej wrote: On 02/06/2013 07:57 PM, Rob Crittenden wrote: Tomas Babej wrote: Hi, this pair of patches improves HBAC rule handling in selinuxusermap commands. Patch 0031 deals with: https://fedorahosted.org/freeipa/ticket/3349 Patch 0032 takes care of: https://fedorahosted.org/freeipa/ticket/3348 and is to be applied on top of Patch 0031. See commit messages for detailed info. Tomas ACK for patch 0032. For patch 0031 we can't change the data type of an existing attribute. It will break backwards compatibility. Can you test with an older client to see if it cares (it may not care about the name of the type). If older clients will work then this is probably ok. I gather that seealso detected as a DN attribute and converted into a DN class and this is blowing up the Str validator? Yes, that was exactly the case. rob I added a workaround for older client versions, tested it with freeipa-client/admintools 2.2, works as expeceted. However, this only should be issue if there is older admintools package on the client than on the server. Outline is such as follows: I added a new flag for DNParam seelalso attribute, called 'allow_malformed' that allows any string to be passed to DNParam. Its value gets wrapped in 'malformed=yes,value=value'. This allows to parse out the string in selinuxusermap-add/mod pre_callback out of the DN and search for the rule with such name so that it's DN gets in LDAP instead. Updated patch attached. Tomas I like where you're going with this, just a couple of comments: 1. Should we come up with a more universal name for allow_malformed? Is this something that we should allow at a higher level? I was thinking allow_raw, or allow_non_dn, or something like that. To me, allow_non_dn sounds is just as specific as allow_malformed, they both refer to DN specifically. I'd go with allow_raw, if need for such pattern may eventually arise for other parameter classes than DNParam. What do you mean by higher level, turning this hack into a feature Param class? I don't see how this would work, each parameter class that implements its own type validation as DNParam needs to override _convert_scalar(). And in every such class we would need to wrap our raw value so that it is represented in the type of this parameter, as we do with DN(('malformed','yes'),('value',value)) now. Maybe we could skip type validation in _convert_scalar default implementation or catch the error raised somehow, and let the type be invalid, but I'm not aware of the consenquences. I would need to investigate. Wouldn't it cause failure deeper in the framework? Or did you by higher level mean simply picking a more general name for the flag so it can be reused in other parameter classes with the same name? 2. I think that if a bad dn is passed in as a Str the conversion into a DN won't be handled: +if 'allow_malformed' in self.flags: +dn = DN(('malformed','yes'),('value',value)) Should this be wrapped in a try/except to raise a ConversionError if it fails? Yes, thanks for that catch. rob Tomas Is it just me, or does the 0031 look overengineered? I think this is a general problem for each Str parameter which we then process/convert to DN in our pre_callbacks. selinuxusermap is one example where this does not work. This fix leaves other examples not working: # ipa trustconfig-mod --setattr ipantfallbackprimarygroup=cn=Default SMB Group,cn=groups,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com ipa: ERROR: invalid 'ipantfallbackprimarygroup': must be Unicode text I would rather propose to not automatically encode DN of known attributes set by *attr: diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 1ebbe7a..e4b9834 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -768,12 +768,6 @@ last, after all sets and adds.), # None means delete this attribute value = None -if ldap.has_dn_syntax(attr): -try: -value = DN(value) -except ValueError: -raise errors.InvalidSyntax(attr=attr) - if attr in newdict: if type(value) in (tuple,): newdict[attr] += list(value) I think this conversion is just done too early as this Str param is processed and converted later in the pre_callback, when needed. The code above introduced inconsistent processing of IPA attributes with DN syntax coming from regular option and from *attr option - Str When I did this change, both selinuxusermap-mod and trustconfig-mod started working: # ipa selinuxusermap-mod foo --setattr=seealso=ipaUniqueID=70e42636-75db-11e2-9df6-001a4a104edc,cn=hbac,dc=rhel64,dc=ad,dc=test --- Modified SELinux User Map foo
Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation
On 02/22/2013 04:02 PM, Ana Krivokapic wrote: On 02/22/2013 10:19 AM, Petr Spacek wrote: On 20.2.2013 11:03, Ana Krivokapic wrote: On 02/18/2013 01:08 PM, Martin Kosek wrote: On 02/18/2013 12:47 PM, Sumit Bose wrote: On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote: On 15.2.2013 15:22, Ana Krivokapic wrote: Hello, The .isalpha() check in validate_domain_name() was too strict, causing some commands like ipa dnsrecord-add to fail. https://fedorahosted.org/freeipa/ticket/3385 I would add --force option rather than removing whole check, if it's possible. Would it be possible to mention RFC in the error message? Something like _('top level domain label must be alphabetic (RFC 1123 section 2.1)') ? IMHO it is handy, because it educates users. The problem is that this check is always done on the last component of the domain_name even if it is just a sub-domain of the FreeIPA domain, where e.g. numbers are valid characters. At the beginning of validate_domain_name() a trailing '.' is stripped away. iirc the trailing '.' is an indication for a complete, fully qualified name. Would it work if the presence of the trailing '.' is saved and the check is only done if there was a '.'? bye, Sumit Sure. Though I am now not 100% sure that some IPA functions do not use this validator with a fqdn hostname without trailing dot. If not, I am for fixing this function as Sumit and Petr suggested. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel After some thought, I decided to change the approach. As pointed out by Sumit, the problem was that the validate_domain_name() function did not distinguish between fqdn and non-fqdn domains (subdomains of the IPA domain). The trailing dot is not a clear indication either, because some IPA functions use this validator with an fqdn without the trailing dot. To fix this, I introduced an additional parameter to this function - a flag which indicates whether the domain name is an fqdn or not. The is .isalpha() check is then performed only in the case of an fqdn. I also improved the error message to mention the relevant RFC, as suggested by Petr. Please don't forget to add --force switch. It could be handy. I added the --force switch to ipa dnsrecord-add and opened a new ticket to handle the rest of the ipa commands that use domain name validation: https://fedorahosted.org/freeipa/ticket/3455 Updated patch is attached. This patch fixed validation only partially. The --force flag you made available will not allow admin to for example add a zone example.zone1 which technically will be resolvable, it is just not a good practice: # ipa dnszone-add example.zone1 --name-server `hostname`. --force ipa: ERROR: invalid 'name': top level domain label must be alphabetic (RFC 1123 section 2.1) To enable this, I think you would need to not postpone the validation to DNS zone pre_callback as you could not check --force flag presence right in the idnsname parameter validator. We may also want to change --force flag label, it now talks only about NS record validation, but we now expanded it a bit, so the label would need to be more general. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 374 Remove ORDERING for IA5 attributeTypes
On 02/26/2013 06:03 PM, Martin Kosek wrote: IA5 string syntax does not have a compatible ORDERING matching rule. Simply use default ORDERING for these attributeTypes as we already do in other cases. https://fedorahosted.org/freeipa/ticket/3398 - This is a follow-up ticket for regression introduced by Rob's 1087. I did not add any update rules for IPA servers that were already wrongly updated as we did not release any IPA version with these bad attributeTypes and I want to keep .update files small. Martin ACK -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
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 From 8eb34dd3190853bd3fa400434be4f7c720bf1354 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@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 95349702038e1d55bd257816f7f61e9557a5..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))
[Freeipa-devel] [PATCHES] 101-107 Fixes for various Coverity issues
Hi, the attached patches 102-107 fix issues found by Coverity which are tracked by tickets #3422-#3427 and remove an unused variable (patch 101). bye, Sumit From 97b3b7dedac28704d51e2fa4b4dc975a20d17ada Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Tue, 19 Feb 2013 12:48:58 +0100 Subject: [PATCH 101/107] ipa-kdb: remove unused variable --- daemons/ipa-kdb/ipa_kdb_principals.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index 46a921cb2f267b977df4a0042d81a49beccb5a91..11c155e64e20d16da98158eb4d7b8034803bf1de 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -535,7 +535,7 @@ static krb5_error_code ipadb_fetch_principals(struct ipadb_context *ipactx, krb5_error_code kerr; char *src_filter = NULL; char *esc_original_princ = NULL; -int ret, i; +int ret; if (!ipactx-lcontext) { ret = ipadb_get_connection(ipactx); -- 1.8.1.2 From d5a64eced4800190221395d19341918ae417f118 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Tue, 19 Feb 2013 12:49:25 +0100 Subject: [PATCH 102/107] ipa-kdb: Uninitialized scalar variable in ipadb_reinit_mspac() There was a code path where ret was used instead of kerr to save a return value. Fixes https://fedorahosted.org/freeipa/ticket/3422 --- daemons/ipa-kdb/ipa_kdb_mspac.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index c72e762ca4c680def9ed97e492f6edfcc5e0677f..d022bd9a958f2ec591a6a338753b510492b2d400 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -1986,12 +1986,11 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx) if (ipactx-mspac ipactx-mspac-num_trusts == 0) { /* Check if there is any trust configured. If not, just return * and do not re-initialize the MS-PAC structure. */ -ret = ipadb_mspac_check_trusted_domains(ipactx); -if (ret == KRB5_KDB_NOENTRY) { -ret = 0; +kerr = ipadb_mspac_check_trusted_domains(ipactx); +if (kerr == KRB5_KDB_NOENTRY) { +kerr = 0; goto done; -} else if (ret != 0) { -ret = EIO; +} else if (kerr != 0) { goto done; } } -- 1.8.1.2 From c36fc476b52315012d2d17994d0337761d25cb5e Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Wed, 27 Feb 2013 12:12:45 +0100 Subject: [PATCH 103/107] ipa-sam: Array compared against 0 in ipasam_set_trusted_domain() ipa_mspac_well_known_sids is a globally defined array so the check was always true. Fixes https://fedorahosted.org/freeipa/ticket/3423 --- daemons/ipa-sam/ipa_sam.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c index 0d4a27cf6def737412f6ab3ca48df8d1339c15b6..b9fc00c8d08821e09b3242a82b3673a88ace0a51 100644 --- a/daemons/ipa-sam/ipa_sam.c +++ b/daemons/ipa-sam/ipa_sam.c @@ -2293,7 +2293,7 @@ static NTSTATUS ipasam_set_trusted_domain(struct pdb_methods *methods, td-trust_forest_trust_info); } - for (i = 0; ipa_mspac_well_known_sids ipa_mspac_well_known_sids[i]; i++) { + for (i = 0; ipa_mspac_well_known_sids[i]; i++) { smbldap_make_mod(priv2ld(ldap_state), entry, mods, LDAP_ATTRIBUTE_SID_BLACKLIST_INCOMING, ipa_mspac_well_known_sids[i]); -- 1.8.1.2 From bffd1ec51ea697887d012ec1eda4171f4affafaf Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Fri, 22 Feb 2013 13:30:30 +0100 Subject: [PATCH 104/107] ipa-kdb: Dereference after null check in ipa_kdb_mspac.c A wrong logic was used to check ipactx. Fixes https://fedorahosted.org/freeipa/ticket/3424 --- daemons/ipa-kdb/ipa_kdb_mspac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index d022bd9a958f2ec591a6a338753b510492b2d400..2c07f4cffe38eb127a7b3f901dc7700e88d6af2a 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -1217,7 +1217,7 @@ static krb5_error_code filter_logon_info(krb5_context context, * */ if (info-info-info3.sidcount != 0) { ipactx = ipadb_get_context(context); -if (!ipactx !ipactx-mspac) { +if (!ipactx || !ipactx-mspac) { return KRB5_KDB_DBNOTINITED; } count = info-info-info3.sidcount; -- 1.8.1.2 From e56a47f82f4600ebcb26c012c382c6686125c14a Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Fri, 22 Feb 2013 13:40:08 +0100 Subject: [PATCH 105/107] ipa-lockout: Wrong sizeof argument in ipa_lockout.c Fixes https://fedorahosted.org/freeipa/ticket/3425 ---
[Freeipa-devel] Using the new LDAP code
Hello, A big refactoring of our LDAP code should be merged soon-ish now. Here's a summary for developers. If you see these outside ipaldap.py, you're looking at deprecated API: - methods with camelCaseNames - methods with _s and _ext postfixes (modify_s, search_ext, ...) The exception is client code and places where we don't want to read the schema (migration, AD). These are still limited to raw python-ldap for now. The LDAPEntry class represents LDAP entries. It behaves like a dictionary of lists: entry.get(attrname) or entry[attrname] should always give you a list. LDAPEntry.dn will give you the entry's DN. Single-value attributes are represented as lists with a single value. (For now, some code still puts bare values instead of lists in entries, in that case you'll still get a bare value from get()/__getitem__. That should be fixed eventually.) The single_value method gets a single value, with checking. Always use `entry.single_value('abc')` instead of `entry.get('abc')[0]`. Also, single_value allows a default: `entry.single_value('abc', None)`. LDAPEntry is case-insensitive and handles attributes with multiple names: entry['cn'] and entry['CN'] and entry['CommonName'] are equivalent. IPA plugins traditionally use (dn, entry_attrs) pairs to represent entries. To make that work, iterating over an LDAPEntry will, for now, yield the DN and the entry itself. Always use keys() or values() when iterating over an entry. LDAPEntry objects are tied to a connection. Do not create them directly, use a connection method like make_entry() or get_entry(). Speaking of connections, there still are two classes for those: ldap2 and IPAdmin. ldap2 is an API plugin created using the IPA settings. It works in Apache (per-thread connections). It also applies the default IPA settings (time records limit). Use IPAdmin if IPA is not installed yet (or when it's being uninstalled), or if you need to connect to a non-default server or bind as a user like the DM. Besides the connecting code, both of these (will ideally) have the same API, based on the old ldap2. A rough summary: - make_entry(dn, attrs) - get_entry(dn) - add_entry(entry) - update_entry(entry) - delete_entry(entry_or_dn) - get_entries(base_dn, [scope, [filter, [attrs_list]]]): simple search - find_entries: more powerful (and backwards-compatible) search - make_filter friends, unchanged from ldap2 ldap2's DN normalization – appending the suffix to DNs that don't end with it – is gone, you need to do that manually now. That should be it, if you don't intend to hack on ipaldap itself or the ldapupdater. If you have any questions, ask! (Or look at the code :) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 374 Remove ORDERING for IA5 attributeTypes
On 02/27/2013 11:58 AM, Petr Viktorin wrote: On 02/26/2013 06:03 PM, Martin Kosek wrote: IA5 string syntax does not have a compatible ORDERING matching rule. Simply use default ORDERING for these attributeTypes as we already do in other cases. https://fedorahosted.org/freeipa/ticket/3398 - This is a follow-up ticket for regression introduced by Rob's 1087. I did not add any update rules for IPA servers that were already wrongly updated as we did not release any IPA version with these bad attributeTypes and I want to keep .update files small. Martin ACK Pushed to master, ipa-3-1, ipa-3-0. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 101-107 Fixes for various Coverity issues
On 02/27/2013 01:39 PM, Martin Kosek wrote: On 02/27/2013 12:35 PM, Sumit Bose wrote: Hi, the attached patches 102-107 fix issues found by Coverity which are tracked by tickets #3422-#3427 and remove an unused variable (patch 101). bye, Sumit I see just one issue. In patch 0105: -global_ipactx = (struct ipa_context *)malloc(sizeof(global_ipactx)); +global_ipactx = (struct ipa_context *)malloc(sizeof(*global_ipactx)); I do not think this will work right. *global_ipactx will just de-reference global_ipactxt and run sizeof on the result, right? I would prefer this change: global_ipactx = (struct ipa_context *)malloc(sizeof(struct ipa_context *)); Martin We just discussed this in Brno - in fact your change should be right as you allocate memory for the whole structure and not just a pointer to the structure (though I personally would prefer sizeof(struct ipa_context) as it is clearer). Hmrph, C language 101 revisited :-) Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] CA name constrains
Hello list, during our last meeting with Simo we discussed support for name constraint extension in CA certificates and clients. The Name Constraints Extensions is defined here: http://tools.ietf.org/html/rfc5280#section-4.2.1.10 Following article could be interesting for you if you like longer stories: Mozilla changes policy to limit risk of subordinate CA certificate abuse Author: Lucian Constantin 19.02.2013 kl 21:50 http://news.idg.no/cw/art.cfm?id=8C9E7CFA-0E65-24B0-1539C891C8F4C09B If I remember correctly, questions were mainly about support on client side and about implications for older clients. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 146-164 LDAP code refactoring (Part 4)
On 26.2.2013 11:03, Petr Viktorin wrote: Thanks. I think you should also add a tearDown method to test_LDAPEntry which disconnects self.conn if it is connected (the same thing test_ldap does). Thanks for the catch, added. ACK. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] CA name constrains
On Wed, 2013-02-27 at 13:55 +0100, Petr Spacek wrote: Hello list, during our last meeting with Simo we discussed support for name constraint extension in CA certificates and clients. The Name Constraints Extensions is defined here: http://tools.ietf.org/html/rfc5280#section-4.2.1.10 Following article could be interesting for you if you like longer stories: Mozilla changes policy to limit risk of subordinate CA certificate abuse Author: Lucian Constantin 19.02.2013 kl 21:50 http://news.idg.no/cw/art.cfm?id=8C9E7CFA-0E65-24B0-1539C891C8F4C09B If I remember correctly, questions were mainly about support on client side and about implications for older clients. I had a chat with Kai Engert (in CC) at DevConf.cz about this, we'll try to work on this as time permits. NSS seem to support this extension but so far we do not have tests covering it apparently. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] What about desktop policies?
El mar, 26-02-2013 a las 15:11 -0500, Dmitri Pal escribió: On 02/25/2013 02:15 PM, Loris Santamaria wrote: Hi all, some customers of ours are interested in managing desktop policies for their linux workstations, really nothing fancy, corporate background and proxy settings are the most common requests. In the past I created Gnome desktop profiles using Sabayon, distributed them using puppet and associated them to user accounts with a Sabayon specific LDAP attribute, a process a bit convoluted, and no longer possible since sabayon is no longer developed. Also it was really buggy, and very gnome specific. I was thinking in how integrate desktop policies in freeIPA in a general manner and I wanted to share my ideas with you. Hopefully some of this may be incorporated in IPA at some point in the future. Properties of a policy: * is a collection of settings * can be associated with users or groups (desktop policy) or with hosts or hostgroups (system policy) * is associated with a consumer, the client software that interprets and applies the policy. This way one could define policies for dconf, policies for kde, policies for WBEM. Properties of a setting * is a key-value pair * must conform to a schema * may be mandatory The schema: * indicates which attributes a policy may consist of * indicates which kind of value may take an attribute. Bool, string, etc. * There may be more than one schema for a given consumer. For example for dconf you may have an evolution schema, a gnome-games schema, etc. Sample policy creation process: 1. The admin creates a new schema in IPA, with a command like ipa schema-add --consumer=dconf gnomeSettingsSchema 2. The admin adds some definition to the schema: ipa schema-add-setting gnomeSettingsSchema --name=/schemas/desktop/gnome/background/picture_filename --type=string --description='File to use for the background image.' 3. He creates a new policy: ipa policy-add corporateBackground --type=desktop --consumer=dconf 4. He adds a setting to the policy: ipa policy-add-setting corporateBackground --name=/schemas/desktop/gnome/background/picture_filename --value=file:///san/wp/wallpaper.jpg --mandatory. Ipa would check that the key is defined in one of the dconf related schemas and the value is acceptable for that key. 5. He associates the policy with users: ipa-policy-add-user corporateBackground --groups=ipausers How should the policy be applied? On the workstation, on startup, an ipa related utility should check if there are any policies related to the workstation, if there are any it should call a helper capable of applying a specific type of policy. Then on user logon another ipa related utility should check if there are any policies associated with the user and call the appropriate helper, if available. For the policy created in the above example, on logon the ipa policy utility would find that there is a policy of type dconf associated with the user. It would check if there is a dconf policy helper installed and if positive it would call the helper passing it the parameters defined in the policy. Hope this is useful at least as a starting point in defining desktop policies in IPA. This is great! Thank you for sharing some ideas. We sort of stayed away from centralized policy management for quite some time. Originally we thought that IPA will do policy management and did a lot of design around it. However at some point we realized that there is an overlap with the system management and content management for which things like puppet are more suitable. We said then that IdM would focus on managing identity related policies that are traditionally served via LDAP. The things that you are talking about resemble to some extent MSFT GPO and we felt that IdM might not be the right place for it. May be it is time to reassess it. I would however not go that route at least yet. Hey puppet is great and we use it a lot to install packages and to distribute configuration files, yet it is not so great to set these key=value kind of settings of which more and more modern software consists of. When you take into consideration gconf settings for gnome 2.x, dconf settings for gnome 3.x, mozilla settings, thunderbird settings it quickly becomes a PITA to manage them distributing around files with puppet. Having those key=value pairs in an ldap would allow a helper on the client to pull only the keys it understands and to merge them in the configuration database in the appropriate way. Of course i took inspiration from AD GPOs, yet I think that IPA should manage these key=value kind of policies in a more
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: 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. Is there a good reason to allow insensitive matching ? in LDAP you can't use true or false either for booleans, you have to use TRUE and FALSE, so I am not so thrilled to allow insensitive matching for this option 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. I do not think we should set the policy to NONE by default, OTOH I see the problem with upgrades. Maybe we should have a way to express additional defaults, per service type. Ie add additional values like: ipakrbAuthsData: nfs:NONE This would be a default but only for services that have the form nfs/*@* This would allow us to avoid magical special casing in the code and allow admins to change the overall default for specific services as needed. Maybe we should add a new ticket for this ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] CA name constrains
On Wed, 2013-02-27 at 08:16 -0500, Simo Sorce wrote: On Wed, 2013-02-27 at 13:55 +0100, Petr Spacek wrote: Hello list, during our last meeting with Simo we discussed support for name constraint extension in CA certificates and clients. The Name Constraints Extensions is defined here: http://tools.ietf.org/html/rfc5280#section-4.2.1.10 Following article could be interesting for you if you like longer stories: Mozilla changes policy to limit risk of subordinate CA certificate abuse Author: Lucian Constantin 19.02.2013 kl 21:50 http://news.idg.no/cw/art.cfm?id=8C9E7CFA-0E65-24B0-1539C891C8F4C09B If I remember correctly, questions were mainly about support on client side and about implications for older clients. I had a chat with Kai Engert (in CC) at DevConf.cz about this, we'll try to work on this as time permits. NSS seem to support this extension but so far we do not have tests covering it apparently. Simo. Btw I opened ticket https://fedorahosted.org/freeipa/ticket/3466 to track this. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 105 Fix remove while iterating in suppress_netgroup_memberof
Hi, this patch fixes https://fedorahosted.org/freeipa/ticket/3464. Honza -- Jan Cholasta From c40f1f123b905fdd0ee4d05d32f3d86e6ffdccc0 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Wed, 27 Feb 2013 14:14:33 +0100 Subject: [PATCH] Fix remove while iterating in suppress_netgroup_memberof. https://fedorahosted.org/freeipa/ticket/3464 --- ipalib/plugins/host.py| 2 +- ipalib/plugins/hostgroup.py | 2 +- tests/test_xmlrpc/test_nesting.py | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py index f464127..4a47f50 100644 --- a/ipalib/plugins/host.py +++ b/ipalib/plugins/host.py @@ -364,7 +364,7 @@ class host(LDAPObject): ng_container = DN(api.env.container_netgroup, api.env.basedn) if 'memberofindirect' in entry_attrs: -for member in entry_attrs['memberofindirect']: +for member in list(entry_attrs['memberofindirect']): memberdn = DN(member) if memberdn.endswith(ng_container): try: diff --git a/ipalib/plugins/hostgroup.py b/ipalib/plugins/hostgroup.py index 7ae438c..9fb1029 100644 --- a/ipalib/plugins/hostgroup.py +++ b/ipalib/plugins/hostgroup.py @@ -99,7 +99,7 @@ class hostgroup(LDAPObject): if 'memberof' in entry_attrs: hgdn = DN(dn) -for member in entry_attrs['memberof']: +for member in list(entry_attrs['memberof']): ngdn = DN(member) if ngdn['cn'] == hgdn['cn']: try: diff --git a/tests/test_xmlrpc/test_nesting.py b/tests/test_xmlrpc/test_nesting.py index a09a798..e1b41b2 100644 --- a/tests/test_xmlrpc/test_nesting.py +++ b/tests/test_xmlrpc/test_nesting.py @@ -790,7 +790,6 @@ class test_nesting(Declarative): managedby_host=[fqdn1], memberof_hostgroup = [u'testhostgroup2'], memberofindirect_hostgroup = [u'testhostgroup1'], -memberofindirect_netgroup = [u'testhostgroup2'], ), ), ), -- 1.8.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] DESIGN: Recover DNA Ranges
Sumit Bose wrote: On Mon, Feb 25, 2013 at 03:12:19PM +0100, Martin Kosek wrote: On 02/25/2013 03:09 PM, Rob Crittenden wrote: Martin Kosek wrote: ... 4) What does NOTE: We will need to be clear that this range has nothing to do with Trust ranges. actually mean? AFAIU, IPA should have all local ranges covered with a local idrange range(s). IPA ranges is completely separate from DNA ranges. You can set/modify all the local ranges you want and it won't affect the UIDs getting assigned. If it does not have it covered, it could happen that for example a new trust would overlap with this user-defined local range and we would have colliding POSIX IDs... Hmm, that's a good point. IMO, dnarange-set and dnanextrange-set should at first check if the range is covered with some local idrange and only then allowed setting the new range. I can do that as well, but again, the local ranges don't really affect the ids we hand out via DNA. rob You are right, that DNA plugin is really not aware of the idranges we set in IPA. But the idrange is still a safeguard for our POSIX IDs to not overlap with trust ranges and I think we should respect that with ipa-replica-manage. I wonder if there was not even a plan to increase cooperation between our idranges and DNA plugin, maybe Sumit or Alexander knows more. If I understand the use case and design properly, it is about re-arranging the sub-ranges each replica can use from the original range, which was given/created at installation time and which is also stored as idrange in DOMAIN.NAME_id_range,cn=ranges,cn=etc,dc=domain,dc=name with objectclass=ipaDomainIDRange. If the re-arrangement does not result in IDs which are outside of this range give by the ipaDomainIDRange object, no conflicts with idranges used by trusted domain will occur, because it is one of the task of the idrange objects to avoid those conflicts. If the original given range is exhausted completely and a completely new DNA range must be created, it should be checked with ipa idrange-find that the new range is not used and a new ipaDomainIDRange object which reserves the local range should be added. There are https://fedorahosted.org/freeipa/ticket/591 which can be used to track the coordinated creation of DNA and id-range. Honestly, I was going to skip the range stuff altogether and just focus on the DNA ranges. I'm primarily interested in saving any range from a server that is going to be deleted. I can save that in the on-base attribute of any existing servers. Where I run into trouble is if all the on-base are populated, then we'd lose range. So I need to alert the user somehow. This leads to wanting to be able to add that back in so I came up with the idea of letting users manage the DNA range directly. It may be possible to check the DOMAIN ranges as well but that bumps the complexity up considerably. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] 0165-0174 LDAP code refactoring (Part 5)
On 31.1.2013 11:03, Petr Viktorin wrote: And hee is another batch of patches. This one is about converting the legacy IPAdmin and raw python-ldap calls to the new wrappers. Patch 165: I have noticed two things that are not really related to your work, but here they are nonetheless: +if self.admin_conn.get_entries( +DN(api.env.container_ranges, self.suffix), +ldap.SCOPE_ONELEVEL, +objectclass=ipaDomainIDRange): Is that a valid filter? +if self.admin_conn.get_entries(cn=accounts, + self.suffix, + ldap.SCOPE_SUBTREE, id_filter): This doesn't seem right as well, why is the DN class not used here? Patch 167: -conn.sasl_interactive_bind_s(None, sasl_auth) +conn.do_sasl_gssapi_bind() sasl_auth is unused after this change, can you please remove it as well (and cb_info too)? -self.sasl_interactive_bind_s, timeout, None, SASL_AUTH) +self.conn.sasl_interactive_bind_s, timeout, None, SASL_AUTH) Again, this is not related to your work, but can we please rename SASL_AUTH to SASL_GSSAPI? Patch 173: -res = con.search_st(str(base), -ldap.SCOPE_SUBTREE, -filterstr=srcfilter, -attrlist=attrs, -timeout=10) +res = con.get_entries(base, con.SCOPE_SUBTREE, srcfilter, attrs) I assume the timeout is there for a reason, can you please keep it? Patch 174: -conn.modify_s( -def_dn, -[(ldap.MOD_REPLACE, -'originfilter', -disable_attr)] -) +entry['originfilter'] = [disable_attr] I think you forgot to call update_entry here. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0007 Web UI: Realm Domains page
Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From a44867fdcd4fda8cce531d689f94f466ca6bb52d Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Wed, 27 Feb 2013 14:49:21 +0100 Subject: [PATCH] Realm Domains page Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 --- install/ui/src/freeipa/app.js | 1 + install/ui/src/freeipa/realmdomains.js | 56 + install/ui/src/freeipa/webui.js | 3 +- install/ui/test/data/ipa_init.json | 3 ++ install/ui/test/data/ipa_init_objects.json | 42 ++ install/ui/test/data/realmdomains_show.json | 24 + ipalib/plugins/internal.py | 3 ++ 7 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 install/ui/src/freeipa/realmdomains.js create mode 100644 install/ui/test/data/realmdomains_show.json diff --git a/install/ui/src/freeipa/app.js b/install/ui/src/freeipa/app.js index 9d89c1aede857ddfc27ebffa306c41172ed56bca..3dcb10f493824923254636c06b715164e419cce5 100644 --- a/install/ui/src/freeipa/app.js +++ b/install/ui/src/freeipa/app.js @@ -41,6 +41,7 @@ define([ './idrange', './netgroup', './policy', +'./realmdomains', './rule', './selinux', './serverconfig', diff --git a/install/ui/src/freeipa/realmdomains.js b/install/ui/src/freeipa/realmdomains.js new file mode 100644 index ..f1dbe506d6931b72f174fc637ab567184e0b0293 --- /dev/null +++ b/install/ui/src/freeipa/realmdomains.js @@ -0,0 +1,56 @@ +/* Authors: + *Ana Krivokapic akriv...@redhat.com + * + * Copyright (C) 2013 Red Hat + * see file 'COPYING' for use and warranty information + * + * 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/. + */ + +define(['./ipa', './jquery', './details', './search', './association', +'./entity'], function (IPA, $) { + +IPA.realmdomains = {}; + +IPA.realmdomains.entity = function (spec) { + +var that = IPA.entity(spec); + +that.init = function () { +that.entity_init(); + +that.builder.details_facet({ +title: IPA.metadata.objects.realmdomains.label, +sections: [ +{ +name: 'identity', +label: IPA.messages.objects.realmdomains.identity, +fields: [ +{ +name: 'associateddomain', +type: 'multivalued' +} +] +} +], +needs_update: true +}); +}; +return that; +}; + +IPA.register('realmdomains', IPA.realmdomains.entity); + +return {}; +}); diff --git a/install/ui/src/freeipa/webui.js b/install/ui/src/freeipa/webui.js index f6c3339ec4b5d3fb8a4cb547407eebf2a19b45af..04b255d8e99f59a777fce6eea230d01b48f52a9f 100644 --- a/install/ui/src/freeipa/webui.js +++ b/install/ui/src/freeipa/webui.js @@ -42,7 +42,8 @@ IPA.admin_navigation = function(spec) { {entity: 'dnsconfig'}, {entity: 'dnsrecord', hidden:true} ] -} +}, +{entity: 'realmdomains'} ]}, {name: 'policy', label: IPA.messages.tabs.policy, children: [ {name: 'hbac', label: IPA.messages.tabs.hbac, children: [ diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json index c16bc992e437e6a5e1be918d46ea5dda33b97562..84663c9cb6c996ba9cf9428548a0cc860fdcd323 100644 --- a/install/ui/test/data/ipa_init.json +++ b/install/ui/test/data/ipa_init.json @@ -380,6 +380,9 @@ type_ad: Active Directory domain, type_local: Local domain }, +realmdomains: { +identity: Realm Domains +}, role: { identity: Role Settings }, diff --git a/install/ui/test/data/ipa_init_objects.json b/install/ui/test/data/ipa_init_objects.json
Re: [Freeipa-devel] [PATCH 0112] Make log messages related to Kerberos more verbose
On 12.2.2013 13:58, Petr Spacek wrote: Hello, Make log messages related to Kerberos more verbose. This change should help people supporting bind-dyndb-ldap to figure out what is happening under covers. Added explanatory error message for case where Kerberos context initialization failed. -- Petr^2 Spacek From 467a5d405f57e2277808c0b33b22480a3167abe4 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 12 Feb 2013 13:49:32 +0100 Subject: [PATCH] Make log messages related to Kerberos more verbose. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/krb5_helper.c | 38 +++--- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index ffa6938d08a37d3470dd9184be2d8ab5c604afdf..25de7f80ee56a6a2c6c6591266edf621914a10b9 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -60,15 +60,15 @@ check_credentials(krb5_context context, krberr = krb5_build_principal(context, mcreds.server, strlen(realm), realm, krbtgt, realm, NULL); - CHECK_KRB5(context, krberr, Failed to build tgt principal); + CHECK_KRB5(context, krberr, Failed to build 'krbtgt/REALM' principal); /* krb5_cc_retrieve_cred filters on both server and client */ mcreds.client = service; krberr = krb5_cc_retrieve_cred(context, ccache, 0, mcreds, creds); if (krberr) { const char * errmsg = krb5_get_error_message(context, krberr); - log_debug(2, Principal not found in cred cache (%s), + log_debug(2, Credentials are not present in cache (%s), errmsg); krb5_free_error_message(context, errmsg); result = ISC_R_FAILURE; @@ -79,7 +79,7 @@ check_credentials(krb5_context context, CHECK_KRB5(context, krberr, Failed to get timeofday); if (now (creds.times.endtime + MIN_TIME)) { - log_debug(2, Credentials expired); + log_debug(2, Credentials in cache expired); result = ISC_R_FAILURE; goto cleanup; } @@ -123,42 +123,46 @@ get_krb5_tgt(isc_mem_t *mctx, const char *principal, const char *keyfile) } krberr = krb5_init_context(context); - if (krberr) { - log_error(Failed to init kerberos context); - return ISC_R_FAILURE; - } + /* This will blow up with older versions of Heimdal Kerberos, but + * this kind of errors are not debuggable without any error message. + * http://mailman.mit.edu/pipermail/kerberos/2013-February/018720.html */ + CHECK_KRB5(NULL, krberr, Kerberos context initialization failed); /* get credentials cache */ CHECK(str_new(mctx, ccname)); CHECK(str_sprintf(ccname, MEMORY:_ld_krb5_cc_%s, principal)); ret = setenv(KRB5CCNAME, str_buf(ccname), 1); if (ret == -1) { - log_error(Failed to set KRB5CCNAME environment variable); + log_error(Failed to set KRB5CCNAME environment variable to + '%s', str_buf(ccname)); result = ISC_R_FAILURE; goto cleanup; } krberr = krb5_cc_resolve(context, str_buf(ccname), ccache); CHECK_KRB5(context, krberr, - Failed to resolve ccache name %s, str_buf(ccname)); + Failed to resolve credentials cache name '%s', + str_buf(ccname)); /* get krb5_principal from string */ krberr = krb5_parse_name(context, principal, kprincpw); CHECK_KRB5(context, krberr, - Failed to parse the principal name %s, principal); + Failed to parse the principal name '%s', principal); /* check if we already have valid credentials */ result = check_credentials(context, ccache, kprincpw); if (result == ISC_R_SUCCESS) { - log_debug(2, Found valid cached credentials); + log_debug(2, Found valid Kerberos credentials in cache); goto cleanup; + } else { + log_debug(2, Attempting to acquire new Kerberos credentials); } /* open keytab */ krberr = krb5_kt_resolve(context, keyfile, keytab); CHECK_KRB5(context, krberr, - Failed to resolve keytab file %s, keyfile); + Failed to resolve keytab file '%s', keyfile); memset(my_creds, 0, sizeof(my_creds)); memset(options, 0, sizeof(options)); @@ -170,15 +174,19 @@ get_krb5_tgt(isc_mem_t *mctx, const char *principal, const char *keyfile) /* get tgt */ krberr = krb5_get_init_creds_keytab(context, my_creds, kprincpw, keytab, 0, NULL, options); - CHECK_KRB5(context, krberr, Failed to init credentials); + CHECK_KRB5(context, krberr, Failed to get initial credentials (TGT) +using principal '%s' and keytab '%s', +principal, keyfile); my_creds_ptr = my_creds; /* store credentials in cache */ krberr = krb5_cc_initialize(context, ccache, kprincpw); - CHECK_KRB5(context, krberr, Failed to initialize ccache); + CHECK_KRB5(context, krberr, Failed to initialize credentials cache +'%s', str_buf(ccname)); krberr = krb5_cc_store_cred(context, ccache, my_creds); - CHECK_KRB5(context, krberr, Failed to store ccache); + CHECK_KRB5(context, krberr, Failed to store credentials +in credentials cache '%s', str_buf(ccname)); result = ISC_R_SUCCESS; -- 1.7.11.7
[Freeipa-devel] [PATCHES] 106-113 Access raw LDAP values directly from LDAPEntry
Hi, these patches add the ability to access and manipulate raw attribute values as they are returned from python-ldap to the LDAPEntry class. This is useful for comparing entries, computing modlists for the modify operation, deleting values that don't match the syntax of an attribute, etc., because we don't need to care what syntax does a particular attribute use, or what Python type is used for it in the framework (raw values are always list of str). It should also make interaction with non-389 DS LDAP servers easier in the framework. (It might be too late for this kind of changes to get into 3.2 now, I'm posting these patches mainly so that you are aware that they exist.) Honza -- Jan Cholasta From b365ef78e5f784661261cba1c51f24703d5a3437 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Tue, 26 Feb 2013 11:27:55 +0100 Subject: [PATCH 1/8] Make LDAPEntry a wrapper around dict rather than a dict subclass. --- ipaserver/ipaldap.py | 100 --- 1 file changed, 64 insertions(+), 36 deletions(-) diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py index bdc2d44..da9a60d 100644 --- a/ipaserver/ipaldap.py +++ b/ipaserver/ipaldap.py @@ -582,8 +582,8 @@ class IPASimpleLDAPObject(object): # r = result[0] # r[0] == r.dn # r[1] == r.data -class LDAPEntry(dict): -__slots__ = ('_conn', '_dn', '_names', '_orig') +class LDAPEntry(object): +__slots__ = ('_conn', '_dn', '_names', '_data', '_orig') def __init__(self, _conn, _dn=None, _obj=None, **kwargs): @@ -601,8 +601,6 @@ class LDAPEntry(dict): Keyword arguments can be used to override values of specific attributes. -super(LDAPEntry, self).__init__() - if isinstance(_conn, LDAPEntry): assert _dn is None _dn = _conn @@ -611,13 +609,15 @@ class LDAPEntry(dict): if isinstance(_dn, LDAPEntry): assert _obj is None _obj = _dn -_dn = DN(_dn._dn) +_dn = _dn._dn if isinstance(_obj, LDAPEntry): +data = dict(_obj._data) orig = _obj._orig else: if _obj is None: _obj = {} +data = {} orig = self assert isinstance(_conn, IPASimpleLDAPObject) @@ -625,8 +625,9 @@ class LDAPEntry(dict): self._conn = _conn self._dn = _dn -self._orig = orig self._names = CIDict() +self._data = data +self._orig = orig self.update(_obj, **kwargs) @@ -655,8 +656,7 @@ class LDAPEntry(dict): return self._orig def __repr__(self): -return '%s(%r, %s)' % (type(self).__name__, self._dn, -super(LDAPEntry, self).__repr__()) +return '%s(%r, %r)' % (type(self).__name__, self._dn, self._data) def copy(self): return LDAPEntry(self) @@ -664,11 +664,8 @@ class LDAPEntry(dict): def clone(self): result = LDAPEntry(self._conn, self._dn) -for name in self.iterkeys(): -super(LDAPEntry, result).__setitem__( -name, deepcopy(super(LDAPEntry, self).__getitem__(name))) - result._names = deepcopy(self._names) +result._data = deepcopy(self._data) if self._orig is not self: result._orig = self._orig.clone() @@ -704,7 +701,7 @@ class LDAPEntry(dict): if keyname == oldname: self._names[altname] = name -super(LDAPEntry, self).__delitem__(oldname) +del self._data[oldname] else: self._names[name] = name @@ -720,7 +717,7 @@ class LDAPEntry(dict): altname = altname.decode('utf-8') self._names[altname] = name -super(LDAPEntry, self).__setitem__(name, value) +self._data[name] = value def setdefault(self, name, default): if name not in self: @@ -737,6 +734,9 @@ class LDAPEntry(dict): name = self._names[name] return name +def _get(self, name): +return self._data[self._get_attr_name(name)] + def __getitem__(self, name): # for python-ldap tuple compatibility if name == 0: @@ -744,16 +744,14 @@ class LDAPEntry(dict): elif name == 1: return self -return super(LDAPEntry, self).__getitem__(self._get_attr_name(name)) +return self._get(name) def get(self, name, default=None): try: -name = self._get_attr_name(name) +return self._get(name) except KeyError: return default -return super(LDAPEntry, self).get(name, default) - def single_value(self, name, default=_missing): Return a single attribute value @@ -763,8 +761,7 @@ class LDAPEntry(dict): If the entry is missing and default is not given, raise KeyError.
Re: [Freeipa-devel] Using the new LDAP code
On 02/27/2013 06:46 AM, Petr Viktorin wrote: Hello, A big refactoring of our LDAP code should be merged soon-ish now. Here's a summary for developers. Great, that's fabulous news and thanks for the good work! IPA plugins traditionally use (dn, entry_attrs) pairs to represent entries. To make that work, iterating over an LDAPEntry will, for now, yield the DN and the entry itself. Always use keys() or values() when iterating over an entry. I'm trying parse what the above means, it seems to be one of the two: 1) There is still a bunch of code that continues to use 2-tuples in the plugins and additional work remains to converge on a single API. 2) All the code that used 2-tuples has been cleaned up and expunged, however if the old 2-tuple methodology was used it would still work. -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0007 Web UI: Realm Domains page
On 02/27/2013 04:20 PM, Ana Krivokapic wrote: Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 The patch looks good, but there is a issue we don't have a precedence for. The mod command is doing dns check for new domains. Currently we can't specify --force option to bypass the check. I see two possible implementations: 1) On update, when user adds or modifies the values, a dialog would pop up and ask user whether he wants to force it. 2) Another option is to disable edit on the list(deletion would be still allowed) and move the add operation to separate action in action list. I prefer the former. Latter might have issues with two modifications (delete and add) at the same time at two different places (facet and add dialog). -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Using the new LDAP code
Hi, On 27.2.2013 17:09, John Dennis wrote: IPA plugins traditionally use (dn, entry_attrs) pairs to represent entries. To make that work, iterating over an LDAPEntry will, for now, yield the DN and the entry itself. Always use keys() or values() when iterating over an entry. I'm trying parse what the above means, it seems to be one of the two: 1) There is still a bunch of code that continues to use 2-tuples in the plugins and additional work remains to converge on a single API. 2) All the code that used 2-tuples has been cleaned up and expunged, however if the old 2-tuple methodology was used it would still work. it's the former, there is still code that uses 2-tuples. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Using the new LDAP code
On 02/27/2013 11:23 AM, Jan Cholasta wrote: Hi, On 27.2.2013 17:09, John Dennis wrote: IPA plugins traditionally use (dn, entry_attrs) pairs to represent entries. To make that work, iterating over an LDAPEntry will, for now, yield the DN and the entry itself. Always use keys() or values() when iterating over an entry. I'm trying parse what the above means, it seems to be one of the two: 1) There is still a bunch of code that continues to use 2-tuples in the plugins and additional work remains to converge on a single API. 2) All the code that used 2-tuples has been cleaned up and expunged, however if the old 2-tuple methodology was used it would still work. it's the former, there is still code that uses 2-tuples. O.K. so we're still a ways away from completing the task. Would this be a correct summary? Phase 1 is completed, a consistent API has been defined and implemented. Phase 2 is converting all the code to use the API defined in Phase 1, a task yet to be done. -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Using the new LDAP code
On 27.2.2013 18:14, John Dennis wrote: On 02/27/2013 11:23 AM, Jan Cholasta wrote: Hi, On 27.2.2013 17:09, John Dennis wrote: IPA plugins traditionally use (dn, entry_attrs) pairs to represent entries. To make that work, iterating over an LDAPEntry will, for now, yield the DN and the entry itself. Always use keys() or values() when iterating over an entry. I'm trying parse what the above means, it seems to be one of the two: 1) There is still a bunch of code that continues to use 2-tuples in the plugins and additional work remains to converge on a single API. 2) All the code that used 2-tuples has been cleaned up and expunged, however if the old 2-tuple methodology was used it would still work. it's the former, there is still code that uses 2-tuples. O.K. so we're still a ways away from completing the task. Would this be a correct summary? Phase 1 is completed, a consistent API has been defined and implemented. Phase 2 is converting all the code to use the API defined in Phase 1, a task yet to be done. Correct. But I don't think converting 2-tuples to the new API will be a huge task. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Using the new LDAP code
On 02/27/2013 12:22 PM, Jan Cholasta wrote: On 27.2.2013 18:14, John Dennis wrote: On 02/27/2013 11:23 AM, Jan Cholasta wrote: Hi, On 27.2.2013 17:09, John Dennis wrote: IPA plugins traditionally use (dn, entry_attrs) pairs to represent entries. To make that work, iterating over an LDAPEntry will, for now, yield the DN and the entry itself. Always use keys() or values() when iterating over an entry. I'm trying parse what the above means, it seems to be one of the two: 1) There is still a bunch of code that continues to use 2-tuples in the plugins and additional work remains to converge on a single API. 2) All the code that used 2-tuples has been cleaned up and expunged, however if the old 2-tuple methodology was used it would still work. it's the former, there is still code that uses 2-tuples. O.K. so we're still a ways away from completing the task. Would this be a correct summary? Phase 1 is completed, a consistent API has been defined and implemented. Phase 2 is converting all the code to use the API defined in Phase 1, a task yet to be done. Correct. But I don't think converting 2-tuples to the new API will be a huge task. Cool! -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] DESIGN: Recover DNA Ranges
On Wed, Feb 27, 2013 at 09:50:21AM -0500, Rob Crittenden wrote: Sumit Bose wrote: On Mon, Feb 25, 2013 at 03:12:19PM +0100, Martin Kosek wrote: On 02/25/2013 03:09 PM, Rob Crittenden wrote: Martin Kosek wrote: ... 4) What does NOTE: We will need to be clear that this range has nothing to do with Trust ranges. actually mean? AFAIU, IPA should have all local ranges covered with a local idrange range(s). IPA ranges is completely separate from DNA ranges. You can set/modify all the local ranges you want and it won't affect the UIDs getting assigned. If it does not have it covered, it could happen that for example a new trust would overlap with this user-defined local range and we would have colliding POSIX IDs... Hmm, that's a good point. IMO, dnarange-set and dnanextrange-set should at first check if the range is covered with some local idrange and only then allowed setting the new range. I can do that as well, but again, the local ranges don't really affect the ids we hand out via DNA. rob You are right, that DNA plugin is really not aware of the idranges we set in IPA. But the idrange is still a safeguard for our POSIX IDs to not overlap with trust ranges and I think we should respect that with ipa-replica-manage. I wonder if there was not even a plan to increase cooperation between our idranges and DNA plugin, maybe Sumit or Alexander knows more. If I understand the use case and design properly, it is about re-arranging the sub-ranges each replica can use from the original range, which was given/created at installation time and which is also stored as idrange in DOMAIN.NAME_id_range,cn=ranges,cn=etc,dc=domain,dc=name with objectclass=ipaDomainIDRange. If the re-arrangement does not result in IDs which are outside of this range give by the ipaDomainIDRange object, no conflicts with idranges used by trusted domain will occur, because it is one of the task of the idrange objects to avoid those conflicts. If the original given range is exhausted completely and a completely new DNA range must be created, it should be checked with ipa idrange-find that the new range is not used and a new ipaDomainIDRange object which reserves the local range should be added. There are https://fedorahosted.org/freeipa/ticket/591 which can be used to track the coordinated creation of DNA and id-range. Honestly, I was going to skip the range stuff altogether and just focus on the DNA ranges. I'm primarily interested in saving any range from a server that is going to be deleted. I can save that in the on-base attribute of any existing servers. Where I run into trouble is if all the on-base are populated, then we'd lose range. So I need to alert the user somehow. If it is only about moving existing DNA range it is fine, because this means that DNA will not give ID which are outside of the idrange of the IPA domain which is created at installation or during upgrade to v3. This leads to wanting to be able to add that back in so I came up with the idea of letting users manage the DNA range directly. It may be possible to check the DOMAIN ranges as well but that bumps the complexity up considerably. But it looks like dnarange-set and dnanextrange-set can also be used to not only move existing DNA ranges but to create new DNA ranges which will lead to ID which are not in the idrange of the IPA domain but might be in an idrange which is used by a trusted domain. So I think a check and a warning are desirable. bye, Sumit rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] DESIGN: Recover DNA Ranges
Sumit Bose wrote: On Wed, Feb 27, 2013 at 09:50:21AM -0500, Rob Crittenden wrote: Sumit Bose wrote: On Mon, Feb 25, 2013 at 03:12:19PM +0100, Martin Kosek wrote: On 02/25/2013 03:09 PM, Rob Crittenden wrote: Martin Kosek wrote: ... 4) What does NOTE: We will need to be clear that this range has nothing to do with Trust ranges. actually mean? AFAIU, IPA should have all local ranges covered with a local idrange range(s). IPA ranges is completely separate from DNA ranges. You can set/modify all the local ranges you want and it won't affect the UIDs getting assigned. If it does not have it covered, it could happen that for example a new trust would overlap with this user-defined local range and we would have colliding POSIX IDs... Hmm, that's a good point. IMO, dnarange-set and dnanextrange-set should at first check if the range is covered with some local idrange and only then allowed setting the new range. I can do that as well, but again, the local ranges don't really affect the ids we hand out via DNA. rob You are right, that DNA plugin is really not aware of the idranges we set in IPA. But the idrange is still a safeguard for our POSIX IDs to not overlap with trust ranges and I think we should respect that with ipa-replica-manage. I wonder if there was not even a plan to increase cooperation between our idranges and DNA plugin, maybe Sumit or Alexander knows more. If I understand the use case and design properly, it is about re-arranging the sub-ranges each replica can use from the original range, which was given/created at installation time and which is also stored as idrange in DOMAIN.NAME_id_range,cn=ranges,cn=etc,dc=domain,dc=name with objectclass=ipaDomainIDRange. If the re-arrangement does not result in IDs which are outside of this range give by the ipaDomainIDRange object, no conflicts with idranges used by trusted domain will occur, because it is one of the task of the idrange objects to avoid those conflicts. If the original given range is exhausted completely and a completely new DNA range must be created, it should be checked with ipa idrange-find that the new range is not used and a new ipaDomainIDRange object which reserves the local range should be added. There are https://fedorahosted.org/freeipa/ticket/591 which can be used to track the coordinated creation of DNA and id-range. Honestly, I was going to skip the range stuff altogether and just focus on the DNA ranges. I'm primarily interested in saving any range from a server that is going to be deleted. I can save that in the on-base attribute of any existing servers. Where I run into trouble is if all the on-base are populated, then we'd lose range. So I need to alert the user somehow. If it is only about moving existing DNA range it is fine, because this means that DNA will not give ID which are outside of the idrange of the IPA domain which is created at installation or during upgrade to v3. This leads to wanting to be able to add that back in so I came up with the idea of letting users manage the DNA range directly. It may be possible to check the DOMAIN ranges as well but that bumps the complexity up considerably. But it looks like dnarange-set and dnanextrange-set can also be used to not only move existing DNA ranges but to create new DNA ranges which will lead to ID which are not in the idrange of the IPA domain but might be in an idrange which is used by a trusted domain. So I think a check and a warning are desirable. That is an excellent point. I've updated the design. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, Feb 27, 2013 at 06:48:27PM +0100, Sumit Bose wrote: On Wed, Feb 27, 2013 at 08:37:18AM -0500, Simo Sorce wrote: On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: 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. Is there a good reason to allow insensitive matching ? in LDAP you can't use true or false either for booleans, you have to use TRUE and FALSE, so I am not so thrilled to allow insensitive matching for this option as well. I'm fine with this. Alexander, any comments. Alexander said on irc that he is fine with the strict case-sensitive handling of the values, as long as the user input is automatically upper-cased by the framework as it is currently done for booleans. Since the WebUI does not allow user entered values this will only affect the CLI. bye, Sumit 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. I do not think we should set the policy to NONE by default, OTOH I see the problem with upgrades. Maybe we should have a way to express additional defaults, per service type. Ie add additional values like: ipakrbAuthsData: nfs:NONE This would be a default but only for services that have the form nfs/*@* This would allow us to avoid magical special casing in the code and allow admins to change the overall default for specific services as needed. Maybe we should add a new ticket for this ? This sounds like a good compromise and will make updating much easier. If we agree to do it this way I can add ipakrbAuthsData: nfs:NONE and fix the code with this ticket. But we would need tickets for the CLI and WebUI to handle this new case. For the WebUI there already is https://fedorahosted.org/freeipa/ticket/3404 , maybe it can be extended to handle this new case as well? As for design documents I think I can added the needed information to http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . bye, Sumit Simo. -- Simo Sorce * Red Hat, Inc * New York
Re: [Freeipa-devel] DESIGN: Recover DNA Ranges
Sumit Bose wrote: On Wed, Feb 27, 2013 at 02:03:24PM -0500, Rob Crittenden wrote: Sumit Bose wrote: But it looks like dnarange-set and dnanextrange-set can also be used to not only move existing DNA ranges but to create new DNA ranges which will lead to ID which are not in the idrange of the IPA domain but might be in an idrange which is used by a trusted domain. So I think a check and a warning are desirable. That is an excellent point. I've updated the design. Thank you. I would like to suggest a slight edit. Instead of That doesn't remove our responsibility to not test for overlaps in the idranges though. We will need to verify that the manual configuration changes do not overlap with all ranges in cn=ranges,cn=etc,$SUFFIX. I would say That doesn't remove our responsibility to not test for overlaps in the idranges though. We will need to verify that the manual configuration changes do * not overlap with ranges from trusted domains (objectclass=ipatrustedaddomainrange) in cn=ranges,cn=etc,$SUFFIX, or reject the change otherwise. * overlap completely with ranges from the local IPA domain (objectclass=ipaDomainIDRange) in cn=ranges,cn=etc,$SUFFIX, or give a warning otherwise which asks the user to add a new suitable idrange. Codewise the logic could be: - check if the new range is a subset of a local idrange, if yes, all is fine - if not, check if it overlaps with an idrange of a trusted domain, if yes, reject - if not, give a warning and ask to add a new idrange for the local domain I took this almost verbatim but I'm more harsh. If the range is outside the local range and not overlapping anything I will still reject it. If we're going to go through the trouble of keeping them in sync we should be consistent. thanks rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] DESIGN: Recover DNA Ranges
On Wed, 2013-02-27 at 15:00 -0500, Rob Crittenden wrote: Sumit Bose wrote: On Wed, Feb 27, 2013 at 02:03:24PM -0500, Rob Crittenden wrote: Sumit Bose wrote: But it looks like dnarange-set and dnanextrange-set can also be used to not only move existing DNA ranges but to create new DNA ranges which will lead to ID which are not in the idrange of the IPA domain but might be in an idrange which is used by a trusted domain. So I think a check and a warning are desirable. That is an excellent point. I've updated the design. Thank you. I would like to suggest a slight edit. Instead of That doesn't remove our responsibility to not test for overlaps in the idranges though. We will need to verify that the manual configuration changes do not overlap with all ranges in cn=ranges,cn=etc,$SUFFIX. I would say That doesn't remove our responsibility to not test for overlaps in the idranges though. We will need to verify that the manual configuration changes do * not overlap with ranges from trusted domains (objectclass=ipatrustedaddomainrange) in cn=ranges,cn=etc,$SUFFIX, or reject the change otherwise. * overlap completely with ranges from the local IPA domain (objectclass=ipaDomainIDRange) in cn=ranges,cn=etc,$SUFFIX, or give a warning otherwise which asks the user to add a new suitable idrange. Codewise the logic could be: - check if the new range is a subset of a local idrange, if yes, all is fine - if not, check if it overlaps with an idrange of a trusted domain, if yes, reject - if not, give a warning and ask to add a new idrange for the local domain I took this almost verbatim but I'm more harsh. If the range is outside the local range and not overlapping anything I will still reject it. If the local ranges ^^ the admin may have already added additional ones, so it needs to be plural. we're going to go through the trouble of keeping them in sync we should be consistent. Otherwise good plan. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] DESIGN: Recover DNA Ranges
On Wed, Feb 27, 2013 at 03:00:10PM -0500, Rob Crittenden wrote: Sumit Bose wrote: On Wed, Feb 27, 2013 at 02:03:24PM -0500, Rob Crittenden wrote: Sumit Bose wrote: But it looks like dnarange-set and dnanextrange-set can also be used to not only move existing DNA ranges but to create new DNA ranges which will lead to ID which are not in the idrange of the IPA domain but might be in an idrange which is used by a trusted domain. So I think a check and a warning are desirable. That is an excellent point. I've updated the design. Thank you. I would like to suggest a slight edit. Instead of That doesn't remove our responsibility to not test for overlaps in the idranges though. We will need to verify that the manual configuration changes do not overlap with all ranges in cn=ranges,cn=etc,$SUFFIX. I would say That doesn't remove our responsibility to not test for overlaps in the idranges though. We will need to verify that the manual configuration changes do * not overlap with ranges from trusted domains (objectclass=ipatrustedaddomainrange) in cn=ranges,cn=etc,$SUFFIX, or reject the change otherwise. * overlap completely with ranges from the local IPA domain (objectclass=ipaDomainIDRange) in cn=ranges,cn=etc,$SUFFIX, or give a warning otherwise which asks the user to add a new suitable idrange. Codewise the logic could be: - check if the new range is a subset of a local idrange, if yes, all is fine - if not, check if it overlaps with an idrange of a trusted domain, if yes, reject - if not, give a warning and ask to add a new idrange for the local domain I took this almost verbatim but I'm more harsh. If the range is outside the local range and not overlapping anything I will still reject it. If we're going to go through the trouble of keeping them in sync we should be consistent. even better, thank you. bye, Sumit thanks rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On 02/27/2013 06:48 PM, Sumit Bose wrote: On Wed, Feb 27, 2013 at 08:37:18AM -0500, Simo Sorce wrote: On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: 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. Is there a good reason to allow insensitive matching ? in LDAP you can't use true or false either for booleans, you have to use TRUE and FALSE, so I am not so thrilled to allow insensitive matching for this option as well. I'm fine with this. Alexander, any comments. 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. I do not think we should set the policy to NONE by default, OTOH I see the problem with upgrades. Maybe we should have a way to express additional defaults, per service type. Ie add additional values like: ipakrbAuthsData: nfs:NONE This would be a default but only for services that have the form nfs/*@* This would allow us to avoid magical special casing in the code and allow admins to change the overall default for specific services as needed. Maybe we should add a new ticket for this ? This sounds like a good compromise and will make updating much easier. If we agree to do it this way I can add ipakrbAuthsData: nfs:NONE and fix the code with this ticket. But we would need tickets for the CLI and WebUI to handle this new case. For the WebUI there already is https://fedorahosted.org/freeipa/ticket/3404 , maybe it can be extended to handle this new case as well? As for design documents I think I can added the needed information to http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . bye, Sumit 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 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com