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

[Jan Cholasta]

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.

-- 
Jan Cholasta