Use X25519 instead of ScalarMult for safety #43
Conversation
|
The docs recommend also replacing |
There was a problem hiding this comment.
Surface level looks good from my read but I can't authoritatively speak to the differences between curve25519.X25519 and curve25519.ScalarMult, I assume very smart crypto folks worked this out for us.
The new version appears to be safer, with more error checks. I believe rollback happens appropriately in both ReadMessage and WriteMessage so I am largely unconcerned about these new underlying error conditions. The added panic is a little troubling but so is the condition that causes it.
I don't see a reason to object right now.
edit: I loaded this branch into nebula and tested on my home network with most hosts running the old curve25519.ScalarMult version, I did not observe any issues.
|
PTAL, I updated |
This upgrade takes care of issues UCB-02-003, UCB-02-004, and UCB-02-006 from the 2021 security audit of Turbo Tunnel by Cure53. GHSA-g9mp-8g3h-3c5c UCB-02-004 flynn/noise#43 UCB-02-003, UCB-02-006: flynn/noise#44
In practice we should never encounter errors, and this does not appear to be a security issue, but the ScalarMult function is deprecated (see golang/go#32670).
@AxbB36 @nbrownus @rawdigits