raiden-network / raiden

Raiden Network
https://developer.raiden.network
Other
1.84k stars 378 forks source link

`LockedTransfer` message from LC deemed invalid #7052

Closed palango closed 3 years ago

palango commented 3 years ago

While running the LC e2e tests I found expired transfers. They seem to be caused by python client not accepting certain messages by the LC.

One example for such a denied message:

{
  "message_data": "{\"type\":\"LockedTransfer\",\"message_identifier\":\"1621260201713\",\"chain_id\":\"4321\",\"token_network_address\":\"0x32626F60da0aF910293EcdeC2123932db4138A81\",\"channel_identifier\":\"2\",\"nonce\":\"1\",\"transferred_amount\":\"0\",\"locked_amount\":\"100\",\"locksroot\":\"0x70f5bc325544293c71c6dbd0b47ce3acc0c8dc1dc2459c2606d9e6a264ff8a42\",\"payment_identifier\":\"1621260201680\",\"token\":\"0x0c7309dF25335dDf05ca0853Fc95Df49421DaF02\",\"recipient\":\"0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec\",\"lock\":{\"amount\":\"100\",\"expiration\":\"335\",\"secrethash\":\"0xec19a77fb42be8b7c4ca467f83aa577007da49bd18fe80c7abb8ca10d9b02d46\"},\"target\":\"0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec\",\"initiator\":\"0x14791697260E4c9A71f18484C9f997B308e59325\",\"metadata\":{\"routes\":[{\"route\":[\"0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec\"]}]},\"signature\":\"0x536ab67c1a7d5243d92edb1f59e3bb64335526a31622110bf9d80f16c26abd464c49b0cf736dc48481aa1b54f1d340acca93caf4540cc3333e725db507f4769d1b\"}",
  "peer_address": "0x14791697260E4c9A71f18484C9f997B308e59325",
  "_exc": "SerializationError(\"Can't deserialize: {'message_identifier': '1621260201713', 'chain_id': '4321', 'token_network_address': '0x32626F60da0aF910293EcdeC2123932db4138A81', 'channel_identifier': '2', 'nonce': '1', 'transferred_amount': '0', 'locked_amount': '100', 'locksroot': '0x70f5bc325544293c71c6dbd0b47ce3acc0c8dc1dc2459c2606d9e6a264ff8a42', 'payment_identifier': '1621260201680', 'token': '0x0c7309dF25335dDf05ca0853Fc95Df49421DaF02', 'recipient': '0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec', 'lock': {'amount': '100', 'expiration': '335', 'secrethash': '0xec19a77fb42be8b7c4ca467f83aa577007da49bd18fe80c7abb8ca10d9b02d46'}, 'target': '0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec', 'initiator': '0x14791697260E4c9A71f18484C9f997B308e59325', 'metadata': {'routes': [{'route': ['0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec']}]}, 'signature': '0x536ab67c1a7d5243d92edb1f59e3bb64335526a31622110bf9d80f16c26abd464c49b0cf736dc48481aa1b54f1d340acca93caf4540cc3333e725db507f4769d1b', '_type': 'raiden.messages.transfers.LockedTransfer'}\")",
  "event": "Not a valid Message",
  "logger": "raiden.network.transport.matrix.utils",
  "level": "warning",
  "greenlet_name": "GMatrixClient.message_worker user_id:@0x517aad51d0e9bbef3c64803f86b3b9136641d9ec:localhost",
  "timestamp": "2021-05-17 14:03:21.897223"
}

Version

This was run with f22184b24f944e76e42c88a46e589b8e686bf3d6 I'm currently testing again with current develop (6730646c785b8d85b8ded9039383ccdde2acfb2c).

Update

This happens on 6730646c785b8d85b8ded9039383ccdde2acfb2c as well:


