Takahiro, On Fri, 17 Apr 2020 at 12:51, AKASHI Takahiro <takahiro.aka...@linaro.org> wrote:
> Heinrich, > > I was notified of some wrong implementation in this patch set. > I think that the change will be contained in efi_signature_verify() > function, but it is so essential in terms of "verification" logic. > > While I'm still investigating the issue to determine what impact > it will have, I would have to, in the worst case, ask you to revert > all the commits. > > Some details: > In my current implementation, a signature in UEFI image will be > verified with one of certificates in UEFI signature database > (PK/KEK/db). > The true logic would be that a signature be verified with a corresponding > certificate also in the image (known as signers), and then that certificate > be verified with UEFI signature database. > > It is particularly crucial if we want to support "intermediate" > certificates (or chain-of-trust) in UEFI image. > > I will keep you updated when I have some progress. > I do not think this is a wrong implementation. It's just that we currently do not have support for multi-certificate chain building and verification. But it would still be possible to do signature verification of an image/variable/capsule against the public key which is contained in the signature database variables(Pk, KEK, db). I think the unimplemented part can well be added to the current implementation as a follow-up patch series, so i don't see a reason why this patch series should not be applied, unless the current series has any other issues. -sughosh > Thanks, > -Takahiro Akashi > > On Tue, Apr 14, 2020 at 11:51:37AM +0900, AKASHI Takahiro wrote: > > One of major missing features in current UEFI implementation is "secure > boot." > > The ultimate goal of my attempt is to implement image authentication > based > > on signature and provide UEFI secure boot support which would be fully > > compliant with UEFI specification, section 32[1]. > > (The code was originally developed by Patrick Wildt.) > > > > This patch set requires one prerequisite[2]. For complete workable cod, > > see my repository[3]. > > > > My "non-volatile" support[4], which is under discussion, is not mandatory > > and so not included here, but this inevitably implies that, for example, > > signature database variables, like db and dbx, won't be persistent unless > > you explicitly run "env save" command. > > Anyhow, Linaro is also working on implementing real "secure storage" > > solution based on TF-A and OP-TEE. > > > > > > Supported features: > > * image authentication based on db and dbx > > * supported signature types are > > EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images) > > EFI_CERT_X509_GUID (x509 certificate for signed images) > > * SecureBoot/SignatureSupport variables > > * SetupMode and user mode > > * variable authentication based on PK and KEK > > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS > > * basic pytest test cases > > > > Unsupported features: (marked as TODO in most cases in the source code, > > and won't be included in this series) > > * hash algorithms other than SHA256 > > * dbt: timestamp(RFC6131)-based certificate revocation > > * dbr: OS recovery > > * xxxDefault: default values for signature stores > > * transition to AuditMode and DeployedMode > > * recording rejected images in EFI_IMAGE_EXECUTION_INFO_TABLE > > * verification "policy", in particular, check against signature's owner > > * private authenticated variables > > * variable authentication with EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS > > * real secure storage support, including hardware-specific PK (Platform > Key) > > installation > > > > TODO's other than "Unsupported features": (won't be included in this > series) > > * fail recovery, in particular, in modifying authenticated variables > > * support read-only attributes of well-defined global variables > > in particular, "SignatureSupport" > > * Extensive test suite (or more test cases) to confirm compatibility > > with EDK2 > > => I requested EDK SCT community to add tests[5]. > > > > Test: > > * My pytest, included in this patch set, passed. > > * efi_selftest passed. (At least no regression.) > > * Travis CI tests have passed. > > > > Known issues: > > * efitools is used in pytest, and its version must be v1.5.2 or later. > > (Solution: You can define EFITOOLS_PATH in defs.py for your own > efitools.) > > > > > > Hints about how to use: > > (Please see other documents, or my pytest scripts, for details.) > > * You can create your own certificates with openssl. > > * You can sign your application with sbsign (on Ubuntu). > > * You can create raw data for signature database with efitools, and > > install/manage authenticated variables with "env -set -e" command > > or efitools' "UpdateVars.efi" application. > > > > > > [1] > https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_final.pdf > > [2] https://lists.denx.de/pipermail/u-boot/2020-January/398057.html > > (extend rsa_verify() for UEFI secure boot) > > [3] http://git.linaro.org/people/takahiro.akashi/u-boot.git/ efi/secboot > > [4] https://lists.denx.de/pipermail/u-boot/2019-September/382835.html > > (non-volatile variables support) > > [5] https://bugzilla.tianocore.org/show_bug.cgi?id=2230 > > [6] https://lists.denx.de/pipermail/u-boot/2020-February/399446.html > > > > > > Changes in v7 (Apr 14, 2020) > > * fix a usage of qsort() in efi_image_parse() (patch#7) > > * merge my document patch[6] (patch#17) > > > > Changes in v6 (Feb 26, 2020) > > * fix 'make htmldocs' warnings (patch#4,#7) > > * refactor efi_load_pe() to avoid test_efi_fit.py error (patch#7) > > * fix pytest warnings (patch#14) > > > > Changes in v5 (Jan 28, 2020) > > * rebased to pre-v2020.04-rc1 (fixed some merge conflicts) > > * remove already-merged commits (v4's patch#1) > > * fix a compile error caused by gcc 9.x (patch#4) > > * return SECURITY_VIOLATION instead of ACCESS_DENIED if authentication > fails > > (patch#7) > > * use qsort() for section sorting (patch#7) > > * add "efidebug test" sub-command (patch#11) > > * add efi_start_image(SECURITY_VIOLATION) test (patch#14) > > > > Changes in v4 (Dec 18, 2019) > > * adjust EFI_SECURE_BOOT dependencies due to a change of RSA extension > patch v5 > > (patch#2) > > * change "imply" to "select" against kconfig dependencies (patch#2) > > * otherwise, no functional changes > > > > Changes in v3 (Dec 9, 2019) > > * allow for arbitrary number of regions in efi_image_region_add() > > (patch#3, #5 and #8) > > * remove a redundant check in a while loop at efi_sigstore_free() > (patch#4) > > > > Changes in v2 (Nov 26, 2019) > > * rebased to v2020.01-rc3 > > * rename IMAGE_DIRECTORY_ENTRY_CERTTABLE to > IMAGE_DIRECTORY_ENTRY_SECURITY > > (patch#1,#9) > > * add comments (patch#1) > > * drop v1's patch#2 as it is no longer necessary > > * drop v1's patch#3 as other "SECURE_BOOT" architectures have renamed > > this option and no longer use it > > * add structure descriptions (patch#3) > > * rework hash calculation code in efi_signature_verify() and remove > > an odd constant, WinIndrectSha256 (patch#3) > > * move travis.yml changes to a separate patch (patch#12, #16) > > * yield_fixture() -> fixture() (patch#12) > > * call console.restart_uboot() at every test case (13,#14) > > * add patch#15; enable UEFI-related configurations by default on sandbox > > * add patch#16; modify Travis CI environment to run UEFI secure boot test > > > > Changes in v1 (Nov 13, 2019) > > * rebased to v2020.01-rc > > * remove already-merged patches > > * re-work the patch set for easier reviews, including > > - move a config definition patch forward (patch#4) > > - refactor/rename verification functions (patch#5/#10) > > - split signature database parser as a separate patch (patch#6) > > - split secure state transition code as a separate patch (patch#8) > > - move most part of init_secure_boot() into init_variables() (patch#8) > > - split test environment setup from test patches (patch#14) > > * add function descriptions (patch#5-#11) > > * make sure the section list is sorted in ascending order in hash > > calculation of PE image (patch#10) > > * add a new "-at" (authenticated access) option to "env -e" (patch#13) > > * list required host packages, in particular udisks2, in pytest > > (patch#14) > > * modify conftest.py to run under python3 (patch#14) > > * use a partition on a disk instead of a whole disk without partition > > table (patch#14) > > * reduce dependency on efitools, yet relying on its host tools (patch#14) > > * modify pytests to catch up wth latest changes of "env -e" syntax > > (patch#15,#16) > > > > RFC (Sept 18, 2019) > > > > AKASHI Takahiro (17): > > efi_loader: add CONFIG_EFI_SECURE_BOOT config option > > efi_loader: add signature verification functions > > efi_loader: add signature database parser > > efi_loader: variable: support variable authentication > > efi_loader: variable: add secure boot state transition > > efi_loader: variable: add VendorKeys variable > > efi_loader: image_loader: support image authentication > > efi_loader: set up secure boot > > cmd: env: use appropriate guid for authenticated UEFI variable > > cmd: env: add "-at" option to "env set -e" command > > cmd: efidebug: add "test bootmgr" sub-command > > efi_loader, pytest: set up secure boot environment > > efi_loader, pytest: add UEFI secure boot tests (authenticated > > variables) > > efi_loader, pytest: add UEFI secure boot tests (image) > > sandbox: add extra configurations for UEFI and related tests > > travis: add packages for UEFI secure boot test > > efi_loader: add some description about UEFI secure boot > > > > .travis.yml | 11 +- > > cmd/efidebug.c | 78 +- > > cmd/nvedit.c | 5 +- > > cmd/nvedit_efi.c | 23 +- > > configs/sandbox64_defconfig | 3 + > > configs/sandbox_defconfig | 3 + > > doc/uefi/uefi.rst | 77 ++ > > include/efi_api.h | 87 ++ > > include/efi_loader.h | 91 +- > > lib/efi_loader/Kconfig | 18 + > > lib/efi_loader/Makefile | 1 + > > lib/efi_loader/efi_boottime.c | 10 +- > > lib/efi_loader/efi_image_loader.c | 462 ++++++++- > > lib/efi_loader/efi_setup.c | 38 + > > lib/efi_loader/efi_signature.c | 809 +++++++++++++++ > > lib/efi_loader/efi_variable.c | 952 ++++++++++++++++-- > > test/py/README.md | 8 + > > test/py/tests/test_efi_secboot/conftest.py | 151 +++ > > test/py/tests/test_efi_secboot/defs.py | 21 + > > .../py/tests/test_efi_secboot/test_authvar.py | 282 ++++++ > > test/py/tests/test_efi_secboot/test_signed.py | 117 +++ > > .../tests/test_efi_secboot/test_unsigned.py | 121 +++ > > 22 files changed, 3237 insertions(+), 131 deletions(-) > > create mode 100644 lib/efi_loader/efi_signature.c > > create mode 100644 test/py/tests/test_efi_secboot/conftest.py > > create mode 100644 test/py/tests/test_efi_secboot/defs.py > > create mode 100644 test/py/tests/test_efi_secboot/test_authvar.py > > create mode 100644 test/py/tests/test_efi_secboot/test_signed.py > > create mode 100644 test/py/tests/test_efi_secboot/test_unsigned.py > > > > -- > > 2.25.2 > > >