Skip to content

MaskPaths: support directory#1068

Merged
mrunalp merged 1 commit into
opencontainers:masterfrom
AkihiroSuda:maskdir
Sep 26, 2016
Merged

MaskPaths: support directory#1068
mrunalp merged 1 commit into
opencontainers:masterfrom
AkihiroSuda:maskdir

Conversation

@AkihiroSuda

@AkihiroSuda AkihiroSuda commented Sep 23, 2016

Copy link
Copy Markdown
Member

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


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

@crosbymichael

Copy link
Copy Markdown
Member

We should just use the maskedPaths for this. You can handle the not a dir error and change the logic

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]>
@AkihiroSuda AkihiroSuda changed the title support maskedDirPaths MaskPaths: support directory Sep 23, 2016
@AkihiroSuda

Copy link
Copy Markdown
Member Author

OK, updated PR

@crosbymichael

crosbymichael commented Sep 23, 2016

Copy link
Copy Markdown
Member

LGTM

Is there any better alternative to using tmpfs mounts for this?

Approved with PullApprove

@mrunalp

mrunalp commented Sep 26, 2016

Copy link
Copy Markdown
Contributor

LGTM

Approved with PullApprove

@mrunalp
mrunalp merged commit 282b254 into opencontainers:master Sep 26, 2016
@AkihiroSuda

Copy link
Copy Markdown
Member Author

@crosbymichael Alternative idea is bind-mounting an empty directory, but it requires managing the empty directory. So I chose tmpfs here.

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.

4 participants