using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
Hi Tom,
I'd like to make sure of your policy about usage of "sudo" on CI.
Do you think that we should always avoid using "sudo" in testing?
I remember that you had allowed us to run sudo in (python)
test scripts on Travis CI when I requested this (for FAT filesystem?).
-Takahiro Akashi
On Tue, Jul 07, 2020 at 12:42:35PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, AKASHI Takahiro wrote:
> > In this test case, an image may have a signature with additional
> > intermediate certificates. A chain of trust will be followed and all
> > the certificates in the middle of chain must be verified before loading.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > test/py/tests/test_efi_secboot/conftest.py| 138 +-
> > test/py/tests/test_efi_secboot/defs.py| 11 +-
> > test/py/tests/test_efi_secboot/openssl.cnf| 48 ++
> > .../test_efi_secboot/test_signed_intca.py | 134 +
> > 4 files changed, 328 insertions(+), 3 deletions(-)
> > create mode 100644 test/py/tests/test_efi_secboot/openssl.cnf
> > create mode 100644 test/py/tests/test_efi_secboot/test_signed_intca.py
> >
> > diff --git a/test/py/tests/test_efi_secboot/conftest.py
> > b/test/py/tests/test_efi_secboot/conftest.py
> > index 34abcd79ae00..e5ac2a2a21b7 100644
> > --- a/test/py/tests/test_efi_secboot/conftest.py
> > +++ b/test/py/tests/test_efi_secboot/conftest.py
> > @@ -37,7 +37,7 @@ def efi_boot_env(request, u_boot_config):
> > global HELLO_PATH
> >
> > image_path = u_boot_config.persistent_data_dir
> > -image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME
> > +image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '.img'
> > image_size = EFI_SECBOOT_IMAGE_SIZE
> > part_size = EFI_SECBOOT_PART_SIZE
> > fs_type = EFI_SECBOOT_FS_TYPE
> > @@ -46,7 +46,7 @@ def efi_boot_env(request, u_boot_config):
> > HELLO_PATH = u_boot_config.build_dir +
> > '/lib/efi_loader/helloworld.efi'
> >
> > try:
> > -mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure'
> > +mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> > check_call('mkdir -p {}'.format(mnt_point), shell=True)
> >
> > # create a disk/partition
> > @@ -170,3 +170,137 @@ def efi_boot_env(request, u_boot_config):
> > yield image_path
> > finally:
> > call('rm -f %s' % image_path, shell=True)
> > +
> > +#
> > +# Fixture for UEFI secure boot test of intermediate certificates
>
> Thanks for adding a test.
>
>
> > +#
> > +@pytest.fixture(scope='session')
> > +def efi_boot_env_intca(request, u_boot_config):
> > +"""Set up a file system to be used in UEFI secure boot test
> > +of intermediate certificates.
> > +
> > +Args:
> > +request: Pytest request object.
> > + u_boot_config: U-boot configuration.
> > +
> > +Return:
> > +A path to disk image to be used for testing
> > +"""
> > +global HELLO_PATH
> > +
> > +image_path = u_boot_config.persistent_data_dir
> > +image_path = image_path + '/' + EFI_SECBOOT_IMAGE_NAME + '_intca.img'
> > +image_size = EFI_SECBOOT_IMAGE_SIZE
> > +part_size = EFI_SECBOOT_PART_SIZE
> > +fs_type = EFI_SECBOOT_FS_TYPE
> > +
> > +if HELLO_PATH == '':
> > +HELLO_PATH = u_boot_config.build_dir +
> > '/lib/efi_loader/helloworld.efi'
> > +
> > +try:
> > +mnt_point = u_boot_config.persistent_data_dir + MNTPNT
> > +check_call('mkdir -p {}'.format(mnt_point), shell=True)
> > +
> > +# create a disk/partition
> > +check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
> > +% (image_path, image_size), shell=True)
> > +check_call('sgdisk %s -n 1:0:+%dMiB'
> > +% (image_path, part_size), shell=True)
> > +# create a file system
> > +check_call('dd if=/dev/zero of=%s.tmp bs=1MiB count=%d'
> > +% (image_path, part_size), shell=True)
> > +check_call('mkfs -t %s %s.tmp' % (fs_type, image_path), shell=True)
> > +check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d
> > conv=notrunc'
> > +% (image_path, image_path, 1), shell=True)
> > +check_call('rm %s.tmp' % image_path, shell=True)
> > +loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB
> > --show -f %s | tr -d "\n"'
> > +% (part_size, image_path),
> > shell=True).decode()
> > +check_output('sudo mount -t %s -o umask=000 %s %s'
> > +% (fs_type, loop_dev, mnt_point),
> > shell=True)
>
> Can we use virt-make-fs to avoid sudo, please. Package libguestfs-tools
> has been added to the Docker image for Gitlab recently.
>
> > +
> > +# Create signature database
> > +## PK
> > +check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048
> > -subj /CN=TEST_PK/ -keyout PK.key -out PK.c
Re: using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
On Thu, Jul 09, 2020 at 09:58:03AM +0900, AKASHI Takahiro wrote: > Hi Tom, > > I'd like to make sure of your policy about usage of "sudo" on CI. > Do you think that we should always avoid using "sudo" in testing? > > I remember that you had allowed us to run sudo in (python) > test scripts on Travis CI when I requested this (for FAT filesystem?). So, the best practices at this time are to have the code try and use guestmount (or similar tools) when possible and fall back to sudo, as Ubuntu breaks guestmount (and similar tools) by default. -- Tom signature.asc Description: PGP signature
Re: using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
Tom, On Wed, Jul 08, 2020 at 11:15:26PM -0400, Tom Rini wrote: > On Thu, Jul 09, 2020 at 09:58:03AM +0900, AKASHI Takahiro wrote: > > > Hi Tom, > > > > I'd like to make sure of your policy about usage of "sudo" on CI. > > Do you think that we should always avoid using "sudo" in testing? > > > > I remember that you had allowed us to run sudo in (python) > > test scripts on Travis CI when I requested this (for FAT filesystem?). > > So, the best practices at this time are to have the code try and use > guestmount (or similar tools) when possible and fall back to sudo, as > Ubuntu breaks guestmount (and similar tools) by default. See the commands log (on my ubuntu 19.10) below: ===8<=== << try 1 >> tmp$ mkdir tmpdir tmp$ virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img libguestfs: error: /usr/bin/supermin exited with error status 1. To see full error messages you may need to enable debugging. Do: export LIBGUESTFS_DEBUG=1 LIBGUESTFS_TRACE=1 and run the command again. For further information, read: http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs You can also run 'libguestfs-test-tool' and post the *complete* output into a bug report or message to the libguestfs mailing list. << try 2 >> tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img ... supermin: kernel: kernel_version 5.3.0-62-generic supermin: kernel: modpath /lib/modules/5.3.0-62-generic cp: cannot open '/boot/vmlinuz-5.3.0-62-generic' for reading: Permission denied supermin: cp -p '/boot/vmlinuz-5.3.0-62-generic' '/var/tmp/.guestfs-1000/appliance.d.op62psoy/kernel': command failed, see earlier errors libguestfs: error: /usr/bin/supermin exited with error status 1, see debug messages above ... << try 3 >> tmp$ sudo chmod a+rw /boot/vmlinuz-5.3.0-62-generic tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img ... tmp$ ls -l tmp.img -rw-r--r-- 1 akashi akashi 1341440 Jul 9 13:50 tmp.img ===>8=== As you can see, virt-make-fs will fail on *standard* ubuntu. You have to change the permission of the current kernel's binary. While I can't make sure, we will have the same issue with guestmount as it will also create a minimum virtual machine before execution. What does it mean? You must change the permission every time when you re-install the OS or re-bump the kernel version. Obviously, I can't do that from my own test script (without sudo). So if you don't have any way (or workaround) to deal with it, libguestfs-tools or guestmount cannot be a solution here. -Takahiro Akashi > -- > Tom
Re: using sudo?, Re: [PATCH v2 8/8] test/py: efi_secboot: add test for intermediate certificates
On Thu, Jul 09, 2020 at 02:33:49PM +0900, AKASHI Takahiro wrote: > Tom, > > On Wed, Jul 08, 2020 at 11:15:26PM -0400, Tom Rini wrote: > > On Thu, Jul 09, 2020 at 09:58:03AM +0900, AKASHI Takahiro wrote: > > > > > Hi Tom, > > > > > > I'd like to make sure of your policy about usage of "sudo" on CI. > > > Do you think that we should always avoid using "sudo" in testing? > > > > > > I remember that you had allowed us to run sudo in (python) > > > test scripts on Travis CI when I requested this (for FAT filesystem?). > > > > So, the best practices at this time are to have the code try and use > > guestmount (or similar tools) when possible and fall back to sudo, as > > Ubuntu breaks guestmount (and similar tools) by default. > > See the commands log (on my ubuntu 19.10) below: > > ===8<=== > << try 1 >> > tmp$ mkdir tmpdir > tmp$ virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir tmp.img > libguestfs: error: /usr/bin/supermin exited with error status 1. > To see full error messages you may need to enable debugging. > Do: > export LIBGUESTFS_DEBUG=1 LIBGUESTFS_TRACE=1 > and run the command again. For further information, read: > http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs > You can also run 'libguestfs-test-tool' and post the *complete* output > into a bug report or message to the libguestfs mailing list. > > << try 2 >> > tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir > tmp.img > ... > supermin: kernel: kernel_version 5.3.0-62-generic > supermin: kernel: modpath /lib/modules/5.3.0-62-generic > cp: cannot open '/boot/vmlinuz-5.3.0-62-generic' for reading: Permission > denied > supermin: cp -p '/boot/vmlinuz-5.3.0-62-generic' > '/var/tmp/.guestfs-1000/appliance.d.op62psoy/kernel': command failed, see > earlier errors > libguestfs: error: /usr/bin/supermin exited with error status 1, see debug > messages above > ... > > << try 3 >> > tmp$ sudo chmod a+rw /boot/vmlinuz-5.3.0-62-generic > tmp$ LIBGUESTFS_DEBUG=1 virt-make-fs -t vfat -s +1M --partition=gpt ./tmpdir > tmp.img > ... > tmp$ ls -l tmp.img > -rw-r--r-- 1 akashi akashi 1341440 Jul 9 13:50 tmp.img > ===>8=== > > As you can see, virt-make-fs will fail on *standard* ubuntu. > You have to change the permission of the current kernel's binary. Yes, exactly. This is an intentional behavior in Ubuntu (and not Debian) and why we cannot rely on the various virt tools working. I fixed the current tests over in http://patchwork.ozlabs.org/project/uboot/patch/20200707155309.24770-1-tr...@konsulko.com/ but need to follow up and try what Stephen was saying to clean it up more still. > While I can't make sure, we will have the same issue with guestmount > as it will also create a minimum virtual machine before execution. > > What does it mean? > You must change the permission every time when you re-install the OS > or re-bump the kernel version. Obviously, I can't do that from my own > test script (without sudo). > So if you don't have any way (or workaround) to deal with it, > libguestfs-tools or guestmount cannot be a solution here. Well, just like the test_fs tests, we try guestmount, if it doesn't work we fall back to just sudo'ing what we need to run directly. I think Ubuntu did something very stupid here. I just don't know if moving CI to be Debian based (and I guess Travis is just working-around the issue by default for us, given the fs tests run there today) is good enough as it will leave everyone else's Ubuntu-based setups broken. -- Tom signature.asc Description: PGP signature
