Sign.signedMessageToKey() can't be called because the Sign.SignatureData class is not public.

[androlo]

I looked around but couldn't find any way to create instances of SignatureData - could have missed it though.

Use case:

I like to use crypto utilities but I always do Ethereum signatures through RPC/IPC calls to the client (eth_sign); that way the private keys never have to leave the clients secure boundaries. To utilize signedMessageToKey I have to be able to create instances of SignatureData though.

There is no RPC binding for doing ecrecover through the client. Being able to call this function, either by creating a SignatureData object manually or if there is an overloaded version which takes a string, would be great.

[conor10]

Makes sense. You're welcome to submit a PR too :)

[androlo]

Ok I will.

[eztierney]

I ran into the same thing yesterday (though I was trying to use signMessage). I put up a simple PR to make SignatureData public: #65

[androlo]

I am making one with a function that takes signature as byte[], which means you don't really need access to any of the internal classes, and a convenience method signedMessageToAddress with the same parameters, but it returns the Ethereum address instead of the public key (so really the same as ecrecover). Going to add the tests today, or maybe tomorrow, and submit.

Did this in Solidity btw. so I'm used to Ethereum crypto (https://github.com/androlo/standard-contracts#crypto).

[androlo]

Seems the implementation is missing some fundamentals. For example, signatures are not properly validated before doing recovery. The only check i find is at the start of the recovery function (https://github.com/web3j/web3j/blob/master/src/main/java/org/web3j/crypto/Sign.java#L97), in which both r and s are (explicitly) allowed to lie outside of their respective ranges.

Here is an example of an invalid signature passing recovery without exceptions, running from within the Sign test file:

    @Test
    public void testInvalidSignature() throws SignatureException {
    	BigInteger signedMessageToKey = Sign.signedMessageToKey(TEST_MESSAGE, new SignatureData((byte) 27, new byte[]{1}, new byte[]{0})); // s=0
    	System.out.println(signedMessageToKey.toString(16)); // 60ce6c46097eb1d137670db055e7d8af842e368f265e2eabe572f89e704b58083893f06452e0cc9b8ee13f7677a8d00eef8ff8a28363fadb400c77f308980b60
    }

I use signature recovery in bank software (and there are deadlines etc) so will go with an alternative, for now, though I could maybe help with some work later, if needed.

Didn't go over the signing but I think recovery will automatically work if the signature is created by the library itself, since those signatures looks like they're always well-formed. I would not recommend supporting user-provided signatures though, until this has been addressed.

Will leave open since the comment about the other PR is in here.

[conor10]

So you'd like to see explicit bounds checked on e, v, r, and s, as per the yellow paper?

ECDSARECOVER(e ∈ B32, v ∈ B1, r ∈ B32, s ∈ B32)

Was there anything else?