Secure passwords in urls #1136

Open spine-o-bot opened 3 years ago

In GitLab by @manuelma on Jan 28, 2021, 12:16

At the moment the password is stored unscrambled in the project.json file. We should fix that for starters, and then investigate and tackle all related security issues. We need to check what does sqlalchemy have to say about it.

Re @jkiviluo

In GitLab by @manuelma on Jan 28, 2021, 12:16

changed the description

In GitLab by @soininen on Jan 29, 2021, 08:30

I hope there is a nice way to make Toolbox secure regarding password but it is a complicated topic. Besides project.json, we need to make sure that passwords are passed securely to Engine and Tools. Major operating systems offer password storages. Could we hook up to those? Individual software also have password storages, e.g. major web browsers. Are there Python compatible implementations available?

In GitLab by @manuelma on Jan 29, 2021, 09:23

we need to make sure that passwords are passed securely to Engine and Tools

What do you mean exactly, what kind of attacks are possible here? Doesn't the hacker need to have complete access to the machine to do some kind of attack in our app?

In GitLab by @soininen on Jan 29, 2021, 09:46

True, I'm not exactly sure what attack vectors we need to consider here. I was just thinking that we tend to write stuff on disk for/during execution. One such thing could be the database URL which may contain username&password. This data shouldn't contain clear text passwords either or at least we need to make sure such files are erased afterwards. Execution has changed a lot recently so I may not be completely up to date here, though.

In GitLab by @manuelma on Jan 29, 2021, 10:12

Thanks for the explanation. I don't immediately recall any component writing urls to files, maybe DT?

For what it's worth:

The 'filters in arrows' feature writes the resource label (e.g. {db_url@Master data}) to the project .json file, but never the url, so it's safe.
The url is passed sometimes to Tool scripts in the command line arguments. But I think the user is responsible for what happens from there.
The url line edit in the DS properties probably supports copying to clipboard, in which case we'd also want to disable it.

In GitLab by @soininen on Jan 29, 2021, 10:16

DT only writes the filter's configuration which does not contain the URL so that should be safe.

The url line edit in the DS properties probably supports copying to clipboard, in which case we'd also want to disable it.

Just tested this. The QLineEdit is correctly set to the password echo mode (gives dots for each character written on the field) and copying is disabled so that should be safe, too.

In GitLab by @PekkaSavolainen on Jan 29, 2021, 10:44

I guess we could use something like cryptography's fernet algorithm for securing the password in project.json but then we have the problem of how to deal with the key that it requires. The other option is to not save the password into project.json at all and then user's just need to provide the password (once) every time they open up toolbox.

In GitLab by @manuelma on Jan 29, 2021, 11:04

The other option is to not save the password into project.json at all and then user's just need to provide the password (once) every time they open up toolbox.

I think we need to go with this.

In GitLab by @soininen on Jan 29, 2021, 11:05

Could something like this work:

If there are passwod in a project, ask user to create a master password and use that to encrypt passwords in project.json
Every time user opens a project with passwords, ask user to enter the project's master password

In GitLab by @soininen on Jan 29, 2021, 11:11

I am a bit concerned about users that have multiple password protected databases and the extra work having to provide passwords for all of them, hence I am proposing all kinds of password storages. Maybe it is not that big problem and making us to ensure secure storing is just too much work. I'm OK if we end up not saving passwords anywhere, too.

In GitLab by @manuelma on Jan 29, 2021, 11:35

That may work. It's a risk though, since our code is open source, so attackers will have access to the encryption algorithm. At that point, all they need to do is generate a project .json file with 'enough' encrypted passwords to unlock the key.

In GitLab by @jkiviluo on Jan 29, 2021, 11:47

There must be precedents for this kind of problem.

Until we have something reasonably secure, there should be a warning that the password is stored insecurely. Current needs don't actually have data that needs to be secured.

I stumbled on this again. The username and password for the DB connection should not be stored in project.json at all, when the project is to be shared with many users.

For example, pymysql offers reading a my.cnf file where those are given, see https://github.com/PyMySQL/PyMySQL/blob/main/pymysql/connections.py#L115. The contents are like

[client]
user     = username
password = password

This has become more urgent now that we've started using MySQL in a project. The most glaring issue is that the credentials are stored in project.json which in turn is version controlled by git forcing users to clear their usernames/passwords before committing to the repository. This particular problem would be solved if the credentials were stored in a separate file from project.json that can be included in .gitignore. Such files have been previously discussed in #1238

The sensitive entries in Data Stores' item dicts are now stored in <project dir>/.spinetoolbox/local/project_local_data.json. Note, that you need to force-save existing projects to make this effective. Additionally, it might be a good idea to add .spinetoolbox/local/ into any .gitignore files your projects may have.

spine-tools / Spine-Toolbox

Secure passwords in urls #1136