diff --git a/build.py b/build.py index 6740d8920..3efeaf795 100755 --- a/build.py +++ b/build.py @@ -413,57 +413,27 @@ def build_libdrmtap_so(): return _single_real_so(sos, f'the libdrmtap meson build dir {build_dir}') -def append_drm_ldconfig_postinst(): - # The DRM package installs libdrmtap.so under a private dir; register it with the - # dynamic linker so the in-process dlopen("libdrmtap.so.0") resolves. Only the DRM - # package calls this, so the stock package's postinst stays byte-identical to upstream. - # - # This block is appended AFTER the stock postinst, which has already run - # `systemctl start rustdesk`. On a FRESH install that ordering is a trap: the root - # service's DRM pre-warm can dlopen("libdrmtap.so.0") BEFORE this ldconfig has - # populated the linker cache, the dlopen fails, and that failure is cached in the - # DRMTAP_LIB OnceLock for the life of the process — so DRM stays disabled until a - # manual restart. So immediately after ldconfig we `try-restart` the unit: it re-runs - # the pre-warm against the now-resolvable soname. `try-restart` is a no-op when the - # unit is not running, so it never spuriously starts the service. - with open('tmpdeb/DEBIAN/postinst', 'a') as f: - f.write( - '\n' - 'if [ "$1" = configure ] && [ -d /usr/lib/rustdesk ]; then\n' - '\tldconfig /usr/lib/rustdesk 2>/dev/null || ldconfig 2>/dev/null || true\n' - '\tif command -v systemctl >/dev/null 2>&1; then\n' - '\t\tsystemctl try-restart rustdesk 2>/dev/null || true\n' - '\tfi\n' - 'fi\n' - ) - - def finalize_deb(version, ships_so, so_basename=None): # Shared deb finalization for build_flutter_deb / build_deb_from_folder. Any DRM .so is assumed - # already staged at tmpdeb/usr/lib/rustdesk/. For a DRM build this adds the soname symlink + the - # ld.so.conf.d drop-in, names the package rustdesk-unattended-wayland with libdrmtap's runtime - # deps (libdrm / EGL / GLESv2), and appends the ldconfig postinst; otherwise it builds the stock - # rustdesk package. Then it writes the control, checksums, builds, and renames the .deb. + # already staged at tmpdeb/usr/lib/rustdesk/. For a DRM build this adds the soname symlink and + # names the package rustdesk-unattended-wayland with libdrmtap's runtime deps (libdrm / EGL / + # GLESv2); the .so is dlopen'd by absolute path so no ld.so.conf.d drop-in or ldconfig postinst is + # needed and the stock postinst is used unchanged. Otherwise it builds the stock rustdesk package. + # Then it writes the control, checksums, builds, and renames the .deb. if ships_so: + # Only the soname symlink is needed. libdrmtap is resolved by ABSOLUTE path + # (/usr/lib/rustdesk/libdrmtap.so.0) at the in-process dlopen site (drmtap_dl.rs), so the deb + # does NOT drop /usr/lib/rustdesk into the system-wide /etc/ld.so.conf.d search path -- that + # would let this private library shadow a system library for every binary on the host, which + # Debian Policy 10.2 forbids. No ld.so.conf.d drop-in and no ldconfig trigger are shipped. system2(f'ln -sf {so_basename} tmpdeb/usr/lib/rustdesk/libdrmtap.so.0') - # TODO(drm, Debian Policy 10.2): dropping /usr/lib/rustdesk into the SYSTEM-WIDE - # linker search path (/etc/ld.so.conf.d) lets a privately-bundled library shadow a - # system library for EVERY binary on the host, which Debian Policy 10.2 forbids. - # The correct fix is to make the in-process dlopen resolve libdrmtap by ABSOLUTE - # path ("/usr/lib/rustdesk/libdrmtap.so.0") -- or link the rustdesk cdylib with an - # rpath of /usr/lib/rustdesk -- and then drop this ld.so.conf.d drop-in entirely. - # That change lives at the dlopen call site in src/ (drmtap_dl.rs), owned by - # another engineer, so it is out of scope for this packaging file; kept until then. - system2('mkdir -p tmpdeb/etc/ld.so.conf.d') - with open('tmpdeb/etc/ld.so.conf.d/rustdesk-unattended-wayland.conf', 'w') as f: - f.write('/usr/lib/rustdesk\n') package_name = 'rustdesk-unattended-wayland' if ships_so else 'rustdesk' drm_depends = ", libdrm2, libegl1, libgles2" if ships_so else "" system2('mkdir -p tmpdeb/DEBIAN') generate_control_file(version, drm_depends, package_name) system2('cp -a ../res/DEBIAN/* tmpdeb/DEBIAN/') - if ships_so: - append_drm_ldconfig_postinst() + # No ldconfig postinst: libdrmtap is dlopen'd by absolute path (see drmtap_dl.rs), so there is + # nothing to register with the linker cache and the stock postinst is used unchanged. md5_file_folder("tmpdeb/") system2('dpkg-deb -b tmpdeb rustdesk.deb;') system2('/bin/rm -rf tmpdeb/') @@ -509,8 +479,8 @@ def build_flutter_deb(version, features): "echo \"#!/bin/sh\" >> tmpdeb/usr/share/rustdesk/files/polkit && chmod a+x tmpdeb/usr/share/rustdesk/files/polkit") # Bundle libdrmtap.so for the DRM/KMS capture path — but ONLY when this build # actually enabled the `drm` feature, so normal packages stay opt-out. The root - # service dlopen-s it in-process (no setcap helper); it lives in a private dir - # that postinst registers with ldconfig so dlopen("libdrmtap.so.0") resolves. + # service dlopen-s it in-process (no setcap helper) from its private dir by + # absolute path (/usr/lib/rustdesk/libdrmtap.so.0), so no linker-path registration. # Bundle libdrmtap.so for a DRM build (opt-in), then finalize the deb. A DRM build ships as a # separately-named rustdesk-unattended-wayland package (finalize_deb marks it # Conflicts/Replaces/Provides rustdesk), so installing it is an explicit choice. @@ -557,7 +527,7 @@ def build_deb_from_folder(version, binary_folder, want_drm=False): # The variant must be decided by the EXPLICIT --drm request, not merely by what happens # to be staged. Cross-check the two and fail loudly on a mismatch: a drm binary staged # WITHOUT its libdrmtap.so.0.* would otherwise be silently shipped as the stock - # `rustdesk` package (no drm deps, no ldconfig, a dlopen that can never resolve), and a + # `rustdesk` package (no drm deps, a dlopen that can never resolve), and a # bundle that DOES carry the .so would be shipped as the consent-bypass variant even # when --drm was never asked for. if want_drm and not ships_so: diff --git a/libs/scrap/src/common/drmtap_dl.rs b/libs/scrap/src/common/drmtap_dl.rs index c49f5f836..c4eb13147 100644 --- a/libs/scrap/src/common/drmtap_dl.rs +++ b/libs/scrap/src/common/drmtap_dl.rs @@ -178,8 +178,16 @@ const DRMTAP_ABI_MAJOR: c_int = 0; impl DrmtapLib { fn load() -> Option { - // soname first (what a packaged .so installs), then the dev symlink. - const LIB_NAMES: [&str; 2] = ["libdrmtap.so.0", "libdrmtap.so"]; + // Absolute install path FIRST: the deb bundles the .so privately under /usr/lib/rustdesk and + // deliberately does NOT register that dir in the system-wide ld.so search path (Debian Policy + // 10.2 forbids a private lib shadowing system libraries for every binary), so the packaged + // build must resolve it by absolute path. The bare sonames remain as a fallback for a dev build + // where the .so is reachable via LD_LIBRARY_PATH or a local ldconfig. + const LIB_NAMES: [&str; 3] = [ + "/usr/lib/rustdesk/libdrmtap.so.0", + "libdrmtap.so.0", + "libdrmtap.so", + ]; unsafe { let (lib, name) = LIB_NAMES .iter() diff --git a/src/ipc.rs b/src/ipc.rs index 8711efe37..1699011bd 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -2007,6 +2007,34 @@ pub async fn start_drm() { /// task itself stays fully async — hence `Send`, hence `tokio::spawn`able — and only forwards /// messages to the wire. On any error / disconnect it returns; the `DrmStopGuard` plus dropping the /// channels tears the worker down, and the client falls back to PipeWire/portal. +/// Concurrency cap on accepted `_drm` consumer connections. Each accepted consumer spawns a worker +/// that opens a DRM context, so even though the peer is authorized we still bound how many a single +/// (buggy or compromised) --server can open, to keep it from exhausting root-service threads/memory. +/// One connection per served display is plenty; the slack covers a reconnect overlapping an old worker +/// still tearing down. +#[cfg(all(target_os = "linux", feature = "drm"))] +const MAX_DRM_CONNS: usize = 8; + +/// Whether a new `_drm` connection is admitted, given the live count taken BEFORE it (the value +/// `AtomicUsize::fetch_add` returns). Pure, so the admission bound is unit-testable without the runtime +/// counter: admit while strictly below the cap, reject at or above it. +#[cfg(all(target_os = "linux", feature = "drm"))] +fn drm_conn_admitted(prev_count: usize) -> bool { + prev_count < MAX_DRM_CONNS +} + +/// Whether a `_drm` peer may keep receiving frames (review 3.3): root (uid 0) always, any other peer +/// only while it still matches the active-session uid, and an unknown peer never (fail closed). Pure, +/// so the per-frame re-authorization decision is unit-testable without a live logind session. +#[cfg(all(target_os = "linux", feature = "drm"))] +fn drm_peer_authorized(peer_uid: Option, active_uid: Option) -> bool { + match peer_uid { + Some(0) => true, + Some(uid) => active_uid == Some(uid), + None => false, + } +} + #[cfg(all(target_os = "linux", feature = "drm"))] async fn handle_drm_conn(stream: Connection) -> ResultType<()> { use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -2027,7 +2055,6 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> { // compromised --server cannot exhaust root-service threads/memory by opening an unbounded number // of streams. One connection per served display is plenty; MAX_DRM_CONNS covers multi-monitor // plus a little slack for a reconnect overlapping an old worker still tearing down. - const MAX_DRM_CONNS: usize = 8; static DRM_CONN_COUNT: AtomicUsize = AtomicUsize::new(0); struct DrmConnGuard; impl Drop for DrmConnGuard { @@ -2035,7 +2062,7 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> { DRM_CONN_COUNT.fetch_sub(1, Ordering::SeqCst); } } - if DRM_CONN_COUNT.fetch_add(1, Ordering::SeqCst) >= MAX_DRM_CONNS { + if !drm_conn_admitted(DRM_CONN_COUNT.fetch_add(1, Ordering::SeqCst)) { DRM_CONN_COUNT.fetch_sub(1, Ordering::SeqCst); log::warn!("drm: too many concurrent _drm connections (>= {MAX_DRM_CONNS}); rejecting"); return Ok(()); @@ -2134,11 +2161,7 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> { // unknown (typically mid-switch), which we treat as fail-closed and stop. The stop latency is // therefore bounded by the service loop's active-uid cache cadence (a few hundred ms), plus we // stop as soon as the cache goes empty at the start of a switch. - let peer_ok = match peer_uid { - Some(0) => true, - Some(uid) => active_uid_cached() == Some(uid), - None => false, - }; + let peer_ok = drm_peer_authorized(peer_uid, active_uid_cached()); if !peer_ok { log::warn!("drm: _drm peer no longer matches the active session (or it is unknown); closing"); break; @@ -3737,4 +3760,32 @@ mod drm_conn_tests { let euid = unsafe { libc::geteuid() }; assert_eq!(peer_uid_from_fd(a.as_raw_fd()), Some(euid)); } + + // Per-frame _drm re-auth decision (review 3.3): root always passes; a non-root peer passes only + // while it still equals the active-session uid; an unknown peer or active session fails closed. + #[test] + fn drm_peer_authorized_matrix() { + // root (uid 0) is always authorized, regardless of the active session (even unknown). + assert!(drm_peer_authorized(Some(0), Some(1000))); + assert!(drm_peer_authorized(Some(0), None)); + // a non-root peer is authorized only while it matches the active-session uid. + assert!(drm_peer_authorized(Some(1000), Some(1000))); + // a non-root peer whose session is no longer active (switched away) is rejected. + assert!(!drm_peer_authorized(Some(1000), Some(1001))); + // fail closed when the active session is momentarily unknown (mid session switch). + assert!(!drm_peer_authorized(Some(1000), None)); + // fail closed when the peer uid could not be determined. + assert!(!drm_peer_authorized(None, Some(1000))); + assert!(!drm_peer_authorized(None, None)); + } + + // _drm admission bound (review 6): 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] + fn drm_conn_admission_bound() { + assert!(drm_conn_admitted(0)); + assert!(drm_conn_admitted(MAX_DRM_CONNS - 1)); // last admitted slot + assert!(!drm_conn_admitted(MAX_DRM_CONNS)); // cap reached -> rejected + assert!(!drm_conn_admitted(MAX_DRM_CONNS + 5)); // over cap -> rejected + } }