Credentials API

[jrieken]

We will offer extension API around keytar such that extensions can use the host OS secret store. Current proposal is:

	/**
	 * Namespace for handling credentials.
	 */
	export namespace credentials {

		/**
		 * Read a previously stored secret from the credential store.
		 *
		 * @param service The service of the credential.
		 * @param account The account of the credential.
		 * @return A promise for the secret of the credential.
		 */
		export function readSecret(service: string, account: string): Thenable<string | undefined>;

		/**
		 * Write a secret to the credential store.
		 *
		 * @param service The service of the credential.
		 * @param account The account of the credential.
		 * @param secret The secret of the credential to write to the credential store.
		 * @return A promise indicating completion of the operation.
		 */
		export function writeSecret(service: string, account: string, secret: string): Thenable<void>;

		/**
		 * Delete a previously stored secret from the credential store.
		 *
		 * @param service The service of the credential.
		 * @param account The account of the credential.
		 * @return A promise resolving to true if there was a secret for that service and account.
		 */
		export function deleteSecret(service: string, account: string): Thenable<boolean>;
	}

[jrieken]

To me this already looks proper, I wonder if we should create a new namespace or not. Until now we have been adding them very conservatively, my first instinct would be to have these functions in env which also host ofter platform'ish things. Another idea would be ExtensionContext tho it scopes things to this extension and I believe the secrets-store is shared for all, right?

[chrmarti]

Yes, it is shared by all extensions. That is given by our decision to use the OS key chain for implementation. No strong opinion on the namespace. I think it should have its own, like env.credentials.*.

[jrieken]

I think it should have its own,

Why do you think so?

[jrieken]

Concern raised during the standup is that now all extensions can read/write secrets on behalf of VS Code. We should consider making this API more explicit such that the editor can prompt (at least once) per extension. Something like a getSecretsStore-function that returns an object to read/write. During that function call we can ask the user if that particular extension is allowed to access secrets, e.g

interface SecretsStorage {
  read(): Thenable,
  write(...): Thenable,
  delete(...):Thenable
}

function getSecretsStorage(): Thenable<SecretsStorage> // <- will prompt the user per extension

With such a flow we would be on the save side, similar to browsers that must first get permissions from the OS to use your geo-locations, and then ask per website that wants to use your geo-locations

[chrmarti]

On the namespace: Since its already 3 functions it is nicer to put them in context of a name 'credentials'. env.readSecret() wouldn't speak as much for it self as env.credentials.readSecret() and env.readCredentialsSecret() is repetitive when used on all 3 functions.

On the confirmation per extension: I'm not convinced the confirmation per extension will add much. From what I have seen on Windows and Linux you unlock the OS key chain once per desktop session, after that all your applications can access it. Only on Mac are you asked per application and credential if that application may access it. An extension can bundle keytar itself and use the unlocked key chains on Windows and Linux and on Mac the prompt would say 'Code Helper' wants to access some credential which isn't much help either.

Unlike with browser's location and other restricted APIs where a webpage cannot bypass the user's decision, extensions have full access.

/cc @chrisdias

[jrieken]

'credentials'. env.readSecret() wouldn't speak as much for it self as env.credentials.readSecret() and env.readCredentialsSecret() is repetitive when used on all 3 functions.

What throws me off here that mixed usage of credentials and secret. What is it we are dealing with? Will there be more things in the credentials namespace which isn't a secret?

An extension can bundle keytar itself and use the unlocked key chains on Windows and Linux and on Mac the prompt would say 'Code Helper' wants to access some credential which isn't much help either.

Sure, we are not going for security but for clarity. Accessing credentials is a very touchy topic and while I might trust a certain extension I might not trust another extension. I think it's somehow more polite/fair to our users to tell them what extension is now going off with their secrets.

[chrmarti]

In my view the secret is just one part of the credential and it is the secret we want to store, not the service or the account name, these are used to access the secret. The credential is account name and secret.

If we ask the user for each secret and extension, wouldn't that lead the user to think that there is some security involved in that question, not just politeness?

Either way, I'd prefer an API with just a single level of promises. Even if we add the user confirmation, we could still go just with promises on the functions without need for a promise on the namespace.

[rmunn]

Let me advance a devil's-advocate argument for not doing this. First, an example, and then I'll get to the argument.

Currently, when I start VS Code in a fresh login session (in Linux) and load a folder connected to a Git repository, I see a popup asking for the passphrase to unlock my SSH private key. That's because VS Code's Git extension has tried to do a git fetch, which in turn has seen that my Git repo has an origin remote with an SSH URL (git@github.com:rmunn/my-cool-project). It tries to connect to that SSH URL, which causes OpenSSH to read my .ssh folder and find my private SSH key. That key is protected with a passphrase, so OpenSSH asks the ssh-agent process (which was automatically started when I logged in) to give it access to that particular private key. The ssh-agent process then says, "I don't yet have an unlocked version of that private key in memory, let me prompt the user" and causes a secure prompt to pop up to ask me for my passphrase. Once I type in my passphrase, that password is not stored in memory; instead, the ssh-agent process simply stores the unlocked private key (for a configurable length of time), using the Linux kernel's mlock syscall to ensure that that region of memory is never written to a swapfile (if it was ever written to disk, an attacker who stole my laptop could theoretically get my unlocked private key out of the swapfile, and do Evil Things with it).

