FactomProject / factomd

Factom Daemon
https://www.factomprotocol.org/
Other
201 stars 92 forks source link

Use official golang ed25519 package #634

Open PaulBernier opened 5 years ago

PaulBernier commented 5 years ago

Factomd has a dependency on fork https://github.com/FactomProject/ed25519/ The repo that it was forked from actually became the official ed25519 package of the Golang crypo library: https://godoc.org/golang.org/x/crypto/ed25519. I would recommend switching to this up to date package instead of the outdated fork. I know the reason for the fork was to add a function to avoid signature malleability, but this problem is actually now fixed in RFC 8032 which the official lib implement. You can see the exact commit here: https://github.com/golang/crypto/commit/c4a91bd4f524f10d064139674cf55852e055ad01#diff-8183aa2c95dc1385c8f58bf1917fcb0b

The change shouldn't be too difficult because this is pretty much the same library (so same structures) but just up to date with some useful new functiosn such as NewKeyFromSeed. Keeping its library up to date allows you to get fixes and improvements for "free".

AdamSLevy commented 5 years ago

@PaulBernier,

@carryforward expressed some concerns about switching crypto packages. I am confident that signatures generated and validated by the official crypto package are completely compatible with the signatures generated and validated by FactomProject's fork of the earlier package.

I expect that in the tests for both of these packages probably show this. If such tests don't exist I could write a test that compares the results from both packages.

I'm leaving this comment as a note for me or anyone who gets to it before me to find those tests and link them or write them.

Brian, would that help move this issue forward?

sanchopansa commented 5 years ago

@carryforward -- could you elaborate what are the concerns about switching to the official Golang library?

AdamSLevy commented 4 years ago

As of go 1.13 the "golang.org/x/crypto" package has been moved into the standard library, so "crypto/ed25519" now can be imported. It is the same as importing "golang.org/x/crypto", which is of course still available.