flathub / com.slack.Slack

https://flathub.org/apps/details/com.slack.Slack
35 stars 36 forks source link

Remove access to secrets & systemd's login1 #217

Closed Stebalien closed 7 months ago

Stebalien commented 1 year ago

Please consider removing access to these d-bus endpoints. They're way too invasive for a chat app and Slack works just fine without them:

  1. Access to org.freedesktop.secrets will give slack access to all passwords. The risk of storing the secrets in the apps state is negligible next to that. Remember: Slack is an electron app, the attack surface is massive.
  2. org.freedesktop.login1 is less powerful, but still unnecessary (slack really doesn't need to know about suspend events) and can be used to, e.g., unlock a computer remotely.

Note: Neither Element nor Signal ask for access to secrets (really, only Chrome does and it actually as a decent reason to do so).

XenonSup commented 1 year ago

Agreed, or please provide a detailed justification of why these new permissions are now required. An update asking for those permissions is somewhat alarming.

evan-a-a commented 1 year ago
  1. Electron apps are just Chromium with slightly better OS integration. The threat model for them is essentially the same as a web browser, which, as you noted, already has access to org.freedesktop.secrets. Storing the secrets in an encrypted keyring is better from a security perspective because it eliminates the possibility of recovering those secrets from disk via offline attacks or via another user with administrative privileges when the keyring is in a locked state. It's almost never a good idea to store sensitive data in plaintext on disk. I'll admit the org.freedesktop.secrets interface isn't great, but it does provide valuable protection for application secrets that would otherwise be trivially exposed.
  2. It's generally good to give applications notice of power state changes so they can perform any cleanup activities needed to write their state to disk. If restricting Slack's capability here is desired, polkit is probably the right tool for the job. The default configuration does already limit access to certain sensitive subsets of the org.freedesktop.login1 endpoint by gating them behind interactive authentication. Ref: https://github.com/systemd/systemd/blob/main/src/login/org.freedesktop.login1.policy
Stebalien commented 1 year ago
  1. As an electron app, the running JavaScript has elevated privileges, unlike in chrome; that's my concern. If the user isn't using full disk encryption and/or home directory encryption in 2023, they have significantly bigger problems than their slack credentials getting leaked. But as-is, this lets slack access every single password, which is a much bigger issue.
  2. Slack is almost certainly running in the active session which will, by default, have access to all these APIs unrestricted.

To be clear, I'm not concerned about myself. I've already disabled access to these APIs with flatseal. I'm concerned about the insecure defaults.

It's generally good to give applications notice of power state changes so they can perform any cleanup activities needed to write their state to disk.

These are two different problems. Slack needs to know when it's being quit/closed, but that should be handled by intercepting a signal. The fact that I might be shutting down isn't relevant.

I'm also confused because the Chrome flatpak doesn't request access to this D-Bus API. Which means either that flatpak is wrong, this is something specific to electron, or this is something specific to slack (which is worrisome because it means that slack is directly talking to D-Bus from JavaScript, and has direct access to the secret store from JavaScript as well).

Stebalien commented 1 year ago

Granted, dbus access is (I assume) only available to the node.js process and chrome itself, not content scripts. So that does significantly mitigate the risk (if you trust slack to not snoop).

evan-a-a commented 7 months ago

We do not plan to change this behavior, so I'm closing the issue.