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] [day] [month] [year] [list]
Message-Id: <8UJXDS.UATQJMAP9FTR@mecka.net>
Date: Thu, 23 May 2024 10:34:56 +0200
From: Manuel Traut <manut@...ka.net>
To: Jens Wiklander <jens.wiklander@...aro.org>
Cc: linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org,
	op-tee@...ts.trustedfirmware.org, Shyam Saini
	<shyamsaini@...ux.microsoft.com>, Ulf Hansson <ulf.hansson@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>, Jerome Forissier
	<jerome.forissier@...aro.org>, Sumit Garg <sumit.garg@...aro.org>,
	Ilias Apalodimas <ilias.apalodimas@...aro.org>, Bart Van Assche
	<bvanassche@....org>, Randy Dunlap <rdunlap@...radead.org>, Ard Biesheuvel
	<ardb@...nel.org>, Arnd Bergmann <arnd@...db.de>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>
Subject: Re: [PATCH v6 0/3] Replay Protected Memory Block (RPMB) subsystem

Hi Jens,

On Wed, May 22 2024 at 17:33:07 +02:00:00, Jens Wiklander 
<jens.wiklander@...aro.org> wrote:
> Hi Manuel,
> 
> On Tue, May 14, 2024 at 5:28 PM Manuel Traut <manut@...ka.net> 
> wrote:
>> 
>>  Hi Jens,
>> 
>>  thank you very much for v6! It took me some time to figure out why 
>> it is
>>  not working.. However it seems to be on the OP-TEE side and not 
>> related
>>  to this kernel series.
>> 
>>  I need this change:
>> 
>>  @@ -1214,12 +1225,13 @@ static TEE_Result tee_rpmb_init(void)
>>          }
>> 
>>          if (rpmb_ctx->reinit) {
>>                  if (rpmb_ctx->legacy_operation || 
>> !rpmb_ctx->key_verified) {
>>                          rpmb_ctx->wr_cnt_synced = false;
>>                          rpmb_ctx->key_derived = false;
>>                          rpmb_ctx->dev_info_synced = false;
>>                          rpmb_ctx->reinit = false;
>>  -                       goto next;
>>                  }
>>                  res = rpmb_probe_reset();
>>                  if (res) {
>> 
>>  @@ -1236,17 +1248,23 @@ static TEE_Result tee_rpmb_init(void)
>>                          if (!memcmp(rpmb_ctx->cid, dev_info.cid,
>>                                     RPMB_EMMC_CID_SIZE)) {
>>                                  rpmb_ctx->reinit = false;
>>  +                               rpmb_ctx->legacy_operation = false;
>>                                  return TEE_SUCCESS;
>>                          }
>>                  }
>>          }
>> 
>>  to ensure that the non legacy mode is selected, even if the first 
>> RPMB
>>  request comes from a compiled in TA.
> 
> That patch didn't quite work in my case, but I think I understand
> what's needed at your end.
> 
> I've prepared patches at https://github.com/OP-TEE/optee_os/pull/6852
> can you try those, please

this also works on my side.

Thanks a lot
Manuel

