Skip to content

Add /proc/scsi to masked paths#35399

Merged
yongtang merged 1 commit intomoby:masterfrom
justincormack:mask-scsi
Nov 3, 2017
Merged

Add /proc/scsi to masked paths#35399
yongtang merged 1 commit intomoby:masterfrom
justincormack:mask-scsi

Conversation

@justincormack
Copy link
Copy Markdown
Contributor

This is writeable, and can be used to remove devices. Containers do
not need to know about scsi devices.

Signed-off-by: Justin Cormack [email protected]

1411-masked-dog

This is writeable, and can be used to remove devices. Containers do
not need to know about scsi devices.

Signed-off-by: Justin Cormack <[email protected]>
@crosbymichael
Copy link
Copy Markdown
Contributor

LGTM

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah thaJeztah added area/runtime Runtime and removed status/0-triage labels Nov 3, 2017
@yongtang yongtang merged commit a8cefcf into moby:master Nov 3, 2017
@justincormack justincormack deleted the mask-scsi branch November 4, 2017 11:22
@vielmetti
Copy link
Copy Markdown

https://twitter.com/ewindisch/status/926443182010916865

Is there a CVE, so that this gets properly handled upstream and downstream?

@thaJeztah
Copy link
Copy Markdown
Member

I'm not sure if a CVE was opened for the kernel

AkihiroSuda added a commit to AkihiroSuda/containerd that referenced this pull request Nov 4, 2017
Port over moby/moby#35399

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/runc that referenced this pull request Nov 4, 2017
@vielmetti
Copy link
Copy Markdown

CVE-2017-16539

@thaJeztah
Copy link
Copy Markdown
Member

Thanks!

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Nov 4, 2017

I wonder if it's correct that he CVE is reported against Moby, as it's a kernel vulnerability; the patch here is just to work around that; http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-16539

@vielmetti
Copy link
Copy Markdown

Yeah, probably the kernel gets the real blame, but I don't think there's a clear understanding yet of which piece of code would need to be fixed.

@thaJeztah
Copy link
Copy Markdown
Member

Kernel patch was created by @cyphar here; https://marc.info/?l=linux-scsi&m=150982199728895&w=2

@cyphar
Copy link
Copy Markdown
Contributor

cyphar commented Nov 4, 2017

@thaJeztah I think that Docker is the right component to be filed against (though it should be noted that the default AppArmor and SELinux setup actually protects against this attack -- so you'd have to misconfigure your system in order to make it exploitable) since we don't use user namespaces by default and we run images as root by default (with CAP_DAC_OVERRIDE enabled). If any of those things weren't true this attack couldn't work even with a misconfigured system.

@vielmetti
Copy link
Copy Markdown

Based on this conversation

https://twitter.com/ewindisch/status/926888008015638530

there's a variant on this attack (known as #GroceryShoppingWithMyKids - and every bug gets an animated GIF) where the attacker writes into /proc/scsi/device_info and can also "write into this arbitrary data append only and DOS kernel via memory allocations".

I've send in a note to update the CVE to reference the patch from @cyphar which I believe addresses this variant as well.

@cyphar
Copy link
Copy Markdown
Contributor

cyphar commented Nov 4, 2017

@vielmetti That is also protected against by the default AppArmor and SELinux profiles (so the same "misconfigured" and "our defaults really should be better if it weren't for legacy reasons" caveats as above). And yes, my patch fixes that issue from the kernel-side as well.

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Nov 8, 2017

@justincormack would it be realistic to write a test of this ?

@mghazizadeh
Copy link
Copy Markdown

how's this tested?

@cyphar
Copy link
Copy Markdown
Contributor

cyphar commented Nov 19, 2017

Well, you could start a container and check whether /proc/scsi is a tmpfs mount (or check that it's empty). Not sure it's worth it though -- we have tests in runc that make sure masked paths work properly and this just uses maskedPaths in the OCI configuration, so Docker isn't actually doing anything here.

KentaTada pushed a commit to KentaTada/crun that referenced this pull request Sep 14, 2020
dims pushed a commit to dims/libcontainer that referenced this pull request Oct 19, 2024
dims pushed a commit to dims/libcontainer that referenced this pull request Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.