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? _______________________________________________ Pki-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/pki-devel
