| 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: <ff78164cc13b4855911116c2d48929a2@intel.com>
Date: Thu, 4 Mar 2021 07:01:14 +0000
From: "Winkler, Tomas" <tomas.winkler@...el.com>
To: Alex Bennée <alex.bennee@...aro.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "maxim.uvarov@...aro.org" <maxim.uvarov@...aro.org>,
"joakim.bech@...aro.org" <joakim.bech@...aro.org>,
"ilias.apalodimas@...aro.org" <ilias.apalodimas@...aro.org>,
"arnd@...aro.org" <arnd@...aro.org>,
"ruchika.gupta@...aro.org" <ruchika.gupta@...aro.org>,
"Huang, Yang" <yang.huang@...el.com>,
"Zhu, Bing" <bing.zhu@...el.com>,
"Matti.Moell@...nsynergy.com" <Matti.Moell@...nsynergy.com>,
"hmo@...nsynergy.com" <hmo@...nsynergy.com>,
"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-nvme@...r.kernel.org" <linux-nvme@...r.kernel.org>,
Ulf Hansson <ulf.hansson@...aro.org>,
Linus Walleij <linus.walleij@...aro.org>,
"Arnd Bergmann" <arnd.bergmann@...aro.org>,
"Usyskin, Alexander" <alexander.usyskin@...el.com>,
Avri Altman <avri.altman@...disk.com>
Subject: RE: [RFC PATCH 2/5] char: rpmb: provide a user space interface
>
> The user space API is achieved via a number of synchronous IOCTLs.
>
> * RPMB_IOC_VER_CMD - simple versioning API
> * RPMB_IOC_CAP_CMD - query of underlying capabilities
> * RPMB_IOC_PKEY_CMD - one time programming of access key
> * RPMB_IOC_COUNTER_CMD - query the write counter
> * RPMB_IOC_WBLOCKS_CMD - write blocks to device
> * RPMB_IOC_RBLOCKS_CMD - read blocks from device
>
> The keys used for programming and writing blocks to the device are
> key_serial_t handles as provided by the keyctl() interface.
>
> [AJB: here there are two key differences between this and the original
> proposal. The first is the dropping of the sequence of preformated frames in
> favour of explicit actions. The second is the introduction of key_serial_t and
> the keyring API for referencing the key to use]
Putting it gently I'm not sure this is good idea, from the security point of view.
The key has to be possession of the one that signs the frames as they are, it doesn't mean it is linux kernel keyring, it can be other party on different system.
With this approach you will make the other usecases not applicable. It is less then trivial to move key securely from one system to another.
We all wished it can be abstracted more but the frames has to come already signed, and the key material is inside the frame.
Thanks
Tomas
>
> Signed-off-by: Alex Bennée <alex.bennee@...aro.org>
> Cc: Ulf Hansson <ulf.hansson@...aro.org>
> Cc: Linus Walleij <linus.walleij@...aro.org>
> Cc: Arnd Bergmann <arnd.bergmann@...aro.org>
> Cc: Ilias Apalodimas <ilias.apalodimas@...aro.org>
> Cc: Tomas Winkler <tomas.winkler@...el.com>
> Cc: Alexander Usyskin <alexander.usyskin@...el.com>
> Cc: Avri Altman <avri.altman@...disk.com>
> ---
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 1 +
> drivers/char/rpmb/Kconfig | 7 +
> drivers/char/rpmb/Makefile | 1 +
> drivers/char/rpmb/cdev.c | 246 ++++++++++++++++++
> drivers/char/rpmb/core.c | 10 +-
> drivers/char/rpmb/rpmb-cdev.h | 17 ++
> include/linux/rpmb.h | 10 +
> include/uapi/linux/rpmb.h | 68 +++++
> 9 files changed, 357 insertions(+), 4 deletions(-) create mode 100644
> drivers/char/rpmb/cdev.c create mode 100644 drivers/char/rpmb/rpmb-
> cdev.h create mode 100644 include/uapi/linux/rpmb.h
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
> b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index a4c75a28c839..0ff2d4d81bb0 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -344,6 +344,7 @@ Code Seq# Include File
> Comments
> 0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-
> remoteproc@...r.kernel.org>
> 0xB6 all linux/fpga-dfl.h
> 0xB7 all uapi/linux/remoteproc_cdev.h <mailto:linux-
> remoteproc@...r.kernel.org>
> +0xB8 80-8F uapi/linux/rpmb.h <mailto:linux-
> mmc@...r.kernel.org>
> 0xC0 00-0F linux/usb/iowarrior.h
> 0xCA 00-0F uapi/misc/cxl.h
> 0xCA 10-2F uapi/misc/ocxl.h
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 076f3983526c..c60b41b6e6bd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15374,6 +15374,7 @@ M: ?
> L: linux-kernel@...r.kernel.org
> S: Supported
> F: drivers/char/rpmb/*
> +F: include/uapi/linux/rpmb.h
> F: include/linux/rpmb.h
>
> RTL2830 MEDIA DRIVER
> diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig index
> 431c2823cf70..9068664a399a 100644
> --- a/drivers/char/rpmb/Kconfig
> +++ b/drivers/char/rpmb/Kconfig
> @@ -9,3 +9,10 @@ config RPMB
> access RPMB partition.
>
> If unsure, select N.
> +
> +config RPMB_INTF_DEV
> + bool "RPMB character device interface /dev/rpmbN"
> + depends on RPMB && KEYS
> + help
> + Say yes here if you want to access RPMB from user space
> + via character device interface /dev/rpmb%d
> diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile index
> 24d4752a9a53..f54b3f30514b 100644
> --- a/drivers/char/rpmb/Makefile
> +++ b/drivers/char/rpmb/Makefile
> @@ -3,5 +3,6 @@
>
> obj-$(CONFIG_RPMB) += rpmb.o
> rpmb-objs += core.o
> +rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o
>
> ccflags-y += -D__CHECK_ENDIAN__
> diff --git a/drivers/char/rpmb/cdev.c b/drivers/char/rpmb/cdev.c new file
> mode 100644 index 000000000000..55f66720fd03
> --- /dev/null
> +++ b/drivers/char/rpmb/cdev.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright(c) 2015 - 2019 Intel Corporation.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/fs.h>
> +#include <linux/uaccess.h>
> +#include <linux/compat.h>
> +#include <linux/slab.h>
> +#include <linux/capability.h>
> +
> +#include <linux/rpmb.h>
> +
> +#include "rpmb-cdev.h"
> +
> +static dev_t rpmb_devt;
> +#define RPMB_MAX_DEVS MINORMASK
> +
> +#define RPMB_DEV_OPEN 0 /** single open bit (position) */
> +
> +/**
> + * rpmb_open - the open function
> + *
> + * @inode: pointer to inode structure
> + * @fp: pointer to file structure
> + *
> + * Return: 0 on success, <0 on error
> + */
> +static int rpmb_open(struct inode *inode, struct file *fp) {
> + struct rpmb_dev *rdev;
> +
> + rdev = container_of(inode->i_cdev, struct rpmb_dev, cdev);
> + if (!rdev)
> + return -ENODEV;
> +
> + /* the rpmb is single open! */
> + if (test_and_set_bit(RPMB_DEV_OPEN, &rdev->status))
> + return -EBUSY;
> +
> + mutex_lock(&rdev->lock);
> +
> + fp->private_data = rdev;
> +
> + mutex_unlock(&rdev->lock);
> +
> + return nonseekable_open(inode, fp);
> +}
> +
> +/**
> + * rpmb_release - the cdev release function
> + *
> + * @inode: pointer to inode structure
> + * @fp: pointer to file structure
> + *
> + * Return: 0 always.
> + */
> +static int rpmb_release(struct inode *inode, struct file *fp) {
> + struct rpmb_dev *rdev = fp->private_data;
> +
> + clear_bit(RPMB_DEV_OPEN, &rdev->status);
> +
> + return 0;
> +}
> +
> +static long rpmb_ioctl_ver_cmd(struct rpmb_dev *rdev,
> + struct rpmb_ioc_ver_cmd __user *ptr) {
> + struct rpmb_ioc_ver_cmd ver = {
> + .api_version = RPMB_API_VERSION,
> + };
> +
> + return copy_to_user(ptr, &ver, sizeof(ver)) ? -EFAULT : 0; }
> +
> +static long rpmb_ioctl_cap_cmd(struct rpmb_dev *rdev,
> + struct rpmb_ioc_cap_cmd __user *ptr) {
> + struct rpmb_ioc_cap_cmd cap;
> +
> + cap.target = rdev->target;
> + cap.block_size = rdev->ops->block_size;
> + cap.wr_cnt_max = rdev->ops->wr_cnt_max;
> + cap.rd_cnt_max = rdev->ops->rd_cnt_max;
> + cap.auth_method = rdev->ops->auth_method;
> + cap.capacity = rpmb_get_capacity(rdev);
> + cap.reserved = 0;
> +
> + return copy_to_user(ptr, &cap, sizeof(cap)) ? -EFAULT : 0; }
> +
> +static long rpmb_ioctl_pkey_cmd(struct rpmb_dev *rdev, key_serial_t
> +__user *k) {
> + key_serial_t keyid;
> +
> + if (get_user(keyid, k))
> + return -EFAULT;
> + else
> + return rpmb_program_key(rdev, keyid); }
> +
> +static long rpmb_ioctl_counter_cmd(struct rpmb_dev *rdev, int __user
> +*ptr) {
> + int count = rpmb_get_write_count(rdev);
> +
> + if (count > 0)
> + return put_user(count, ptr);
> + else
> + return count;
> +}
> +
> +static long rpmb_ioctl_wblocks_cmd(struct rpmb_dev *rdev,
> + struct rpmb_ioc_blocks_cmd __user *ptr) {
> + struct rpmb_ioc_blocks_cmd wblocks;
> + int sz;
> + long ret;
> + u8 *data;
> +
> + if (copy_from_user(&wblocks, ptr, sizeof(struct
> rpmb_ioc_blocks_cmd)))
> + return -EFAULT;
> +
> + /* Don't write more blocks device supports */
> + if (wblocks.count > rdev->ops->wr_cnt_max)
> + return -EINVAL;
> +
> + sz = wblocks.count * 256;
> + data = kmalloc(sz, GFP_KERNEL);
> +
> + if (!data)
> + return -ENOMEM;
> +
> + if (copy_from_user(data, wblocks.data, sz))
> + ret = -EFAULT;
> + else
> + ret = rpmb_write_blocks(rdev, wblocks.key, wblocks.addr,
> +wblocks.count, data);
> +
> + kfree(data);
> + return ret;
> +}
> +
> +static long rpmb_ioctl_rblocks_cmd(struct rpmb_dev *rdev,
> + struct rpmb_ioc_blocks_cmd __user *ptr) {
> + struct rpmb_ioc_blocks_cmd rblocks;
> + int sz;
> + long ret;
> + u8 *data;
> +
> + if (copy_from_user(&rblocks, ptr, sizeof(struct
> rpmb_ioc_blocks_cmd)))
> + return -EFAULT;
> +
> + if (rblocks.count > rdev->ops->rd_cnt_max)
> + return -EINVAL;
> +
> + sz = rblocks.count * 256;
> + data = kmalloc(sz, GFP_KERNEL);
> +
> + if (!data)
> + return -ENOMEM;
> +
> + ret = rpmb_read_blocks(rdev, rblocks.addr, rblocks.count, data);
> +
> + if (ret == 0)
> + ret = copy_to_user(rblocks.data, data, sz);
> +
> + kfree(data);
> + return ret;
> +}
> +
> +/**
> + * rpmb_ioctl - rpmb ioctl dispatcher
> + *
> + * @fp: a file pointer
> + * @cmd: ioctl command RPMB_IOC_SEQ_CMD RPMB_IOC_VER_CMD
> +RPMB_IOC_CAP_CMD
> + * @arg: ioctl data: rpmb_ioc_ver_cmd rpmb_ioc_cap_cmd
> pmb_ioc_seq_cmd
> + *
> + * Return: 0 on success; < 0 on error
> + */
> +static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned long
> +arg) {
> + struct rpmb_dev *rdev = fp->private_data;
> + void __user *ptr = (void __user *)arg;
> +
> + switch (cmd) {
> + case RPMB_IOC_VER_CMD:
> + return rpmb_ioctl_ver_cmd(rdev, ptr);
> + case RPMB_IOC_CAP_CMD:
> + return rpmb_ioctl_cap_cmd(rdev, ptr);
> + case RPMB_IOC_PKEY_CMD:
> + return rpmb_ioctl_pkey_cmd(rdev, ptr);
> + case RPMB_IOC_COUNTER_CMD:
> + return rpmb_ioctl_counter_cmd(rdev, ptr);
> + case RPMB_IOC_WBLOCKS_CMD:
> + return rpmb_ioctl_wblocks_cmd(rdev, ptr);
> + case RPMB_IOC_RBLOCKS_CMD:
> + return rpmb_ioctl_rblocks_cmd(rdev, ptr);
> + default:
> + dev_err(&rdev->dev, "unsupported ioctl 0x%x.\n", cmd);
> + return -ENOIOCTLCMD;
> + }
> +}
> +
> +static const struct file_operations rpmb_fops = {
> + .open = rpmb_open,
> + .release = rpmb_release,
> + .unlocked_ioctl = rpmb_ioctl,
> + .owner = THIS_MODULE,
> + .llseek = noop_llseek,
> +};
> +
> +void rpmb_cdev_prepare(struct rpmb_dev *rdev) {
> + rdev->dev.devt = MKDEV(MAJOR(rpmb_devt), rdev->id);
> + rdev->cdev.owner = THIS_MODULE;
> + cdev_init(&rdev->cdev, &rpmb_fops);
> +}
> +
> +void rpmb_cdev_add(struct rpmb_dev *rdev) {
> + cdev_add(&rdev->cdev, rdev->dev.devt, 1); }
> +
> +void rpmb_cdev_del(struct rpmb_dev *rdev) {
> + if (rdev->dev.devt)
> + cdev_del(&rdev->cdev);
> +}
> +
> +int __init rpmb_cdev_init(void)
> +{
> + int ret;
> +
> + ret = alloc_chrdev_region(&rpmb_devt, 0, RPMB_MAX_DEVS,
> "rpmb");
> + if (ret < 0)
> + pr_err("unable to allocate char dev region\n");
> +
> + return ret;
> +}
> +
> +void __exit rpmb_cdev_exit(void)
> +{
> + unregister_chrdev_region(rpmb_devt, RPMB_MAX_DEVS); }
> diff --git a/drivers/char/rpmb/core.c b/drivers/char/rpmb/core.c index
> a2e21c14986a..e26d605e48e1 100644
> --- a/drivers/char/rpmb/core.c
> +++ b/drivers/char/rpmb/core.c
> @@ -12,6 +12,7 @@
> #include <linux/slab.h>
>
> #include <linux/rpmb.h>
> +#include "rpmb-cdev.h"
>
> static DEFINE_IDA(rpmb_ida);
>
> @@ -277,6 +278,7 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev)
> return -EINVAL;
>
> mutex_lock(&rdev->lock);
> + rpmb_cdev_del(rdev);
> device_del(&rdev->dev);
> mutex_unlock(&rdev->lock);
>
> @@ -371,9 +373,6 @@ struct rpmb_dev *rpmb_dev_register(struct device
> *dev, u8 target,
> if (!ops->read_blocks)
> return ERR_PTR(-EINVAL);
>
> - if (ops->type == RPMB_TYPE_ANY || ops->type >
> RPMB_TYPE_MAX)
> - return ERR_PTR(-EINVAL);
> -
> rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
> if (!rdev)
> return ERR_PTR(-ENOMEM);
> @@ -396,6 +395,8 @@ struct rpmb_dev *rpmb_dev_register(struct device
> *dev, u8 target,
> if (ret)
> goto exit;
>
> + rpmb_cdev_add(rdev);
> +
> dev_dbg(&rdev->dev, "registered device\n");
>
> return rdev;
> @@ -412,11 +413,12 @@ static int __init rpmb_init(void) {
> ida_init(&rpmb_ida);
> class_register(&rpmb_class);
> - return 0;
> + return rpmb_cdev_init();
> }
>
> static void __exit rpmb_exit(void)
> {
> + rpmb_cdev_exit();
> class_unregister(&rpmb_class);
> ida_destroy(&rpmb_ida);
> }
> diff --git a/drivers/char/rpmb/rpmb-cdev.h b/drivers/char/rpmb/rpmb-
> cdev.h new file mode 100644 index 000000000000..e59ff0c05e9d
> --- /dev/null
> +++ b/drivers/char/rpmb/rpmb-cdev.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
> +/*
> + * Copyright (C) 2015-2018 Intel Corp. All rights reserved */ #ifdef
> +CONFIG_RPMB_INTF_DEV int __init rpmb_cdev_init(void); void __exit
> +rpmb_cdev_exit(void); void rpmb_cdev_prepare(struct rpmb_dev *rdev);
> +void rpmb_cdev_add(struct rpmb_dev *rdev); void rpmb_cdev_del(struct
> +rpmb_dev *rdev); #else static inline int __init rpmb_cdev_init(void) {
> +return 0; } static inline void __exit rpmb_cdev_exit(void) {} static
> +inline void rpmb_cdev_prepare(struct rpmb_dev *rdev) {} static inline
> +void rpmb_cdev_add(struct rpmb_dev *rdev) {} static inline void
> +rpmb_cdev_del(struct rpmb_dev *rdev) {} #endif /*
> CONFIG_RPMB_INTF_DEV
> +*/
> diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h index
> 718ba7c91ecd..fe44f60efe31 100644
> --- a/include/linux/rpmb.h
> +++ b/include/linux/rpmb.h
> @@ -8,9 +8,13 @@
>
> #include <linux/types.h>
> #include <linux/device.h>
> +#include <linux/cdev.h>
> +#include <uapi/linux/rpmb.h>
> #include <linux/kref.h>
> #include <linux/key.h>
>
> +#define RPMB_API_VERSION 0x80000001
> +
> /**
> * struct rpmb_ops - RPMB ops to be implemented by underlying block
> device
> *
> @@ -51,6 +55,8 @@ struct rpmb_ops {
> * @dev : device
> * @id : device id
> * @target : RPMB target/region within the physical device
> + * @cdev : character dev
> + * @status : device status
> * @ops : operation exported by block layer
> */
> struct rpmb_dev {
> @@ -58,6 +64,10 @@ struct rpmb_dev {
> struct device dev;
> int id;
> u8 target;
> +#ifdef CONFIG_RPMB_INTF_DEV
> + struct cdev cdev;
> + unsigned long status;
> +#endif /* CONFIG_RPMB_INTF_DEV */
> const struct rpmb_ops *ops;
> };
>
> diff --git a/include/uapi/linux/rpmb.h b/include/uapi/linux/rpmb.h new file
> mode 100644 index 000000000000..3957b785cdd5
> --- /dev/null
> +++ b/include/uapi/linux/rpmb.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR
> +BSD-3-Clause) */
> +/*
> + * Copyright (C) 2015-2018 Intel Corp. All rights reserved
> + * Copyright (C) 2021 Linaro Ltd
> + */
> +#ifndef _UAPI_LINUX_RPMB_H_
> +#define _UAPI_LINUX_RPMB_H_
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct rpmb_ioc_ver_cmd - rpmb api version
> + *
> + * @api_version: rpmb API version.
> + */
> +struct rpmb_ioc_ver_cmd {
> + __u32 api_version;
> +} __packed;
> +
> +enum rpmb_auth_method {
> + RPMB_HMAC_ALGO_SHA_256 = 0,
> +};
> +
> +/**
> + * struct rpmb_ioc_cap_cmd - rpmb capabilities
> + *
> + * @target: rpmb target/region within RPMB partition.
> + * @capacity: storage capacity (in units of 128K)
> + * @block_size: storage data block size (in units of 256B)
> + * @wr_cnt_max: maximal number of block that can be written in a single
> request.
> + * @rd_cnt_max: maximal number of block that can be read in a single
> request.
> + * @auth_method: authentication method: currently always
> HMAC_SHA_256
> + * @reserved: reserved to align to 4 bytes.
> + */
> +struct rpmb_ioc_cap_cmd {
> + __u16 target;
> + __u16 capacity;
> + __u16 block_size;
> + __u16 wr_cnt_max;
> + __u16 rd_cnt_max;
> + __u16 auth_method;
> + __u16 reserved;
> +} __attribute__((packed));
> +
> +/**
> + * struct rpmb_ioc_blocks_cmd - read/write blocks to/from RPMB
> + *
> + * @keyid: key_serial_t of key to use
> + * @addr: index into device (units of 256B blocks)
> + * @count: number of 256B blocks
> + * @data: pointer to data to write/read */ struct rpmb_ioc_blocks_cmd
> +{
> + __s32 key; /* key_serial_t */
> + __u32 addr;
> + __u32 count;
> + __u8 __user *data;
> +} __attribute__((packed));
> +
> +
> +#define RPMB_IOC_VER_CMD _IOR(0xB8, 80, struct rpmb_ioc_ver_cmd)
> +#define RPMB_IOC_CAP_CMD _IOR(0xB8, 81, struct rpmb_ioc_cap_cmd)
> +#define RPMB_IOC_PKEY_CMD _IOW(0xB8, 82, key_serial_t)
> +#define RPMB_IOC_COUNTER_CMD _IOR(0xB8, 84, int) #define
> +RPMB_IOC_WBLOCKS_CMD _IOW(0xB8, 85, struct rpmb_ioc_blocks_cmd)
> #define
> +RPMB_IOC_RBLOCKS_CMD _IOR(0xB8, 86, struct rpmb_ioc_blocks_cmd)
> +
> +#endif /* _UAPI_LINUX_RPMB_H_ */
> --
> 2.20.1
Powered by blists - more mailing lists