bserdar/took

Plaintext Token Storage in .took.yaml

sgayou opened this issue · 5 comments

The .took.yaml configuration file stores secret and sensitive tokens for required functionality. The last access token is written to disk and stored. I am unsure why -- is this in order to support refresh tokens?

This issue could be a bit controversial as you can argue that as long as the user directory is locked down correctly (good job in 989ea1d, was going to mention that one), that should be enough and if anyone else gets access, some other vulnerability had to be exploited / the user may have made a mistake in another way. You could argue that the user should encrypt their own hard-drive, etc. Examples of this would be SSH private keys (although they do support encryption, it is not required).

Still, this design dramatically increases the attack surface and potential for secure token leakage. One simple example involves other CLI tools that store tokens to the hard disk. There are plenty of examples of users checking in these tokens (i.e. AWS tokens) to github. Can we do better?

Here's a few ideas:

  1. Store tokens in environment variables. Tokens/functionality would only be valid for the current session. You log out, you'll need to go through auth again to get a new token. This seems like a pretty good improvement.
  2. Encrypt the tokens and client secret. This could get complicated -- I don't know if golang has some form of cross-platform keystore. I suppose we could scrypt/AES the data. Would make the usability a little worse, but potentially help with security. Would require the user chooses a good password and we salt/hash use a good derivation function, etc.
  3. Use OS specific key store. Very OS specific, probably hard to implement in a good, cross-platform way. Unclear as I'm not a golang expert.

Any other thoughts?

I struggled with this one as well. Some of these would render took useless for the scenarios it is designed for. The use case I had in mind was that the user logs in once, and from that point forward, the user doesn't have to provide credentials again until the refresh token expires, because simply running 'took token cfg' would return the last token, or refresh it if necessary and return that one without any need to reauth.

One solution I can think of is: we encrypt ~/.took.yaml using a password user chooses, and have a "took login" option that will decrypt the file, stay running and serve the decrypted file to other took sessions via a domain socket under homedir. Is that overkill? I've seen other tools that store plaintext tokens under user dir. Is this really something to worry about?

Well, here's another example I was thinking about over the weekend. Amazon has something that appears to be a bit similar (my apologies to Amazon if I completely misunderstood what's going on here -- entirely possible. They also have plenty of other mitigations that could help when something like this occurs)

They have the concept of AWS cli apps that can interact with your AWS account for management and pretty much everything under the sun. See here: https://docs.aws.amazon.com/cli/latest/userguide/cli-config-files.html

The file is: ~/.aws/credentials (https://docs.aws.amazon.com/cli/latest/userguide/cli-config-files.html)

[default]
aws_access_key_id=AKIAIOSFODNN7EXAMPLE
aws_secret_access_key=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY

Effectively, we have an important, secret key on the user file-system. I'm sure you can guess what this leads to. If we search github for all credentials files that contain aws_secret_access_key, we get this:
https://github.com/search?q=filename%3Acredentials+aws_secret_access_key. Hundreds of pages of potential credentials. A lot are censored or test examples, but some do appear to look real/random. This is quite a well known issue at this point: https://blog.kchung.co/api-keys-api-keys-everywhere/

People make mistakes. With took, I can see someone backing up their home directory and not realizing that the .took.yaml has sensitive data. It isn't quite as obvious as a password manager file. Will the average user realize sensitive data is in the file? Documentation may be one partial solution to this.

That's why I like the environment variable idea. I'd prefer they aren't stored to a disk even via encryption, but that's certainly far better than plaintext. (AES and Argon2!) We're trading off usability for slight improvements in security. I think it's worth it, but happy to hear arguments either way.

As noted, I don't really understand what we're going to be using this for internally. Please feel free to e-mail me that use case if you'd like, may make my suggestions make more sense. Possible I'm misunderstanding use cases. :)

This will not be used internally. This will be used by customers to access our APIs from their scripts. The idea is to have the customer get a token, and then use this token from the scripts to get subscription information about systems, or errata information, etc. In a typical use case, the user logs in once, and then they can run their scripts without a need to log in again until the refresh token expires. With tokens in env vars, they're stuck in one terminal session, and when that's gone, they have to get a new token.

Leaning towards the encrypted tokens idea for now. I'll think a bit more.

This was a PoC, but then it worked. The 'secure' branch now contains encrypted token support. User can setup a password to ecnrypt ~/.took.yaml. Once this is done, user has to run took decrypt with the password to access the stored tokens. Ecnryption code is under crypta/.

Took decrypt runs another copy of took with the entered password. That copy of took remains in memory, listening to a domain socket ~/.took.s and responds to encrypt/decrypt requests. It dies if left idle for 10 mins, after which you have to run took decrypt again.

This is fixed as of d80093a