That's a lot of complexity, but it Just Works™. And the argument is this: if VS Code adds an interface to keytar (or any other credential-management service), are extensions going to start relying on it? Will the Git extension end up not being able to use the system's SSH agent for unlocking passphrase-protected private keys, because its developers didn't think about the existence of other systems besides keytar? In the case of the Git extension specifically, the answer is almost certainly no, because Git access over SSH is a long-established method and plenty of people rely on it, and there would be no good reason for removing that feature (which works perfectly well) from the code. But in the case of some future extension that does something sensitive (say, managing your OAuth2 client IDs and secrets for you), what if the developers see the existence of the keytar API and take the path of least resistance, when a better solution exists that they didn't see?

This may or may not be a particularly likely scenario, but I think it's one that should be considered. Will adding this API cause people to ignore alternative options?

[dtretyakov]

We need a first class support of credentials store in VS Code API to store credentials for web service connection. It will allow extension developers not to bundle packages like keytar and not to implement their own credential stores.

In my vision use case described by @rmunn is not representative since it too specific for git. It's up to extension developer to use this API or not. In Git extension it may not be valuable due to existence of Git credentials store, but for VSTS/GitHub and other web service integrations it makes sence.

[chrmarti]

Agreed, the inclusion of keytar will not change what the Git extension or others relying on ssh keys should do.

On asking the user for confirmation before passing credentials to an extension: Offline discussions suggest that this is seen as "security theater" because it suggests added security when in fact we can't add any additional security beyond what the OS provides:

• Windows unlocks the entire credential store for all processes when the user logs in.
• Linux (libsecret on Ubuntu) asks the user for permission to unlock the entire credential store for all processes once per desktop session.
• Mac asks the user for confirmation for each credential and process.

What we would need to add on top of that is sandboxing of extensions, they have access to the full node.js API at the moment.

[jrieken]

On asking the user for confirmation before passing credentials to an extension

Again, this isn't about confirmation or making this more secure but to inform people. We have a number of dupes on this issue which is simply about showing a message. We also have UI to show which extension added what status bar item. So, my guess is that folks maybe wanna know what extension is using their credentials.

[chrmarti]

Are you suggesting to have a purely informational message without a button to "deny" access? Like: Extension x.y.z is accessing your credentials for service a.b.c. - OK.

As much as I would like that transparency (and also a way to deny access), it will incorrectly suggest that the user will always know when an extension accesses credentials, but we cannot guarantee that as long as extensions are not sandboxed.

Status bar items are different because there is not much gain in an extension working around the reporting there. Credentials are more sensitive and any kind of suggested control should to be backed by actual security measures, IMHO.

[jrieken]

As much as I would like that transparency (and also a way to deny access), it will incorrectly suggest that the user will always know when an extension accesses credentials, but we cannot guarantee that as long as extensions are not sandboxed

On the mac we could actually do that. Only the main process is able to get the nice prompt and before that happens we could show a modal dialog asking specifically for that extension. An "always"-choice should then be remembered in the keychain itself. Yes, on other platforms we cannot do that but that shouldn't stop us from being as good as possible per platform.

Again, the same sample: There is many ways to spoof on my geolocation, e.g. an app can make a http call to a server that derives my location from my IP. However, for most folks that is too much work and they want to use a utility API. On the mac that exists and when being used it shows its users. To me no sense of false security, just UI.

/cc @Microsoft/vscode

[chrmarti]

How do we make sure that an extension cannot trick the extension host or the main process into handing out credentials on behalf of some other extension? If we cannot secure that in a sandbox-way, the UI will suggest a false sense of security.

This doesn't compare well with the location service case because the OS might have access to a GPS unit and the precise location deserves better protection than the one guessed from the IP address. It is also not clear to me if the location service asks the user for confirmation for each application, maybe it's more a power-consumption than a security thing.

[kieferrm]

Pls see https://github.com/kieferrm/vscode-keytar-sample which demos what happens when using plain keytar.

[joaomoreno]

I personally think the location services are a great analogy. It's definitely not a power-consumption thing, but a privacy thing. The power-consumption thing is right next to it:

I don't think we'll get to a point in which we'll provide any type of reasonable security. The best we can do is provide awareness to the user. This has always been our motto: clicking that Install button is the contract upon which we start attributing fault of any possible risk on the user's conscious choice.

[mak42]

Any idea when this feature will be available?

[chrmarti]

The discussion on if and how to surface this in the UI is still ongoing.

[dtretyakov]

Hi, do you have any new details about discussion of Credentials API?

[chrmarti]

@dtretyakov No, this has stalled.

[vscodebot]

This issue is being closed to keep the number of issues in our inbox on a manageable level, we are closing issues that have been on the backlog for a long time but have not gained traction: We look at the number of votes the issue has received and the number of duplicate issues filed. If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding and happy coding!