Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-05-02 Thread Fraser Tweedale
On Fri, Apr 22, 2016 at 07:50:06PM -0400, John Magne wrote: > I took a look at the stuff alee asked for. > > CFU even took a quick look when I asked her a couple of questions. > She was unsure of something (as was I) and she would like to be able > to take a closer look next week. I will give my q

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-23 Thread Fraser Tweedale
ubkey.getAlgorithm().equals("EC") > +? PrivateKey.Type.EC > +: PrivateKey.Type.RSA; > +return wrapper.unwrapPrivate(encPrivKey.getBits(), keyType, pubkey); > +} > > > - Original Message - > > From: "Fraser Tweed

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-22 Thread John Magne
; > Cc: pki-devel@redhat.com > Sent: Wednesday, April 20, 2016 9:58:54 PM > Subject: Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication > support > > Thanks Ade. Updated patch 0096 attached. Comments inline. > > On Wed, Apr 20, 2016 at 11:30:52AM -0400

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-21 Thread Fraser Tweedale
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 Tweed

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-21 Thread Ade Lee
ACK on latest 96 and 99. I will ask cfu or jmagne to look at the KeyRetrieveRunner logic today. Ade 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:

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-20 Thread Fraser Tweedale
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

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-20 Thread Ade Lee
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()? 2. In addInstanceToAuthorityKeyHosts(), you are still using only the hostname. Should be host:port 3. The logic in the KeyRetrieverRunner class looks O

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-19 Thread Fraser Tweedale
New version of 0097 attached (0097-4). The only change is some minor improvements to the pki-ipa-retrieve-key Python program. Cheers, Fraser On Tue, Apr 19, 2016 at 07:32:16PM +1000, Fraser Tweedale wrote: > Both issues addressed in latest patchset. Two new patches in the > mix; the order is: >

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-19 Thread Fraser Tweedale
Both issues addressed in latest patchset. Two new patches in the mix; the order is: 0095-4, 0098, 0099, 0096-4, 0097-3 (tip) I also added another attribute to schema for the authority certificate serial number. It is not used in current code but I have a hunch it may be needed for renewal,

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-14 Thread Fraser Tweedale
On Thu, Apr 14, 2016 at 04:35:10PM -0400, Ade Lee wrote: > Still reviewing .. ACK on 87-95 (inclusive). > Thanks. I pushed 87..94 to master. I left 95 for now, in case of further schema changes. 6d72a9c7fc067df42a3259fc5ea87b65e94f76ad Lightweight CAs: add exceptions for missing signing key o

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-14 Thread Fraser Tweedale
On Thu, Apr 14, 2016 at 05:34:45PM -0400, Ade Lee wrote: > Couple of points on 96/97. > > 1. First off, I'm not sure you followed my concern about being able to > distinguish between CA instances. > > On an IPA system, this is not an issue because there is only one CA on > the server. In this ca

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-14 Thread Ade Lee
Couple of points on 96/97. 1. First off, I'm not sure you followed my concern about being able to distinguish between CA instances. On an IPA system, this is not an issue because there is only one CA on the server. In this case, I imagine there will be a well known directory which custodia would

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-14 Thread Ade Lee
Still reviewing .. ACK on 87-95 (inclusive). On Thu, 2016-04-14 at 16:18 +1000, Fraser Tweedale wrote: > On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote: > > On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote: > > > Still reviewing .. > > > > > > See comment on 87. ACK on 88

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-13 Thread Fraser Tweedale
On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote: > On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote: > > Still reviewing .. > > > > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95. > > > > Ade > > > > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote: > > > Tha

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-13 Thread Fraser Tweedale
On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote: > Still reviewing .. > > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95. > > Ade > > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote: > > Thanks for review, Ade. Comments to specific feedback inline. > > Rebased and updated

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-13 Thread Ade Lee
96-2 looks like it does not apply. Please rebase . On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote: > Thanks for review, Ade. Comments to specific feedback inline. > Rebased and updated patches attached. The substantive changes are: > > - KeyRetriever implementations are now required

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-13 Thread Ade Lee
Still reviewing .. See comment on 87. ACK on 88,89,90,91,92,93, 94, 95. Ade On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote: > Thanks for review, Ade. Comments to specific feedback inline. > Rebased and updated patches attached. The substantive changes are: > > - KeyRetriever implem

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-10 Thread Fraser Tweedale
Thanks for review, Ade. Comments to specific feedback inline. Rebased and updated patches attached. The substantive changes are: - KeyRetriever implementations are now required NOT to import the key themselves. Instead the API is updated with KeyRetriever.retrieveKey returning a Result, whi

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-04-08 Thread Ade Lee
On Thu, 2016-03-31 at 16:37 +1000, Fraser Tweedale wrote: > Ade, thanks for the review. I address your comments inline. > Updated patches are attached, and there are new patches implementing > schema changes and the key acquisition framework. Pursuant to > comment (4), I have split patch 0084 int

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-03-30 Thread Fraser Tweedale
Ade, thanks for the review. I address your comments inline. Updated patches are attached, and there are new patches implementing schema changes and the key acquisition framework. Pursuant to comment (4), I have split patch 0084 into six different patches. To keep things in order I retire patch n

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-03-24 Thread Ade Lee
A few comments. 1. One of the first things that struck me as odd was making CertificateAuthority implement Runnable. I think it would be cleaner to have a static inner class called AuthorityMonitor or similar to which we pass in the CertificateAuthority. 2. I do like the fact that the caMap upda

Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-03-21 Thread Fraser Tweedale
On Fri, Mar 18, 2016 at 02:30:24PM +1000, Fraser Tweedale wrote: > Hi all, > > The attached patches implement replication support for lightweight > CAs. These patches do not implement key replication via Custodia > (my next task) but they do implement the persistent search thread > and appropriat

[Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support

2016-03-19 Thread Fraser Tweedale
Hi all, The attached patches implement replication support for lightweight CAs. These patches do not implement key replication via Custodia (my next task) but they do implement the persistent search thread and appropriate** API behaviour when the signing keys are not yet available. ** In most ca