diff mbox series

[v3,3/3] Azure: Add loop devices and CAP_SYS_ADMIN for sandbox test.py tests

Message ID 20210621185156.9108-4-alpernebiyasak@gmail.com
State Accepted
Commit e22ec9c6927bb565b89556eb7dc7856790778e46
Delegated to: Tom Rini
Headers show
Series Fix CIs skipping filesystem, EFI secure boot and EFI capsule tests | expand

Commit Message

Alper Nebi Yasak June 21, 2021, 6:51 p.m. UTC
The filesystem test setup needs to prepare disk images for its tests,
with either guestmount or loop mounts. The former requires access to the
host fuse device (added in a previous patch), the latter requires access
to host loop devices. Both mounts also need additional privileges since
docker's default configuration prevents the containers from mounting
filesystems (for host security).

Add any available loop devices to the container and try to add as few
privileges as possible to run these tests, which narrow down to adding
SYS_ADMIN capability and disabling apparmor confinement. However, this
much still seems to be insecure enough to let malicious container
processes escape as root on the host system [1].

[1] https://blog.trailofbits.com/2019/07/19/understanding-docker-container-escapes/

Since the mentioned tests are marked to run only on the sandbox board,
add these additional devices and privileges only when testing with that.

An alternative to using mounts is modifying the filesystem tests to use
virt-make-fs (like some EFI tests do), but it fails to generate a
partitionless FAT filesystem image on Debian systems. Other more
feasible alternatives are using guestfish or directly using libguestfs
Python bindings to create and populate the images, but switching the
test setups to these is nontrivial and is left as future work.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---

(no changes since v2)

Changes in v2:
- Always pass in /dev/fuse to Azure's docker run invocation.
- Remove "and some EFI tests" from comment (no longer applies to that
  block of code).

 .azure-pipelines.yml | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Simon Glass June 26, 2021, 6:29 p.m. UTC | #1
Hi Alper,

On Mon, 21 Jun 2021 at 12:52, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>
> The filesystem test setup needs to prepare disk images for its tests,
> with either guestmount or loop mounts. The former requires access to the
> host fuse device (added in a previous patch), the latter requires access
> to host loop devices. Both mounts also need additional privileges since
> docker's default configuration prevents the containers from mounting
> filesystems (for host security).
>
> Add any available loop devices to the container and try to add as few
> privileges as possible to run these tests, which narrow down to adding
> SYS_ADMIN capability and disabling apparmor confinement. However, this
> much still seems to be insecure enough to let malicious container
> processes escape as root on the host system [1].
>
> [1] https://blog.trailofbits.com/2019/07/19/understanding-docker-container-escapes/
>
> Since the mentioned tests are marked to run only on the sandbox board,
> add these additional devices and privileges only when testing with that.
>
> An alternative to using mounts is modifying the filesystem tests to use
> virt-make-fs (like some EFI tests do), but it fails to generate a
> partitionless FAT filesystem image on Debian systems. Other more
> feasible alternatives are using guestfish or directly using libguestfs
> Python bindings to create and populate the images, but switching the
> test setups to these is nontrivial and is left as future work.
>
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Always pass in /dev/fuse to Azure's docker run invocation.
> - Remove "and some EFI tests" from comment (no longer applies to that
>   block of code).
>
>  .azure-pipelines.yml | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)

Shouldn't this be done in gitlab too?

Regards,
Simon
Tom Rini June 26, 2021, 8:46 p.m. UTC | #2
On Sat, Jun 26, 2021 at 12:29:56PM -0600, Simon Glass wrote:
> Hi Alper,
> 
> On Mon, 21 Jun 2021 at 12:52, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
> >
> > The filesystem test setup needs to prepare disk images for its tests,
> > with either guestmount or loop mounts. The former requires access to the
> > host fuse device (added in a previous patch), the latter requires access
> > to host loop devices. Both mounts also need additional privileges since
> > docker's default configuration prevents the containers from mounting
> > filesystems (for host security).
> >
> > Add any available loop devices to the container and try to add as few
> > privileges as possible to run these tests, which narrow down to adding
> > SYS_ADMIN capability and disabling apparmor confinement. However, this
> > much still seems to be insecure enough to let malicious container
> > processes escape as root on the host system [1].
> >
> > [1] https://blog.trailofbits.com/2019/07/19/understanding-docker-container-escapes/
> >
> > Since the mentioned tests are marked to run only on the sandbox board,
> > add these additional devices and privileges only when testing with that.
> >
> > An alternative to using mounts is modifying the filesystem tests to use
> > virt-make-fs (like some EFI tests do), but it fails to generate a
> > partitionless FAT filesystem image on Debian systems. Other more
> > feasible alternatives are using guestfish or directly using libguestfs
> > Python bindings to create and populate the images, but switching the
> > test setups to these is nontrivial and is left as future work.
> >
> > Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> > ---
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - Always pass in /dev/fuse to Azure's docker run invocation.
> > - Remove "and some EFI tests" from comment (no longer applies to that
> >   block of code).
> >
> >  .azure-pipelines.yml | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> Shouldn't this be done in gitlab too?

