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

[Bug]: Tests fail when using malcontent and system bus not available #5607

Closed
4 tasks done
dbnicholson opened this issue Nov 29, 2023 · 6 comments
Closed
4 tasks done
Labels

Comments

@dbnicholson
Copy link
Contributor

Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for a bug that matches the one I want to file, without success.
  • If this is an issue with a particular app, I have tried filing it in the appropriate issue tracker for the app (e.g. under https://github.com/flathub/) and determined that it is an issue with Flatpak itself.
  • This issue is not a report of a security vulnerability (see here if you need to report a security issue).

Flatpak version

1.14.4

What Linux distribution are you using?

Endless OS

Linux distribution version

5.2

What architecture are you using?

x86_64

How to reproduce

  1. Build flatpak with malcontent
  2. Run the tests in an environment where the D-Bus system bus isn't available. I'm doing this in a container, but I think you could also set DBUS_SYSTEM_BUS_ADDRESS to a bogus value.

Expected Behavior

Tests pass or skip if the system bus isn't available.

Actual Behavior

The tests that try to launch flatpaks fail because the parental controls check fails if the system bus can't be connected to. If flatpak isn't built with libmalcontent, that's all skipped.

Additional Information

There's already a FLATPAK_SYSTEM_HELPER_ON_SESSION check that was added for the same purpose to skip install tests. It seems a bit out of place for skipping run tests, but maybe that's fine. It's also generally ugly to allow bypassing your access control system by simply setting an environment variable. #2993 exists for a more thorough handling.

@dbnicholson
Copy link
Contributor Author

I think these are the potential options.

  1. Fix Add mock parental controls implementation for unit tests #2993.
  2. Start a temporary system bus during the tests since the tests will succeed if the bus is available but the service isn't.
  3. Reuse the FLATPAK_SYSTEM_HELPER_ON_SESSION environment variable in the run check and allow if the system bus isn't available and it's set.
  4. Allow running the app when the system bus isn't available. Initially I was opposed to that, but the app is also allowed to run when the malcontent D-Bus service isn't available. I don't know if there's really much difference between the service being unavailable and the whole system bus being unavailable, although you can then bypass parental controls by setting a bogus DBUS_SYSTEM_BUS_ADDRESS environment variable.
  5. Make the tests skip the run portions when malcontent is being used but the system bus isn't available.

@wjt
Copy link
Contributor

wjt commented Nov 29, 2023

I think you can already bypass parental controls by setting DBUS_SYSTEM_BUS_ADDRESS to a bus that is not the system bus, like $DBUS_SESSION_BUS_ADDRESS. In that case, the code will hit the G_DBUS_ERROR_SERVICE_UNKNOWN path which fails open.

@smcv
Copy link
Collaborator

smcv commented Nov 29, 2023

The whole parental-controls thing is more speed-bump than anything else. The original PR said:

it’s always going to be possible to bypass this unless the system is entirely locked down (LSM, secure boot, only allowing the child to use the computer under supervision, etc.).

The idea is that this needs to not accidentally fail (for example, due to the appstream being temporarily unavailable), and it needs to be too hard for a moderately technically able child to bypass. Any child who can use a terminal is probably out of scope.

See also #5076.

@smcv
Copy link
Collaborator

smcv commented Nov 29, 2023

#5076 (comment) has another option for dealing with this: basically hard-code the well-known location of the system bus, and refuse to run without successfully accessing the system bus if it appears that we ought to be able to access it.

@dbnicholson
Copy link
Contributor Author

It definitely is hard to integrate with testing. I'm hitting exactly #5076. Sorry I didn't notice it before.

I did start working on something like checking for the existence of the socket, but I was only scoping it to tests. However, given that you can already fake this out by setting DBUS_SYSTEM_BUS_ADDRESS to any dbus-daemon you start and it will happily allow the app since there will be no malcontent service running (actually it fails earlier because accounts-service isn't available), perhaps the failure to connect to the system bus should just fail open all the time.

To wit:

$ DBUS_SYSTEM_BUS_ADDRESS=$DBUS_SESSION_BUS_ADDRESS flatpak run -v --command=echo org.gnome.Calculator success
F: No installations directory in /etc/flatpak/installations.d. Skipping
F: Opening system flatpak installation at path /var/lib/flatpak
F: Opening user flatpak installation at path /home/dan/.local/share/flatpak
F: Opening user flatpak installation at path /home/dan/.local/share/flatpak
F: Opening system flatpak installation at path /var/lib/flatpak
F: Skipping parental controls check for app/org.gnome.Calculator/x86_64/stable since parental controls are disabled globally
F: Opening user flatpak installation at path /home/dan/.local/share/flatpak
F: Opening system flatpak installation at path /var/lib/flatpak
F: Regenerating ld.so.cache /home/dan/.var/app/org.gnome.Calculator/.ld.so/72311c424147f6d084a54a17d1340b0700fc4cc4f3bd4494f8c0435b42046657
F: /var/lib/flatpak/runtime/org.gnome.Platform/x86_64/45/1d2947259cdada54ac757fb46f076d40b60451b0395d050cea45c23e7a80d997/files/lib32 does not exist
F: Running: 'bwrap --args 24 ldconfig -X -C /run/ld-so-cache-dir/72311c424147f6d084a54a17d1340b0700fc4cc4f3bd4494f8c0435b42046657.sWgiHI'
F: /var/lib/flatpak/runtime/org.gnome.Platform/x86_64/45/1d2947259cdada54ac757fb46f076d40b60451b0395d050cea45c23e7a80d997/files/lib32 does not exist
F: Cleaning up unused container id 3639734983
F: Cleaning up per-app-ID state for org.gnome.Boxes
F: Allocated instance id 308003684
F: Add values in dir '/org/gnome/calculator/', prefix is '/org/gnome/Calculator/'
F: Add defaults in dir /org/gnome/Calculator/
F: Add locks in dir /org/gnome/Calculator/
F: writing D-Conf values to /home/dan/.var/app/org.gnome.Calculator/config/glib-2.0/settings/keyfile
F: Allowing dri access
F: Allowing wayland access
F: Running 'bwrap --args 39 xdg-dbus-proxy --args=41'
F: Running 'bwrap --args 39 echo success'
success

@dbnicholson
Copy link
Contributor Author

Closing this as a duplicate of #5076.

@dbnicholson dbnicholson closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants