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

Suggestion: Rename "Fallback to X11 windowing system" to communicate security implications #627

Open
ell1e opened this issue Dec 28, 2023 · 23 comments

Comments

@ell1e
Copy link

ell1e commented Dec 28, 2023

It seems as per this discussion that when "Fallback to X11 windowing system" is enabled, with any Wayland desktop session a sandboxed app will be actually truly contained even if it tries X11 shenanigans, since if at launch Wayland is available then unsafe X11 is automatically cut off. This means disabling "Fallback to X11 windowing system" is considerably less safe, which the naming doesn't seem to really convey.

Therefore, I'd suggest the following rename: "Fallback to X11 windowing system" -> "Only allow X11 access if Wayland unavailable"

I'm hoping this might make the implications clearer, both of what it means when it is disabled and the additional security of this option when enabled for Wayland desktop users.

As a side note, a ton of apps on Flathub have both Wayland and X11 enabled, but have this fallback-only option disabled. I have tried enabling it for some like Audacity, Element, or Bluefish, and not only does it lead to way better UI since many apps will otherwise ill-advisedly default to X11, but it doesn't seem to come with any downsides other than worse security. So I wonder if that is a common packaging mistake?

@rusty-snake
Copy link

This means disabling "Fallback to X11 windowing system" is considerably less safe, which the naming doesn't seem to really convey.

First half: It is less secure.
Second half: If you leave "X11 windowing system" on. Otherwise it makes no difference.

The documentation (F1 or "Hamburger Menu") explains it. However the documentation is not easy to discover.

Related topic, FYI: 99% of the time when a flatpak uses share=ipc this is only required for some X11 shared memory.

Therefore, I'd suggest the following rename: "Fallback to X11 windowing system" -> "Only allow X11 access if Wayland unavailable"

Shorter suggestion: "X11 windowing system if Wayland windowing system is unavailable".
No answer to the if, just the how in case we do.

So I wonder if that is a common packaging mistake?

