magic-wormhole / magic-wormhole-transit-relay

Transit Relay server for Magic-Wormhole
MIT License
164 stars 42 forks source link

Securing transit helper #10

Open ggeorgovassilis opened 5 years ago

ggeorgovassilis commented 5 years ago

I could not find instructions on how to secure the transit helper. It should be possible to set up a transit helper with a shared secret so that only clients who know the secret can use the transit helper.

warner commented 5 years ago

Hm, that's an interesting idea. There's not currently any support for that: the helper will accept connections from anybody who wants, and will glue it to the second connection that uses the same token.

Are you thinking of a sort of password/token that's baked into authorized clients? Or some kind of single-use access token that the e.g. mailbox server might allocate?

To use either kind of token securely, we'd need to encrypt the connection to the transit helper. We don't currently do that, because all the actual data is already encrypted (using a key derived from the PAKE session key). If we don't encrypt the setup handshake too, then someone could steal the access token by snooping the network traffic (and if it's a single-use token, an active network adversary could prevent the real user from spending that token, and use it themselves).

Implementation-wise, we'd put this into the handshake message. Clients currently send please relay $TOKEN for side $SIDE\n to the helper before doing anything else, then wait to hear ok\n before proceeding. We'd want to change that to something like please-$SECRET relay $TOKEN for side $SIDE\n, and the helper would reject/drop the connection if it doesn't get the $SECRET that it wants.

We'd need to make some decisions about compatibility: if a new client (configured to include the $SECRET) talks to an old helper (which doesn't require a secret), should the helper accept the request? Doing so probably requires some sort of negotiation between the two sides, and probably isn't very useful in practice, so I'd avoid it. In that case, secret-enabled clients can only talk to a secret-expecting helper.

ggeorgovassilis commented 5 years ago

Are you thinking of a sort of password/token that's baked into authorized clients?

A token should do. The use case is running a relay for a private group of users. Abuse can be regulated by invalidating existing tokens and issuing new ones.

To use either kind of token securely, we'd need to encrypt the connection to the transit helper.

I wouldn't want to voice an opinion on that topic. If security is a concern then the entire communication should be encrypted anyway regardless of this use case.

Doing so probably requires some sort of negotiation between the two sides

That would increase usability, but specifying a protocol version as a command line argument would probably also do.

sigwinch28 commented 4 years ago

I was implementing this today and had some points w.r.t. @warner's comments

Are you thinking of a sort of password/token that's baked into authorized clients? Or some kind of single-use access token that the e.g. mailbox server might allocate?

I had considered configuring the relay with a single shared token/password (twist transitrelay --password=hunter2 or similar). The idea I had was that the transit relay has a single shared password it's expecting to receive/authenticate in some way from clients.

To use either kind of token securely, we'd need to encrypt the connection to the transit helper. We don't currently do that, because all the actual data is already encrypted (using a key derived from the PAKE session key). If we don't encrypt the setup handshake too, then someone could steal the access token by snooping the network traffic (and if it's a single-use token, an active network adversary could prevent the real user from spending that token, and use it themselves).

Can't we use some kind of HMAC + nonce in this scenario? I don't see how a MITM attack could do anything useful here:

I realise it might require a 3-way handshake, though:

CLIENT->SERVER: please relay $TOKEN for $SIDE and i will auth
SERVER->CLIENT: please auth using nonce $SERVER_GENERATED_NONCE
-- client computes $RESP = HMAC($TOKEN:$SERVER_GENERATED_NONCE, $PASSWORD+$SOME_CONTEXT)
CLIENT->SERVER: the auth response is $RESP
-- server computes HMAC($TOKEN:SERVER_GENERATED_NONCE, $PASSWORD+$SOME_CONTEXT) and compares, disconnecting client if they don't match

Implementation-wise, we'd put this into the handshake message. Clients currently send please relay $TOKEN for side $SIDE\n to the helper before doing anything else, then wait to hear ok\n before proceeding. We'd want to change that to something like please-$SECRET relay $TOKEN for side $SIDE\n, and the helper would reject/drop the connection if it doesn't get the $SECRET that it wants.

