fix(#2391): drop "." entirely from credential/snapshot name charset
Per mara: "i would have even disallowed ., we are making up the rules here lets go strict". validate_credential_name now restricts to [A-Za-z0-9_-] (no dot at all) instead of [A-Za-z0-9_.-] + a separate ".." substring check — simpler rule, and there's no legitimate need for a dot in either a systemd credential id or a hive- prefixed snapshot label. Matching hivectl client-side check + wire-proto doc comments updated.
This commit is contained in:
parent
720ac81235
commit
2c079afd65
3 changed files with 25 additions and 22 deletions
|
|
@ -1740,10 +1740,11 @@ async fn subvol_upgrade(socket: &Path, name: &str, yes: bool) -> Result<()> {
|
|||
/// `subvol snapshot create <agent> --label <label>` — create a read-only
|
||||
/// btrfs snapshot of an agent's state subvolume. Unlike `upgrade`, this does
|
||||
/// NOT stop the agent: btrfs snapshots are atomic + consistent to take
|
||||
/// against a live subvolume. `label` is mandatory and must start with
|
||||
/// `hive-` — hive-priv enforces the same prefix as an allow-list, so this
|
||||
/// check is belt-and-suspenders (fail fast client-side with a clear
|
||||
/// message).
|
||||
/// against a live subvolume. `label` is mandatory, must start with
|
||||
/// `hive-`, and is otherwise restricted to `[A-Za-z0-9_-]` (no `.` at all
|
||||
/// — mara: "we are making up the rules here, lets go strict"). hive-priv
|
||||
/// enforces the same rules server-side, so this check is
|
||||
/// belt-and-suspenders (fail fast client-side with a clear message).
|
||||
async fn subvol_snapshot_create(name: &str, label: String) -> Result<()> {
|
||||
if !agent_exists(name)? {
|
||||
bail!("no agent named {name:?} (no state dir under the agents root)");
|
||||
|
|
@ -1751,8 +1752,13 @@ async fn subvol_snapshot_create(name: &str, label: String) -> Result<()> {
|
|||
if !label.starts_with("hive-") {
|
||||
bail!("snapshot label {label:?} must start with \"hive-\"");
|
||||
}
|
||||
if label.contains("..") {
|
||||
bail!("snapshot label {label:?} must not contain \"..\"");
|
||||
if !label
|
||||
.bytes()
|
||||
.all(|b| b.is_ascii_alphanumeric() || matches!(b, b'_' | b'-'))
|
||||
{
|
||||
bail!(
|
||||
"snapshot label {label:?} must be [A-Za-z0-9_-] only (no \".\" — hive-priv rejects it)"
|
||||
);
|
||||
}
|
||||
let path = hive_c0re::priv_client::snapshot_agent_subvolume(name, &label)
|
||||
.await
|
||||
|
|
|
|||
|
|
@ -445,25 +445,21 @@ fn validate_snapshot_name(name: &str) -> Result<()> {
|
|||
}
|
||||
|
||||
/// A systemd credential id must be a short token — restrict to
|
||||
/// `[A-Za-z0-9_.-]` so it can't inject extra `--load-credential` argv or
|
||||
/// break the `name:path` shape.
|
||||
/// `[A-Za-z0-9_-]` (no `.`) so it can't inject extra `--load-credential`
|
||||
/// argv or break the `name:path` shape. `.` is deliberately excluded, not
|
||||
/// just a bare `..`: this name gets interpolated into filesystem paths
|
||||
/// (snapshot labels) and there's no legitimate need for a dot in either a
|
||||
/// systemd credential id or a `hive-`-prefixed snapshot label — we're
|
||||
/// defining this token format from scratch, so keep it maximally strict
|
||||
/// rather than allow-then-patch each traversal-adjacent character
|
||||
/// (mara: "we are making up the rules here, lets go strict").
|
||||
fn validate_credential_name(name: &str) -> Result<()> {
|
||||
if name.is_empty()
|
||||
|| !name
|
||||
.bytes()
|
||||
.all(|b| b.is_ascii_alphanumeric() || matches!(b, b'_' | b'.' | b'-'))
|
||||
.all(|b| b.is_ascii_alphanumeric() || matches!(b, b'_' | b'-'))
|
||||
{
|
||||
bail!("invalid credential name {name:?}: must be non-empty [A-Za-z0-9_.-]");
|
||||
}
|
||||
// `.` is in the allowed charset (systemd credential ids and snapshot
|
||||
// labels both legitimately use dots), but a bare `..` reads as a
|
||||
// directory-traversal token to anyone auditing this path — and a
|
||||
// caller that builds a path via `PathBuf::from(name)` instead of the
|
||||
// current `format!("...{name}...")` embedding would actually be
|
||||
// exploitable. Reject it outright rather than relying on every future
|
||||
// caller getting the embedding right.
|
||||
if name.contains("..") {
|
||||
bail!("invalid credential name {name:?}: must not contain \"..\"");
|
||||
bail!("invalid credential name {name:?}: must be non-empty [A-Za-z0-9_-]");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -199,7 +199,7 @@ pub struct BindMount {
|
|||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct CredentialMount {
|
||||
/// systemd credential id (e.g. `otel-headers`); inner units inherit
|
||||
/// it by this name. Restricted to `[A-Za-z0-9_.-]` by hive-priv.
|
||||
/// it by this name. Restricted to `[A-Za-z0-9_-]` (no `.`) by hive-priv.
|
||||
pub name: String,
|
||||
/// Host path to the secret file, forwarded via nspawn
|
||||
/// `--load-credential=<name>:<host_path>`.
|
||||
|
|
@ -599,7 +599,8 @@ pub enum PrivRequest {
|
|||
/// an allow-list hive-priv enforces so only hivectl-issued names
|
||||
/// can reach the `btrfs subvolume snapshot` shellout — and
|
||||
/// otherwise follows the same charset as a credential name
|
||||
/// (non-empty `[A-Za-z0-9_.-]`); becomes part of the snapshot path.
|
||||
/// (non-empty `[A-Za-z0-9_-]`, no `.`); becomes part of the
|
||||
/// snapshot path.
|
||||
snapshot_name: String,
|
||||
},
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue