flathub / com.visualstudio.code-oss

https://flathub.org/apps/details/com.visualstudio.code-oss
21 stars 8 forks source link

Harden filesystem permissions #38

Open vilgotf opened 3 years ago

vilgotf commented 3 years ago

Currently $HOME is exposed in its entirety. I don't believe this is needed, but to change this users will probably need to extend their sandbox according to their needs.


There's in my opinion three things vscode should be exposed to by default:

  1. Global gitconfig (could either be ~/.gitconfig or $XDG_CONFIG_HOME/git/config) (ro)
  2. GPG keys (~/.gnupg) (ro)
  3. Code (Either xdg-documents or the proposed xdg-projects (GNOME Builder uses ~/Projects) (rw)

With $HOME not exposed to the sandbox it's mounted as a tmpfs. We obviously want language package managers to store their files persistently (to avoid needless downloading and compiling), but since we can't rely on users configuring them (or them even being configurable) to follow XDG [1] exposing directories as needed is required[2].

I do however believe that no additional directories outside of the three previous listed should be exposed by default. This is because users are already forced to add environment variable overrides to enable additional languages (FLATPAK_ENABLE_SDK_EXT=), so adding filesystem overrides for that language's package manager (if it doesn't follow XDG) doesn't seem like much additional work[2].

As a first step in moving to a more secure sandbox, the --filesystem=host permission could be changed out to --filesystem=home (since host AFAIK only adds some dirs to /run/host which I don't think many users utilize)

[1]: All XDG dirs are mapped inside of the sandbox, e.g. $XDG_DATA_HOME is mapped to ~/var/app/$APP_ID/data. [2]: Flatseal makes managing flatpak permissions incredibly easy

gasinvein commented 3 years ago

Honestly, I don't think it makes sense to try to make something like IDE secure-by-default. Without filesystem access, the app would be almost completely unusable until the user manually grants some permissions.

3. Code (Either xdg-documents or the proposed xdg-projects (GNOME Builder uses ~/Projects) (rw)

Code can be stored almost anywhere, no way we can provide an exhaustive list of possible locations.

As a first step in moving to a more secure sandbox, the --filesystem=host permission could be changed out to --filesystem=home (since host AFAIK only adds some dirs to /run/host which I don't think many users utilize)

home is no more secure than host, since both allow sandbox escape, effectively nullifying any isolation.

Note, however, that Electron 14 will likely support portals, so we can reconsider hardening default permissions once VS Code updates to portal-enabled Electron.

vilgotf commented 3 years ago

Honestly, I don't think it makes sense to try to make something like IDE secure-by-default. Without filesystem access, the app would be almost completely unusable until the user manually grants some permissions. Code can be stored almost anywhere, no way we can provide an exhaustive list of possible locations.

There is in my opinion nothing wrong with providing opinionated defaults. And with something like Flatseal, personalizing the sandbox is very easy.

home is no more secure than host, since both allow sandbox escape, effectively nullifying any isolation.

This is true (I phrased it badly) however I still think this should be changed.

Note, however, that Electron 14 will likely support portals, so we can reconsider hardening default permissions once VS Code updates to portal-enabled Electron.

Won't filesystem permissions still be needed for the built in terminal?

gasinvein commented 3 years ago

Won't filesystem permissions still be needed for the built in terminal?

I guess not more than for the app itself. If you open some project folder (passing it through via portal), the built-in terminal will have access to that folder as well. This probably won't provide as good UX as direct access, due to weird paths and fuse mount, but it should work.

vilgotf commented 2 years ago

Electron >14 is used as of code 1.65, bringing portal support when GTK_USE_PORTAL=1 is set.