[crypto] Fast & Loose Key validation in FastNFT

[huitseeker]

The single error case of this TryFrom is just an invalid length error. There is a host of other problems completely ignored by the current implementation:

• there are sequences of 32 bytes that will structurally not ever be usable as an Ed25519 Public Key in any way,
• there are sequences of 32 bytes that will 100% be usable as an Ed25519 Public Key, but that demonstrably and unambiguously aim at tricking anybody who does so.

The above takes exactly none of that into account. Further, several of those checks will not be performed by check_internal's dalek::PublicKey::from_bytes (and the library has a nice warning to mention some of that).

I admit it's probably a completely orthogonal point to this PR, and worth tackling in a different issue (probably extracted from this comment), but I'd appreciate a spectacular comment on PublicKeyBytes making this clear. Here is an example of my personal minimum bar for the word "spectacular".

_Originally posted by @huitseeker in https://github.com/MystenLabs/fastnft/pull/94#discussion_r776341393_

[gdanezis]

This issue on ed25519 suggests that this is a doc bug, and no further validity check is needed: https://github.com/dalek-cryptography/ed25519-dalek/pull/150

This is also further evidenced by the docs on curve25519-dalek doc, that suggest that all edwards points are valid by construction: https://github.com/dalek-cryptography/curve25519-dalek/blob/main/src/edwards.rs#L79-L86

[gdanezis]

@huitseeker also says:

https://github.com/MystenLabs/fastnft/pull/122#issuecomment-1005744318

[huitseeker]

We are also dependent on verify_batch for performance, hence we must check for small subgroup points and more generally rogue key attacks. Checking public keys by validating they represent a point on curve is not enough.

On small subgroup components: https://github.com/novifinancial/ed25519-speccheck https://eprint.iacr.org/2020/1244

[kchalkias]

Due to verify batch we have to be consistent here (there are 2 reasons for that, but it's too complex to explain here - i.e., if we're not careful and use randomized batch sig verification inside contracts, then one could even break determinism - and result to liveness issues).

The zebra lib specifically designed their protocol to support that, and Diem's implementation was stricter than zebra (based on our EdDSA security paper). This will be part of the generic crypto agility effort (all of the algorithms will need a key validation mechanism to be on the safe side).