{
  "message_data": "{\"type\":\"LockedTransfer\",\"message_identifier\":\"1621265195804\",\"chain_id\":\"4321\",\"token_network_address\":\"0xb68e6813Ba15640F5515B51D3183c62480062226\",\"channel_identifier\":\"2\",\"nonce\":\"1\",\"transferred_amount\":\"0\",\"locked_amount\":\"100\",\"locksroot\":\"0xd3be0c928fc1356713bdcdf91bdb6f6d958370cbb750b7cccfb0f52b649348c9\",\"payment_identifier\":\"1621265195775\",\"token\":\"0xcf7BF34aA95850429C64E195C354cC3d142158b3\",\"recipient\":\"0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec\",\"lock\":{\"amount\":\"100\",\"expiration\":\"338\",\"secrethash\":\"0x247b96f18e82f5d38ef90989ec924675bdb21944216f44f7d4915812c3a1f420\"},\"target\":\"0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec\",\"initiator\":\"0x14791697260E4c9A71f18484C9f997B308e59325\",\"metadata\":{\"routes\":[{\"route\":[\"0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec\"]}]},\"signature\":\"0xdccf079182f39327115f9900315323539b2b03fbf8849e4d883e02a834d65bed74ee8c35a4c164fe3e71acffdfb09aa251bb654ecbccca39633c1b3630525f841c\"}",
  "peer_address": "0x14791697260E4c9A71f18484C9f997B308e59325",
  "_exc": "SerializationError(\"Can't deserialize: {'message_identifier': '1621265195804', 'chain_id': '4321', 'token_network_address': '0xb68e6813Ba15640F5515B51D3183c62480062226', 'channel_identifier': '2', 'nonce': '1', 'transferred_amount': '0', 'locked_amount': '100', 'locksroot': '0xd3be0c928fc1356713bdcdf91bdb6f6d958370cbb750b7cccfb0f52b649348c9', 'payment_identifier': '1621265195775', 'token': '0xcf7BF34aA95850429C64E195C354cC3d142158b3', 'recipient': '0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec', 'lock': {'amount': '100', 'expiration': '338', 'secrethash': '0x247b96f18e82f5d38ef90989ec924675bdb21944216f44f7d4915812c3a1f420'}, 'target': '0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec', 'initiator': '0x14791697260E4c9A71f18484C9f997B308e59325', 'metadata': {'routes': [{'route': ['0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec']}]}, 'signature': '0xdccf079182f39327115f9900315323539b2b03fbf8849e4d883e02a834d65bed74ee8c35a4c164fe3e71acffdfb09aa251bb654ecbccca39633c1b3630525f841c', '_type': 'raiden.messages.transfers.LockedTransfer'}\")",
  "event": "Not a valid Message",
  "logger": "raiden.network.transport.matrix.utils",
  "level": "warning",
  "greenlet_name": "GMatrixClient.message_worker user_id:@0x517aad51d0e9bbef3c64803f86b3b9136641d9ec:localhost",
  "timestamp": "2021-05-17 15:26:36.971584"
}
karlb commented 3 years ago

The actual validation error is

ValidationError: {'metadata': {'routes': {0: {'address_metadata': ['Missing data for required field.']}}}}
palango commented 3 years ago

Now we're running into problems when verifying the signature:

{"message": "<LockedTransfer ...>", "signer": "b'*W\\x82\\xde\\xb7?\\x14nmx\\xc0v\\xaa\\xad+mN\\x14\\xa7o'", "peer_address": "0x14791697260E4c9A71f18484C9f997B308e59325", "event": "Message not signed by sender!", "logger": "raiden.network.transport.matrix.utils", "level": "warning", "greenlet_name": "GMatrixClient.message_worker user_id:@0x517aad51d0e9bbef3c64803f86b3b9136641d9ec:localhost", "timestamp": "2021-05-18 13:32:40.882767"}

This is caused by replacing the not provided metadata with None or an empty dict.

karlb commented 3 years ago

The signature check most likely fails because we have the address_metadata key in RouteMetadata._serialize_canonicaljson even though it is missing in the original message.

andrevmatos commented 3 years ago

I think it doesn't make sense to make this field optional in the schema, but require a change in signature. Either we require it in both or we ensure it's added to the signed bytestring only if it's present, making this change compatible with clients not providing it, of which I'd prefer the later, since this was intended to be an optional optimization.