search

dec05eba / gpu-screen-recorder-issues

GPU Screen Recorder issue tracker
11 stars 0 forks source link

[BUG] After uninstall and reinstall, rootful mode does not ask for a password, meaning it was not correctly uninstalled. #20

Closed lbrame closed 2 months ago

lbrame commented 2 months ago

Describe the bug After installing GPU Screen Recorder from Flathub, and running it in rootful mode on GNOME Wayland session on Fedora 40, it only asks for root access the first time. It never asks for that password prompt again. However, this behaviour persists across complete reinstalls and computer reboots - meaning that whatever binary was installed the first time asking for root access was not completely removed or permissions given were completely reverted.

To Reproduce GUI option:

  1. Install GPU Screen Recorder from Flathub
  2. Select the root access mode
  3. Select "record"
  4. Accept the password prompt
  5. Uninstall the flatpak with the --delete-data flag
  6. Reboot the computer
  7. Install the program again
  8. Re-do steps from 2 to 4

Expected behavior The program asks for the root password again, because whatever it uses for privilege elevation was reverted

Screenshots The program no longer asks for the root password, because whatever it did after requesting root access the first time is still there.

Desktop (please complete the following information):

Additional context

lbrame commented 2 months ago

I have found further information by looking at the install / uninstall scripts and doing some find-fu on my system. Apparently, this program writes to ~/.local/share/gpu-screen-recorder. It leaves there a kms-server-proxy-1 file, which is an executable binary. Deleting that file seems to resolve the issue.

Personally, I think behaviour like this should be documented as it is simply bad practice to leave random binaries around the user's system, especially if those binaries could be used for privilege elevation. Even adding some warnings about it in an alert would probably be enough!

I also suggest including it in the uninstall script upstream: https://git.dec05eba.com/gpu-screen-recorder/tree/uninstall.sh

The script does not remove this file.

dec05eba commented 2 months ago

That uninstall script only applies to when you install gpu screen recorder from source and when you install it from source kms-server-proxy wont exist. That file only exists when you use it from flatpak and flatpak doesn't provide any way to do extra steps at uninstall (to my knowledge) so its not possible for gpu screen recorder to remove that file from the flatpak. I can add information about that file in the flatpak (flathub) description that the user has to remove it manually.

lbrame commented 2 months ago

I think that would be good - the first thing I did was looking at the README in the Flathub Github.

The better way would be placing the file inside of the Flatpak's private filesystem, found in $HOME/.var/app/com.x.y. The best practice on Flatpak is actually not to use the real user's home-dir, but treat that directory as a "virtual $HOME", and include the path to ./local/share/gpu-screen-recording there. The manifest should also be modified to not have both filesystem=host and filesystem=home disabled. You don't need it since, once you elevate with pexec, you are out of the scope of the sandbox anyway and you can still read /dev/ from root. When that's done, it should require no code changes for the Flatpak to query its own internal home directory for the file. That would be uninstalled automatically when flatpak uninstall --delete-data is called on the package ID.

For example, here is how com.spotify.Client (a fairly good example of good packaging practices) has its private directory look like on my system:

~/.var/app/com.spotify.Client
% tree -L 2     
.
├── cache
│   ├── fontconfig
│   ├── mesa_shader_cache
│   ├── spotify
│   └── tmp
├── config
│   ├── glib-2.0
│   ├── ibus
│   ├── pulse
│   ├── spotify
│   └── user-dirs.dirs
└── data

Also, thanks for the prompt response!

dec05eba commented 2 months ago

That wouldn't work because the file is not removable by the user, especially if I want to make it work without having to install the flatpak system-wide in the future