Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-22 Thread Jan Zelený
Rob Crittenden rcrit...@redhat.com wrote:
 Jakub Hrozek wrote:
  On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:
  Rob Crittenden wrote:
  Jakub Hrozek wrote:
  -BEGIN PGP SIGNED MESSAGE-
  Hash: SHA1
  
  On 02/17/2011 04:35 AM, Rob Crittenden wrote:
  Add default roles and permissions for HBAC, SUDO and pw policy
  
  Created some default roles as examples. In doing so I realized that
  we were completely missing default rules for HBAC, SUDO and password
  policy so I added those as well.
  
  I ran into a problem when the updater has a default record and an add
  at the same time, it should handle it better now.
  
  ticket 585
  
  rob
  
  I'm not sure about the HBAC rules ACIs. They are specified as:
  
  'target = ldap:///cn=*,cn=hbac,$SUFFIX;'
  
  while HBAC rules' DN is:
  
  'ipauniqueid=*,cn=hbac,$SUFFIX'.
  
  But HBAC rules do have a cn: attribute, so maybe the ACIs would work?
  
  No, you're right, this is wrong. I'll fix it up and resubmit.
  
  The patch also needs rebasing on top of recent changes to
  install/updates/Makefile.am
  
  Other than that, looks OK to me.
  
  btw when I was reviewing this patch, I noticed we add a DNS
  Administrators privilege in dns.ldif. Would it make sense to add DNS
  administration to Security Architect (replication management) and
  IT Specialist (hosts management)?
  
  The DNS stuff is added only if DNS is enabled on the server so I can't
  add them by default.
  
  rob
  
  Updated patch.
  
  rob
  
  Interdiff looks fine, but I'm not able to apply the patch (not even
  3-way merge), can you rebase?
 
 done

The patch now applies ok (just one whitespace warning), ack

Jan

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-22 Thread Martin Kosek
On Tue, 2011-02-22 at 13:14 +0100, Jan Zelený wrote:
 Rob Crittenden rcrit...@redhat.com wrote:
  Jakub Hrozek wrote:
   On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:
   Rob Crittenden wrote:
   Jakub Hrozek wrote:
   -BEGIN PGP SIGNED MESSAGE-
   Hash: SHA1
   
   On 02/17/2011 04:35 AM, Rob Crittenden wrote:
   Add default roles and permissions for HBAC, SUDO and pw policy
   
   Created some default roles as examples. In doing so I realized that
   we were completely missing default rules for HBAC, SUDO and password
   policy so I added those as well.
   
   I ran into a problem when the updater has a default record and an add
   at the same time, it should handle it better now.
   
   ticket 585
   
   rob
   
   I'm not sure about the HBAC rules ACIs. They are specified as:
   
   'target = ldap:///cn=*,cn=hbac,$SUFFIX;'
   
   while HBAC rules' DN is:
   
   'ipauniqueid=*,cn=hbac,$SUFFIX'.
   
   But HBAC rules do have a cn: attribute, so maybe the ACIs would work?
   
   No, you're right, this is wrong. I'll fix it up and resubmit.
   
   The patch also needs rebasing on top of recent changes to
   install/updates/Makefile.am
   
   Other than that, looks OK to me.
   
   btw when I was reviewing this patch, I noticed we add a DNS
   Administrators privilege in dns.ldif. Would it make sense to add DNS
   administration to Security Architect (replication management) and
   IT Specialist (hosts management)?
   
   The DNS stuff is added only if DNS is enabled on the server so I can't
   add them by default.
   
   rob
   
   Updated patch.
   
   rob
   
   Interdiff looks fine, but I'm not able to apply the patch (not even
   3-way merge), can you rebase?
  
  done
 
 The patch now applies ok (just one whitespace warning), ack
 
 Jan
 
 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

I have to NACK this. I have found some issues in the new LDAP records:

1) A wrong groupdn for the following ACI in 40-delegation.update:
add:aci: '(target = ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX;)(version
3.0;acl permission:Add SUDO rule;allow (add) groupdn = ldap:///cn=Add
SUDOrule,cn=permissions,cn=pbac,$SUFFIX;)'

It should be dap:///cn=Add SUDO rule,cn=permissions,cn=pbac,$SUFFIX

2) Another wrong target for few ACIs:
ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX
is used instead of
ldap:///ipaUniqueID=*,cn=sudorules,cn=sudo,$SUFFIX


3) Missing Description for the following new privileges:
Write IPA Configuration
Modify Users and Reset passwords
Modify Group membership

Remainder looks good.

Martin


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-22 Thread Rob Crittenden

Martin Kosek wrote:

