Skip to content

Change /dev to be mounted by default with /noexec - #725

Merged
vbatts merged 1 commit into
opencontainers:masterfrom
rhatdan:master
Oct 22, 2021
Merged

Change /dev to be mounted by default with /noexec#725
vbatts merged 1 commit into
opencontainers:masterfrom
rhatdan:master

Conversation

@rhatdan

Copy link
Copy Markdown
Contributor

Podman had an issue, where someone was attemptig to mount all tmpfs
within the container as noexec. They were able to get most of it done
but "/dev", because it was done down in the runtime spec.

I can think of no reason why "/dev", should not be mounted with noexec
especially within a container. I know it is not mounted by default in
Fedora that way, but I do not know why.

Debian looks like it has made the change, and only one bug a couple of
years ago showed issues, which would not apply to containers.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=940171

Anyways this would make containers "slightly" more secure, and I think it
is worth doing.

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

@rhatdan

Copy link
Copy Markdown
ContributorAuthor

@mrunalp PTAL

@rhatdan

Copy link
Copy Markdown
ContributorAuthor

@runtime-tools-maintainers PTAL

@rhatdan

Copy link
Copy Markdown
ContributorAuthor

@rhatdan

Copy link
Copy Markdown
ContributorAuthor

For what it is worth, I ran this change through Podman's test suite with no failures.
podman-container-tools/podman#11349

@tianontianon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, although I'm not sure whether many other projects are using this particular code? (I believe runc, containerd, etc all have their own versions of this)

@rhatdan

Copy link
Copy Markdown
ContributorAuthor

Well it is used in Podman, Buildah, CRI-O.

@mrunalp

mrunalp commented Sep 26, 2021

Copy link
Copy Markdown
Contributor

LGTM

Approved with PullApprove

@giuseppegiuseppe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@vbatts

Copy link
Copy Markdown
Member

bah, Travis needs to be removed and switched to GitHub actions

@amye

amye commented Sep 29, 2021

Copy link
Copy Markdown

@caniszczyk ^
TravisCI is blocking things

@kolyshkin

Copy link
Copy Markdown
Contributor

bah, Travis needs to be removed and switched to GitHub actions

PTAL #727, #728@vbatts

@kolyshkin

Copy link
Copy Markdown
Contributor

Let's merge this one only after #728 so we have CI.

@kolyshkin

Copy link
Copy Markdown
Contributor

@rhatdan can you please rebase? This repo has CI now :)

Podman had an issue, where someone was attemptig to mount all tmpfs
within the container as noexec. They were able to get most of it done
but "/dev", because it was done down in the runtime spec.
I can think of no reason why "/dev", should not be mounted with noexec
especially within a container. I know it is not mounted by default in
Fedora that way, but I do not know why.
Debian looks like it has made the change, and only one bug a couple of
years ago showed issues, which would not apply to containers.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=940171
Anyways this would make containers "slightly" more secure, and I think it
is worth doing.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@vbatts
vbatts merged commit 744b5ee into opencontainers:masterOct 22, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@rhatdan@mrunalp@vbatts@amye@kolyshkin@giuseppe@tianon