Skip to content

site_cache_dir: use /var/tmp instead of /var/cache on unix#148

Merged
gaborbernat merged 1 commit intotox-dev:mainfrom
efiop:site_cache_dir
Mar 10, 2023
Merged

site_cache_dir: use /var/tmp instead of /var/cache on unix#148
gaborbernat merged 1 commit intotox-dev:mainfrom
efiop:site_cache_dir

Conversation

@efiop
Copy link
Contributor

@efiop efiop commented Mar 10, 2023

Turns out /var/cache might be non-writable by regular users (e.g. on ubuntu), so we are better off using /var/tmp which is and it is what was suggested in original appdirs discussion in ActiveState/appdirs#77

site_cache_dir was introduced very recently and it seems fine to make this breaking change right now.

Related #145

Turns out `/var/cache` might be non-writable by regular users (e.g. on ubuntu),
so we are better off using `/var/tmp` which is and it is what was suggested in
original appdirs discussion in ActiveState/appdirs#77
@gaborbernat gaborbernat merged commit c9202f7 into tox-dev:main Mar 10, 2023
@ThomasWaldmann
Copy link
Contributor

ThomasWaldmann commented Mar 16, 2023

Hmm, this change somehow seems counter-intuitive.

Also, check what the FHS says about this (5.5 and 5.15):

https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch05.html

Especially that stuff in /var/tmp is deleted "less frequently", but in a site-specific (not: app-specific) manner seems to make it unsuitable for application cache files. Cache files are not temporary files, they are just files that can be rebuilt (but that might be expensive, so you don't want some external "site cleanup cron job" delete that stuff).

Maybe the only thing that caused the initial issue (see top post) with /var/cache is that the package installation script (running as root) would need to create a subdirectory (like /var/cache/APPNAME) there with suitable permissions?

@efiop
Copy link
Contributor Author

efiop commented Mar 16, 2023

@ThomasWaldmann The application was not installed by root, but user-installed applications want to share expensive cache between users as well. In practical terms it seems systemd will delete files that weren't even read by anyone in /var/tmp once a month https://systemd.io/TEMPORARY_DIRECTORIES , which could be considered a free cache expiration policy 😄

I agree that /var/cache seems more suitable on paper, hence why I implemented it like that initially. But app-specific and site-specific don't seem mutually exclusive to me, as it is normal that the site has its own cache policy (e.g. when running low on space). I totally get that this is stretching /var/tmp a bit. Maybe what we need here is site_tmp_dir with persistent(or some other name) option to switch from /tmp to /var/tmp? Wdyt?

@andersk
Copy link
Contributor

andersk commented Nov 9, 2023

/var/tmp is an insecure location to store anything with a predictable filename, because any other user could have written it first. This leads to vulnerabilities categorized under CWE-377 and CAPEC-149.

This breaking change in platformdirs’ behavior is especially dangerous because it will have introduced these vulnerabilities into applications that were previously secure!

@ThomasWaldmann is correct that applications should put their own cache data in a subdirectory of /var/cache (e.g. /var/cache/cups), and the application’s package is responsible for ensuring the subdirectory exists and giving it the correct permissions.

Please consider reverting this.

@gaborbernat
Copy link
Member

is correct that applications should put their own cache data in a subdirectory of /var/cache (e.g. /var/cache/cups), and the application’s package is responsible for ensuring the subdirectory exists and giving it the correct permissions.

Open a PR with this change. We do not plan to revert.

andersk added a commit to andersk/platformdirs that referenced this pull request Nov 9, 2023
This directory was changed from /var/cache to /var/tmp in tox-dev#148 due to
permissions issues. However, /var/tmp is an insecure location to store
anything with a predictable filename, because any other user could
have written it first. This leads to vulnerabilities categorized under
CWE-377 and CAPEC-149.

To deal with the permissions issues, applications should put their own
cache data in a subdirectory of /var/cache (e.g. /var/cache/cups), and
the application’s package is responsible for ensuring the subdirectory
exists and giving it the correct permissions.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk
Copy link
Contributor

andersk commented Nov 9, 2023

Opened #239.

@efiop
Copy link
Contributor Author

efiop commented Nov 10, 2023

This breaking change in platformdirs’ behavior is especially dangerous because it will have introduced these vulnerabilities into applications that were previously secure!

Just to be clear, site_cache_dir was introduced about a week before this PR, realistically I doubt anyone was using it during that period.

Opened #239.

Thanks, let's continue there.

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.

4 participants