From 28ba2e8f3eeee6bd81e5af431d79a612f85e11bf Mon Sep 17 00:00:00 2001 From: dec05eba Date: Sun, 6 Oct 2024 22:26:47 +0200 Subject: gsr-kms-server 'security': only allow gpu-screen-recorder to get framebuffer --- README.md | 2 +- kms/client/kms_client.c | 83 ++++++++++++++++++++++++-------------------- kms/server/kms_server.c | 92 ++++++++++++++++++++++++++++++++++++++++++++----- project.conf | 2 +- src/utils.c | 15 ++------ 5 files changed, 134 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 4b9f2e1..663f821 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ This software works on X11 and Wayland on AMD, Intel and NVIDIA. ### AMD/Intel/Wayland root permission When recording a window or when using the `-w portal` option under AMD/Intel no special user permission is required, however when recording a monitor (or when using wayland) the program needs root permission (to access KMS).\ -This is safe in GPU Screen Recorder as the part that needs root access has been moved to its own small program that only does one thing.\ +This is safe in GPU Screen Recorder as the part that needs root access has been moved to its own small program that only does one thing and it only allows GPU Screen Recorder to run it.\ For you as a user this only means that if you installed GPU Screen Recorder as a flatpak then a prompt asking for root password will show up when you start recording. # Performance On a system with a i5 4690k CPU and a GTX 1080 GPU:\ diff --git a/kms/client/kms_client.c b/kms/client/kms_client.c index 468e3a6..b72c64b 100644 --- a/kms/client/kms_client.c +++ b/kms/client/kms_client.c @@ -146,47 +146,41 @@ static bool create_socket_path(char *output_path, size_t output_path_size) { return true; } -static void string_copy(char *dst, const char *src, int len) { - int src_len = strlen(src); - int min_len = src_len; - if(len - 1 < min_len) - min_len = len - 1; - memcpy(dst, src, min_len); - dst[min_len] = '\0'; -} - -static bool find_program_in_path(const char *program_name, char *filepath, int filepath_len) { - const char *path = getenv("PATH"); - if(!path) +static bool readlink_realpath(const char *filepath, char *buffer) { + char symlinked_path[PATH_MAX]; + ssize_t bytes_written = readlink(filepath, symlinked_path, sizeof(symlinked_path) - 1); + if(bytes_written == -1 && errno == EINVAL) { + /* Not a symlink */ + snprintf(symlinked_path, sizeof(symlinked_path), "%s", filepath); + } else if(bytes_written == -1) { return false; + } else { + symlinked_path[bytes_written] = '\0'; + } - int program_name_len = strlen(program_name); - const char *end = path + strlen(path); - while(path != end) { - const char *part_end = strchr(path, ':'); - const char *next = part_end; - if(part_end) { - next = part_end + 1; - } else { - part_end = end; - next = end; - } + if(!realpath(symlinked_path, buffer)) + return false; - int len = part_end - path; - if(len + 1 + program_name_len < filepath_len) { - memcpy(filepath, path, len); - filepath[len] = '/'; - memcpy(filepath + len + 1, program_name, program_name_len); - filepath[len + 1 + program_name_len] = '\0'; + return true; +} - if(access(filepath, F_OK) == 0) - return true; - } +static bool strcat_safe(char *str, int size, const char *str_to_add) { + const int str_len = strlen(str); + const int str_to_add_len = strlen(str_to_add); + if(str_len + str_to_add_len + 1 >= size) + return false; - path = next; - } + memcpy(str + str_len, str_to_add, str_to_add_len); + str[str_len + str_to_add_len] = '\0'; + return true; +} - return false; +static void file_get_directory(char *filepath) { + char *end = strrchr(filepath, '/'); + if(end == NULL) + filepath[0] = '\0'; + else + *end = '\0'; } int gsr_kms_client_init(gsr_kms_client *self, const char *card_path) { @@ -206,11 +200,24 @@ int gsr_kms_client_init(gsr_kms_client *self, const char *card_path) { } char server_filepath[PATH_MAX]; - if(!find_program_in_path("gsr-kms-server", server_filepath, sizeof(server_filepath))) { - fprintf(stderr, "gsr error: gsr_kms_client_init: gsr-kms-server is not installed\n"); + if(!readlink_realpath("/proc/self/exe", server_filepath)) { + fprintf(stderr, "gsr error: gsr_kms_client_init: failed to resolve /proc/self/exe\n"); + return -1; + } + file_get_directory(server_filepath); + + if(!strcat_safe(server_filepath, sizeof(server_filepath), "/gsr-kms-server")) { + fprintf(stderr, "gsr error: gsr_kms_client_init: gsr-kms-server path too long\n"); return -1; } + if(access(server_filepath, F_OK) != 0) { + fprintf(stderr, "gsr error: gsr_kms_client_init: gsr-kms-server is not installed (%s not found)\n", server_filepath); + return -1; + } + + fprintf(stderr, "gsr info: gsr_kms_client_init: setting up connection to %s\n", server_filepath); + const bool inside_flatpak = getenv("FLATPAK_ID") != NULL; const char *home = getenv("HOME"); if(!home) @@ -251,7 +258,7 @@ int gsr_kms_client_init(gsr_kms_client *self, const char *card_path) { } local_addr.sun_family = AF_UNIX; - string_copy(local_addr.sun_path, self->initial_socket_path, sizeof(local_addr.sun_path)); + snprintf(local_addr.sun_path, sizeof(local_addr.sun_path), "%s", (const char*)self->initial_socket_path); const mode_t prev_mask = umask(0000); const int bind_res = bind(self->initial_socket_fd, (struct sockaddr*)&local_addr, sizeof(local_addr.sun_family) + strlen(local_addr.sun_path)); diff --git a/kms/server/kms_server.c b/kms/server/kms_server.c index c6460ad..53770b4 100644 --- a/kms/server/kms_server.c +++ b/kms/server/kms_server.c @@ -1,3 +1,7 @@ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + #include "../kms_shared.h" #include @@ -6,6 +10,7 @@ #include #include +#include #include #include #include @@ -413,13 +418,79 @@ static double clock_get_monotonic_seconds(void) { return (double)ts.tv_sec + (double)ts.tv_nsec * 0.000000001; } -static void string_copy(char *dst, const char *src, int len) { - int src_len = strlen(src); - int min_len = src_len; - if(len - 1 < min_len) - min_len = len - 1; - memcpy(dst, src, min_len); - dst[min_len] = '\0'; +static bool readlink_realpath(const char *filepath, char *buffer) { + char symlinked_path[PATH_MAX]; + ssize_t bytes_written = readlink(filepath, symlinked_path, sizeof(symlinked_path) - 1); + if(bytes_written == -1 && errno == EINVAL) { + /* Not a symlink */ + snprintf(symlinked_path, sizeof(symlinked_path), "%s", filepath); + } else if(bytes_written == -1) { + return false; + } else { + symlinked_path[bytes_written] = '\0'; + } + + if(!realpath(symlinked_path, buffer)) + return false; + + return true; +} + +static void file_get_directory(char *filepath) { + char *end = strrchr(filepath, '/'); + if(end == NULL) + filepath[0] = '\0'; + else + *end = '\0'; +} + +static bool string_ends_with(const char *str, const char *ends_with) { + const int len = strlen(str); + const int ends_with_len = strlen(ends_with); + return len >= ends_with_len && memcmp(str + len - ends_with_len, ends_with, ends_with_len) == 0; +} + +// This is not foolproof, but the assumption is that gsr-kms-server and gpu-screen-recorder are installed in the same directory +// in a location that only the root user can write to (usually /usr/bin or /usr/local/bin) and if the client runs from that location +// and is called gpu-screen-recorder then gsr-kms-server can only be used by a malicious program if the malicious program +// had root access, to modify that program install directory. +static bool is_remote_peer_program_gpu_screen_recorder(int socket_fd) { + // TODO: Use SO_PEERPIDFD on kernel >= 6.5 to avoid a race condition in the /proc/ check + struct ucred cred; + socklen_t ucred_len = sizeof(cred); + if(getsockopt(socket_fd, SOL_SOCKET, SO_PEERCRED, &cred, &ucred_len) == -1) { + fprintf(stderr, "kms server error: failed to get peer credentials, error: %s\n", strerror(errno)); + return false; + } + + char self_directory[PATH_MAX]; + if(!readlink_realpath("/proc/self/exe", self_directory)) { + fprintf(stderr, "kms server error: failed to resolve /proc/self/exe\n"); + return false; + } + file_get_directory(self_directory); + + char peer_directory[PATH_MAX]; + char peer_exe_path[PATH_MAX]; + snprintf(peer_exe_path, sizeof(peer_exe_path), "/proc/%d/exe", (int)cred.pid); + if(!readlink_realpath(peer_exe_path, peer_directory)) { + fprintf(stderr, "kms server error: failed to resolve /proc/self/exe\n"); + return false; + } + + if(!string_ends_with(peer_directory, "/gpu-screen-recorder")) { + fprintf(stderr, "kms server error: only gpu-screen-recorder can use gsr-kms-server. client program location is %s\n", peer_directory); + return false; + } + + file_get_directory(peer_directory); + + if(strcmp(self_directory, peer_directory) != 0) { + fprintf(stderr, "kms server error: the client program is in directory %s but only programs in %s can run gsr-kms-server\n", peer_directory, self_directory); + return false; + } + + return true; } int main(int argc, char **argv) { @@ -478,7 +549,7 @@ int main(int argc, char **argv) { while(clock_get_monotonic_seconds() - start_time < connect_timeout_sec) { struct sockaddr_un remote_addr = {0}; remote_addr.sun_family = AF_UNIX; - string_copy(remote_addr.sun_path, domain_socket_path, sizeof(remote_addr.sun_path)); + snprintf(remote_addr.sun_path, sizeof(remote_addr.sun_path), "%s", domain_socket_path); // TODO: Check if parent disconnected if(connect(socket_fd, (struct sockaddr*)&remote_addr, sizeof(remote_addr.sun_family) + strlen(remote_addr.sun_path)) == -1) { if(errno == ECONNREFUSED || errno == ENOENT) { @@ -505,6 +576,11 @@ int main(int argc, char **argv) { goto done; } + if(!is_remote_peer_program_gpu_screen_recorder(socket_fd)) { + res = 3; + goto done; + } + for(;;) { gsr_kms_request request; request.version = 0; diff --git a/project.conf b/project.conf index 6b107b0..3c36402 100644 --- a/project.conf +++ b/project.conf @@ -5,7 +5,7 @@ version = "4.1.11" platforms = ["posix"] [config] -ignore_dirs = ["kms/server", "build"] +ignore_dirs = ["kms/server", "build", "debug-build"] #error_on_warning = "true" [define] diff --git a/src/utils.c b/src/utils.c index 42f4c40..6fe5904 100644 --- a/src/utils.c +++ b/src/utils.c @@ -464,23 +464,14 @@ static bool try_card_has_valid_plane(const char *card_path) { return false; } -static void string_copy(char *dst, const char *src, int len) { - int src_len = strlen(src); - int min_len = src_len; - if(len - 1 < min_len) - min_len = len - 1; - memcpy(dst, src, min_len); - dst[min_len] = '\0'; -} - bool gsr_get_valid_card_path(gsr_egl *egl, char *output, bool is_monitor_capture) { if(egl->dri_card_path) { - string_copy(output, egl->dri_card_path, 127); + snprintf(output, 128, "%s", egl->dri_card_path); return is_monitor_capture ? try_card_has_valid_plane(output) : true; } for(int i = 0; i < 10; ++i) { - snprintf(output, 127, DRM_DEV_NAME, DRM_DIR_NAME, i); + snprintf(output, 128, DRM_DEV_NAME, DRM_DIR_NAME, i); if(try_card_has_valid_plane(output)) return true; } @@ -494,7 +485,7 @@ bool gsr_card_path_get_render_path(const char *card_path, char *render_path) { char *render_path_tmp = drmGetRenderDeviceNameFromFd(fd); if(render_path_tmp) { - string_copy(render_path, render_path_tmp, 127); + snprintf(render_path, 128, "%s", render_path_tmp); free(render_path_tmp); close(fd); return true; -- cgit v1.2.3