Skip to content

Commit

Permalink
feat: add configurable socket permissions
Browse files Browse the repository at this point in the history
Have `pueue` set the permissions of the socket created. Previously, the
permissions where unspecified and (at least for me) defaulted to `755`.

As part of this change, the default settings are moving to `700` which
is more restricted than the previous default.

Signed-off-by: JP-Ellis <josh@jpellis.me>
  • Loading branch information
JP-Ellis committed Jul 8, 2024
1 parent 57b0497 commit 7bfc04e
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ The new state design fixes this issue, which allows Pueue to do subprocess state
- Add `--all` and `--group` to `pueue log`. [#509](https://github.com/Nukesor/pueue/issues/509)
- Add `pueue reset --groups [group_names]` to allow resetting individual groups. [#482](https://github.com/Nukesor/pueue/issues/482) \
This also refactors the way resets are done internally, resulting in a cleaner code architecture.
- Ability to set the Unix socket permissions through the new `unix_socket_permissions` configuration option. [#544](https://github.com/Nukesor/pueue/pull/544)

## \[3.4.1\] - 2024-06-04

Expand Down
23 changes: 21 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pueue/tests/daemon/integration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod restart;
mod restore;
/// Tests for shutting down the daemon.
mod shutdown;
mod socket_permissions;
mod spawn;
mod start;
mod stashed;
Expand Down
56 changes: 56 additions & 0 deletions pueue/tests/daemon/integration/socket_permissions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
use anyhow::Result;
use std::fs;
use std::os::unix::fs::PermissionsExt;

use anyhow::Context;

use crate::helper::*;

/// Make sure that the socket permissions are appropriately set.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[cfg(not(target_os = "windows"))]
async fn test_socket_permissions_default() -> Result<()> {
let (settings, _tempdir) = daemon_base_setup()?;
let shared = &settings.shared;
let mut child = standalone_daemon(shared).await?;

assert_eq!(
fs::metadata(shared.unix_socket_path())?
.permissions()
.mode()
// The permissions are masked with 0o777 to only get the last 3
// digits.
& 0o777,
0o700
);

child.kill()?;
Ok(())
}

/// Make sure that the socket permissions can be changed
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[cfg(not(target_os = "windows"))]
async fn test_socket_permissions_modified() -> Result<()> {
let (mut settings, _tempdir) = daemon_base_setup()?;
settings.shared.unix_socket_permissions = Some(0o777);
let shared = &settings.shared;
settings
.save(&Some(settings.shared.runtime_directory().join("pueue.yml")))
.context("Couldn't write pueue config to temporary directory")?;

let mut child = standalone_daemon(shared).await?;

assert_eq!(
fs::metadata(shared.unix_socket_path())?
.permissions()
.mode()
// The permissions are masked with 0o777 to only get the last 3
// digits.
& 0o777,
0o777
);

child.kill()?;
Ok(())
}
1 change: 1 addition & 0 deletions pueue_lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ command-group = { workspace = true }
dirs = "5.0"
handlebars = { workspace = true }
log = { workspace = true }
nix = { version = "0.29.0", features = ["fs"] }
rand = "0.8"
rcgen = "0.13"
rev_buf_reader = "0.3"
Expand Down
33 changes: 30 additions & 3 deletions pueue_lib/src/network/socket/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use async_trait::async_trait;
use log::info;
use rustls::pki_types::ServerName;
use tokio::io::{AsyncRead, AsyncWrite};
use tokio::net::{TcpListener, TcpStream, UnixListener, UnixStream};
use tokio::net::{TcpListener, TcpStream, UnixListener, UnixSocket, UnixStream};
use tokio_rustls::TlsAcceptor;

use crate::error::Error;
Expand Down Expand Up @@ -142,8 +142,35 @@ pub async fn get_listener(settings: &Shared) -> Result<GenericListener, Error> {
})?;
}

let unix_listener = UnixListener::bind(&socket_path)
.map_err(|err| Error::IoPathError(socket_path, "creating unix socket", err))?;
// The various nix platforms handle socket permissions in different
// ways, but generally prevent the socket's permissions from being
// changed once it is being listened on.
let socket = UnixSocket::new_stream()
.map_err(|err| Error::IoError("creating unix socket".to_string(), err))?;
socket.bind(&socket_path).map_err(|err| {
Error::IoPathError(socket_path.clone(), "binding unix socket to path", err)

Check warning on line 151 in pueue_lib/src/network/socket/unix.rs

View check run for this annotation

Codecov / codecov/patch

pueue_lib/src/network/socket/unix.rs#L151

Added line #L151 was not covered by tests
})?;

if let Some(mode) = settings.unix_socket_permissions {
nix::sys::stat::fchmodat(
None,
&socket_path,
nix::sys::stat::Mode::from_bits_truncate(mode),
nix::sys::stat::FchmodatFlags::FollowSymlink,
)
.map_err(|err| {
Error::IoPathError(
socket_path.clone(),
"setting socket permissions",
std::io::Error::from_raw_os_error(err as i32),
)

Check warning on line 166 in pueue_lib/src/network/socket/unix.rs

View check run for this annotation

Codecov / codecov/patch

pueue_lib/src/network/socket/unix.rs#L162-L166

Added lines #L162 - L166 were not covered by tests
})?;
}

Check warning on line 168 in pueue_lib/src/network/socket/unix.rs

View check run for this annotation

Codecov / codecov/patch

pueue_lib/src/network/socket/unix.rs#L168

Added line #L168 was not covered by tests

let unix_listener = socket.listen(1024).map_err(|err| {
Error::IoPathError(socket_path.clone(), "listening on unix socket", err)

Check warning on line 171 in pueue_lib/src/network/socket/unix.rs

View check run for this annotation

Codecov / codecov/patch

pueue_lib/src/network/socket/unix.rs#L171

Added line #L171 was not covered by tests
})?;

return Ok(Box::new(unix_listener));
}

Expand Down
8 changes: 8 additions & 0 deletions pueue_lib/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ pub struct Shared {
/// The path to the unix socket.
#[cfg(not(target_os = "windows"))]
pub unix_socket_path: Option<PathBuf>,
/// Unix socket permissions. Typically specified as an octal number and
/// defaults to `0o700` which grants only the current user access to the
/// socket. For a client to connect to the daemon, the client must have
/// read/write permissions.
#[cfg(not(target_os = "windows"))]
pub unix_socket_permissions: Option<nix::sys::stat::mode_t>,

/// The TCP hostname/ip address.
#[serde(default = "default_host")]
Expand Down Expand Up @@ -147,6 +153,8 @@ impl Default for Shared {
unix_socket_path: None,
#[cfg(not(target_os = "windows"))]
use_unix_socket: true,
#[cfg(not(target_os = "windows"))]
unix_socket_permissions: Some(0o700),
host: default_host(),
port: default_port(),

Expand Down
2 changes: 2 additions & 0 deletions pueue_lib/tests/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ pub fn get_shared_settings(
use_unix_socket,
#[cfg(not(target_os = "windows"))]
unix_socket_path: None,
#[cfg(not(target_os = "windows"))]
unix_socket_permissions: Some(0o700),
pid_path: None,
host: "localhost".to_string(),
port: pick_unused_port()
Expand Down

0 comments on commit 7bfc04e

Please sign in to comment.