On Tue, 2011-02-22 at 13:14 +0100, Jan Zelený wrote:

Rob Crittendenrcrit...@redhat.com  wrote:

Jakub Hrozek wrote:

On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:

Rob Crittenden wrote:

Jakub Hrozek wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/17/2011 04:35 AM, Rob Crittenden wrote:

Add default roles and permissions for HBAC, SUDO and pw policy

Created some default roles as examples. In doing so I realized that
we were completely missing default rules for HBAC, SUDO and password
policy so I added those as well.

I ran into a problem when the updater has a default record and an add
at the same time, it should handle it better now.

ticket 585

rob


I'm not sure about the HBAC rules ACIs. They are specified as:

'target = ldap:///cn=*,cn=hbac,$SUFFIX;'

while HBAC rules' DN is:

'ipauniqueid=*,cn=hbac,$SUFFIX'.

But HBAC rules do have a cn: attribute, so maybe the ACIs would work?


No, you're right, this is wrong. I'll fix it up and resubmit.


The patch also needs rebasing on top of recent changes to
install/updates/Makefile.am

Other than that, looks OK to me.

btw when I was reviewing this patch, I noticed we add a DNS
Administrators privilege in dns.ldif. Would it make sense to add DNS
administration to Security Architect (replication management) and
IT Specialist (hosts management)?


The DNS stuff is added only if DNS is enabled on the server so I can't
add them by default.

rob


Updated patch.

rob


Interdiff looks fine, but I'm not able to apply the patch (not even
3-way merge), can you rebase?


done


The patch now applies ok (just one whitespace warning), ack

Jan

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


I have to NACK this. I have found some issues in the new LDAP records:

1) A wrong groupdn for the following ACI in 40-delegation.update:
add:aci: '(target = ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX;)(version
3.0;acl permission:Add SUDO rule;allow (add) groupdn = ldap:///cn=Add
SUDOrule,cn=permissions,cn=pbac,$SUFFIX;)'

It should be dap:///cn=Add SUDO rule,cn=permissions,cn=pbac,$SUFFIX

2) Another wrong target for few ACIs:
ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX
is used instead of
ldap:///ipaUniqueID=*,cn=sudorules,cn=sudo,$SUFFIX


3) Missing Description for the following new privileges:
Write IPA Configuration
Modify Users and Reset passwords
Modify Group membership

Remainder looks good.

Martin


Thanks for the careful review. Updated patch attached.

rob


freeipa-rcrit-728-4-roles.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-22 Thread Martin Kosek
On Tue, 2011-02-22 at 09:22 -0500, Rob Crittenden wrote:
 Martin Kosek wrote:
  On Tue, 2011-02-22 at 13:14 +0100, Jan Zelený wrote:
  Rob Crittendenrcrit...@redhat.com  wrote:
  Jakub Hrozek wrote:
  On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:
  Rob Crittenden wrote:
  Jakub Hrozek wrote:
  -BEGIN PGP SIGNED MESSAGE-
  Hash: SHA1
 
  On 02/17/2011 04:35 AM, Rob Crittenden wrote:
  Add default roles and permissions for HBAC, SUDO and pw policy
 
  Created some default roles as examples. In doing so I realized that
  we were completely missing default rules for HBAC, SUDO and password
  policy so I added those as well.
 
  I ran into a problem when the updater has a default record and an add
  at the same time, it should handle it better now.
 
  ticket 585
 
  rob
 
  I'm not sure about the HBAC rules ACIs. They are specified as:
 
  'target = ldap:///cn=*,cn=hbac,$SUFFIX;'
 
  while HBAC rules' DN is:
 
  'ipauniqueid=*,cn=hbac,$SUFFIX'.
 
  But HBAC rules do have a cn: attribute, so maybe the ACIs would work?
 
  No, you're right, this is wrong. I'll fix it up and resubmit.
 
  The patch also needs rebasing on top of recent changes to
  install/updates/Makefile.am
 
  Other than that, looks OK to me.
 
  btw when I was reviewing this patch, I noticed we add a DNS
  Administrators privilege in dns.ldif. Would it make sense to add DNS
  administration to Security Architect (replication management) and
  IT Specialist (hosts management)?
 
  The DNS stuff is added only if DNS is enabled on the server so I can't
  add them by default.
 
  rob
 
  Updated patch.
 
  rob
 
  Interdiff looks fine, but I'm not able to apply the patch (not even
  3-way merge), can you rebase?
 
  done
 
  The patch now applies ok (just one whitespace warning), ack
 
  Jan
 
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
 
  I have to NACK this. I have found some issues in the new LDAP records:
 
  1) A wrong groupdn for the following ACI in 40-delegation.update:
  add:aci: '(target = ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX;)(version
  3.0;acl permission:Add SUDO rule;allow (add) groupdn = ldap:///cn=Add
  SUDOrule,cn=permissions,cn=pbac,$SUFFIX;)'
 
  It should be dap:///cn=Add SUDO rule,cn=permissions,cn=pbac,$SUFFIX
 
  2) Another wrong target for few ACIs:
  ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX
  is used instead of
  ldap:///ipaUniqueID=*,cn=sudorules,cn=sudo,$SUFFIX
 
 
  3) Missing Description for the following new privileges:
  Write IPA Configuration
  Modify Users and Reset passwords
  Modify Group membership
 
  Remainder looks good.
 
  Martin
 
 Thanks for the careful review. Updated patch attached.
 
 rob

