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

Sandboxing neovim #238

Closed
infokiller opened this issue Nov 17, 2019 · 15 comments
Closed

Sandboxing neovim #238

infokiller opened this issue Nov 17, 2019 · 15 comments

Comments

@infokiller
Copy link
Contributor

I think it's useful to sandbox the neovim instance started by firenvim, to add an additional layer of defense.
For Windows, I only know of Sandboxie (which recently announced they are open sourcing it).
For Linux, the following tech is relevant:

  • firejail
  • bubblewrap
  • systemd-nspawn
  • docker
  • any other software based on containers
  • VMs: will likely have too much overhead, but I'm listing them for completeness.

I'm currently using firejail on my machines, so I will try to write a firejail profile that grants read-only access to the neovim initialization files and plugins, and read-write access to the edited file.

@glacambre
Copy link
Owner

Related issue: netblue30/firejail#3007

@alerque
Copy link
Contributor

alerque commented Nov 17, 2019

I for one specifically don't want my Neovim instance sandboxed. Half of what I love about having my real editor is that I have my full real editor including it's access to my system tools, user scripts, even the built in Terminal.

If course I'm not opposed to there being an optional layer of defense for those that want it that I don't have to use, but I'm also not sure that there is really a threat model either. Given that the UI instance itself runs inside an iframe — what exactly is the exposure that would be mitigated by jailing the editor?

@infokiller
Copy link
Contributor Author

Yes, my intention is only to make this possible and effective, not to force everyone to do it.
Re threat model: I probably don't know enough about browsers to define it accurately, so take this with a grain of salt, but I have a few ideas.

The text that neovim starts with is untrusted since it comes from a random textarea in an untrusted webpage. This can be exploited to craft special input that causes neovim to run code using modelines or other means.
More generally, since this extension invokes a native process, it can be used to circumvent the browser sandbox.

Another possible threat is if the extension in the store is compromised (for example by compromising the account used to upload it).

It would be cool if we could cryptographically attest that the extension that starts neovim is running known good code.

BTW, it may be worth looking into https://github.com/google/end-to-end/wiki/Threat-model
It was last updated 5 years ago, but it looks well though out, and most parts are probably still relevant.

@alerque
Copy link
Contributor

alerque commented Nov 17, 2019

The modeline security vector has been quite thoroughly vetted. There is a reason you can't set anything there that executes code, only set specific whitelisted values. I use Neovim all day every day and quite frequently on untrusted input, if untrusted input to vim was a security threat to my machine then sandboxing Firenvim is the least of my worries.

I don't know a lot about the browser sandbox side of things, but code execution scopes are pretty important in the browser and if untrusted code in a website can execute something in my plugin scope, then I'm screwed too.

If the extension store is compromised, how are you going to ensure it runs your sandbox directives rather than it's own preferred native process?

I'm not saying there is no possible value here, but just sandboxing Firenvim sounds more like hand waving then actual security. I'm happy to be proved wrong!

@infokiller
Copy link
Contributor Author

The modeline security vector has been quite thoroughly vetted. There is a reason you can't set anything there that executes code, only set specific whitelisted values.

And issues still come up: https://news.ycombinator.com/item?id=20098691
I doubt this will be the last issue caused from editing untrusted input.

I use Neovim all day every day and quite frequently on untrusted input, if untrusted input to vim was a security threat to my machine then sandboxing Firenvim is the least of my worries.

Why is that?

I don't know a lot about the browser sandbox side of things, but code execution scopes are pretty important in the browser and if untrusted code in a website can execute something in my plugin scope, then I'm screwed too.

If the extension store is compromised, how are you going to ensure it runs your sandbox directives rather than it's own preferred native process?

IIUC, invoking the neovim process is done from an existing neovim process, so if the vim plugin code is not compromised, the browser extension won't be able to invoke anything it wants.

I'm not saying there is no possible value here, but just sandboxing Firenvim sounds more like hand waving then actual security. I'm happy to be proved wrong!

Sure, and I appreciate that, since I actually care about security and don't want to create a security theater :)

@glacambre
Copy link
Owner

glacambre commented Nov 17, 2019

The text that neovim starts with is untrusted since it comes from a random textarea in an untrusted webpage. This can be exploited to craft special input that causes neovim to run code using modelines or other means.

Actually, modelines don't work for firenvim because firenvim asks neovim to open an empty file and only after that copies the page's content into the buffer.
Edit: Modelines now work in Firenvim.

Another possible threat is if the extension in the store is compromised (for example by compromising the account used to upload it).

