diff mbox series

[1/2] test/py: efi_secboot: apply autopep8

Message ID 20200615231644.13250-1-takahiro.akashi@linaro.org
State Superseded, archived
Delegated to: Heinrich Schuchardt
Headers show
Series [1/2] test/py: efi_secboot: apply autopep8 | expand

Commit Message

AKASHI Takahiro June 15, 2020, 11:16 p.m. UTC
Python's autopep8 can automatically correct some of warnings from pylint
and rewrite the code in a pretty print format. So just do it.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 test/py/tests/test_efi_secboot/conftest.py    | 162 ++++++++++--------
 test/py/tests/test_efi_secboot/defs.py        |  14 +-
 .../py/tests/test_efi_secboot/test_authvar.py |   1 +
 test/py/tests/test_efi_secboot/test_signed.py |   1 +
 .../tests/test_efi_secboot/test_unsigned.py   |  37 ++--
 5 files changed, 118 insertions(+), 97 deletions(-)

Comments

Heinrich Schuchardt July 6, 2020, 10:45 a.m. UTC | #1
On 16.06.20 01:16, AKASHI Takahiro wrote:
> Python's autopep8 can automatically correct some of warnings from pylint
> and rewrite the code in a pretty print format. So just do it.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  test/py/tests/test_efi_secboot/conftest.py    | 162 ++++++++++--------
>  test/py/tests/test_efi_secboot/defs.py        |  14 +-
>  .../py/tests/test_efi_secboot/test_authvar.py |   1 +
>  test/py/tests/test_efi_secboot/test_signed.py |   1 +
>  .../tests/test_efi_secboot/test_unsigned.py   |  37 ++--
>  5 files changed, 118 insertions(+), 97 deletions(-)
>
> diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
> index 5ac0389064e8..f74b4b109a7b 100644
> --- a/test/py/tests/test_efi_secboot/conftest.py
> +++ b/test/py/tests/test_efi_secboot/conftest.py
> @@ -10,6 +10,8 @@ from subprocess import call, check_call, check_output, CalledProcessError
>  from defs import *
>
>  # from test/py/conftest.py
> +
> +
>  def tool_is_in_path(tool):
>      for path in os.environ["PATH"].split(os.pathsep):
>          fn = os.path.join(path, tool)
> @@ -20,13 +22,15 @@ def tool_is_in_path(tool):
>  #
>  # Fixture for UEFI secure boot test
>  #
> +
> +
>  @pytest.fixture(scope='session')
>  def efi_boot_env(request, u_boot_config):
>      """Set up a file system to be used in UEFI secure boot test.
>
>      Args:
>          request: Pytest request object.
> -	u_boot_config: U-boot configuration.
> +        u_boot_config: U-boot configuration.
>
>      Return:
>          A path to disk image to be used for testing
> @@ -48,20 +52,21 @@ def efi_boot_env(request, u_boot_config):
>
>          # create a disk/partition
>          check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
> -                            % (image_path, image_size), shell=True)
> +                   % (image_path, image_size), shell=True)
>          check_call('sgdisk %s -n 1:0:+%dMiB'
> -                            % (image_path, part_size), shell=True)
> +                   % (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)
> +                   % (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)
> +                   % (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()
> +        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)
> +                     % (fs_type, loop_dev, mnt_point), shell=True)
>
>          # suffix
>          # *.key: RSA private key in PEM
> @@ -73,75 +78,88 @@ def efi_boot_env(request, u_boot_config):
>          # *.efi.signed: signed UEFI image
>
>          # Create signature database
> -        ## PK
> -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365'
> -                            % mnt_point, shell=True)
> -        check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth'
> -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> -                            shell=True)
> -        ## PK_null for deletion
> -        check_call('cd %s; sleep 2; touch PK_null.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK_null.esl PK_null.auth'
> -                            % (mnt_point, EFITOOLS_PATH), shell=True)
> -        ## KEK
> -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365'
> -                            % mnt_point, shell=True)
> -        check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth'
> -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> -                            shell=True)
> -        ## db
> -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ -keyout db.key -out db.crt -nodes -days 365'
> -                            % mnt_point, shell=True)
> -        check_call('cd %s; %scert-to-efi-sig-list -g %s db.crt db.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth'
> -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> -                            shell=True)
> -        ## db1
> -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db1/ -keyout db1.key -out db1.crt -nodes -days 365'
> -                            % mnt_point, shell=True)
> -        check_call('cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db1.esl db1.auth'
> -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> -                            shell=True)
> -        ## db1-update
> -        check_call('cd %s; %ssign-efi-sig-list -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth'
> -                            % (mnt_point, EFITOOLS_PATH), shell=True)
> -        ## dbx (TEST_dbx certificate)
> -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365'
> -                            % mnt_point, shell=True)
> -        check_call('cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth'
> -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> -                            shell=True)
> -        ## dbx_hash (digest of TEST_db certificate)
> -        check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth'
> -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> -                            shell=True)
> -        ## dbx_hash1 (digest of TEST_db1 certificate)
> -        check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth'
> -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> -                            shell=True)
> -        ## dbx_db (with TEST_db certificate)
> -        check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth'
> -                            % (mnt_point, EFITOOLS_PATH),
> -                            shell=True)
> +        # PK
> +        check_call(
> +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365' %
> +            mnt_point,
> +            shell=True)
> +        check_call(
> +            'cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth' %
> +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> +        # PK_null for deletion
> +        check_call(
> +            'cd %s; sleep 2; touch PK_null.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK_null.esl PK_null.auth' %
> +            (mnt_point, EFITOOLS_PATH), shell=True)
> +        # KEK
> +        check_call(
> +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365' %
> +            mnt_point,
> +            shell=True)
> +        check_call(
> +            'cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth' %
> +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> +        # db
> +        check_call(
> +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ -keyout db.key -out db.crt -nodes -days 365' %
> +            mnt_point,
> +            shell=True)
> +        check_call(
> +            'cd %s; %scert-to-efi-sig-list -g %s db.crt db.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth' %
> +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> +        # db1
> +        check_call(
> +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db1/ -keyout db1.key -out db1.crt -nodes -days 365' %
> +            mnt_point,
> +            shell=True)
> +        check_call(
> +            'cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db1.esl db1.auth' %
> +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> +        # db1-update
> +        check_call(
> +            'cd %s; %ssign-efi-sig-list -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth' %
> +            (mnt_point, EFITOOLS_PATH), shell=True)
> +        # dbx (TEST_dbx certificate)
> +        check_call(
> +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365' %
> +            mnt_point,
> +            shell=True)
> +        check_call(
> +            'cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth' %
> +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> +        # dbx_hash (digest of TEST_db certificate)
> +        check_call(
> +            'cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth' %
> +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> +        # dbx_hash1 (digest of TEST_db1 certificate)
> +        check_call(
> +            'cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth' %
> +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> +        # dbx_db (with TEST_db certificate)
> +        check_call(
> +            'cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth' %
> +            (mnt_point, EFITOOLS_PATH), shell=True)
>
>          # Copy image
>          check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
>
> -        ## Sign image
> +        # Sign image
>          check_call('cd %s; sbsign --key db.key --cert db.crt helloworld.efi'
> -                            % mnt_point, shell=True)
> -        ## Sign already-signed image with another key
> -        check_call('cd %s; sbsign --key db1.key --cert db1.crt --output helloworld.efi.signed_2sigs helloworld.efi.signed'
> -                            % mnt_point, shell=True)
> -        ## Digest image
> -        check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth'
> -                            % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
> -                            shell=True)
> -        check_call('cd %s; %shash-to-efi-sig-list helloworld.efi.signed db_hello_signed.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello_signed.hash db_hello_signed.auth'
> -                            % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
> -                            shell=True)
> -        check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db_hello_signed.hash dbx_hello_signed.auth'
> -                            % (mnt_point, EFITOOLS_PATH),
> -                            shell=True)
> -
> +                   % mnt_point, shell=True)
> +        # Sign already-signed image with another key
> +        check_call(
> +            'cd %s; sbsign --key db1.key --cert db1.crt --output helloworld.efi.signed_2sigs helloworld.efi.signed' %
> +            mnt_point,
> +            shell=True)

Please, use the format() method.

See a discussion here:
https://realpython.com/python-string-formatting/#4-template-strings-standard-library

Best regards

Heinrich

> +        # Digest image
> +        check_call(
> +            'cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth' %
> +            (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True)
> +        check_call(
> +            'cd %s; %shash-to-efi-sig-list helloworld.efi.signed db_hello_signed.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello_signed.hash db_hello_signed.auth' %
> +            (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True)
> +        check_call(
> +            'cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db_hello_signed.hash dbx_hello_signed.auth' %
> +            (mnt_point, EFITOOLS_PATH), shell=True)
>
>          check_call('sudo umount %s' % loop_dev, shell=True)
>          check_call('sudo losetup -d %s' % loop_dev, shell=True)
> diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py
> index d6222809c547..099f453979ff 100644
> --- a/test/py/tests/test_efi_secboot/defs.py
> +++ b/test/py/tests/test_efi_secboot/defs.py
> @@ -1,21 +1,21 @@
>  # SPDX-License-Identifier:      GPL-2.0+
>
>  # Disk image name
> -EFI_SECBOOT_IMAGE_NAME='test_efi_secboot.img'
> +EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
>
>  # Size in MiB
> -EFI_SECBOOT_IMAGE_SIZE=16
> -EFI_SECBOOT_PART_SIZE=8
> +EFI_SECBOOT_IMAGE_SIZE = 16
> +EFI_SECBOOT_PART_SIZE = 8
>
>  # Partition file system type
> -EFI_SECBOOT_FS_TYPE='vfat'
> +EFI_SECBOOT_FS_TYPE = 'vfat'
>
>  # Owner guid
> -GUID='11111111-2222-3333-4444-123456789abc'
> +GUID = '11111111-2222-3333-4444-123456789abc'
>
>  # v1.5.1 or earlier of efitools has a bug in sha256 calculation, and
>  # you need build a newer version on your own.
> -EFITOOLS_PATH=''
> +EFITOOLS_PATH = ''
>
>  # Hello World application for sandbox
> -HELLO_PATH=''
> +HELLO_PATH = ''
> diff --git a/test/py/tests/test_efi_secboot/test_authvar.py b/test/py/tests/test_efi_secboot/test_authvar.py
> index 148aa3123e4f..359adba4b4b7 100644
> --- a/test/py/tests/test_efi_secboot/test_authvar.py
> +++ b/test/py/tests/test_efi_secboot/test_authvar.py
> @@ -11,6 +11,7 @@ This test verifies variable authentication
>  import pytest
>  from defs import *
>
> +
>  @pytest.mark.boardspec('sandbox')
>  @pytest.mark.buildconfigspec('efi_secure_boot')
>  @pytest.mark.buildconfigspec('cmd_fat')
> diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
> index 441f4906c865..c100832a2375 100644
> --- a/test/py/tests/test_efi_secboot/test_signed.py
> +++ b/test/py/tests/test_efi_secboot/test_signed.py
> @@ -11,6 +11,7 @@ This test verifies image authentication for signed images.
>  import pytest
>  from defs import *
>
> +
>  @pytest.mark.boardspec('sandbox')
>  @pytest.mark.buildconfigspec('efi_secure_boot')
>  @pytest.mark.buildconfigspec('cmd_efidebug')
> diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
> index c42c5ddc4774..3748b51ee7e9 100644
> --- a/test/py/tests/test_efi_secboot/test_unsigned.py
> +++ b/test/py/tests/test_efi_secboot/test_unsigned.py
> @@ -11,6 +11,7 @@ This test verifies image authentication for unsigned images.
>  import pytest
>  from defs import *
>
> +
>  @pytest.mark.boardspec('sandbox')
>  @pytest.mark.buildconfigspec('efi_secure_boot')
>  @pytest.mark.buildconfigspec('cmd_efidebug')
> @@ -28,10 +29,10 @@ class TestEfiUnsignedImage(object):
>              # Test Case 1
>              output = u_boot_console.run_command_list([
>                  'host bind 0 %s' % disk_img,
> -		'fatload host 0:1 4000000 KEK.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> -		'fatload host 0:1 4000000 PK.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> +                'fatload host 0:1 4000000 KEK.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> +                'fatload host 0:1 4000000 PK.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
>              assert(not 'Failed to set EFI variable' in ''.join(output))
>
>              output = u_boot_console.run_command_list([
> @@ -55,12 +56,12 @@ class TestEfiUnsignedImage(object):
>              # Test Case 2
>              output = u_boot_console.run_command_list([
>                  'host bind 0 %s' % disk_img,
> -		'fatload host 0:1 4000000 db_hello.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> -		'fatload host 0:1 4000000 KEK.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> -		'fatload host 0:1 4000000 PK.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> +                'fatload host 0:1 4000000 db_hello.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> +                'fatload host 0:1 4000000 KEK.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> +                'fatload host 0:1 4000000 PK.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
>              assert(not 'Failed to set EFI variable' in ''.join(output))
>
>              output = u_boot_console.run_command_list([
> @@ -79,12 +80,12 @@ class TestEfiUnsignedImage(object):
>              # Test Case 3a, rejected by dbx
>              output = u_boot_console.run_command_list([
>                  'host bind 0 %s' % disk_img,
> -		'fatload host 0:1 4000000 db_hello.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> -		'fatload host 0:1 4000000 KEK.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> -		'fatload host 0:1 4000000 PK.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> +                'fatload host 0:1 4000000 db_hello.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> +                'fatload host 0:1 4000000 KEK.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> +                'fatload host 0:1 4000000 PK.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
>              assert(not 'Failed to set EFI variable' in ''.join(output))
>
>              output = u_boot_console.run_command_list([
> @@ -101,8 +102,8 @@ class TestEfiUnsignedImage(object):
>          with u_boot_console.log.section('Test Case 3b'):
>              # Test Case 3b, rejected by dbx even if db allows
>              output = u_boot_console.run_command_list([
> -		'fatload host 0:1 4000000 db_hello.auth',
> -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize db'])
> +                'fatload host 0:1 4000000 db_hello.auth',
> +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize db'])
>              assert(not 'Failed to set EFI variable' in ''.join(output))
>
>              output = u_boot_console.run_command_list([
>
AKASHI Takahiro July 8, 2020, 4:22 a.m. UTC | #2
On Mon, Jul 06, 2020 at 12:45:54PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 01:16, AKASHI Takahiro wrote:
> > Python's autopep8 can automatically correct some of warnings from pylint
> > and rewrite the code in a pretty print format. So just do it.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> >  test/py/tests/test_efi_secboot/conftest.py    | 162 ++++++++++--------
> >  test/py/tests/test_efi_secboot/defs.py        |  14 +-
> >  .../py/tests/test_efi_secboot/test_authvar.py |   1 +
> >  test/py/tests/test_efi_secboot/test_signed.py |   1 +
> >  .../tests/test_efi_secboot/test_unsigned.py   |  37 ++--
> >  5 files changed, 118 insertions(+), 97 deletions(-)
> >
> > diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
> > index 5ac0389064e8..f74b4b109a7b 100644
> > --- a/test/py/tests/test_efi_secboot/conftest.py
> > +++ b/test/py/tests/test_efi_secboot/conftest.py
> > @@ -10,6 +10,8 @@ from subprocess import call, check_call, check_output, CalledProcessError
> >  from defs import *
> >
> >  # from test/py/conftest.py
> > +
> > +
> >  def tool_is_in_path(tool):
> >      for path in os.environ["PATH"].split(os.pathsep):
> >          fn = os.path.join(path, tool)
> > @@ -20,13 +22,15 @@ def tool_is_in_path(tool):
> >  #
> >  # Fixture for UEFI secure boot test
> >  #
> > +
> > +
> >  @pytest.fixture(scope='session')
> >  def efi_boot_env(request, u_boot_config):
> >      """Set up a file system to be used in UEFI secure boot test.
> >
> >      Args:
> >          request: Pytest request object.
> > -	u_boot_config: U-boot configuration.
> > +        u_boot_config: U-boot configuration.
> >
> >      Return:
> >          A path to disk image to be used for testing
> > @@ -48,20 +52,21 @@ def efi_boot_env(request, u_boot_config):
> >
> >          # create a disk/partition
> >          check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
> > -                            % (image_path, image_size), shell=True)
> > +                   % (image_path, image_size), shell=True)
> >          check_call('sgdisk %s -n 1:0:+%dMiB'
> > -                            % (image_path, part_size), shell=True)
> > +                   % (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)
> > +                   % (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)
> > +                   % (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()
> > +        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)
> > +                     % (fs_type, loop_dev, mnt_point), shell=True)
> >
> >          # suffix
> >          # *.key: RSA private key in PEM
> > @@ -73,75 +78,88 @@ def efi_boot_env(request, u_boot_config):
> >          # *.efi.signed: signed UEFI image
> >
> >          # Create signature database
> > -        ## PK
> > -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365'
> > -                            % mnt_point, shell=True)
> > -        check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > -                            shell=True)
> > -        ## PK_null for deletion
> > -        check_call('cd %s; sleep 2; touch PK_null.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK_null.esl PK_null.auth'
> > -                            % (mnt_point, EFITOOLS_PATH), shell=True)
> > -        ## KEK
> > -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365'
> > -                            % mnt_point, shell=True)
> > -        check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > -                            shell=True)
> > -        ## db
> > -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ -keyout db.key -out db.crt -nodes -days 365'
> > -                            % mnt_point, shell=True)
> > -        check_call('cd %s; %scert-to-efi-sig-list -g %s db.crt db.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > -                            shell=True)
> > -        ## db1
> > -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db1/ -keyout db1.key -out db1.crt -nodes -days 365'
> > -                            % mnt_point, shell=True)
> > -        check_call('cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db1.esl db1.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > -                            shell=True)
> > -        ## db1-update
> > -        check_call('cd %s; %ssign-efi-sig-list -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth'
> > -                            % (mnt_point, EFITOOLS_PATH), shell=True)
> > -        ## dbx (TEST_dbx certificate)
> > -        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365'
> > -                            % mnt_point, shell=True)
> > -        check_call('cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > -                            shell=True)
> > -        ## dbx_hash (digest of TEST_db certificate)
> > -        check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > -                            shell=True)
> > -        ## dbx_hash1 (digest of TEST_db1 certificate)
> > -        check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > -                            shell=True)
> > -        ## dbx_db (with TEST_db certificate)
> > -        check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth'
> > -                            % (mnt_point, EFITOOLS_PATH),
> > -                            shell=True)
> > +        # PK
> > +        check_call(
> > +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365' %
> > +            mnt_point,
> > +            shell=True)
> > +        check_call(
> > +            'cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth' %
> > +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> > +        # PK_null for deletion
> > +        check_call(
> > +            'cd %s; sleep 2; touch PK_null.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK_null.esl PK_null.auth' %
> > +            (mnt_point, EFITOOLS_PATH), shell=True)
> > +        # KEK
> > +        check_call(
> > +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365' %
> > +            mnt_point,
> > +            shell=True)
> > +        check_call(
> > +            'cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth' %
> > +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> > +        # db
> > +        check_call(
> > +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ -keyout db.key -out db.crt -nodes -days 365' %
> > +            mnt_point,
> > +            shell=True)
> > +        check_call(
> > +            'cd %s; %scert-to-efi-sig-list -g %s db.crt db.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth' %
> > +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> > +        # db1
> > +        check_call(
> > +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db1/ -keyout db1.key -out db1.crt -nodes -days 365' %
> > +            mnt_point,
> > +            shell=True)
> > +        check_call(
> > +            'cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db1.esl db1.auth' %
> > +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> > +        # db1-update
> > +        check_call(
> > +            'cd %s; %ssign-efi-sig-list -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth' %
> > +            (mnt_point, EFITOOLS_PATH), shell=True)
> > +        # dbx (TEST_dbx certificate)
> > +        check_call(
> > +            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365' %
> > +            mnt_point,
> > +            shell=True)
> > +        check_call(
> > +            'cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth' %
> > +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> > +        # dbx_hash (digest of TEST_db certificate)
> > +        check_call(
> > +            'cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth' %
> > +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> > +        # dbx_hash1 (digest of TEST_db1 certificate)
> > +        check_call(
> > +            'cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth' %
> > +            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
> > +        # dbx_db (with TEST_db certificate)
> > +        check_call(
> > +            'cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth' %
> > +            (mnt_point, EFITOOLS_PATH), shell=True)
> >
> >          # Copy image
> >          check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
> >
> > -        ## Sign image
> > +        # Sign image
> >          check_call('cd %s; sbsign --key db.key --cert db.crt helloworld.efi'
> > -                            % mnt_point, shell=True)
> > -        ## Sign already-signed image with another key
> > -        check_call('cd %s; sbsign --key db1.key --cert db1.crt --output helloworld.efi.signed_2sigs helloworld.efi.signed'
> > -                            % mnt_point, shell=True)
> > -        ## Digest image
> > -        check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
> > -                            shell=True)
> > -        check_call('cd %s; %shash-to-efi-sig-list helloworld.efi.signed db_hello_signed.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello_signed.hash db_hello_signed.auth'
> > -                            % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
> > -                            shell=True)
> > -        check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db_hello_signed.hash dbx_hello_signed.auth'
> > -                            % (mnt_point, EFITOOLS_PATH),
> > -                            shell=True)
> > -
> > +                   % mnt_point, shell=True)
> > +        # Sign already-signed image with another key
> > +        check_call(
> > +            'cd %s; sbsign --key db1.key --cert db1.crt --output helloworld.efi.signed_2sigs helloworld.efi.signed' %
> > +            mnt_point,
> > +            shell=True)

This patch will be included in a next version (v3) of follow-up patch.
> 
> Please, use the format() method.

This comment is not related to this patch.

In addition, even after reading the link (and discussions in python
ML referred to in this article), I don't see any benefit of using
.format() in this context.
As test_efi_secboot has already been merged, I won't make changes.

FYI, I'd prefer to use "f-string" which was introduced in Python3.6
if readability is a problem.

-Takahiro Akashi

> See a discussion here:
> https://realpython.com/python-string-formatting/#4-template-strings-standard-library
> 
> Best regards
> 
> Heinrich
> 
> > +        # Digest image
> > +        check_call(
> > +            'cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth' %
> > +            (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True)
> > +        check_call(
> > +            'cd %s; %shash-to-efi-sig-list helloworld.efi.signed db_hello_signed.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello_signed.hash db_hello_signed.auth' %
> > +            (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True)
> > +        check_call(
> > +            'cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db_hello_signed.hash dbx_hello_signed.auth' %
> > +            (mnt_point, EFITOOLS_PATH), shell=True)
> >
> >          check_call('sudo umount %s' % loop_dev, shell=True)
> >          check_call('sudo losetup -d %s' % loop_dev, shell=True)
> > diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py
> > index d6222809c547..099f453979ff 100644
> > --- a/test/py/tests/test_efi_secboot/defs.py
> > +++ b/test/py/tests/test_efi_secboot/defs.py
> > @@ -1,21 +1,21 @@
> >  # SPDX-License-Identifier:      GPL-2.0+
> >
> >  # Disk image name
> > -EFI_SECBOOT_IMAGE_NAME='test_efi_secboot.img'
> > +EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
> >
> >  # Size in MiB
> > -EFI_SECBOOT_IMAGE_SIZE=16
> > -EFI_SECBOOT_PART_SIZE=8
> > +EFI_SECBOOT_IMAGE_SIZE = 16
> > +EFI_SECBOOT_PART_SIZE = 8
> >
> >  # Partition file system type
> > -EFI_SECBOOT_FS_TYPE='vfat'
> > +EFI_SECBOOT_FS_TYPE = 'vfat'
> >
> >  # Owner guid
> > -GUID='11111111-2222-3333-4444-123456789abc'
> > +GUID = '11111111-2222-3333-4444-123456789abc'
> >
> >  # v1.5.1 or earlier of efitools has a bug in sha256 calculation, and
> >  # you need build a newer version on your own.
> > -EFITOOLS_PATH=''
> > +EFITOOLS_PATH = ''
> >
> >  # Hello World application for sandbox
> > -HELLO_PATH=''
> > +HELLO_PATH = ''
> > diff --git a/test/py/tests/test_efi_secboot/test_authvar.py b/test/py/tests/test_efi_secboot/test_authvar.py
> > index 148aa3123e4f..359adba4b4b7 100644
> > --- a/test/py/tests/test_efi_secboot/test_authvar.py
> > +++ b/test/py/tests/test_efi_secboot/test_authvar.py
> > @@ -11,6 +11,7 @@ This test verifies variable authentication
> >  import pytest
> >  from defs import *
> >
> > +
> >  @pytest.mark.boardspec('sandbox')
> >  @pytest.mark.buildconfigspec('efi_secure_boot')
> >  @pytest.mark.buildconfigspec('cmd_fat')
> > diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
> > index 441f4906c865..c100832a2375 100644
> > --- a/test/py/tests/test_efi_secboot/test_signed.py
> > +++ b/test/py/tests/test_efi_secboot/test_signed.py
> > @@ -11,6 +11,7 @@ This test verifies image authentication for signed images.
> >  import pytest
> >  from defs import *
> >
> > +
> >  @pytest.mark.boardspec('sandbox')
> >  @pytest.mark.buildconfigspec('efi_secure_boot')
> >  @pytest.mark.buildconfigspec('cmd_efidebug')
> > diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
> > index c42c5ddc4774..3748b51ee7e9 100644
> > --- a/test/py/tests/test_efi_secboot/test_unsigned.py
> > +++ b/test/py/tests/test_efi_secboot/test_unsigned.py
> > @@ -11,6 +11,7 @@ This test verifies image authentication for unsigned images.
> >  import pytest
> >  from defs import *
> >
> > +
> >  @pytest.mark.boardspec('sandbox')
> >  @pytest.mark.buildconfigspec('efi_secure_boot')
> >  @pytest.mark.buildconfigspec('cmd_efidebug')
> > @@ -28,10 +29,10 @@ class TestEfiUnsignedImage(object):
> >              # Test Case 1
> >              output = u_boot_console.run_command_list([
> >                  'host bind 0 %s' % disk_img,
> > -		'fatload host 0:1 4000000 KEK.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > -		'fatload host 0:1 4000000 PK.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > +                'fatload host 0:1 4000000 KEK.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > +                'fatload host 0:1 4000000 PK.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> >              assert(not 'Failed to set EFI variable' in ''.join(output))
> >
> >              output = u_boot_console.run_command_list([
> > @@ -55,12 +56,12 @@ class TestEfiUnsignedImage(object):
> >              # Test Case 2
> >              output = u_boot_console.run_command_list([
> >                  'host bind 0 %s' % disk_img,
> > -		'fatload host 0:1 4000000 db_hello.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > -		'fatload host 0:1 4000000 KEK.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > -		'fatload host 0:1 4000000 PK.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > +                'fatload host 0:1 4000000 db_hello.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
> > +                'fatload host 0:1 4000000 KEK.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > +                'fatload host 0:1 4000000 PK.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> >              assert(not 'Failed to set EFI variable' in ''.join(output))
> >
> >              output = u_boot_console.run_command_list([
> > @@ -79,12 +80,12 @@ class TestEfiUnsignedImage(object):
> >              # Test Case 3a, rejected by dbx
> >              output = u_boot_console.run_command_list([
> >                  'host bind 0 %s' % disk_img,
> > -		'fatload host 0:1 4000000 db_hello.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> > -		'fatload host 0:1 4000000 KEK.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > -		'fatload host 0:1 4000000 PK.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> > +                'fatload host 0:1 4000000 db_hello.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
> > +                'fatload host 0:1 4000000 KEK.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
> > +                'fatload host 0:1 4000000 PK.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
> >              assert(not 'Failed to set EFI variable' in ''.join(output))
> >
> >              output = u_boot_console.run_command_list([
> > @@ -101,8 +102,8 @@ class TestEfiUnsignedImage(object):
> >          with u_boot_console.log.section('Test Case 3b'):
> >              # Test Case 3b, rejected by dbx even if db allows
> >              output = u_boot_console.run_command_list([
> > -		'fatload host 0:1 4000000 db_hello.auth',
> > -		'setenv -e -nv -bs -rt -at -i 4000000,$filesize db'])
> > +                'fatload host 0:1 4000000 db_hello.auth',
> > +                'setenv -e -nv -bs -rt -at -i 4000000,$filesize db'])
> >              assert(not 'Failed to set EFI variable' in ''.join(output))
> >
> >              output = u_boot_console.run_command_list([
> >
>
diff mbox series

Patch

diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
index 5ac0389064e8..f74b4b109a7b 100644
--- a/test/py/tests/test_efi_secboot/conftest.py
+++ b/test/py/tests/test_efi_secboot/conftest.py
@@ -10,6 +10,8 @@  from subprocess import call, check_call, check_output, CalledProcessError
 from defs import *
 
 # from test/py/conftest.py
+
+
 def tool_is_in_path(tool):
     for path in os.environ["PATH"].split(os.pathsep):
         fn = os.path.join(path, tool)
@@ -20,13 +22,15 @@  def tool_is_in_path(tool):
 #
 # Fixture for UEFI secure boot test
 #
+
+
 @pytest.fixture(scope='session')
 def efi_boot_env(request, u_boot_config):
     """Set up a file system to be used in UEFI secure boot test.
 
     Args:
         request: Pytest request object.
-	u_boot_config: U-boot configuration.
+        u_boot_config: U-boot configuration.
 
     Return:
         A path to disk image to be used for testing
@@ -48,20 +52,21 @@  def efi_boot_env(request, u_boot_config):
 
         # create a disk/partition
         check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
-                            % (image_path, image_size), shell=True)
+                   % (image_path, image_size), shell=True)
         check_call('sgdisk %s -n 1:0:+%dMiB'
-                            % (image_path, part_size), shell=True)
+                   % (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)
+                   % (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)
+                   % (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()
+        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)
+                     % (fs_type, loop_dev, mnt_point), shell=True)
 
         # suffix
         # *.key: RSA private key in PEM
@@ -73,75 +78,88 @@  def efi_boot_env(request, u_boot_config):
         # *.efi.signed: signed UEFI image
 
         # Create signature database
-        ## PK
-        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365'
-                            % mnt_point, shell=True)
-        check_call('cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth'
-                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
-                            shell=True)
-        ## PK_null for deletion
-        check_call('cd %s; sleep 2; touch PK_null.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK_null.esl PK_null.auth'
-                            % (mnt_point, EFITOOLS_PATH), shell=True)
-        ## KEK
-        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365'
-                            % mnt_point, shell=True)
-        check_call('cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth'
-                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
-                            shell=True)
-        ## db
-        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ -keyout db.key -out db.crt -nodes -days 365'
-                            % mnt_point, shell=True)
-        check_call('cd %s; %scert-to-efi-sig-list -g %s db.crt db.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth'
-                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
-                            shell=True)
-        ## db1
-        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db1/ -keyout db1.key -out db1.crt -nodes -days 365'
-                            % mnt_point, shell=True)
-        check_call('cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db1.esl db1.auth'
-                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
-                            shell=True)
-        ## db1-update
-        check_call('cd %s; %ssign-efi-sig-list -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth'
-                            % (mnt_point, EFITOOLS_PATH), shell=True)
-        ## dbx (TEST_dbx certificate)
-        check_call('cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365'
-                            % mnt_point, shell=True)
-        check_call('cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth'
-                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
-                            shell=True)
-        ## dbx_hash (digest of TEST_db certificate)
-        check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth'
-                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
-                            shell=True)
-        ## dbx_hash1 (digest of TEST_db1 certificate)
-        check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth'
-                            % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
-                            shell=True)
-        ## dbx_db (with TEST_db certificate)
-        check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth'
-                            % (mnt_point, EFITOOLS_PATH),
-                            shell=True)
+        # PK
+        check_call(
+            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout PK.key -out PK.crt -nodes -days 365' %
+            mnt_point,
+            shell=True)
+        check_call(
+            'cd %s; %scert-to-efi-sig-list -g %s PK.crt PK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth' %
+            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
+        # PK_null for deletion
+        check_call(
+            'cd %s; sleep 2; touch PK_null.esl; %ssign-efi-sig-list -c PK.crt -k PK.key PK PK_null.esl PK_null.auth' %
+            (mnt_point, EFITOOLS_PATH), shell=True)
+        # KEK
+        check_call(
+            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout KEK.key -out KEK.crt -nodes -days 365' %
+            mnt_point,
+            shell=True)
+        check_call(
+            'cd %s; %scert-to-efi-sig-list -g %s KEK.crt KEK.esl; %ssign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth' %
+            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
+        # db
+        check_call(
+            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ -keyout db.key -out db.crt -nodes -days 365' %
+            mnt_point,
+            shell=True)
+        check_call(
+            'cd %s; %scert-to-efi-sig-list -g %s db.crt db.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth' %
+            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
+        # db1
+        check_call(
+            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db1/ -keyout db1.key -out db1.crt -nodes -days 365' %
+            mnt_point,
+            shell=True)
+        check_call(
+            'cd %s; %scert-to-efi-sig-list -g %s db1.crt db1.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db1.esl db1.auth' %
+            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
+        # db1-update
+        check_call(
+            'cd %s; %ssign-efi-sig-list -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth' %
+            (mnt_point, EFITOOLS_PATH), shell=True)
+        # dbx (TEST_dbx certificate)
+        check_call(
+            'cd %s; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_dbx/ -keyout dbx.key -out dbx.crt -nodes -days 365' %
+            mnt_point,
+            shell=True)
+        check_call(
+            'cd %s; %scert-to-efi-sig-list -g %s dbx.crt dbx.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx.esl dbx.auth' %
+            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
+        # dbx_hash (digest of TEST_db certificate)
+        check_call(
+            'cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db.crt dbx_hash.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash.crl dbx_hash.auth' %
+            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
+        # dbx_hash1 (digest of TEST_db1 certificate)
+        check_call(
+            'cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 db1.crt dbx_hash1.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx dbx_hash1.crl dbx_hash1.auth' %
+            (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH), shell=True)
+        # dbx_db (with TEST_db certificate)
+        check_call(
+            'cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth' %
+            (mnt_point, EFITOOLS_PATH), shell=True)
 
         # Copy image
         check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
 
-        ## Sign image
+        # Sign image
         check_call('cd %s; sbsign --key db.key --cert db.crt helloworld.efi'
-                            % mnt_point, shell=True)
-        ## Sign already-signed image with another key
-        check_call('cd %s; sbsign --key db1.key --cert db1.crt --output helloworld.efi.signed_2sigs helloworld.efi.signed'
-                            % mnt_point, shell=True)
-        ## Digest image
-        check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth'
-                            % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
-                            shell=True)
-        check_call('cd %s; %shash-to-efi-sig-list helloworld.efi.signed db_hello_signed.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello_signed.hash db_hello_signed.auth'
-                            % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
-                            shell=True)
-        check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db_hello_signed.hash dbx_hello_signed.auth'
-                            % (mnt_point, EFITOOLS_PATH),
-                            shell=True)
-
+                   % mnt_point, shell=True)
+        # Sign already-signed image with another key
+        check_call(
+            'cd %s; sbsign --key db1.key --cert db1.crt --output helloworld.efi.signed_2sigs helloworld.efi.signed' %
+            mnt_point,
+            shell=True)
+        # Digest image
+        check_call(
+            'cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth' %
+            (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True)
+        check_call(
+            'cd %s; %shash-to-efi-sig-list helloworld.efi.signed db_hello_signed.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db db_hello_signed.hash db_hello_signed.auth' %
+            (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True)
+        check_call(
+            'cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx db_hello_signed.hash dbx_hello_signed.auth' %
+            (mnt_point, EFITOOLS_PATH), shell=True)
 
         check_call('sudo umount %s' % loop_dev, shell=True)
         check_call('sudo losetup -d %s' % loop_dev, shell=True)
diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py
index d6222809c547..099f453979ff 100644
--- a/test/py/tests/test_efi_secboot/defs.py
+++ b/test/py/tests/test_efi_secboot/defs.py
@@ -1,21 +1,21 @@ 
 # SPDX-License-Identifier:      GPL-2.0+
 
 # Disk image name
-EFI_SECBOOT_IMAGE_NAME='test_efi_secboot.img'
+EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img'
 
 # Size in MiB
-EFI_SECBOOT_IMAGE_SIZE=16
-EFI_SECBOOT_PART_SIZE=8
+EFI_SECBOOT_IMAGE_SIZE = 16
+EFI_SECBOOT_PART_SIZE = 8
 
 # Partition file system type
-EFI_SECBOOT_FS_TYPE='vfat'
+EFI_SECBOOT_FS_TYPE = 'vfat'
 
 # Owner guid
-GUID='11111111-2222-3333-4444-123456789abc'
+GUID = '11111111-2222-3333-4444-123456789abc'
 
 # v1.5.1 or earlier of efitools has a bug in sha256 calculation, and
 # you need build a newer version on your own.
-EFITOOLS_PATH=''
+EFITOOLS_PATH = ''
 
 # Hello World application for sandbox
-HELLO_PATH=''
+HELLO_PATH = ''
diff --git a/test/py/tests/test_efi_secboot/test_authvar.py b/test/py/tests/test_efi_secboot/test_authvar.py
index 148aa3123e4f..359adba4b4b7 100644
--- a/test/py/tests/test_efi_secboot/test_authvar.py
+++ b/test/py/tests/test_efi_secboot/test_authvar.py
@@ -11,6 +11,7 @@  This test verifies variable authentication
 import pytest
 from defs import *
 
+
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('efi_secure_boot')
 @pytest.mark.buildconfigspec('cmd_fat')
diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
index 441f4906c865..c100832a2375 100644
--- a/test/py/tests/test_efi_secboot/test_signed.py
+++ b/test/py/tests/test_efi_secboot/test_signed.py
@@ -11,6 +11,7 @@  This test verifies image authentication for signed images.
 import pytest
 from defs import *
 
+
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('efi_secure_boot')
 @pytest.mark.buildconfigspec('cmd_efidebug')
diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
index c42c5ddc4774..3748b51ee7e9 100644
--- a/test/py/tests/test_efi_secboot/test_unsigned.py
+++ b/test/py/tests/test_efi_secboot/test_unsigned.py
@@ -11,6 +11,7 @@  This test verifies image authentication for unsigned images.
 import pytest
 from defs import *
 
+
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('efi_secure_boot')
 @pytest.mark.buildconfigspec('cmd_efidebug')
@@ -28,10 +29,10 @@  class TestEfiUnsignedImage(object):
             # Test Case 1
             output = u_boot_console.run_command_list([
                 'host bind 0 %s' % disk_img,
-		'fatload host 0:1 4000000 KEK.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
-		'fatload host 0:1 4000000 PK.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
+                'fatload host 0:1 4000000 KEK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
+                'fatload host 0:1 4000000 PK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
             assert(not 'Failed to set EFI variable' in ''.join(output))
 
             output = u_boot_console.run_command_list([
@@ -55,12 +56,12 @@  class TestEfiUnsignedImage(object):
             # Test Case 2
             output = u_boot_console.run_command_list([
                 'host bind 0 %s' % disk_img,
-		'fatload host 0:1 4000000 db_hello.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
-		'fatload host 0:1 4000000 KEK.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
-		'fatload host 0:1 4000000 PK.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
+                'fatload host 0:1 4000000 db_hello.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize db',
+                'fatload host 0:1 4000000 KEK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
+                'fatload host 0:1 4000000 PK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
             assert(not 'Failed to set EFI variable' in ''.join(output))
 
             output = u_boot_console.run_command_list([
@@ -79,12 +80,12 @@  class TestEfiUnsignedImage(object):
             # Test Case 3a, rejected by dbx
             output = u_boot_console.run_command_list([
                 'host bind 0 %s' % disk_img,
-		'fatload host 0:1 4000000 db_hello.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
-		'fatload host 0:1 4000000 KEK.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
-		'fatload host 0:1 4000000 PK.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
+                'fatload host 0:1 4000000 db_hello.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx',
+                'fatload host 0:1 4000000 KEK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize KEK',
+                'fatload host 0:1 4000000 PK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize PK'])
             assert(not 'Failed to set EFI variable' in ''.join(output))
 
             output = u_boot_console.run_command_list([
@@ -101,8 +102,8 @@  class TestEfiUnsignedImage(object):
         with u_boot_console.log.section('Test Case 3b'):
             # Test Case 3b, rejected by dbx even if db allows
             output = u_boot_console.run_command_list([
-		'fatload host 0:1 4000000 db_hello.auth',
-		'setenv -e -nv -bs -rt -at -i 4000000,$filesize db'])
+                'fatload host 0:1 4000000 db_hello.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000,$filesize db'])
             assert(not 'Failed to set EFI variable' in ''.join(output))
 
             output = u_boot_console.run_command_list([