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.

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

RFE: --read-only: add sub-option to make /dev readonly as well #12937

Closed
martinetd opened this issue Jan 20, 2022 · 6 comments · Fixed by #19422
Closed

RFE: --read-only: add sub-option to make /dev readonly as well #12937

martinetd opened this issue Jan 20, 2022 · 6 comments · Fixed by #19422
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@martinetd
Copy link

martinetd commented Jan 20, 2022

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind feature

Description

podman run --read-only leaves /tmp, /var/tmp /run (tmpfs with default 50% size of ram), /dev (tmpfs with 64MB) and /dev/shm (default 64MB).

  • There is --read-only-tmpfs to skip mounting /tmp /var/tmp, and /run

  • There is --shm-size which allows changing /dev/shm size (although --shm-size=0 means unlimited, so there is no way of forbidding writes to /dev/shm -- at best you can set size to e.g. 1 which will allow creating a single file <4k then fail with ENOSPC (or create a few hundred thousands of empty files which would consume roughly as many megabytes of memory...))

  • And I do not see any option at all for /dev, so a container could also create arbitrary files there (and consume a few hundred MBs of memory creating as many empty files as they can then filling one up)

Would it make sense to also be able to limit these accesses, e.g. not mount /dev/shm at all, and remount /dev as read-only after devices have been created? (works fine doing it manually in a privileged container)

It's always possible to limit the container memory with -m, so my consuming memory argument above is work-aroundable, but the main point here is to have no directory writable at all to limit the attack surface: being able to download an arbitrary binary in /dev and executing it is easier than trying to modify running programs code.
Saying the container is read-only but allowing writes in /dev is a bit counter-intuitive in my opinion.

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 20, 2022
@giuseppe
Copy link
Member

you could achieve something like that with:

$ podman run  --mount=type=tmpfs,target=/dev,ro=true -v /dev/null:/dev/pts -v /dev/null:/dev/shm --rm  alpine cat /proc/self/mountinfo

mounting /dev read-only is tricky though. The OCI runtime itself can fail (both runc and crun do not handle it correctly) and handling of TTYs can also be affected.

Using the command above, I've found an issue in crun: containers/crun#857

@rhatdan
Copy link
Member

rhatdan commented Jan 20, 2022

If you mount /dev readonly, then can you still write to the devices?

@rhatdan
Copy link
Member

rhatdan commented Jan 20, 2022

--read-only-tmpfs should probably include /dev/shm and potentially set /dev to be read-only if that would be allowed.

@rhatdan
Copy link
Member

rhatdan commented Jan 20, 2022

sh-5.1# podman run -v /usr:/usr --privileged -ti fedora sh
sh-5.1# mount -o remount,ro /dev
sh-5.1# touch /dev/dan
touch: cannot touch '/dev/dan': Read-only file system
sh-5.1# echo hello > /dev/null 

Looks like it should work.

@martinetd
Copy link
Author

@giuseppe thanks for the idea!

for /dev/shm, using --mount type=tmpfs,target=/dev/shm,ro=true as well is a bit closer to what I want (will get EROFS errors instead of "not a directory") and works quite well, I'll be using that until/if read-only-tmpfs takes care of it :)

I don't think masking /dev/pts is necessary, it's a different mount but it's a special kernel mount type that doesn't allow creating arbitrary files, it's a bit like the many filesystems in /sys even root will just get permissino denied if they just try to create a file naively.

/dev itself is a bit more tricky though because we'd want to leave it read-write while the OCI runtime populates it and only turn it readonly before passing the ball on to the container. It might be possible to get something working by manually mounting all required devices individually but I don't think I want to get down that hole... :)
(This made me try to adjust the size with --mount type=tmpfs,target=/dev,tmpfs-size=xxx but that seems to break the device creations as well even if it's read-write)

@rhatdan yes, remounting /dev read-only is fine, you just need devices to preexist (so a container that would handle hotplug (because e.g. it's privileged and runs udev) won't work, but a normal container where the oci runtime creates devices at start will work). Thanks for taking a look as well :)

rhatdan added a commit to rhatdan/podman that referenced this issue Jan 21, 2022
Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jan 22, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

rhatdan added a commit to rhatdan/podman that referenced this issue Feb 21, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Feb 21, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Feb 21, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Feb 22, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Feb 28, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Mar 1, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Mar 3, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Mar 9, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Mar 31, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Apr 1, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Apr 4, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Apr 22, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Apr 23, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Apr 26, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Apr 27, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue May 3, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue May 3, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue May 3, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue May 3, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jun 13, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jun 16, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jun 17, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jun 17, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jul 12, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Aug 2, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Aug 4, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Aug 9, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Aug 10, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Aug 16, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Sep 5, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Oct 20, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Oct 25, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Oct 28, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Oct 31, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Oct 31, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Nov 1, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Nov 1, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Nov 1, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Nov 1, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Nov 7, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Dec 3, 2022
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue May 10, 2023
Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jul 29, 2023
The intention of --read-only-tmpfs=fals when in --read-only mode was to
not allow any processes inside of the container to write content
anywhere, unless the caller also specified a volume or a tmpfs. Having
/dev and /dev/shm writable breaks this assumption.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan added a commit to rhatdan/podman that referenced this issue Jul 30, 2023
The intention of --read-only-tmpfs=fals when in --read-only mode was to
not allow any processes inside of the container to write content
anywhere, unless the caller also specified a volume or a tmpfs. Having
/dev and /dev/shm writable breaks this assumption.

Fixes: containers#12937

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Oct 30, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
3 participants