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

Secure against interception of RevealSecret messages #473

Open
2 tasks
hackaugusto opened this issue Mar 30, 2017 · 42 comments
Open
2 tasks

Secure against interception of RevealSecret messages #473

hackaugusto opened this issue Mar 30, 2017 · 42 comments

Comments

@hackaugusto
Copy link
Contributor

hackaugusto commented Mar 30, 2017

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 Provide a receipt for payments #368 )
  4. Enforce constant lock expiration through out the path

Tasklist

  • Implement a test case where the attack is simulated
    • Implement one fix

Related

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

@hackaugusto hackaugusto added this to the MVP milestone Mar 30, 2017
@LefterisJP LefterisJP modified the milestones: MVP, Red Eyes Apr 6, 2018
@LefterisJP
Copy link
Contributor

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.

@LefterisJP LefterisJP removed this from the Red Eyes milestone Jun 11, 2018
@Zerim
Copy link

Zerim commented Aug 16, 2018

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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

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

  • the same solution works for all transport layer alternatives, and
  • we can easily be sure that the target of the encryption is the target of the payment.
  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
Copy link
Contributor

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

@andrevmatos
Copy link
Contributor

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
Copy link
Contributor

pirapira commented Nov 20, 2018

@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
Copy link
Contributor

Here is a Python Diffie-Hellman library.

@pirapira
Copy link
Contributor

I talked with @hackaugusto . I think I heard

  • there can be alternative/complementary ways to do this
  • Diffie-Helman is an option (symmetric key encryption is faster)
  • writing a failing test first, might not be worth it

@pirapira
Copy link
Contributor

pirapira commented Nov 20, 2018

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

  • which symmetric key encryption scheme to use?
  • does Diffie-Helman require one additional signing on the initiator? (Need to prevent mediators from impersonating).
  • figure out message formats with additional fields
  • figure out if WAL needs more information
  • add a unit test for Diffie-Helman && symmetric key encryption

Initiator-->Target encryption of RevealSecret

  • add new fields for Diffie-Helman key exchange in LockedTransfer and RequestSecret messages
  • encrypt the secret in RevealSecret message from the initiator to the target.

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

@pirapira
Copy link
Contributor

pirapira commented Nov 20, 2018

@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
Copy link
Contributor

err508 commented Nov 21, 2018

@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
Copy link
Contributor

andrevmatos commented Nov 21, 2018

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:

  • No direct use/leak of eth priv key, but safe (as the constant signature won't leak, is unique and private per user, and deterministically generated from the eth PK)
  • Browser/mobile/hardwallet compatible, as only requirement is e.g. eth_sign/personal_sign/eip191
  • Fast shared secret derivation, fastest symmetric enc/decryption
  • Can be used to encrypt chats with more than 2 peers (cc @luehrsFred )

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

@err508
Copy link
Contributor

err508 commented Nov 21, 2018

@andrevmatos could you elaborate on 3.?

@andrevmatos
Copy link
Contributor

andrevmatos commented Nov 21, 2018

@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
Copy link
Contributor

err508 commented Nov 21, 2018

pubkey will be retrieved from partner's public displayname

could you explain how this part works in more detail?

@andrevmatos
Copy link
Contributor

andrevmatos commented Nov 21, 2018

@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
Copy link
Contributor

andrevmatos commented Nov 21, 2018

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
Copy link
Contributor

err508 commented Nov 22, 2018

@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
Copy link
Contributor

@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).

  • for forward secrecy, each Secret will be encrypted with a freshly agreed symmetric key.
  • DOS attack: encryption does not help. Payment channel settlements anyway assume the availability of blockchain to both parties.

@andrevmatos
Copy link
Contributor

@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
Copy link
Contributor

andrevmatos commented Nov 22, 2018

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
Copy link
Contributor

err508 commented Nov 22, 2018

@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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor

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

@andrevmatos
Copy link
Contributor

andrevmatos commented Nov 23, 2018

@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
Copy link
Contributor

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
Copy link
Contributor

pirapira commented Nov 23, 2018

@hackaugusto explained me

  1. Allow C to choose the secret (or part of it, related Provide a receipt for payments #368 )

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

Reasons:

  • construction doesn't involve a new crypto primitive
  • it's fine, or even desirable for the public to see when Initiator is sending a valid secret.

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
Copy link
Contributor

pirapira commented Nov 26, 2018

@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:

  • the onchain settlement requires the target's signature on the secret.
  • the offchain settlements requires the target's signature on the secret (in other words, the backward RevealSecret messages contain the target's signature on the secret).

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
Copy link
Contributor

err508 commented Nov 26, 2018

@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
Copy link
Contributor

pirapira commented Nov 26, 2018

@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
Copy link
Contributor

@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
Copy link
Contributor

err508 commented Nov 26, 2018

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
Copy link
Contributor

@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
Copy link
Contributor

err508 commented Nov 26, 2018

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
Copy link
Contributor

andrevmatos commented Nov 26, 2018

@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
Copy link
Contributor

@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
Copy link
Contributor

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

@pirapira pirapira removed their assignment Dec 11, 2018
@pirapira
Copy link
Contributor

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
Copy link
Collaborator

ulope commented Sep 17, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants