Conceptual feedback on new ECC JEP

[Michael StJohns]

Below

On 9/4/2018 8:57 AM, Adam Petcher wrote:
> On 9/1/2018 2:03 PM, Michael StJohns wrote:
>
>> On 8/23/2018 1:50 PM, Adam Petcher wrote:
>>> It will only support a subset of the API that is supported by the 
>>> implementation in SunEC. In particular, it will reject any private 
>>> keys with scalar values specified using BigInteger (as in 
>>> ECPrivateKeySpec), and its private keys will not return scalar 
>>> values as BigInteger (as in ECPrivateKey.getS()). 
>>
>> Um... why?   EC Private keys are integers.... I've said this multiple 
>> times and - with the single exception of EDDSA keys because of a very 
>> idiosyncratic (and IMHO short-sighted) RFC specification - all of the 
>> EC private keys of whatever curve can be expressed as integers.
>>
>
> The explanation is in the JEP:
>
> "The existing API for ECC private keys has some classes that specify 
> private scalar values using BigInteger. There is no way to get a value 
> out of a BigInteger (into, for example, a fixed-length array) without 
> branching."
>
> There is no problem with making private keys integers in the API. The 
> problem is specifically with BigInteger and its implementation. 
> BigInteger stores the value in the shortest int array possible. To 
> access the value, you need to branch on the length of the array, which 
> leaks whether the high-order bits of the private key are 0.
>
>

*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.

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.

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?

Later, Mike