>> 
>>  Thanks for your work, it makes it really easy now to implement ARM
>>  System Ready IR with an fTPM and continue the boot measurements in 
>> the
>>  initrd with the new tpm2.target in systemd v256.
> 
> Thanks, much appreciated.
> 
> Cheers,
> Jens
> 
>> 
>>  Regards
>>  Manuel
>> 
>>  On Tue, May 07, 2024 at 11:16:16AM +0200, Jens Wiklander wrote:
>>  > Hi,
>>  >
>>  > This patch set introduces a new RPMB subsystem, based on patches 
>> from [1],
>>  > [2], and [3]. The RPMB subsystem aims at providing access to RPMB
>>  > partitions to other kernel drivers, in particular the OP-TEE 
>> driver. A new
>>  > user space ABI isn't needed, we can instead continue using the 
>> already
>>  > present ABI when writing the RPMB key during production.
>>  >
>>  > I've added and removed things to keep only what is needed by the 
>> OP-TEE
>>  > driver. Since the posting of [3], there has been major changes in 
>> the MMC
>>  > subsystem so "mmc: block: register RPMB partition with the RPMB 
>> subsystem"
>>  > is in practice completely rewritten.
>>  >
>>  > With this OP-TEE can access RPMB during early boot instead of 
>> having to
>>  > wait for user space to become available as in the current design 
>> [4].
>>  > This will benefit the efi variables [5] since we wont rely on 
>> userspace as
>>  > well as some TPM issues [6] that were solved.
>>  >
>>  > The OP-TEE driver finds the correct RPMB device to interact with 
>> by
>>  > iterating over available devices until one is found with a 
>> programmed
>>  > authentication matching the one OP-TEE is using. This enables 
>> coexisting
>>  > users of other RPMBs since the owner can be determined by who 
>> knows the
>>  > authentication key.
>>  >
>>  > The corresponding secure world OP-TEE patches are available at 
>> [7].
>>  >
>>  > I've put myself as a maintainer for the RPMB subsystem as I have 
>> an
>>  > interest in the OP-TEE driver to keep this in good shape. 
>> However, if you'd
>>  > rather see someone else taking the maintainership that's fine 
>> too. I'll
>>  > help keep the subsystem updated regardless.
>>  >
>>  > [1] 
>> https://lore.kernel.org/lkml/20230722014037.42647-1-shyamsaini@linux.microsoft.com/
>>  > [2] 
>> https://lore.kernel.org/lkml/20220405093759.1126835-2-alex.bennee@linaroorg/
>>  > [3] 
>> https://lore.kernel.org/linux-mmc/1478548394-8184-2-git-send-email-tomaswinkler@intel.com/
>>  > [4] 
>> https://optee.readthedocs.io/en/latest/architecture/secure_storage.html#rpmb-secure-storage
>>  > [5] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c44b6be62e8dd4ee0a308c36a70620613e6fc55f
>>  > [6] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7269cba53d906cf257c139d3b3a53ad272176bca
>>  > [7] https://github.com/jenswi-linaro/optee_os/tree/rpmb_probe_v6
>>  >
>>  > Thanks,
>>  > Jens
>>  >
>>  > Changes since v5:
>>  > Manuel Traut reported and investigated an error on an i.MX8MM, 
>> the root
>>  > cause was identified as insufficient alignment on frames sent to 
>> the RPMB
>>  > device. Fixed in the OP-TEE driver as described below.
>>  > * "rpmb: add Replay Protected Memory Block (RPMB) subsystem"
>>  >   - Adding a missing EXPORT_SYMBOL_GPL()
>>  > * "optee: probe RPMB device using RPMB subsystem"
>>  >   - Replacing the old OPTEE_RPC_CMD_RPMB ABI with 
>> OPTEE_RPC_CMD_RPMB_FRAMES
>>  >     to get rid of the small header struct rpmb_req (now removed) 
>> causing
>>  >     the problem.
>>  >   - Matching changes on the secure side + support for 
>> re-initializing
>>  >     RPMB in case a boot stage has used RPMB, the latter also 
>> reported by
>>  >     Manuel Traut.
>>  >
>>  > Changes since v4:
>>  > * "rpmb: add Replay Protected Memory Block (RPMB) subsystem"
>>  >   - Describing struct rpmb_descr as RPMB description instead of 
>> descriptor
>>  > * "mmc: block: register RPMB partition with the RPMB subsystem"
>>  >   - Addressing review comments
>>  >   - Adding more comments for struct rpmb_frame
>>  >   - Fixing assignment of reliable_wr_count and capacity in 
>> mmc_blk_rpmb_add()
>>  > * "optee: probe RPMB device using RPMB subsystem"
>>  >   - Updating struct rpmb_dev_info to match changes in "rpmb: add 
>> Replay
>>  >     Protected Memory Block (RPMB) subsystem"
>>  >
>>  > Changes since v3:
>>  > * Move struct rpmb_frame into the MMC driver since the format of 
>> the RPMB
>>  >   frames depend on the implementation, one format for eMMC, 
>> another for
>>  >   UFS, and so on
>>  > * "rpmb: add Replay Protected Memory Block (RPMB) subsystem"
>>  >   - Adding Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
>>  >   - Adding more description of the API functions
>>  >   - Removing the set_dev_info() op from struct rpmb_ops, the 
>> needed information
>>  >     is supplied in the arguments to rpmb_dev_register() instead.
>>  >   - Getting rid of struct rpmb_ops since only the route_frames() 
>> op was
>>  >     remaining, store that op directly in struct rpmb_dev
>>  >   - Changed rpmb_interface_register() and 
>> rpmb_interface_unregister() to use
>>  >     notifier_block instead of implementing the same thing 
>> ourselves
>>  > * "mmc: block: register RPMB partition with the RPMB subsystem"
>>  >   - Moving the call to rpmb_dev_register() to be done at the end 
>> of
>>  >     mmc_blk_probe() when the device is fully available
>>  > * "optee: probe RPMB device using RPMB subsystem"
>>  >   - Use IS_REACHABLE(CONFIG_RPMB) to determine if the RPMB 
>> subsystem is
>>  >     available
>>  >   - Translate TEE_ERROR_STORAGE_NOT_AVAILABLE if encountered in 
>> get_devices()
>>  >     to recognize the error in optee_rpmb_scan()
>>  >   - Simplified optee_rpmb_scan() and optee_rpmb_intf_rdev()
>>  >
>>  > Changes since v2:
>>  > * "rpmb: add Replay Protected Memory Block (RPMB) subsystem"
>>  >   - Fixing documentation issues
>>  >   - Adding a "depends on MMC" in the Kconfig
>>  >   - Removed the class-device and the embedded device, struct 
>> rpmb_dev now
>>  >     relies on the parent device for reference counting as 
>> requested
>>  >   - Removed the now unneeded rpmb_ops get_resources() and 
>> put_resources()
>>  >     since references are already taken in 
>> mmc_blk_alloc_rpmb_part() before
>>  >     rpmb_dev_register() is called
>>  >   - Added rpmb_interface_{,un}register() now that
>>  >     class_interface_{,un}register() can't be used ay longer
>>  > * "mmc: block: register RPMB partition with the RPMB subsystem"
>>  >   - Adding the missing error cleanup in alloc_idata()
>>  >   - Taking the needed reference to md->disk in 
>> mmc_blk_alloc_rpmb_part()
>>  >     instead of in mmc_rpmb_chrdev_open() and 
>> rpmb_op_mmc_get_resources()
>>  > * "optee: probe RPMB device using RPMB subsystem"
>>  >   - Registering to get a notification when an RPMB device comes 
>> online
>>  >   - Probes for RPMB devices each time an RPMB device comes 
>> online, until
>>  >     a usable device is found
>>  >   - When a usable RPMB device is found, call
>>  >     optee_enumerate_devices(PTA_CMD_GET_DEVICES_RPMB)
>>  >   - Pass type of rpmb in return value from 
>> OPTEE_RPC_CMD_RPMB_PROBE_NEXT
>>  >
>>  > Changes since Shyam's RFC:
>>  > * Removed the remaining leftover rpmb_cdev_*() function calls
>>  > * Refactored the struct rpmb_ops with all the previous ops 
>> replaced, in
>>  >   some sense closer to [3] with the route_frames() op
>>  > * Added rpmb_route_frames()
>>  > * Added struct rpmb_frame, enum rpmb_op_result, and enum 
>> rpmb_type from [3]
>>  > * Removed all functions not needed in the OP-TEE use case
>>  > * Added "mmc: block: register RPMB partition with the RPMB 
>> subsystem", based
>>  >   on the commit with the same name in [3]
>>  > * Added "optee: probe RPMB device using RPMB subsystem" for 
>> integration
>>  >   with OP-TEE
>>  > * Moved the RPMB driver into drivers/misc/rpmb-core.c
>>  > * Added my name to MODULE_AUTHOR() in rpmb-core.c
>>  > * Added an rpmb_mutex to serialize access to the IDA
>>  > * Removed the target parameter from all rpmb_*() functions since 
>> it's
>>  >   currently unused
>>  >
>>  > Jens Wiklander (3):
>>  >   rpmb: add Replay Protected Memory Block (RPMB) subsystem
>>  >   mmc: block: register RPMB partition with the RPMB subsystem
>>  >   optee: probe RPMB device using RPMB subsystem
>>  >
>>  >  MAINTAINERS                       |   7 +
>>  >  drivers/misc/Kconfig              |  10 ++
>>  >  drivers/misc/Makefile             |   1 +
>>  >  drivers/misc/rpmb-core.c          | 233 
>> +++++++++++++++++++++++++++++
>>  >  drivers/mmc/core/block.c          | 241 
>> +++++++++++++++++++++++++++++-
>>  >  drivers/tee/optee/core.c          |  30 ++++
>>  >  drivers/tee/optee/device.c        |   7 +
>>  >  drivers/tee/optee/ffa_abi.c       |   8 +
>>  >  drivers/tee/optee/optee_private.h |  21 ++-
>>  >  drivers/tee/optee/optee_rpc_cmd.h |  35 +++++
>>  >  drivers/tee/optee/rpc.c           | 166 ++++++++++++++++++++
>>  >  drivers/tee/optee/smc_abi.c       |   7 +
>>  >  include/linux/rpmb.h              | 136 +++++++++++++++++
>>  >  13 files changed, 899 insertions(+), 3 deletions(-)
>>  >  create mode 100644 drivers/misc/rpmb-core.c
>>  >  create mode 100644 include/linux/rpmb.h
>>  >
>>  > --
>>  > 2.34.1
>>  >
> 



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