Input validation on SECP256K1 address generation

[hornc]

Background
I didn't appreciate that the blake2b hashing algorithm will hash any data to produce an otherwise valid looking f1... SECP256k1 filecoin address.

While stepping through the process, I used a compressed SECP256k1 key (33 bytes starting with 02 or 03) as input and got a valid looking address as output, and it took me some time to backtrack and realise my mistake :)

This code looks like it is doing some input validation:

go-address/address.go

Lines 225 to 228 in 5769c0d

	case SECP256K1, Actor:
	if len(payload) != PayloadHashLength {
	return Undef, ErrInvalidPayload
	}

ensuring the resulting hash is 20 bytes long (PayloadHashLength), but that is set by the config passed to the hash() function

go-address/constants.go

Line 65 in 5769c0d

var payloadHashConfig = &blake2b.Config{Size: PayloadHashLength}

, so will always produce a 20 byte result, unless the code is changed. There is no way a user can trigger this validation via input.

This is academic, but I'm interested to know whether all possible hashed data producing f1... filecoin address will always have a corresponding uncompressed SECP256k1 public key (65 bytes beginning with 04), or whether some otherwise valid addresses are impossible to access. (i.e. there is no key that could hash to it). Are there enough hash collisions in blake2b to allow this?

Suggestions

In order to protect others from falling into the same trap and generating unintentionally inaccessible addresses:

• Add some input validation on SECP256k1 address generation to reject any data which is not an uncompressed SECP256k1 public key, i.e. exactly 65 byte beginning with an 04. There doesn't seem to be any sensible way to pad or infer a correct key from any other data payload (that I can see). There doesn't seem to be any valid reason to run the algorithm with data other than a valid uncompressed public key -- it's most likely to be an error. (experimenting with blake2b hash collisions is the only reason I can think of ...).

• Add / improve documentation on the cmd/fcaddr example tool to make it clear that a valid uncompressed SECP256k1 public key is required to get the expected behaviour. The current messaging "converting Secp256k1 to address" is wrong if the user didn't provide Secp256k1 data. It is simply converting any provided data to an f1... style address by blake2b hashing.

The command line tool could accept compressed SECP256k1 public keys to be helpful, but I'm not sure it's worth adding that logic so long as the uncompressed requirement is clear. This is a filecoin utility, not a secp256k1 util.

Please let me know if I'm missing anything with the above. I'm still learning about Filecoin and trying to help others from my mistakes.

Examples for testing
I believe this is the lowest sensible input value: (uncompressed public key representing 0,0 on the secp265k1 curve, in hex)

./fcaddr -type=secp256k1 <<< 0400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

Examples of invalid, non-SECP256K1 input, producing otherwise valid addresses:

./fcaddr -type=secp256k1 <<< 00
f1bavntex3o2drym5bxgmtucbjkl7kzjpguc7yuzq

./fcaddr -type=secp256k1 <<< 0000
f1mtlotjg4lohjmvwvxubbrxqyspatkel7lzgzb2y