Skip to content
This repository has been archived by the owner on Nov 16, 2021. It is now read-only.

ed25519: double scalarmult fix - return full point #172

Merged
merged 1 commit into from
Aug 21, 2018

Conversation

ph4r05
Copy link
Contributor

@ph4r05 ph4r05 commented Aug 20, 2018

Double scalar multiplication returns fully valid ED point (Just one more multiplication).

  • Makes further operations easier because many operations expect full point (with valid T coordinate), not the partial.
  • ge25519_scalarmult_base_niels already returns full point so it would make it more consistent
  • Scalar multiplications perform a large number of curve25519_mul, typically (64*const), one more before returning from the function is IMO small overhead.
  • if ge25519_scalarmult returns partial point one cannot easily make it the full point because after ge25519_scalarmult returns, the temporary point is not accessible. To make it full point much more expensive inversion would be needed.

If you prefer not to add multiplication there is another alternative - a bit more difficult IMO. We would have to generalize scalarmult method to return ge25519_p1p1 point so we can make it both partial and full points. There would be then scalarmult method which is a simple wrapper for scalarmult returning ge25519_p1p1 and making a partial point from it.

I personally like the proposed idea more because it is backward compatible change with small overhead, is consistent with scalarmult base and does not make API more complex.

@ph4r05 ph4r05 force-pushed the ed25519-double-scalarmult branch from f362d12 to 9c41c0f Compare August 20, 2018 13:17
@ph4r05
Copy link
Contributor Author

ph4r05 commented Aug 20, 2018

Similar to
#171

- return fully valid ed point
@ph4r05 ph4r05 force-pushed the ed25519-double-scalarmult branch from 9c41c0f to c64b97a Compare August 21, 2018 15:19
@onvej-sl
Copy link
Contributor

ACK

@prusnak prusnak merged commit 72da171 into trezor:master Aug 21, 2018
@prusnak
Copy link
Member

prusnak commented Aug 21, 2018

Thx!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants