From 70ec3c3a94f51ddd57fee784ab6f70227b74f8ea Mon Sep 17 00:00:00 2001 From: Duncan Tebbs Date: Tue, 18 Feb 2020 13:49:42 +0000 Subject: [PATCH 1/4] include sender ethereum address in signed data --- pyClient/test_commands/scenario.py | 48 ++++++++++++----------- pyClient/zeth/joinsplit.py | 11 +++++- pyClient/zeth/utils.py | 8 ++++ zeth-contracts/contracts/Groth16Mixer.sol | 1 + 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/pyClient/test_commands/scenario.py b/pyClient/test_commands/scenario.py index 10bd12370..18b52975f 100644 --- a/pyClient/test_commands/scenario.py +++ b/pyClient/test_commands/scenario.py @@ -246,7 +246,11 @@ def compute_h_sig_attack_nf( # Compute the joinSplit signature joinsplit_sig = joinsplit.joinsplit_sign( - signing_keypair, sender_eph_pk, ciphertexts, proof_json) + signing_keypair, + charlie_eth_address, + sender_eph_pk, + ciphertexts, + proof_json) tx_hash = zeth_client.mix( sender_eph_pk, @@ -327,14 +331,6 @@ def charlie_corrupt_bob_deposit( (output_note1, pk_bob), (output_note2, pk_bob)]) - # Sign the primary inputs, pk_sender and the ciphertexts - joinsplit_sig = joinsplit.joinsplit_sign( - joinsplit_keypair, - pk_sender, - ciphertexts, - proof_json - ) - # ### ATTACK BLOCK # Charlie intercepts Bob's deposit, corrupts it and # sends her transaction before Bob's transaction is accepted @@ -347,16 +343,20 @@ def charlie_corrupt_bob_deposit( result_corrupt1 = None try: + joinsplit_sig_charlie = joinsplit.joinsplit_sign( + joinsplit_keypair, + charlie_eth_address, + pk_sender, + ciphertexts, + proof_json) tx_hash = zeth_client.mix( pk_sender, fake_ciphertext0, fake_ciphertext1, proof_json, joinsplit_keypair.vk, - joinsplit_sig, + joinsplit_sig_charlie, charlie_eth_address, - # Pay an arbitrary amount (1 wei here) that will be refunded - # since the `mix` function is payable Web3.toWei(BOB_DEPOSIT_ETH, 'ether'), 4000000) result_corrupt1 = \ @@ -378,25 +378,23 @@ def charlie_corrupt_bob_deposit( new_joinsplit_keypair = signing.gen_signing_keypair() # Sign the primary inputs, pk_sender and the ciphertexts - new_joinsplit_sig = joinsplit.joinsplit_sign( - new_joinsplit_keypair, - pk_sender, - [fake_ciphertext0, fake_ciphertext1], - proof_json - ) result_corrupt2 = None try: + joinsplit_sig_charlie = joinsplit.joinsplit_sign( + new_joinsplit_keypair, + charlie_eth_address, + pk_sender, + [fake_ciphertext0, fake_ciphertext1], + proof_json) tx_hash = zeth_client.mix( pk_sender, fake_ciphertext0, fake_ciphertext1, proof_json, new_joinsplit_keypair.vk, - new_joinsplit_sig, + joinsplit_sig_charlie, charlie_eth_address, - # Pay an arbitrary amount (1 wei here) that will be refunded since the - # `mix` function is payable Web3.toWei(BOB_DEPOSIT_ETH, 'ether'), 4000000) result_corrupt2 = \ @@ -412,13 +410,19 @@ def charlie_corrupt_bob_deposit( # ### ATTACK BLOCK # Bob transaction is finally mined + joinsplit_sig_bob = joinsplit.joinsplit_sign( + joinsplit_keypair, + bob_eth_address, + pk_sender, + ciphertexts, + proof_json) tx_hash = zeth_client.mix( pk_sender, ciphertexts[0], ciphertexts[1], proof_json, joinsplit_keypair.vk, - joinsplit_sig, + joinsplit_sig_bob, bob_eth_address, Web3.toWei(BOB_DEPOSIT_ETH, 'ether'), 4000000) diff --git a/pyClient/zeth/joinsplit.py b/pyClient/zeth/joinsplit.py index dd8855f6e..0cb269aae 100644 --- a/pyClient/zeth/joinsplit.py +++ b/pyClient/zeth/joinsplit.py @@ -20,7 +20,7 @@ from zeth.zksnark import IZKSnarkProvider, GenericProof, GenericVerificationKey from zeth.utils import EtherValue, get_trusted_setup_dir, \ hex_digest_to_binary_string, digest_to_binary_string, encrypt, \ - decrypt, int64_to_hex, encode_message_to_bytes + decrypt, int64_to_hex, encode_message_to_bytes, encode_eth_address from zeth.prover_client import ProverClient from api.util_pb2 import ZethNote, JoinsplitInput import api.prover_pb2 as prover_pb2 @@ -538,7 +538,11 @@ def joinsplit( # Sign signature = joinsplit_sign( - signing_keypair, sender_eph_pk, ciphertexts, proof_json) + signing_keypair, + sender_eth_address, + sender_eph_pk, + ciphertexts, + proof_json) # By default transfer exactly v_in, otherwise allow caller to manually # specify. @@ -684,6 +688,7 @@ def _encode_proof_and_inputs(proof_json: GenericProof) -> Tuple[bytes, bytes]: def joinsplit_sign( signing_keypair: JoinsplitSigKeyPair, + sender_eth_address: str, sender_eph_pk: EncryptionPublicKey, ciphertexts: List[bytes], proof_json: GenericProof, @@ -698,11 +703,13 @@ def joinsplit_sign( assert len(ciphertexts) == constants.JS_INPUTS # The message to sign consists of (in order): + # - senders Ethereum address # - senders public encryption key # - ciphertexts # - proof elements # - public input elements h = sha256() + h.update(encode_eth_address(sender_eth_address)) h.update(encode_encryption_public_key(sender_eph_pk)) for ciphertext in ciphertexts: h.update(ciphertext) diff --git a/pyClient/zeth/utils.py b/pyClient/zeth/utils.py index 69906ae55..418a29c78 100644 --- a/pyClient/zeth/utils.py +++ b/pyClient/zeth/utils.py @@ -74,6 +74,14 @@ def encode_abi(type_names: List[str], data: List[bytes]) -> bytes: return eth_abi.encode_abi(type_names, data) # type: ignore +def encode_eth_address(eth_addr: str) -> bytes: + """ + Binary encoding of ethereum address to 32 bytes + """ + # Strip the leading '0x' and hex-decode. + return bytes.fromhex(hex_extend_32bytes(eth_addr[2:])) + + def encode_g1_to_bytes(group_el: G1) -> bytes: """ Encode a group element into a byte string diff --git a/zeth-contracts/contracts/Groth16Mixer.sol b/zeth-contracts/contracts/Groth16Mixer.sol index 6309b7104..7c117f42e 100644 --- a/zeth-contracts/contracts/Groth16Mixer.sol +++ b/zeth-contracts/contracts/Groth16Mixer.sol @@ -86,6 +86,7 @@ contract Groth16Mixer is BaseMixer { // 2.a Verify the signature on the hash of data_to_be_signed bytes32 hash_to_be_signed = sha256( abi.encodePacked( + uint256(msg.sender), pk_sender, ciphertext0, ciphertext1, From e6cf5021b78d178eb2d3df22185a152433b2cd41 Mon Sep 17 00:00:00 2001 From: Duncan Tebbs Date: Tue, 18 Feb 2020 13:50:07 +0000 Subject: [PATCH 2/4] pyclient: test case for sending mix requests from the wrong address --- pyClient/test_commands/scenario.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/pyClient/test_commands/scenario.py b/pyClient/test_commands/scenario.py index 18b52975f..8350a9c12 100644 --- a/pyClient/test_commands/scenario.py +++ b/pyClient/test_commands/scenario.py @@ -407,6 +407,35 @@ def charlie_corrupt_bob_deposit( assert(result_corrupt2 is None), \ "Charlie managed to corrupt Bob's deposit the second time!" + # Case3: Charlie uses the correct mix data, but attempts to send the mix + # call from his own address (thereby receiving the output). + result_corrupt3 = None + try: + joinsplit_sig_bob = joinsplit.joinsplit_sign( + joinsplit_keypair, + bob_eth_address, + pk_sender, + ciphertexts, + proof_json) + tx_hash = zeth_client.mix( + pk_sender, + ciphertexts[0], + ciphertexts[1], + proof_json, + joinsplit_keypair.vk, + joinsplit_sig_bob, + charlie_eth_address, + Web3.toWei(BOB_DEPOSIT_ETH, 'ether'), + 4000000) + result_corrupt3 = \ + wait_for_tx_update_mk_tree(zeth_client, mk_tree, tx_hash) + except Exception as e: + print( + f"Charlie's third corruption attempt" + + f" successfully rejected! (msg: {e})" + ) + assert(result_corrupt3 is None), \ + "Charlie managed to corrupt Bob's deposit the third time!" # ### ATTACK BLOCK # Bob transaction is finally mined From 440e3d0e207e49db0c92a0a3adcc0320d44611f0 Mon Sep 17 00:00:00 2001 From: Duncan Tebbs Date: Fri, 21 Feb 2020 15:24:20 +0000 Subject: [PATCH 3/4] renaming and documentation changes --- pyClient/test_commands/scenario.py | 40 ++++++++++++++++-------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/pyClient/test_commands/scenario.py b/pyClient/test_commands/scenario.py index 8350a9c12..24b9fa937 100644 --- a/pyClient/test_commands/scenario.py +++ b/pyClient/test_commands/scenario.py @@ -245,7 +245,7 @@ def compute_h_sig_attack_nf( (output_note2, pk_charlie)]) # Compute the joinSplit signature - joinsplit_sig = joinsplit.joinsplit_sign( + joinsplit_sig_charlie = joinsplit.joinsplit_sign( signing_keypair, charlie_eth_address, sender_eph_pk, @@ -258,7 +258,7 @@ def compute_h_sig_attack_nf( ciphertexts[1], proof_json, signing_keypair.vk, - joinsplit_sig, + joinsplit_sig_charlie, charlie_eth_address, # Pay an arbitrary amount (1 wei here) that will be refunded since the # `mix` function is payable @@ -277,14 +277,18 @@ def charlie_corrupt_bob_deposit( Charlie tries to break transaction malleability and corrupt the coins bob is sending in a transaction She does so by intercepting bob's transaction and either: - - case 1: replacing the ciphertexts (or pk_sender) by garbage/arbitrary data + - case 1: replacing the ciphertexts (or sender_eph_pk) by garbage/arbitrary + data - case 2: replacing the ciphertexts by garbage/arbitrary data and using a - new OT-signature + new OT-signature + - case 3: Charlie replays the mix call of Bob, to try to receive the vout Both attacks should fail, - - case 1: the signature check should fail, else Charlie broke UF-CMA - of the OT signature + - case 1: the signature check should fail, else Charlie broke UF-CMA of the + OT signature - case 2: the h_sig/vk verification should fail, as h_sig is not a function - of vk any longer + of vk any longer + - case 3: the signature check should fail, because `msg.sender` will no match + the value used in the mix parameters (Bob's Ethereum Address). NB. If the adversary were to corrupt the ciphertexts (or the encryption key), replace the OT-signature by a new one and modify the h_sig accordingly so that the check on the signature verification (key h_sig/vk) passes, the proof would @@ -327,7 +331,7 @@ def charlie_corrupt_bob_deposit( # Encrypt the coins to bob pk_bob = keystore["Bob"].addr_pk.k_pk - (pk_sender, ciphertexts) = joinsplit.encrypt_notes([ + (sender_eph_pk, ciphertexts) = joinsplit.encrypt_notes([ (output_note1, pk_bob), (output_note2, pk_bob)]) @@ -337,7 +341,7 @@ def charlie_corrupt_bob_deposit( # Case 1: replacing the ciphertexts by garbage/arbitrary data # Corrupt the ciphertexts - # (another way would have been to overwrite pk_sender) + # (another way would have been to overwrite sender_eph_pk) fake_ciphertext0 = urandom(32) fake_ciphertext1 = urandom(32) @@ -346,11 +350,11 @@ def charlie_corrupt_bob_deposit( joinsplit_sig_charlie = joinsplit.joinsplit_sign( joinsplit_keypair, charlie_eth_address, - pk_sender, + sender_eph_pk, ciphertexts, proof_json) tx_hash = zeth_client.mix( - pk_sender, + sender_eph_pk, fake_ciphertext0, fake_ciphertext1, proof_json, @@ -377,18 +381,18 @@ def charlie_corrupt_bob_deposit( fake_ciphertext1 = urandom(32) new_joinsplit_keypair = signing.gen_signing_keypair() - # Sign the primary inputs, pk_sender and the ciphertexts + # Sign the primary inputs, sender_eph_pk and the ciphertexts result_corrupt2 = None try: joinsplit_sig_charlie = joinsplit.joinsplit_sign( new_joinsplit_keypair, charlie_eth_address, - pk_sender, + sender_eph_pk, [fake_ciphertext0, fake_ciphertext1], proof_json) tx_hash = zeth_client.mix( - pk_sender, + sender_eph_pk, fake_ciphertext0, fake_ciphertext1, proof_json, @@ -414,11 +418,11 @@ def charlie_corrupt_bob_deposit( joinsplit_sig_bob = joinsplit.joinsplit_sign( joinsplit_keypair, bob_eth_address, - pk_sender, + sender_eph_pk, ciphertexts, proof_json) tx_hash = zeth_client.mix( - pk_sender, + sender_eph_pk, ciphertexts[0], ciphertexts[1], proof_json, @@ -442,11 +446,11 @@ def charlie_corrupt_bob_deposit( joinsplit_sig_bob = joinsplit.joinsplit_sign( joinsplit_keypair, bob_eth_address, - pk_sender, + sender_eph_pk, ciphertexts, proof_json) tx_hash = zeth_client.mix( - pk_sender, + sender_eph_pk, ciphertexts[0], ciphertexts[1], proof_json, From 2cbb603e79afb0e0ffcc1163072111ee010d9c1e Mon Sep 17 00:00:00 2001 From: Duncan Tebbs Date: Fri, 21 Feb 2020 17:04:22 +0000 Subject: [PATCH 4/4] .travis: fix after mac config changes --- .travis.yml | 13 ------------- scripts/ci | 34 ++++++++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/.travis.yml b/.travis.yml index c7f161c77..2f8fdb914 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,19 +5,6 @@ addons: apt: packages: - python3-virtualenv - homebrew: - packages: - - gmp - - grpc - - protobuf - - boost - - openssl - - cmake - - libtool - - autoconf - - automake - - python - # - llvm matrix: include: diff --git a/scripts/ci b/scripts/ci index 09625a367..2c97d8e27 100755 --- a/scripts/ci +++ b/scripts/ci @@ -70,13 +70,14 @@ function build() { cxx_flags="-Werror" if [ "${platform}" == "Darwin" ] ; then + openssl_path=$(brew --prefix openssl) export PATH="/usr/local/opt/llvm/bin:/usr/local/bin:${PATH}" - export PKG_CONFIG_PATH="/usr/local/opt/openssl/lib/pkgconfig" - export LIBRARY_PATH="/usr/local/opt/openssl/lib" - export LDFLAGS="-L/usr/local/lib" - export CPPFLAGS="-I/usr/local/include" + export PKG_CONFIG_PATH="${openssl_path}/lib/pkgconfig" + export LIBRARY_PATH="${openssl_path}/lib" + export LDFLAGS="-L/usr/local/lib -L${openssl_path}/lib" + export CPPFLAGS="-I/usr/local/include -I${openssl_path}/include" - cxx_flags="${cxx_flags} -I/usr/local/opt/openssl/include" + cxx_flags="${cxx_flags} -I${openssl_path}/include" cxx_flags="${cxx_flags} -Wno-deprecated-declarations" fi @@ -103,11 +104,28 @@ function build() { function ci_setup() { - if ! [ "${platform}" == "Linux" ] ; then - return + if [ "${platform}" == "Darwin" ] ; then + # Some of these commands can fail (if packages are already installed, + # etc), hence the `|| echo`. + brew unlink python@2 + brew update || echo + brew install \ + gmp \ + grpc \ + protobuf \ + boost \ + openssl \ + cmake \ + libtool \ + autoconf \ + automake \ + python \ + || echo fi - sudo apt install -y python3-venv + if [ "${platform}" == "Linux" ] ; then + sudo apt install -y python3-venv + fi }