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

Add /proc/acpi to masked paths #37404

Merged
merged 1 commit into from Jul 6, 2018

Conversation

Projects
None yet
7 participants
@runcom
Member

runcom commented Jul 6, 2018

The deafult OCI linux spec in oci/defaults{_linux}.go in Docker/Moby
from 1.11 to current upstream master does not block /proc/acpi pathnames
allowing attackers to modify host's hardware like enabling/disabling
bluetooth or turning up/down keyboard brightness and probably other stuff. SELinux prevents all
of this if enabled. Probably apparmor does prevent this as well but I haven't tested.

This is reproducible with just:

docker run --rm -ti alpine sh -c 'echo "2" >/proc/acpi/ibm/kbdlight' (turns up the brightness of the kbd)
docker run --rm -ti alpine sh -c 'echo "enable" >/proc/acpi/ibm/bluetooth' (enable bluetooth)

We've reserved CVE-2018-10892 for this.

@thaJeztah @cyphar et all, PTAL

Signed-off-by: Antonio Murdaca runcom@redhat.com

Add /proc/acpi to masked paths
The deafult OCI linux spec in oci/defaults{_linux}.go in Docker/Moby
from 1.11 to current upstream master does not block /proc/acpi pathnames
allowing attackers to modify host's hardware like enabling/disabling
bluetooth or turning up/down keyboard brightness. SELinux prevents all
of this if enabled.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

@runcom runcom requested a review from tonistiigi Jul 6, 2018

@cyphar

This comment has been minimized.

Contributor

cyphar commented Jul 6, 2018

Probably apparmor does prevent this as well but I haven't tested.

AppArmor does protect against this (it protects against all writes to /proc files unless they are inside /proc/sys/ or /proc/<pid>/). I've tested this on openSUSE Tumbleweed and /proc/acpi writes are disallowed inside containers.

LGTM.

@runcom

This comment has been minimized.

Member

runcom commented Jul 6, 2018

failures seems to be unrelated

@n4ss

This comment has been minimized.

n4ss commented Jul 6, 2018

LGTM!

@n4ss

n4ss approved these changes Jul 6, 2018

@cpuguy83

LGTM

@andrewhsu

LGTM

@thaJeztah

LGTM

@codecov

This comment has been minimized.

codecov bot commented Jul 6, 2018

Codecov Report

Merging #37404 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #37404      +/-   ##
==========================================
+ Coverage   34.91%   34.93%   +0.01%     
==========================================
  Files         610      610              
  Lines       44884    44884              
==========================================
+ Hits        15672    15679       +7     
+ Misses      27092    27087       -5     
+ Partials     2120     2118       -2

@thaJeztah thaJeztah merged commit 86a41e4 into moby:master Jul 6, 2018

8 checks passed

codecov/patch Coverage not affected when comparing b322705...569b970
Details
codecov/project 34.93% (+0.01%) compared to b322705
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 41241 has succeeded
Details
janky Jenkins build Docker-PRs 50000 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 10423 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 21349 has succeeded
Details
z Jenkins build Docker-PRs-s390x 10308 has succeeded
Details

@runcom runcom deleted the runcom:no-acpi branch Jul 6, 2018

@n4ss

This comment has been minimized.

n4ss commented Jul 7, 2018

Additional Information on CVE-2018-10892

Since this is linked from the CVE (which I'll make sure we amend and add additional documentation because the description shows extremely small understanding of what ACPI is and can be used for, as well as the use of the procfs interface for this purpose) and people will visit this issue, a few key points:

  1. Both SELinux and AppArmor with the default profiles protect against this (if you use an official Docker packaging and don't disable security features, you're safe).
  2. The /proc/acpi interface is either removed or deprecated since 2011/2012 of almost every kernel / drivers (if you use a kernel > 2.6 - current version is 4.17.4 - and no legacy/bad drivers, you're safe). [1]
  3. If your /proc/acpi is not empty (except button on some laptops for the LiD light), please report it to your distribution or driver provider support

[1] Please see the note on the kernel config option that gates all /proc/acpi population: https://github.com/torvalds/linux/blob/master/drivers/acpi/Kconfig#L100

@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Jul 7, 2018

Locking the conversation on this PR, as it got a bit of attention on Twitter, and I want to prevent a long discussion from hiding that summary.

Feel free to discuss on Slack, the forums, or open a new ticket it more discussion is needed

@moby moby locked and limited conversation to collaborators Jul 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.