fix: restrict WriteAgentStateFile to explicit filename allowlist
Addresses mara's security review: replace validate_state_filename (which accepted any non-traversal filename) with a tight allowlist containing only the two known credential filenames: forge-token and matrix-token. Also addresses argus review feedback: - drop issue tag from priv_proto.rs doc comment - add comment explaining the path-detection heuristic in forge.rs - add note about create_dir_all uid=0 edge case in write_agent_state_file
This commit is contained in:
parent
eb51362d50
commit
89092caba4
3 changed files with 29 additions and 16 deletions
|
|
@ -302,6 +302,12 @@ async fn mint_and_persist_token(name: &str, path: &Path, scopes: &str) -> Result
|
|||
let token = mint_token(name, scopes).await?;
|
||||
|
||||
// Agent state paths must go through hive-priv.
|
||||
// Heuristic: any path containing an "agents" component is considered
|
||||
// an agent state path (matches `/var/lib/hyperhive/agents/<name>/...`).
|
||||
// All current callers pass either CORE_TOKEN_PATH (no "agents" component)
|
||||
// or `token_path(name)` (under AGENT_STATE_ROOT which contains "agents").
|
||||
// If a future non-agent path ever gains an "agents" component, this guard
|
||||
// would incorrectly route it to priv — add an explicit exclusion then.
|
||||
if path.components().any(|c| c.as_os_str() == "agents") {
|
||||
let filename = path
|
||||
.file_name()
|
||||
|
|
|
|||
|
|
@ -334,20 +334,19 @@ async fn exec(req: PrivRequest, writer: &mut OwnedWriteHalf) -> Result<(String,
|
|||
}
|
||||
}
|
||||
|
||||
/// Validate a filename destined for `WriteAgentStateFile`. Rejects
|
||||
/// path separators, null bytes, newlines, and `..` to prevent path traversal.
|
||||
/// Explicit allowlist of filenames `WriteAgentStateFile` may write.
|
||||
/// New credential files must be added here deliberately — hive-priv
|
||||
/// rejects any filename not in this list.
|
||||
const ALLOWED_STATE_FILENAMES: &[&str] = &["forge-token", "matrix-token"];
|
||||
|
||||
/// Validate a filename destined for `WriteAgentStateFile` against the
|
||||
/// explicit allowlist. Only known credential filenames are accepted.
|
||||
fn validate_state_filename(filename: &str) -> Result<()> {
|
||||
if filename.is_empty() {
|
||||
bail!("state filename must not be empty");
|
||||
}
|
||||
if filename == ".." || filename == "." {
|
||||
bail!("state filename must not be . or ..");
|
||||
}
|
||||
if filename
|
||||
.bytes()
|
||||
.any(|b| b == 0 || b == b'\n' || b == b'\r' || b == b'/')
|
||||
{
|
||||
bail!("state filename {filename:?} contains null byte, newline, or path separator");
|
||||
if !ALLOWED_STATE_FILENAMES.contains(&filename) {
|
||||
bail!(
|
||||
"state filename {filename:?} not in allowlist {:?}",
|
||||
ALLOWED_STATE_FILENAMES
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -366,6 +365,13 @@ fn write_agent_state_file(
|
|||
let state_dir = PathBuf::from(AGENT_STATE_ROOT)
|
||||
.join(agent_name)
|
||||
.join("state");
|
||||
// NOTE: `create_dir_all` is normally a no-op — lifecycle creates and chowns
|
||||
// the state dir during spawn. On the rare edge where the dir doesn't exist
|
||||
// yet (container being provisioned for the first time), the newly created dir
|
||||
// is root:root. The `stat state_dir` chown below will then see uid=0 and
|
||||
// leave the file root-owned (0600). The agent won't be able to read it until
|
||||
// its lifecycle completes. If that happens, a `systemctl restart hive-c0re`
|
||||
// after provisioning will re-mint and re-write the token correctly.
|
||||
std::fs::create_dir_all(&state_dir)
|
||||
.with_context(|| format!("create state dir {}", state_dir.display()))?;
|
||||
|
||||
|
|
|
|||
|
|
@ -241,12 +241,13 @@ pub enum PrivRequest {
|
|||
///
|
||||
/// Required because hive-c0re runs as the unprivileged `hive-core`
|
||||
/// user and cannot write to agent-owned (0755) state directories
|
||||
/// after the privsep introduced in #702.
|
||||
/// after the privsep that moved c0re from root to a dedicated unix user.
|
||||
WriteAgentStateFile {
|
||||
/// Logical agent name (validated by `validate_agent_name`).
|
||||
agent_name: String,
|
||||
/// Plain filename within the state dir — no path separators allowed.
|
||||
/// Example: `"forge-token"`, `"matrix-token"`.
|
||||
/// Allowlisted credential filename within the state dir.
|
||||
/// Only `"forge-token"` and `"matrix-token"` are accepted;
|
||||
/// hive-priv rejects any other value.
|
||||
filename: String,
|
||||
/// File content to write. Written as-is; caller is responsible for
|
||||
/// including any trailing newline.
|
||||
|
|
|
|||
Loading…
Reference in a new issue