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.

Sign up for GitHub

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

Jump to bottom

Add /proc/acpi and /proc/keys to masked paths #2443

Merged
merged 2 commits into from
Jul 6, 2018
Merged

Add /proc/acpi and /proc/keys to masked paths #2443

merged 2 commits into from
Jul 6, 2018

Conversation

thaJeztah
Copy link
Member

relates to CVE-2018-10892

should probably be cherry-picked to release-branches

@thaJeztah
Copy link
Member Author

Don't think the AppArmor contrib needs updating, but stumbled upon it while searching what to update; https://github.com/containerd/containerd/blob/master/contrib/apparmor/template.go#L52-L60

@thaJeztah
Add /proc/acpi to masked paths
8b42ade 
relates to CVE-2018-10892

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Add /proc/keys to masked paths
fe64b06 
This leaks information about keyrings on the host. Keyrings are
not namespaced.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah changed the title Add /proc/acpi to masked paths Add /proc/acpi and /proc/keys to masked paths Jul 6, 2018
@thaJeztah
Copy link
Member Author

Added a second commit to also mask /proc/keys (similar to moby/moby#36368)

Happy to squash if preferred

@mlaventure
Copy link
Contributor

mlaventure commented Jul 6, 2018 •
edited
Loading

Masking it will hide all the subdirectories, but it wouldn't hurt to add deny /proc/acpi/** w to the apparmor config.

@thaJeztah
Copy link
Member Author

I can do that as a follow-up to not block this PR (don't think the profile was used currently?)

@mlaventure
Copy link
Contributor

works for me.

@wking wking mentioned this pull request Jul 6, 2018
Merged
@codecov-io
Copy link

codecov-io commented Jul 6, 2018 •
edited
Loading

Codecov Report

Merging #2443 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2443      +/-   ##
==========================================
+ Coverage      45%   45.01%   +0.01%     
==========================================
  Files          92       92              
  Lines        9412     9414       +2     
==========================================
+ Hits         4236     4238       +2     
  Misses       4493     4493              
  Partials      683      683
Flag Coverage Δ
#linux 49.24% <100%> (+0.01%) ⬆️
#windows 41.3% <ø> (ø) ⬆️
Impacted Files Coverage Δ
oci/spec_unix.go 98.4% <100%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e01779a...fe64b06. Read the comment docs.

@dmcgowan
Copy link
Member

dmcgowan commented Jul 6, 2018

LGTM

1 similar comment
@crosbymichael
Copy link
Member

LGTM

@crosbymichael crosbymichael merged commit b416337 into containerd:master Jul 6, 2018
@thaJeztah thaJeztah deleted the mask_acpi branch July 6, 2018 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlaventure mlaventure mlaventure approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thaJeztah @mlaventure @codecov-io @dmcgowan @crosbymichael