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

[Jan Cholasta]

Dne 3.6.2015 v 17:44 Martin Basti napsal(a):
> On 03/06/15 15:21, Fraser Tweedale wrote:
>> On Wed, Jun 03, 2015 at 01:55:47PM +0200, Milan Kubik wrote:
>>> 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
>> Thanks for finding, fixing and testing!  ACK from me.
>>
>> I also present a fix of my own.  It fixes a problem where
>> service-mod deleted all certificates when
>> '--addattr usercertificate=XXX' was used instead of
>> '--usercertificate=XXX' options.
>>
>> Cheers,
>> Fraser
> ACK
>

Pushed both to master: 62e98671142cbc30366109a2a1b631c1ef0cae5c

-- 
Jan Cholasta