raiden-network / raiden

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

Secure against interception of RevealSecret messages #473

Open hackaugusto opened 7 years ago

hackaugusto commented 7 years ago

Problem Definition

For a mediated transfer ABC, where B is the attacker, B is choosing the expiration for the lock BC and is in a position to choose a low expiration time, given that C receives the BC mediated transfer and send a SecretRequest to A, followed by the SecretReveal message from A to C, if B is in a position to intercept the message he can learn the secret while making sure that C won't, that means B can wait for the BC lock to expire and then unlock the AB lock.

Note: Assuming that an attacker can intercept and drop messages and that we don't have guaranteed delivery of messages.

Solution

Possible solutions:

  1. Use transport provided E2E encryption
  2. Encrypt the secret with C's public key
  3. Allow C to choose the secret (or part of it, related #368 )
  4. Enforce constant lock expiration through out the path

Tasklist

Related

Provide a receipt for payments #368 Add Ack authentication #44

LefterisJP commented 6 years ago

As we had discused in the Berlin meeting we would need message encryption for this issue. As we can have message encryption with Matrix transport this could be handled easily then but for Red Eyes we don't want to switch encryption on. As such this issue is going out of Red Eyes.

Zerim commented 6 years ago

Hey folks, came across the link to this issue while reading the docs, and wanted to make sure I understand what's being described correctly.

As it is written, this line is a bit confusing to me:

if B is in a position to intercept the message he can learn the secret while making sure that C won't

When would B be able to intercept a message that is being passed to C and prevent C from receiving? Isn't the path through the overlay network only meant to represent the path of the payment channels? Why would messages also be sent along this path, rather than just directly from A to C?

Or is this describing a man-in-the-middle attack where somehow B has also compromised A or C's access to the internet?

Thanks in advance for the clarification.

hackaugusto commented 6 years ago

Hi @Zerim ,

It describes the second case [...] a man-in-the-middle attack where somehow B has also compromised A or C's access to the internet

hackaugusto commented 6 years ago

Enforce constant lock expiration through out the path

Having a constant lock expiration is sufficient only if the secret is revealed on-chain, if the secret is revealed off-chain then the constant expiration is not sufficient.

pirapira commented 6 years ago

I had a discussion about this with @hackaugusto and @LefterisJP .

I prefer encryption with the receiver's Ethereum public key because

  1. RequestSecret message should contain the public key of the target.
  2. The initiator should check the public key against the address of the target.
  3. The initiator should encrypt the secret in RevealSecret using the public key of the target.

It's possible to encrypt data with Ethereum public keys: https://github.com/LimelabsTech/eth-ecies.

pirapira commented 6 years ago

I found a Python library, too https://pypi.org/project/eciespy/ .

andrevmatos commented 6 years ago

I've already thought about ECIES, the problem with it is that it may lock us out of scenarios where we don't have the private key: e.g. light clients on browser, eth node-side signing (#1402).

pirapira commented 6 years ago

@andrevmatos, hm, that sounds like a blocker.

I came up with some more alternatives. Are these clear? Does anybody have more ideas?

  1. use a transport-layer encryption, and somehow make sure no man-in-the-middle is possible
  2. encryption by Ethereum public keys (ECIES)
  3. clients hold a transfer-wise fresh key in the memory 3a. key sharing (like Diffie-Helman. Put a few additional fields in LockedTransfer and RequestSecret. Then RevealSecret can be encrypted. Encryption and decryption should be fast because symmetric-key encryption. 3b. lightweight public key encryption: the target generates and puts a public key in RequestSecret contains a public key.

My favorite is 3a. The runtime overhead seems the smallest. No dependency on a transport layer. No need to access the Ethereum priv key.

pirapira commented 6 years ago

Here is a Python Diffie-Hellman library.

pirapira commented 6 years ago

I talked with @hackaugusto . I think I heard

pirapira commented 6 years ago

I guess the next step is for me to figure out how to do the Diffie-Helman option.

Initiator-->Target encryption of RevealSecret

Target-->Mediator-->...-->Initiator encryption of RevealSecret No need.

pirapira commented 6 years ago

@hackaugusto @andrevmatos @LefterisJP Do we need encryption on the backward RevealSecret? I'm thinking. --> Got an answer, no need to. Because, I think, in any unhappy case, the unhappy party can go onchain and use the secret.

err508 commented 6 years ago

@pirapira

I came up with some more alternatives. Are these clear? Does anybody have more ideas?

  1. use a transport-layer encryption, and somehow make sure no man-in-the-middle is possible
  2. encryption by Ethereum public keys (ECIES)
  3. clients hold a transfer-wise fresh key in the memory 3a. key sharing (like Diffie-Helman. Put a few additional fields in LockedTransfer and RequestSecret. Then RevealSecret can be encrypted. Encryption and decryption should be fast because symmetric-key encryption. 3b. lightweight public key encryption: the target generates and puts a public key in RequestSecret contains a public key.

We have another issue, where alternatives were discussed. I strongly support to combine it with your proposal 2. to rule out different classes of attacks.

andrevmatos commented 6 years ago

Again, ECIES for me, although interesting, is a blocker due to the requirement of PK access. But DH is interesting, here is my proposal:

  1. Deterministically (to statelessly regenerate it across restarts/resets/client roaming) generate a private key for curve25519 through signing a constant message per network: e.g. curve25519pk = eth_sign('encrypt:<chain_id>) (or a substring of it)
  2. "Publish" the respespective public key as part of matrix's displayname: e.g. displayname = f'{eth_sign(user_id)}:{curve25519pk.getPublic()}'
  3. Partners get this pubkey, and compose a shared secret with its own private key. Symmetric encryption then is used with this shared key.
  4. Ownership still validated through privatekey ownership proof of user_id (currently already used in matrix).

Advantages:

E.g. of JS implementation: https://github.com/indutny/elliptic Comparison table: http://safecurves.cr.yp.to/

err508 commented 6 years ago

@andrevmatos could you elaborate on 3.?

andrevmatos commented 6 years ago

@err508 curve25519 ECDH (encryption) pubkey will be retrieved from partner's public displayname on matrix' service. This curve has the property that A's privkey point multiplied by B's pubkey == B's privkey point multiplied by A's pubkey == shared secret that can be used to exchange symmetrically encrypted messages between them without prior knowledge but each other's pubkey. Demo code. Additionally, we could include this pubkey in signed data in displayname to guarantee no MITM by transport server operator as well.

err508 commented 6 years ago

pubkey will be retrieved from partner's public displayname

could you explain how this part works in more detail?

andrevmatos commented 6 years ago

@err508 Currently, matrix user's displayname is the signature of the userId, and is used to prevent personification: displayname = eth_sign(user_id) My proposal is that we include this new metadata (one's ecdh/messaging pubkey) as a e.g. collon-separated new field in this displayname, that is retrieved, parsed and validated by any interested partner. So, it'd become: displayname = f'{eth_sign(user_id)}:{ecdh_pubkey}' Or better, if we want to include this pubkey also in signature, to ensure no MITM by operator:

eth_privkey = b'\x22...'
ecdh_privkey = nacl.PrivateKey(eth_sign(eth_privkey, f'encrypt:{chain_id}'))
ecdh_pubkey = ecdh_privkey.public_key
user_id = "@0xdeadbeef:transportraiden.network"
sig_data = ':'.join([user_id, ecdh_pubkey])
sig = eth_sign(eth_privkey, sig_data)
displayname = ':'.join([sig, ecdh_pubkey])  # notice displayname is ':'.join of both signature and pubkey

# then on partner, something like:
sig, ecdh_pubkey = user.displayname.split(':')  # <= the answer to your question
sig_data = ':'.join([user.user_id, ecdh_pubkey])
address = extract_eth_address_from_userid(user.user_id)
assert eth_verify(sig, sig_data) == address, 'Invalid peer signature'
# then proceed to use partner's ecdh_pubkey and own privkey to derive the shared secret to communicate with it

Notice in this last, more complete example, ecdh_pubkey both goes appended in the displayname, retrievable by anyone interested in communicating with it, and on hashed/signed data that is validated by every peer, and user is ignored if this signature is invalid.

andrevmatos commented 6 years ago

Python's implementation (not sure if compatible with js's ellyptic, but the idea is the same): https://pynacl.readthedocs.io/en/stable/public/

err508 commented 6 years ago

@andrevmatos

Currently, matrix user's displayname is the signature of the userId, and is used to prevent personification: displayname = eth_sign(user_id)

I'm not sure what happens if Mallory uses mallory's_displayname = mallory's_eth_sign(your_user_id) before or after you have used it.

And in your proposal, how does user.user_id.eth_address get set?

pirapira commented 6 years ago

@err508

@pirapira

  1. clients hold a transfer-wise fresh key in the memory 3a. key sharing (like Diffie-Helman. Put a few additional fields in LockedTransfer and RequestSecret. Then RevealSecret can be encrypted. Encryption and decryption should be fast because symmetric-key encryption.

We have another issue, where alternatives were discussed. I strongly support to combine it with your proposal 2. to rule out different classes of attacks.

Because of @andrevmatos 's requirement, I'm going after 3a (symmetric session key).

andrevmatos commented 6 years ago

@err508 This is how displayname is set on the client. It comes automatically from matrix server on each event related to the user to interested partners (users participating in the same room, etc). This is how the events are validated in the clients, to ensure user's authenticity. My proposal is to change it a little to include ecdh_pubkey in signed data (so it's validated) and publish it appended to the displayname/signature, so it can be retrieved by peers. I'm fully available to answer any question on current matrix transport and implementation. @pirapira My solution is transport only, and won't require changes to the messages formats or core logic. PFS can be achieved easily by generating a key per session, but this has the drawback of locking us out of using full matrix room persistency afterwards (which is planned to get rid of half of the exchanged messages, all Delivered).

andrevmatos commented 6 years ago

I'm not sure what happens if Mallory uses mallory's_displayname = mallory's_eth_sign(your_user_id) before or after you have used it.

Oh, missed this: peer's user_id must contain the eth address. The whole user_id currently is input as signed data, and recovered from the signature in displayname (which is set by the user and distributed by the server to all interested clients). The recovered address must be the same as the eth address inside user_id. Basically, it works as proof-of-private-key-ownership, where a matrix user proves it controls the PK associated with a given eth address. So, mallory would be ignored right away.

err508 commented 6 years ago

@andrevmatos @pirapira If we derive a secure ecdh_pubkey, we could use matrix's olm, no? Have you assessed using it? I think we should do that, before combining dependencies. @pirapira Do you want to parrallelize the DH part? We rely on a secure asymetrical scheme and your eyes on that would be very helpful. Thx @andrevmatos I have to look more into that. When is the user_id.eth_address verified against the one that has deposited funds on chain?

andrevmatos commented 6 years ago

@err508 everytime a user is used for anything in matrix, through the function in the second link in my comment above. That validation is the bridge between a matrix user and the corresponding eth address.

pirapira commented 6 years ago

@andrevmatos D-H session key generation doesn't require Delivered. I'll piggy-back on LockedTransfer and RequestSecret. I prefer end-to-end encryption when there are not much drawbacks.

pirapira commented 6 years ago

@err508 "parallelize" means creating a separate GitHub issue?

andrevmatos commented 6 years ago

@pirapira care to explain how my solution is not e2e encryption? None of my proposals require Delivered, it was just a thought about using e2e encryption with or without forward secrecy, as with would block us of using room persistency (planned to get rid of the Delivered message)

pirapira commented 6 years ago

e2e

@pirapira care to explain how my solution is not e2e encryption?

Without thinking, I excluded the transport layer from the endpoint. Let me rephrase. I think the missing encryption is a problem in the Raiden protocol, and should be solved in it if cheap.

Delivered

None of my proposals require Delivered, it was just a thought about using e2e encryption with or without forward secrecy, as with would block us of using room persistency (planned to get rid of the Delivered message)

Oh, I didn't understand your comment then:

this has the drawback of locking us out of using full matrix room persistency afterwards (which is planned to get rid of half of the exchanged messages, all Delivered).

pirapira commented 6 years ago

@hackaugusto explained me

  1. Allow C to choose the secret (or part of it, related #368 )

in the issue description. For this issue, now I prefer this solution, rather than encryption with session keys.

Reasons:

Sorry for my changing opinions. I'm willing to clean up my previous comments if that helps. I have to reflect a bit over how I proceeded with this issue. I think I should have read everything over carefully or talked with somebody before picking a choice.

pirapira commented 6 years ago

@hackaugusto came up with another solution: to require the target's signature on the secret [edit: not on RevealSecret from the initiator to the target, but on backward RevealSecrets and on smart contract settlements].

I think this might involve:

I think the offchain part only is already useful. If the backward RevealSecret needs to contain the target's signature on the secret, an interceptor needs to reveal the secret onchain, so the target notices the attack.

err508 commented 6 years ago

@pirapira

I can't see how this solves the problem coherently; a mediator might still be able to intercept the signed secret and selectively disclose it, no? Encrypting the secret with the targets public key fullfills the same function (i.e. assuming initiator and target follow the protocol; any mediating node will know, that the secret originated from the target) but intercepting an encrypted secret doesn't yield any information, where intercepting a signed secret does.

In general, I'd reuse as much existing primitives/schemes as possible, not only to reduce engineering overhead...

@err508 everytime a user is used for anything in matrix, through the function in the second link in my comment above. That validation is the bridge between a matrix user and the corresponding eth address.

@andrevmatos I see, would you be so kind to point me to the code where the onchain properties of the eth address are validated(For example that we have a channel with user_id.eth_address ), before we process the messages from the corresponding matrix user?

pirapira commented 6 years ago

@err508

but intercepting an encrypted secret doesn't yield any information, where intercepting a signed secret does.

Yes, but the mediator does not have an advantage over the target because the target has already acquired the secret (see, the secret has been signed by the target already). See my edit above.

  1. the initiator sends RevealSecret plain to the target (visible from anybody).
  2. the target signs the secret and sends back RevealSecret backwards
  3. a mediator (or the initiator) does not issue the balance proof unless the secret is signed by the target.
  4. the PaymentChannel smart contract does not unlock a hash lock unless the secret is signed by the target.

I think 4. is optional.

pirapira commented 6 years ago

@err508 on platforms like MetaMask, private keys are not available for decrypting messages, but for signing messages. That's one reason why "requiring the target's signature on the secret" approach is interesting. Also we already use signatures but we haven't used encryption yet, so the signature approach keeps the number of crypto schemes low.

err508 commented 6 years ago

I see, but could an interceptor with the ability to censor traffic from the target selectively disclose the signed secret only to a subset of mediating nodes, forcing the target and other mediators to go onchain?

I understand that we might not want to use the eth private key for encryption directly, but if understand @andrevmatos proposal correctly, we might derive a child_keypair that we can use in our own code.

pirapira commented 6 years ago

@err508

could an interceptor with the ability to censor traffic from the target selectively disclose the signed secret only to a subset of mediating nodes, forcing the target and other mediators to go onchain?

I agree that this path exists. I doubt any solutions prevent these kinds of behavior.

err508 commented 6 years ago

I agree that it might be difficult to prevent this behaviour, but if we use per-hop encryption, interception is not useful anymore and an attacker is at least required to be part of the route to gain knowledge of the secret, right?

andrevmatos commented 6 years ago

@andrevmatos I see, would you be so kind to point me to the code where the onchain properties of the eth address are validated(For example that we have a channel with user_id.eth_address ), before we process the messages from the corresponding matrix user?

@err508 transport is informed of "interesting" partners (today, addresses we've a channel with or a pending incoming/outgoing mediated transfer) through Transport.whitelist method, which is also called from Transport.start_health_check. Events (invites, messages) coming from whitelisted addresses are always processed and pushed to core, ignored otherwise. Transport does no further validation, this is just a first-line of defense/dos protection. It's core's responsibility to fully validate these events against the state and ignore/warn/close/settle on wrong state transitions/messages.

pirapira commented 6 years ago

@err508

an attacker is at least required to be part of the route to gain knowledge of the secret, right?

Yes, in my threat model for this issue, a malicious mediator has access to what goes through the transport layer. There are ways to make sure that the target is the first one who can open the hashlock.

pirapira commented 5 years ago

Removing my own assignment. There seem to be more urgent security issues around.

pirapira commented 5 years ago

I heard from our advisors that the same keypair should not be used for different primitives (signature & encryption). And one of them started talking about deriving child keypairs.

@err508

we might derive a child_keypair

ulope commented 5 years ago

With the switch to matrix-nio as the underlying Matrix library we will get Olm E2E support almost 'for free'. See #4292.