mirror of
https://github.com/nestriness/nestri.git
synced 2026-09-19 17:25:19 +03:00
fix(nesinit): do not mount over the share tree, and check who serves an address
Four findings from review, all of them real. The relay's directory was mounted on the tree a session's shares live in. A fresh tmpfs there hides every directory the image prepared underneath it: the install, the user state, the work directory, and the mount point the log share is attached to from fstab. A box would have come up with a socket and without any of the places its workload looks for its files, and the exact-path check could not notice, because what fstab mounts is a directory inside that tree rather than the tree itself. It moves to /run, which is where a runtime socket belongs, is a tmpfs already, and has nothing else mounted inside it. It was also owned by this process and closed to everyone else, which stopped the workload traversing it to reach the relay at all. The directory is now readable and searchable, and still writable by nothing but this process, which is what makes the socket in it unreplaceable; the socket itself is what the workload is allowed to connect to. The permission belongs on the socket rather than on the path. The address served to a reader was built once at startup and served forever, so a reader that polls for a better one could only ever get the first. An endpoint does not know all of its own addresses when it binds: the first is the one that works on the same network and fails from anywhere else. It is now rebuilt per read, which is what makes polling for it worth doing. And the address was taken from whoever held a path in a directory the workload can write. Workload code could unlink the socket a service was listening on, bind its own, and every read afterwards would hand the client an address of its choosing -- a session given to somebody else rather than a session that fails. The peer's credentials are now checked before a byte is read, from the kernel rather than from anything the peer says about itself, and an address served by the workload's own user is refused and said loudly. That check is only worth something while the workload has a user of its own, so the image grows one. Two users, and they must stay two: one runs the services that ship in the image, the other is who a workload runs as. Sharing one does not weaken the check, it makes every session fail it. A workload running as root is every user at once and cannot be told apart from anything; the check stands down there and says so at boot instead, because refusing root would refuse whatever legitimately serves the address as well. Also bumps tinyvec by a patch release. It does not build on this toolchain -- `vec` resolves to the module and not the macro -- which made every crate that depends on an endpoint, including this one, unbuildable. Pre-existing and nothing to do with this change; the lockfile said the same version before it.
This commit is contained in:
@@ -292,7 +292,24 @@ pub async fn run_stats_ipc_listener(
|
||||
tracing::info!("stats IPC listener exited");
|
||||
}
|
||||
|
||||
pub async fn run_ticket_ipc_listener(socket_path: PathBuf, ticket: crate::NestriTicket) {
|
||||
/// Serve the address a client needs, rebuilt on every read.
|
||||
///
|
||||
/// **The ticket is built per connection and not once at startup.** An endpoint
|
||||
/// does not know all of its own addresses when it binds: a direct one is there
|
||||
/// immediately, and a relayed or hole-punched one becomes known seconds later.
|
||||
/// A ticket captured once therefore carries only the address that was available
|
||||
/// first, which is the one that works on the same network and fails from
|
||||
/// anywhere else — and whoever reads this polls precisely so that a better
|
||||
/// answer can replace it. Serving a snapshot made that polling pointless: every
|
||||
/// read returned the same local-only address forever.
|
||||
///
|
||||
/// The stream name is generated once and kept, because it identifies this
|
||||
/// session rather than describing how to reach it. Only the addresses change.
|
||||
pub async fn run_ticket_ipc_listener(
|
||||
socket_path: PathBuf,
|
||||
endpoint: iroh::Endpoint,
|
||||
stream_name: String,
|
||||
) {
|
||||
if socket_path.exists() {
|
||||
let _ = std::fs::remove_file(&socket_path);
|
||||
}
|
||||
@@ -319,6 +336,9 @@ pub async fn run_ticket_ipc_listener(socket_path: PathBuf, ticket: crate::Nestri
|
||||
loop {
|
||||
match listener.accept().await {
|
||||
Ok((mut stream, _)) => {
|
||||
// Asked of the endpoint now, so an address it has learned since
|
||||
// the last read is in this answer.
|
||||
let ticket = crate::NestriTicket::new(endpoint.addr(), stream_name.clone());
|
||||
if let Err(e) = stream.write_all(format!("{ticket}\n").as_bytes()).await {
|
||||
tracing::warn!("could not write ticket to IPC: {e}");
|
||||
}
|
||||
|
||||
@@ -207,7 +207,10 @@ async fn main() -> Result<()> {
|
||||
|
||||
// ── Accept mode: generate ticket, wait for desktop-app to connect ─────
|
||||
let stream_name = ticket::generate_stream_name();
|
||||
let ticket = NestriTicket::new(endpoint_addr, stream_name);
|
||||
// For the log line below only. What a reader of the socket gets is built
|
||||
// per read from the endpoint itself, because the addresses this can be
|
||||
// reached at are not all known yet.
|
||||
let ticket = NestriTicket::new(endpoint_addr, stream_name.clone());
|
||||
|
||||
tracing::info!("╔═══════════════╗");
|
||||
tracing::info!("║ NESTRI TICKET ║");
|
||||
@@ -245,7 +248,12 @@ async fn main() -> Result<()> {
|
||||
|
||||
let ticket_ipc = args.ticket_ipc.clone();
|
||||
tokio::spawn({
|
||||
async move { ipc_listener::run_ticket_ipc_listener(ticket_ipc, ticket).await }
|
||||
// The endpoint rather than a ticket made from it: the addresses it can
|
||||
// be reached at are not all known yet, and whoever reads this socket
|
||||
// re-reads it so that a better one can replace the first.
|
||||
let endpoint = endpoint.clone();
|
||||
let stream_name = stream_name.clone();
|
||||
async move { ipc_listener::run_ticket_ipc_listener(ticket_ipc, endpoint, stream_name).await }
|
||||
});
|
||||
|
||||
// Accept loop
|
||||
|
||||
Reference in New Issue
Block a user