Skip to content

Commit

Permalink
Fixed credential passing and signal soundness
Browse files Browse the repository at this point in the history
  • Loading branch information
kotauskas committed Aug 20, 2020
1 parent 6c0a578 commit 475bf25
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 25 deletions.
7 changes: 4 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "interprocess"
version = "0.1.1"
version = "0.2.0"
authors = ["Kotauskas <[email protected]>"]
edition = "2018"
license = "MIT"
Expand All @@ -15,7 +15,7 @@ keywords = ["ipc", "shared_memory", "pipe", "mailslot", "unix_domain_socket"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
tokio = {version = "0.2", optional = true}
#tokio = {version = "0.2", optional = true}

[target.'cfg(windows)'.dependencies]
winapi = {version = "0.3.9", features = ["std", "winbase", "winerror", "processthreadsapi", "fileapi", "handleapi", "namedpipeapi"]}
Expand All @@ -25,7 +25,8 @@ spin = "0.5"
cfg-if = "0.1"
intmap = "0.7"
lazy_static = "1.4"
thiserror = "1.0"

[features]
default = []
tokio_integration = ["tokio"]
#tokio_integration = ["tokio"]
5 changes: 3 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
//!
//! [MIT license]: https://choosealicense.com/licenses/mit/ " "
// If the trait itself was pub(crate), it wouldn't work as a supertrait on public traits. We use a
// private module instead to make it impossible to name the trait from outside the crate.
#![allow(unused_unsafe)]

// If an operating system is not listed here, the `compile_error!` is invoked
#[cfg(not(any(
Expand Down Expand Up @@ -124,6 +123,8 @@ pub(crate) mod private {
}
};
}
// If the trait itself was pub(crate), it wouldn't work as a supertrait on public traits. We use a
// private module instead to make it impossible to name the trait from outside the crate.
pub trait Sealed {}
}

