Merge lp://qastaging/~jml/pkgme-service/lzma-support into lp://qastaging/pkgme-service

Proposed by Jonathan Lange
Status: Merged
Approved by: James Westby
Approved revision: 133
Merged at revision: 121
Proposed branch: lp://qastaging/~jml/pkgme-service/lzma-support
Merge into: lp://qastaging/pkgme-service
Prerequisite: lp://qastaging/~jml/pkgme-service/move-utils-from-tasks
Diff against target: 177 lines (+93/-21)
2 files modified
src/djpkgme/osutils.py (+33/-21)
src/djpkgme/tests/test_osutils.py (+60/-0)
To merge this branch: bzr merge lp://qastaging/~jml/pkgme-service/lzma-support
Reviewer Review Type Date Requested Status
James Westby (community) Approve
Review via email: mp+124930@code.qastaging.launchpad.net

Commit message

Support .tar.lzma files.

Description of the change

Adds a thing to try to use the 'tar' command to extract what we're given. This is only used as a fallback after the existing tarfile and zipfile extractors, since I'm assuming that spawning subprocesses is more expensive.

This gives us lzma support, as tar automatically detects compression to use and as lzma support is available on any precise machine, due to apt depending on xz-utils, which provides it.

There are direct tests, as well as a test for prepare_file.

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

Hi,

In execing tar there are a few things we have to be careful of to prevent
attacks.

--force-local will mean that if someone uploads a tarball with a colon in the name
it won't try and connect to a remote machine to find the file.

We want --no-same-owner and --no-same-permissions, but they are default for
non-root users, so I don't know if we want to be explicit or not.

In addition, I think this will fix some issues we see in scoreboard with
e.g. unable to process tarfiles, or trying to extract to absolute paths.

Rather than fixing those, we could just replace the existing tarfile extractor
with this one. You are right that it is more expensive though, so maybe we
don't want to.

Thanks,

James

review: Needs Fixing
133. By Jonathan Lange

Only use 'tar' command, rather than Python's tarfile to try to extract
tarballs.

Add some options to prevent certain security exploits.

Revision history for this message
Jonathan Lange (jml) wrote :

We haven't demonstrated that the greater expense matters. We have demonstrated that the errors do. Have deleted the old _try_extract_tarfile. We now rely only on try_extract_with_tar_command.

Have added the extra options for defense-in-depth.

Revision history for this message
James Westby (james-w) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches