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?
Thanks, Fraser -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
