On Tue, Jan 07, 2020 at 04:45:54PM -0500, Brian Bouterse wrote:
> We had two bugs filed recently [0][1] which suggest that when using the
> default backend for Pulp, i.e. pulpcore.app.models.storage.FileSystem
> Pulp should not be "moving" files. This is the default behavior Django
> gives us, and it destroys data when sync'ed from file:/// for example
> [1].
> I propose that with 3.1 we fix this bug by switching
> pulpcore.app.models.storage.FileSystem to leave files in place and
> either hard-link (same filesystem) or copy (different filesystem).
> This will require files to be cleaned up where before the were "moved"
> so they didn't need cleanup. This will include worker temp directories,
> uploaded files, uploaded files w/ the chunked API, and downloaded files
> during sync. I believe this is all straightforward, but an important
> side-effect of this change to identify. Plugin writers that manage
> files would also need to handle this, but can mostly rely on pulpcore
> cleaning up the worker temp directories and user-uploaded files.
> What do you think about this? Do you have concerns? Is there something
> better we can do?
Yes, I have concerns:
- Aren't there files that are readable for the worker ("pulp" user), but
may be not for the user doing the import? The user could try to
import them. "file:///etc/pulp/settings.py" comes to mind as a
juicy target (Pulp pun intended).
Can we ensure that this can't happen? And which part of Pulp must
ensure this?
- In general, we must not trust paths supplied by the user; things
like missing path normalization and path comparisons using
startswith() make me nervous
(https://access.redhat.com/security/cve/cve-2018-10917
comes to mind).
_______________________________________________
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev
