Superalgos / Superalgos

Free, open-source crypto trading bot, automated bitcoin / cryptocurrency trading software, algorithmic trading bots. Visually design your crypto trading bot, leveraging an integrated charting system, data-mining, backtesting, paper trading, and multi-server crypto bot deployments.
https://www.superalgos.org
Apache License 2.0
4.22k stars 5.55k forks source link

encrypt secrets #846

Open krisj opened 3 years ago

krisj commented 3 years ago

Among all the things that keep me away from devoting any further time to the project is the lack of basic security. I am realising only now that all secrets are stored in plain text! Exchange keys in particular, and for any upcoming API support - please implement strong encryption and do not store secrets in plain text.

Luis-Fernando-Molina commented 3 years ago

You are right, until now noone came with the solution of how to provide the keys to a bot if they are encrypted with a password that is known only to a human.

Ok, the bot could know the password, but it could not be encrypted, which will leave us with a similar situation.

I haven't try to think this much further to be honest..., but if you know how to do it, or how it is done elsewhere, let us know, or even better submit a PR with the solution and we will be happy to merge it.

krisj commented 3 years ago

Well there are a few ways to go.

  1. use the Node's built-in "crypto" module to encrypt things using the user's provided password before storing into the json files and the same to decrypt before use. User would have to type in the password every time the node starts and you could store it in some temporary ENV var. Later you could add a slightly un-safe method of "storing" the password for some time, but that would have to be designed with some timeout/ auto-logoff etc. A simple "enter password every time you start a session approach" would work fine at the beginning I think. You could also give an option to store in plain text if they dodn want to re-type pwd every time a sessions starts and then show a warning about unencrypted keys as a reminder.
  2. use a publicly available secrets manager solution like Hashicorp's Vault, AWS Secrets Manager or Google Cloud Secret Manager or similar. But that would mean no off-line access. Maybe that would be a good solution for the live nodes which need to be online anyway, so you would authorize them during the setup via GUI, but then for the normal operations they can keep themselves authorised via one of these cloud-based secret management solutions. That would also mean users having to create their accounts there with API keys etc I believe.
  3. come up with something more complex, but I don't think it is necessary.
Luis-Fernando-Molina commented 3 years ago

I like the approach number 1, which would cover the case hackers / virus / malware access the content of the hard drive, which is probably the most common thing to happen.

Still the keys would be unencrypted in-memory at the UI and Task Server while being in use by the trading bot. Also unencrypted in the communications between the UI and the Task Server process even if it is on a different machine (current version is expected to be used at your own LAN.

I think it could be a good step in the right direction. Thanks for the suggestion.

krisj commented 3 years ago

well - unencrypted in memory for sure - no way around that unless you run some crazy sub-VM setup, but that's overengineering in this case. I would also suggest running the UI over HTTPS instead of HTTP. Even with a self-signed cert would be fine initially.

from what I am hearing there are various ways hackers are fishing for keys these days. virus-like malware. sometimes even infected libraries that come in via build processes. It's getting crazy. in-memory keys are much much harder to get to (but also not impossible)

the cloud-based solutions I mentioned are really well-engineered and have some very cool features built-in by the way. Worth checking them out even if you don't want to integrate them. Gives you an insight into how they approach it.

Luis-Fernando-Molina commented 3 years ago

Thanks for the tips, will keep this issue open so that it come be done when the time is right.

PLuv commented 3 years ago

I am following this issue. This is something I will be happy to implement in due time if it is not picked up by someone else in the meantime.

Luis-Fernando-Molina commented 3 years ago

@PLuv please do it, it is getting difficult for me to find time for this.

ssplatt commented 2 years ago

In other web application spaces, Environment Variables are used to provide secrets to the application instead of hard coding them into a plain text configuration file that is available directly by the application (through the UI, or by an API call). If the variables are accessible directly then the Application needs to have authentication built into it which would "unlock" the encrypted secrets. Even if using a database instead of flat files, authentication in the application itself would be required.

isysd-mirror commented 2 years ago

I would like to suggest a different way that encrypting the files (not memory) could be done. Give bots key pairs, and encrypt to both the user and their approved bot's public keys. This could all be managed by plugging in an asymmetric keyring like PGP provides. Integrating PGP would have benefits elsewhere, as well, i.e. in tracking git commit signatures.

In regards to HTTPS, the keys used for x509 could also be used for this encryption. I wouldn't recommend it, though, since it is ugly on the client side.