| 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
| ||
|
Message-ID: <CAPDyKFoBC+GaGerGDEAjg9q4ayV9mMBKkfFk3nO-zcQzOZ_H6Q@mail.gmail.com>
Date: Mon, 7 Aug 2023 17:00:48 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Shyam Saini <shyamsaini@...ux.microsoft.com>
Cc: linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org,
op-tee@...ts.trustedfirmware.org, linux-scsi@...r.kernel.org,
Alex Bennée <alex.bennee@...aro.org>,
Tomas Winkler <tomas.winkler@...el.com>,
Linus Walleij <linus.walleij@...aro.org>,
Arnd Bergmann <arnd.bergmann@...aro.org>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>,
Sumit Garg <sumit.garg@...aro.org>,
Tyler Hicks <code@...icks.com>,
"Srivatsa S . Bhat" <srivatsa@...il.mit.edu>,
Paul Moore <paul@...l-moore.com>,
Allen Pais <apais@...ux.microsoft.com>
Subject: Re: [RFC, PATCH 1/1] rpmb: add Replay Protected Memory Block (RPMB) driver
On Sat, 22 Jul 2023 at 03:41, Shyam Saini
<shyamsaini@...ux.microsoft.com> wrote:
>
> From: Alex Bennée <alex.bennee@...aro.org>
>
> [This is patch 1 from [1] Alex's submission and this RPMB layer was
> originally proposed by [2]Thomas Winkler ]
>
> A number of storage technologies support a specialised hardware
> partition designed to be resistant to replay attacks. The underlying
> HW protocols differ but the operations are common. The RPMB partition
> cannot be accessed via standard block layer, but by a set of specific
> commands: WRITE, READ, GET_WRITE_COUNTER, and PROGRAM_KEY. Such a
> partition provides authenticated and replay protected access, hence
> suitable as a secure storage.
>
> The initial aim of this patch is to provide a simple RPMB Driver which
> can be accessed by Linux's optee driver to facilitate fast-path for
> RPMB access to optee OS(secure OS) during the boot time. [1] Currently,
> Optee OS relies on user-tee supplicant to access eMMC RPMB partition.
>
> A TEE device driver can claim the RPMB interface, for example, via
> class_interface_register(). The RPMB driver provides a series of
> operations for interacting with the device.
I don't quite follow this. More exactly, how will the TEE driver know
what RPMB device it should use?
>
> * program_key - a one time operation for setting up a new device
> * get_capacity - introspect the device capacity
> * get_write_counter - check the write counter
> * write_blocks - write a series of blocks to the RPMB device
> * read_blocks - read a series of blocks from the RPMB device
>
> The detailed operation of implementing the access is left to the TEE
> device driver itself.
>
> The framing details and HW specific bits (JDEC vs NVME frames) are
> left to the lower level TEE driver to worry about.
>
> Without kernel fast path to RPMB access doesn't work when IMA try to
> extend ftpm's PCR registers.
>
> This fast-path would require additional work in linux optee driver and
> as well as in MMC driver.
>
> [1] https://lore.kernel.org/lkml/20220405093759.1126835-2-alex.bennee@linaro.org/
> [2] https://lore.kernel.org/linux-mmc/1478548394-8184-2-git-send-email-tomas.winkler@intel.com/
> [3] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
>
> Signed-off-by: Alex Bennée <alex.bennee@...aro.org>
> Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> Signed-off-by: Shyam Saini <shyamsaini@...ux.microsoft.com>
>
[...]
> +/**
> + * rpmb_dev_find_device() - return first matching rpmb device
> + * @data: data for the match function
> + * @match: the matching function
> + *
> + * Return: matching rpmb device or NULL on failure
> + */
> +static
> +struct rpmb_dev *rpmb_dev_find_device(const void *data,
> + int (*match)(struct device *dev,
> + const void *data))
> +{
> + struct device *dev;
> +
> + dev = class_find_device(&rpmb_class, NULL, data, match);
> +
> + return dev ? to_rpmb_dev(dev) : NULL;
> +}
> +
> +struct device_with_target {
> + const struct device *dev;
> + u8 target;
> +};
> +
> +static int match_by_parent(struct device *dev, const void *data)
> +{
> + const struct device_with_target *d = data;
> + struct rpmb_dev *rdev = to_rpmb_dev(dev);
> +
> + return (d->dev && dev->parent == d->dev && rdev->target == d->target);
> +}
> +
> +/**
> + * rpmb_dev_find_by_device() - retrieve rpmb device from the parent device
> + * @parent: parent device of the rpmb device
> + * @target: RPMB target/region within the physical device
> + *
> + * Return: NULL if there is no rpmb device associated with the parent device
> + */
> +struct rpmb_dev *rpmb_dev_find_by_device(struct device *parent, u8 target)
> +{
> + struct device_with_target t;
> +
> + if (!parent)
> + return NULL;
> +
> + t.dev = parent;
> + t.target = target;
> +
> + return rpmb_dev_find_device(&t, match_by_parent);
> +}
> +EXPORT_SYMBOL_GPL(rpmb_dev_find_by_device);
Is this what the TEE driver would be calling to find the rpmb device/partition?
> +
> +/**
> + * rpmb_dev_unregister() - unregister RPMB partition from the RPMB subsystem
> + * @rdev: the rpmb device to unregister
> + * Return:
> + * * 0 on success
> + * * -EINVAL on wrong parameters
> + */
> +int rpmb_dev_unregister(struct rpmb_dev *rdev)
> +{
> + if (!rdev)
> + return -EINVAL;
> +
> + mutex_lock(&rdev->lock);
> + rpmb_cdev_del(rdev);
I can't find the function above. I guess it should be included as a
part of the patch too?
> + device_del(&rdev->dev);
> + mutex_unlock(&rdev->lock);
> +
> + rpmb_dev_put(rdev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(rpmb_dev_unregister);
[...]
> +/**
> + * rpmb_dev_register - register RPMB partition with the RPMB subsystem
> + * @dev: storage device of the rpmb device
> + * @target: RPMB target/region within the physical device
> + * @ops: device specific operations
> + *
> + * Return: a pointer to rpmb device
> + */
> +struct rpmb_dev *rpmb_dev_register(struct device *dev, u8 target,
> + const struct rpmb_ops *ops)
> +{
> + struct rpmb_dev *rdev;
> + int id;
> + int ret;
> +
> + if (!dev || !ops)
> + return ERR_PTR(-EINVAL);
> +
> + if (!ops->program_key)
> + return ERR_PTR(-EINVAL);
> +
> + if (!ops->get_capacity)
> + return ERR_PTR(-EINVAL);
> +
> + if (!ops->get_write_counter)
> + return ERR_PTR(-EINVAL);
> +
> + if (!ops->write_blocks)
> + return ERR_PTR(-EINVAL);
> +
> + if (!ops->read_blocks)
> + return ERR_PTR(-EINVAL);
> +
> + rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
> + if (!rdev)
> + return ERR_PTR(-ENOMEM);
> +
> + id = ida_simple_get(&rpmb_ida, 0, 0, GFP_KERNEL);
> + if (id < 0) {
> + ret = id;
> + goto exit;
> + }
> +
> + mutex_init(&rdev->lock);
> + rdev->ops = ops;
> + rdev->id = id;
> + rdev->target = target;
> +
> + dev_set_name(&rdev->dev, "rpmb%d", id);
> + rdev->dev.class = &rpmb_class;
> + rdev->dev.parent = dev;
> +
> + rpmb_cdev_prepare(rdev);
Ditto.
> +
> + ret = device_register(&rdev->dev);
> + if (ret)
> + goto exit;
> +
> + rpmb_cdev_add(rdev);
Ditto.
> +
> + dev_dbg(&rdev->dev, "registered device\n");
> +
> + return rdev;
> +
> +exit:
> + if (id >= 0)
> + ida_simple_remove(&rpmb_ida, id);
> + kfree(rdev);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(rpmb_dev_register);
> +
[...]
Kind regards
Uffe
Powered by blists - more mailing lists