General you can go to the repository where the flatpak manifest is located (usually https://github.com/flathub), search if this has already be discussed and, if not, suggest the permission change.

If a flatpak that works (for you) with Wayland has only x11 but not wayland this is either due to some known problems with Wayland or because it was forgotten to add when the flatpak manifest was created (trial and error until it works(= shows a window)) or when the program added support for Wayland.

If a flatpak that works (for you) with Wayland has both x11 and wayland rather than fallback-x11 and wayland it is a "soft violation" of the best practice from below links (there are exceptions where programs actually need both).

@ell1e
Copy link
Author

ell1e commented Dec 28, 2023

I personally think a word like "only" is vital. It's not clear that this somehow overrides the X11 windowing system general option IMHO, unless the fallback option has a clearly restriction-indicating wording like, "X11 window system only if".

@ell1e
Copy link
Author

ell1e commented Dec 28, 2023

If a flatpak that works (for you) with Wayland has both x11 and wayland rather than fallback-x11 and wayland it is a "soft violation"

Yes, exactly, that seems to be very common, and out of a bunch of apps I tested, only one broke if I enabled the fallback restriction (which was Avidemux). All the others started up fine, so it seems to be an oversight that it's not enabled.

@flexagoon
Copy link

disabling "Fallback to X11 windowing system" is considerably less safe

Wouldn't that be only if the regular x11 permission is enabled? In my experience, most apps usually have either x11 or x11-fallback enabled, not both, so disabling X11 fallback means that app will never use X11

I wonder if that is a common packaging mistake?

If you tested the app and it works fine with X11 access disabled, you can create an issue/PR in the repository of that specific Flatpak package

@ell1e
Copy link
Author

ell1e commented Jan 26, 2024

Most apps on flathub for me seem to be having x11 enabled on flathub, usually always when x11 fallback is set. Maybe this is in part due to the confusing naming? It seems like nobody else really seems to fully understand that either.

I'm suggesting that it's not clear with x11 & x11 fallback also enabled what exactly is happening, as a non-advanced user. (I know now from this ticket, but I didn't before.) I feel like it's easy to interpret like it has full x11 access, through the x11 base permission, and then even more through the x11 fallback one somehow. It's not clear the fallback is a restriction in this case and that disabling it will grant more permissions, hence me suggesting a different name.

@flexagoon
Copy link

Does the x11-fallback permission take precedence over the regular x11 permission? So when both are enabled, the app will still use Wayland by default?

@ell1e
Copy link
Author

ell1e commented Jan 26, 2024

Oh, doesn't it? I thought it did! Now I don't even know anymore 😂

@flexagoon
Copy link

Ok, yes, it does:

--socket=fallback-x11 - show windows using X11, if Wayland is not available, overrides x11 socket permission

https://docs.flatpak.org/en/latest/sandbox-permissions.html

In that case I agree that the description should probably be changed to something like "Only use X11 when Wayland is unavailable" or "Use Wayland by default, fall back to X11"

@flexagoon
Copy link

The issue with having "only" in the name is that it makes it seem like a purely restrictive permission - but it's only restrictive when the regular x11 permission is enabled, otherwise it only adds extra permissions.

Maybe the UI should be changed to have an "Allow X11 windowing system" drop-down with the possible options being "Always", "When not using Wayland" and "Never"?

@ell1e
Copy link
Author

ell1e commented Jan 26, 2024

I like the drop-down suggestion a lot. I propose the option names "Always enabled", "Only when desktop session isn't using Wayland", "Always disabled". (Sorry if this isn't up for debate or change anyway, but I hope this input is useful to someone.)

@tchx84
Copy link
Owner

tchx84 commented Mar 27, 2024

@ell1e feel free to send a PR changing this particular description to something like Fallback to X11 windowing system if Wayland is unavailable.

@ell1e
Copy link
Author

ell1e commented Mar 27, 2024

I still think it should be named something like "Allow falling back to X11 but only if Wayland isn't available". Otherwise I don't think how it's less insecure than it currently sounds like is quite clear.

@flexagoon
Copy link

flexagoon commented Mar 27, 2024

@tchx84 what do you think about my three-way dropdown suggestion? That would indicate that it overrides the regular x11 permission, and would also avoid the problem of having a really long description.

The way it would map to Flatpak permissions is like so:

"Never":

  • x11 is always DISABLED
  • fallback-x11 is always DISABLED

"When not using Wayland":

  • x11 is set to default value
  • fallback-x11 is always ENABLED

"Always":

  • x11 is always ENABLED
  • fallback-x11 is always DISABLED

@ell1e
Copy link
Author

ell1e commented Mar 27, 2024

(I wasn't asked but: in my personal uninformed opinion that three options solution might work well. Maybe "When not using Wayland" would better be named "When Wayland is unavailable"? But I like it, I think it would be an improvement.)

@tchx84
Copy link
Owner

tchx84 commented Mar 27, 2024

@tchx84 what do you think about my three-way dropdown suggestion?

It's a good suggestion, when considering this specific case in isolation but, if Flatseal were to move its UI to a situational-inspired UI, it would simply not scale with all other possible situations / combinations.

EDIT: I think the best way of moving forward with this is to clarify the messaging, via a better description and documentation.

@ell1e
Copy link
Author

ell1e commented Apr 2, 2024

I see! My preferred order of things would be 1. this solution but maybe a clearer wording like if Wayland is unavailable, followed by 2. a text that very clearly uses "only if" like here, followed by 3. this slightly less specific (but shorter) text. I think any of these options improve the situation so I think any of them would be helpful. I hope this provides useful input, as unimportant as my personal thoughts as a random user are.

@Malix-Labs
Copy link

I didn't understand this setting before you proposed the new title

I think it would be nice to indeed change its title, or at least show a tooltip

@smcv
Copy link

smcv commented Jul 29, 2024

As a data point for this, a Flatpak user was recently sufficiently confused by the interaction of the fallback-x11 and x11 options that they mistakenly reported it as a Flatpak security vulnerability: based on what Flatseal shows, they assumed that their app was being given unconditional X11 access even though its manifest didn't explicitly ask for that, and even though that wasn't really true (testing ended up proving that Flatpak was working as intended).

As described in flatpak/flatpak#5881 (comment), Flatpak's CLI also presents this confusingly. It's really a tri-state, but the internal representation had to be chosen to provide backwards compatibility.

I think it would be better for Flatseal to represent these two permissions as a drop-down with 3 options: that's what is really going on, and I can't think of a non-confusing representation for these two permissions as two independent booleans. The least-bad representation with booleans that I can think of is that if fallback-x11 is enabled, then x11 could be greyed out? But that seems pretty confusing too.

For the record, the interaction between them is:

  • If fallback-x11 is "allowed", then it takes precedence over x11, and for backwards compatibility it also implies x11. Your app gets X11 access if and only if Wayland doesn't seem to be available.
  • If not, then we look at whether x11 is allowed; and based on that, either your app always gets X11 access, or never gets X11 access.

@flexagoon
Copy link

@smcv yes, that's what I suggested as well, but it seems like @tchx84 is against that. Although I still think this is the best way to do this, because IMO the way this permission is handled by Flatpak itself is wrong - it really should be a tri-state

@smcv
Copy link

smcv commented Jul 31, 2024

IMO the way this permission is handled by Flatpak itself is wrong - it really should be a tri-state

It effectively is a tri-state, but its representation in the metadata is weird. I don't like that, but it was a necessary evil: otherwise, we wouldn't have had backwards compatibility (and it would have taken years for app maintainers to feel that they could safely switch from x11 to fallback-x11).

@hfiguiere
Copy link
Contributor

I have tried enabling it for some like Audacity,

Audacity flatpak de-facto maintainer here. Inherently you are just demonstrating that allowing users to change random settings like that because "OMG it's unsafe" isn't a good idea. By disabling X11 from Audacity you basically disable the ability to display any of the audio plugin UI. This my also lead to crashes.

But I guess breaking features is part of the "choice".

@flexagoon
Copy link

@hfiguiere that's the point though, app maintainers enable all required permissions for the app to make it work for everyone, but users can disable some of the permissions if they don't need them

Eg. I don't use any audio plugins, so I don't sacrifice anything by disabling X11 and network for Audacity

@ell1e
Copy link
Author

ell1e commented Jul 31, 2024

It would be useful if apps like Audacity could store a comment on why certain permissions are needed that will then show up in flatseal. However, maybe it's just me, but that seems like a somewhat separate issue. The vast majority of apps doesn't seem to truly require X11 anymore these days.

Edit: maybe Audacity could detect this on startup and show some sort of warning? If it doesn't do that already.

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

No branches or pull requests

7 participants