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

[Martin Basti]

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.

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0263-Fix-regression-in-host-and-service-plugin.patch
Type: text/x-patch
Size: 3309 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150603/ded21153/attachment.bin>