linux-kernel - Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <286a9ec3aa052cc1ed61e7257af492325d8e7279.camel@kernel.org>
Date:   Fri, 11 Mar 2022 18:39:14 +0200
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Nayna Jain <nayna@...ux.ibm.com>, linux-integrity@...r.kernel.org,
        keyrings@...r.kernel.org
Cc:     dhowells@...hat.com, zohar@...ux.ibm.com,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, dimitri.ledkov@...onical.com,
        seth@...shee.me, rnsastry@...ux.ibm.com,
        Masahiro Yamada <masahiroy@...nel.org>
Subject: Re: [PATCH v11 0/4] integrity: support including firmware
 ".platform" keys at build time

On Thu, 2022-03-10 at 16:44 -0500, Nayna Jain wrote:
> Some firmware support secure boot by embedding static keys to verify the
> Linux kernel during boot. However, these firmware do not expose an
> interface for the kernel to load firmware keys onto the ".platform"
> keyring, preventing the kernel from verifying the kexec kernel image
> signature.
> 
> This patchset exports load_certificate_list() and defines a new function
> load_builtin_platform_cert() to load compiled in certificates onto the
> ".platform" keyring.
> 
> Changelog:
> v11:
> * Added a new patch to conditionally build extract-cert if
> PLATFORM_KEYRING is enabled.
> 
> v10:
> * Fixed the externs warning for Patch 3.
> 
> v9:
> * Rebased on Jarkko's repo - 
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git
> 
> v8:
> * Includes Jarkko's feedback on patch description and removed Reported-by
> for Patch 1.
> 
> v7:
> * Incldues Jarkko's feedback on patch description for Patch 1 and 3.
> 
> v6:
> * Includes Jarkko's feedback:
>  * Split Patch 2 into two.
>  * Update Patch description.
> 
> v5:
> * Renamed load_builtin_platform_cert() to load_platform_certificate_list()
> and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as
> suggested by Mimi Zohar.
> 
> v4:
> * Split into two patches as per Mimi Zohar and Dimitri John Ledkov
> recommendation.
> 
> v3:
> * Included Jarkko's feedback
>  ** updated patch description to include approach.
>  ** removed extern for function declaration in the .h file.
> * Included load_certificate_list() within #ifdef CONFIG_KEYS condition.
> 
> v2:
> * Fixed the error reported by kernel test robot
> * Updated patch description based on Jarkko's feedback.
> 
> Nayna Jain (4):
>   certs: export load_certificate_list() to be used outside certs/
>   integrity: make integrity_keyring_from_id() non-static
>   certs: conditionally build extract-cert if platform keyring is enabled
>   integrity: support including firmware ".platform" keys at build time
> 
>  certs/Makefile                                |  9 ++++++--
>  certs/blacklist.c                             |  1 -
>  certs/common.c                                |  2 +-
>  certs/common.h                                |  9 --------
>  certs/system_keyring.c                        |  1 -
>  include/keys/system_keyring.h                 |  6 +++++
>  security/integrity/Kconfig                    | 10 ++++++++
>  security/integrity/Makefile                   | 15 +++++++++++-
>  security/integrity/digsig.c                   |  2 +-
>  security/integrity/integrity.h                |  9 ++++++++
>  .../integrity/platform_certs/platform_cert.S  | 23 +++++++++++++++++++
>  .../platform_certs/platform_keyring.c         | 23 +++++++++++++++++++
>  12 files changed, 94 insertions(+), 16 deletions(-)
>  delete mode 100644 certs/common.h
>  create mode 100644 security/integrity/platform_certs/platform_cert.S
> 
> 
> base-commit: fb5abce6b2bb5cb3d628aaa63fa821da8c4600f9

I tuned tags. Nayna, Mimi is this how it should be:

commit f38ebfc7469c4924087dab2203479ee9bb6911ac (HEAD -> master, origin/next, origin/master, origin/HEAD, next)
Author: Nayna Jain <nayna@...ux.ibm.com>
Date:   Thu Mar 10 16:44:50 2022 -0500

    integrity: support including firmware ".platform" keys at build time
    
    Allow firmware keys to be embedded in the Linux kernel and loaded onto
    the ".platform" keyring on boot.
    
    The firmware keys can be specified in a file as a list of PEM encoded
    certificates using new config INTEGRITY_PLATFORM_KEYS. The certificates
    are embedded in the image by converting the PEM-formatted certificates
    into DER(binary) and generating
    security/integrity/platform_certs/platform_certificate_list file at
    build time. On boot, the embedded certs from the image are loaded onto
    the ".platform" keyring at late_initcall(), ensuring the platform keyring
    exists before loading the keys.
    
    Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>
    Tested-by: Mimi Zohar <zohar@...ux.ibm.com>
    Signed-off-by: Nayna Jain <nayna@...ux.ibm.com>
    Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>

commit 233bb42918e71f143e6f31d99763348768eb1619
Author: Nayna Jain <nayna@...ux.ibm.com>
Date:   Thu Mar 10 16:44:49 2022 -0500

    certs: conditionally build extract-cert if platform keyring is enabled
    
    extract-cert is used outside certs/ by INTEGRITY_PLATFORM_KEYRING.
    Also build extract-cert if INTEGRITY_PLATFORM_KEYRING is enabled.
    
    Signed-off-by: Nayna Jain <nayna@...ux.ibm.com>
    Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>
    Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>

commit 5e1a41f1c2ee838af6ac39d1cdd4ed0f509186a1
Author: Nayna Jain <nayna@...ux.ibm.com>
Date:   Thu Mar 10 16:44:48 2022 -0500

    integrity: make integrity_keyring_from_id() non-static
    
    Make integrity_keyring_from_id() non-static so that it is accessible
    by other files in security/integrity.
    
    Signed-off-by: Nayna Jain <nayna@...ux.ibm.com>
    Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>
    Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>
    Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>

commit 05489d362bde0f25c4e921d15cad4492d22147ae
Author: Nayna Jain <nayna@...ux.ibm.com>
Date:   Thu Mar 10 16:44:47 2022 -0500

    certs: export load_certificate_list() to be used outside certs/
    
    load_certificate_list() parses certificates embedded in the kernel
    image to load them onto the keyring.
    
    Commit "2565ca7f5ec1 (certs: Move load_system_certificate_list to a common
    function)" made load_certificate_list() a common function in the certs/
    directory. Export load_certificate_list() outside certs/ to be used by
    load_platform_certificate_list() for loading compiled in platform keys
    onto the .platform keyring at boot time.
    
    Signed-off-by: Nayna Jain <nayna@...ux.ibm.com>
    Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>
    Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>
    Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>

BR, Jarkko