Use the security ID of the user as sidecar identifier

It turns out that the session identifier is not a good permission boundary and will lead to permission denied for other processes belonging to the same session id, but different security id.

sidecar/Cargo.toml

@@ -89,8 +89,7 @@ nix = { version = "0.26.2", features = ["socket", "mman"] }
 sendfd = { version = "0.4", features = ["tokio"] }
 
 [target.'cfg(windows)'.dependencies]
-winapi = { version = "=0.2.8" }
-kernel32-sys = "0.2.2"
+winapi = { version = "0.3.9", features = ["securitybaseapi", "sddl"] }
 
 [target.'cfg(windows_seh_wrapper)'.dependencies]
 microseh = "0.1.1"

sidecar/src/setup/windows.rs

@@ -3,28 +3,37 @@
 
 use crate::one_way_shared_memory::open_named_shm;
 use crate::primary_sidecar_identifier;
+use crate::setup::Liaison;
 use arrayref::array_ref;
 use datadog_ipc::platform::metadata::ProcessHandle;
 use datadog_ipc::platform::{Channel, PIPE_PATH};
-use kernel32::{CreateFileA, CreateNamedPipeA};
 use libc::getpid;
 use std::error::Error;
 use std::ffi::CString;
-use std::os::windows::io::{FromRawHandle, OwnedHandle};
+use std::os::windows::io::{FromRawHandle, OwnedHandle, RawHandle};
 use std::ptr::null_mut;
 use std::time::{Duration, Instant};
 use std::{env, io, mem};
 use tokio::net::windows::named_pipe::NamedPipeServer;
 use tracing::warn;
 use winapi::{
-    DWORD, ERROR_ACCESS_DENIED, ERROR_PIPE_BUSY, FILE_FLAG_FIRST_PIPE_INSTANCE,
-    FILE_FLAG_OVERLAPPED, GENERIC_READ, GENERIC_WRITE, INVALID_HANDLE_VALUE, LPSECURITY_ATTRIBUTES,
-    OPEN_EXISTING, PIPE_ACCESS_INBOUND, PIPE_ACCESS_OUTBOUND, PIPE_READMODE_BYTE, PIPE_TYPE_BYTE,
-    PIPE_UNLIMITED_INSTANCES, SECURITY_ATTRIBUTES,
+    shared::{
+        minwindef::DWORD,
+        winerror::{ERROR_ACCESS_DENIED, ERROR_PIPE_BUSY},
+    },
+    um::{
+        fileapi::{CreateFileA, OPEN_EXISTING},
+        handleapi::INVALID_HANDLE_VALUE,
+        minwinbase::SECURITY_ATTRIBUTES,
+        winbase::{
+            CreateNamedPipeA, FILE_FLAG_FIRST_PIPE_INSTANCE, FILE_FLAG_OVERLAPPED,
+            PIPE_ACCESS_INBOUND, PIPE_ACCESS_OUTBOUND, PIPE_READMODE_BYTE, PIPE_TYPE_BYTE,
+            PIPE_UNLIMITED_INSTANCES,
+        },
+        winnt::{GENERIC_READ, GENERIC_WRITE},
+    },
 };
 
-use crate::setup::Liaison;
-
 pub type IpcClient = NamedPipeServer;
 pub type IpcServer = OwnedHandle;
 
