Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some possible improvements to encrypted/internal/e2c/e2c.go #4

Open
cathugger opened this issue Nov 1, 2021 · 0 comments
Open

some possible improvements to encrypted/internal/e2c/e2c.go #4

cathugger opened this issue Nov 1, 2021 · 0 comments

Comments

@cathugger
Copy link

ive noticed
https://github.com/Arceliar/ironwood/blob/main/encrypted/internal/e2c/e2c.go

func Ed25519PrivateKeyToCurve25519(pk ed25519.PrivateKey) []byte {
	h := sha512.New()
	h.Write(pk.Seed())
	out := h.Sum(nil)
	return out[:curve25519.ScalarSize]
}

not performing any of the usual scalar masking as done in https://github.com/teserakt-io/golang-ed25519/blob/master/extra25519/extra25519.go#L16-L26 for example.
it should probably be done as per https://neilmadden.blog/2020/05/28/whats-the-curve25519-clamping-all-about/
it's probably ~FINE because curve25519.ScalarMult used by nacl/box does it on its own but honestly it's not exactly clear whether all users of this value will do that, so I suggest doing it explicitly for brevity.

also Ed25519PublicKeyToCurve25519 should probably just use https://pkg.go.dev/github.com/teserakt-io/golang-ed25519/extra25519#PublicKeyToCurve25519 or https://pkg.go.dev/filippo.io/edwards25519#Point.BytesMontgomery instead of messing with bigints because bigints are kinda eh. likely slower and not constant-time (yes i know that derivering stuff from pubkey doesn't exactly require constant time).
also upstream this part was copied from changed as per FiloSottile/age@e63c22e

or you can just ignore this stuff because current code is kinda okay

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

No branches or pull requests

1 participant