Good job! Its OK now. ACK

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-22 Thread Rob Crittenden

Martin Kosek wrote:

On Tue, 2011-02-22 at 09:22 -0500, Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2011-02-22 at 13:14 +0100, Jan Zelený wrote:

Rob Crittendenrcrit...@redhat.com   wrote:

Jakub Hrozek wrote:

On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:

Rob Crittenden wrote:

Jakub Hrozek wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/17/2011 04:35 AM, Rob Crittenden wrote:

Add default roles and permissions for HBAC, SUDO and pw policy

Created some default roles as examples. In doing so I realized that
we were completely missing default rules for HBAC, SUDO and password
policy so I added those as well.

I ran into a problem when the updater has a default record and an add
at the same time, it should handle it better now.

ticket 585

rob


I'm not sure about the HBAC rules ACIs. They are specified as:

'target = ldap:///cn=*,cn=hbac,$SUFFIX;'

while HBAC rules' DN is:

'ipauniqueid=*,cn=hbac,$SUFFIX'.

But HBAC rules do have a cn: attribute, so maybe the ACIs would work?


No, you're right, this is wrong. I'll fix it up and resubmit.


The patch also needs rebasing on top of recent changes to
install/updates/Makefile.am

Other than that, looks OK to me.

btw when I was reviewing this patch, I noticed we add a DNS
Administrators privilege in dns.ldif. Would it make sense to add DNS
administration to Security Architect (replication management) and
IT Specialist (hosts management)?


The DNS stuff is added only if DNS is enabled on the server so I can't
add them by default.

rob


Updated patch.

rob


Interdiff looks fine, but I'm not able to apply the patch (not even
3-way merge), can you rebase?


done


The patch now applies ok (just one whitespace warning), ack

Jan

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


I have to NACK this. I have found some issues in the new LDAP records:

1) A wrong groupdn for the following ACI in 40-delegation.update:
add:aci: '(target = ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX;)(version
3.0;acl permission:Add SUDO rule;allow (add) groupdn = ldap:///cn=Add
SUDOrule,cn=permissions,cn=pbac,$SUFFIX;)'

It should be dap:///cn=Add SUDO rule,cn=permissions,cn=pbac,$SUFFIX

2) Another wrong target for few ACIs:
ldap:///cn=*,cn=sudorules,cn=sudo,$SUFFIX
is used instead of
ldap:///ipaUniqueID=*,cn=sudorules,cn=sudo,$SUFFIX


3) Missing Description for the following new privileges:
Write IPA Configuration
Modify Users and Reset passwords
Modify Group membership

Remainder looks good.

Martin


Thanks for the careful review. Updated patch attached.

rob


Good job! Its OK now. ACK

Martin



pushed to master

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-21 Thread Rob Crittenden

Rob Crittenden wrote:

Jakub Hrozek wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/17/2011 04:35 AM, Rob Crittenden wrote:

Add default roles and permissions for HBAC, SUDO and pw policy

Created some default roles as examples. In doing so I realized that we
were completely missing default rules for HBAC, SUDO and password policy
so I added those as well.

I ran into a problem when the updater has a default record and an add at
the same time, it should handle it better now.

ticket 585

rob



I'm not sure about the HBAC rules ACIs. They are specified as:

'target = ldap:///cn=*,cn=hbac,$SUFFIX;'

while HBAC rules' DN is:

'ipauniqueid=*,cn=hbac,$SUFFIX'.

But HBAC rules do have a cn: attribute, so maybe the ACIs would work?


No, you're right, this is wrong. I'll fix it up and resubmit.



The patch also needs rebasing on top of recent changes to
install/updates/Makefile.am

Other than that, looks OK to me.

btw when I was reviewing this patch, I noticed we add a DNS
Administrators privilege in dns.ldif. Would it make sense to add DNS
administration to Security Architect (replication management) and IT
Specialist (hosts management)?


The DNS stuff is added only if DNS is enabled on the server so I can't
add them by default.

rob


Updated patch.

rob


freeipa-rcrit-728-2-roles.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-21 Thread Jakub Hrozek
On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:
 Rob Crittenden wrote:
 Jakub Hrozek wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 On 02/17/2011 04:35 AM, Rob Crittenden wrote:
 Add default roles and permissions for HBAC, SUDO and pw policy
 
 Created some default roles as examples. In doing so I realized that we
 were completely missing default rules for HBAC, SUDO and password policy
 so I added those as well.
 
 I ran into a problem when the updater has a default record and an add at
 the same time, it should handle it better now.
 
 ticket 585
 
 rob
 
 
 I'm not sure about the HBAC rules ACIs. They are specified as:
 
 'target = ldap:///cn=*,cn=hbac,$SUFFIX;'
 
 while HBAC rules' DN is:
 
 'ipauniqueid=*,cn=hbac,$SUFFIX'.
 
 But HBAC rules do have a cn: attribute, so maybe the ACIs would work?
 
 No, you're right, this is wrong. I'll fix it up and resubmit.
 
 
 The patch also needs rebasing on top of recent changes to
 install/updates/Makefile.am
 
 Other than that, looks OK to me.
 
 btw when I was reviewing this patch, I noticed we add a DNS
 Administrators privilege in dns.ldif. Would it make sense to add DNS
 administration to Security Architect (replication management) and IT
 Specialist (hosts management)?
 
 The DNS stuff is added only if DNS is enabled on the server so I can't
 add them by default.
 
 rob
 
 Updated patch.
 
 rob

Interdiff looks fine, but I'm not able to apply the patch (not even
3-way merge), can you rebase?



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-21 Thread Rob Crittenden

Jakub Hrozek wrote:

On Mon, Feb 21, 2011 at 10:11:38AM -0500, Rob Crittenden wrote:

Rob Crittenden wrote:

Jakub Hrozek wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/17/2011 04:35 AM, Rob Crittenden wrote:

Add default roles and permissions for HBAC, SUDO and pw policy

Created some default roles as examples. In doing so I realized that we
were completely missing default rules for HBAC, SUDO and password policy
so I added those as well.

I ran into a problem when the updater has a default record and an add at
the same time, it should handle it better now.

ticket 585

rob



I'm not sure about the HBAC rules ACIs. They are specified as:

'target = ldap:///cn=*,cn=hbac,$SUFFIX;'

while HBAC rules' DN is:

'ipauniqueid=*,cn=hbac,$SUFFIX'.

But HBAC rules do have a cn: attribute, so maybe the ACIs would work?


No, you're right, this is wrong. I'll fix it up and resubmit.



The patch also needs rebasing on top of recent changes to
install/updates/Makefile.am

Other than that, looks OK to me.

btw when I was reviewing this patch, I noticed we add a DNS
Administrators privilege in dns.ldif. Would it make sense to add DNS
administration to Security Architect (replication management) and IT
Specialist (hosts management)?


The DNS stuff is added only if DNS is enabled on the server so I can't
add them by default.

rob


Updated patch.

rob


Interdiff looks fine, but I'm not able to apply the patch (not even
3-way merge), can you rebase?


done


freeipa-rcrit-728-3-roles.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 728 default roles

2011-02-18 Thread Jakub Hrozek
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/17/2011 04:35 AM, Rob Crittenden wrote:
 Add default roles and permissions for HBAC, SUDO and pw policy
 
 Created some default roles as examples. In doing so I realized that we
 were completely missing default rules for HBAC, SUDO and password policy
 so I added those as well.
 
 I ran into a problem when the updater has a default record and an add at
 the same time, it should handle it better now.
 
 ticket 585
 
 rob
 

I'm not sure about the HBAC rules ACIs. They are specified as:

'target = ldap:///cn=*,cn=hbac,$SUFFIX;'

while HBAC rules' DN is:

'ipauniqueid=*,cn=hbac,$SUFFIX'.

But HBAC rules do have a cn: attribute, so maybe the ACIs would work?

The patch also needs rebasing on top of recent changes to
install/updates/Makefile.am

Other than that, looks OK to me.

btw when I was reviewing this patch, I noticed we add a DNS
Administrators privilege in dns.ldif. Would it make sense to add DNS
administration to Security Architect (replication management) and IT
Specialist (hosts management)?
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk1eirkACgkQHsardTLnvCUSeACgzxH00FEw+065sYEji+hlOkZQ
nBQAniLmDvUV24cnqw3bArlBckAl5gsL
=O/zW
-END PGP SIGNATURE-

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel