Skip to content

filepersist: brush up #154

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.

Sign up for GitHub

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

Open
johanot wants to merge 2 commits into
base: main
Choose a base branch
from
Open

filepersist: brush up #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c9fe88
filepersist: allow being explicit about ownership and perms for publi…
johanot Aug 1, 2025
059655d
Merge branch 'main' into file-impl-improvements
srhb Dec 2, 2025
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
48 changes: 46 additions & 2 deletions nixos/file-test.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ testLib.mkFaytheTest ({ nodes, ... }: {
systemd.services.faythe.preStart = ''
mkdir -p ${cert_path}
'';

users.users.certuser = {
isNormalUser = true;
};

users.groups.certgroup = {};
})
];
faytheExtraConfig = {
Expand All @@ -29,6 +35,20 @@ testLib.mkFaytheTest ({ nodes, ... }: {
cn = "path1.${domain}";
key_file_name = "key.pem";
}
{
name = "path2-test";
cn = "path2.${domain}";
key_file_name = "key.pem";
cert_file_perms = {
user = "certuser";
group = "certgroup";
mode = "644";
};
key_file_perms = {
user = "certuser";
mode = "600";
};
}
];
}
];
Expand All @@ -39,11 +59,35 @@ testLib.mkFaytheTest ({ nodes, ... }: {

client.wait_until_succeeds("""
journalctl -u faythe | grep "path1-test" | grep -q "touched"
journalctl -u faythe | grep -q "changing group for"
openssl x509 -in ${cert_path}/path1-test/fullchain.pem -text -noout | grep -q "Issuer: CN=Pebble Intermediate"
""")

client.succeed("""
openssl x509 -in ${cert_path}/path1-test/fullchain.pem -text -noout | grep -q "Issuer: CN=Pebble Intermediate"
test "$(stat -c %a ${cert_path}/path1-test/fullchain.pem)" == "644"
test "$(stat -c %a ${cert_path}/path1-test/key.pem)" == "640"
""")

with subtest("First time issue with custom permissions and user"):
client.wait_until_succeeds("stat ${cert_path}/path2-test")

client.wait_until_succeeds("""
journalctl -u faythe | grep "path2-test" | grep -q "touched"
openssl x509 -in ${cert_path}/path2-test/fullchain.pem -text -noout | grep -q "Issuer: CN=Pebble Intermediate"
""")

client.succeed("""
test "$(stat -c %U ${cert_path}/path2-test/fullchain.pem)" == "certuser"
test "$(stat -c %U ${cert_path}/path2-test/key.pem)" == "certuser"
""")

client.succeed("""
test "$(stat -c %G ${cert_path}/path2-test/fullchain.pem)" == "certgroup"
test "$(stat -c %G ${cert_path}/path2-test/key.pem)" == "root"
""")

client.succeed("""
test "$(stat -c %a ${cert_path}/path2-test/fullchain.pem)" == "644"
test "$(stat -c %a ${cert_path}/path2-test/key.pem)" == "600"
""")
'';
})
37 changes: 35 additions & 2 deletions src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use std::fmt::Formatter;
use std::path::PathBuf;
use chrono::{TimeZone, Utc};
pub type CertName = String;
use serde::{Deserialize, Deserializer};

#[derive(Debug, Clone, Serialize)]
pub struct CertSpec {
Expand Down Expand Up @@ -158,11 +159,41 @@ pub trait Persistable {
async fn persist(&self, cert: Certificate) -> Result<(), PersistError>;
}

#[derive(Debug, Clone, Serialize)]
#[derive(Debug, Default, Clone, Serialize)]
pub struct FilePersistSpec {
pub private_key_path: PathBuf,
pub public_key_path: PathBuf
pub public_key_path: PathBuf,
pub cert_file_perms: Option<FilePermissions>,
pub key_file_perms: Option<FilePermissions>,
}

#[derive(Clone, Deserialize, Serialize, Debug)]
pub struct FilePermissions {
pub user: Option<String>,
pub group: Option<String>,
#[serde(deserialize_with = "deserialize_file_mode")]
pub mode: Option<FileMode>,
}

#[derive(Clone, Deserialize, Serialize, Debug)]
pub struct FileMode(pub u32);


fn deserialize_file_mode<'de, D>(deserializer: D) -> Result<Option<FileMode>, D::Error>
where
D: Deserializer<'de>,
{
let opt = <Option<String>>::deserialize(deserializer)?;
match opt {
Some(octal_str) => {
u32::from_str_radix(&octal_str, 8)
.map_err(serde::de::Error::custom)
.map(|mode| Some(FileMode(mode)))
}
None => Ok(None),
}
}

// We don't care about the mem use difference here because we're only using VaultPersistSpec in
// prod, which is the largest variant anyway, so we're always paying the full cost anyway (and
// saving ~240B per spec doesn't matter in the order of 100s of specs.)
Expand Down Expand Up @@ -459,6 +490,8 @@ pub mod tests {
sub_directory: None,
cert_file_name: None,
key_file_name: None,
cert_file_perms: None,
key_file_perms: None,
}
}

Expand Down
66 changes: 50 additions & 16 deletions src/file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ extern crate walkdir;

use crate::config::{FileMonitorConfig, FaytheConfig, ConfigContainer};
use std::collections::{HashMap, HashSet};
use crate::common::{ValidityVerifier, CertSpecable, CertSpec, SpecError, PersistSpec, TouchError, IssueSource, FilePersistSpec, Cert, PersistError, CertName};
use crate::common::{ValidityVerifier, CertSpecable, CertSpec, FileMode, FilePermissions, SpecError, PersistSpec, TouchError, IssueSource, FilePersistSpec, Cert, PersistError, CertName};
use std::fs::{File, OpenOptions};
use std::path::{Path, PathBuf};
use acme_lib::Certificate;
Expand All @@ -13,8 +13,10 @@ use std::time::SystemTime;
use std::os::unix::fs::PermissionsExt;
use crate::log;
use std::fs;
use std::fs::Permissions;
use self::walkdir::WalkDir;
use std::process::Command;
use std::process::Stdio;

pub fn read_certs(config: &FileMonitorConfig) -> Result<HashMap<CertName, FileCert>, FileError> {
let mut certs = HashMap::new();
Expand Down Expand Up @@ -121,6 +123,10 @@ pub struct FileSpec {
pub cert_file_name: Option<String>,
#[serde(default)]
pub key_file_name: Option<String>,
#[serde(default)]
pub cert_file_perms: Option<FilePermissions>,
#[serde(default)]
pub key_file_perms: Option<FilePermissions>,
}

impl IssueSource for FileSpec {
Expand All @@ -144,6 +150,8 @@ impl CertSpecable for FileSpec {
persist_spec: PersistSpec::File(FilePersistSpec{
private_key_path: absolute_file_path(monitor_config, &names, &names.key),
public_key_path: absolute_file_path(monitor_config, &names, &names.cert),
cert_file_perms: self.cert_file_perms.clone(),
key_file_perms: self.key_file_perms.clone(),
}),
})
}
Expand All @@ -154,7 +162,7 @@ impl CertSpecable for FileSpec {
let sub_dir = absolute_dir_path(monitor_config, names.sub_directory.as_ref());
if names.sub_directory.is_some() && !sub_dir.exists() {
fs::create_dir(&sub_dir)?;
sub_dir.metadata()?.permissions().set_mode(0o655) // rw-r-xr-x
sub_dir.metadata()?.permissions().set_mode(0o755) // rwxr-xr-x
}
let file_path = absolute_file_path(monitor_config, &names, &names.meta);
let mut _file = OpenOptions::new().truncate(true).write(true).create(true).open(file_path)?;
Expand Down Expand Up @@ -216,28 +224,54 @@ pub fn persist(spec: &FilePersistSpec, cert: &Certificate) -> Result<(), Persist
let mut priv_file = File::create(&spec.private_key_path)?;
let pub_buf = cert.certificate().as_bytes();
let priv_buf = cert.private_key().as_bytes();

// Ownership and permissions
let pub_key_permissions = Permissions::from_mode(spec.cert_file_perms.as_ref().map_or(FileMode(0o644), |p| p.mode.as_ref().map(|m| m.clone()).unwrap_or(FileMode(0o644))).0); // default: rw-r--r--
let priv_key_permissions = Permissions::from_mode(spec.key_file_perms.as_ref().map_or(FileMode(0o640), |p| p.mode.as_ref().map(|m| m.clone()).unwrap_or(FileMode(0o640))).0); // default: rw-r-----
pub_file.set_permissions(pub_key_permissions)?;
priv_file.set_permissions(priv_key_permissions)?;
if let Some(perms) = &spec.cert_file_perms {
if perms.user.is_some() || perms.group.is_some() {
chown(perms.user.as_deref(), perms.group.as_deref(), &spec.public_key_path)?;
}
}
if let Some(perms) = &spec.key_file_perms {
if perms.user.is_some() || perms.group.is_some() {
chown(perms.user.as_deref(), perms.group.as_deref(), &spec.private_key_path)?;
}
}

// Flush contents to files
pub_file.write_all(pub_buf)?;
priv_file.write_all(priv_buf)?;
let mut priv_permissions = priv_file.metadata()?.permissions();
priv_permissions.set_mode(0o640); // rw-r------
match spec.public_key_path.parent() {
Some(d) => chgrp("certpull", d), //TODO: don't hardcode group
None => Err(PersistError::File(FileError::IO))
}?;
Ok(())
}

fn chgrp(group: &str, path: &Path) -> Result<(), PersistError> {
log::data("changing group for", &path.as_os_str());
fn chown(user: Option<&str>, group: Option<&str>, path: &Path) -> Result<(), PersistError> {
let mut ownership_arg = user.unwrap_or("").to_string();
if let Some(g) = group {
ownership_arg.push(':');
ownership_arg.push_str(g);
}
log::info(&format!("changing ownership to: {} for path: {}", &ownership_arg, &path.display()));

let mut cmd = Command::new("chgrp");
match cmd.arg("-R")
.arg(group)
.arg(path.as_os_str())
let mut cmd = Command::new("chown");
match cmd
.arg(ownership_arg)
.arg(path)
.stderr(Stdio::piped())
.output() {

Ok(_) => Ok(()),
Err(e) => { log::error("chgroup failed", &e); Err(PersistError::File(FileError::IO)) }
Ok(output) => {
match output.status.success() {
true => Ok(()),
false => {
log::error("chown failed", &output.stderr);
Err(PersistError::File(FileError::IO))
}
}
},
Err(e) => { log::error("chown failed", &e); Err(PersistError::File(FileError::IO)) }
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/monitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ mod tests {
sub_directory: None,
cert_file_name: None,
key_file_name: None,
cert_file_perms: None,
key_file_perms: None,
}]
.to_vec()
}
Expand Down
Loading