From a767d6efd3240baf64eef169113921e42eca9096 Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Tue, 28 Jul 2026 11:09:20 -0300 Subject: [PATCH] drm: test the half of the accept-time authorization that had none The review called the accept-time authorization decision the single most important invariant in this PR, and noted it has no test. Half of it did: drm_peer_authorized_matrix covers the uid rule. The other half, the /proc//exe identity match that stops a DIFFERENT program running as the right uid from being handed the screen, did not. We said last round that testing it needs a second process with a different executable, so it was integration rather than unit work. That was too pessimistic: the negative case needs ANY foreign executable, not a second build of rustdesk, and /bin/sleep is one. So the test covers all three outcomes: our own pid matches, a live process running another binary is rejected, and a peer whose pid cannot be resolved is rejected rather than admitted. The test synchronizes on the child having exec'd before it looks. spawn returns while the child is still a copy of us, and until exec completes /proc//exe points at OUR binary, so reading it too early sees a match and the assertion passes for the wrong reason. It failed exactly that way under the parallel suite and passed when run alone. A real peer has necessarily exec'd and connected before it can be authorized, so the window exists only in the test. 102 tests pass, three consecutive full runs, both configs build. --- src/ipc/drm.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/ipc/drm.rs b/src/ipc/drm.rs index 0857b9379..82acf2675 100644 --- a/src/ipc/drm.rs +++ b/src/ipc/drm.rs @@ -1780,6 +1780,61 @@ mod drm_conn_tests { assert!(!drm_peer_authorized(None, None)); } + // The accept-time authorization has two halves. `drm_peer_authorized_matrix` above covers the uid + // half; this covers the other one, the /proc//exe identity match, which is what stops a + // DIFFERENT program running as the right uid from being handed the screen. It needs no second + // build of rustdesk: any process whose executable differs from ours is a valid negative, so the + // test spawns one. + #[test] + fn accept_time_exe_match_accepts_only_our_own_executable() { + // Our own pid must match: same /proc//exe by construction. + let me = std::process::id(); + assert!( + super::ipc_auth::ensure_peer_executable_matches_current_by_pid_opt(Some(me), "_drm").is_ok(), + "the test process must match its own executable" + ); + + // A real, live process running a DIFFERENT executable must not. + let mut other = std::process::Command::new("/bin/sleep") + .arg("30") + .spawn() + .expect("/bin/sleep should be spawnable in the test environment"); + // Wait for the exec to actually happen before looking. `spawn` returns once the child + // exists, and until it finishes exec'ing, /proc//exe still points at OUR binary, so + // reading it too early sees a match and the assertion below passes for the wrong reason. + // Synchronize on an INDEPENDENT observation (the link changing) rather than on the check + // being tested. A real peer has necessarily exec'd and connected before it is authorized, + // so this window exists only in the test. + let ours = std::fs::read_link(format!("/proc/{me}/exe")).ok(); + let peer_link = format!("/proc/{}/exe", other.id()); + let mut exec_done = false; + for _ in 0..200 { + match std::fs::read_link(&peer_link) { + Ok(p) if Some(&p) != ours.as_ref() => { + exec_done = true; + break; + } + _ => std::thread::sleep(std::time::Duration::from_millis(10)), + } + } + let res = if exec_done { + super::ipc_auth::ensure_peer_executable_matches_current_by_pid_opt(Some(other.id()), "_drm") + } else { + Err(anyhow::anyhow!("child never exec'd; nothing was tested")) + }; + let _ = other.kill(); + let _ = other.wait(); + assert!(exec_done, "the spawned child never exec'd, so the negative case was not exercised"); + assert!( + res.is_err(), + "a peer running another executable must be rejected, got {res:?}" + ); + + // No pid at all is a rejection, not a pass: the check fails closed when it cannot identify + // the peer, which is the case that matters if the kernel ever stops answering SO_PEERCRED. + assert!(super::ipc_auth::ensure_peer_executable_matches_current_by_pid_opt(None, "_drm").is_err()); + } + // _drm admission bound: admit strictly below MAX_DRM_CONNS, reject at and above it. // `prev_count` is the live count taken before this connection (what fetch_add returns). #[test]