[Freeipa-devel] [PATCH] 0106 Make host/service cert revocation aware of lightweight CAs

[Jan Cholasta]

On 7.9.2016 11:05, Fraser Tweedale wrote:
> On Wed, Sep 07, 2016 at 10:39:59AM +0200, Jan Cholasta wrote:
>> On 7.9.2016 10:28, Fraser Tweedale wrote:
>>> On Wed, Sep 07, 2016 at 08:32:42AM +0200, Jan Cholasta wrote:
>>>> On 6.9.2016 19:36, Fraser Tweedale wrote:
>>>>> On Tue, Sep 06, 2016 at 10:19:14AM +0200, Jan Cholasta wrote:
>>>>>> On 5.9.2016 17:30, Fraser Tweedale wrote:
>>>>>>> On Mon, Sep 05, 2016 at 11:59:11PM +1000, Fraser Tweedale wrote:
>>>>>>>> On Tue, Aug 30, 2016 at 10:39:16AM +0200, Jan Cholasta wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 26.8.2016 07:42, Fraser Tweedale wrote:
>>>>>>>>>> On Fri, Aug 26, 2016 at 03:37:17PM +1000, Fraser Tweedale wrote:
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> Attached patch fixes https://fedorahosted.org/freeipa/ticket/6221.
>>>>>>>>>>> It depends on Honza's PR #20
>>>>>>>>>>> https://github.com/freeipa/freeipa/pull/20.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Fraser
>>>>>>>>>>>
>>>>>>>>>> It does help to attach the patch :)
>>>>>>>>>
>>>>>>>>> I think it would be better to call cert-find once per host-del/service-del
>>>>>>>>> with the --host/--service option specified. That way you'll get all
>>>>>>>>> certificates for the given host/service at once.
>>>>>>>>>
>>>>>>>>> Honza
>>>>>>>>>
>>>>>>>> I agree that is a nicer approach.
>>>>>>>>
>>>>>>>> 'revoke_certs' is called from several other places besides just
>>>>>>>> host/service_del.  If we want to land this fix Real Soon I'd suggest
>>>>>>>> we either:
>>>>>>>>
>>>>>>>> A) Define function 'revoke_certs_from_cert_find', call it from
>>>>>>>> host/service_del, and leave 'revoke_certs' alone; or
>>>>>>>>
>>>>>>>> B) Land the patch as-is and do a bigger refactor at a later time.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>
>>>>> Updated patch attached; comments inline.
>>>>>
>>>>>> C) Use cert-find-based revoke_certs() everywhere; use the --certificate
>>>>>> option of cert-find in the other places to get information about specific
>>>>>> certificates.
>>>>>>
>>>>> As discussed on IRC, I have implemented this option.  The caveat is
>>>>> that for host/service-mod, we incur call to cert_find for each
>>>>> removed certificate.
>>>>
>>>> It's worth noting that A) and B) suffer from the same caveat.
>>>>
>>>>>
>>>>>>>>
>>>>>>> Updated patch for option (A) is attached.
>>>>>>
>>>>>> 1) Instead of
>>>>>>
>>>>>>         if result['status'] in {'REVOKED', 'REVOKED_EXPIRED'}:
>>>>>>
>>>>>> use:
>>>>>>
>>>>>>         if result['revoked']:
>>>>>>
>>>>> Done.
>>>>>
>>>>>>
>>>>>> 2)
>>>>>>
>>>>>> +        if 'cacn' not in cert:
>>>>>> +            # cert is known to Dogtag, but CA appears to have been
>>>>>> +            # deleted.  We cannot revoke this cert via IPA anymore.
>>>>>> +            # We could go directly to Dogtag to revoke it, but the
>>>>>> +            # issuer's cert should have been revoked so never mind.
>>>>>> +            continue
>>>>>>
>>>>>> Or, it could be a cert issued by a 3rd party CA.
>>>>>>
>>>>> I updated to comment to include this.
>>>>>
>>>>>>
>>>>>> 3) host-mod/service-mod do not revoke certs:
>>>>>>
>>>>>> $ ipa cert-request test.csr --principal host/test.example.com
>>>>>>   Serial number: 13
>>>>>>
>>>>>> $ ipa cert-show 13
>>>>>>   Revoked: False
>>>>>>   Owner host: test.example.com
>>>>>>
>>>>>> $ ipa host-mod test.example.com --certificate=
>>>>>>
>>>>>> $ ipa cert-show 13
>>>>>>   Revoked: False
>>>>>>
>>>>> Nice find.  This was a pre-existing bug: nothing gets revoked when
>>>>> all certs are removed.  Here is the fix:
>>>>>
>>>>>     -        if certs and self.api.Command.ca_is_enabled()['result']:
>>>>>     +        ca_is_enabled = self.api.Command.ca_is_enabled()['result']
>>>>>     +        if 'usercertificate' in options and ca_is_enabled:
>>>>>                  ... revocation code
>>>>
>>>> OK. Since it is a different bug, it should be fixed in a separate patch and
>>>> have a separate ticket.
>>>>
>>>>>
>>>>> Finally, host/service-remove-cert does not revoke the cert because
>>>>> of (I think) a bug in cert-find.  If the cert does not exist on a
>>>>> host/service the cert-find cannot find it with --certificate option.
>>>>> Because host/service-remove-cert uses a post_callback to revoke the
>>>>> cert, cert-find doesn't find it thus no revocation occurs.
>>>>>
>>>>> Honza could you check whether this is indeed a bug/limitation of
>>>>> cert-find or is it the smog in Saigon affecting me?
>>>>
>>>> It's a bug - FTFY, <https://github.com/freeipa/freeipa/pull/64>.
>>>>
>>>> Functional ACK. Full ACK once my fix is merged and the host/service-mod is
>>>> split off into a separate patch.
>>>>
>>> To clarify - you want only the fix discussed above in the separate
>>> patch?
>>
>> I want the fix for sub-CA revocation in one patch and the fix for
>> host/service-mod with empty --certificate in other patch.
>>
> Updated patch 106 attached.  Well send patch for other fix
> separately.

Thanks, ACK.

Pushed to:
master: daeaf2a8234ba684352d98fbc8d734100e6d63d1
ipa-4-4: d3f3869e6d496cfec2c9c02373f97ebafe73ce93

-- 
Jan Cholasta