Read-only Metadata forwarding and hashing scheme

[ezdac]

Abstract

Allow for arbitrary Metadata being sent along a Locked-Transfer, and transition to the Metadata being a read-only data structure for mediators, where the metadata is passed on as-is from the previous hop, and the hash of that data is included in the signature for the mediator's message.

This supersedes #7021 to some extent.

Motivation

We want to provide a means to have better forwards compatibility with arbitrary Metadata sent along a Locked-Transfer,
and we want to coordinate this with the LC, so that the transition is as smooth as possible.

Specification

@andrevmatos proposed specs for the protocol of handling of the Metadata, which would result in following PC behaviour:

• LockedTransfer's metadata on mediation is passed as is*, completely unchanged;
• The only deserialisation/serialisation that the Metadata goes through before being passed on is JSON decoding
• LockedTransfer.message_hash remains the keccak of the bytes _packed_data + metadata.hash - this is the additionalHash and included in the signed-data
• For validation of the LT's signature (LockedTransfer.sender), the LockedTransfer.metadata.hash is created as the keccak of the exact canonical-JSON serialized received metadata, independent on how the Metadata is deserialised to our internal representation
• Only when/where needed internally, we decode metadata known fields and change it to match a given schema: e.g. decoding from; these mappings are only used for internal logic, and doesn't reach the created/relayed message.
  • We try our best to support any format received here, and if we can't for something critical, instead of failing validation (which would put us out-of-sync), we fail the corresponding effect, e.g. fail to mediate the transfer, but still accept the received nonce (which will expire later, obviously).
• Anything unknown is forwarded as is, and we sign it as is.
  • This could be a security risk (arbitrary hashes), but since this is signed strictly as part of a larger message (LockedTransfer), which is EIP191-prefixed, and also is guaranteed to contain at least some known fields (or else we coudln't route the transfer), there's no risk of it being used elsewhere/replayed.
• Read-only access does require that we don't prune the routes anymore, and that we support searching our address in the routes to infer the next hop (Dont prune route on mediation #7090)

Changes needed

• pass the JSON-decoded metadata through the state machine, so that it is still present upon creation of the SendLockedTransfer event on mediation to the next hop
• support deserialisation to class Metadata for internal processing and verification only
• support serialisation from class Metadata's internal representation for sending LockedTransfer only on initiation

This most likely will boil down to adding a _original_data field to the class Metadata, which represents received metadata as is:
deserialisation

• if original_data is present, deserialise to the internal representation (known fields and internal decoding scheme) and verify the incoming signature with the _original_data

serialisation

• _original_data can only be not present, when the Metadata is part of a LockedTransfer because our node is the initiator
• if _original_data is not present, use a well supported serialisation scheme (LCs should be able to process it internally easily), and use the hashing scheme as described above for signing
• if _original_data is present, just use the original_data as the serialised data and use the hashing scheme as described above for signing

write-access

• the Metadata respresentation should be frozen, so that it can't be modified once created

Backwards Compatibility

This feature is backwards incompatible with the current 2.0.0 version, under some conditions:

1. as soon as we change the schema or add an additional key to the metadata (like e.g. the encrypted secret [META] Allow encrypted secret to be sent on LockedTransfer's metadata #7071), the signature verification will break because of mismatched Metadata.hash
2. since the locked-transfer's hash is generated in a fixed scheme, that is not directly derived from the exact metadata sent (on signing) or received (on validation), a possibly backwards compatible scheme would have to comply to the hashing scheme, namely encoding checksummed addresses also in the sent out message data.

raiden/raiden/messages/metadata.py

Lines 33 to 48 in 3a3636e

	def _canonical_dict(self) -> dict:
	"""Return a dict that can be dumped as json
	Used to build signatures.
	"""
	route = [to_checksum_address(address) for address in self.route]
	if not self.address_metadata:
	# We add a default value of {} when validating. Normalize this to
	# no key when signing.
	return {"route": route}
	address_metadata = {
	to_checksum_address(address): metadata
	for address, metadata in self.address_metadata.items()
	}
	return {"route": route, "address_metadata": address_metadata}

Since we want to implement 1.very soon, we should assume backwards-incompatibility in any case.

[andrevmatos]

Yes, that's about it. Just about incompatible change's 2., just to clarify if I understood correctly, this is a breaking change because currently the data which gets signed is different from that sent (through the method you linked), but after this change, that won't be needed anymore, and you'll be free to send the addresses non-checksummed (i.e. as per your internal representation on initiator).

I'd say this breaking change is worth it because it defines a protocol for this part of the message encoding which hopefully won't need to be changed again later: new fields in a diversity of formats will be able to be added to the metadata without having to break compatibility or having to coordinate on some specific type (e.g. address) serialization format, everything will be accepted and forwarded.

[ezdac]

and you'll be free to send the addresses non-checksummed (i.e. as per your internal representation on initiator).

Exactly, actually this will take some weirdness out of our Metadata hashing scheme that was WIP currently.

I'd say this breaking change is worth it

I totally agree