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 the msgpack rpc? #375

Closed
gabriel opened this issue Apr 15, 2015 · 17 comments
Closed

Secure the msgpack rpc? #375

gabriel opened this issue Apr 15, 2015 · 17 comments

Comments

@gabriel
Copy link
Contributor

gabriel commented Apr 15, 2015

Do we need to secure the msgpack rpc since certain things like password, secret_ui are sent through it?

Maybe the sock file should only be user readable?

@maxtaco
Copy link
Contributor

maxtaco commented Apr 15, 2015

This will definitely work on Linux/OSX. On windows, we might need to use TCP/IP bound to localhost, in which case server and client can authenticate via some read-only. The client sends the token to the server, and the server disconnects unless it gets the right token.

@maxtaco
Copy link
Contributor

maxtaco commented Apr 15, 2015

There's also more descriptions of how to simulate a Unix stream socket here

@akalin
Copy link
Contributor

akalin commented Apr 15, 2015

Might be stating the obvious here, but if we have to stick with TCP/IP for Windows, we'd have to worry about e.g. replay attacks with tokens and possibly other stuff. That makes me inclined to go with named pipes, or other similar authenticated methods.

@maxtaco
Copy link
Contributor

maxtaco commented Apr 15, 2015

...but only from adversaries on the same machine. i'm not sure how user A would be able to replay user B's session. is there something I'm missing?

@akalin
Copy link
Contributor

akalin commented Apr 15, 2015

Yeah, only adversaries on the same machine. But that's what we're securing against in the original issue, right?

@maxtaco
Copy link
Contributor

maxtaco commented Apr 15, 2015

Right, but if I'm B I probably can't see A's communication to the server over localhost if I'm not going out to the network. Unless there's some hack I'm not thinking of. If the communication were actually going over the network, we couldn't have that security.

@akalin
Copy link
Contributor

akalin commented Apr 15, 2015

I suppose that @gabriel was assuming that the malicious user on the same machine can only make new connections to localhost, and that the malicious user wouldn't be able to read/intercept existing connections to localhost.

However, thinking about this leads me to another plausible 'malicious user' scenario. The daemon uses a predictable path for the unix socket, i.e. /tmp/keybase-{username}. It creates the directory with restricted privileges, but if the directory already exists it uses the existing one. So a malicious user can pre-create that directory, then once the daemon launches, it could replace the unix socket file with its own version. (Unless I'm missing something.)

So we should probably use mktmp() or something, and refuse to use an existing directory, in addition to setting the permissions on the created files. This may also be an argument for keeping this directory in $HOME vs. $TMP.

May be worth adding some checks in the client, too.

Thinking out loud, I'm wondering if there would be an easier way to intercept a localhost TCP connection. I'd have to think about that more.

@akalin akalin closed this as completed Apr 15, 2015
@akalin akalin reopened this Apr 15, 2015
@maxtaco
Copy link
Contributor

maxtaco commented Apr 15, 2015

These are good points. The client and server would have to know where to
rendezvous.

Gpgagent keeps the socket in the home dir maybe for this reason

It might be sufficient to fstat the socket file and assert ownership.

On Wednesday, April 15, 2015, Fred Akalin [email protected] wrote:

Reopened #375 #375.


Reply to this email directly or view it on GitHub
#375 (comment).

@gabriel
Copy link
Contributor Author

gabriel commented Apr 15, 2015

We also might want to codesign the binaries to prevent modification? Even if we restrict the app to run as the user a different program running as the same user could mitm the rpc calls?

I think Gatekeeper in OSX does this kind of stuff to enforce XPC calls and helper tools (I guess this is why binaries must be signed by a developer id cert?)... but we could do something more keybase-y?

@akalin
Copy link
Contributor

akalin commented Apr 15, 2015

Do we need to guard against attackers that can modify our binaries? I think at that point it's already game over.

@akalin
Copy link
Contributor

akalin commented Apr 15, 2015

(Also, we still want it to be possible to clone our repo and start using keybase right away, right?)

@gabriel
Copy link
Contributor Author

gabriel commented Apr 15, 2015

I think its to prevent other running applications (running as the same
user) from being able to modify the binary? I think Gatekeeper + App
Sandbox allows apple to isolate running applications. We don't run in a
sandbox though (since we're heavily filesystem based.)

For the Keybase.app Gatekeeper will verify the codesign. If someone tries
to modify that, it won't be runnable. But we don't codesign or check
signature for keybased.

Unless we run keybased from a kbfs shared folder? Hahah... I think there is
a chicken/egg problem there ;)

On Wed, Apr 15, 2015 at 2:56 PM, Fred Akalin [email protected]
wrote:

Do we need to guard against attackers that can modify our binaries? I
think at that point it's already game over.


Reply to this email directly or view it on GitHub
#375 (comment).

@gabriel
Copy link
Contributor Author

gabriel commented Apr 15, 2015

If you are downloading source we can set a compile time flag to not verify
code signature for people who want to run it from the repo.

I guess all user applications have read access to all the unencrypted data
on a local kbfs mount anyway so yeah I guess this is unnecessary overkill...

I guess we might still want to encrypt the rpc traffic just so passwords
don't get compromised

On Wed, Apr 15, 2015 at 2:57 PM, Fred Akalin [email protected]
wrote:

(Also, we still want it to be possible to clone our repo and start using
keybase right away, right?)


Reply to this email directly or view it on GitHub
#375 (comment).

@maxtaco
Copy link
Contributor

maxtaco commented Apr 16, 2015

We should make the FUSE mount to kbfs readable only by the user it's running for. FUSE definitely gives us the ability to do that.

I still don't see the need to encrypt the RPC traffic, I think we can use simpler techniques to make sure alice isn't reading bob's traffic. It's very doable since it's just bouncing off the kernel, and never sent over the network.

@akalin, as for your concern about bob moving alice's socket out of the way, I'm of course wrong, just fstating the file and asserting ownership isn't good enough, since bob can create a file, and alice can rename it while keeping bob's ownership. Maybe let's go with the home directory approach. If it's good enough for gpg....

maxtaco added a commit that referenced this issue Apr 16, 2015
this seems safer than in /tmp where others can tamper.
@maxtaco
Copy link
Contributor

maxtaco commented Apr 16, 2015

The above commit should cover us in the non-windows case. I don't see how alice can tamper with that RPC connection. But this doesn't help on windows.

@patrickxb patrickxb added this to the Milestone 3 milestone Apr 30, 2015
@maxtaco
Copy link
Contributor

maxtaco commented Sep 23, 2015

See #555.

@maxtaco
Copy link
Contributor

maxtaco commented Sep 23, 2015

Also see #443.

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

4 participants