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

Sat Apr 23 09:21:47 UTC 2016

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 quick thoughts.
> 
> 1. I agree that HSM support is not in the patch, seems fine to move that
> to a future ticket.
> 
Thanks.  I filed ticket: https://fedorahosted.org/pki/ticket/2292

> Here is one thing I was kind of worried about:
> This is the code that imports the archive of the desired private key.
> 
> 
> ublic static PrivateKey importPKIArchiveOptions(
> +            CryptoToken token, PrivateKey unwrappingKey,
> +            PublicKey pubkey, byte[] data)
> +            throws InvalidBERException, Exception {
> +        ByteArrayInputStream in = new ByteArrayInputStream(data);
> +        PKIArchiveOptions options = (PKIArchiveOptions)
> +            (new PKIArchiveOptions.Template()).decode(in);
> +        EncryptedKey encKey = options.getEncryptedKey();
> +        EncryptedValue encVal = encKey.getEncryptedValue();
> +        AlgorithmIdentifier algId = encVal.getSymmAlg();
> +        BIT_STRING encSymKey = encVal.getEncSymmKey();
> +        BIT_STRING encPrivKey = encVal.getEncValue();
> 
> This the wrapper object that is build off of the caSigningUnit key gotten
> in the other patch, the RetrieverThread like this:
> 
> 
> 
>  PrivateKey unwrappingKey = hostCA.mSigningUnit.getPrivateKey();
> 
> 
> 
> The code below works fine if said key is RSA. I talked over with CFU and she said there
> could be a chance this key is ECC for an ECC CA.
> 
> We both think the rest of the code in this routine is fine, except for possibly that.
> She is also not even sure if JSS can support an ECC private key wrapper.
> 
Yes, it is currently not supported in JSS (I'm unsure if NSS
supports it).  However, because the first release of Lightweight CAs
is to support FreeIPA sub-CAs feature, and FreeIPA does not yet
support EC CA, I don't think it's a show-stopper.  I filed a ticket
for key replication with non-RSA CA:

https://fedorahosted.org/pki/ticket/2291

> She requests you guys give her a day or two to look at it.
> 
No problem.  Thank you (and Christina) for the review.

Cheers,
Fraser

> Except for the hsm issue, the code that calls this routine in the thread seems fine too.
> 
> +
> +        KeyWrapper wrapper = token.getKeyWrapper(KeyWrapAlgorithm.RSA);
> +        wrapper.initUnwrap(unwrappingKey, null);
> 
> 
> 
> 
> 
> 
> +        SymmetricKey sk = wrapper.unwrapSymmetric(
> +            encSymKey.getBits(), SymmetricKey.Type.DES3, 0);
> +
> +        ASN1Value v = algId.getParameters();
> +        v = ((ANY) v).decodeWith(new OCTET_STRING.Template());
> +        byte iv[] = ((OCTET_STRING) v).toByteArray();
> +        IVParameterSpec ivps = new IVParameterSpec(iv);
> +
> +        wrapper = token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
> +        wrapper.initUnwrap(sk, ivps);
> +        PrivateKey.Type keyType = pubkey.getAlgorithm().equals("EC")
> +            ? PrivateKey.Type.EC
> +            : PrivateKey.Type.RSA;
> +        return wrapper.unwrapPrivate(encPrivKey.getBits(), keyType, pubkey);
> +    }
> 
> 
> ----- Original Message -----
> > From: "Fraser Tweedale" <ftweedal at redhat.com>
> > To: "Ade Lee" <alee at redhat.com>
> > Cc: pki-devel at 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, 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
> > Pki-devel at redhat.com
> > https://www.redhat.com/mailman/listinfo/pki-devel