search

coinbase / kryptology

Apache License 2.0
855 stars 125 forks source link

Fix test/dkg/ed25519/main.go generator. #33

Closed Olshansk closed 2 years ago

Olshansk commented 2 years ago

When creating a new DKG participant, the generator point must lie on the Edward's curve and cannot be a random point from a hardcoded test fixtures.

Tested with go run test/dkg/ed25519/main.go two commits behind head due to https://github.com/coinbase/kryptology/issues/30.

type=routine risk=low impact=sev5

Olshansk commented 2 years ago

@mikelodder7 Thanks for the response!

Like you said, there's definitely value in merging this in since the previous approach crashed, while the currently approach works but has flaws.

With that being said, I wanted to clarify my understanding and was hoping you could provide some additional details

  1. We do not need to worry about fixing point P and generating the curve because that is handled by curve := v1.Ed25519(). Is that correct?

  2. The current approach simply generates a random integer, not based off of any hash function that accepts bytes as input. The source of the entropy is what rand.Reader uses. Is there something I'm missing or misunderstanding?

  3. It seems like the other suggested approaches use several rounds of pseudorandom number generation and composing the results of hash functions, which I'm definitely not doing in this PR. In order to make sure I'm using this library without any security holes, do you know if it has an example of one of these approaches?