Hackage credentials file is world-readable when saved

[nh2]

saveCreds (

stack/src/Stack/Upload.hs

Line 112 in bcf73ec

saveCreds :: Config -> HackageCreds -> IO ()

) creates ~/.stack/upload/credentials.json (containing Hackage username and password) without explicit file permissions, which results in the default umask being used, which is typically rw-rw-r, e.g. on Ubuntu.

This means that when your home directory is also world-readable (on Ubuntu it is, rwxr-xr-x), other users can grab your Hackage credentials.

For single user systems this is less problematic but not ideal for e.g. university setups where all users' homes are mounted over NFS.

The fix would be for stack to create this file with rw------- permissions (and ideally check this when reading so that upgrades from old versions of stack can notice the problem).

Independently, as a person who generally dislikes any on-disk plaintext passwords, I'd prefer if the default behaviour was not to save the credentials on disk, but use a flag for that.

[nh2]

It's unclear to me how to safely create this credentials file (atomically with the right permissions) in a cross-platform fashion.

open(filename, O_RDWR|O_CREAT, 0600) would do on Unixes, but to do that we need the unix package (unix-compat, which stack currently uses, does not provide it).

I've asked this on http://stackoverflow.com/questions/37304650/how-to-create-a-user-only-readable-file-cross-platform to get further input.

It seems to me that without further info, we have to conditionally depend on the unix package to make this safe at least on Unix (a touch; chmod 600; write approach isn't guaranteed to be safe because somebody else can open the file for reading before the permissions change applies, especially with inotify this is very feasible).

[fmthoma]

In addition to restricting the file permissions, I would suggest not saving the password by default. There could be a config option, or a prompt when first running stack upload, so the user can decide whether or not to save the password to disk, but this should be opt-in and not default behavior.

[mat8913]

Would it be safe to do something like mkdir tmp; chmod 700 tmp; touch tmp/file; chmod 600 tmp/file; mv tmp/file .; rmdir tmp; write?

[nh2]

Would it be safe to do something like mkdir tmp; chmod 700 tmp; touch tmp/file; chmod 600 tmp/file; mv tmp/file .; rmdir tmp; write?

Yes, that should work. I don't tink you necessarily need to create the temporary directory, putting your temp file into the same dir as the destination would also work.

I see your key point though, this can be done using just functions from directory, e.g. renameFile and setPermissions. Good find!

(To detail on the case of using unix: rm, setFileCreationMask, openFd with the correct permissions would also be sufficient,which is the equivalent of the C code I've mentioned above.)

[decentral1se]

There could be a config option

Please note, since ca6ba8f (Stack 1.5.0) it is possible to pass:

save-hackage-creds: false

This is not the default (I believe) but a message is shown.

I believe the permissions issue is still not addressed. I'll take a look.

[decentral1se]

@nh2, I took a quick look again here ...

Would it be safe to do something like mkdir tmp; chmod 700 tmp; touch tmp/file; chmod 600 tmp/file; mv tmp/file .; rmdir tmp; write?

Yes, that should work. I don't tink you necessarily need to create the temporary directory, putting your temp file into the same dir as the destination would also work.

Hmm, but is this not the same as you mentioned in:

(a touch; chmod 600; write approach isn't guaranteed to be safe because somebody else can open the file for reading before the permissions change applies, especially with inotify this is very feasible).

I'm really not sure what this would even mean on a Windows machine.

I'm inclined to think this whole file saving is a bad idea now as mentioned in comments above. Especially thinking cross-platform. Could we not just load username/password in from environment variables (user can deal with this safely)? Any objections?

[mattaudesse]

FWIW I agree with @lwm - automatically having credentials stored in the clear doesn't seem worth the risk considering how little effort it would require for users who understood the vulnerability and wanted to set it up manually anyway.

[snoyberg]

What about changing the default from "save to the file" to "prompt the user if the password should be saved"? We can also give the user a message that, if they're tired of getting prompted, they can set save-hackage-creds: false.

[decentral1se]

@nh2 ^^^

Seems like we remove the default of saving with this suggested approach. save-hackage-creds could easily be set in the global project configuration for security conscious users to not have to think about this again.

I see cabal also allows to save credentials in plain text in a configuration file. To be honest, the only decent way I've seen of keeping credentials safe on disk is using the gpg tool to encrypt and then having them decrypted and piped out - which I'm almost sure this is too much work to implement here.

[nh2]

Hmm, but is this not the same as you mentioned in:

@lwm The difference is the mv. I said touch + chmod + write isn't safe, but @mat8913 suggested touch + chmod + mv + write, which IIRC would be safe.

I'm really not sure what this would even mean on a Windows machine.

Which part do you mean, the inotify bit? What I meant is simply that if somebody wants to snatch your password by exploiting the fact that it's world-readable for a short moment, it is easy to do so with tools like inotify.

What about changing the default from "save to the file" to "prompt the user if the password should be saved"?

@snoyberg Sounds good!

Seems like we remove the default of saving with this suggested approach. save-hackage-creds could easily be set in the global project configuration for security conscious users to not have to think about this again.

@lwm I may have been a bit unclear in titling the issue -- my problem isn't that stack saves passwords in plain text files (this is very common and useful, e.g. ~/.aws/credentials and ~/.ssh/id_rsa also are plain text passwords); the problem is only that the permissions of those files are world-readable or a short amount of time.

Whether we ask for the password to be saved seems completely orthogonal to this -- whenever we save credentials on disk, we should ensure that the permissions are right before the contents are written. The touch + chmod + mv + write approach should solve that well.