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 2 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
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
83 changes: 83 additions & 0 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 @@ -451,6 +452,9 @@ impl<'a> EntryFields<'a> {
if let Ok(mode) = self.header.mode() {
set_perms(dst, None, mode, self.preserve_permissions)?;
}
if self.preserve_ownerships {
set_ownerships(dst, None, self.header.uid().ok(), self.header.gid().ok())?;
}
return Ok(Unpacked::__Nonexhaustive);
} else if kind.is_hard_link() || kind.is_symlink() {
let src = match self.link_name()? {
Expand Down Expand Up @@ -556,6 +560,9 @@ impl<'a> EntryFields<'a> {
if let Ok(mode) = self.header.mode() {
set_perms(dst, None, mode, self.preserve_permissions)?;
}
if self.preserve_ownerships {
set_ownerships(dst, None, self.header.uid().ok(), self.header.gid().ok())?;
}
return Ok(Unpacked::__Nonexhaustive);
}

Expand Down Expand Up @@ -635,11 +642,87 @@ impl<'a> EntryFields<'a> {
if let Ok(mode) = self.header.mode() {
set_perms(dst, Some(&mut f), mode, self.preserve_permissions)?;
}
if self.preserve_ownerships {
set_ownerships(
dst,
Some(&mut f),
self.header.uid().ok(),
self.header.gid().ok(),
)?;
}
liushuyu marked this conversation as resolved.
Show resolved Hide resolved
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: Option<u64>,
gid: Option<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: Option<u64>,
gid: Option<u64>,
) -> io::Result<()> {
use std::os::unix::prelude::*;

// -1 (0xffffffff) means no ownership change in most libc implementations
liushuyu marked this conversation as resolved.
Show resolved Hide resolved
let uid = uid.unwrap_or(0xFFFF_FFFF);
let gid = gid.unwrap_or(0xFFFF_FFFF);
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>,
_: Option<u64>,
_: Option<u64>,
) -> io::Result<()> {
Ok(())
}

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

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

let td = t!(TempBuilder::new().prefix("tar-rs").tempdir());
let rdr = Cursor::new(tar!("ownership.tar"));
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.