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

go/common/crypto/signature: Use Ed25519ph instead of the ad-hoc context support #2103

Closed
2 of 5 tasks
Yawning opened this issue Sep 17, 2019 · 3 comments
Closed
2 of 5 tasks
Assignees
Labels
c:security Category: security sensitive p:1 Priority: core feature s:wontfix Status: this will not be worked on

Comments

@Yawning
Copy link
Contributor

Yawning commented Sep 17, 2019

We have an ad-hoc method of signing with a context to our signatures, for the purpose of domain separation. It is probably a good idea to get rid of the ad-hoc construct and transition to using Ed25519ph from RFC 8032.

The standard library's Ed25519 implementation supports neither (yet), but will support Ed25519ph "soon". Note that we aren't exactly at the mercy of what the standard library does, assuming we transition to a different Ed25519 implementation, and implementing the variants is trivial.

  • Find/write a Go implementation of Ed25519ph/Ed25519ctx.
  • Done: Ours now supports both, and upstream will get Ed25519ph support in Go 1.14.
  • Add support for Ed25519ph/Ed25519ctx to our ring fork (or switch to a different implementation).
  • Done: Switched to ed25519-dalek (runtime: Switch the ed25519 implementation to -dalek #2106).
  • Switch to the new signing algorithm (Blocked on go: Use a faster ed25519 implementation #1533, devops signoff).
  • Use context based signing for tendermint and libp2p signatures.
  • Use more descriptive contexts, that are longer than 8 bytes.
@Yawning Yawning added p:1 Priority: core feature c:security Category: security sensitive labels Sep 17, 2019
@Yawning Yawning self-assigned this Sep 17, 2019
@Yawning
Copy link
Contributor Author

Yawning commented Sep 17, 2019

Unfortunately ring does not support either Ed25519ctx or Ed25519ph, and I have suspicions that the author will not accept support for either, because adding such likely goes against the goals of the project.

This is important enough that we could/should consider either forking ring (again) or using a different implementation if one that supports what we need already exists.

@Yawning
Copy link
Contributor Author

Yawning commented Sep 19, 2019

For what it's worth #1533 has most of this done. It is currently impossible to register a custom signing algorithm for use with tendermint without forking the code, but everything else seemed to transition gracefully.

@Yawning Yawning changed the title go/common/crypto/signature: Use Ed25519ctx instead of the ad-hoc context support go/common/crypto/signature: Use Ed25519ph instead of the ad-hoc context support Oct 3, 2019
@Yawning Yawning added the s:wontfix Status: this will not be worked on label Oct 4, 2019
@Yawning
Copy link
Contributor Author

Yawning commented Oct 4, 2019

Nevermind, supporting garbage HSMs that can't even sign 2 KiB of payload, or generate more than a handful of signatures in a second is apparently more important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:security Category: security sensitive p:1 Priority: core feature s:wontfix Status: this will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant