Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the Secrets API #9348

Closed
fbricon opened this issue Apr 14, 2021 · 14 comments · Fixed by #9463
Closed

Implement the Secrets API #9348

fbricon opened this issue Apr 14, 2021 · 14 comments · Fixed by #9463
Assignees
Labels
vscode issues related to VSCode compatibility

Comments

@fbricon
Copy link

fbricon commented Apr 14, 2021

This is a follow up on #9288

Feature Description:

We're developing a Red Hat authentication provider (https://github.com/redhat-developer/vscode-redhat-account), but Theia is currently missing a series of APIs for it to work (as per https://che-incubator.github.io/vscode-theia-comparator/status.html).

ExtensionContext.secrets
SecretStorage
SecretStorageChangeEvent

@vince-fugnitto vince-fugnitto added the vscode issues related to VSCode compatibility label Apr 14, 2021
@vinokurig vinokurig self-assigned this Apr 20, 2021
@vinokurig
Copy link
Contributor

@fbricon Can you describe how are you going to use the Secrets API ?

@vinokurig
Copy link
Contributor

vinokurig commented Apr 26, 2021

While implementing this issue I've faced with a problem: as this API is supposed to work with sensitive data e.g. passwords, tokens, it is needed to encrypt the input data. Vscode does it in it's non open source vscode-encrypt library: https://github.com/microsoft/vscode/blob/1.55.2/src/vs/platform/encryption/electron-main/encryptionMainService.ts#L25. We can do it with the help of third party encryption tools like crypto-js but it requires a secret string. The problem is that we have to store this secret key hidden. So any ideas?

@breglerj
Copy link

According to the update notes the secrets API "allows extensions to store sensitive information in the OS credential manager or keystore". Couldn't you use keytar to implement the same functionality? Then you wouldn't need to manage a password in Theia.

@vinokurig
Copy link
Contributor

vinokurig commented Apr 27, 2021

According to the update notes the secrets API "allows extensions to store sensitive information in the OS credential manager or keystore".

We still need to encrypt the data before storing it: https://github.com/microsoft/vscode/blob/2e89c2d4ba5659c77ccde02605b87181658f8137/src/vs/workbench/api/browser/mainThreadSecretState.ts#L61

@paul-marechal
Copy link
Member

Securing passwords using reversible encryption is tricky... But since it's already done by other software (e.g. Chrome) I tried to look at how they do it, and found the following pieces:

So this makes me wonder if we'll need to write some node native extensions to leverage those OS APIs?

@paul-marechal
Copy link
Member

VS Code might need to encrypt things on their own because they store things in the LocalStorage?

microsoft/vscode@d0481dc#diff-6115f2e94c7a042ec80daa1347f65b83dd06c950d49e7f71621faec2b92fae2bR31

If we use keytar and send informations to the backend we might not need to encrypt since the system's credential manager should take care of that already?

@marcdumais-work
Copy link
Contributor

@vinokurig Do I understand correctly:

vscode-encrypt relies on "security by obscurity", encrypting a string before it's used/stored in an external tool? i.e. this only works because it's not immediately clear how the string is encrypted?

Us, on the other hand, need to rely on an open mechanism. We can still encrypt a string, but since the algorithm will be public, we need to provide some kind of unique seed or key, so that the result will not be (too) predictable. Whatever key we use, we'll need to store it or be able to re-generate it, so we can eventually decrypt the information.

@marcdumais-work
Copy link
Contributor

marcdumais-work commented Apr 27, 2021

VS Code might need to encrypt things on their own because they store things in the LocalStorage?

Not sure about that. I guess it depends what they are / we should be protecting-from. From the Chrome question linked above, the Chrome designers goal seems only to protect against other users accessing the data, I guess on disk.

"So still it eventually depends on the user password credentials. Once the user password (or rather its SHA1 hash) is known, all entries are decryptable. As said, this is by design. Even Microsoft's Edge (Chromium edition) uses this system now, as claimed in this blogpost."

If something is stored in local storage, I do not think know there's an easy way to extract that info, even logged-on as the user, outside the narrow context of code executing in the browser, sharing the same origin. So, in a sense, it feels more secure vs what Chrome does. But security is tricky.

@paul-marechal
Copy link
Member

If something is stored in local storage, I do not think know there's an easy way to extract that info, even logged-on as the user,

I don't think the LocalStorage is secure by default, somewhat related: https://stackoverflow.com/a/41052194/7983255

I was able to effortlessly read Chrome's LocalStorage ldb files on my machine using Python's leveldb module.

What I linked earlier concerned the way Chrome stores saved passwords only.

@paul-marechal
Copy link
Member

paul-marechal commented Apr 27, 2021

Looking at my own installation, vscode-encrypt is a Node.js native addon, located at Microsoft VS Code\resources\app\node_modules.asar.unpacked\vscode-encrypt\build\Release\vscode-encrypt-native.node. I wonder what kind of API they actually use.

@vinokurig
Copy link
Contributor

@marcdumais-work

@vinokurig Do I understand correctly:

vscode-encrypt relies on "security by obscurity", encrypting a string before it's used/stored in an external tool? i.e. this only works because it's not immediately clear how the string is encrypted?

Us, on the other hand, need to rely on an open mechanism. We can still encrypt a string, but since the algorithm will be public, we need to provide some kind of unique seed or key, so that the result will not be (too) predictable. Whatever key we use, we'll need to store it or be able to re-generate it, so we can eventually decrypt the information.

Exactly, that is the problem.

@vinokurig
Copy link
Contributor

@marechal-p

VS Code might need to encrypt things on their own because they store things in the LocalStorage?

microsoft/vscode@d0481dc#diff-6115f2e94c7a042ec80daa1347f65b83dd06c950d49e7f71621faec2b92fae2bR31

If we use keytar and send informations to the backend we might not need to encrypt since the system's credential manager should take care of that already?

The disadvantage of using keytar is that it is a OS utility so if theia is running in a container which may be redeployed, all the data will go away. This is the case for Eclipse Che.

@vinokurig
Copy link
Contributor

@marechal-p

Securing passwords using reversible encryption is tricky... But since it's already done by other software (e.g. Chrome) I tried to look at how they do it, and found the following pieces:

https://superuser.com/a/146744
https://security.stackexchange.com/a/170485
So this makes me wonder if we'll need to write some node native extensions to leverage those OS APIs?

This works in very few browsers, it works in chrome, but it doesn't work in firefox and safari.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants