[Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates

[Milan Kubik]

On 06/03/2015 01:17 PM, Martin Basti wrote:
> On 02/06/15 16:03, Jan Cholasta wrote:
>> Dne 2.6.2015 v 12:36 Martin Basti napsal(a):
>>> On 02/06/15 11:42, Fraser Tweedale wrote:
>>>> On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote:
>>>>> On 01/06/15 06:40, Fraser Tweedale wrote:
>>>>>> New version of patch; ``{host,service}-show --out=FILE`` now writes
>>>>>> all certs to FILE.  Rebased on latest master.
>>>>>>
>>>>>> Thanks,
>>>>>> Fraser
>>>>>>
>>>>>> On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote:
>>>>>>> Updated patch attached.  Notably restores/adds revocation behaviour
>>>>>>> to host-mod and service-mod.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Fraser
>>>>>>>
>>>>>>> On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:
>>>>>>>> On 27/05/15 15:53, Fraser Tweedale wrote:
>>>>>>>>> This patch adds supports for multiple user / host 
>>>>>>>>> certificates.  No
>>>>>>>>> schema change is needed ('usercertificate' attribute is already
>>>>>>>>> multi-value).  The revoke-previous-cert behaviour of host-mod and
>>>>>>>>> user-mod has been removed but revocation behaviour of -del and
>>>>>>>>> -disable is preserved.
>>>>>>>>>
>>>>>>>>> The latest profiles/caacl patchset (0001..0013 v5) depends on 
>>>>>>>>> this
>>>>>>>>> patch for correct cert-request behaviour.
>>>>>>>>>
>>>>>>>>> There is one design question (or maybe more, let me know): the
>>>>>>>>> `--out=FILENAME' option to {host,service} show saves ONE 
>>>>>>>>> certificate
>>>>>>>>> to the named file.  I propose to either:
>>>>>>>>>
>>>>>>>>> a) write all certs, suffixing suggested filename with either a
>>>>>>>>>     sequential numerical index, e.g. "cert.pem" becomes
>>>>>>>>>     "cert.pem.1", "cert.pem.2", and so on; or
>>>>>>>>>
>>>>>>>>> b) as above, but suffix with serial number and, if there are
>>>>>>>>>     different issues, some issuer-identifying information.
>>>>>>>>>
>>>>>>>>> Let me know your thoughts.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Fraser
>>>>>>>>>
>>>>>>>>>
>>>>>>>> Is there a possible way how to store certificates into one file?
>>>>>>>> I read about possibilities to have multiple certs in one .pem
>>>>>>>> file, but I'm
>>>>>>>> not cert guru :)
>>>>>>>>
>>>>>>>> I personally vote for serial number in case there are multiple
>>>>>>>> certificates,
>>>>>>>> if ^ is no possible.
>>>>>>>>
>>>>>>>>
>>>>>>>> 1)
>>>>>>>> +            if len(certs) > 0:
>>>>>>>>
>>>>>>>> please use only,
>>>>>>>> if certs:
>>>>>>>>
>>>>>>>> 2)
>>>>>>>> You need to re-generate API/ACI.txt in this patch
>>>>>>>>
>>>>>>>> 3)
>>>>>>>> syntax error:
>>>>>>>> +        for dercert in certs_der
>>>>>>>>
>>>>>>>>
>>>>>>>> 4)
>>>>>>>> command
>>>>>>>> ipa user-mod ca_user --certificate=<ceritifcate>
>>>>>>>>
>>>>>>>> removes the current certificate from the LDAP, by design.
>>>>>>>> Should be the old certificate(s) revoked? You removed that part in
>>>>>>>> the code.
>>>>>>>>
>>>>>>>> only the --addattr='usercertificate=<cert>' appends new value 
>>>>>>>> there
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> Martin Basti
>>>>>>>>
>>>>> My objections/proposed solutions in attached patch.
>>>>>
>>>>> * VERSION
>>>>> * In the previous version normalized values was stored in LDAP, so I
>>>>> added
>>>>> it back.  (I dont know why there is no normalization in param
>>>>> settings, but
>>>>> normalization for every certificate is done in callback. I will 
>>>>> file a
>>>>> ticket for this)
>>>>> * IMO only normalized certificates should be compared in the old
>>>>> certificates detection
>>>>>
>>>> I incorporated your suggested changes in new patch (attached).
>>>>
>>>> There were no proposed changes to the other patchset (0001..0013)
>>>> since rebase.
>>>>
>>>> Thanks,
>>>> Fraser
>>> Thank you,
>>> ACK
>>> Martin^2
>>>
>>
>> Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212
>>
> Regression found.
>
> Patch to fix the issue is attached.
>
The fix works, thanks.

Milan