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

Backport regression fixes from 1.10.3 into 1.8.x #4474

Merged
merged 8 commits into from
Jan 21, 2022

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented Oct 11, 2021

@debarshiray, you might want these if you are doing a new 1.8.x release for RHEL. They fix regressions that happened when we fixed CVE-2021-21261 in 1.8.5.

I haven't tested this, perhaps @debarshiray could take responsibility for that?

This branch is based on #4456, please review that first.


  • Fix build with GCC 11

    From: Michael Catanzaro

    See:
    https://gitlab.gnome.org/GNOME/glib/-/commit/fab561f8d05794329184cd81f9ab9d9d77dcc22a
    (cherry picked from commit 9b34768)

  • portal: Don't leak fd used for serialized environment

    Otherwise we'll run out of file descriptors eventually, when starting
    a sufficiently large number of subsandboxes.

    Resolves: portal: --env-fd is leaked #4285
    Fixes: aeb6a7a "portal: Convert --env in extra-args into --env-fd"
    (cherry picked from commit f2fbc75)
    (cherry picked from commit b4c6aa1)

  • portal: Use a GArray to store fds

    This will allow us to add additional mapping entries for fds to be
    used internally by flatpak run, in particular --env-fd.

    Defer the second pass through the fd array until the last possible
    moment, so that any extra fds we want to add (like the --env-fd) have
    already been added by then.

    Helps: portal: --env-fd can collide with fds passed across D-Bus #4286
    (cherry picked from commit a09d07f)
    (cherry picked from commit 77b484c)

  • portal: Remap --env-fd into child process's fd space

    Just because we can allocate a new, unused fd in the portal's fd space,
    that doesn't mean that fd number is going to be unused in the child
    process's fd space: we might need to remap it.

    Resolves: portal: --env-fd can collide with fds passed across D-Bus #4286
    Fixes: aeb6a7a "portal: Convert --env in extra-args into --env-fd"
    (cherry picked from commit 526dae9)
    (cherry picked from commit 101a3c5)

  • tests: Remove hard-coded references to x86_64

    Distributions run these tests on other architectures, but hard-coding
    x86_64 to look for in output dooms that to failure.

    (cherry picked from commit ba381ae)
    (cherry picked from commit 4089b69)

  • run: Don't let XDG_RUNTIME_DIR from user override the value we set

    We use bwrap --setenv XDG_RUNTIME_DIR to set it to /run/user/UID,
    regardless of what it is on the host system, but the changes made
    to resolve CVE-2021-21261 unintentionally broke this by overwriting it
    with the user's XDG_RUNTIME_DIR.

    In practice this worked for most people, who either have
    XDG_RUNTIME_DIR set to the same value we use (which is the conventional
    setup from systemd-logind and elogind), or entirely unset (if they do not
    have systemd-logind or elogind). However, it broke Wayland and other
    XDG_RUNTIME_DIR-based protocols for people who intentionally set up an
    XDG_RUNTIME_DIR that is different.

    Fixes: 6d1773d "run: Convert all environment variables into bwrap arguments"
    Resolves: XDG_RUNTIME_DIR not set inside of sandbox #4372
    (cherry picked from commit d3e6e71)

  • tests: Don't reset XDG_RUNTIME_DIR locally

    If we do, it interferes with xdg-dbus-proxy, causing test failure under
    some circumstances: the test passes on a development system, but fails
    when run on a qemu virtual machine in Debian's autopkgtest framework.

    Fixes: 6e5b02e "run: Don't let XDG_RUNTIME_DIR from user override the value we set"
    (cherry picked from commit 7bf6ecf)
    (cherry picked from commit 9c12cb4)

  • Update NEWS

@smcv smcv marked this pull request as draft October 11, 2021 13:53
@smcv smcv force-pushed the 1.8-regression-fix-backports branch from 0ce8311 to 00f4cc3 Compare October 26, 2021 14:57
@smcv smcv marked this pull request as ready for review October 26, 2021 14:57
@smcv
Copy link
Collaborator Author

smcv commented Oct 26, 2021

I haven't tested this, perhaps @debarshiray could take responsibility for that?

Still true.

@smcv smcv force-pushed the 1.8-regression-fix-backports branch from 00f4cc3 to 2bf057b Compare October 26, 2021 15:29
mcatanzaro and others added 8 commits October 26, 2021 16:29
Otherwise we'll run out of file descriptors eventually, when starting
a sufficiently large number of subsandboxes.

Resolves: flatpak#4285
Fixes: aeb6a7a "portal: Convert --env in extra-args into --env-fd"
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit f2fbc75)
(cherry picked from commit b4c6aa1)
This will allow us to add additional mapping entries for fds to be
used internally by `flatpak run`, in particular --env-fd.

Defer the second pass through the fd array until the last possible
moment, so that any extra fds we want to add (like the --env-fd) have
already been added by then.

Helps: flatpak#4286
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit a09d07f)
(cherry picked from commit 77b484c)
Just because we can allocate a new, unused fd in the portal's fd space,
that doesn't mean that fd number is going to be unused in the child
process's fd space: we might need to remap it.

Resolves: flatpak#4286
Fixes: aeb6a7a "portal: Convert --env in extra-args into --env-fd"
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 526dae9)
(cherry picked from commit 101a3c5)
Distributions run these tests on other architectures, but hard-coding
x86_64 to look for in output dooms that to failure.

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit ba381ae)
(cherry picked from commit 4089b69)
We use `bwrap --setenv XDG_RUNTIME_DIR` to set it to `/run/user/UID`,
regardless of what it is on the host system, but the changes made
to resolve CVE-2021-21261 unintentionally broke this by overwriting it
with the user's XDG_RUNTIME_DIR.

In practice this worked for most people, who either have
XDG_RUNTIME_DIR set to the same value we use (which is the conventional
setup from systemd-logind and elogind), or entirely unset (if they do not
have systemd-logind or elogind). However, it broke Wayland and other
XDG_RUNTIME_DIR-based protocols for people who intentionally set up an
XDG_RUNTIME_DIR that is different.

Fixes: 6d1773d "run: Convert all environment variables into bwrap arguments"
Resolves: flatpak#4372
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d3e6e71)
If we do, it interferes with xdg-dbus-proxy, causing test failure under
some circumstances: the test passes on a development system, but fails
when run on a qemu virtual machine in Debian's autopkgtest framework.

Fixes: 6e5b02e "run: Don't let XDG_RUNTIME_DIR from user override the value we set"
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 7bf6ecf)
(cherry picked from commit 9c12cb4)
Signed-off-by: Simon McVittie <smcv@collabora.com>
@debarshiray
Copy link
Contributor

Okay, these patches look good to me!

I built and smoke tested them out on RHEL >= 8.4 by:

  • flatpak uninstall --all
  • flatpak install flathub org.gnome.Weather
  • flatpak run org.gnome.Weather
  • flatpak install flathub org.gnome.design.Palette
  • flatpak run org.gnome.design.Palette

I'd like to cut a new release from the flatpak-1.8.x branch, and once that's done, the RHEL QE team will be able to give them a very thorough shake.

@smcv
Copy link
Collaborator Author

smcv commented Jan 21, 2022

Unfortunately, 1.8.x is also vulnerable to CVE-2021-43860, and if RHEL ships flatpak-builder, it's probably vulnerable to CVE-2022-21682 (which needs a new Flatpak feature to fix). Do you want to do a 1.8.6 without fixing those, then a 1.8.7 afterwards to fix those, or what?

I tried to backport patches for those to 1.2.x in #4674 and #4672, but I'm not 100% sure my backports are correct, particularly CVE-2021-43860. Some of the conflicts won't be a problem for 1.8.x because they're a result of 1.7.x work, but some involve changes that happened in 1.9.x.

@smcv
Copy link
Collaborator Author

smcv commented Jan 21, 2022

Unfortunately, 1.8.x is also vulnerable to CVE-2021-43860, and if RHEL ships flatpak-builder, it's probably vulnerable to CVE-2022-21682 (which needs a new Flatpak feature to fix)

... never mind, I see you have a PR.

@debarshiray
Copy link
Contributor

Unfortunately, 1.8.x is also vulnerable to CVE-2021-43860, and if
RHEL ships flatpak-builder, it's probably vulnerable to CVE-2022-21682
(which needs a new Flatpak feature to fix)

Yes, I am working through the backports for flatpak in #4697 Once that's done, I will start with flatpak-builder.

Do you want to do a 1.8.6 without fixing those, then a 1.8.7 afterwards to fix
those, or what?

Actually, if you are up for rolling a 1.8.6 before #4697 is ready, then that would be the best option for me. In that case, a 1.8.7 afterwards for #4697

It would let me parallelize some parts of the RHEL pipeline, and if there is QE feedback, we would be able to address those in 1.8.7.

@alexlarsson
Copy link
Member

I can do a 1.8.6 release now.

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

Successfully merging this pull request may close these issues.

None yet

4 participants