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

Regression in CVE-2021-41133 fixes: While trying to apply extra data: Failed to block syscall 442 #4458

Closed
smcv opened this issue Oct 8, 2021 · 10 comments · Fixed by #4460
Closed
Labels

Comments

@smcv
Copy link
Collaborator

smcv commented Oct 8, 2021

Linux distribution and version

Debian 11 (kernel 5.10.46-4, libseccomp 2.5.1-1); Ubuntu 20.04 (kernel 5.11.0-27.29~20.04.1, libseccomp 2.5.1-1ubuntu1~20.04.1)

Flatpak version

1.12.0

Description of the problem

Installing extensions that have extra data (openh264, NVIDIA) fails with:

While trying to apply extra data: Failed to block syscall 442

Steps to reproduce

  • Install flatpak from the PPA on a fresh Ubuntu 20.04 VM (the live desktop session will do)
  • Add Flathub as a remote
  • Install e.g. org.gnome.Weather
@smcv
Copy link
Collaborator Author

smcv commented Oct 8, 2021

Debian testing/unstable, with kernel 5.14.9-2 and libseccomp 2.5.2-2, is OK.

@smcv
Copy link
Collaborator Author

smcv commented Oct 8, 2021

I think what's happening here is that adding seccomp rules fails if non-native architectures are allowed (so this will break extra-data, and also apps that allow non-native syscalls like Steam) and we are trying to block a syscall that libseccomp doesn't know about.

We can block a syscall by its native syscall number for the current architecture, even if the syscall is not known to libseccomp; but if we have enabled multiarch, arch_syscall_translate() will fail to translate it from syscall number to name, and then back to a syscall number for the other architecture.

Making the seccomp filter into an allowlist instead of a denylist (which we want to do anyway, as security hardening) would solve this, but I don't know libseccomp well enough to work out how best to do that.

@alexlarsson
Copy link
Member

Is that really true? It seems to me that arch_syscall_translate() failing will return EFAULT, which is specifically ignored by flatpak as:

      if (r < 0 && r == -EFAULT /* unknown syscall */)
        return flatpak_fail_error (error, FLATPAK_ERROR_SETUP_FAILED, _("Failed to block syscall %d"), scall);

@smcv
Copy link
Collaborator Author

smcv commented Oct 8, 2021

Notice that it says r == -EFAULT there, and not the r != EFAULT that I think you might be reading it as (and might have intended)...

@smcv
Copy link
Collaborator Author

smcv commented Oct 8, 2021

If we fail to block the i386 version of a recently-added syscall, then we're potentially reintroducing CVE-2021-41133 for users with a newer kernel than their libseccomp, for example Debian 11 users with a backported Debian 12 kernel (which is quite a common thing to do for newer hardware support).

@alexlarsson
Copy link
Member

Looking at this, I think you're right that the intention was to say r != -EFAULT. Can you try if that fixes it for you?

I agree however, that this is sub-optimal in terms of security.

@smcv
Copy link
Collaborator Author

smcv commented Oct 8, 2021

Can you try if that fixes it for you?

Not trivially, I don't have a permanent development environment for all these older distros. I'll try to get a test-build done but it is going to take a little while.

smcv added a commit to smcv/flatpak that referenced this issue Oct 8, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
smcv added a commit to smcv/flatpak that referenced this issue Oct 8, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
@mwleeds
Copy link
Collaborator

mwleeds commented Oct 8, 2021

Making the seccomp filter into an allowlist instead of a denylist (which we want to do anyway, as security hardening) would solve this, but I don't know libseccomp well enough to work out how best to do that.

Maybe you already saw, but there is a runc commit linked to from the moby commit you linked in the flatpak source, and they seem to have managed to implement an allowlist by returning ENOSYS for syscall numbers higher than the highest one defined:
opencontainers/runc@7a8d716#diff-6446b89ce3c220c1a94143ecf944a5655982d444ec30b41cf7c9f74487614fe3R261

As pointed out in their commit and source comment, there are various caveats and limitations, but it might still be better than a blocklist.

(I'm not suggesting that translating that prior art into C code for Flatpak is easy, just wanted to point that out in case you didn't see.)

@mwleeds mwleeds added the bug label Oct 8, 2021
alexlarsson pushed a commit that referenced this issue Oct 8, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: #4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
alexlarsson pushed a commit that referenced this issue Oct 8, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: #4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
@smcv
Copy link
Collaborator Author

smcv commented Oct 8, 2021

they seem to have managed to implement an allowlist by returning ENOSYS for syscall numbers higher than the highest one defined

Sorry, writing code to output raw BPF is way outside my knowledge - I'm already having to learn libseccomp as I go (which probably makes me not an ideal person to be implementing this security boundary, but hopefully I'm better than nothing). If you want to try their approach, please go ahead, but it is not something that I will be able to help you with.

@smcv
Copy link
Collaborator Author

smcv commented Oct 8, 2021

See also seccomp/libseccomp#11 and seccomp/libseccomp#286 which seem to be attempts to solve this in libseccomp. (Although each of those two issues suggests that the other one is the real solution...)

alexlarsson pushed a commit that referenced this issue Oct 8, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: #4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
smcv added a commit to smcv/flatpak that referenced this issue Oct 11, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
ahayzen pushed a commit to ahayzen/flatpak that referenced this issue Oct 19, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
ahayzen pushed a commit to ahayzen/flatpak that referenced this issue Oct 20, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
smcv added a commit that referenced this issue Oct 26, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: #4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Nov 2, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Nov 2, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Nov 2, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Nov 2, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Nov 3, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Nov 3, 2021
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Jan 26, 2022
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Jan 27, 2022
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
debarshiray pushed a commit to debarshiray/flatpak that referenced this issue Jan 27, 2022
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6)
(cherry picked from commit 270701f)
(cherry picked from commit a0055e4)
ahayzen pushed a commit to ahayzen/flatpak that referenced this issue Jan 27, 2022
The error-handling here was

    if (r < 0 && r == -EFAULT)

but Alex says it was almost certainly intended to be

    if (r < 0 && r != -EFAULT)

so that syscalls not known to libseccomp are not a fatal error.

Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.

This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.

Resolves: flatpak#4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants