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

Improve support for podman out of the box #4774

Closed
wants to merge 4 commits into from
Closed

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Dec 3, 2020

As stated in #4763 I am using the helper script with podman for some time, but it does not work out of the box so I had to maintain couple of changes and make sure the correct selinux permissions are in place and containers run as expected.

This PR provides a couple of changes that should make the use of podman as a container engine more usable out of the box, but it will probably require some polishing:

  • podman is detected by existence of /bin/podman -- the simplest I was able to figure out.
    • It can be invoked through the docker CLI, but why when we already know it is there.
  • podman requires the files and directories that are mounted to the container to have container_file_t SELinux type as a security measure to make sure evil containers do not have access unintended files on the whole local filesystem (if used improperly)
    • This is certainly needed for the build directory (all the other files created under it inherit this context by itself)
    • This is needed for the testcase files, but only from not-privileged container (see later)
    • This is needed for the source files if local checkout is used to build fuzzers (not included here as it might be more intrusive than users would expect as it requires recursive change of context of all the source files)
  • in podman having both --privileged and --cap-add SYS_PTRACE is invalid (not sure what it should do with docker anyway -- maybe worth checking docker docs):
    Error: invalid config provided: CapAdd and privileged are mutually exclusive options
    
    • podman very happily run all stuff in non-privileged containers, but the only issue I noticed was in reproduce subcommand, that Leaksanitizer complains (not sure if it has any functional difference, but it goes away when I add --cap-add SYS_PTRACE instead, which sounds a better solution):
    ==8==LeakSanitizer has encountered a fatal error.
    ==8==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
    ==8==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
    

I did no run any comprehensive testsuite of the helper so there might be still some corner cases that will not work. This could be solved by running some tests in CI not only on your default systems, but also in Fedora.

@jonathanmetzman
Copy link
Contributor

Thanks for this patch.
Unfortunately, I don't think we have the cycles to maintain code to support podman. We'd rather people used docker.
I don't think we can accept this patch.

@Jakuje
Copy link
Contributor Author

Jakuje commented Dec 7, 2020

Hi, thank you for your time. The scope of the PR can be changed. I have put together all the things I know that might need changes to work flawlessly. If there is an issue with approach to a particular change or some change itself, I can rewrite it or drop it. Some of the changes might be even useful for docker. Do you need all containers to be privileged?

@jonathanmetzman jonathanmetzman mentioned this pull request Dec 7, 2020
@jonathanmetzman
Copy link
Contributor

Hi, thank you for your time. The scope of the PR can be changed. I have put together all the things I know that might need changes to work flawlessly. If there is an issue with approach to a particular change or some change itself, I can rewrite it or drop it. Some of the changes might be even useful for docker.

Would need to think about these. But is using docker so annoying for you? It's much easier for us to not worry about podman especially since I don't know what it is nor do I use it.

Do you need all containers to be privileged?

This is a good question. But I'm not sure what the answer is. We could maybe think about making them unprivileged but I don't know if this is a high priority for us.

@Jakuje
Copy link
Contributor Author

Jakuje commented Dec 8, 2020

Using podman is as simple as installing it from my package manager and it all just works in sane manner. The containers do not require any privileged daemons and run from normal user.

https://podman.io/

As I go through the steps in official installation steps for docker, it would requires for me to install third party repositories, software, modifying my kernel configuration (probably breaking podman) and running (untrusted) daemon on my machine to achieve technically the same.

https://docs.docker.com/engine/install/fedora/

@evverx
Copy link
Contributor

evverx commented Nov 6, 2022

Just for the record podman more or less worked with https://github.com/containers/podman/blob/main/docker (in the sense that it was possible to build and run fuzz targets) until I pulled eef03e0. Unfortunately

podman run --platform linux/amd64 ... gcr.io/oss-fuzz/<project-name>

always pulls 3 year old images like https://console.cloud.google.com/gcr/images/oss-fuzz/GLOBAL/systemd and replaces local images with them. I removed the "platform" argument from helper.py to get it around. Another option would be to name local images differently to avoid conflicts with gcr.io/oss-fuzz/<project-name> or just drop those old images (assuming they aren't used anywhere).

@Jakuje
Copy link
Contributor Author

Jakuje commented Nov 6, 2022

I am still keeping track and updating this branch that I use for running the tools with docker successfully even with the recent changes regarding the arm, if I remember right, but looks like I did not push the changes. I will try to do that next week.

@evverx
Copy link
Contributor

evverx commented Nov 6, 2022

