Conceptual feedback on new ECC JEP

[Adam Petcher]

On 9/4/2018 2:01 PM, Michael StJohns wrote:

> Below
>
> *buzz* wrong answer.   Sorry.   The internal storage of the key can be 
> anything you want it to be if you want to prevent a non-constant-time 
> issue for normal calculation.  But the import/export of the key really 
> isn't subject to the cargo cult "must not branch" dogma - hint - I'm 
> moving across a security boundary line anyway.    So if I do a 
> "getEncoded()" or a "getS()" on an ECPrivateKey object or provide a 
> BigInteger on an ECPrivateKeySpec there is no additional threat over 
> the presence of the private key bits in public space.

I think what you are suggesting is that the implementation should 
convert between BigInteger and the internal representation when 
necessary. The problem with this approach is that it is too easy to 
inadvertently supply a BigInteger to the implementation, and this would 
result in a branch. I understand that this branch may be acceptable in 
some circumstances, but we would need something in the API to tell the 
implementation whether it is okay to branch or not. I think the simplest 
way to do that is to have a provider that never branches. If branching 
is okay, then SunEC can be used.

>
> If you believe this to be such a problem, then I'd suggest instead 
> updating BigInteger to allow for BigEndian or LittleEndian encoding 
> input/output and fix the constant time issue there inside the 
> implementation rather than breaking public APIs.

BigInteger wasn't designed for this sort of thing, and changing it so 
that supports constant-time encoding/decoding is a massive undertaking, 
if it is even possible. It would be more reasonable to add a new public 
type for integers that supports constant-time operations, but I don't 
think that helps us here.

>
> Alternately, I guess you could throw some sort of exception if someone 
> tries to call getS() or getEncoded().  Or you could do what PKCS11 
> does for non-extractable private keys and only class the private key 
> as a PrivateKey - make it opaque.  But then how do you create the key 
> from stored information?

That's essentially the plan. Calling PrivateKey::getEncoded will return 
null, which is a convention for non-extractable keys. Trying to convert 
from/to an ECPrivateKeySpec using the KeyFactory in the new provider 
will result in an exception---so you won't have an object to call getS() 
on.

To create the key from stored information, the best way is to construct 
a PKCS8EncodedKeySpec using the encoded key. If you are starting with a 
BigInteger, and if branching is acceptable, you can use the KeyFactory 
from SunEC to convert an ECPrivateKeySpec to PrivateKey to get the 
encoded value.