Improve VSCode SecretStorage documentation to highlight capablities and limitations

[datosh]

The documentation for SecretStorage is very minimal and does not assist extension developers to make informed decisions about the the security trade-offs they are making. It currently reads:

"Represents a storage utility for secrets, information that is sensitive."

The store function provides little additional information:

"Store a secret under a given key."

I propose that the documentation should be extended to help developers answer the following questions:

• What are the security properties of the SecretStorage class? Does it protect my secret from other extensions? processes? OS users? (as far as I understand VSCode/Electron/Chromium/... only the last is true).
• What are the differences in security on common platforms: Windows, Linux, MacOS, as SecretStorage eventually relies on OS primitives.
• Mention the silent fallback to basically unencrypted secrets on Linux systems. Have you considered surfacing this information (even as a warning) to users? I assume most are not aware of this.

Let me know if you agree with this feedback and see the value. I am super happy to also provide a first draft to update the documentation accordingly.

/cc @TylerLeonhardt

[datosh]

I put a similar request in electron/electron#42318 as it is the underlying implementation and logical next step for a user to go.

[TylerLeonhardt]

Hi @datosh 👋 this makes good sense to me. I'd review a PR of the first draft.

• What are the security properties of the SecretStorage class? Does it protect my secret from other extensions? processes? OS users? (as far as I understand VSCode/Electron/Chromium/... only the last is true).
  • Extensions do not have the ability to access the electron API required to encrypt/decrypt secrets so only things running in the main process of VS Code would have the ability to encrypt/decrypt secrets this
  • VS Code API only lets an extension interact with secrets specific to that extension
• What are the differences in security on common platforms: Windows, Linux, MacOS, as SecretStorage eventually relies on OS primitives.
  • Windows & macOS are essentially "what safeStorage does", Linux is a bit more complicated:
    • a libsecret compatible keyring if one is configured (we then defer to that keyring)
    • the weaker encryption option that we ask the user if they want to use
    • in-memory secrets
• Mention the silent fallback to basically unencrypted secrets on Linux systems. Have you considered surfacing this information (even as a warning) to users? I assume most are not aware of
  • We do. The user gets a notification that asks them if they want to use weaker encryption... until then, the secrets are stored in-memory

[datosh]

Sorry that this took a while, @TylerLeonhardt.

The following line...

Extensions do not have the ability to access the electron API required to encrypt/decrypt secrets so only things running in the main process of VS Code would have the ability to encrypt/decrypt secrets this

made me think that you might be unaware of the security properties of Electrons SafeStorage API. Therefore I played it safe and reached out to MSRC before posting more detailed information. This process is now complete.

So: As part of a week of security research @wakeward and I figured out how an extension can read the VSCode sqlite3 db (which stores the encrypted values), as well as read from the keyring and then decrypt/read all the secrets from other extensions. To do this, there is no need to talk to the electron API, but simple file reads and decryption in JS/TS.

We have documented our findings in a blog post and have also released the PoC extension for folks to test and play around with.

I think the VSCode extension API documentation should be very clear that the provided secret manager is not able to protect the secrets from any other extension or even applications on the same system. It does provide protection when using a multi-user system (when backed by libsecret), but this is nowhere reflected in the docs, and might simply be lost in translation when going from Chrome -> Electron -> VSCode

I think Electron should really put out the big disclaimer, because the sandboxing model we have in Chrome, is just not available in Electron apps.
EDIT: This part was a wrong assumption on my side. The electron maintainers highlighted that the sandboxing capabilities from Chrome are actually available in Electron as well. I was not aware of this fact. Was it an active decision in VSCode's design to disable the sandboxing (for extensions)? Maybe to allow more easy integration with linters etc installed on a user's system?

---

• We do. The user gets a notification that asks them if they want to use weaker encryption... until then, the secrets are stored in-memory

Awesome - I was able to very this. I wasn't aware, but that is pretty cool and great UX!

[TylerLeonhardt]

I can't speak to the sandboxing... but I believe this is the issue related to it #202385 keep an eye on that.

I think documenting this is a bit tricky because:

• API-wise it's all the same depending on what kind of VS Code you're using... VS Code for Desktop, VS Code for the Web, or a fork of VS Code
• Implementations of these are different. On VS Code desktop, as you've seen, we depend on the Electron SafeStorage API, on vscode.dev we have a different implementation since there's no Electron, and forks are not owned by us so it's unclear.
• In some cases, those implementations are abstracted from us directly, like Electron's API.

So, to document this correctly I think it's important to:

• Call out that the implementation of this API might be different per-platform (this can be in the actual typings)
• Call out, briefly, what we use for the targets we own (Desktop, Web)
• Link to the abstractions (Electron) for further information - linking to Electron's quite nice overview on each platform. If something isn't right in Electron or its docs that is something that should be fixed there since VS Code is not the only consumers. Thankfully, we have the talent our team (@deepak1556) to drive Electron changes if something needs to happen.

I'll make a first pass at this, but I'd love to know your thoughts.

[deepak1556]

#202385 is unrelated to the sandbox model discussed here. Discussions related to sandboxing this process is tracked in #52116