[PATCH v3 0/3] Fix CIs skipping filesystem, EFI secure boot and EFI capsule tests

[PATCH v3 0/3] Fix CIs skipping filesystem, EFI secure boot and EFI capsule tests

[Alper Nebi Yasak]

After my previous patch to fix filesystem tests [1] was merged, I
noticed the GitLab CI was still skipping them and wanted to figure out
why. In short: libguestfs tools (virt-make-fs, guestmount) fail because
they need an installed kernel and the host /dev/fuse device, loop mounts
need the host /dev/loop* devices, and mounting filesystems (loop and
guestmount) fails because Docker containers need extra permissions to
mount devices normally disabled for host security.

[1] https://patchwork.ozlabs.org/project/uboot/patch/20210520190947.21773-1-alpernebiyasak@gmail.com/

Patch #1 is meant to install a kernel into the container image that
libguestfs can use, but the image will need to be regenerated manually.

Patch #2 makes virt-make-fs work, which should make these EFI tests run
again. But guestmount doesn't work with this much because it needs more
permissions to actually mount a filesystem.

Patch #3 makes mounting filesystems and loop devices work, which should
make the filesystem tests run again. This is separate from patch #2
because the parts using guestmount can theoretically be rewritten to use
guestfish which would make the filesystem tests work without this patch,
and giving mount permissions to the container processes seem to be
insecure. So, this patch can be dropped if you think the impact isn't
worth it.

Similar changes to patch #2, #3 should be applicable to the GitLab CI
and probably necessary to get the same effect, but I don't think its
configuration is accessible to me.

Changes in v3:
- Only set /boot/vmlinu* as readable.

v2: https://patchwork.ozlabs.org/project/uboot/list/?series=248583

Changes in v2:
- Always pass in /dev/fuse to Azure's docker run invocation.
- Drop patch to install kernel in CI scripts

v1: https://patchwork.ozlabs.org/project/uboot/list/?series=247294

Alper Nebi Yasak (3):
  tools: docker: Install a readable kernel for libguestfs-tools
  Azure: Add fuse device for test.py tests
  Azure: Add loop devices and CAP_SYS_ADMIN for sandbox test.py tests

 .azure-pipelines.yml    | 17 ++++++++++++++++-
 tools/docker/Dockerfile |  4 ++++
 2 files changed, 20 insertions(+), 1 deletion(-)

-- 
2.32.0

[PATCH v3 1/3] tools: docker: Install a readable kernel for libguestfs-tools

[Alper Nebi Yasak]

The filesystem and EFI (capsule and secure boot) test setups try to use
guestmount and virt-make-fs respectively to prepare disk images to run
tests on. However, these libguestfs tools need a kernel image and fail
with the following message (revealed in debug/trace mode) if it can't
find one:

    supermin: failed to find a suitable kernel (host_cpu=x86_64).

    I looked for kernels in /boot and modules in /lib/modules.

    If this is a Xen guest, and you only have Xen domU kernels
    installed, try installing a fullvirt kernel (only for
    supermin use, you shouldn't boot the Xen guest with it).

This failure then causes these tests to be skipped in CIs. Install a
kernel package in the Docker containers so the CIs can run these
tests with libguestfs tools again (assuming the container is run with
necessary host devices and privileges). As this kernel would be only
used for virtualization, we can use the kernel package specialized for
that. On Ubuntu systems kernel images are not readable by non-root
users, so explicitly add read permissions with chmod as well.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---

Changes in v3:
- Only set /boot/vmlinu* as readable.

Changes in v2:
- Add tag "Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>"

 tools/docker/Dockerfile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index d2f0074ee8a6..579df82b56e7 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -71,6 +71,7 @@ RUN apt-get update && apt-get install -y \
 	libssl-dev \
 	libudev-dev \
 	libusb-1.0-0-dev \
+	linux-image-kvm \
 	lzma-alone \
 	lzop \
 	mount \
@@ -99,6 +100,9 @@ RUN apt-get update && apt-get install -y \
 	zip \
 	&& rm -rf /var/lib/apt/lists/*
 
+# Make kernels readable for libguestfs tools to work correctly
+RUN chmod +r /boot/vmlinu*
+
 # Manually install libmpfr4 for the toolchains
 RUN wget http://mirrors.kernel.org/ubuntu/pool/main/m/mpfr4/libmpfr4_3.1.4-1_amd64.deb && dpkg -i libmpfr4_3.1.4-1_amd64.deb && rm libmpfr4_3.1.4-1_amd64.deb
 
-- 
2.32.0

[PATCH v3 2/3] Azure: Add fuse device for test.py tests

[Alper Nebi Yasak]

The EFI secure boot and capsule test setups need to prepare disk images
for their tests using virt-make-fs, which requires access to the host
fuse device. This is not exposed to the docker container by default and
has to be added explicitly. Add it.

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.

 .azure-pipelines.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 35ab7f30b276..976868dd2eb7 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -318,7 +318,8 @@ jobs:
           # as sandbox testing need create files like spi flash images, etc.
           # (TODO: clean up this in the future)
           chmod 777 .
-          docker run -v $PWD:$(work_dir) $(ci_runner_image) /bin/bash $(work_dir)/test.sh
+          # 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
 
   - job: build_the_world
     displayName: 'Build the World'
-- 
2.32.0

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

[Alper Nebi Yasak]

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(-)

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'
-- 
2.32.0

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

[Simon Glass]

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

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2377 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Simon Glass]

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

Re: [PATCH v3 1/3] tools: docker: Install a readable kernel for libguestfs-tools

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Mon, Jun 21, 2021 at 09:51:54PM +0300, Alper Nebi Yasak wrote:

> The filesystem and EFI (capsule and secure boot) test setups try to use
> guestmount and virt-make-fs respectively to prepare disk images to run
> tests on. However, these libguestfs tools need a kernel image and fail
> with the following message (revealed in debug/trace mode) if it can't
> find one:
> 
>     supermin: failed to find a suitable kernel (host_cpu=x86_64).
> 
>     I looked for kernels in /boot and modules in /lib/modules.
> 
>     If this is a Xen guest, and you only have Xen domU kernels
>     installed, try installing a fullvirt kernel (only for
>     supermin use, you shouldn't boot the Xen guest with it).
> 
> This failure then causes these tests to be skipped in CIs. Install a
> kernel package in the Docker containers so the CIs can run these
> tests with libguestfs tools again (assuming the container is run with
> necessary host devices and privileges). As this kernel would be only
> used for virtualization, we can use the kernel package specialized for
> that. On Ubuntu systems kernel images are not readable by non-root
> users, so explicitly add read permissions with chmod as well.
> 
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 2/3] Azure: Add fuse device for test.py tests

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

On Mon, Jun 21, 2021 at 09:51:55PM +0300, Alper Nebi Yasak wrote:

> The EFI secure boot and capsule test setups need to prepare disk images
> for their tests using virt-make-fs, which requires access to the host
> fuse device. This is not exposed to the docker container by default and
> has to be added explicitly. Add it.
> 
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]