Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update the flatpak to only use wayland, x11-fallback #284

Open
SnuggleCovenant opened this issue Sep 1, 2023 · 10 comments · May be fixed by flathub/net.davidotek.pupgui2#24
Open

update the flatpak to only use wayland, x11-fallback #284

SnuggleCovenant opened this issue Sep 1, 2023 · 10 comments · May be fixed by flathub/net.davidotek.pupgui2#24
Labels
enhancement New feature or request

Comments

@SnuggleCovenant
Copy link

Is your feature request related to a problem? Please describe.
the flatpak rates this app to be insecure partly because of the use of x11 by default which is insecure and allows, among other things, any other app currently running to play with the data pupgui uses in memory

Describe the solution you'd like
prioritize wayland, with x11-fallback for users without wayland

Describe alternatives you've considered
i've been running it as such on my side using flatseal

Additional context

@SnuggleCovenant SnuggleCovenant added the enhancement New feature or request label Sep 1, 2023
@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 1, 2023

ProtonUp-Qt does already use Wayland by default if it is available (checked using Flatseal, and also verified with xeyes), though as you pointed out it does not use fallback-x11.

image

Where did you see the warning, exactly? I can't say I have seen this for ProtonUp-Qt or other applications. Just curious :-)

@SnuggleCovenant
Copy link
Author

SnuggleCovenant commented Sep 1, 2023

It's nice to know. Thank you for checking. You can see the warning in flathub https://flathub.org/apps/net.davidotek.pupgui2

I didn't mean to close this. I just made a github account for this and I'm unfamiliar with the UI.

@SnuggleCovenant
Copy link
Author

reopen :S

@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 1, 2023

Ah, thanks, I didn't see this on Flathub.

This is interesting to see, aside from the X11 warning (or "legacy windowing system"), I'm interested in:

  • "Can acquire arbitrary permissions" - Do you know what this refers to?
  • "Can access hardware devices" - Unless I'm mistaken, ProtonUp-Qt shouldn't need to access anything like this (mics, speakers, etc) - I did see in the Flatpak manifest it specifies device=all but I'm not sure what this refers to.

Perhaps like with the X11 issue you brought up, this is just something ProtonUp-Qt has to mark?

The runtime issue was reported in #283 and will be fixed in flathub/net.davidotek.pupgui2#22. The warning

The remaining issues with file access are unavoidable really, as ProtonUp-Qt needs access specific files to accommodate game launchers. It also requires permissions to create some files like for SteamTinkerLaunch. You can see all the filesystem permissions here: https://github.com/flathub/net.davidotek.pupgui2/blob/master/net.davidotek.pupgui2.json#L12-L35


The reason I ask is that a "blanket PR" fixing up various Flatpak permissions in order to resolve as many of these warnings on Flathub as possible, would be more beneficial. Essentially discussing and fixing the Flatpak permissions generally :-)

@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 1, 2023

I tested very quickly with the X11 fallback flag (and with the X11 socket disabled), as well as by disabling devices=all, both toggled from Flatseal, and I have not noticed any problems so far.

I am not sure why Flathub is marking that ProtonUp-Qt can acquire arbitrary permissions, but aside from this, those two changes appear safe to make (pending further and much more in-depth testing).

@SnuggleCovenant
Copy link
Author

SnuggleCovenant commented Sep 1, 2023

I see "Can acquire arbitrary permissions" in gnome-software as well. I've found this discussion tangentially related: https://reddit.com/r/linux/comments/ybh241/comment/itm931g/?utm_source=share&utm_medium=web2x&context=3

An app is flagged as GS_APP_PERMISSIONS_ESCAPE_SANDBOX when the following permissions are requested:

I would venture that flathub uses the same reference.

@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 1, 2023

Thanks for looking into that, probably not much can bee done about that "acquire arbitrary permissions" thing then, as this :create is used for SteamTinkerLaunch: https://github.com/flathub/net.davidotek.pupgui2/blob/master/net.davidotek.pupgui2.json#L31 (as for the name GNOME Software uses, "escape_sandbox", ProtonUp-Qt already has to do this to run the STL install script on SteamOS, see here)

This concern was raised before in this issue: flathub/net.davidotek.pupgui2#17

I also found out that ProtonUp-Qt needs devices=all for gamepad support: flathub/net.davidotek.pupgui2#14

So probably the only actionable change in this issue is to set the X11 fallback instead of using X11 by default.

@DavidoTek
Copy link
Owner

Thanks for looking into that, probably not much can bee done about that "acquire arbitrary permissions" thing then, as this :create is used for SteamTinkerLaunch: https://github.com/flathub/net.davidotek.pupgui2/blob/master/net.davidotek.pupgui2.json#L31 (as for the name GNOME Software uses, "escape_sandbox", ProtonUp-Qt already has to do this to run the STL install script on SteamOS, see here)

Yes, thanks for clarifying that.

So probably the only actionable change in this issue is to set the X11 fallback instead of using X11 by default.

I removed fallback-x11 on purpose because there was a regression with Qt as it would try to launch with x11 even if only wayland was available. We could create a PR and test whether this behavior is fixed with newer version of Qt/Wayland

flathub/net.davidotek.pupgui2@0a4fd4e

@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 4, 2023

I tested (via Flatseal) enabling the fallback-x11 socket and disabling the x11 socket, and ProtonUp-Qt appears to work. Creating a PR to build a Flatpak and find this out would be useful too, I had this configuration enabled while doing some testing for another issue and didn't realise until now, so it appears to work but more thorough testing would be good.

It seems like OP has tested this too without much issue as well.

Was there an upstream Qt/PySide issue for this that we could check on to see if this regression was marked as fixed?

@sonic2kk
Copy link
Contributor

Just to document: This may be held back until #312 can be resolved, since that issue is a case where Wayland is causing a crash and fallback-x11 is required. Although manual intervention is still required in that case, if this was implemented, a user would have to switch off fallback-x11, which is an additional step to the current workaround which is simply to disable the wayland socket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants