TestInvalidFuzzed is flaky

[KyleMaas]

This has apparently been happening enough that it's part of the code for the test:

https://github.com/ssbc/go-ssb/blob/master/message/legacy/verify_invalid_test.go#L20

Where it's crashing is here:

https://github.com/ssbc/go-ssb/blob/master/message/legacy/verify_invalid_test.go#L82

So...I think we've got some data in testdata-fuzzed.json which is invalid.

See #237

[KyleMaas]

Specifically, it's this message:

go-ssb/message/legacy/testdata-fuzzed.json

Line 90 in 9fe9976

{

[KyleMaas]

Ah, it has a signature that's too long and is overflowing the decoding buffer.

[KyleMaas]

Yeah, I think this is actually a bug in the test data. ed25519 signatures should always be the same length, AFAIK. In the data, they're not.

[KyleMaas]

@decentral1se

Mind sanity checking my assumptions on this? From what I understand, an ed25519 signature should always result in an output of 64 bytes (SignatureSize from crypto/ed25519 ), correct?

[decentral1se]

@KyleMaas I think you're on to something suspecting the test data!

For the length, yep, it's also tested also further down:

go-ssb/message/legacy/testdata-fuzzed.json

Lines 914 to 918 in d83e578

	"signature": "RjXbPdILiOAUZHvhCz+/aZz4gHxG9w1IB/KipZXvjt93vLpadnvZhwLUNgm7hZlc9TLt0HhBPjAiVFPIbfjMAwLUNgm7hZlc9TLt0HhBPjAiVFPIbfjMAw==.sig.ed25519"
	},
	"error": "Signature must decode to a value with 64 bytes",
	"valid": false,
	"id": "%dxYBn6ufvrC1URGE9oWh+YQxBnZT2gUNdl5LW5HpzH4=.sha256"

Removing that entry does start to make the test pass again. So perhaps we just need to set some guard deeper down in the stack to avoid the explosion and bubble up an error? I guess the code was changed at some point and stopped bubbling errors up... (has valid: false in the test data...)

Somewhere here?

go-ssb/message/legacy/signature.go

Line 24 in d83e578

func ExtractSignature(b []byte) ([]byte, Signature, error) {

[KyleMaas]

@decentral1se

I tried adding a check here in my experimental branch:

532060c

But it's still broken.

However, knowing that it's always supposed to decode as 64 bytes definitely gives me something to go on.

[KyleMaas]

I think I figured it out. It's due to a bug related to data being padded and the length measurements not taking that into account.

[KyleMaas]

So...not a problem with the test data. It was actually a bug in the signature check due to problems with how the data decoded with padding. So this was actually quite useful for catching that.

[decentral1se]

Brilliant @KyleMaas! Great to get a fix out for something so core as sig validation 👏 👏 👏

[KyleMaas]

Thanks! Glad we could get this one picked off the list.