I wouldn't worry too much about this, I use different passwords for all my accounts (and only store them in my brain) and use 2FA for my email/firefox accounts.

It would be cool if we could cryptographically attest that the extension that starts neovim is running known good code.

This is not something I'm going to do for now because the release process is currently too manual for me to add another burden, but ultimately I could probably add a hash of the built extension to the git tags I use to mark releases.

code execution scopes are pretty important in the browser and if untrusted code in a website can execute something in my plugin scope, then I'm screwed too

Yes, browsers make it pretty hard for pages to escalate privileges (here an interesting article about how they do this in webextensions), however mistakes can still happen.

Someone wrote an apparmor profile for tridactyl's native messenger, the result is here. Perhaps this could serve as inspiration for an apparmor profile for firenvim.

@alerque
Copy link
Contributor

alerque commented Nov 17, 2019

The 2002 CVE I knew about. I was looking for a way to circumvent modeline limitations myself in 2015. The 2016 CVE I knew about. How did I miss the 2019 CVE

What were you saying about an extra layer of sandboxing? 😉

@infokiller
Copy link
Contributor Author

Thanks for the discussion folks, looks like there's no quick win for now :)
On my end, I will try to write that firejail profile (and maybe play with apparmot as well), and will let you know if I come up with something working.
I'm not sure if this issue should remain open or not for future visibility and documentation, but in any event I suggest linking to it from the SECURITY doc so that others can chime in.
Does that sound reasonable?

@glacambre
Copy link
Owner

I'm keeping this issue open - sandboxing would be nice to have for users who care about it. I've mentionned this issue in SECURITY.md as you suggested :)

@justinmk
Copy link
Contributor

How did I miss the 2019 CVE

Since then, Nvim/Vim disables 'modelineexpr' by default. So there's not much exploitable surface left. Previously Vim did its typical whack-a-mole approach.

@NilsIrl
Copy link
Contributor

NilsIrl commented Jan 2, 2020

I have 0 experience with vim or webextensions so what I'm about to say might be total BS, but whatever, you guys can correct me if I'm wrong.

Couldn't you communicate with nvim using native messaging which would prevent websites from accessing the nvim instance? I'm not sure if it would however, prevent other extensions to present them selves as firenvim.

If this is possible it would be so much better than anything else in my opinion and wouldn't even require a handshake and stuff. There would be no password which you have to rely on.

EDIT: I re-read SECURITY.md, and it seems that native messaging is already used to start nvim however the rest of the communication is done over TCP. So actually what I'm saying is to do everything over native messages rather than just the initial handshake to get the password.

@glacambre
Copy link
Owner

Using NativeMessaging instead of websockets is a pretty good idea and is in fact what Firenvim used to do (before commit 47ed0ce). The problem with this approach is that it doubles the amount of IPC that needs to happen in order to make pages communicate with Neovim, which in turn more than quadruples the latency between a keypress and its effect becoming visible in the page.
Note that as stated in SECURITY.md, web pages would need to be able to guess both the port neovim was started on and the one-time password generated by Firenvim, which are both totally random, so while things aren't perfect, they aren't entirely bad either (they used to be better though…).

Thanks for the suggestion :)

@NilsIrl
Copy link
Contributor

NilsIrl commented Jan 11, 2020

which in turn more than quadruples the latency between keypress

Yeah we definitively don't want that (Although it might be the placebo/nocebo effect I feel like there is some latency as it is).

Could WebAssembly help here?

@glacambre
Copy link
Owner

Yeah we definitively don't want that (Although it might be the placebo/nocebo effect I feel like there is some latency as it is).

No, you're absolutely right, there's a lot of latency. Some of it probably can't be fixed (I think latency is the reason why the Oni devs decided to switch to an "embed vim/nvim as a library" approach rather than using Neovim's RPC interface) but Firenvim can still be improved in two ways:

  • First, switching from a dom-renderer to a webgl renderer should improve things a lot.
  • Second, the websocket code I wrote is completely unoptimized. My base64, sha1 and websocket-decoding code all heavily rely on strings rather than arrays and this is a pretty bad thing to do if you're aiming for performance.

Could WebAssembly help here?

I don't think so, Firenvim doesn't do anything computationally expensive.

@glacambre
Copy link
Owner

Sandboxing Firenvim with apparmor is actually super easy. Just run aa-genprof ~/.local/share/firenvim/firenvim, use Firenvim a few times on multiple websites and then edit the generated profile to make it a bit more generic. I'm quite impressed with how simple this is :).

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

No branches or pull requests

5 participants