On 03/19/2014 02:33 PM, Jan Cholasta wrote:
On 13.3.2014 13:41, Jan Cholasta wrote:
On 12.3.2014 19:59, Petr Viktorin wrote:
Certmonger is not configured/started in CA-less installs.

That's expected.


I tested fresh installs and upgrades; renewals work fine for me.

161-184 look OK

185: one more nitpick:
     cert = entry['usercertificate'][0]
Shouldn't that use entry.single_value?

I did not feel like changing this, because this is used in the original
code and the userCertificate LDAP attribute is multi-value.

Could you add a comment saying we don't care which of the certificates is returned? For me `entry[...][0]` is a red flag, since the order usually stays the same but it's not guaranteed, so it can change in the worst moment. If nothing else we shouldn't be leaving it in the code as an example of ipaldap usage.



186-189 look OK

190: Is
     fqdn = entries[0].dn[1].value
     return api.env.host == fqdn
safe? Can they differ in case, for example?

I guess so, will fix.


191-196 look OK

Note that patches 178 & 179 were already pushed. Also, patch 190 was
changed to store information about which CA instance is master in LDAP.

Updated patches attached.

Note that I changed the path for CSR export to /var/lib/ipa/ca.csr to
make it more SELinux-friendly (not in the policy yet, see
<https://bugzilla.redhat.com/show_bug.cgi?id=1077689>).



--
PetrĀ³

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

Reply via email to