tari-project / tari-crypto

Tari Cryptography library
BSD 3-Clause "New" or "Revised" License
20 stars 25 forks source link

Enforce domain separation on Schnorr signatures #178

Open AaronFeickert opened 1 year ago

AaronFeickert commented 1 year ago

Currently, Schnorr signatures allow for an optional domain separator, and use a default domain separator if one is not provided.

This is risky, and has already led to unintentional misuse.

While using a default domain separator helps with simplicity, it may be more trouble than it's worth.

I think the implementation should be changed to mandate a specified domain separator, which forces the implementer to at least think about, and hopefully opt for, safe choices.

CjS77 commented 1 year ago

My opinion is that the current API is a good balance of ease-of-use and safety and prefer the status quo.

There may be other ways to flag that domain separation should be used in "production settings", though. One example might be compiler warnings.

AaronFeickert commented 1 year ago

As noted, there is (to my knowledge) no common or established standard for Schnorr-type signatures over the Ristretto group. However, there is a defined ciphersuite in the draft FROST IRTF specification. While FROST was built for threshold signatures, verification is more general.

Unfortunately, it chooses not to define a per-context domain separator, instead using a global one. I happen to think this is a bad idea, but it does provide some kind of "default" signature, albeit with the same cross-context risks.

If that kind of maybe-someday-standard compliance is desired, there would need to be additional changes to how the hashing API works, since the linked ciphersuite specification doesn't do things like length prepending, relying heavily on certain fixed-length encodings (which I happen to think is unnecessarily brittle for no good reason).

bgorlick commented 4 months ago

I added a small fix to the fn, whether it gets used or not, up to you guys. Analysis here seems rational.