Re: [Freeipa-devel] [PATCH] Add DRM to IPA

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Petr Spacek

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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Alexander Bokovoy

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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Alexander Bokovoy

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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Alexander Bokovoy

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

2014-04-08 Thread Petr Spacek

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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Petr Spacek

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

2014-04-08 Thread Petr Viktorin

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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Petr Spacek

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

2014-04-08 Thread Petr Viktorin

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

2014-04-08 Thread Petr Viktorin


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

2014-04-08 Thread Ludwig Krispenz

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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Petr Viktorin

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

2014-04-08 Thread Jan Cholasta

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

2014-04-08 Thread Sumit Bose
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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Petr Viktorin

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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Rob Crittenden

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

2014-04-08 Thread Rob Crittenden

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

2014-04-08 Thread Ade Lee
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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Alexander Bokovoy

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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Ade Lee
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

2014-04-08 Thread Martin Kosek
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

2014-04-08 Thread Misnyovszki Adam
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

2014-04-08 Thread Petr Viktorin

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

2014-04-08 Thread Petr Viktorin

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

2014-04-08 Thread Petr Viktorin

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

2014-04-08 Thread James
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

2014-04-08 Thread Justin Brown
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

2014-04-08 Thread Rob Crittenden

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

2014-04-08 Thread Simo Sorce
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

2014-04-08 Thread Gabe Alford
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