botan
botan copied to clipboard
randombit
Add EC_Scalar and EC_AffinePoint types
This is the other side of #3979 where we push the interface that's exposed to the rest of the library to a non-BigInt oriented model.
#4027
coverage: 91.693% (-0.03%) from 91.726% when pulling 8042e9d380805e4cf61383224074358dc59c30b3 on jack/new-ec-types into 7fad1d22d223f3e92ecfa6f67954b5892b30ab8a on master.
I will go through this in the coming days. Now with the underlying PR merged. 👍
BTW I should mention in terms of reviewing, the overall structure behind the new types isn't that important since it's going to change quite a bit to handle the possibility of different backend implementations. That part is literally just the first thing that came to mind that works. Whereas EC_Scalar and EC_AffinePoints interfaces, and how they are used to implement the various schemes, are IMO "final" and so should deserve more scrutiny.
OK with eb4f1bda91a13a8f72f3b3a70945143cbd616746 in place, previous comment can be disregarded. I think this is more or less the "final" [*] approach for implementing the new scalar and point types. You can see in #4143 what the bridge looks like for pcurves.
[*] Not ideal in any way, I'd like there to be fewer allocations, dynamic_cast, etc in here - but it works and doesn't seem to have that much overhead in practice. I expect we'll iterate on this in the future. A lot of what I'd like to do isn't possible until we've eliminated the real weirdo curves and all the deprecated interfaces, EC_Point, etc. [**]
[**] Not technically true - we could get away with a third internal EC implementation - classic BigInt approach, pcurves, plus a third generic curve but bounded size impl. But it just doesn't seem worth it from a complexity perspective. I expect that in Botan 4, with the restrictions on curves in place, we'll transition away from BigInt entirely within EC.
Deferring to 3.6 - it's better that all of this cook in master for a few months before we commit to anything wrt public API
Was not sure if you were planning further review of this
I believe I wasn't fully done but no need to block the merge.
OK I'm going to go ahead and merge. Feel free to do a retro-review anytime, we have plenty of time between now and 3.6