While I remember it, we should also document that a malicious user who has view access to passwords (but not "show secret") can access the password like this:

• Create a repository using the credential, with remote URI http://evil.com/.
• Wait for Phabricator to perform a pull request.
• Their evil server can capture the credential.

One way to prevent this might be to let you lock credentials to certain use cases, although this is a little bit fluff right now since the two use cases are Harbormaster and Diffusion, which both allow arbitrary third-party URIs. In the case of Nuance, we might have credentials used for trusted URIs, so maybe this "use case lock" can really be "Only send this credential to trusted services."

And we should possibly consider splitting the permissions more granularly:

• View
• Show Secret
• Edit

...based on further feedback and how hard it is to make this distinction clear in the UI without splitting things. I think the material effect here of actually separating these permissions probably isn't hugely important, but properly separating them in the UI (not just with explanatory text) might be clearer than mere documentation.

Some ideas for encrypting everything:

• We can put an encryption key on the filesystem. This offers little protection in most cases since compromising the filesystem and database usually happens at the same time, but would protect backups, at least. It adds some management complexity with multiple web frontends.
• We can provide some echo secret | bin/passphrase load-key command, which makes a local HTTP request and loads the key into APC. Installs would include this in their restart script, and source the secret from some ephemeral location (in theory, manual entry at the CLI). This would also need to be passed to the daemons. But, roughly, we get an ephemeral key into process memory at restart time and that should raise the barrier at least slightly (although attackers with write access to the filesystem can read data out of APC fairly easily by editing the Phabricator code). This sounds nice at first, but the more I think about it the less I like it.
• Beyond these cases, we could handle decryption over Conduit (since it rarely needs to be performed) and have a single server capable of performing decryption. However, compromising any other server allows you to change all the metadata on the object (so anyone can see it) and then always make a decryption call.

Broadly, any scheme we select where Phabricator has a persistent copy of the decryption key does not present a huge barrier to most online attacks. Putting an optional key on the filesystem is probably simplest to deter offline attacks (e.g., against backups) and limited-access attacks (e.g., an SQL hole which lets you select arbitrary data).

Ideally, we should do all the crypto work for the "encrypted" credential type on the client. This requires figuring out some kind of reasonable, appropriately licensed JS library we can use.

@asherkin suggests this:

https://github.com/bitwiseshiftleft/sjcl/tree/version-0.8

◀ Merged tasks: T5513.

davedash: epriestley: nice yeah, the subtasks I really want from that are generate SSH key and show public key, so I can automatically connect phab to Github

◀ Merged tasks: T5734.

In T4721#53743, @epriestley wrote:

And we should possibly consider splitting the permissions more granularly:

• View
• Show Secret
• Edit

+1

In T4721#140545, @joshuaspence wrote:

In T4721#53743, @epriestley wrote:

And we should possibly consider splitting the permissions more granularly:

• View
• Show Secret
• Edit

+1

+1
This makes sense to me.