Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ fi
echo "usage: install.sh --server URL (--token TOKEN | --register KEY) [--interval SECONDS] [--iface LIST] [--insecure]" >&2
echo " install.sh --upgrade [--iface LIST] [--interval SECONDS]" >&2
echo " install.sh --uninstall" >&2
echo "--name NAME names the node --register creates; the hostname otherwise" >&2
echo "--name NAME names the node --register creates; the hostname otherwise. < > \" ' are dropped from it; the panel renames a node freely" >&2
exit 2
}
# Names the node a registration creates. A token belongs to a node that already
Expand Down
34 changes: 30 additions & 4 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1053,10 +1053,10 @@ pub async fn agent_register(
Ok(_) => {}
}

// The name comes from a machine not yet vouched for: control characters would
// break the panel's rows, and the length must be bounded. `chars()` rather
// than bytes, so the cut falls on a character boundary.
let name: String = name.trim().chars().filter(|c| !c.is_control()).take(64).collect();
// The name comes from a machine not yet vouched for, and is shown to every
// visitor until the operator renames the node. Trimmed last: a name made of
// markup characters and spaces must come out empty, not as a blank.
let name = db::reported_text(&name, 64).trim().to_owned();
let name = if name.is_empty() { "unnamed".to_owned() } else { name };
// Field defaults live in `Node`'s serde attributes and nowhere else.
// `Node::default()` is a different set of values -- private, reset day 0 --
Expand Down Expand Up @@ -3513,6 +3513,32 @@ mod tests {
assert_eq!(app.db.nodes().unwrap().len(), 1);
}

/// The name comes from an unvouched machine: markup characters are dropped,
/// and what that leaves is trimmed, so a name made only of them is `unnamed`.
#[tokio::test]
async fn a_registered_name_loses_markup_and_is_never_blank() {
let app = std::sync::Arc::new(app());
open_register(Admin, State(app.clone()), panel_headers()).await;
let key = app.db.get("register_key").unwrap();
for (sent, stored) in [("< >", "unnamed"), ("<b>web</b>", "bweb/b"), ("Tom's \"VPS\"", "Toms VPS")] {
let mut headers = HeaderMap::new();
headers.insert("authorization", format!("Bearer {key}").parse().unwrap());
let reply = agent_register(
State(app.clone()),
ConnectInfo("198.51.100.7:40000".parse().unwrap()),
headers,
sent.to_owned(),
)
.await;
let token = String::from_utf8(
axum::body::to_bytes(reply.into_body(), usize::MAX).await.unwrap().to_vec(),
)
.unwrap();
let id = app.db.node_by_token(&token).unwrap().expect("token opens a node");
assert_eq!(app.db.node(id).unwrap().unwrap().name, stored, "sent {sent:?}");
}
}

/// The ceiling on the anonymous route: a leaked key cannot fill the table.
#[tokio::test]
async fn one_window_stops_registering_at_the_limit() {
Expand Down
42 changes: 25 additions & 17 deletions src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ use serde::{Deserialize, Serialize};
use tokio::runtime::RuntimeFlavor;
use tracing::info;

/// A string an agent reported, cut down to what may be shown. Control
/// characters break the panel's rows, and the characters markup is made of are
/// dropped because the value reaches a theme from a machine nobody has vouched
/// for: a theme that sets it as HTML would otherwise run a node's script in the
/// hub's origin, beside the administrator's session. No real OS name, kernel,
/// CPU model or host name contains any of them. `chars()` rather than bytes, so
/// the cut falls on a character boundary.
pub fn reported_text(raw: &str, max: usize) -> String {
raw.chars().filter(|c| !c.is_control() && !matches!(c, '<' | '>' | '"' | '\'')).take(max).collect()
}

pub struct Db {
/// A read-only connection to the same file, for the history charts. A week
/// of one node's probe results is a scan of 98 ms at four 60-second probes
Expand Down Expand Up @@ -1255,23 +1266,12 @@ impl Db {
/// moves to `country_prev_ip` / `country_prev` if it had an answer, and a
/// source equal to that address takes its answer back without a lookup.
pub fn save_facts(&self, id: i64, f: &serde_json::Value, ip: &str, source: &str) -> Result<bool> {
// The same rule `api::agent_register` applies to the name it receives:
// these values come from an unvouched machine, control characters break
// the panel's rows, and the length must be bounded. Six of them -- os,
// kernel, arch, virt, cpu_name, agent_version -- go straight into the
// anonymous public frame, which is rebuilt and pushed to every viewer
// every two seconds, so without a ceiling one node would determine that
// frame's size. 128 rather than 64: a real PRETTY_NAME runs to about 60
// characters and a CPU model to about 50.
let s = |k: &str| {
f.get(k)
.and_then(|v| v.as_str())
.unwrap_or("")
.chars()
.filter(|c| !c.is_control())
.take(128)
.collect::<String>()
};
// Six of these -- os, kernel, arch, virt, cpu_name, agent_version -- go
// straight into the anonymous public frame, which is rebuilt and pushed to
// every viewer every two seconds, so without a ceiling one node would
// determine that frame's size. 128 rather than 64: a real PRETTY_NAME runs
// to about 60 characters and a CPU model to about 50.
let s = |k: &str| reported_text(f.get(k).and_then(|v| v.as_str()).unwrap_or(""), 128);
let n = |k: &str| f.get(k).and_then(|v| v.as_i64()).unwrap_or(0);
let conn = self.conn();
conn.execute(
Expand Down Expand Up @@ -3476,6 +3476,14 @@ mod tests {
let stored = db.node(id).unwrap().unwrap();
assert_eq!(stored.os.chars().count(), 128);
assert_eq!(stored.hostname, "xy", "control characters break the panel's rows");

db.save_facts(id, &serde_json::json!({"os": "<img src=x onerror=\"a()\">'Debian'"}), "ip", "")
.unwrap();
assert_eq!(
db.node(id).unwrap().unwrap().os,
"img src=x onerror=a()Debian",
"no markup reaches a theme"
);
}

/// A correction must survive the node's return. `all_traffic` gates the month
Expand Down
Loading