trezor / trezor-mcu

:lock: Don't use this repo, use the new monorepo instead:
https://github.com/trezor/trezor-firmware
GNU Lesser General Public License v3.0
318 stars 255 forks source link

Ethereum Signing messages V value #418

Closed juanfranblanco closed 6 years ago

juanfranblanco commented 6 years ago

I am working on the integration of Trezor with Nethereum and I am confused with the V value returned when signing a message. R and S are ok with the Ethereum prefix but I get a value of "e0" for V instead of "1c" (28)

The private key I am using is: "0x2e14c29aaecd1b7c681154d41f50c4bb8b6e4299a431960ed9e860e39cae6d29" Message: "Hello"

And output from Nethereum is:

0x51fecae8077056185bf25a2bd32670134f28b24b01cb5c10b0a0ebd3dffd3a676ab0b498e26f319450c76da9631e34f76ebf70cd7aee41c821609faa95c195e01c

and the ouput from Trezor is:

0x51fecae8077056185bf25a2bd32670134f28b24b01cb5c10b0a0ebd3dffd3a676ab0b498e26f319450c76da9631e34f76ebf70cd7aee41c821609faa95c195e0e0

Thoughts?

juanfranblanco commented 6 years ago

Confirmed it seems that the last byte is being copied to the V position, a new test shows the same byte at 63 and 64.

matejcik commented 6 years ago

I'm not sure how you are signing the message. Can you provide seed+path that produce the private key?

In any case, in my testing the last byte is not copied.

prusnak commented 6 years ago

How do you communicate with Trezor?

Which Trezor model and firmware do you use?

TBH I think this is an Nethereum issue, because Trezor does not serialize the signature into a hex string. It returns raw R, S and V values as numbers.

@juanfranblanco can you show me the code which takes the EthereumMessageSignature message (which contains the R, S and V values) and creates the hex serialization?

juanfranblanco commented 6 years ago

@matejcik @prusnak thanks for the quick response.

My seed is the following:

deposit van crouch super viable electric bamboo nephew hold whip nation ankle wonder bottom win bomb dog search globe shrug primary spy limb knock

I am using https://github.com/MelbourneDeveloper/Trezor.Net, but they are just starting on the integration with Ethereum. Note this is just the Message signing.. (transaction signing is another story, but things first)

The R + S returned are valid for both, when signing in Nethereum they are the same (this is the normal Nethereum)

 var signer = new Nethereum.Signer.EthereumMessageSigner();
                        var messageSignature = signer.EncodeUTF8AndSign("Hello", new EthECKey(
                            "0x2e14c29aaecd1b7c681154d41f50c4bb8b6e4299a431960ed9e860e39cae6d29"));

Here I get "0x51fecae8077056185bf25a2bd32670134f28b24b01cb5c10b0a0ebd3dffd3a676ab0b498e26f319450c76da9631e34f76ebf70cd7aee41c821609faa95c195e01c", so this is R+S+V (v = 28)

When connecting to Trezor.. using Trezor.Net using the following (not all out of the box.. ) as I am trying to make it work :)

   var message = new EthereumSignMessage
                        {
                            AddressNs = KeyPath.Parse("m/44'/60'/0'/0/0").Indexes,
                            Message = Encoding.UTF8.GetBytes("Hello").ToHex().ToHexBytes()
                        };

                        var messageSignature = await trezorManager.SendMessageAsync<EthereumMessageSignature, EthereumSignMessage>(message);

Trezor.Net uses as a response the following proto, which seems correct based on your other stuff, and obviously I get an ouput ;).

[ProtoBuf.ProtoContract()]
    public class EthereumMessageSignature : ProtoBuf.IExtensible
    {
        private ProtoBuf.IExtension __pbn__extensionData;
        ProtoBuf.IExtension ProtoBuf.IExtensible.GetExtensionObject(bool createIfMissing)
            => ProtoBuf.Extensible.GetExtensionObject(ref __pbn__extensionData, createIfMissing);

        [ProtoBuf.ProtoMember(1, Name = @"address")]
        public byte[] Address { get; set; }
        public bool ShouldSerializeAddress() => Address != null;
        public void ResetAddress() => Address = null;

        [ProtoBuf.ProtoMember(2, Name = @"signature")]
        public byte[] Signature { get; set; }
        public bool ShouldSerializeSignature() => Signature != null;
        public void ResetSignature() => Signature = null;
    }

Here the output from Trezor is the following, after converting to Hex the signature byte array 0x51fecae8077056185bf25a2bd32670134f28b24b01cb5c10b0a0ebd3dffd3a676ab0b498e26f319450c76da9631e34f76ebf70cd7aee41c821609faa95c195e0e0

As you can see the result signature matches Nethereums, apart from the V value which is the same as the penultimate byte value in the array.

prusnak commented 6 years ago

I just did the test in Python and it works as expected:

$ trezorctl load_device -m 'deposit van crouch super viable electric bamboo nephew hold whip nation ankle wonder bottom win bomb dog search globe shrug primary spy limb knock'

$ trezorctl ethereum_sign_message -n "m/44'/60'/0'/0/0" "Hello"
Please confirm action on your Trezor device
message: Hello
address: 0x6a1d4583b83e5ef91eea1e591ad333bd04853399
signature: 0x51fecae8077056185bf25a2bd32670134f28b24b01cb5c10b0a0ebd3dffd3a676ab0b498e26f319450c76da9631e34f76ebf70cd7aee41c821609faa95c195e01c

So the problem is in the .NET code.

juanfranblanco commented 6 years ago

@prusnak thank you :D that really helps, and apologies for wasting your time !

prusnak commented 6 years ago

Can you inspect the contents of messageSignature returned here in the debugger/watch?

var messageSignature = await trezorManager.SendMessageAsync<EthereumMessageSignature, EthereumSignMessage>(message);

If it shows the bad V value it is a problem in Trezor.Net. If it shows the correct V value, the problem is further, most probably in the hex serialization of the message.

juanfranblanco commented 6 years ago

Yes I have found the problem now in Trezor.Net the last byte was reading the penultimate byte for all the messages. Thanks again @prusnak https://github.com/MelbourneDeveloper/Trezor.Net/blob/master/src/Trezor.Net/Manager/TrezorManagerBase.cs#L309

prusnak commented 6 years ago

Great!

MelbourneDeveloper commented 6 years ago

@juanfranblanco sorry for the mistake. I will work with you to get it fixed.

MelbourneDeveloper commented 6 years ago

@prusnak thanks for investigating!