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.