My proof-of-concept simply sends a fixed-length password in the auth message:

please relay $TOKEN for $SIDE and the password is $PASSWORD

We'd need to make some decisions about compatibility: if a new client (configured to include the $SECRET) talks to an old helper (which doesn't require a secret), should the helper accept the request? Doing so probably requires some sort of negotiation between the two sides, and probably isn't very useful in practice, so I'd avoid it. In that case, secret-enabled clients can only talk to a secret-expecting helper.

Yeah, it seems very reasonable that if you do something like:

wormhole --transit-helper [...] --transit-password [...] send [...]`

and the transit helper doesn't support this authentication mechanism, the client should error out.

Edit: I see that this might still allow an attacker to hijack a legitimate session, but it wouldn't allow them to initiate one.

ggeorgovassilis commented 4 years ago

@sigwinch28 thank you for looking into this. I guess it comes down to deciding what threats you want to guard against. The use case I'm thinking of is running a relay for myself and friends, An attacker capable of stealing tokens/passwords probably doesn't need the relay at all. Since you are working on this, would you mind implementing the functionality in a way that the relay accepts a list of passwords? This would make it easier revoking individual user access without having to re-distribute replacement passwords to all other users.

sigwinch28 commented 4 years ago

As a drive-by contributor I'm a bit weary of adding complexity to this codebase, but my outline is:

This will require a bit of work on the client side to ensure that we don't start authenticating to random servers that don't support it (i.e. we should probably only send authentication handshakes to the listed relay, not a relay picked by the other party).

ggeorgovassilis commented 4 years ago

What are user names needed for? Isn't a list of passwords enough (assuming the relay operator issues them)?

Sent from mobile - please excuse typos and terseness.


From: Joe Harrison notifications@github.com Sent: Friday, March 6, 2020 12:20:36 PM To: warner/magic-wormhole-transit-relay magic-wormhole-transit-relay@noreply.github.com Cc: George Georgovassilis g.georgovassilis@gmail.com; Author author@noreply.github.com Subject: Re: [warner/magic-wormhole-transit-relay] Securing transit helper (#10)

As a drive-by contributor I'm a bit weary of adding complexity to this codebase, but my outline is:

This will require a bit of work on the client side to ensure that we don't start authenticating to random servers that don't support it (i.e. we should probably only send authentication handshakes to the listed relay, not a relay picked by the other party).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/warner/magic-wormhole-transit-relay/issues/10?email_source=notifications&email_token=AA2XX4AW2DMR6SDU7KPM5DTRGDMAJA5CNFSM4IS34NS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOBASDY#issuecomment-595724559, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AA2XX4E35FNJQBTCG2OAZOTRGDMAJANCNFSM4IS34NSQ.

sigwinch28 commented 4 years ago

What are user names needed for? Isn't a list of passwords enough (assuming the relay operator issues them)? Sent from mobile - please excuse typos and terseness.

If we don't want to send passwords in the clear (because interception of the handshake would allow an attacker to use your relay themselves by using your password) we need to either encrypt the connection between the client and server (as @warner suggested) or use something like an HMAC to verify that we're communicating with a legitimate client (perhaps via a MITM) which knows the key.

If we encrypt the connection between the client and server we could just send the password over the wire and check it on the server. This requires some effort: the client will need a way to check the authenticity of the server, e.g.:

If we use an HMAC, then the server needs to know which password was used to sign the message or else it would have to compute N HMACs and check each of them against the one the client sent using a constant-time algorithm to avoid timing attacks (where N is the number of passwords)

sigwinch28 commented 4 years ago

I've filed #13 and #14 as a lead-up to this (though they are by no means required).

sigwinch28 commented 4 years ago

On second thoughts, given @warner's inclination to reimplement magic-wormhole in Rust, I might write an alternative relay server implementation in Rust that uses tokio, ready to plug in to whatever wire format they choose.