This change is because 12 and 21 were consired valid inputs, rendering the installation useless (it would skip it).

Good catch. We need to fix this in other scripts as well. #1881

Using an en dash implies you can select both, I believe. So I changed to "or".

The dash is a relic of the same logic being used with more options (i.e. 1-5). That said, the "or" is clearer

Yes. The en dash implies multiple selections here, which is not the case.

This only reflects the last ptrace scope value set, but that is fine, because the ujust toggle removes any previous detected. What matters is the current value. If it's 0, it's also fine, since from 0 to 1 there is a security benefit, so the check is just for the value 1.

Why is this removed?

@RKNF404 See #1842 for rationale.

The purpose of that section is to allow the app to work with the permission lockdown. It also has a few improvements over what upstream offers. I dont see why it needs to be removed

I disagree with you. If the user hardens his Flatpak environment (perm lockdown), it's his responsability to unbreak any app, not ours.

It also has a few improvements over what upstream offers.

Yes, which can cause unexpected breakage. We should not be the ones dictating what Steam should and should not have access. The lockdown is either globally or for no app. I don't see a reason to treat Steam in a different manner.

If the user hardens his Flatpak environment (perm lockdown), it's his responsability to unbreak any app, not ours.

That is part of the purpose of this ujust. To bend the environment keep steam functional.

Yes, which can cause unexpected breakage

Edge cases, compared to the app not working at all.

We should not be the ones dictating what Steam should and should not have access. The lockdown is either globally or for no app. I don't see a reason to treat Steam in a different manner.

Could you elaborate on what you mean by this? Also, this doesn't point out any advantages to it being removed.

And, we do do it for other apps, mainly Flatseal and Warehouse for ease of use, which fits the exact same use-case of this ujust.

If you move the Steam Flatpak permissions to the harden flatpak command, this it's a bit less worse, but I still, in principle, disagree with that. It's just too many permissions for us to deal with.

In the case of Steam, we're basically re-doing all the permissions, whereas Flatseal and Warehouse are just some simpler ones.

Not really. The permissions aren't just selected because steam wont run without them, they are selected to fit steam's general operational boundaries, and again we are accounting for cases where it wont work so edge case breakage > completely dysfunctional.

I don't maintain the Steam Flatpak, and so don't you. We cannot know if/when a permission is needed, so we should upstream, and not make arbitrary judgements of ours, when they may be incorrect.

Also, if a user has hardened his Flatpak, he may not even want to give the permissions we give. For example, we give access to devel, required for anti-cheats to work, but in a context of hardened flatpak environment, with anti-cheat support not enabled, we would be exposing more attack surface for a user that doesn't want to have that. I prefer to let the user himself handle this, since he'll then learn more about permissions and know which ones are required, which ones do what etc.

If a user doesn't harden his Flatpak env., we also keep that file unnecessarily in the Flatpak overrides folder.

This is the case with any configuration, also, unlikely unless steam changes how it functions.

Yes, but Steam has many configs that we override, and not just one or two.

Unlikely does not mean impossible, and Steam is not a focus of secureblue, nor should it be. The idea here is to make a simple request that is future-proof. Flatpak permissions are not future-proof.

Could you elaborate on this? I'm not sure what you mean

If upstream gets a permission change, we'd have to update our script to reflect that, but that won't automatically update users' overrides. We would have to make an announcement just for that, and remind users to re-apply it themselves. It would also be a bit terrible to do that, since it would only get applied if you run the command to install Steam again, which doesn't make sense.

Right, so there is minimum no benefit, maximum Steam works most of the time. I struggle to see the benefit in removing this.

Steam works most of the time should not be our target.
Steam working as expected, all of the time, should be our target.
Flatpak Steam by itself is already something the Valve developers don't really think it's that good due to some problems specific to that method of packaging. We wouldn't want to complicate that even more.

I don't see a point to mantaining something that has no benefit for the distro. Steam itself is already proprietary, our Flatpak permissions will just make us different from other users that use the same Flatpak, potentially creating more friction due to things we cannot control.

Most of these points don't apply if it is conditional.

I disagree. They still apply; they just come with a question before getting applied.

I think this change should be moved to a separate PR to be discussed further, so that it isn't blocking other changes in this PR.
Because I don't see much benefit to this change currently.

I disagree. Feel free to use the issue to discuss it.

There's no noticeable benefit to keep it, and many downsides.

If you want to, feel free to make it an option (with a question) to the Harden Flatpak command.

I disagree. Feel free to use the issue to discuss it.

This is the reason it should be a separate PR, like I said, to not gate the other changes in this PR.

There's no noticeable benefit to keep it, and many downsides.

I completely disagree, this change should be deferred.

If you want to, feel free to make it an option (with a question) to the Harden Flatpak command.

It doesn't belong in the scope of the Flatpak permission lockdown script. The point in that script isn't to manually pre-configure random apps. Steam configuration does fit in here. Hence my point, this needs more discussion.

Let's see what others think of making a separate topic to discuss. But, for now, with all due respect, I'll the PR as it is.

Reboot is only required if karg or ptrace are changed. Logout in the case of xwayland. This is why my version sets flags and only runs the reboot/logout prompt in those circumstances.

@spaceoden There is no way we can know if karg, ptrace and/or xwayland are changed, unless we change the scripts to add such detection, and even then it may not work as expected.

Requesting the user to reboot is simpler and safer too.

We just flip the relevant flag in the same code block where we run the karg/ptrace/xwayland changing command.

I respect that it is indeed simpler code to unconditionally request reboot, but telling users to reboot unnecessarily is a UX degradation.

As I understand it, the commands may still be ran, even if they result in nothing happening, so the detection would be invalid.

The karg, ptrace and xwayland commands all are contained within if blocks. They only run under certain conditions. You reassign a variable in the same if block, and you've tracked whether that block has run.

@spaceoden Right; I've readded the reboot condition.

In case of Xwayland, I think it's fine to reboot, since that's what the script uses.

You could use another flag (I called mine need_relog) and duplicate the if [[ ${need_reboot} == "true" ]]; then block in the else case after it.

But you don't need to implement this now. We can merge your current version that has xwayland requesting a reboot, and I can just offer a PR afterward that adds logic for requesting a logout instead.

Sure. You should then also change the Xwayland script itself to require a logout instead of reboot. Personally, I think reboots are fine and are more in line with how rpm-ostree works.

Maybe Xwayland only works in logout in non-atomic distribution models. A separate discussion should be created so we can take a deeper look into it. Thanks.

I agree, all scripts that recommend rebooting should prompt for it. #1876

Just like recommending reboots when unneeded, recommending reboots when logging out is enough is subpar.

Our xwayland toggle adjusts a systemd user config override (for KDE and Gnome at least). These are loaded when a user logs in. That's why logging out is sufficient to reset them. There's generally no difference between ostree and non-atomic distros regarding user services.

I don't think we should default to "yes" for enabling anti-cheat. It's a hardening reduction that's only required by few games.

I disagree. For instance, the game with most players that would require such a change is ARC Raiders on Steam, which uses EAC. ( Ref: https://steamdb.info/charts/ )

If an user is installing Steam, I would also expect him to play multiplayer games whenever he wants. Of course, he can toggle it later.

If you want, I can add an echo for "You can use the command command to toggle this later.", as to remind the user.

@spaceoden is correct

Right, this will be changed to default NO.

What is this? also, typo

The ######## are for better readability.

What do you mean by "this"?

Typo will be fixed, thanks.

please remove these, they aren't necessary

It's for better readability, as it makes each thing/toggle visually separate and easier to distinguish. Without those kind of titles, it's a bit hard to discern what is what easily.

Please remove this

It's for better readability, as it makes each thing/toggle visually separate and easier to distinguish. Without those kind of titles, it's a bit hard to discern what is what easily.

It's clutter and were no issues without this. Please remove it.

please remove these

It's for better readability, as it makes each thing/toggle visually separate and easier to distinguish. Without those kind of titles, it's a bit hard to discern what is what easily.

Compatibility issues? It doesn't start in this case. I think the previous text was fine.

I added something in order to explain to the user why Steam needs it.

Would Disabling hardened_malloc for Steam to ensure proper functionality. be good?

It's implied that it needs it for proper functionality, otherwise why would the script be disabling it?

It's still not clear to me why someone would ever answer no to this. This seems like a solution in search of a problem.

In advanced use cases where the use either doesn't want the script to automatically change his overrides, for either "keeping it clean" (my case) or for people that want even more granularity and granting permissions on an individual basis.

advanced users can simply manage things themselves.

This is very spammy, please remove all these unnecessary echoes.

It's to let the user know what will be run. Otherwise the user will have to inspect the script himself. I think it's fine, those only run if the user has agreed to the general overrides.

Some empty echos are for the newlines for better readability.

It's to let the user know what will be run. Otherwise the user will have to inspect the script himself.

No, this is not how this works. If we did this for every change we make, that would be completely unsustainable.

you are missing indentation here

these sub-questions can be asked up-front and then we only need to run a single command

That works too. I used kind of the GNOME approach where the more you interact with, the more it reveals, so that's why they are in sub-questions. I have no strong preference for either, but I'll do as requested.