Expand Down
122 changes: 117 additions & 5 deletions src/os/unix/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ use std::{
};
use spin::RwLock;
use intmap::IntMap;
use thiserror::Error;
use cfg_if::cfg_if;
use lazy_static::lazy_static;
cfg_if! {
Expand Down Expand Up @@ -241,11 +242,48 @@ lazy_static! {
///
/// [`HandlerOptions`]: struct.HandlerOptions.html " "
#[inline]
pub fn set_handler(signal_type: SignalType, handler: SignalHandler) {
pub fn set_handler(signal_type: SignalType, handler: SignalHandler) -> Result<(), SetHandlerError> {
HandlerOptions::for_signal(signal_type)
.set_new_handler(handler)
.set()
}
/// Installs the specified handler for the specified unsafe signal, using the default values for the flags.
///
/// See [`HandlerOptions`] builder if you'd like to customize the flags.
///
/// # Safety
/// See the [`set_unsafe`] safety notes.
///
/// # Example
/// ```no_run
/// use interprocess::os::unix::signal::{self, SignalType, SignalHandler};
///
/// let handler = unsafe {
/// // Since signal handlers are restricted to a specific set of system calls, creating a
/// // handler from an arbitrary function is unsafe because it might perform a system call
/// // outside the list, and there's no real way to know that at compile time with the
/// // current version of Rust. Since we're only using the write() system call here, this
/// // is safe.
/// SignalHandler::from_fn(|| {
/// println!("Oh no, the motherboard broke!");
/// std::process::abort();
/// })
/// };
///
/// unsafe {
/// // Install our handler for the MemoryBusError signal type.
/// signal::set_unsafe_handler(SignalType::MemoryBusError, handler);
/// }
/// ```
///
/// [`HandlerOptions`]: struct.HandlerOptions.html " "
/// [`set_unsafe`]: struct.HandlerOptions.html#method.set_unsafe " "
#[inline]
pub unsafe fn set_unsafe_handler(signal_type: SignalType, handler: SignalHandler) -> Result<(), SetHandlerError> {
HandlerOptions::for_signal(signal_type)
.set_new_handler(handler)
.set_unsafe()
}
/// Installs the specified handler for the specified real-time signal, using the default values for the flags.
///
/// See [`HandlerOptions`] builder if you'd like to customize the flags.
Expand All @@ -271,7 +309,7 @@ pub fn set_handler(signal_type: SignalType, handler: SignalHandler) {
///
/// [`HandlerOptions`]: struct.HandlerOptions.html " "
#[inline]
pub fn set_rthandler(rtsignal: u32, handler: SignalHandler) {
pub fn set_rthandler(rtsignal: u32, handler: SignalHandler) -> Result<(), SetHandlerError> {
HandlerOptions::for_rtsignal(rtsignal)
.set_new_handler(handler)
.set()
Expand Down Expand Up @@ -429,7 +467,33 @@ impl HandlerOptions {
self
}
/// Installs the signal handler.
pub fn set(self) {
#[inline]
pub fn set(self) -> Result<(), SetHandlerError> {
if let Ok(val) = SignalType::try_from(self.signal) {
if val.is_unsafe() {
return Err(SetHandlerError::UnsafeSignal);
}
}
unsafe {self.set_unsafe()}
}

/// Installs the signal handler, even if the signal being handled is unsafe.
///
/// # Safety
/// The handler and all code that may or may not execute afterwards must be prepared for the aftermath of what might've caused the signal. [`SegmentationFault`], for example, might be caused by hitting a stack protector as a response to a thread's stack overflow — in such a case, continuing the program might result in undefined behavior. The Rust runtime actually sets its own handler for `SegmentationFault` signals which converts those signals to proper shutdowns, i.e. ignoring it essentially disables stack protectors, which is unsound.
///
/// [`SegmentationFault`]: enum.SignalType.html#variant.SegmentationFault " "
pub unsafe fn set_unsafe(self) -> Result<(), SetHandlerError> {
if let Ok(val) = SignalType::try_from(self.signal) {
if val.is_unblockable() {
return Err(SetHandlerError::UnblockableSignal(val));
}
} else if !is_valid_rtsignal(self.signal as u32) {
return Err(SetHandlerError::RealTimeSignalOutOfBounds {
attempted: self.signal as u32,
max: NUM_REALTIME_SIGNALS,
});
}
let handlers = HANDLERS.upgradeable_read();
let new_flags = self.flags_as_i32();
let mut need_to_upgrade_handle = false;
Expand Down Expand Up @@ -470,8 +534,8 @@ impl HandlerOptions {
self.signal,
hook_val,
new_flags,
)
}.expect("unexpected signal handler installation failure")
)?
}
}
if need_to_upgrade_handle {
let mut handlers = handlers.upgrade();
Expand All @@ -482,6 +546,7 @@ impl HandlerOptions {
new_flags,
));
}
Ok(())
}

#[inline]
Expand Down Expand Up @@ -516,6 +581,28 @@ impl HandlerOptions {
}
}

#[derive(Debug, Error)]
pub enum SetHandlerError {
/// An unsafe signal was attempted to be handled using `set` instead of `unsafe_set`.
#[error("an unsafe signal was attempted to be handled using `set` instead of `unsafe_set`")]
UnsafeSignal,
/// The signal which was attempted to be handled is not allowed to be handled by the POSIX specification. This can either be [`ForceSuspend`] or [`Kill`].
///
/// [`Kill`]: enum.SignalType.html#variant.Kill " "
/// [`ForceSuspend`]: enum.SignalType.html#variant.ForceSuspend " "
#[error("the signal {:?} cannot be handled", .0)]
UnblockableSignal (SignalType),
/// The specified real-time signal is not available on this OS.
#[error("the real-time signal number {} is not available ({} is the highest possible)", .attempted, .max)]
RealTimeSignalOutOfBounds {
attempted: u32,
max: u32,
},
/// An unexpected OS error ocurred during signal handler setup.
#[error("{}", .0)]
UnexpectedSystemCallFailure (#[from] io::Error),
}

/// The actual hook which is passed to `sigaction` which dispatches signals according to the global handler map (the `HANDLERS` static).
extern fn signal_receiver(signum: i32) {
let catched = panic::catch_unwind(|| {
Expand Down Expand Up @@ -797,6 +884,7 @@ pub fn send_rt_to_group(signal: impl Into<Option<u32>>, pid: impl Into<u32>) ->
///
/// [`i32`]: https://doc.rust-lang.org/std/primitive.i32.html " "
/// [`u32`]: https://doc.rust-lang.org/std/primitive.u32.html " "
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[repr(i32)]
#[non_exhaustive]
pub enum SignalType {
Expand Down Expand Up @@ -965,6 +1053,30 @@ pub enum SignalType {
/// [`setrlimit`]: https://www.man7.org/linux/man-pages/man2/setrlimit.2.html " "
FileSizeLimitExceeded = SIGXFSZ,
}
impl SignalType {
/// Returns `true` if the value is a special signal which cannot be blocked or handled ([`Kill`] or [`ForceSuspend`]), `false` otherwise.
///
/// [`Kill`]: #variant.Kill " "
/// [`ForceSuspend`]: #variant.ForceSuspend " "
#[inline]
pub fn is_unblockable(self) -> bool {
match self {
Self::Kill
| Self::ForceSuspend
=> true,
_ => false,
}
}
/// Returns `true` if the value is an unsafe signal which requires unsafe code when setting a handling method, `false` otherwise.
pub fn is_unsafe(self) -> bool {
match self {
Self::SegmentationFault
| Self::MemoryBusError
=> true,
_ => false,
}
}
}
impl From<SignalType> for i32 {
fn from(op: SignalType) -> Self {
unsafe {
Expand Down
69 changes: 54 additions & 15 deletions src/os/unix/udsocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use libc::{
pid_t, uid_t, gid_t,
AF_UNIX,
SOCK_STREAM, SOCK_DGRAM,
SOL_SOCKET,
SOL_SOCKET, SO_PASSCRED,
SCM_RIGHTS, SCM_CREDENTIALS,
MSG_TRUNC, MSG_CTRUNC, MSG_PEEK,
sockaddr_un,
Expand Down Expand Up @@ -268,12 +268,20 @@ impl UdStreamListener {
// but you can cast any narrow pointer to any other narrow pointer
&addr as *const _ as *const _,
addrlen as u32,
) != -1 {
// FIXME the standard library uses 128 here without an option to change this
// number, why? If std has solid reasons to do this, remove this notice and
// document the method's behavior on this matter explicitly; otherwise, add
// an option to change this value.
libc::listen(socket, 128) != 1
) != -1
// FIXME the standard library uses 128 here without an option to change this
// number, why? If std has solid reasons to do this, remove this notice and
// document the method's behavior on this matter explicitly; otherwise, add
// an option to change this value.
&& libc::listen(socket, 128) != -1 {
let passcred: c_int = 1;
libc::setsockopt(
socket,
SOL_SOCKET,
SO_PASSCRED,
&passcred as *const _ as *const _,
mem::size_of_val(&passcred) as u32,
) != -1
} else {
false
}
Expand Down Expand Up @@ -513,12 +521,23 @@ impl UdStream {
}
};
let success = unsafe {
libc::connect(
if libc::connect(
socket,
// Same as in UdSocketListener::bind()
&addr as *const _ as *const _,
addrlen as u32,
) != -1
) != -1 {
let passcred: c_int = 1;
libc::setsockopt(
socket,
SOL_SOCKET,
SO_PASSCRED,
&passcred as *const _ as *const _,
mem::size_of_val(&passcred) as u32,
) != -1
} else {
false
}
};
if success {
Ok(unsafe {
Expand Down Expand Up @@ -783,15 +802,24 @@ impl UdSocket {
}
};
let success = unsafe {
// If binding didn't fail, start listening and return true if it succeeded and false if
// it failed; if binding failed, short-circuit to returning false
libc::bind(
if libc::bind(
socket,
// Double cast because you cannot cast a reference to a pointer of arbitrary type
// but you can cast any narrow pointer to any other narrow pointer
&addr as *const _ as *const _,
addrlen as u32,
) != -1
) != -1 {
let passcred: c_int = 1;
libc::setsockopt(
socket,
SOL_SOCKET,
SO_PASSCRED,
&passcred as *const _ as *const _,
mem::size_of_val(&passcred) as u32,
) != -1
} else {
false
}
};
if success {
Ok(
Expand Down Expand Up @@ -841,12 +869,23 @@ impl UdSocket {
}
};
let success = unsafe {
libc::connect(
if libc::connect(
socket,
// Same as in UdSocketListener::bind()
&addr as *const _ as *const _,
addrlen as u32,
) != -1
) != -1 {
let passcred: c_int = 1;
libc::setsockopt(
socket,
SOL_SOCKET,
SO_PASSCRED,
&passcred as *const _ as *const _,
mem::size_of_val(&passcred) as u32,
) != -1
} else {
false
}
};
if success {
Ok(unsafe {
Expand Down

0 comments on commit 475bf25

Please sign in to comment.