diff options
| author | ojacobson <ojacobson@noreply.codeberg.org> | 2025-07-23 00:05:17 +0200 |
|---|---|---|
| committer | ojacobson <ojacobson@noreply.codeberg.org> | 2025-07-23 00:05:17 +0200 |
| commit | 64639acbab02aa4103cbe44199e38991269b2137 (patch) | |
| tree | 7996fdc7d2034c9da85b4d7d2ad5ebbbdc6a2804 /src/umask.rs | |
| parent | 0867790d87ebbbedb6b20b52139055e109031033 (diff) | |
| parent | dc240ca270f86552e999c81d864b4cb0c687a88e (diff) | |
Add a `--umask` option to determine what permissions new files/databases get.
The new `--umask` option takes one of three values:
* `--umask masked`, the default, takes the inherited umask and forces o+rwx on.
* `--umask inherit` takes the inherited umask as-is.
* `--umask OCTAL` sets the umask to exactly `OCTAL` and is broadly equivalent to `umask OCTAL && pilcrow --umask inherit`.
This fell out of a conversation with @wlonk, who is working on notifications. Since notifications may require [VAPID] keys, the server will need a way to store those keys. That would generally be "in the pilcrow database," which lead me to the observation that Pilcrow creates that database as world-readable by default. "World-readable" and "encryption/signing keys" are not things that belong in the same sentence.
[VAPID]: https://datatracker.ietf.org/doc/html/rfc8292
The most "obvious" solution would be to set the permissions used for the sqlite database when it's created. That's harder than it sounds: sqlite has no built-in facility for doing this. The closest thing that exists today is the [`modeof`] query parameter, which copies the permissions (and ownership) from some other file. We also can't reliably set the permissions ourselves, as sqlite may - depending on build options and configuration - [create multiple files][wal].
[`modeof`]: https://www.sqlite.org/uri.html
[wal]: https://www.sqlite.org/wal.html
Using `umask` is a whole-process solution to this. As Pilcrow doesn't attempt to create other files, there's little issue with doing it this way, but this is a design risk for future work if it creates files that are _intended_ to be readable by more than just the Pilcrow daemon user.
Merges options-umask into main.
Diffstat (limited to 'src/umask.rs')
| -rw-r--r-- | src/umask.rs | 116 |
1 files changed, 116 insertions, 0 deletions
diff --git a/src/umask.rs b/src/umask.rs new file mode 100644 index 0000000..32a82ad --- /dev/null +++ b/src/umask.rs @@ -0,0 +1,116 @@ +use std::{fmt, str}; + +use nix::{ + libc::mode_t, + sys::stat::{self, Mode}, +}; + +#[derive(Clone, Copy)] +pub enum Umask { + Masked, + Inherit, + Set(Mode), +} + +impl Umask { + pub fn set(self) { + match self { + Self::Masked => Self::masked(), + Self::Inherit => Self::inherit(), + Self::Set(mode) => Self::set_to(mode), + } + } + + fn masked() { + // In the year 2025, this is _still_ the only reasonable way to check the + // process' current umask. This is also why we don't calculate a default + // umask and stick it in Clap's `default_value_t` - I'd prefer not to touch + // the process' umask before we know what we're doing with it, but there's + // no other way to look at the current one. + // + // The choice of `all` here is a complicated compromise. In theory, nothing + // will ever observe this umask, as we immediately overwrite it immediately + // below. As of this writing, there is nothing in this service that could + // create a file in this interval, which could then be affected by the + // temporary umask. However, future code changes _could_ introduce something + // that races with this, such as a signal handler that does more than just + // exiting the process. + // + // Using `all` makes sure that _any_ file created in that interval is + // created with "deny all" file permission bits, which will make it useless + // even for this program. Hopefully, that failure will be immediate enough + // to attract attention. The other alternatives which might work are zero + // (don't mask off any permissions, things might be globally readable) + // or 0o0027 (the permissions we'd want to use on most Linux distros, which + // could then silently hide the above race condition). + let current = stat::umask(Mode::all()); + + // In addition to whatever we inherit, by default, we ensure that files + // created by pilcrow are not world-readable or world-writeable. However, + // we respect the inherited group and user permissions on the assumption + // that the user, or their administrator, has either made a decision about + // group permissions, or are relying on the distro's defaults. + // + // The main thing `pilcrow` creates are its database and its backup + // database, which can contain both confidential information (users' + // conversations) and sensitive information (the service's configuration + // and any API tokens, keys, &c used by the service). + // + // There is no way to tell `sqlite` to restrict the permissions of database + // files directly, which is otherwise what we'd prefer. + let desired = current | Mode::S_IRWXO; + + stat::umask(desired); + } + + fn inherit() { + // Here's a complete list of steps required to inherit the umask from the calling process: + } + + fn set_to(mode: Mode) { + stat::umask(mode); + } +} + +impl str::FromStr for Umask { + type Err = Error; + + fn from_str(s: &str) -> Result<Self, Self::Err> { + let umask = match s { + "masked" => Self::Masked, + "inherit" => Self::Inherit, + octal => { + let mode = mode_t::from_str_radix(octal, 8)?; + let mode = Mode::from_bits(mode).ok_or(Error::UnknownBits)?; + Self::Set(mode) + } + }; + + Ok(umask) + } +} + +impl fmt::Display for Umask { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Masked => "masked".fmt(f), + Self::Inherit => "inherit".fmt(f), + Self::Set(mode) => write!(f, "{mode:o}"), + } + } +} + +/// Errors occurring during umask option parsing. +#[derive(Debug, thiserror::Error)] +pub enum Error { + /// Failed to parse a umask value from the input. + #[error(transparent)] + Parse(#[from] std::num::ParseIntError), + + /// The provided umask contained invalid bits. (See the constants associated with [`Mode`] for + /// valid umask bits.) + // We dont need to hold onto the actual umask value here - Clap does that for us, and prints + // the value as the user input it, which beats anything we could do here. + #[error("unknown bits in umask")] + UnknownBits, +} |
