Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signature alone is not enough to identify signed data #1238

Merged
merged 2 commits into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 52 additions & 44 deletions raiden_contracts/data/contracts.json

Large diffs are not rendered by default.

19 changes: 18 additions & 1 deletion raiden_contracts/data/source/services/MonitoringService.sol
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ contract MonitoringService is Utils {
address raiden_node_address = recoverAddressFromRewardProof(
address(this),
token_network.chain_id(),
token_network_address,
non_closing_participant,
non_closing_signature,
reward_amount,
reward_proof_signature
Expand Down Expand Up @@ -350,6 +352,8 @@ contract MonitoringService is Utils {
function recoverAddressFromRewardProof(
address monitoring_service_contract_address,
uint256 chain_id,
address token_network_address,
address non_closing_participant,
bytes memory non_closing_signature,
uint256 reward_amount,
bytes memory signature
Expand All @@ -358,16 +362,29 @@ contract MonitoringService is Utils {
pure
returns (address signature_address)
{
// This message shows the intention of the signer to pay
// a reward to a Monitoring Service, provided that the
// call of updateNonClosingBalanceProof() succeeds.
// The triple (non_closing_participant, non_closing_signature, token_network_address)
// uniquely identifies the call that's supposed to be made.
// (Just checking non_closing_signature is not enough because
// when an attacker tampers with the payload, the signature
// verification doesn't fail but emits a different address.)
// (Without a token_network, there will be some ambiguity
// what the payload means.)
bytes32 message_hash = keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n181", // 20 + 32 + 32 + 65 + 32
"\x19Ethereum Signed Message:\n221", // 20 + 32 + 32 + 20 + 20 + 65 + 32
monitoring_service_contract_address,
chain_id,
uint256(MessageTypeId.MSReward),
token_network_address,
non_closing_participant,
non_closing_signature,
reward_amount
));

signature_address = ECVerify.ecverify(message_hash, signature);
require(signature_address == non_closing_participant, "Reward proof with wrong non_closing_participant");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ contract MonitoringServiceInternalsTest is MonitoringService {
function recoverAddressFromRewardProofPublic(
address monitoring_service_contract_address,
uint256 chain_id,
address token_network_address,
address non_closing_participant,
bytes memory non_closing_signature,
uint256 reward_amount,
bytes memory signature
Expand All @@ -66,6 +68,8 @@ contract MonitoringServiceInternalsTest is MonitoringService {
return MonitoringService.recoverAddressFromRewardProof(
monitoring_service_contract_address,
chain_id,
token_network_address,
non_closing_participant,
non_closing_signature,
reward_amount,
signature
Expand Down
4 changes: 4 additions & 0 deletions raiden_contracts/tests/test_monitoring_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ def f(monitoring_service_contract: Contract) -> Dict:
privatekey=get_private_key(B),
monitoring_service_contract_address=monitoring_service_contract.address,
chain_id=token_network.functions.chain_id().call(),
token_network_address=token_network.address,
non_closing_participant=B,
non_closing_signature=non_closing_signature_B,
reward_amount=REWARD_AMOUNT,
)
Expand Down Expand Up @@ -408,6 +410,8 @@ def test_recoverAddressFromRewardProof(
recovered_address = recoverAddressFromRewardProof(
monitoring_service_contract_address=monitoring_service_internals.address,
chain_id=token_network.functions.chain_id().call(),
token_network_address=token_network.address,
non_closing_participant=B,
non_closing_signature=monitor_data_internal["non_closing_signature"],
reward_amount=REWARD_AMOUNT,
signature=monitor_data_internal["reward_proof_signature"],
Expand Down
2 changes: 2 additions & 0 deletions raiden_contracts/tests/test_print_gas.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ def print_gas_monitoring_service(
privatekey=get_private_key(B),
monitoring_service_contract_address=monitoring_service_external.address,
chain_id=token_network.functions.chain_id().call(),
token_network_address=token_network.address,
non_closing_participant=B,
reward_amount=reward_amount,
non_closing_signature=non_closing_signature_B,
)
Expand Down
8 changes: 8 additions & 0 deletions raiden_contracts/utils/proofs.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,17 @@ def pack_withdraw_message(
def pack_reward_proof(
monitoring_service_contract_address: HexAddress,
chain_id: int,
token_network_address: HexAddress,
non_closing_participant: HexAddress,
non_closing_signature: bytes,
reward_amount: int,
) -> bytes:
return (
Web3.toBytes(hexstr=monitoring_service_contract_address)
+ encode_single("uint256", chain_id)
+ encode_single("uint256", MessageTypeId.MSReward)
+ Web3.toBytes(hexstr=token_network_address)
+ Web3.toBytes(hexstr=non_closing_participant)
+ non_closing_signature
+ encode_single("uint256", reward_amount)
)
Expand Down Expand Up @@ -231,13 +235,17 @@ def sign_reward_proof(
privatekey: str,
monitoring_service_contract_address: HexAddress,
chain_id: int,
token_network_address: HexAddress,
non_closing_participant: HexAddress,
non_closing_signature: bytes,
reward_amount: int,
v: int = 27,
) -> bytes:
packed_data = pack_reward_proof(
monitoring_service_contract_address=monitoring_service_contract_address,
chain_id=chain_id,
token_network_address=token_network_address,
non_closing_participant=non_closing_participant,
non_closing_signature=non_closing_signature,
reward_amount=reward_amount,
)
Expand Down