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

Use os.open to ensure that files for the cache have correct perms #22

Merged
merged 1 commit into from
Apr 22, 2014

Conversation

dstufft
Copy link
Contributor

@dstufft dstufft commented Apr 22, 2014

The previous file handling code suffered from a few issues.

  1. It would only create the leaf directory if it didn't exist but would error if any parent directories didn't exist. This will use os.makedirs to create anything needed.
  2. Directories created by FileCache uses the default umask, instead we'll make this configurable and make it default to 0700 which will prevent any access from anyone but the user who owns the process.
  3. Files created by FileCache use the default umask, instead we'll make this configurable and make it defalt to 0600 so only the user who owns the process can read/write them.
  4. Following the cache file mode bits above, we'll also ensure that we create the files securely to prevent against symlink attacks or someone dropping a file with bad permissions before we open it.

The previous file handling code suffered from a few issues.

1. It would only create the leaf directory if it didn't exist but
   would error if any parent directories didn't exist. This will
   use os.makedirs to create anything needed.
2. Directories created by FileCache uses the default umask, instead
   we'll make this configurable and make it default to 0700 which
   will prevent any access from anyone but the user who owns the
   process.
3. Files created by FileCache use the default umask, instead we'll
   make this configurable and make it defalt to 0600 so only the
   user who owns the process can read/write them.
4. Following the cache file mode bits above, we'll also ensure that
   we create the files securely to prevent against symlink attacks
   or someone dropping a file with bad permissions before we open
   it.
@dstufft
Copy link
Contributor Author

dstufft commented Apr 22, 2014

Note that this is technically backwards incompatible. It will break in the case that anyone was expecting to share the same cache directory between processed owned by different users. However given that pickle files are essentially executable code I think it makes sense to be secure by default.

@ionrock
Copy link
Contributor

ionrock commented Apr 22, 2014

Ah ok. That does make sense. Thanks for the use case and explanation!

ionrock added a commit that referenced this pull request Apr 22, 2014
Use os.open to ensure that files for the cache have correct perms
@ionrock ionrock merged commit f4cf535 into psf:master Apr 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants