coinbase / kryptology

Apache License 2.0
847 stars 123 forks source link

DST computation isn't very computationy #20

Closed seebs closed 2 years ago

seebs commented 2 years ago

The spec linked for the hashing function using the "DST" value talks about creating a value, but in fact this is just a static string which doesn't contain some of the suggested information, or vary in the expected ways. See:

https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-hash-to-curve-10#section-3.1

  1. Tags SHOULD begin with a fixed identification string that is unique to the application.

  2. Tags SHOULD include a version number.

  3. For applications that define multiple ciphersuites, each ciphersuite's tag MUST be different. For this purpose, it is RECOMMENDED to include a ciphersuite identifier in each tag.

  4. For applications that use multiple encodings, either to the same curve or to different curves, each encoding MUST use a different tag. For this purpose, it is RECOMMENDED to include the encoding's Suite ID (Section 8) in the domain separation tag. For independent encodings based on the same suite, each tag should also include a distinct identifier, e.g., "ENC1" and "ENC2".

Contrast with:

    DST := []byte("Coinbase_tECDSA")

    [...]
    uniformBytes, err := ExpandMessageXmd(f, msg, DST, lenInBytes)

which uses a fixed string of length 15, one less than the RECOMMENDED 16 minimum.

mikelodder7 commented 2 years ago

We should probably remove this function since we have found a better method for hashing to scalar fields. I'm going to propose the removal here. Anyone reading this, feel free to submit a PR.

mikelodder7 commented 2 years ago

See #45