In GitLab we don't control how docker is launched, is the problem.
That's up to the admins and we sometimes do, sometimes don't have the
capabilities enabled.  That probably means we should update the CI doc
and also email the various CI admins about updating things.
Simon Glass June 27, 2021, 2:43 p.m. UTC | #3
Hi Tom,

On Sat, 26 Jun 2021 at 14:46, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Jun 26, 2021 at 12:29:56PM -0600, Simon Glass wrote:
> > Hi Alper,
> >
> > On Mon, 21 Jun 2021 at 12:52, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
> > >
> > > The filesystem test setup needs to prepare disk images for its tests,
> > > with either guestmount or loop mounts. The former requires access to the
> > > host fuse device (added in a previous patch), the latter requires access
> > > to host loop devices. Both mounts also need additional privileges since
> > > docker's default configuration prevents the containers from mounting
> > > filesystems (for host security).
> > >
> > > Add any available loop devices to the container and try to add as few
> > > privileges as possible to run these tests, which narrow down to adding
> > > SYS_ADMIN capability and disabling apparmor confinement. However, this
> > > much still seems to be insecure enough to let malicious container
> > > processes escape as root on the host system [1].
> > >
> > > [1] https://blog.trailofbits.com/2019/07/19/understanding-docker-container-escapes/
> > >
> > > Since the mentioned tests are marked to run only on the sandbox board,
> > > add these additional devices and privileges only when testing with that.
> > >
> > > An alternative to using mounts is modifying the filesystem tests to use
> > > virt-make-fs (like some EFI tests do), but it fails to generate a
> > > partitionless FAT filesystem image on Debian systems. Other more
> > > feasible alternatives are using guestfish or directly using libguestfs
> > > Python bindings to create and populate the images, but switching the
> > > test setups to these is nontrivial and is left as future work.
> > >
> > > Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> > > ---
> > >
> > > (no changes since v2)
> > >
> > > Changes in v2:
> > > - Always pass in /dev/fuse to Azure's docker run invocation.
> > > - Remove "and some EFI tests" from comment (no longer applies to that
> > >   block of code).
> > >
> > >  .azure-pipelines.yml | 16 +++++++++++++++-
> > >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > Shouldn't this be done in gitlab too?
>
> In GitLab we don't control how docker is launched, is the problem.
> That's up to the admins and we sometimes do, sometimes don't have the
> capabilities enabled.  That probably means we should update the CI doc
> and also email the various CI admins about updating things.

OK I see, well once we have the docs I can try it.

Regards,
Simon
Tom Rini July 6, 2021, 12:59 a.m. UTC | #4
On Mon, Jun 21, 2021 at 09:51:56PM +0300, Alper Nebi Yasak wrote:

> The filesystem test setup needs to prepare disk images for its tests,
> with either guestmount or loop mounts. The former requires access to the
> host fuse device (added in a previous patch), the latter requires access
> to host loop devices. Both mounts also need additional privileges since
> docker's default configuration prevents the containers from mounting
> filesystems (for host security).
> 
> Add any available loop devices to the container and try to add as few
> privileges as possible to run these tests, which narrow down to adding
> SYS_ADMIN capability and disabling apparmor confinement. However, this
> much still seems to be insecure enough to let malicious container
> processes escape as root on the host system [1].
> 
> [1] https://blog.trailofbits.com/2019/07/19/understanding-docker-container-escapes/
> 
> Since the mentioned tests are marked to run only on the sandbox board,
> add these additional devices and privileges only when testing with that.
> 
> An alternative to using mounts is modifying the filesystem tests to use
> virt-make-fs (like some EFI tests do), but it fails to generate a
> partitionless FAT filesystem image on Debian systems. Other more
> feasible alternatives are using guestfish or directly using libguestfs
> Python bindings to create and populate the images, but switching the
> test setups to these is nontrivial and is left as future work.
> 
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 976868dd2eb7..e36a27c97d56 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -318,8 +318,22 @@  jobs:
           # as sandbox testing need create files like spi flash images, etc.
           # (TODO: clean up this in the future)
           chmod 777 .
+          # Filesystem tests need extra docker args to run
+          set --
+          if [[ "${TEST_PY_BD}" == "sandbox" ]]; then
+              # mount -o loop needs the loop devices
+              if modprobe loop; then
+                  for d in $(find /dev -maxdepth 1 -name 'loop*'); do
+                      set -- "$@" --device $d:$d
+                  done
+              fi
+              # Needed for mount syscall (for guestmount as well)
+              set -- "$@" --cap-add SYS_ADMIN
+              # Default apparmor profile denies mounts
+              set -- "$@" --security-opt apparmor=unconfined
+          fi
           # Some tests using libguestfs-tools need the fuse device to run
-          docker run --device /dev/fuse:/dev/fuse -v $PWD:$(work_dir) $(ci_runner_image) /bin/bash $(work_dir)/test.sh
+          docker run "$@" --device /dev/fuse:/dev/fuse -v $PWD:$(work_dir) $(ci_runner_image) /bin/bash $(work_dir)/test.sh
 
   - job: build_the_world
     displayName: 'Build the World'