On 7/20/20 7:52 AM, AKASHI Takahiro wrote:
Heinrich,
On Fri, Jul 17, 2020 at 12:29:06PM +0200, Heinrich Schuchardt wrote:
On 17.07.20 09:16, 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 takahiro.akashi@linaro.org
Thanks for providing all these tests and for rebasing on 53ce9a6ed98b6 ("test: use virt-make-fs to build image").
You should have run autopep8 before your patch:)
Essentially this patch could have been split into two:
- style corrections for existing code
- new tests
Will split this patch into several commits.
Unfortunatly the setup not working correctly. 'make tests' shows:
test/py/tests/test_efi_secboot/test_authvar.py FFFFF test/py/tests/test_efi_secboot/test_signed.py .F..FF test/py/tests/test_efi_secboot/test_signed_intca.py sss test/py/tests/test_efi_secboot/test_unsigned.py ...
As long as I run the tests in my local environment, I've never seen any failures.
Was that after rebasing on efi-2020-10?
SKIPPED [3] test/py/tests/test_efi_secboot/conftest.py:254: Setup failed: cd build-sandbox/mnt_efisecure; sbsign --key TestCert.key --cert TestCert.crt --addcert TestSub.crt --out helloworld.efi.signed_ab helloworld.efi
Please read the cover letter: ===8<=== Prerequisite ============ All the required patches have been merged. You can fetch the whole workable repository from here[1].
One patch[2] to sbsigntools must also be applied so that we wil be able to sign an image with intermediate certificates. It is required here for testing.
(snip)
Test
- The added new pytest (test_signed_intca.py) passed locally.
- Travis CI passed, except the new pytest added here due to a new feature in sbsigntools as mentioned above. (the latest vesion is still running though.)
===>8===
Travis CI skips the tests currently:
test/py/tests/test_efi_secboot/test_authvar.py sssss test/py/tests/test_efi_secboot/test_signed.py ssssss test/py/tests/test_efi_secboot/test_unsigned.py sss
Tom did not apply https://patchwork.ozlabs.org/project/uboot/patch/20200714061856.4487-1-xypro... .
You would have to exchange the Dockerfile at the top of .
I guess that you are not using the latest source of sbsigntools.
I am using Debian sbsigntool version: 0.9.2-2.
If you want to use any patched version of sbsigntool for testing, you will have to proved the necessary patch for the Dockerfile in https://gitlab.denx.de/u-boot/gitlab-ci-runner.git
and you will have to build the same sbsigntool for Travis.
If you replace as follows in test/run you get the extra skip messages:
%s/--bd/-ra --bd/g
test/py/tests/test_efi_secboot/conftest.py | 134 ++++++++++++++++- test/py/tests/test_efi_secboot/defs.py | 8 +- test/py/tests/test_efi_secboot/openssl.cnf | 48 +++++++ .../test_efi_secboot/test_signed_intca.py | 135 ++++++++++++++++++ 4 files changed, 317 insertions(+), 8 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 c6709700a876..20d0cbf3ab01 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'
I would prefer a separate constant for EFI_SECBOOT_IMAGE_NAME + '_intca.img' to ensure that conftest.py and test_signed_intca.py use the same value.
'separate constant to use the same value?' I don't get your point.
Anyhow, *.py files don't use a image file name directly, but get it from a test fixture. So I don't think that we need any change here.
It does not make sense to me to define a constant for half of the file name and still relying on the rest to match between the different Python tests. Please, provide constants for the whole file names.
if HELLO_PATH == '':Shouldn't we set HELLO_PATH = 'lib/efi_loader/helloworld.efi' in defs.py and use another variable name here?
An explicit path is a remnant int the past when helloworld was not compiled properly. So I will delete all the stuff including the code below.
bin_path = u_boot_config.build_dir + '/' + HELLO_PATH
HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'Capitalization should only be used for constants.
Best regards
Heinrich
@@ -87,21 +87,21 @@ def efi_boot_env(request, u_boot_config): # db1-update check_call('cd %s; %ssign-efi-sig-list -t "2020-04-06" -a -c KEK.crt -k KEK.key db db1.esl db1-update.auth' % (mnt_point, EFITOOLS_PATH), shell=True)
## dbx (TEST_dbx certificate)
# 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 -t "2020-04-05" -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)
# 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 -t "2020-04-05" -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)
# 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 -t "2020-04-05" -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)
# dbx_db (with TEST_db certificate) check_call('cd %s; %ssign-efi-sig-list -t "2020-04-05" -c KEK.crt -k KEK.key dbx db.esl dbx_db.auth' % (mnt_point, EFITOOLS_PATH), shell=True)@@ -112,10 +112,10 @@ def efi_boot_env(request, u_boot_config): # 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
# 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
# Digest image check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -t "2020-04-07" -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth' % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True)@@ -136,3 +136,123 @@ 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 +#
+@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'
- if HELLO_PATH == '':
See comment above.
HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'Capitalization should only be used for constants.
- try:
mnt_point = u_boot_config.build_dir + '/mnt_efisecure'check_call('rm -rf {}'.format(mnt_point), shell=True)check_call('mkdir -p {}'.format(mnt_point), shell=True)# Create signature database# PKcheck_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)# KEKcheck_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)# We will have three-tier hierarchy of certificates:# TestRoot: Root CA (self-signed)# TestSub: Intermediate CA (signed by Root CA)# TestCert: User certificate (signed by Intermediate CA, and used# for signing an image)## NOTE:# I consulted the following EDK2 document for certificate options:# BaseTools/Source/Python/Pkcs7Sign/Readme.md# Please not use them as they are in product system. They are# for test purpose only.# TestRootcheck_call('cp %s/test/py/tests/test_efi_secboot/openssl.cnf %s'% (u_boot_config.source_dir, mnt_point), shell=True)check_call('cd %s; openssl genrsa -out TestRoot.key 2048; openssl req --config openssl.cnf -extensions v3_ca -new -x509 -days 365 -key TestRoot.key -out TestRoot.crt -subj "/CN=TEST_root/"; touch index.txt'% mnt_point, shell=True)# TestSubcheck_call('cd %s; openssl genrsa -out TestSub.key 2048; openssl req -new -key TestSub.key -out TestSub.csr -subj "/CN=TEST_sub/"; openssl ca --config openssl.cnf -in TestSub.csr -out TestSub.crt -extensions v3_int_ca -days 365 -batch -rand_serial -cert TestRoot.crt -keyfile TestRoot.key'% mnt_point, shell=True)# TestCertcheck_call('cd %s; openssl genrsa -out TestCert.key 2048; openssl req -new -key TestCert.key -out TestCert.csr -subj "/CN=TEST_cert/"; openssl ca --config openssl.cnf -in TestCert.csr -out TestCert.crt -extensions usr_cert -days 365 -batch -rand_serial -cert TestSub.crt -keyfile TestSub.key'% mnt_point, shell=True)# db# for TestCertcheck_call('cd %s; %scert-to-efi-sig-list -g %s TestCert.crt TestCert.esl; %ssign-efi-sig-list -c KEK.crt -k KEK.key db TestCert.esl db_a.auth'% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),shell=True)# for TestSubcheck_call('cd %s; %scert-to-efi-sig-list -g %s TestSub.crt TestSub.esl; %ssign-efi-sig-list -t "2020-07-16" -c KEK.crt -k KEK.key db TestSub.esl db_b.auth'% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),shell=True)# for TestRootcheck_call('cd %s; %scert-to-efi-sig-list -g %s TestRoot.crt TestRoot.esl; %ssign-efi-sig-list -t "2020-07-17" -c KEK.crt -k KEK.key db TestRoot.esl db_c.auth'% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),shell=True)## dbx (hash of certificate with revocation time)# for TestCertcheck_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestCert.crt TestCert.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx TestCert.crl dbx_a.auth'% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),shell=True)Not to be corrected in this patch:
-t 0 means revocation for all times.
The values won't affect the test cases here, but I will add more *meaningful* values.
Shouldn't we have a test that checks that revocation time stamps are correctly used? As we use explicit timestamps now for sign-efi-sig-list this should be feasible.
Good point, but it's orthogonal to "intermediate certificates" patch.
That is why I wrote "Not to be corrected in this patch".
Best regards
Heinrich
-Takahiro Akashi
# for TestSubcheck_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestSub.crt TestSub.crl; %ssign-efi-sig-list -t "2020-07-18" -c KEK.crt -k KEK.key dbx TestSub.crl dbx_b.auth'% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),shell=True)# for TestRootcheck_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 TestRoot.crt TestRoot.crl; %ssign-efi-sig-list -t "2020-07-19" -c KEK.crt -k KEK.key dbx TestRoot.crl dbx_c.auth'% (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),shell=True)# Sign image# additional intermediate certificates may be included# in SignedDatacheck_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)# signed by TestCertcheck_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --out helloworld.efi.signed_a helloworld.efi'% (mnt_point, SBSIGN_PATH), shell=True)# signed by TestCert with TestSub in signaturecheck_call('cd %s; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSub.crt --out helloworld.efi.signed_ab helloworld.efi'% (mnt_point, SBSIGN_PATH), shell=True)# signed by TestCert with TestSub and TestRoot in signaturecheck_call('cd %s; cat TestSub.crt TestRoot.crt > TestSubRoot.crt; %ssbsign --key TestCert.key --cert TestCert.crt --addcert TestSubRoot.crt --out helloworld.efi.signed_abc helloworld.efi'% (mnt_point, SBSIGN_PATH), shell=True)check_call('virt-make-fs --partition=gpt --size=+1M --type=vfat {} {}'.format(mnt_point, image_path), shell=True)check_call('rm -rf {}'.format(mnt_point), shell=True)- except CalledProcessError as e:
pytest.skip('Setup failed: %s' % e.cmd)return- else:
yield image_path- finally:
call('rm -f %s' % image_path, shell=True)diff --git a/test/py/tests/test_efi_secboot/defs.py b/test/py/tests/test_efi_secboot/defs.py index ba6b9f391e60..a26bbc103cd9 100644 --- a/test/py/tests/test_efi_secboot/defs.py +++ b/test/py/tests/test_efi_secboot/defs.py @@ -1,14 +1,20 @@ # SPDX-License-Identifier: GPL-2.0+
# Disk image name -EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot.img' +EFI_SECBOOT_IMAGE_NAME = 'test_efi_secboot'
# Owner guid 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. +# The path must terminate with '/'. EFITOOLS_PATH = ''
+# "--addcert" option of sbsign must be available, otherwise +# you need build a newer version on your own. +# The path must terminate with '/'. +SBSIGN_PATH = ''
# Hello World application for sandbox HELLO_PATH = '' diff --git a/test/py/tests/test_efi_secboot/openssl.cnf b/test/py/tests/test_efi_secboot/openssl.cnf new file mode 100644 index 000000000000..f684f1df7e69 --- /dev/null +++ b/test/py/tests/test_efi_secboot/openssl.cnf @@ -0,0 +1,48 @@ +[ ca ] +default_ca = CA_default
+[ CA_default ] +new_certs_dir = . +database = ./index.txt +serial = ./serial +default_md = sha256 +policy = policy_min
+[ req ] +distinguished_name = def_distinguished_name
+[def_distinguished_name]
+# Extensions +# -addext " ... = ..." +# +[ v3_ca ]
- # Extensions for a typical Root CA.
- basicConstraints = critical,CA:TRUE
- keyUsage = critical, digitalSignature, cRLSign, keyCertSign
- subjectKeyIdentifier = hash
- authorityKeyIdentifier = keyid:always,issuer
+[ v3_int_ca ]
- # Extensions for a typical intermediate CA.
- basicConstraints = critical, CA:TRUE
- keyUsage = critical, digitalSignature, cRLSign, keyCertSign
- subjectKeyIdentifier = hash
- authorityKeyIdentifier = keyid:always,issuer
+[ usr_cert ]
- # Extensions for user end certificates.
- basicConstraints = CA:FALSE
- keyUsage = critical, nonRepudiation, digitalSignature, keyEncipherment
- extendedKeyUsage = clientAuth, emailProtection
- subjectKeyIdentifier = hash
- authorityKeyIdentifier = keyid,issuer
+[ policy_min ]
- countryName = optional
- stateOrProvinceName = optional
- localityName = optional
- organizationName = optional
- organizationalUnitName = optional
- commonName = supplied
- emailAddress = optional
diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py new file mode 100644 index 000000000000..1240174758cf --- /dev/null +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py @@ -0,0 +1,135 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2020, Linaro Limited +# Author: AKASHI Takahiro takahiro.akashi@linaro.org +# +# U-Boot UEFI: Image Authentication Test (signature with certificates chain)
+""" +This test verifies image authentication for a signed image which is signed +by user certificate and contains additional intermediate certificates in its +signature. +"""
+import pytest
+@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('efi_secure_boot') +@pytest.mark.buildconfigspec('cmd_efidebug') +@pytest.mark.buildconfigspec('cmd_fat') +@pytest.mark.buildconfigspec('cmd_nvedit_efi') +@pytest.mark.slow +class TestEfiSignedImageExt(object):
- def test_efi_signed_image_ext1(self, u_boot_console, efi_boot_env_intca):
"""Test Case 1 - authenticated by root CA in db"""u_boot_console.restart_uboot()disk_img = efi_boot_env_intcawith u_boot_console.log.section('Test Case 1a'):# Test Case 1a, with no Int CA and not authenticated by root CAoutput = u_boot_console.run_command_list(['host bind 0 %s' % disk_img,'fatload host 0:1 4000000 db_c.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 'Failed to set EFI variable' not in ''.join(output)output = u_boot_console.run_command_list(['efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""','efidebug boot next 1','efidebug test bootmgr'])assert '\'HELLO_a\' failed' in ''.join(output)assert 'efi_start_image() returned: 26' in ''.join(output)with u_boot_console.log.section('Test Case 1b'):# Test Case 1b, signed and authenticated by root CAoutput = u_boot_console.run_command_list(['efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""','efidebug boot next 2','bootefi bootmgr'])assert 'Hello, world!' in ''.join(output)- def test_efi_signed_image_ext2(self, u_boot_console, efi_boot_env_intca):
"""Test Case 2 - authenticated by root CA in db"""u_boot_console.restart_uboot()disk_img = efi_boot_env_intcawith u_boot_console.log.section('Test Case 2a'):# Test Case 2a, unsigned and not authenticated by root CAoutput = 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'])assert 'Failed to set EFI variable' not in ''.join(output)output = u_boot_console.run_command_list(['efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""','efidebug boot next 1','efidebug test bootmgr'])assert '\'HELLO_abc\' failed' in ''.join(output)assert 'efi_start_image() returned: 26' in ''.join(output)with u_boot_console.log.section('Test Case 2b'):# Test Case 2b, signed and authenticated by root CAoutput = u_boot_console.run_command_list(['fatload host 0:1 4000000 db_b.auth','setenv -e -nv -bs -rt -at -i 4000000,$filesize db','efidebug boot next 1','efidebug test bootmgr'])assert '\'HELLO_abc\' failed' in ''.join(output)assert 'efi_start_image() returned: 26' in ''.join(output)with u_boot_console.log.section('Test Case 2c'):# Test Case 2c, signed and authenticated by root CAoutput = u_boot_console.run_command_list(['fatload host 0:1 4000000 db_c.auth','setenv -e -nv -bs -rt -at -i 4000000,$filesize db','efidebug boot next 1','efidebug test bootmgr'])assert 'Hello, world!' in ''.join(output)- def test_efi_signed_image_ext3(self, u_boot_console, efi_boot_env_intca):
"""Test Case 3 - revoked by dbx"""u_boot_console.restart_uboot()disk_img = efi_boot_env_intcawith u_boot_console.log.section('Test Case 3a'):# Test Case 3a, revoked by int CA in dbxoutput = u_boot_console.run_command_list(['host bind 0 %s' % disk_img,'fatload host 0:1 4000000 dbx_b.auth','setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx','fatload host 0:1 4000000 db_c.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 'Failed to set EFI variable' not in ''.join(output)output = u_boot_console.run_command_list(['efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""','efidebug boot next 1','efidebug test bootmgr'])assert 'Hello, world!' in ''.join(output)# Or,# assert '\'HELLO_abc\' failed' in ''.join(output)# assert 'efi_start_image() returned: 26' in ''.join(output)with u_boot_console.log.section('Test Case 3b'):# Test Case 3b, revoked by root CA in dbxoutput = u_boot_console.run_command_list(['fatload host 0:1 4000000 dbx_c.auth','setenv -e -nv -bs -rt -at -i 4000000,$filesize dbx','efidebug boot next 1','efidebug test bootmgr'])assert '\'HELLO_abc\' failed' in ''.join(output)assert 'efi_start_image() returned: 26' in ''.join(output)