@Jakuje just out of curiosity I wonder why podman pulls those images with --platform=... even though they are already built locally? According to containers/podman#12682 (comment)

Note that when $ARCH is specified, Podman will also pessimistically pull the image down

it seems to be intentional (even though it isn't compatible with docker) but I can't seem to figure out why.

We'd rather people used docker. I don't think we can accept this patch.

@jonathanmetzman is there any chance this patch can be reconsidered? On Fedora (and some other rpm-based distributions) podman is much more stable than docker. For example on Fedora 36 the OSS-Fuzz toolchain is broken at the moment as far as I can tell because when docker is used ./infra/helper.py run ... gets stuck in llvm-symbolizer somewhere for whatever reason so all the images have to be rebuilt with symbolize=0 to make it work and then crashes have to be copied to the host to get actual backtraces.

@evverx
Copy link
Contributor

evverx commented Nov 6, 2022

Having experimented with podman and ./infra/helper.py a bit more I think the "selinux" part of this patch can be dropped (assuming --privileged is passed to make the diff even smaller). It appears these days ./infra/helper.py should work with podman with selinux enabled in most cases without having to relabel directories. I tested it as a non-privileged user and ran ./infra/helper.py without sudo:

$ id
uid=1000(vagrant) gid=1000(vagrant) groups=1000(vagrant) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

$ getenforce
Enforcing

$ ls -ldZ .
drwxr-xr-x. 16 vagrant vagrant unconfined_u:object_r:user_home_t:s0 4096 Nov  6 19:58 .

$ ls -ldZ build
drwxr-xr-x. 5 vagrant vagrant unconfined_u:object_r:user_home_t:s0 4096 Nov  6 19:56 build

$ ls -ldZ dbus-broker
drwxr-xr-x. 9 vagrant vagrant unconfined_u:object_r:user_home_t:s0 4096 Nov  6 19:55 dbus-broker

(./infra/helper.py with docker with selinux enabled goes sideways when it tries to bind-mount something as far as I can see).

@Jakuje
Copy link
Contributor Author

Jakuje commented Nov 7, 2022

Having experimented with podman and ./infra/helper.py a bit more I think the "selinux" part of this patch can be dropped (assuming --privileged is passed to make the diff even smaller).

AFAIK there is still some change needed to make all use cases working with SELinux. If your files and directories have already updated selinux labels, it will probably look like the changes are not needed, but if not, you still need them. But other option might be using the :z flags with the volumes, which should be shorter.

Running privileged container that has access to the whole of your system might be also blocker for some.

I am still keeping track and updating this branch that I use for running the tools with docker successfully even with the recent changes regarding the arm, if I remember right, but looks like I did not push the changes. I will try to do that next week.

In this I was wrong. I rebased the changes, but implemented some hacks to prefer local images over the remote ones to get the work done and then I forgot about that. Seems like what is described in containers/podman/issues#12682 is the problem we are hitting there. But would this be solved with providing the explicit --arch to all the podman commands?

@evverx
Copy link
Contributor

evverx commented Nov 7, 2022

Running privileged container that has access to the whole of your system might be also blocker for some.

Agreed.

But would this be solved with providing the explicit --arch to all the podman commands?

Based on my experiments podman always pulls those images regardless of whether they are built with --platform=... or not so adding --platform=... to docker buildx build ... (run by helper.py build_images ...) doesn't seem to work either. To make helper.py work either --platform=... should be removed or --pull=never should be added to docker run. With --pull=never all the base images should be pulled manually when necessary.

@evverx
Copy link
Contributor

evverx commented Nov 9, 2022

Before I forget I ended up pointing helper.py to a script adding --pull=never when it sees gcr.io/oss-fuzz/

#!/usr/bin/bash

args=()
is_build=false
for a in "$@"; do
  if [[ "$a" == "build" ]]; then
    is_build=true
  elif [[ "$a" =~ ^gcr\.io/oss-fuzz/ && "$is_build" != "true" ]]; then
    args+=("--pull=never")
  fi
  args+=("$a")
done

[ -e /etc/containers/nodocker ] || \
echo "Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg." >&2
exec /usr/bin/podman "${args[@]}"

It's ugly but it more or less works even with --architecture aarch64 on amd64 (as long as qemu-user is installed). Once conmon-v2.1.5 (where containers/conmon#315 is fixed) lands in Fedora it should be possible to start running all sorts of fuzz targets there.

I still think that it would be great if it was possible to integrate podman natively into helper.py though. Until then the kludge should more or less suffice.

Sign up for free to 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.

3 participants