search

randombit / botan

Cryptography Toolkit
https://botan.randombit.net
BSD 2-Clause "Simplified" License
2.59k stars 570 forks source link

Split loading and generating ECC private key c'tors #4437

Closed reneme closed 1 week ago

reneme commented 1 week ago

Currently, the ECC private keys provide two constructors:

  1. taking an AlgorithmIdentifier and key bits
  2. taking an RNG, an EC_Group and an optional secret value as BigInt

The second option either generates a new key or loads the optional secret value within the provided domain. If a user wants to load an ECC private key for which they don't have an AlgorithmIdentifier handy, they have to use this second constructor and "hope" that they'll use it correctly. E.g. by passing a Null_RNG to at least get some error if used incorrectly.

This PR deprecates the second constructor and replaces it by two new ones:

  1. taking an RNG and a domain, to perform a key generation
  2. taking a domain and a secret value, to perform a key loading

Users that never call the now-deprecated constructor with the optional secret value won't ever see the deprecation. Their code will now just bind to the new "generating" constructor. This is achieved by making the secret value mandatory in the now-deprecated constructor.

coveralls commented 1 week ago

Coverage Status

coverage: 91.072% (-0.001%) from 91.073% when pulling be5bbc2a04b52b8d0f294b1fd8fb1bf45ad9b666 on Rohde-Schwarz:feature/ecc_constructor into 162a3890fb0bcad46adb72a37ea30bfab81331d2 on randombit:master.

reneme commented 1 week ago

To be considered: When using the now-deprecated constructor we have access to an RNG. This allows us to do a random blinding when performing the multiplication with the base point, to get the associated public key. The new "key loading" constructor doesn't have an RNG and hence won't be able to use random blinding.

However, the same is true for the existing (and probably much more useful) constructor taking an AlgorithmIdentifier.

randombit commented 1 week ago

Thanks! I had this on the docket re #4027

Re the removed RNG being used for blinding, indeed I think historically that is the reason for this (kind of weird in retrospect) combined generator-or-load constructor that took an RNG instance.