Re: [Freeipa-devel] [PATCH] Add DRM to IPA
On 04/07/2014 10:40 PM, Rob Crittenden wrote: Ade Lee wrote: This patch adds the capability of installing a Dogtag DRM to an IPA instance. With this patch, when ipa-server-install is run, a Dogtag CA and a Dogtag DRM are created. The DRM shares the same tomcat instance and DS instance as the Dogtag CA. Moreover, the same admin user/agent (and agent cert) can be used for both subsystems. Certmonger is also confgured to monitor the new subsystem certificates. It is also possible to clone the DRM. When the IPA instance is cloned, if --enable-ca and --enable-drm are specified, the DRM is cloned as well. Installing a DRM requires the user to have a Dogtag CA instance. We can look into possibly relaxing that requirement in a later patch. I am still working on patches for a ipa-drm-install script, which would be used to add a DRM to an existing master (that includes a dogtag CA), or an existing clone. Please review, Thanks, Ade Yikes, I wonder if the changes to ipaserver/install/cainstance.py should be pushed ASAP. Oops, looks like a change that should go to IPA 3.3.x. What is the implication? freeipa-spec.in needs a dependency on pki-kra. Let us stop here. Please see a following RFE I filed: https://fedorahosted.org/freeipa/ticket/4058 I would prefer it KRA files and specifics would be in a new subpackage like freeipa-server-kra. Otherwise we will need to rework it again when we would be splitting CA to freeipa-server-pki in 4.1. I would prefer to start the right modularization now as I do not think that every FreeIPA server needs to run CA/KRA, i.e. it does not need to have the bits installed either. I am also quite worried about the duplication that the new drminstance.py introduces. There is a lot of functions which do more or less the same thing and have most of the handling code the same with only a very small and predictable pki/kra change. For example __http_proxy function seems to be exactly the same. It would be great to avoid this duplication and rather have some common ground utilized by both PKI and KRA. Otherwise it will be very difficult to maintain the new code. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] questions regarding ldap schema for pkcs11
On 7.4.2014 15:07, Rob Crittenden wrote: Simo Sorce wrote: On Fri, 2014-04-04 at 13:19 +0200, Petr Spacek wrote: On 4.4.2014 10:20, Ludwig Krispenz wrote: In the review discussion for the ldap schema for pkcs11 there was one topic, which we wanted to get the opinion from a broader audience before making a final decision. I'll add my opinion for the record: In pkcs11 there are many boolean attributes, like CKA_EXTRACTABLE, CKA_DERIVE, CKA_VERIFY and there are two suggestions how to represent them in ldap. 1] one ldap attribute for each pkcs11 attribute. This was my initial proposal to define a ldap attribute with boolean syntax. Most attributes have default values and need not to be present example: pkcs11extractable: true pkcs11derive: false pkcs11verify: true 2] one ldap attribute with pkcs11 attributes as values During the review Simo suggested to have a single attribute (or a few of them, key,cert,...) and for each pkcs11 attribute with value true add it as a value example: pkcs11keyFlags: CKA_EXTRACTABLE pkcs11keyFlags: CKA_VERIFY Pros Cons pro 1] : one ldap attribute for each pkcs11 attribute. * direct mapping of pkcs11attributes * required or allowed attributes are defined in an objectclass con 1]: * huge number of schema attributes, which will probably not be needed I don't think it is a problem. We have *huge* schema full of almost never-used attributes. Look at printerAbstract objectClass ... pro 2]: one ldap attribute with pkcs11 attributes as values * smaller schema definition IPA schema + all the RFCs created a huge pile of schema definitions already and 389 can cope with it. (We are speaking about adding tens of attributes, not hundreds or thousands!) * possible to add new attributes/flags without extending the schema Schema change is a little problem in comparison with updating clients (to get any value from the new flag). Note that we are talking about booleans defined by PKCS#11 standard so we can't add any boolean anyway. IMHO any IPA-specific booleans should go to a separate object class to separate them from pure PKCS#11 schema. con 2]: * no input validation, application could set undefined flags * since presence of a flag means TRUE, and absence FALSE all default true values need to be present To conclude it - I like approach [1]: One ldap attribute for each pkcs11 attribute. After much consideration I think one attribute per boolean is ok, I think the most convincing argument came from Honza when he said sometimes the default may depend on additional factors not explicit in the object, in that case setting or not setting a string would always be wrong and we would need to end up with additional definitions like: CKA_VERIFY_TRUE/CKA_VERIFY_FALSE as opposed to them missing which could indicate default (or we end up adding also CKA_VERIFY_DEFAULT ...). I prefer the one-per-boolean as well and for the same reason: it doesn't require magic values defined elsewhere. Over the weekend I prepared a great argument about this and look, I am sick for one day and suddenly don't have to post it anymore :-) Glad we reached an agreement on this. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 09:50, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) I don't have a strong opinion on this, but I think a constant *is* useful, if we can agree on a good name for it. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 10:09, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. IMO this is an overkill. I would prefer if we did what I suggested earlier: put DNSName into a new module dnsutil and make the constants global for the module. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:09, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. IMO this is an overkill. I would prefer if we did what I suggested earlier: put DNSName into a new module dnsutil and make the constants global for the module. That would work too. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 10:14, Jan Cholasta wrote: On 8.4.2014 10:09, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. IMO this is an overkill. I would prefer if we did what I suggested earlier: put DNSName into a new module dnsutil and make the constants global for the module. I consider this less readable but I'm not Python expert so I'm fine this approach. Note: The difference between @ and real origin is like pointer to origin and origin value in C. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 10:19, Petr Spacek wrote: On 8.4.2014 10:14, Jan Cholasta wrote: On 8.4.2014 10:09, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. IMO this is an overkill. I would prefer if we did what I suggested earlier: put DNSName into a new module dnsutil and make the constants global for the module. I consider this less readable but I'm not Python expert so I'm fine this approach. Well, it's the same thing python-dns does: dns.name.Name, dns.name.root, ... Note: The difference between @ and real origin is like pointer to origin and origin value in C. I think a good name would be self, but that doesn't work very well in Python :-) Perhaps self_origin is better? -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 10:29, Jan Cholasta wrote: On 8.4.2014 10:19, Petr Spacek wrote: On 8.4.2014 10:14, Jan Cholasta wrote: On 8.4.2014 10:09, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. IMO this is an overkill. I would prefer if we did what I suggested earlier: put DNSName into a new module dnsutil and make the constants global for the module. I consider this less readable but I'm not Python expert so I'm fine this approach. Well, it's the same thing python-dns does: dns.name.Name, dns.name.root, ... Note: The difference between @ and real origin is like pointer to origin and origin value in C. I think a good name would be self, but that doesn't work very well in Python :-) Perhaps self_origin is better? I'm not big fan of SELF :-) What is wrong with origin_sign? It seems more understandable than self_origin to me. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0507 Allow anonymous read access to containers
On 04/07/2014 05:00 PM, Simo Sorce wrote: On Mon, 2014-04-07 at 16:43 +0200, Martin Kosek wrote: On 04/03/2014 01:34 PM, Petr Viktorin wrote: Hello, This adds anonymous read access to containers, as discussed in this thread: https://www.redhat.com/archives/freeipa-devel/2014-March/msg00442.html Additionally access is granted for $SUFFIX itself with targetfilter (objectclass=domain), and attributes objectclass, dc, info, nisDomain, associatedDomain. These are raw ACIs, not permission-based ones. Starting a new sub-thread to differential from the LDIF/update file fixes. I tested the new ACI and it worked ok for me (is a prerequisite for easy testing of the subsequent ACI patches). I assume you plan to handle cn=etc tree in other patch. ACK from me in that case (not pushing right now to let Simo raise any concerns he may have). Thanks, pushed to master: 0e659983a6454370021a748d7534cad9febd6cc1 Martin I do not have any concern on the ACI itself, I only mused about ldif +update vs update only, sorry if I gave the worng impression. Simo. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 10:31, Petr Spacek wrote: On 8.4.2014 10:29, Jan Cholasta wrote: On 8.4.2014 10:19, Petr Spacek wrote: On 8.4.2014 10:14, Jan Cholasta wrote: On 8.4.2014 10:09, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. IMO this is an overkill. I would prefer if we did what I suggested earlier: put DNSName into a new module dnsutil and make the constants global for the module. I consider this less readable but I'm not Python expert so I'm fine this approach. Well, it's the same thing python-dns does: dns.name.Name, dns.name.root, ... Note: The difference between @ and real origin is like pointer to origin and origin value in C. I think a good name would be self, but that doesn't work very well in Python :-) Perhaps self_origin is better? I'm not big fan of SELF :-) What is wrong with origin_sign? It seems more understandable than self_origin to me. The word sign does not really capture the meaning of it. It is a sign as long as you look at it in text form, but it has a specific meaning and IMO we should capture that meaning in the name rather than what it looks like in text. The constant is named empty in python-dns, IMO we should name it the same. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0029-0046] Internationalized domain names in DNS plugin
On 8.4.2014 10:49, Jan Cholasta wrote: On 8.4.2014 10:31, Petr Spacek wrote: On 8.4.2014 10:29, Jan Cholasta wrote: On 8.4.2014 10:19, Petr Spacek wrote: On 8.4.2014 10:14, Jan Cholasta wrote: On 8.4.2014 10:09, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Jan Cholasta wrote: On 8.4.2014 10:01, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Petr Spacek wrote: On 8.4.2014 09:22, Jan Cholasta wrote: On 4.4.2014 12:59, Petr Spacek wrote: On 3.4.2014 15:35, Jan Cholasta wrote: I would shorten origin_sign to just sign. Sign of what? Decay? :-) I don't think that sign is descriptive enough, I would personally stick with origin_sign. Whoops, I meant origin. The _sign bit seems a little bit redundant to me. Origin is an established term for the name of the parent. name = blabla.example. origin of name = example. Anyway, I still think that DNSName('@') is better than any constant with cryptic name. Honza, can you see any problem with this? I know this creates instance again and again, but is it a real problem? I would like to avoid premature optimization... :-) What about making all these fixed names as constants and then simply return those? class DNSName: DNS_ORIGIN = DNSName('@') ... @staticmethod ... def ... return DNS_ORIGIN they would be singletons... Unfortunately you can't do that, because at the time DNSName is parsed, it is not defined yet: class X: ... x = X() ... Traceback (most recent call last): File stdin, line 1, in module File stdin, line 2, in X NameError: name 'X' is not defined Then we can split these classes: define DNSName with purely static methods as child of DNSNameBase which would have all the expected methods. DNSName can then have constants of DNSNameBase() and return those. IMO this is an overkill. I would prefer if we did what I suggested earlier: put DNSName into a new module dnsutil and make the constants global for the module. I consider this less readable but I'm not Python expert so I'm fine this approach. Well, it's the same thing python-dns does: dns.name.Name, dns.name.root, ... Note: The difference between @ and real origin is like pointer to origin and origin value in C. I think a good name would be self, but that doesn't work very well in Python :-) Perhaps self_origin is better? I'm not big fan of SELF :-) What is wrong with origin_sign? It seems more understandable than self_origin to me. The word sign does not really capture the meaning of it. It is a sign as long as you look at it in text form, but it has a specific meaning and IMO we should capture that meaning in the name rather than what it looks like in text. The constant is named empty in python-dns, IMO we should name it the same. Okay, use whatever and please add comment # invented by jcholast :-) -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0504 Default read ACIs for Sudo objects
On 04/07/2014 01:30 PM, Martin Kosek wrote: On 04/03/2014 12:09 PM, Petr Viktorin wrote: Hello, This adds read permissions to read Sudo commands, command groups, rules. Read access is given to all authenticated users. Looks good. What about ou=sudoers? I think we should also allow it in this patch for authenticated users. This is the tree that clients use to read sudo. This new version does that. It needs my patches 0508-0509 since the ou=sudoers permission is not tied to a specific Object plugin. -- Petr³ From 11f8d647cc3976dc2d30908482c0dd7720cce270 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Mon, 7 Apr 2014 14:56:34 +0200 Subject: [PATCH] Add managed read permissions to Sudo objects and ou=sudoers Part of the work for: https://fedorahosted.org/freeipa/ticket/1313 and: https://fedorahosted.org/freeipa/ticket/3566 --- ipalib/plugins/sudocmd.py | 13 + ipalib/plugins/sudocmdgroup.py | 12 ipalib/plugins/sudorule.py | 18 ++ .../install/plugins/update_managed_permissions.py | 17 +++-- 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/ipalib/plugins/sudocmd.py b/ipalib/plugins/sudocmd.py index 35c01aa85a11fc42f73078c85beff6d049980509..4c7ea7f884c931950da629c92ee746f4a470a6ba 100644 --- a/ipalib/plugins/sudocmd.py +++ b/ipalib/plugins/sudocmd.py @@ -51,6 +51,7 @@ class sudocmd(LDAPObject): object_name = _('sudo command') object_name_plural = _('sudo commands') object_class = ['ipaobject', 'ipasudocmd'] +permission_filter_objectclasses = ['ipasudocmd'] # object_class_config = 'ipahostobjectclasses' search_attributes = [ 'sudocmd', 'description', @@ -63,6 +64,18 @@ class sudocmd(LDAPObject): } uuid_attribute = 'ipauniqueid' rdn_attribute = 'ipauniqueid' +managed_permissions = { +'System: Read Sudo Commands': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'description', 'ipauniqueid', 'memberof', 'objectclass', +'sudocmd', +}, +}, +} + label = _('Sudo Commands') label_singular = _('Sudo Command') diff --git a/ipalib/plugins/sudocmdgroup.py b/ipalib/plugins/sudocmdgroup.py index 0afa45819c96b5d4a7b71db3c69fabd6878b348a..471c8b858aec15d8a166a0ed7c0efcaddb99e0a2 100644 --- a/ipalib/plugins/sudocmdgroup.py +++ b/ipalib/plugins/sudocmdgroup.py @@ -55,6 +55,7 @@ class sudocmdgroup(LDAPObject): object_name = _('sudo command group') object_name_plural = _('sudo command groups') object_class = ['ipaobject', 'ipasudocmdgrp'] +permission_filter_objectclasses = ['ipasudocmdgrp'] default_attributes = [ 'cn', 'description', 'member', ] @@ -62,6 +63,17 @@ class sudocmdgroup(LDAPObject): attribute_members = { 'member': ['sudocmd'], } +managed_permissions = { +'System: Read Sudo Command Groups': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'businesscategory', 'cn', 'description', 'ipauniqueid', +'member', 'o', 'objectclass', 'ou', 'owner', 'seealso', +}, +}, +} label = _('Sudo Command Groups') label_singular = _('Sudo Command Group') diff --git a/ipalib/plugins/sudorule.py b/ipalib/plugins/sudorule.py index 2463325024da7c2b6aab40fc9e03150bb6645635..3f2c4063ce385d15f0551f663cba227a1269c62e 100644 --- a/ipalib/plugins/sudorule.py +++ b/ipalib/plugins/sudorule.py @@ -96,6 +96,7 @@ class sudorule(LDAPObject): object_name = _('sudo rule') object_name_plural = _('sudo rules') object_class = ['ipaassociation', 'ipasudorule'] +permission_filter_objectclasses = ['ipasudorule'] default_attributes = [ 'cn', 'ipaenabledflag', 'externaluser', 'description', 'usercategory', 'hostcategory', @@ -115,6 +116,23 @@ class sudorule(LDAPObject): 'ipasudorunas': ['user', 'group'], 'ipasudorunasgroup': ['group'], } +managed_permissions = { +'System: Read Sudo Rules': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'cmdcategory', 'cn', 'description', 'externalhost', +'externaluser', 'hostcategory', 'hostmask', 'ipaenabledflag', +'ipasudoopt', 'ipasudorunas', 'ipasudorunasextgroup', +'ipasudorunasextuser', 'ipasudorunasgroup', +'ipasudorunasgroupcategory', 'ipasudorunasusercategory', +
[Freeipa-devel] [PATCHES] 0508-0509 Add support for non-object managed permissions
Patch 0508: This documents the inputs for the permission updater in the module itself. This is taken from the design page. I expect it'll need an addition now and then, so I think it's better to have this near the code it corresponds to. Patch 0509: So far the new default permissions have been tied to an Object plugin, and took the ACI location and objectclass filter from the object. However there are some permissions that are not tied to an IPA object, for instance ones dealing with a compat tree. However, these permissions should behave similarly to the Object-based ones, so it makes sense to use the same updater with them. A question is where the non-Object permissions should be stored. I can think of several alternatives: a) in a special data file, like .update files b) in a new plugin type c) somewhere in the code I went for c) for simplicity, but feel free to discuss. (CCing Rob since he had some strong opinions in this area.) This patch makes ipapermlocation, ipapermtargetfilter and other Permission attributes overridable, and adds a central list of non-object permissions to the updater module. (For now, the list is empty). My patch 0504.2 (Default read ACIs for Sudo objects) will add a non-object permission for ou=sudoers. -- Petr³ From aa98fbd527727a301737c365dcfeb3245d6a51b2 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Thu, 27 Mar 2014 12:17:37 +0100 Subject: [PATCH] Document the managed permission updater operation The method was explained on the [Design] page, but as the updater is extended the design page would become obsolete. Document the operation in the docstring of the plugin itself. Design: http://www.freeipa.org/page/V3/Managed_Read_permissions#Default_Permission_Updater --- .../install/plugins/update_managed_permissions.py | 34 ++ 1 file changed, 34 insertions(+) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index 603f3f0b74c97b14be0992d2c110f5bb6cd0e0e6..b2548f4f12aab2ae05c3c4a63e38eb8ca2b65ad6 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -17,6 +17,40 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/. + +Plugin for updating managed permissions. + +The permissions are declared in Object plugins in the managed_permissions +attribute, which is a dictionary mapping permission names to a template +for the updater. +For example, an entry could look like this: + +managed_permissions = { +'System: Read Object A': { +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': {'cn', 'description'}, +'replaces_global_anonymous_aci': True, +}, +} + +The permission name must start with the System: prefix. + +The template dictionary can have the following keys: +* ipapermbindruletype, ipapermright + - Directly used as attributes on the permission. + - Replaced when upgrading an existing permission +* ipapermdefaultattr + - Used as attribute of the permission. + - When upgrading, only new values are added; all old values are kept. +* replaces_global_anonymous_aci + - If true, any attributes specified (denied) in the legacy global anonymous +read ACI will be added to excluded_attributes of the new permission. + - Has no effect when existing permissions are updated. + +No other keys are allowed in the template + + from ipalib import errors from ipapython.dn import DN from ipalib.plugable import Registry -- 1.9.0 From 333b846ed1d7228d4e6bd86b63b248a91debc8c0 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Thu, 27 Mar 2014 15:36:54 +0100 Subject: [PATCH] Add support for non-object default permissions Add support for managed permissions that are not tied to an object class and thus can't be defined in an Object plugin. Permission attributes that were previously taken from the object can now be overriden, and if no object is present they default to general permission defaults. A dict is added to hold templates for the non-object permissions. --- .../install/plugins/update_managed_permissions.py | 55 +- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index b2548f4f12aab2ae05c3c4a63e38eb8ca2b65ad6..8415b93a8cdac5ac23d07f0664e8f401239e4c25 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -34,12 +34,22 @@ }, } +For permissions not tied to an object plugin, a NONOBJECT_PERMISSIONS +dict of the same format is defined in this module. + The permission name must start
Re: [Freeipa-devel] global account lockout
Looks like there was a great discussion while I was away :-) There seem to be great concerns (and mybe confusion) about replication update resoultions, conflicts and amount of meta data stored. I think it's not as bad as you may think. Large amounts of metadata can only accumulate for multivalued attributes, for single valued attributes only the latest value and in some cases one! previous value is stored. For multivalued attributes the amount of data were considerably reduced with the fix for #569 The replication protocol and its update resolution is designed to follow a single master model: the final result should be the same as if the modifications were applied in the order of the csns on a single master, this implies that the last change wins. T There are some cases where conflicting entries or conflict attributes are generated. conflict entries should only be generated if the same entry (dn) is concurrently added on different masters. Conflict attribute are only genrated in some rare corner cases. In the case of krbLastFailedAuth or krbFailedLoginCount always the value with the highest csn will win, no scenario for a conflict. Replication storms. In my opinion the replication of a mod of one or two attribute in a entry will be faster than the bind itself. If an attacker knows all the dns of the entries in a server the denial of service could be that it just does a sequence of failed logins for any user and nobody will be able to login any more, replication would help to propagate this to other servers, but not prevent it. This would also be the case if only the final lockout state is replicated. I like the idea of replicating the attributes changed at failed logins (or reset) only. Regards, Ludwig On 04/08/2014 12:56 AM, Simo Sorce wrote: On Mon, 2014-04-07 at 14:28 -0600, Rich Megginson wrote: On 04/07/2014 01:00 PM, Simo Sorce wrote: On Mon, 2014-04-07 at 14:47 -0400, Dmitri Pal wrote: On 04/07/2014 02:31 PM, Simo Sorce wrote: On Mon, 2014-04-07 at 10:22 -0600, Rich Megginson wrote: On 04/07/2014 10:13 AM, Simo Sorce wrote: On Mon, 2014-04-07 at 12:10 -0400, Simo Sorce wrote: On Mon, 2014-04-07 at 12:01 -0400, Simo Sorce wrote: On Mon, 2014-04-07 at 11:26 -0400, Rob Crittenden wrote: Ludwig Krispenz wrote: Hi, please review the following feature design. It introduces a global account lockout, while trying to keep the replication traffic minimal. In my opinion for a real global account lockout the basic lockout attributes have to be replicated otherwise the benefit is minimal: an attacker could perform (maxFailedcount -1) login attempts on every server before the global lockout is set. But the design page describes how it could be done if it should be implemented - maybe the side effect that accounts could the be unlocked on any replica has its own benefit. http://www.freeipa.org/page/V4/Replicated_lockout One weakness with this is there is still a window for extra password attempts if one is clever, (m * (f-1))+1 to be exact, where m is the number of masters and f is the # of allowed failed logins. Yes, but that is a problem that cannot be solved w/o full replication at every authentication attempt. What we tried to achieve is a middle ground to at least ease administration and still lock em up earlier. Let me add that we could have yet another closer step by finding a way to replicate only failed attempts and not successful attempts in some case. Assuming a setup where most people do not fail to enter their password it would make for a decent compromise. That could be achieved by not storing lastsuccessful auth except when that is needed to clear failed logon attempts (ie when the failed logon counter is 0) If we did that then we would not need a new attribute actually, as failed logins would always be replicated. However it would mean that last Successful auth would never be accurate on any server. Or perhaps we could have a local last successful auth and a global one by adding one new attribute, and keeping masking only the successful auth. The main issue about all these possibilities is how do we present them ? And how do we make a good default ? I think a good default is defined by these 2 characteristics: 1. lockouts can be dealt with on any replica w/o having the admin hunt down where a user is locked. 2. at least successful authentications will not cause replication storms If we can afford to cause replications on failed authentication by default, then we could open up replication for failedauth and failedcount attributes but still bar the successful auth attribute. Unlock would simply consist in forcibly setting failed count to 0 (which is replicated so it would unlock all servers). This would work w/o introducing new attributes and only with minimal logic changes in the KDC/pwd-extop plugins I think. Sigh re[plying again to myself. note that the main issue with replicating failed accounts is that you can cause replication storms
Re: [Freeipa-devel] [PATCH] 0504 Default read ACIs for Sudo objects
On 04/08/2014 11:03 AM, Petr Viktorin wrote: On 04/07/2014 01:30 PM, Martin Kosek wrote: On 04/03/2014 12:09 PM, Petr Viktorin wrote: Hello, This adds read permissions to read Sudo commands, command groups, rules. Read access is given to all authenticated users. Looks good. What about ou=sudoers? I think we should also allow it in this patch for authenticated users. This is the tree that clients use to read sudo. This new version does that. It needs my patches 0508-0509 since the ou=sudoers permission is not tied to a specific Object plugin. I would also allow 'ou', otherwise an authenticated user cannot read the ou=sudoers RDN. I will comment on NONOBJECT_PERMISSIONS in the other thread. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0508-0509 Add support for non-object managed permissions
On 04/08/2014 11:03 AM, Petr Viktorin wrote: Patch 0508: This documents the inputs for the permission updater in the module itself. This is taken from the design page. I expect it'll need an addition now and then, so I think it's better to have this near the code it corresponds to. Patch 0509: So far the new default permissions have been tied to an Object plugin, and took the ACI location and objectclass filter from the object. However there are some permissions that are not tied to an IPA object, for instance ones dealing with a compat tree. However, these permissions should behave similarly to the Object-based ones, so it makes sense to use the same updater with them. A question is where the non-Object permissions should be stored. I can think of several alternatives: a) in a special data file, like .update files b) in a new plugin type c) somewhere in the code I went for c) for simplicity, but feel free to discuss. (CCing Rob since he had some strong opinions in this area.) This patch makes ipapermlocation, ipapermtargetfilter and other Permission attributes overridable, and adds a central list of non-object permissions to the updater module. (For now, the list is empty). My patch 0504.2 (Default read ACIs for Sudo objects) will add a non-object permission for ou=sudoers. The patch is functional, but I am not really a big fan of placing it in the plugin. I would prefer if the ACI definition is also in the sudo plugin together with other definition. It would be then much easier to audit all sudo-related ACIs. Why can't we add this ACI to sudorule object managed permissions and just override the location and target? I am not insisting on a specific format, I would simply prefer to have all plugin object related ACIs close together. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0508-0509 Add support for non-object managed permissions
On 04/08/2014 12:53 PM, Martin Kosek wrote: On 04/08/2014 11:03 AM, Petr Viktorin wrote: Patch 0508: This documents the inputs for the permission updater in the module itself. This is taken from the design page. I expect it'll need an addition now and then, so I think it's better to have this near the code it corresponds to. Patch 0509: So far the new default permissions have been tied to an Object plugin, and took the ACI location and objectclass filter from the object. However there are some permissions that are not tied to an IPA object, for instance ones dealing with a compat tree. However, these permissions should behave similarly to the Object-based ones, so it makes sense to use the same updater with them. A question is where the non-Object permissions should be stored. I can think of several alternatives: a) in a special data file, like .update files b) in a new plugin type c) somewhere in the code I went for c) for simplicity, but feel free to discuss. (CCing Rob since he had some strong opinions in this area.) This patch makes ipapermlocation, ipapermtargetfilter and other Permission attributes overridable, and adds a central list of non-object permissions to the updater module. (For now, the list is empty). My patch 0504.2 (Default read ACIs for Sudo objects) will add a non-object permission for ou=sudoers. The patch is functional, but I am not really a big fan of placing it in the plugin. I would prefer if the ACI definition is also in the sudo plugin together with other definition. It would be then much easier to audit all sudo-related ACIs. Why can't we add this ACI to sudorule object managed permissions and just override the location and target? I can do that. Most of the changes make this overriding possible, where the permission is actually defined is a detail. I am not insisting on a specific format, I would simply prefer to have all plugin object related ACIs close together. My reasoning is that finding the definition would not be straightforward. All the object-specific permissions so far are defined in their plugins, as determined by --type. This one won't have --type, and it's not clear if it should be in sudorule, sudocmd or sudocmdgroup. But, I don't have a strong preference. A `git grep` will always show the definition. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 261 Fix upload of CA certificate to LDAP in CA-less install
Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4300. Honza -- Jan Cholasta From 7439c75bc2db63ebf2268a02e4972fefbc7d828a Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Tue, 8 Apr 2014 13:12:47 +0200 Subject: [PATCH] Fix upload of CA certificate to LDAP in CA-less install. https://fedorahosted.org/freeipa/ticket/4300 --- ipaserver/install/dsinstance.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index be8c5c4..9256c12 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -233,6 +233,7 @@ class DsInstance(service.Service): self.domain = domain_name self.serverid = None self.pkcs12_info = None +self.cacert_name = None self.ca_is_configured = True self.dercert = None self.idstart = None @@ -642,6 +643,8 @@ class DsInstance(service.Service): nickname, self.fqdn, cadb) dsdb.create_pin_file() +self.cacert_name = dsdb.cacert_name + if self.ca_is_configured: dsdb.track_server_cert( nickname, self.principal, dsdb.passwd_fname, @@ -685,7 +688,7 @@ class DsInstance(service.Service): certdb = certs.CertDB(self.realm, nssdir=dirname, subject_base=self.subject_base) -dercert = certdb.get_cert_from_db(certdb.cacert_name, pem=False) +dercert = certdb.get_cert_from_db(self.cacert_name, pem=False) conn = ipaldap.IPAdmin(self.fqdn) conn.do_simple_bind(DN(('cn', 'directory manager')), self.dm_password) -- 1.8.5.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0148: ipa-sam: when deleting subtree, deal with possible LDAP errors
On Tue, Mar 11, 2014 at 03:39:57PM +0100, Petr Spacek wrote: On 11.3.2014 15:32, Alexander Bokovoy wrote: after discussing with Petr Spacek, following patch fixes ticket 4224. Code seems okay but I didn't do functional test. To just close this tread the changes form this patch are already applied by 'ipa-sam: cache gid to sid and uid to sid requests in idmap cache' master: d6a7923f71eb69bac53d6ff904086a9abd103dbc ipa-3-3: 13cd4faf551d7781d27c36bef0e7cbf515e072d2 bye, Sumit -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 261 Fix upload of CA certificate to LDAP in CA-less install
On 04/08/2014 01:16 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4300. Honza Works for me, ACK. Pushed to master: 915cd6942c0acb00688ba7a8b0d2519be9a47fb3 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 9 CA-less tests generate failure
On 04/04/2014 10:59 AM, Misnyovszki Adam wrote: Hi, CA-less test suite always generate failures when installing revoked certificates. This is a known issue, described in https://fedorahosted.org/freeipa/ticket/4270 , this fix skips these tests, outputting a notification message for the ticket. Now it outputs this: [amisnyov@host freeipa]$ ./make-test ipatests/test_integration/test_caless.py:TestServerInstall.test_revoked_http /usr/bin/nosetests -v --with-doctest --doctest-tests --exclude=plugins ipatests/test_integration/test_caless.py:TestServerInstall.test_revoked_http IPA server install with revoked HTTP certificate ... SKIP: Known CA-less installation defect, see https://fedorahosted.org/freeipa/ticket/4270 -- Ran 1 test in 1020.253s OK (SKIP=1) == passed under '/usr/bin/python2.7' ** pass ** https://fedorahosted.org/freeipa/ticket/4271 There could be another possible solution, I could write a nose plugin to enable raising warnings instead of skipping a test. This could be achieved by adding a @unittest.expectedFailure for a specific test, so if it fails, it counts as an error/warning. There is a poc in a nose ticket located in http://code.google.com/p/python-nose/issues/detail?id=428 , not sure how much time it takes to implement it as a plugin, or is it even worth, because if this is implemented, we could also use this feature when eg. DNS is not configured, this is why RFC. Thanks Adam Thanks for question. I personally think this solution is good enough for now. We may even chose to switch to pytest in the future, so I would not invest too much in python-nose specific plugins at this point. ACK. Pushed to master, ipa-3-3. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 0510-0511 Add managed read permissions to group hostgroup
Hello, These add read permissions to read user groups and hostgroups. For most attributes, anonymous read access is given. For member, memberOf, memberUID, read access is given only to authenticated users. -- Petr³ From af2054d54dbb9818255b87e2b78ecc37b87e469a Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 26 Mar 2014 15:17:34 +0100 Subject: [PATCH] Add managed read permissions to group Part of the work for: https://fedorahosted.org/freeipa/ticket/3566 --- ipalib/plugins/group.py | 20 1 file changed, 20 insertions(+) diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py index 318f0746a2f66f68db2b22e17b0d1689ad9ce3bc..644954d94a50e7a1222cc0cfc9b5de1eac47238a 100644 --- a/ipalib/plugins/group.py +++ b/ipalib/plugins/group.py @@ -137,6 +137,26 @@ class group(LDAPObject): 'sudorule'], } rdn_is_primary_key = True +managed_permissions = { +'System: Read Groups': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'anonymous', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'businesscategory', 'cn', 'description', 'gidnumber', +'ipaexternalmember', 'ipauniqueid', 'mepmanagedby', 'o', +'objectclass', 'ou', 'owner', 'seealso', +}, +}, +'System: Read Group Membership': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'member', 'memberof', 'memberuid', +}, +}, +} label = _('User Groups') label_singular = _('User Group') -- 1.9.0 From fb03d37b87e2177e0f7487991a7dcfdd3ecd624b Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 26 Mar 2014 16:21:26 +0100 Subject: [PATCH] Add managed read permission to hostgroup Part of the work for: https://fedorahosted.org/freeipa/ticket/3566 --- ipalib/plugins/hostgroup.py | 19 +++ 1 file changed, 19 insertions(+) diff --git a/ipalib/plugins/hostgroup.py b/ipalib/plugins/hostgroup.py index a3dd3a4a9bad24fe966abc7294a3c8aebd6fadf7..2addf20640bda967b0d3a0f0a56f7f8012b7da60 100644 --- a/ipalib/plugins/hostgroup.py +++ b/ipalib/plugins/hostgroup.py @@ -72,6 +72,25 @@ class hostgroup(LDAPObject): 'memberindirect': ['host', 'hostgroup'], 'memberofindirect': ['hostgroup', 'hbacrule', 'sudorule'], } +managed_permissions = { +'System: Read Hostgroups': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'anonymous', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'businesscategory', 'cn', 'description', 'ipauniqueid', 'o', +'objectclass', 'ou', 'owner', 'seealso', +}, +}, +'System: Read Hostgroup Membership': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'member', 'memberof', +}, +}, +} label = _('Host Groups') label_singular = _('Host Group') -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object
On 04/03/2014 04:31 PM, Alexander Bokovoy wrote: On Wed, 02 Apr 2014, Martin Kosek wrote: On 04/01/2014 12:03 PM, Jan Pazdziora wrote: On Tue, Apr 01, 2014 at 10:05:39AM +0200, Tomas Babej wrote: Yes, that was the intention. Mistake on my part, I'll send updated patches. Updated patch attached. Ack based on reading the code and documentation for slapi_ch_free_string. Ok, thanks. Though I would like this patch to be also functionally tested that it does not break anything, ideally together with your other ipa-range patches. ACK to 0162, 0161, 0158 (should be applied in this order). # ipa idrange-find 2 ranges matched Range name: AD.TEST_id_range First Posix ID of the range: 111500 Number of IDs in the range: 20 First RID of the corresponding RID range: 0 Domain SID of the trusted domain: S-1-5-21-2275361654-3393353068-3720134936 Range type: Active Directory domain range Range name: T.VDA.LI_id_range First Posix ID of the range: 91740 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 1 Range type: local domain range Number of entries returned 2 # ipa idrange-add AD.TEST_1_id_range First Posix ID of the range: 111900 Number of IDs in the range: 20 First RID of the corresponding RID range: 0 First RID of the secondary RID range: 100 ipa: ERROR: Constraint violation: New primary rid range overlaps with existing primary rid range. the message comes from the ipa-range-check plugin. Pushed all 3 to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0158] Extend ipa-range-check DS plugin to handle range types
On 04/01/2014 10:52 AM, Tomas Babej wrote: On 04/01/2014 10:40 AM, Alexander Bokovoy wrote: On Tue, 01 Apr 2014, Tomas Babej wrote: From 736b3f747188696fd4a46ca63d91a6cca942fd56 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Wed, 5 Mar 2014 12:28:18 +0100 Subject: [PATCH] Extend ipa-range-check DS plugin to handle range types The ipa-range-check plugin used to determine the range type depending on the value of the attributes such as RID or secondary RID base. This approached caused variety of issues since the portfolio of ID range types expanded. The patch makes sure the following rules are implemented: * No ID range pair can overlap on base ranges, with exception of two ipa-ad-trust-posix ranges belonging to the same forest * For any ID range pair of ranges belonging to the same domain: * Both ID ranges must be of the same type * For ranges of ipa-ad-trust type or ipa-local type: * Primary RID ranges can not overlap * For ranges of ipa-local type: * Primary and secondary RID ranges can not overlap * Secondary RID ranges cannot overlap For the implementation part, the plugin was extended with a domain ID to forest root domain ID mapping derivation capabilities. https://fedorahosted.org/freeipa/ticket/4137 -static int slapi_entry_to_range_info(struct slapi_entry *entry, +struct domain_info { +char *domain_id; +char *forest_root_id; +struct domain_info *next; +}; + +static void free_domain_info(struct domain_info *info) { +if (info != NULL) { +slapi_ch_free_string((info-domain_id)); +slapi_ch_free_string((info-forest_root_id)); +free_domain_info(info-next); +free(info); +} +} Please, don't use recursion in the freeing part, there is really no pressing need to do so. Just use while() like you do in get_forest_root_id(): +/* Searches for the domain_info struct with the specified domain_id + * in the linked list. Returns the forest root domain's ID, or NULL for + * local ranges. */ + +static char* get_forest_root_id(struct domain_info *head, char* domain_id) { + +/* For local ranges there is no forest root domain, + * so consider only ranges with domain_id set */ +if (domain_id != NULL) { +while(head) { +if (strcasecmp(head-domain_id, domain_id) == 0) { +return head-forest_root_id; +} +head = head-next; +} + } + +return NULL; +} + Fixed, updated patch attached. Pushed to master based on Alexander's ACK in patch 161. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0162] ipa-pwd-extop: Fix memory leak in ipapwd_pre_bind
On 04/01/2014 01:59 PM, Alexander Bokovoy wrote: On Tue, 01 Apr 2014, Tomas Babej wrote: Hi, We need to free the entry before returning from the function. https://fedorahosted.org/freeipa/ticket/4295 ACK. Pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Random Certificate Serial Numbers
Dmitri Pal wrote: On 04/07/2014 03:48 AM, Martin Kosek wrote: Hi Rob, Ade and others, In the past, Rob was investigating enabling random certificate serial numbers for FreeIPA PKI [1]. We also have a ticket [2] planned to enable it for 4.0. Can we simply switch it on for PKI with pkispawn attribute: [CA] pki_random_serial_numbers_enable=True or is there any drawback or risk we should investigate. I am just thinking, does PKI handle collisions anyhow? When for example two PKI masters generate 2 certificates of the same serial (unlikely though it could happen)? Currently, we assign different slice of serial range to different PKI masters, do we want to do that also for random serial? Thanks for info [1] http://dogtagpki.org/wiki/Random_Certificate_Serial_Numbers [2] https://fedorahosted.org/freeipa/ticket/2016 Any impact on upgrades? It only affects new installs. Any impact on certmonger? I seriously doubt it. The only potential issue is seriously long serial numbers but that isn't specific to random values. I had an install using this a year or so ago and I don't recall any major issues. Unfortunately that system has gone off the deep end so I no longer have the changes. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add DRM to IPA
Martin Kosek wrote: On 04/07/2014 10:40 PM, Rob Crittenden wrote: Ade Lee wrote: This patch adds the capability of installing a Dogtag DRM to an IPA instance. With this patch, when ipa-server-install is run, a Dogtag CA and a Dogtag DRM are created. The DRM shares the same tomcat instance and DS instance as the Dogtag CA. Moreover, the same admin user/agent (and agent cert) can be used for both subsystems. Certmonger is also confgured to monitor the new subsystem certificates. It is also possible to clone the DRM. When the IPA instance is cloned, if --enable-ca and --enable-drm are specified, the DRM is cloned as well. Installing a DRM requires the user to have a Dogtag CA instance. We can look into possibly relaxing that requirement in a later patch. I am still working on patches for a ipa-drm-install script, which would be used to add a DRM to an existing master (that includes a dogtag CA), or an existing clone. Please review, Thanks, Ade Yikes, I wonder if the changes to ipaserver/install/cainstance.py should be pushed ASAP. Oops, looks like a change that should go to IPA 3.3.x. What is the implication? freeipa-spec.in needs a dependency on pki-kra. Let us stop here. Please see a following RFE I filed: https://fedorahosted.org/freeipa/ticket/4058 I would prefer it KRA files and specifics would be in a new subpackage like freeipa-server-kra. Otherwise we will need to rework it again when we would be splitting CA to freeipa-server-pki in 4.1. Yes, that is a question I didn't ask: Is the DRM going to be configured by default on all new installs? I would prefer to start the right modularization now as I do not think that every FreeIPA server needs to run CA/KRA, i.e. it does not need to have the bits installed either. I think the decision on a separate sub-package will be dependent upon whether it is default or not, otherwise we can get away with freeipa-server-ca and just lump everything in there. I am also quite worried about the duplication that the new drminstance.py introduces. There is a lot of functions which do more or less the same thing and have most of the handling code the same with only a very small and predictable pki/kra change. For example __http_proxy function seems to be exactly the same. It would be great to avoid this duplication and rather have some common ground utilized by both PKI and KRA. Otherwise it will be very difficult to maintain the new code. I touched on some of that too, but some of this is just inevitable I think which is why I didn't pound on it too hard. An abstraction would be nice, but I'm not sure abstracting for two things, and only in the installer, is worth the effort. I could be wrong. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Random Certificate Serial Numbers
On Mon, 2014-04-07 at 09:48 +0200, Martin Kosek wrote: Hi Rob, Ade and others, In the past, Rob was investigating enabling random certificate serial numbers for FreeIPA PKI [1]. We also have a ticket [2] planned to enable it for 4.0. Can we simply switch it on for PKI with pkispawn attribute: [CA] pki_random_serial_numbers_enable=True Putting in this parameter in pkispawn means changing the method of assigning serial numbers for the CA that is being installed (ie. a new master) Thus this will affect new masters only. When the CA is cloned, it will inherit its method of assigning serial numbers from the master. I need to check the code to see what happens if you specify the above directive in pkispawn for a clone. Are you considering changing the serial number assignment for existing masters? or is there any drawback or risk we should investigate. I am just thinking, does PKI handle collisions anyhow? When for example two PKI masters generate 2 certificates of the same serial (unlikely though it could happen)? Collisions are not supposed to happen. Range number assignment is automatically managed so that different masters are assigned different ranges so that collisions cannot happen. Collisions can occur if ranges overlap -- ie. if you are manually updating ranges and end up using overlapping ranges. Currently, we assign different slice of serial range to different PKI masters, do we want to do that also for random serial? Yes. Range management is done automatically. Different masters are assigned different ranges to prevent collisions. Random serial numbers will be generated within the assigned range. Thanks for info [1] http://dogtagpki.org/wiki/Random_Certificate_Serial_Numbers [2] https://fedorahosted.org/freeipa/ticket/2016 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0034] Deny LDAP binds for user accounts with expired principal
On 03/27/2014 02:40 PM, Martin Kosek wrote: On 01/07/2014 01:47 PM, Tomas Babej wrote: On 01/07/2014 07:23 AM, Alexander Bokovoy wrote: On Mon, 06 Jan 2014, Tomas Babej wrote: On 01/06/2014 12:16 PM, Tomas Babej wrote: On 04/15/2013 12:43 PM, Tomas Babej wrote: On 04/08/2013 03:55 PM, Martin Kosek wrote: On 04/01/2013 09:52 PM, Rob Crittenden wrote: Tomas Babej wrote: On 02/12/2013 06:23 PM, Simo Sorce wrote: On Tue, 2013-02-12 at 18:03 +0100, Tomas Babej wrote: On 02/12/2013 05:50 PM, Tomas Babej wrote: Hi, This patch adds a check for krbprincipalexpiration attribute to pre_bind operation in ipa-pwd-extop dirsrv plugin. If the principal is expired, auth is denied and LDAP_INVALID_CREDENTIALS along with the error message is sent back to the client. Since krbprincipalexpiration attribute is not mandatory, if there is no value set, the check is passed. https://fedorahosted.org/freeipa/ticket/3305 Comments inline. @@ -1166,6 +1173,29 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb) goto done; } +/* check the krbPrincipalExpiration attribute is present */ +ret = slapi_entry_attr_find(entry, krbPrincipalExpiration, attr); +if (!ret) { ret is not a boolean so probably better ti use (ret != 0) +/* if it is, check whether the principal has not expired */ + +principal_expire = slapi_entry_attr_get_charptr(entry, + krbprincipalexpiration); +memset(expire_tm, 0, sizeof (expire_tm)); + +if (strptime(principal_expire, %Y%m%d%H%M%SZ, expire_tm)){ style issue missing space between ) and { +expire_time = mktime(expire_tm); +/* this might have overflown on 32-bit system */ This comment does not help to point out what you want to put in evidence. if there is an overflow what is the consequence ? Or does it mean the next condition is taking that into account ? Yeah, this was rather ill-worded. Replaced by a rather verbose comment that hopefully clears things out. +if (current_time expire_time expire_time 0){ style issue missing space between ) and { +LOG_FATAL(kerberos principal has expired in user entry: %s\n, + dn); I think a better phrasing would be: The kerberos principal on %s is expired\n +errMesg = Kerberos principal has expired.; s/has/is/ The rest looks good to me. Simo. Styling issues fixed and updated patch attached :) Small nit, the expiration error should probably use 'in' not 'on'. I'm not sure LDAP_INVALID_CREDENTIALS is the right return code. This implies that the password is wrong. I think that LDAP_UNWILLING_TO_PERFORM is probably more correct here. It is what we return on other policy failures. rob Additional findings: 1) Lets not try to get current_time every time, but only when its needed (i.e. krbPrincipalExpiration is set) - AFAIK, it is not so cheap operation: +/* get current time*/ +current_time = time(NULL); 2) Why using slapi_entry_attr_get_charptr and thus let 389-ds find the attribute again when we already have a pointer for the attribute? Would slapi_attr_first_value following slapi_entry_attr_find be faster approach? +ret = slapi_entry_attr_find(entry, krbPrincipalExpiration, attr); +if (ret != 0) { +/* if it is, check whether the principal has not expired */ + +principal_expire = slapi_entry_attr_get_charptr(entry, + krbprincipalexpiration); This way, we would not create a copy of this attribute value - we do not need a copy, we just do check against it. 3) Shouldn't we return -1 in the end when the auth fails? We do that in other pre_callbacks. Otherwise we just return 0 (success): +if (auth_failed){ +slapi_send_ldap_result(pb, LDAP_INVALID_CREDENTIALS, NULL, errMesg, + 0, NULL); +} + return 0; Martin Thank you both for the review. I updated and retested the patch, with your suggestions incorporated. Tomas I rebased the patch on top of current master. Bumping, since this is planned as part of 3.4 (and there were some requests for this feature). Tomas I updated the commit message to reflect better what the current version of the patch implements. Tomas From a93d1ec3ca8c7b6e8e754b474257695ba9018e07 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Mon, 6 Jan 2014 12:11:24 +0100 Subject: [PATCH] ipa-pwd-extop: Deny LDAP binds for user accounts with expired principal Adds a check for krbprincipalexpiration attribute to pre_bind operation in ipa-pwd-extop dirsrv plugin. If the principal is expired, auth is denied and LDAP_UNWILLING_TO_PERFORM along with the error message is sent back to the client. Since krbprincipalexpiration attribute is not mandatory, if there is no value set, the check is passed.
Re: [Freeipa-devel] [PATCH 0034] Deny LDAP binds for user accounts with expired principal
On Tue, 08 Apr 2014, Martin Kosek wrote: +auth_failed = true; +goto done; +} +} I think indenting is broken for these two brackets. Thanks Alexander, fixed. Updated version attached. Tomas Simo, Alexander - are you know OK with Tomas' patch? The patch review is now taking more than a year, so I think it would be great to finish it :) Martin Resending, it seems the message fell between cracks. I think I've asked for a rebase and Simo asked for that too. And then there were comments from Simo's side. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0034] Deny LDAP binds for user accounts with expired principal
On 04/08/2014 04:23 PM, Alexander Bokovoy wrote: On Tue, 08 Apr 2014, Martin Kosek wrote: +auth_failed = true; +goto done; +} +} I think indenting is broken for these two brackets. Thanks Alexander, fixed. Updated version attached. Tomas Simo, Alexander - are you know OK with Tomas' patch? The patch review is now taking more than a year, so I think it would be great to finish it :) Martin Resending, it seems the message fell between cracks. I think I've asked for a rebase and Simo asked for that too. And then there were comments from Simo's side. Ah, ok - thanks. I finally see it: http://www.redhat.com/archives/freeipa-devel/2014-March/msg00417.html It went to different sub-thread so I missed it. Well, Tomas - the ball is on your lawn then :) Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add DRM to IPA
On Tue, 2014-04-08 at 09:52 -0400, Rob Crittenden wrote: Martin Kosek wrote: On 04/07/2014 10:40 PM, Rob Crittenden wrote: Ade Lee wrote: This patch adds the capability of installing a Dogtag DRM to an IPA instance. With this patch, when ipa-server-install is run, a Dogtag CA and a Dogtag DRM are created. The DRM shares the same tomcat instance and DS instance as the Dogtag CA. Moreover, the same admin user/agent (and agent cert) can be used for both subsystems. Certmonger is also confgured to monitor the new subsystem certificates. It is also possible to clone the DRM. When the IPA instance is cloned, if --enable-ca and --enable-drm are specified, the DRM is cloned as well. Installing a DRM requires the user to have a Dogtag CA instance. We can look into possibly relaxing that requirement in a later patch. I am still working on patches for a ipa-drm-install script, which would be used to add a DRM to an existing master (that includes a dogtag CA), or an existing clone. Please review, Thanks, Ade Yikes, I wonder if the changes to ipaserver/install/cainstance.py should be pushed ASAP. Oops, looks like a change that should go to IPA 3.3.x. What is the implication? This error is unlikely to have any real effect because when connecting to the database using client auth, we determine how to connect to the database through the parameter that defines the certificate nickname. So we're probably fine with just changing it in master. freeipa-spec.in needs a dependency on pki-kra. Let us stop here. Please see a following RFE I filed: https://fedorahosted.org/freeipa/ticket/4058 I would prefer it KRA files and specifics would be in a new subpackage like freeipa-server-kra. Otherwise we will need to rework it again when we would be splitting CA to freeipa-server-pki in 4.1. Yes, that is a question I didn't ask: Is the DRM going to be configured by default on all new installs? The code I wrote presupposes this - but this is something that needs to be decided by IPA. Now given that the DRM is going to use the same tomcat instance and database as the CA - and given that we are probably going to want to have a DRM when we start issuing user certs, I see no reason not to add a DRM when we add a CA. To miminize complexity, I would suggest that we keep the requirement that DRM requires Dogtag CA. I would prefer to start the right modularization now as I do not think that every FreeIPA server needs to run CA/KRA, i.e. it does not need to have the bits installed either. I think the decision on a separate sub-package will be dependent upon whether it is default or not, otherwise we can get away with freeipa-server-ca and just lump everything in there. For noted above, because we will likely want DRM for user certs, I would suggest that DRM be installed by default when we install a Dogtag CA. As far as package dependencies, there are in fact very few additional dependencies for the DRM as most of the Java code is in common libraries already required by the CA. In fact, there is only one new package pki-kra, which contains a few KRA specific java classes. I am also quite worried about the duplication that the new drminstance.py introduces. There is a lot of functions which do more or less the same thing and have most of the handling code the same with only a very small and predictable pki/kra change. For example __http_proxy function seems to be exactly the same. It would be great to avoid this duplication and rather have some common ground utilized by both PKI and KRA. Otherwise it will be very difficult to maintain the new code. I touched on some of that too, but some of this is just inevitable I think which is why I didn't pound on it too hard. An abstraction would be nice, but I'm not sure abstracting for two things, and only in the installer, is worth the effort. I could be wrong. This initial patch was to get things working and start some of these discussions. As we consider adding additional subsystems like the TKS and TPS, having some common ground will make things simpler to maintain. I will look into abstracting to reduce code duplication. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0508-0509 Add support for non-object managed permissions
On 04/08/2014 01:14 PM, Petr Viktorin wrote: On 04/08/2014 12:53 PM, Martin Kosek wrote: On 04/08/2014 11:03 AM, Petr Viktorin wrote: ... The patch is functional, but I am not really a big fan of placing it in the plugin. I would prefer if the ACI definition is also in the sudo plugin together with other definition. It would be then much easier to audit all sudo-related ACIs. Why can't we add this ACI to sudorule object managed permissions and just override the location and target? I can do that. Most of the changes make this overriding possible, where the permission is actually defined is a detail. I am not insisting on a specific format, I would simply prefer to have all plugin object related ACIs close together. My reasoning is that finding the definition would not be straightforward. All the object-specific permissions so far are defined in their plugins, as determined by --type. This one won't have --type, and it's not clear if it should be in sudorule, sudocmd or sudocmdgroup. But, I don't have a strong preference. A `git grep` will always show the definition. IMO sudorule is fine, I personally see it as an overarching plugin for sudo, sudocmds and sudocmdgroups are just part of the sudorule. We may just want to somehow differentiate the non--type ACIs from the regular --type ones. Whether it is a different attribute in the Object or a setting in managed permission is something I will leave up to you. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On Mon, 07 Apr 2014 09:43:10 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 03/27/2014 03:37 PM, Misnyovszki Adam wrote: On Wed, 26 Mar 2014 13:15:55 +0100 Petr Viktorin pvikt...@redhat.com wrote: [...] Looks great! I'm just concerned about the error returned when the task takes too long: $ ipa automember-rebuild --type group ipa: ERROR: LDAP timeout I don't think it's sufficiently clear from this that waiting for the task timed out, but the task was actually started successfully. A custom error with a more descriptive message would be useful. Also I've noticed that the nstaskstatus of a successful task is: Automember rebuild task finished. Processed (1) entries. This looks helpful; we could return it as the summary. Hi, both fixed. Greets Adam Sorry for the delay! 'Automember' is a translatable string, so please wrap it in _() when raising TaskTimeout. Also please update the tests. Otherwise with a little rebase it's good to go. Hi, see the attached modifications, tests corrected, and added for no-wait, also rebased for current master. Greets AdamFrom 942a83926d3af0a314c5ad8a2f78ef02d5be553e Mon Sep 17 00:00:00 2001 From: Adam Misnyovszki amisn...@redhat.com Date: Tue, 25 Mar 2014 14:47:03 +0100 Subject: [PATCH 1/2] automember rebuild nowait feature added automember-rebuild uses asynchronous 389 task, and returned success even if the task didn't run. this patch fixes this issue adding a --nowait parameter to 'ipa automember-rebuild', defaulting to False, thus when the script runs without it, it waits for the 'nstaskexitcode' attribute, which means the task has finished. Old usage can be enabled using --nowait, and returns the DN of the task for further polling. New tests added also. https://fedorahosted.org/freeipa/ticket/4239 --- API.txt| 7 ++- VERSION| 4 +- ipalib/errors.py | 16 ++ ipalib/plugins/automember.py | 70 +- ipatests/test_xmlrpc/test_automember_plugin.py | 67 ipatests/test_xmlrpc/xmlrpc_test.py| 10 6 files changed, 149 insertions(+), 25 deletions(-) diff --git a/API.txt b/API.txt index 14dde56832793f8dd9fa6795a5ba79d0a2431d51..a0285d49466b887bc9aeb4b4190cc0d99687cf6d 100644 --- a/API.txt +++ b/API.txt @@ -201,12 +201,15 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_rebuild -args: 0,4,3 +args: 0,7,3 +option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Str('hosts*') +option: Flag('no_wait?', autofill=True, default=False) +option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup')) option: Str('users*') option: Str('version?', exclude='webui') -output: Output('result', type 'bool', None) +output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) output: Output('summary', (type 'unicode', type 'NoneType'), None) output: Output('value', type 'unicode', None) command: automember_remove_condition diff --git a/VERSION b/VERSION index 7c6722965bc3b37b71e036ce7f2b2472fd662877..e787e371318b2a817a7d18c1bb1750db9130192e 100644 --- a/VERSION +++ b/VERSION @@ -89,5 +89,5 @@ IPA_DATA_VERSION=2010061412 # # IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=81 -# Last change: amisnyov - user plugin extend +IPA_API_VERSION_MINOR=82 +# Last change: amisnyov - automember nowait add diff --git a/ipalib/errors.py b/ipalib/errors.py index 311127f62e54017c85541d27276020a9f950ab0f..8ef35f590390eda1e847589d669cd4d28644a6a5 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -1530,6 +1530,22 @@ class DNSDataMismatch(ExecutionError): format = _('DNS check failed: Expected {%(expected)s} got {%(got)s}') +class TaskTimeout(DatabaseError): + +**4213** Raised when an LDAP task times out + +For example: + + raise TaskTimeout() +Traceback (most recent call last): + ... +TaskTimeout: Automember LDAP task timeout, Task DN: '' + + +errno = 4213 +format = _(%(task)s LDAP task timeout, Task DN: '%(task_dn)s') + + class CertificateError(ExecutionError): **4300** Base class for Certificate execution errors (*4300 - 4399*). diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py index a12bfb52522e38bc083d0750dc66c894a4aeba53..56bba18f0df48134e42714e89f691d7e4ee98da5 100644 --- a/ipalib/plugins/automember.py +++ b/ipalib/plugins/automember.py @@ -17,8
Re: [Freeipa-devel] [PATCHES] 0508-0509 Add support for non-object managed permissions
On 04/08/2014 04:39 PM, Martin Kosek wrote: On 04/08/2014 01:14 PM, Petr Viktorin wrote: On 04/08/2014 12:53 PM, Martin Kosek wrote: On 04/08/2014 11:03 AM, Petr Viktorin wrote: ... The patch is functional, but I am not really a big fan of placing it in the plugin. I would prefer if the ACI definition is also in the sudo plugin together with other definition. It would be then much easier to audit all sudo-related ACIs. Why can't we add this ACI to sudorule object managed permissions and just override the location and target? I can do that. Most of the changes make this overriding possible, where the permission is actually defined is a detail. I am not insisting on a specific format, I would simply prefer to have all plugin object related ACIs close together. My reasoning is that finding the definition would not be straightforward. All the object-specific permissions so far are defined in their plugins, as determined by --type. This one won't have --type, and it's not clear if it should be in sudorule, sudocmd or sudocmdgroup. But, I don't have a strong preference. A `git grep` will always show the definition. IMO sudorule is fine, I personally see it as an overarching plugin for sudo, sudocmds and sudocmdgroups are just part of the sudorule. We may just want to somehow differentiate the non--type ACIs from the regular --type ones. Whether it is a different attribute in the Object or a setting in managed permission is something I will leave up to you. I went with a non_object key in the managed permission info. Attaching new patches. -- Petr³ From bc05ca06c450e6782d3a2e8becd80fd620fbb66a Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Thu, 27 Mar 2014 12:17:37 +0100 Subject: [PATCH] Document the managed permission updater operation The method was explained on the [Design] page, but as the updater is extended the design page would become obsolete. Document the operation in the docstring of the plugin itself. Design: http://www.freeipa.org/page/V3/Managed_Read_permissions#Default_Permission_Updater --- .../install/plugins/update_managed_permissions.py | 34 ++ 1 file changed, 34 insertions(+) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index 603f3f0b74c97b14be0992d2c110f5bb6cd0e0e6..b2548f4f12aab2ae05c3c4a63e38eb8ca2b65ad6 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -17,6 +17,40 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/. + +Plugin for updating managed permissions. + +The permissions are declared in Object plugins in the managed_permissions +attribute, which is a dictionary mapping permission names to a template +for the updater. +For example, an entry could look like this: + +managed_permissions = { +'System: Read Object A': { +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': {'cn', 'description'}, +'replaces_global_anonymous_aci': True, +}, +} + +The permission name must start with the System: prefix. + +The template dictionary can have the following keys: +* ipapermbindruletype, ipapermright + - Directly used as attributes on the permission. + - Replaced when upgrading an existing permission +* ipapermdefaultattr + - Used as attribute of the permission. + - When upgrading, only new values are added; all old values are kept. +* replaces_global_anonymous_aci + - If true, any attributes specified (denied) in the legacy global anonymous +read ACI will be added to excluded_attributes of the new permission. + - Has no effect when existing permissions are updated. + +No other keys are allowed in the template + + from ipalib import errors from ipapython.dn import DN from ipalib.plugable import Registry -- 1.9.0 From 70f1788534ff1821faad3a5c10e23bbf363dc03d Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Thu, 27 Mar 2014 15:36:54 +0100 Subject: [PATCH] Allow overriding all attributes of default permissions Allow overriding ipapermtarget, ipapermtargetfilter, ipapermlocation, objectclass of default managed permissions. This allows defining permissions that are not tied to an object type. Default values are same as before. Also, do not reset ipapermbindruletype when updating an existing managed permission. --- .../install/plugins/update_managed_permissions.py | 50 +- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index b2548f4f12aab2ae05c3c4a63e38eb8ca2b65ad6..d938eecf175867f3a6a61a68d5f384bf9e79c055 100644 ---
Re: [Freeipa-devel] [PATCH] 0504 Default read ACIs for Sudo objects
On 04/08/2014 12:46 PM, Martin Kosek wrote: On 04/08/2014 11:03 AM, Petr Viktorin wrote: On 04/07/2014 01:30 PM, Martin Kosek wrote: On 04/03/2014 12:09 PM, Petr Viktorin wrote: Hello, This adds read permissions to read Sudo commands, command groups, rules. Read access is given to all authenticated users. Looks good. What about ou=sudoers? I think we should also allow it in this patch for authenticated users. This is the tree that clients use to read sudo. This new version does that. It needs my patches 0508-0509 since the ou=sudoers permission is not tied to a specific Object plugin. I would also allow 'ou', otherwise an authenticated user cannot read the ou=sudoers RDN. I will comment on NONOBJECT_PERMISSIONS in the other thread. Right, I wonder how I missed that. New patch attached; it needs 0508-0509.2. -- Petr³ From 6c426c9a66a755dddf387e2396abbeaead3d3eb1 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Mon, 7 Apr 2014 14:56:34 +0200 Subject: [PATCH] Add managed read permissions to Sudo objects and ou=sudoers Part of the work for: https://fedorahosted.org/freeipa/ticket/1313 and: https://fedorahosted.org/freeipa/ticket/3566 --- ipalib/plugins/sudocmd.py | 13 + ipalib/plugins/sudocmdgroup.py | 12 ipalib/plugins/sudorule.py | 31 +++ 3 files changed, 56 insertions(+) diff --git a/ipalib/plugins/sudocmd.py b/ipalib/plugins/sudocmd.py index 35c01aa85a11fc42f73078c85beff6d049980509..4c7ea7f884c931950da629c92ee746f4a470a6ba 100644 --- a/ipalib/plugins/sudocmd.py +++ b/ipalib/plugins/sudocmd.py @@ -51,6 +51,7 @@ class sudocmd(LDAPObject): object_name = _('sudo command') object_name_plural = _('sudo commands') object_class = ['ipaobject', 'ipasudocmd'] +permission_filter_objectclasses = ['ipasudocmd'] # object_class_config = 'ipahostobjectclasses' search_attributes = [ 'sudocmd', 'description', @@ -63,6 +64,18 @@ class sudocmd(LDAPObject): } uuid_attribute = 'ipauniqueid' rdn_attribute = 'ipauniqueid' +managed_permissions = { +'System: Read Sudo Commands': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'description', 'ipauniqueid', 'memberof', 'objectclass', +'sudocmd', +}, +}, +} + label = _('Sudo Commands') label_singular = _('Sudo Command') diff --git a/ipalib/plugins/sudocmdgroup.py b/ipalib/plugins/sudocmdgroup.py index 0afa45819c96b5d4a7b71db3c69fabd6878b348a..471c8b858aec15d8a166a0ed7c0efcaddb99e0a2 100644 --- a/ipalib/plugins/sudocmdgroup.py +++ b/ipalib/plugins/sudocmdgroup.py @@ -55,6 +55,7 @@ class sudocmdgroup(LDAPObject): object_name = _('sudo command group') object_name_plural = _('sudo command groups') object_class = ['ipaobject', 'ipasudocmdgrp'] +permission_filter_objectclasses = ['ipasudocmdgrp'] default_attributes = [ 'cn', 'description', 'member', ] @@ -62,6 +63,17 @@ class sudocmdgroup(LDAPObject): attribute_members = { 'member': ['sudocmd'], } +managed_permissions = { +'System: Read Sudo Command Groups': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'businesscategory', 'cn', 'description', 'ipauniqueid', +'member', 'o', 'objectclass', 'ou', 'owner', 'seealso', +}, +}, +} label = _('Sudo Command Groups') label_singular = _('Sudo Command Group') diff --git a/ipalib/plugins/sudorule.py b/ipalib/plugins/sudorule.py index 2463325024da7c2b6aab40fc9e03150bb6645635..88fd86e31b95bb49b69a5a3dfdb7bf153784fbfc 100644 --- a/ipalib/plugins/sudorule.py +++ b/ipalib/plugins/sudorule.py @@ -96,6 +96,7 @@ class sudorule(LDAPObject): object_name = _('sudo rule') object_name_plural = _('sudo rules') object_class = ['ipaassociation', 'ipasudorule'] +permission_filter_objectclasses = ['ipasudorule'] default_attributes = [ 'cn', 'ipaenabledflag', 'externaluser', 'description', 'usercategory', 'hostcategory', @@ -115,6 +116,36 @@ class sudorule(LDAPObject): 'ipasudorunas': ['user', 'group'], 'ipasudorunasgroup': ['group'], } +managed_permissions = { +'System: Read Sudo Rules': { +'replaces_global_anonymous_aci': True, +'ipapermbindruletype': 'all', +'ipapermright': {'read', 'search', 'compare'}, +'ipapermdefaultattr': { +'cmdcategory', 'cn', 'description', 'externalhost', +'externaluser', 'hostcategory', 'hostmask', 'ipaenabledflag', +'ipasudoopt', 'ipasudorunas',
Re: [Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added
On 04/08/2014 04:17 PM, Misnyovszki Adam wrote: On Mon, 07 Apr 2014 09:43:10 +0200 Petr Viktorin pvikt...@redhat.com wrote: On 03/27/2014 03:37 PM, Misnyovszki Adam wrote: On Wed, 26 Mar 2014 13:15:55 +0100 Petr Viktorin pvikt...@redhat.com wrote: [...] Looks great! I'm just concerned about the error returned when the task takes too long: $ ipa automember-rebuild --type group ipa: ERROR: LDAP timeout I don't think it's sufficiently clear from this that waiting for the task timed out, but the task was actually started successfully. A custom error with a more descriptive message would be useful. Also I've noticed that the nstaskstatus of a successful task is: Automember rebuild task finished. Processed (1) entries. This looks helpful; we could return it as the summary. Hi, both fixed. Greets Adam Sorry for the delay! 'Automember' is a translatable string, so please wrap it in _() when raising TaskTimeout. Also please update the tests. Otherwise with a little rebase it's good to go. Hi, see the attached modifications, tests corrected, and added for no-wait, also rebased for current master. Greets Adam Looks good overall, but why do you now set `self.msg_summary`? Keep in mind that currently the same Command object is reused for every automember_rebuild command, including commands that run in parallel in different threads. It should never be modified. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Ipa-server-install Firewall Support
Not sure where to jump in but I had one comment: Puppet-IPA [1] + Shorewall make a lovely pair :) Cheers, James [1] https://github.com/purpleidea/puppet-ipa On Mon, Apr 7, 2014 at 7:51 PM, Dmitri Pal d...@redhat.com wrote: On 04/07/2014 09:00 AM, Rob Crittenden wrote: Simo Sorce wrote: On Fri, 2014-04-04 at 09:59 +0200, Petr Spacek wrote: On 4.4.2014 09:17, Martin Kosek wrote: On 04/04/2014 09:04 AM, Justin Brown wrote: I would actually do it the opposite way and open the ports after the FreeIPA server is fully configured. After all, I do not think we want to open the ports when the server is just half-configured and for example some ACIs are missing. My thinking was that nothing would be listening on these ports if the install doesn't succeed, but there's really necessity to modify the firewall configuration early. (All of the internal install communication will be over a local interface (to netfilter) and unblock anyways. I don't have any problem in delaying firewall configuration to the end of install. If ipa-server-install does succeed without configuring the firewalld, then we will indeed have no other option than to do it early. I am thinking that we may want to put all the firewalld configuration in ipaserver/install/firewalldinstance.py, and then make the firewalld configuration the actual step of the installation. Something like: ... Configuring Firewall (firewalld) [1/2]: looking up the right zone [2/2]: allowing ports Done configuring Firewall (firewalld). ... The Service class derived object can be really simple, we would just reuse the functionality it already has + let us properly hook into it in ipa-{server,replica}-install and the uninstallation. It would also make it easier to split this functionality to freeipa-server-firewalld if we chose to in a future. In general I agree with the idea, thank you Justin for working on that! I would like to emphasis the necessity to work without NetworkManager and FirewallD. New dependencies make Debian folks unhappy ... On the other hand, it is perfectly fine to skip firewall configuration if NM/FirewallD/DBus is not available. Have a nice day! Should be easy, probe for the dbus firewalld service and just skip (not error out) if it is not there. Set a variable in that case that will cause the installer to throw the classic banner we have now which warns you about what ports need to be opened at the end of the install. Probably just need to spit out a large, preferably flashing warning that the firewall has not been automatically configured. Perhaps even multiple times: one in-line and one at the install summary at the end. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Thanks for looking into this! Would it be possible to summarize this thread in a design page on the wiki? -- Thank you, Dmitri Pal Sr. Engineering Manager IdM portfolio Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Ipa-server-install Firewall Support
Dmitri, I'd be more than happy to, but I'm having trouble figuring out where it should go. Could you send me a link to a similar design page? Thanks, Justin On Mon, Apr 7, 2014 at 6:51 PM, Dmitri Pal d...@redhat.com wrote: On 04/07/2014 09:00 AM, Rob Crittenden wrote: Simo Sorce wrote: On Fri, 2014-04-04 at 09:59 +0200, Petr Spacek wrote: On 4.4.2014 09:17, Martin Kosek wrote: On 04/04/2014 09:04 AM, Justin Brown wrote: I would actually do it the opposite way and open the ports after the FreeIPA server is fully configured. After all, I do not think we want to open the ports when the server is just half-configured and for example some ACIs are missing. My thinking was that nothing would be listening on these ports if the install doesn't succeed, but there's really necessity to modify the firewall configuration early. (All of the internal install communication will be over a local interface (to netfilter) and unblock anyways. I don't have any problem in delaying firewall configuration to the end of install. If ipa-server-install does succeed without configuring the firewalld, then we will indeed have no other option than to do it early. I am thinking that we may want to put all the firewalld configuration in ipaserver/install/firewalldinstance.py, and then make the firewalld configuration the actual step of the installation. Something like: ... Configuring Firewall (firewalld) [1/2]: looking up the right zone [2/2]: allowing ports Done configuring Firewall (firewalld). ... The Service class derived object can be really simple, we would just reuse the functionality it already has + let us properly hook into it in ipa-{server,replica}-install and the uninstallation. It would also make it easier to split this functionality to freeipa-server-firewalld if we chose to in a future. In general I agree with the idea, thank you Justin for working on that! I would like to emphasis the necessity to work without NetworkManager and FirewallD. New dependencies make Debian folks unhappy ... On the other hand, it is perfectly fine to skip firewall configuration if NM/FirewallD/DBus is not available. Have a nice day! Should be easy, probe for the dbus firewalld service and just skip (not error out) if it is not there. Set a variable in that case that will cause the installer to throw the classic banner we have now which warns you about what ports need to be opened at the end of the install. Probably just need to spit out a large, preferably flashing warning that the firewall has not been automatically configured. Perhaps even multiple times: one in-line and one at the install summary at the end. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Thanks for looking into this! Would it be possible to summarize this thread in a design page on the wiki? -- Thank you, Dmitri Pal Sr. Engineering Manager IdM portfolio Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Ipa-server-install Firewall Support
Justin Brown wrote: Dmitri, I'd be more than happy to, but I'm having trouble figuring out where it should go. Could you send me a link to a similar design page? I'd put it under here: http://www.freeipa.org/page/V4_Proposals There is a template at http://www.freeipa.org/page/Feature_template So maybe something like http://www.freeipa.org/page/V4/Firewalld rob Thanks, Justin On Mon, Apr 7, 2014 at 6:51 PM, Dmitri Pal d...@redhat.com wrote: On 04/07/2014 09:00 AM, Rob Crittenden wrote: Simo Sorce wrote: On Fri, 2014-04-04 at 09:59 +0200, Petr Spacek wrote: On 4.4.2014 09:17, Martin Kosek wrote: On 04/04/2014 09:04 AM, Justin Brown wrote: I would actually do it the opposite way and open the ports after the FreeIPA server is fully configured. After all, I do not think we want to open the ports when the server is just half-configured and for example some ACIs are missing. My thinking was that nothing would be listening on these ports if the install doesn't succeed, but there's really necessity to modify the firewall configuration early. (All of the internal install communication will be over a local interface (to netfilter) and unblock anyways. I don't have any problem in delaying firewall configuration to the end of install. If ipa-server-install does succeed without configuring the firewalld, then we will indeed have no other option than to do it early. I am thinking that we may want to put all the firewalld configuration in ipaserver/install/firewalldinstance.py, and then make the firewalld configuration the actual step of the installation. Something like: ... Configuring Firewall (firewalld) [1/2]: looking up the right zone [2/2]: allowing ports Done configuring Firewall (firewalld). ... The Service class derived object can be really simple, we would just reuse the functionality it already has + let us properly hook into it in ipa-{server,replica}-install and the uninstallation. It would also make it easier to split this functionality to freeipa-server-firewalld if we chose to in a future. In general I agree with the idea, thank you Justin for working on that! I would like to emphasis the necessity to work without NetworkManager and FirewallD. New dependencies make Debian folks unhappy ... On the other hand, it is perfectly fine to skip firewall configuration if NM/FirewallD/DBus is not available. Have a nice day! Should be easy, probe for the dbus firewalld service and just skip (not error out) if it is not there. Set a variable in that case that will cause the installer to throw the classic banner we have now which warns you about what ports need to be opened at the end of the install. Probably just need to spit out a large, preferably flashing warning that the firewall has not been automatically configured. Perhaps even multiple times: one in-line and one at the install summary at the end. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Thanks for looking into this! Would it be possible to summarize this thread in a design page on the wiki? -- Thank you, Dmitri Pal Sr. Engineering Manager IdM portfolio Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] global account lockout
On Tue, 2014-04-08 at 12:00 +0200, Ludwig Krispenz wrote: Replication storms. In my opinion the replication of a mod of one or two attribute in a entry will be faster than the bind itself. Think about the amplification effect in an environment with 20 replicas. 1 login attempt - 20+ replication messages Now think about what happen bandwidth wise when a few thousand people all authenticate at the same time across the infrastructure, you deploy more servers to scale better and you get *more* traffic, at some point servers actually get slower as they are busy with replication related operations. Think what happen if one of these servers is in a satellite office on a relatively slow link and every morning it receives a flooding of replication data ... that is 99% useless because most of tat data is not relevant in that office. If an attacker knows all the dns of the entries in a server the denial of service could be that it just does a sequence of failed logins for any user and nobody will be able to login any more, This is perfectly true which is why we do not permanently lockout users by default and which is why I personally dislike lockouts. A much better mechanism to deal with brute force attacks is throttling, but it is also somewhat harder to implement as you need to either have an async model to delay answers or you need to tie threads for the delay time. Still a far superior measure than replicating status around at all times. replication would help to propagate this to other servers, but not prevent it. This would also be the case if only the final lockout state is replicated. Yes but the amount of replicated information would be far less. With our default 1/5th less on average as 5 is the number of failed attempts before the final lockout kicks in. So you save a lot of bandwidth. I like the idea of replicating the attributes changed at failed logins (or reset) only. I think this is reasonable indeed, the common case is that users tend to get their password right, and if you are under a password guessing attack you should stop it. The issue is though that sometimes you have misconfigured services with bad keytabs that will try over and over again to init, even if the account is locked, or maybe (even worse) they try a number of bad keys, but lower than the failed count, before getting to the right one (thus resetting the failed count). If they do this often you can still self-DoS even without a malicious attacker :-/ Something like this is what we have experienced for real and cause us to actually disable replication of all the lockout related attributes in the past. 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] [DOC] document that wildcards are not supported in FreeIPA = 3.2
Hello, Not sure how relevant this patch is to the current documentation considering (I believe) that wildcards are supported in versions 3.3 and up. Patch for https://fedorahosted.org/freeipa/ticket/3616 Thanks, Gabe From 1cc5d540027e1f01912263f83d6a2cceb0731cea Mon Sep 17 00:00:00 2001 From: Gabe redhatri...@gmail.com Date: Tue, 8 Apr 2014 20:58:58 -0600 Subject: [PATCH] [DOC] wildcards are not supported in IPA = 3.2 https://fedorahosted.org/freeipa/ticket/3616 --- src/user_guide/en-US/DNS.xml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/user_guide/en-US/DNS.xml b/src/user_guide/en-US/DNS.xml index e787db5d08279f5d113caf28dae29e972da8ebbb..4e6b9dd1562b179f1c4c48b60757ebee829003a5 100644 --- a/src/user_guide/en-US/DNS.xml +++ b/src/user_guide/en-US/DNS.xml @@ -89,6 +89,12 @@ /listitem /itemizedlist /note + important + titleIMPORTANT/title + para + In IPA; versions 3.2 and lower, wildcards cannot be used when configuring DNS names. Only explicit DNS domain names are supported. + /para + /important /section section id=dns-file titleThe IPA;-Generated DNS File/title -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel