[Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support
Fraser Tweedale
ftweedal at redhat.com
Fri Apr 22 00:03:49 UTC 2016
On Thu, Apr 21, 2016 at 10:29:32AM -0400, Ade Lee wrote:
> ACK on latest 96 and 99.
>
> I will ask cfu or jmagne to look at the KeyRetrieveRunner logic today.
>
> Ade
>
Thanks Ade; I'll wait for their feedback before I merge it.
Cheers,
Fraser
> On Thu, 2016-04-21 at 14:58 +1000, Fraser Tweedale wrote:
> > Thanks Ade. Updated patch 0096 attached. Comments inline.
> >
> > On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
> > > Comments:
> > >
> > > 95 - ack
> > >
> > > 96 -
> > >
> > > 1. You have made the return type of initSigUnit() to be boolean.
> > > Should you be checking the return value in init()?
> > >
> > It is not needed to check it here; only when re-entering init from
> > the KeyReplicatorRunner thread.
> >
> > > 2. In addInstanceToAuthorityKeyHosts(), you are still using only
> > > the
> > > hostname. Should be host:port
> > >
> > Good pickup. Fixed in latest patch.
> >
> > > 3. The logic in the KeyRetrieverRunner class looks OK to me, but
> > > I'd
> > > like cfu and/or jmagne to check it and make sure we are calling the
> > > right primitives to wrap/unwrap inside the cryptographic token.
> > >
> > > Also I'd like them to confirm that this would wor for an HSM.
> > > Statements like the following make me question that:
> > > CryptoToken token = manager.getInternalKeyStorageToken()
> > >
> > It won't work on HSM. Can I get an HSM to test with? ;) I've filed
> > a ticket for HSM support[1]. FreeIPA does not yet support HSM[2] so
> > I think we can put it in 10.4 milestone (I've put it there for now).
> >
> > [1] https://fedorahosted.org/pki/ticket/2292
> > [2] https://fedorahosted.org/freeipa/ticket/5608
> >
> > > 4. Can you explain what happens if for some reason the script fails
> > > to
> > > retrieve the key? Do we end up retrying later and if so, when?
> > >
> > If the script fails to retrieve the key, it does not retry
> > automatically. I filed a ticket[3] to implement retry with
> > backoff (this patchset is big enough already!) and put it in
> > 10.3.1 milestone (that's up for discussion).
> >
> > [3] https://fedorahosted.org/pki/ticket/2293
> >
> > Right now, the following events cause authority reinitialisation,
> > entailing key retrieval if necessary:
> >
> > - Dogtag is restarted
> > - LDAP disconnect-reconnect
> > - LDAP modification of authority replicated from another clone
> >
> > > 97- ACK
> > >
> > > 98 - ACK
> > >
> > Thanks. Any feedback on patch 0099?
More information about the Pki-devel
mailing list