Skip to content

config-linux: add maskedDirPaths#582

Closed
AkihiroSuda wants to merge 1 commit into
opencontainers:masterfrom
AkihiroSuda:maskdir
Closed

config-linux: add maskedDirPaths#582
AkihiroSuda wants to merge 1 commit into
opencontainers:masterfrom
AkihiroSuda:maskdir

Conversation

@AkihiroSuda

@AkihiroSuda AkihiroSuda commented Sep 23, 2016

Copy link
Copy Markdown
Member

maskedDirPaths is used to mask directories rather than files.
For example, the /sys/firmware directory should be masked because it can contain some sensitive files (moby/moby#26618):

  • /sys/firmware/acpi/tables/{SLIC,MSDM}: Windows license information:
  • /sys/firmware/ibft/target0/chap-secret: iSCSI CHAP secret

IMO the issue should be fixed but no need to get this PR in v1.0

runc PR: opencontainers/runc#1068

Signed-off-by: Akihiro Suda [email protected]

@AkihiroSuda
config-linux: add maskedDirPaths
8cced19 
maskedDirPaths is used to mask directories rather than files.
For example, the /sys/firmware directory should be masked because it can contain some sensitive files:
 - /sys/firmware/acpi/tables/{SLIC,MSDM}: Windows license information:
 - /sys/firmware/ibft/target0/chap-secret: iSCSI CHAP secret

Signed-off-by: Akihiro Suda <[email protected]>
This was referenced Sep 23, 2016
Merged
Closed
@AkihiroSuda

AkihiroSuda commented Sep 23, 2016

Copy link
Copy Markdown
Member Author

Alternative idea is to make sure maskedPaths supports directories as well.
RFC.

@cyphar

cyphar commented Sep 23, 2016

Copy link
Copy Markdown
Member

IMO we should just make makedPaths mask directories as well -- since it's called maskedPaths not maskedFiles.

@AkihiroSuda

AkihiroSuda commented Sep 23, 2016

Copy link
Copy Markdown
Member Author

Note that we need consideration for non-existent paths if we support directories in maskedPaths.

For compatibility, we should just bind mount /dev/null for any non-existent path, because we cannot determine if the path points a file or a directory.
However, binding /dev/null looks strange for directories.

WDYT?

@cyphar

cyphar commented Sep 23, 2016

Copy link
Copy Markdown
Member

@AkihiroSuda I'm not sure. IMO these are implementation quirks -- what matters is that the path is not accessible (you could implement it purely with apparmor and IMO it would be "valid").

@crosbymichael

crosbymichael commented Sep 23, 2016

Copy link
Copy Markdown
Member

We already mask dirs with maskedPaths so we should not have this in the spec and fix any issues you are seeing with this.

@flx42 flx42 mentioned this pull request Apr 27, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AkihiroSuda @cyphar @crosbymichael