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

archive: allow preserving ownerships when unpacking #276

Merged
merged 5 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ jobs:
shell: bash
- run: cargo test
- run: cargo test --no-default-features
- name: Run cargo test with root
run: sudo -E $(which cargo) test
if: ${{ matrix.os == 'ubuntu-latest' }}

rustfmt:
name: Rustfmt
Expand Down
12 changes: 12 additions & 0 deletions src/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub struct ArchiveInner<R: ?Sized> {
pos: Cell<u64>,
unpack_xattrs: bool,
preserve_permissions: bool,
preserve_ownerships: bool,
preserve_mtime: bool,
overwrite: bool,
ignore_zeros: bool,
Expand Down Expand Up @@ -54,6 +55,7 @@ impl<R: Read> Archive<R> {
inner: ArchiveInner {
unpack_xattrs: false,
preserve_permissions: false,
preserve_ownerships: false,
preserve_mtime: true,
overwrite: true,
ignore_zeros: false,
Expand Down Expand Up @@ -126,6 +128,15 @@ impl<R: Read> Archive<R> {
self.inner.preserve_permissions = preserve;
}

/// Indicate whether numeric ownership ids (like uid and gid on Unix)
/// are preserved when unpacking this entry.
///
/// This flag is disabled by default and is currently only implemented on
/// Unix.
pub fn set_preserve_ownerships(&mut self, preserve: bool) {
self.inner.preserve_ownerships = preserve;
}

/// Indicate whether files and symlinks should be overwritten on extraction.
pub fn set_overwrite(&mut self, overwrite: bool) {
self.inner.overwrite = overwrite;
Expand Down Expand Up @@ -308,6 +319,7 @@ impl<'a> EntriesFields<'a> {
preserve_permissions: self.archive.inner.preserve_permissions,
preserve_mtime: self.archive.inner.preserve_mtime,
overwrite: self.archive.inner.overwrite,
preserve_ownerships: self.archive.inner.preserve_ownerships,
};

// Store where the next entry is, rounding up by 512 bytes (the size of
Expand Down
115 changes: 106 additions & 9 deletions src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub struct EntryFields<'a> {
pub data: Vec<EntryIo<'a>>,
pub unpack_xattrs: bool,
pub preserve_permissions: bool,
pub preserve_ownerships: bool,
pub preserve_mtime: bool,
pub overwrite: bool,
}
Expand Down Expand Up @@ -444,13 +445,36 @@ impl<'a> EntryFields<'a> {

/// Returns access to the header of this entry in the archive.
fn unpack(&mut self, target_base: Option<&Path>, dst: &Path) -> io::Result<Unpacked> {
fn set_perms_ownerships(
dst: &Path,
f: Option<&mut std::fs::File>,
header: &Header,
perms: bool,
ownerships: bool,
) -> io::Result<()> {
// ownerships need to be set first to avoid stripping SUID bits in the permissions ...
if ownerships {
set_ownerships(dst, &f, header.uid()?, header.gid()?)?;
}
// ... then set permissions, SUID bits set here is kept
if let Ok(mode) = header.mode() {
set_perms(dst, f, mode, perms)?;
}

Ok(())
}

let kind = self.header.entry_type();

if kind.is_dir() {
self.unpack_dir(dst)?;
if let Ok(mode) = self.header.mode() {
set_perms(dst, None, mode, self.preserve_permissions)?;
}
set_perms_ownerships(
dst,
None,
&self.header,
self.preserve_permissions,
self.preserve_ownerships,
)?;
return Ok(Unpacked::__Nonexhaustive);
} else if kind.is_hard_link() || kind.is_symlink() {
let src = match self.link_name()? {
Expand Down Expand Up @@ -553,9 +577,13 @@ impl<'a> EntryFields<'a> {
// Only applies to old headers.
if self.header.as_ustar().is_none() && self.path_bytes().ends_with(b"/") {
self.unpack_dir(dst)?;
if let Ok(mode) = self.header.mode() {
set_perms(dst, None, mode, self.preserve_permissions)?;
}
set_perms_ownerships(
dst,
None,
&self.header,
self.preserve_permissions,
self.preserve_ownerships,
)?;
return Ok(Unpacked::__Nonexhaustive);
}

Expand Down Expand Up @@ -632,14 +660,83 @@ impl<'a> EntryFields<'a> {
})?;
}
}
if let Ok(mode) = self.header.mode() {
set_perms(dst, Some(&mut f), mode, self.preserve_permissions)?;
}
set_perms_ownerships(
dst,
Some(&mut f),
&self.header,
self.preserve_permissions,
self.preserve_ownerships,
)?;
if self.unpack_xattrs {
set_xattrs(self, dst)?;
}
return Ok(Unpacked::File(f));

fn set_ownerships(
dst: &Path,
f: &Option<&mut std::fs::File>,
uid: u64,
gid: u64,
) -> Result<(), TarError> {
_set_ownerships(dst, f, uid, gid).map_err(|e| {
TarError::new(
format!(
"failed to set ownerships to uid={:?}, gid={:?} \
for `{}`",
uid,
gid,
dst.display()
),
e,
)
})
}

#[cfg(unix)]
fn _set_ownerships(
dst: &Path,
f: &Option<&mut std::fs::File>,
uid: u64,
gid: u64,
) -> io::Result<()> {
use std::os::unix::prelude::*;

match f {
Some(f) => unsafe {
let fd = f.as_raw_fd();
if libc::fchown(fd, uid as u32, gid as u32) != 0 {
liushuyu marked this conversation as resolved.
Show resolved Hide resolved
Err(io::Error::last_os_error())
} else {
Ok(())
}
},
None => unsafe {
let path = std::ffi::CString::new(dst.as_os_str().as_bytes()).map_err(|e| {
io::Error::new(
io::ErrorKind::Other,
format!("path contains null character: {:?}", e),
)
})?;
if libc::lchown(path.as_ptr(), uid as u32, gid as u32) != 0 {
liushuyu marked this conversation as resolved.
Show resolved Hide resolved
Err(io::Error::last_os_error())
} else {
Ok(())
}
},
}
}

// Windows does not support posix numeric ownership IDs
#[cfg(any(windows, target_arch = "wasm32"))]
fn _set_ownerships(
_: &Path,
_: &Option<&mut std::fs::File>,
_: u64,
_: u64,
) -> io::Result<()> {
Ok(())
}

fn set_perms(
dst: &Path,
f: Option<&mut std::fs::File>,
Expand Down
54 changes: 54 additions & 0 deletions tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,3 +1385,57 @@ fn header_size_overflow() {
err
);
}

#[test]
#[cfg(unix)]
fn ownership_preserving() {
use std::os::unix::prelude::*;

let mut rdr = Vec::new();
let mut ar = Builder::new(&mut rdr);
let data: &[u8] = &[];
let mut header = Header::new_gnu();
// file 1 with uid = 580800000, gid = 580800000
header.set_gid(580800000);
header.set_uid(580800000);
t!(header.set_path("iamuid580800000"));
header.set_size(0);
header.set_cksum();
t!(ar.append(&header, data));
// file 2 with uid = 580800001, gid = 580800000
header.set_uid(580800001);
t!(header.set_path("iamuid580800001"));
header.set_cksum();
t!(ar.append(&header, data));
// file 3 with uid = 580800002, gid = 580800002
header.set_gid(580800002);
header.set_uid(580800002);
t!(header.set_path("iamuid580800002"));
header.set_cksum();
t!(ar.append(&header, data));
t!(ar.finish());

let rdr = Cursor::new(t!(ar.into_inner()));
let td = t!(TempBuilder::new().prefix("tar-rs").tempdir());
let mut ar = Archive::new(rdr);
ar.set_preserve_ownerships(true);

if unsafe { libc::getuid() } == 0 {
assert!(ar.unpack(td.path()).is_ok());
// validate against premade files
// iamuid580800001 has this ownership: 580800001:580800000
let meta = std::fs::metadata(td.path().join("iamuid580800000")).unwrap();
assert_eq!(meta.uid(), 580800000);
assert_eq!(meta.gid(), 580800000);
let meta = std::fs::metadata(td.path().join("iamuid580800001")).unwrap();
assert_eq!(meta.uid(), 580800001);
assert_eq!(meta.gid(), 580800000);
let meta = std::fs::metadata(td.path().join("iamuid580800002")).unwrap();
assert_eq!(meta.uid(), 580800002);
assert_eq!(meta.gid(), 580800002);
} else {
// it's not possible to unpack tar while preserving ownership
// without root permissions
assert!(ar.unpack(td.path()).is_err());
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK this is unlikely to ever get executed, even in CI, so could a test be written that runs in normal user mode as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK this is unlikely to ever get executed, even in CI, so could a test be written that runs in normal user mode as well?

I think it would be very difficult. chown itself is a privileged operation, even on Windows. So it would be not possible to test it properly using normal user permissions.

Although, there might be a way to change gid on a file that is currently owned by the current effective user (file uid == user euid). But this kind of test would not cover the uid change, so it would be very difficult if not impossible to thoroughly test this functionality under a normal user account.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is the situation can you arrange for the test to happen with sudo on CI?

Also ideally tthis wouldn't check in a *.tar file but build it up here so the tar file can be easily changed as well.

}
Binary file added tests/archives/ownership.tar
Binary file not shown.