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

Issue 2 check ownership groups #7

Closed
Show file tree
Hide file tree
Changes from all 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 Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,6 @@ winapi = { version = "0.3", features = ["winbase"] }

[dev-dependencies]
diff = "0.1.10"

[dependencies]
Copy link
Contributor

Choose a reason for hiding this comment

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

This dependency should be Unix only, see this section of the Cargo book for a how to

Suggested change
[dependencies]
[target.'cfg(unix)'.dependencies]

users = "0.11.0"
80 changes: 79 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,11 @@ The API comes in two flavors:
#[cfg(target_os = "windows")]
extern crate winapi;

use std::io;
use std::path::Path;

#[cfg(target_os = "unix")]
use std::os::unix::fs::PermissionsExt;
/// Returns `true` if there is a file at the given path and it is
/// executable. Returns `false` otherwise.
///
Expand All @@ -68,6 +71,8 @@ where
path.as_ref().is_executable()
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Adding random whitespace isn't good practice for creating a PR. You don't want to change anything more than necessary


/// An extension trait for `std::fs::Path` providing an `is_executable` method.
///
/// See the module documentation for examples.
Expand All @@ -79,12 +84,45 @@ pub trait IsExecutable {
fn is_executable(&self) -> bool;
}

/// Returns `Result<Path, io::Error>` if there is a file at the given path and the
/// current run-level is permitted to execute it.
///
/// See the module documentation for details.
#[cfg(unix)]
pub fn is_permitted<P>(path: P) -> Result<::std::path::PathBuf, io::Error>
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the full path for io here and get rid of the import on line 58, it's unneeded unless compiling for Unix

where
P: AsRef<Path>
{
path.as_ref().is_permitted()
}

/// An extension trait for `std::fs::Path` providing an `is_permitted` method.
///
/// See the module documentation for examples.
#[cfg(unix)]
pub trait IsPermitted {
Copy link
Contributor

Choose a reason for hiding this comment

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

Both this trait declaration and is_permitted might as well go into the mod unix, You may need to add a pub use statement to re-export what's needed to the base of the crate if that's what you're after

/// Returns `Result<Path, io::Error>` that describes if a particular file
/// exists at the given path and the run-level of the current context meets
/// the appropriate user, group, admin, root/system-level membership.
///
/// Note: *this does not inspect whether the `Path` is executable.*
fn is_permitted(&self) -> Result<::std::path::PathBuf, ::std::io::Error>;
}

#[cfg(unix)]
mod unix {
use Path;
use std::os::unix::fs::MetadataExt;
use std::os::unix::fs::PermissionsExt;
use std::path::Path;

extern crate users;
use self::users::{
group_access_list,
get_effective_uid,
};
use std::fs;
use super::IsExecutable;
use super::IsPermitted;

impl IsExecutable for Path {
fn is_executable(&self) -> bool {
Expand All @@ -96,6 +134,46 @@ mod unix {
metadata.is_file() && permissions.mode() & 0o111 != 0
}
}

/// Could this target path be ran with executable permissions
/// by this runtime's run-level user?
///
/// Suppose the GID for the file in question is (Gm).
/// Assuming that the set of all groups shared by the user,
/// (G* := { Gn, Gn+1, Gn+2, ...}), is a superset of the set whose only
/// entry is the user's GID, (G_user := G* - Gn).
/// Then, Ǝ a possibility some arbitary GID, like (Gn+14) for example,
/// happens to be the GID belonging to group on that file.
///
/// In other words, we'll collect each of the (G*) entries
/// and see if there's a match. Otherwise, we check who owns the file and
/// perform a similar check.
impl IsPermitted for Path {
Copy link
Contributor

Choose a reason for hiding this comment

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

The formatting within this impl is questionable. I'd recommend trying to use rustfmt if you can do without massively changing other parts of the lib, or tidying this up yourself by hand

fn is_permitted(&self) -> Result<::std::path::PathBuf, ::std::io::Error> {
let (metadata, buf) = match self.metadata() {
Ok(md) => { (Some(md), self.to_path_buf()) },
Err(e) => { return Err(e) }
};
match metadata.unwrap().is_file() {
true => {
let file_gid: u32 = fs::metadata(buf.to_str().unwrap()).unwrap().gid();
if let Some(_gid_match) = group_access_list()
.unwrap()
.into_iter()
.take_while(|grp| file_gid != grp.gid())
.last() { return Ok(buf) }
else if fs::metadata(self.to_str().unwrap())
.unwrap().uid() == get_effective_uid() {
Ok(buf)
}
else {
Err(::std::io::Error::new(::std::io::ErrorKind::PermissionDenied, "Access denied."))
}
}
false => { Err(::std::io::Error::new(::std::io::ErrorKind::NotFound, "Path not found")) },
}
}
}
}

#[cfg(target_os = "windows")]
Expand Down
1 change: 1 addition & 0 deletions tests/i_am_permitted_by_file_ownership
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions tests/i_am_permitted_by_group_membership
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

32 changes: 31 additions & 1 deletion tests/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
extern crate diff;
extern crate is_executable;

use is_executable::is_executable;
use is_executable::{ is_executable, is_permitted };
Copy link
Contributor

Choose a reason for hiding this comment

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

This import won't work when compiling for not-Unix. Importing is_permitted must be done in mod unix


#[cfg(unix)]
mod unix {
Expand Down Expand Up @@ -71,6 +71,36 @@ mod unix {
fn not_executable_directory() {
assert!(!is_executable("."));
}

#[test]
fn permitted_by_membership() {
// `chmod 670 ./tests/i_am_permitted_by_group_membership`
let path: &str = "./tests/i_am_permitted_by_group_membership";
{
let check = is_permitted(path).ok().unwrap();
assert_eq!(
check,
::std::path::PathBuf::from(path),
"Testing whether the file's GID is in the set of the user's groups"
);
}
}

#[test]
fn permitted_by_ownership() {
// `chmod 700 ./tests/i_am_permitted_by_file_ownership`
let path: &str = "./tests/i_am_permitted_by_file_ownership";
{
let check = is_permitted(path).ok().unwrap();
assert_eq!(
check,
::std::path::PathBuf::from(path),
"Testing whether the file's UID bits exclusively allow owners to execute
this file, and that the UID of this user is happens to be the file's owner."
);
}
}

}

#[cfg(target_os = "windows")]
Expand Down