@@ -70,7 +79,7 @@ impl Liaison for NamedPipeLiaison {
         // Have a ProcessHandle::Getter() so that we don't immediately block in case the sidecar is
         // still starting up, but only the first time we want to submit shared memory
         Ok(Channel::from_client_handle_and_pid(
-            unsafe { OwnedHandle::from_raw_handle(pipe) },
+            unsafe { OwnedHandle::from_raw_handle(pipe as RawHandle) },
             ProcessHandle::Getter(Box::new(move || {
                 // Await the shared memory handle which will contain the pid of the sidecar
                 // As it may not be immediately available during startup
@@ -129,7 +138,7 @@ impl Liaison for NamedPipeLiaison {
                 65536,
                 65536,
                 0,
-                &mut sec_attributes as LPSECURITY_ATTRIBUTES,
+                &mut sec_attributes,
             )
         } {
             INVALID_HANDLE_VALUE => {
@@ -143,7 +152,9 @@ impl Liaison for NamedPipeLiaison {
                     Err(error)
                 }
             }
-            h => Ok(Some(unsafe { OwnedHandle::from_raw_handle(h) })),
+            h => Ok(Some(unsafe {
+                OwnedHandle::from_raw_handle(h as RawHandle)
+            })),
         }
     }
 
@@ -188,16 +199,15 @@ pub type DefaultLiason = NamedPipeLiaison;
 
 #[cfg(test)]
 mod tests {
+    use super::Liaison;
     use futures::future;
-    use kernel32::CloseHandle;
     use rand::distributions::Alphanumeric;
     use rand::{thread_rng, Rng};
     use std::io::Write;
     use std::os::windows::io::IntoRawHandle;
     use tokio::io::AsyncReadExt;
     use tokio::net::windows::named_pipe::NamedPipeServer;
-
-    use super::Liaison;
+    use winapi::um::{handleapi::CloseHandle, winnt::HANDLE};
 
     #[tokio::test]
     async fn test_shared_dir_can_connect_to_socket() -> anyhow::Result<()> {
@@ -232,7 +242,7 @@ mod tests {
 
             // for this test: Somehow, NamedPipeServer remains tangled with the event-loop and won't
             // free itself in time
-            unsafe { CloseHandle(raw_handle) };
+            unsafe { CloseHandle(raw_handle as HANDLE) };
             std::mem::forget(srv);
 
             liaison

sidecar/src/windows.rs

@@ -1,20 +1,39 @@
 // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
+
 use crate::enter_listener_loop;
 use crate::setup::pid_shm_path;
 use datadog_ipc::platform::{
     named_pipe_name_from_raw_handle, FileBackedHandle, MappedMem, NamedShmHandle,
 };
 use futures::FutureExt;
-use kernel32::WTSGetActiveConsoleSessionId;
+use lazy_static::lazy_static;
 use manual_future::ManualFuture;
 use spawn_worker::{SpawnWorker, Stdio};
-use std::io;
+use std::ffi::CStr;
+use std::io::{self, Error};
 use std::os::windows::io::{AsRawHandle, IntoRawHandle, OwnedHandle};
+use std::ptr::null_mut;
 use std::sync::{Arc, Mutex};
 use std::time::Instant;
 use tokio::net::windows::named_pipe::{NamedPipeServer, ServerOptions};
 use tokio::select;
+use tracing::{error, info};
+use winapi::{
+    shared::{
+        sddl::ConvertSidToStringSidA,
+        winerror::{ERROR_INSUFFICIENT_BUFFER, ERROR_NO_TOKEN},
+    },
+    um::{
+        handleapi::CloseHandle,
+        processthreadsapi::{
+            GetCurrentProcess, GetCurrentThread, OpenProcessToken, OpenThreadToken,
+        },
+        securitybaseapi::GetTokenInformation,
+        winbase::LocalFree,
+        winnt::{TokenUser, HANDLE, TOKEN_QUERY, TOKEN_USER},
+    },
+};
 
 #[no_mangle]
 pub extern "C" fn ddog_daemon_entry_point() {
@@ -33,13 +52,13 @@ pub extern "C" fn ddog_daemon_entry_point() {
         {
             Ok(ok) => ok,
             Err(err) => {
-                tracing::error!("Couldn't store pid to shared memory: {err}");
+                error!("Couldn't store pid to shared memory: {err}");
                 return;
             }
         };
         shm.as_slice_mut().copy_from_slice(&pid.to_ne_bytes());
 
-        tracing::info!("Starting sidecar, pid: {}", pid);
+        info!("Starting sidecar, pid: {}", pid);
 
         let acquire_listener = move || unsafe {
             let (closed_future, close_completer) = ManualFuture::new();
@@ -61,11 +80,11 @@ pub extern "C" fn ddog_daemon_entry_point() {
         };
 
         if let Err(err) = enter_listener_loop(acquire_listener) {
-            tracing::error!("Error: {err}")
+            error!("Error: {err}")
         }
     }
 
-    tracing::info!(
+    info!(
         "shutting down sidecar, pid: {}, total runtime: {:.3}s",
         pid,
         now.elapsed().as_secs_f64()
@@ -109,6 +128,81 @@ pub fn setup_daemon_process(listener: OwnedHandle, spawn_cfg: &mut SpawnWorker)
     Ok(())
 }
 
-pub fn primary_sidecar_identifier() -> u32 {
-    unsafe { WTSGetActiveConsoleSessionId() }
+lazy_static! {
+    static ref SIDECAR_IDENTIFIER: String = fetch_sidecar_identifier();
+}
+
+fn fetch_sidecar_identifier() -> String {
+    unsafe {
+        let mut access_token = null_mut();
+
+        loop {
+            if OpenThreadToken(GetCurrentThread(), TOKEN_QUERY, 1, &mut access_token) != 0 {
+                break;
+            }
+            let mut err = Error::last_os_error();
+            if err.raw_os_error() == Some(ERROR_NO_TOKEN as i32) {
+                if OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut access_token) != 0 {
+                    break;
+                }
+                err = Error::last_os_error();
+            }
+            error!("Failed fetching thread token: {:?}", err);
+            return "".to_string();
+        }
+
+        let mut info_buffer_size = 0;
+        if GetTokenInformation(
+            access_token,
+            TokenUser,
+            null_mut(),
+            0,
+            &mut info_buffer_size,
+        ) == 0
+        {
+            let err = Error::last_os_error();
+            if err.raw_os_error() != Some(ERROR_INSUFFICIENT_BUFFER as i32) {
+                error!("Failed fetching thread token: {:?}", err);
+                CloseHandle(access_token);
+                return "".to_string();
+            }
+        }
+
+        let user_token_mem = Vec::<u8>::with_capacity(info_buffer_size as usize);
+        let user_token = user_token_mem.as_ptr() as *const TOKEN_USER;
+        if GetTokenInformation(
+            access_token,
+            TokenUser,
+            user_token as *mut _,
+            info_buffer_size,
+            &mut info_buffer_size,
+        ) == 0
+        {
+            error!("Failed fetching thread token: {:?}", Error::last_os_error());
+            CloseHandle(access_token);
+            return "".to_string();
+        }
+
+        let mut string_sid = null_mut();
+        let success = ConvertSidToStringSidA((*user_token).User.Sid, &mut string_sid);
+        CloseHandle(access_token);
+
+        if success == 0 {
+            error!("Failed stringifying SID: {:?}", Error::last_os_error());
+            return "".to_string();
+        }
+
+        let str = String::from_utf8_lossy(CStr::from_ptr(string_sid).to_bytes()).to_string();
+        LocalFree(string_sid as HANDLE);
+        str
+    }
+}
+
+pub fn primary_sidecar_identifier() -> &'static str {
+    SIDECAR_IDENTIFIER.as_str()
+}
+
+#[test]
+fn test_fetch_identifier() {
+    assert!(primary_sidecar_identifier().starts_with("S-"));
 }