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

[Jan Cholasta]

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.

-- 
Jan Cholasta