Skip to content

Permission Manager: add permission management for screencopy - #9930

Merged
vaxerski merged 13 commits into
hyprwm:mainfrom
vaxerski:permission-mgr
Apr 8, 2025
Merged

Permission Manager: add permission management for screencopy#9930
vaxerski merged 13 commits into
hyprwm:mainfrom
vaxerski:permission-mgr

Conversation

@vaxerski

@vaxerski vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member

This adds a permission manager for denying access to screencopy. Obviously, it depends on hyprland-qtutils. Will not work and fall back to the old "take permissions with no questions asked" if it's missing.

As usual, Hyprland is the first environment to implement this.

Basically, every time an app wants to copy your screen, it will pop up a notification:
image

These permissions are configurable as well, and you can allow / deny globally, or per app.

Closes #9915

@jbeich for BSD: we use /proc/pid/exe to determine the app's binary path. This is not portable. Do you have any ideas for BSD ways to get a process' binary path / name?

Otherwise it's not a dealbreaker, users will just not be able to configure permissions per-app. Either globally, or every app will ask every time it's restarted.

TODO:

  • Default rules in the config + rules for xdph?
  • some testing
  • BSD stuff

Config keyword

permission = [app path], [permission], [mode]

path = empty for a wildcard, otherwise e.g. /usr/bin/grim
permission = screencopy for now
mode = ask, deny, allow

@fxzzi

fxzzi commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

https://paste.rs/3j8ZU

crash to tty for me when triggering something which requires permissions on NixOS. No permissions request window opens and instead I'm sent back to tty. Haven't added any of the new permission rules or anything, I simply added this PR as a patch.

Am I missing something or na

@vaxerski

vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

would be nice to give a crash report and not a log. Generally it shouldnt crash at all so ye

@fxzzi

fxzzi commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

would be nice to give a crash report and not a log. Generally it shouldnt crash at all so ye

mbmbmb your great software has been actually pretty stable so i forgot that crash reports even existed

hyprlandCrashReport106996.txt

lmk if you need a dbg one

@vaxerski

vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

try nao if not get debug

@fxzzi

fxzzi commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

working now thanks :)
Screenshot from 25 04 06 19:46:07

@vaxerski

vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

why are there two lol

@fufexan

fufexan commented Apr 6, 2025

Copy link
Copy Markdown
Member

Works fine on NixOS (built by changing the flake url to this branch).

@fxzzi

fxzzi commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

Non issue, I just pressed ma screenshot button twice without accepting the first one xd

I mean maybe yeah it would make sense to only show one popup for each executable but idk :P

@fufexan

fufexan commented Apr 6, 2025

Copy link
Copy Markdown
Member

I guess choosing "allow" or "deny" in the prompt are only persisted throughout the session lifetime, as I don't see anything being written in the hyprland config, nor in $XDG_CACHE_HOME/$XDG_DATA_HOME.

@vaxerski

vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

ah, right. Yes. Allow once means allow for this one app that is running until it exits.
Allow and remember will allow it for that executable for as long as hyprland is running

@fufexan

fufexan commented Apr 6, 2025

Copy link
Copy Markdown
Member

Useful for testing on NixOS

    permission = [
      # Allow xdph and grim
      "${config.programs.hyprland.portalPackage}/libexec/.xdg-desktop-portal-hyprland-wrapped, screencopy, allow"
      "${lib.getExe pkgs.grim}, screencopy, allow"
      # Optionally allow non-pipewire capturing
      "${lib.getExe pkgs.wl-screenrec}, screencopy, allow"
    ];

The xdph and grim lines will be added to the default config in the NixOS module. Possibly in the HM one as well, but it's harder to access the system config there so I'd rather leave that to the user.

@alba4k

alba4k commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

Where does this save the preference when choosing "Allow and remember app"? What is the point of having both the permission config keyword and this option, if they do the same thing?

edit: oh it doesn't survive after the session closes. Still, where is the preference saved?

@alba4k

alba4k commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

