Skip to content

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

Merged
crosbymichael merged 2 commits intocontainerd:masterfrom
thaJeztah:mask_acpi
Jul 6, 2018
Merged

Add /proc/acpi and /proc/keys to masked paths#2443
crosbymichael merged 2 commits intocontainerd:masterfrom
thaJeztah:mask_acpi

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Jul 6, 2018

relates to CVE-2018-10892

should probably be cherry-picked to release-branches

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Jul 6, 2018

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
Copy Markdown
Member Author

thaJeztah commented Jul 6, 2018

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

Happy to squash if preferred

@mlaventure
Copy link
Copy Markdown
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
Copy Markdown
Member Author

thaJeztah commented Jul 6, 2018

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

@mlaventure
Copy link
Copy Markdown
Contributor

mlaventure commented Jul 6, 2018

works for me.

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

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
Copy Markdown
Member

dmcgowan commented Jul 6, 2018

LGTM

1 similar comment
@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Jul 6, 2018

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