[Freeipa-devel] [PATCHES 0001-0011 v3] Profile management

[Martin Basti]

On 20/05/15 16:41, Fraser Tweedale wrote:
> Hi Honza, Martin et al,
>
> Latest patches attached.  On top of previous patches (most review
> matters addressed**) patches 0008..0011 add support for profiles and
> user certificates to `ipa cert-request'.
>
> ** those that were not are being tracked at [1]; please add anything
>     I missed.
>
> Some points to note:
>
> - usercertificate is not yet a multi-valued attribute for users,
>    hosts and services.
>
>    QUESTION - we do want to allow multiple certificates for all
>    principal types, not just users?  Or have I got that wrong.
Changing schema can cause issues in future, we already burn ourselves 
several times.
If you plan to have multi valued attribute in close future, could be 
better to have mutltivalued schema now, instead of make this change in 
future?
>
> - "DN and SAN match principal" checks are not implemented for users
>    yet.
>
> - ACL was added to allow user principals to request their own
>    certificates, however, this will be further subject to CA/profile
>    ACLs which are to come.
>
> - Pursuant to [2] revocation logic was removed from `cert-request'
>
> [1] http://idm.etherpad.corp.redhat.com/rhel72-cert-mgmt-progress
> [2] http://www.freeipa.org/page/V4/User_Certificates#Revocation_of_the_Certificates
>
> Thanks,
> Fraser
>   
Thanks for updated patches:

Please update required pki version in freeipa.spec.in

Patch 0001:
Again, will be this change done after upgrade, instead of new installation?
config.set("CA", "pki_profiles_in_ldap", "True")

I blame method 'ca_enable_ldap_profile_subsystem' in upgrade to do this, 
but I'm not sure.


Patch 0005:
1)
just nitpick:
profile_id_pattern = re.compile('^[a-zA-Z]\w*$')
PROFILE_ID_PATTERN = re.compile('^profileId=(\w+)', re.MULTILINE)

In PROFILE_ID_PATTERN should be '^profileId=([a-zA-Z]\w*)' to be consistent

Patch 0007:
There are still modifications in ipa-server-isntall

Patch 0010:
1)
+aci: (targetattr = "usercertificate")(version 3.0;acl 
"selfservice:Users can manage their own X.509 certificates";allow 
(write) userdn = "ldap:///self";)
  This is not in any upgrade file

2)
+# User certificates
+dn: $SUFFIX
+add:aci:(targetattr = "ipasshpubkey")(version 3.0;acl 
"selfservice:Users can manage their own X.509 certificates";allow 
(write) userdn = "ldap:///self";)
Is this right? (ipasshpubkey) This ACI with different name is already there.

3)
IMO this should not be there.

              'replaces': [
-                '(targetattr = "givenname || sn || cn || displayname || 
title || initials || loginshell || gecos || homephone || mobile || pager 
|| facsimiletelephonenumber || telephonenumber || street || roomnumber 
|| l || st || postalcode || manager || secretary || description || 
carlicense || labeleduri || inetuserhttpurl || seealso || employeetype 
|| businesscategory || ou || mepmanagedentry || objectclass")(target = 
"ldap:///uid=*,cn=users,cn=accounts,$SUFFIX")(version 3.0;acl 
"permission:Modify Users";allow (write) groupdn = "ldap:///cn=Modify 
Users,cn=permissions,cn=pbac,$SUFFIX";)',
+                '(targetattr = "givenname || sn || cn || displayname || 
title || initials || loginshell || gecos || homephone || mobile || pager 
|| facsimiletelephonenumber || telephonenumber || street || roomnumber 
|| l || st || postalcode || manager || secretary || description || 
carlicense || labeleduri || inetuserhttpurl || seealso || employeetype 
|| businesscategory || usercertificate || ou || mepmanagedentry || 
objectclass")(target = 
"ldap:///uid=*,cn=users,cn=accounts,$SUFFIX")(version 3.0;acl 
"permission:Modify Users";allow (write) groupdn = "ldap:///cn=Modify 
Users,cn=permissions,cn=pbac,$SUFFIX";)',
              ],

this replace replaces permissions written in old style. New permission 
are updated automaticaly (AFAIK)

PAtch 0011:
Please dont use '_' in code, this is for  ugettext and may cause an 
issue in future
+        bind_service, _, _ = bind_principal

Martin^2

-- 
Martin Basti