Also, would it be possible to have an option to get a notification (using hyprland's system) every time a permission is granted "behind the scenes"? That would prevent applications from just using grim to capture the screen without the used ever noticing. That, or a way to know if the screen is currently being shared (e.g. hyprctl permissionusage screencopy?).

I just noticed, when closing xdph's window asking for confirmation, no further requests will be made if I ever try to choose a new window (and the permission will never be obtained, even if I restart the service).

@outfoxxed

Copy link
Copy Markdown
Member

Instead of a black screen, we should put some text on it saying permission was denied so its obvious that something isn't broken if it happens unintentionally IMO.

Also, would it be possible to have an option to get a notification (using hyprland's system) every time a permission is granted "behind the scenes"?

If we were to do this I'd personally like to see it use the standard notification service or run a command.

@fufexan

fufexan commented Apr 6, 2025

Copy link
Copy Markdown
Member

If we were to do this I'd personally like to see it use the standard notification service or run a command.

Which may be broken/nonexistent, thus that should fallback to hyprerror.

@vaxerski

vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

Also, would it be possible to have an option to get a notification (using hyprland's system) every time a permission is granted "behind the scenes"? That would prevent applications from just using grim to capture the screen without the used ever noticing. That, or a way to know if the screen is currently being shared (e.g. hyprctl permissionusage screencopy?).

APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING
APP IS SCREENSHARING

Instead of a black screen, we should put some text on it saying permission was denied so its obvious that something isn't broken if it happens unintentionally IMO.

good idea.

@vaxerski

vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

for screensharing there already is a socket2 event.

You have to keep in mind that "screenshot" and "1 frame of a 60fps screenshare" is the same.

@alba4k

alba4k commented Apr 6, 2025

Copy link
Copy Markdown
Contributor

what I was mainly thinking of was the possibility of a custom waybar module that indicates if the screen is being shared. I guess I'll think baout it later

Btw the text on the message sounds a bit weird

An application hyprpicker (/usr/bin/hyprpicker) is requesting access to your screen (screenshot / screenshare). Do you want to allow it to?

wouldn't something like

The application hyprpicker (/usr/bin/hyprpicker) is requesting access to your screen (screenshot / screenshare). Do you want to allow it?

be better

@vaxerski

vaxerski commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

what I was mainly thinking of was the possibility of a custom waybar module that indicates if the screen is being shared. I guess I'll think baout it later

check the socket2 event

@github-actions github-actions Bot added the render label Apr 6, 2025
@ghost

ghost commented Apr 6, 2025

Copy link
Copy Markdown

hyprland is slowly becoming a desktop environment...

@vaxerski

vaxerski commented Apr 7, 2025

Copy link
Copy Markdown
Member Author

yep, thanks!

Comment thread src/managers/permissions/DynamicPermissionManager.cpp Outdated
@vaxerski
vaxerski merged commit 260d8e1 into hyprwm:main Apr 8, 2025
@Juhan280

Juhan280 commented Apr 8, 2025

Copy link
Copy Markdown

I have some concerns regarding the security of this permission system. I don’t know what the current behavior is since I can’t build it and see for myself at this moment. But I want to ask what happens when an app modifies the config file to give itself the permission or uses hyprctl or the Unix socket (I don’t know if it’s there or not). Since Hyprland live reloads, will it simply be able to bypass the permission system?

@vaxerski

vaxerski commented Apr 9, 2025

Copy link
Copy Markdown
Member Author

essentially yes. However, the point is not to stop targeted attacks at you specifically (in which case I'd say rm -rf ~ is more destructive at this point) but rather generic attacks. Especially with the adoption of the unified wayland extension for screencopy, I want to avoid a situation where someone can write a generic screen snooper in 20 lines like we could on X with a generic keylogger.

If you are targetted personally, you have different problems.

@Juhan280

Juhan280 commented Apr 9, 2025

Copy link
Copy Markdown

Yeah, rm -rf ~ is more destructive. But the point is not that. It's about stealing sensitive information from your screen.

Once people are aware of this feature, that same generic screen snooper, if it wants to include Hyprland users as its victims, can detect whether it's running in a Hyprland session or not and implement the workaround in under 10 lines.

@ErrorNoInternet

ErrorNoInternet commented Apr 9, 2025

Copy link
Copy Markdown
Contributor

could just make it require a restart to be changed (and show a warning banner if it was changed after startup), not perfect but at least an improvement

@Juhan280

Juhan280 commented Apr 9, 2025

Copy link
Copy Markdown

One thing we can do is ask the user what to do when permission-related configurations are changed. For example, just like how there is a popup when an app tries to access privileged resources, we could show a popup when a permission is modified. The popup could state what permissions were changed and maybe include a button to show all active permission rules.

Then, the user might have some options such as: Allow once, Deny once, Always Allow for this session (which is the current behavior), or Always Deny for this session.

This might also be configured using config file (e.g runtime_permission_modification = allow|ask|deny) but it shouldn't be automatically updated at runtime.

There should also be a way to monitor permission changes that were denied (e.g., by using Always Deny) via the Unix socket2 event or another method.

@alba4k

alba4k commented Apr 9, 2025

Copy link
Copy Markdown
Contributor

The way I see this is it shouldn't be thought of as a way to secure yourself from malicious software, but rather as a way to get some extra control over when you want to share your screen (so privacy, not security)

One thing we can do is ask the user what to do when permission-related configurations are changed.

what about a hyprland system notification? I believe those should ststz to be implemented more

@Juhan280

Juhan280 commented Apr 9, 2025

Copy link
Copy Markdown

The way I see this is it shouldn't be thought of as a way to secure yourself from malicious software, but rather as a way to get some extra control over when you want to share your screen (so privacy, not security)

Yeah, I am also talking about privacy. Suppose, you don't want any software to access your screen for privacy reasons, so you deny the screenshare rule. But the thing is, the current implementation has a security flaw which allows any software to nullify that rule using only a few lines of code. So yes, this is still related to privacy.

what about a hyprland system notification? I believe those should ststz to be implemented more

Regarding that, I am not sure. I just gave a suggestion about what could be done. If you find it better and it works fine, I have no issues with it.
But I gotta ask, will using both hyprland notification and the system notification at the same time not seem out of place?

@vaxerski

vaxerski commented Apr 9, 2025

Copy link
Copy Markdown
Member Author

This might also be configured using config file (e.g runtime_permission_modification = allow|ask|deny) but it shouldn't be automatically updated at runtime.

Sounds reasonable, make a discussion

@Juhan280

Copy link
Copy Markdown

Someone else feel free to open the discussion (I don't even know what's right category for this)

If nobody does, I may open it in a few days

@Kommynct

Kommynct commented Apr 10, 2025

Copy link
Copy Markdown

One thing we can do is ask the user what to do when permission-related configurations are changed. For example, just like how there is a popup when an app tries to access privileged resources, we could show a popup when a permission is modified. The popup could state what permissions were changed and maybe include a button to show all active permission rules.

Then, the user might have some options such as: Allow once, Deny once, Always Allow for this session (which is the current behavior), or Always Deny for this session.

This might also be configured using config file (e.g runtime_permission_modification = allow|ask|deny) but it shouldn't be automatically updated at runtime.

There should also be a way to monitor permission changes that were denied (e.g., by using Always Deny) via the Unix socket2 event or another method.

Wouldn't the obvious workaround be to modify the permission and that setting and then kill the session?

You'd be incentivized to kill the session even with a notification, that would prevent the user from knowing and they might think the session just randomly crashed.

@Juhan280

Copy link
Copy Markdown

Yes, except it can no longer do that silently unlike before. And you would have the chance to investigate it

If you have any better solution, feel free to state it

@Kommynct

Kommynct commented Apr 10, 2025

Copy link
Copy Markdown

Yes, except it can no longer do that silently unlike before. And you would have the chance to investigate it

If you have any better solution, feel free to state it

I just don't think it'd accomplish much beyond making things less convenient to enable runtime_permission_modification if a malicious client already has a reason to just kill the session to prevent the user from seeing the notification, no? Might be better to spend that effort elsewhere.

I don't think there's a good workaround as long as scripts can freely call hyprctl dispatch exit. They'd need to be hyprland specific to abuse the permissions already, so what would this really accomplish?

Maybe hyprland should notify on launch what killed it last, I dunno, I don't think that's possible either.

@3l0w 3l0w mentioned this pull request Apr 10, 2025
9 tasks
@ErrorNoInternet

Copy link
Copy Markdown
Contributor

At that point you might as well steal the user's files in their home directory, if you have malicious code running in the first place it's no longer Hyprland's responsibility to safeguard itself from every possible attack

@Kommynct

Copy link
Copy Markdown

At that point you might as well steal the user's files in their home directory, if you have malicious code running in the first place it's no longer Hyprland's responsibility to safeguard itself from every possible attack

Sure, i'm just saying that the permissions dialog already does the job fully, considering bypassing that would require a hyprland specific exploit already, bypassing runtime_permission_modification would be completely trivial, making that setting pointless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement permission controls for non-portal screencopy sessions

10 participants