2024-05-10 10:18:43

by Marek Behún

[permalink] [raw]
Subject: [PATCH v10 0/9] Turris Omnia MCU driver

Hello Andy, Hans, Ilpo, Arnd, Gregory, and others,

this is v10 of the series adding Turris Omnia MCU driver.

This series still depends on the immutable branch between LEDs and
locking, introducing devm_mutex_init(), see the PR
https://lore.kernel.org/linux-leds/[email protected]/

See also cover letters for v1 to v9:
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/

Changes since v9:
- in functions where mcu->client is referred to multiple times added
a helper variable client = mcu->client, suggested by Andy
- changed instances of
if (!err)
do_something;
return err;
patterns to the more traditional pattern
if (err)
return err;
do_something;
return 0;
as suggested by Andy
- fixed some typos reported by Andy
- changed crc32_be(0xffffffff, ...) to crc32_be(~0, ...), as suggested
by Andy
- changed to use the DECI unit macro from units.h instead of hardcoded
10, as suggested by Andy
- changed -EINTR to -ERESTARTSYS in trng read and signature read
- changed the mutex lock in signature read to interruptible mutex lock,
as pointed out by Andy

Marek Behún (9):
dt-bindings: firmware: add cznic,turris-omnia-mcu binding
platform: cznic: Add preliminary support for Turris Omnia MCU
platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs
platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup
platform: cznic: turris-omnia-mcu: Add support for MCU watchdog
platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
platform: cznic: turris-omnia-mcu: Add support for digital message
signing via debugfs
ARM: dts: turris-omnia: Add MCU system-controller node
ARM: dts: turris-omnia: Add GPIO key node for front button

.../ABI/testing/debugfs-turris-omnia-mcu | 13 +
.../sysfs-bus-i2c-devices-turris-omnia-mcu | 126 ++
.../firmware/cznic,turris-omnia-mcu.yaml | 86 ++
MAINTAINERS | 5 +
.../dts/marvell/armada-385-turris-omnia.dts | 35 +-
drivers/platform/Kconfig | 2 +
drivers/platform/Makefile | 1 +
drivers/platform/cznic/Kconfig | 50 +
drivers/platform/cznic/Makefile | 9 +
.../platform/cznic/turris-omnia-mcu-base.c | 450 +++++++
.../platform/cznic/turris-omnia-mcu-debugfs.c | 208 ++++
.../platform/cznic/turris-omnia-mcu-gpio.c | 1039 +++++++++++++++++
.../cznic/turris-omnia-mcu-sys-off-wakeup.c | 257 ++++
.../platform/cznic/turris-omnia-mcu-trng.c | 103 ++
.../cznic/turris-omnia-mcu-watchdog.c | 128 ++
drivers/platform/cznic/turris-omnia-mcu.h | 216 ++++
include/linux/turris-omnia-mcu-interface.h | 249 ++++
17 files changed, 2976 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/debugfs-turris-omnia-mcu
create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
create mode 100644 Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml
create mode 100644 drivers/platform/cznic/Kconfig
create mode 100644 drivers/platform/cznic/Makefile
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-base.c
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-debugfs.c
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-gpio.c
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-trng.c
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-watchdog.c
create mode 100644 drivers/platform/cznic/turris-omnia-mcu.h
create mode 100644 include/linux/turris-omnia-mcu-interface.h

--
2.43.2



2024-05-10 10:18:49

by Marek Behún

[permalink] [raw]
Subject: [PATCH v10 6/9] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

Add support for true random number generator provided by the MCU.
New Omnia boards come without the Atmel SHA204-A chip. Instead the
crypto functionality is provided by new microcontroller, which has
a TRNG peripheral.

Signed-off-by: Marek Behún <[email protected]>
---
drivers/platform/cznic/Kconfig | 2 +
drivers/platform/cznic/Makefile | 1 +
.../platform/cznic/turris-omnia-mcu-base.c | 6 +-
.../platform/cznic/turris-omnia-mcu-gpio.c | 2 +-
.../platform/cznic/turris-omnia-mcu-trng.c | 103 ++++++++++++++++++
drivers/platform/cznic/turris-omnia-mcu.h | 8 ++
6 files changed, 120 insertions(+), 2 deletions(-)
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-trng.c

diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index e262930b3faf..6edac80d5fa3 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -18,6 +18,7 @@ config TURRIS_OMNIA_MCU
depends on I2C
select GPIOLIB
select GPIOLIB_IRQCHIP
+ select HW_RANDOM
select RTC_CLASS
select WATCHDOG_CORE
help
@@ -27,6 +28,7 @@ config TURRIS_OMNIA_MCU
- board poweroff into true low power mode (with voltage regulators
disabled) and the ability to configure wake up from this mode (via
rtcwake)
+ - true random number generator (if available on the MCU)
- MCU watchdog
- GPIO pins
- to get front button press events (the front button can be
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
index 687f7718c0a1..eae4c6b341ff 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU) += turris-omnia-mcu.o
turris-omnia-mcu-y := turris-omnia-mcu-base.o
turris-omnia-mcu-y += turris-omnia-mcu-gpio.o
turris-omnia-mcu-y += turris-omnia-mcu-sys-off-wakeup.o
+turris-omnia-mcu-y += turris-omnia-mcu-trng.o
turris-omnia-mcu-y += turris-omnia-mcu-watchdog.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index f44996588d38..1d4153a96526 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -380,7 +380,11 @@ static int omnia_mcu_probe(struct i2c_client *client)
if (err)
return err;

- return omnia_mcu_register_gpiochip(mcu);
+ err = omnia_mcu_register_gpiochip(mcu);
+ if (err)
+ return err;
+
+ return omnia_mcu_register_trng(mcu);
}

static const struct of_device_id of_omnia_mcu_match[] = {
diff --git a/drivers/platform/cznic/turris-omnia-mcu-gpio.c b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
index 966a39656644..d4498e8f30cb 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-gpio.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
@@ -167,7 +167,7 @@ static const struct omnia_gpio omnia_gpios[64] = {
};

/* mapping from interrupts to indexes of GPIOs in the omnia_gpios array */
-static const u8 omnia_int_to_gpio_idx[32] = {
+const u8 omnia_int_to_gpio_idx[32] = {
[__bf_shf(OMNIA_INT_CARD_DET)] = 4,
[__bf_shf(OMNIA_INT_MSATA_IND)] = 5,
[__bf_shf(OMNIA_INT_USB30_OVC)] = 6,
diff --git a/drivers/platform/cznic/turris-omnia-mcu-trng.c b/drivers/platform/cznic/turris-omnia-mcu-trng.c
new file mode 100644
index 000000000000..fbde00f3fca1
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-trng.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU TRNG driver
+ *
+ * 2024 by Marek Behún <[email protected]>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/hw_random.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+
+#include "turris-omnia-mcu.h"
+
+#define OMNIA_CMD_TRNG_MAX_ENTROPY_LEN 64
+
+static irqreturn_t omnia_trng_irq_handler(int irq, void *dev_id)
+{
+ struct omnia_mcu *mcu = dev_id;
+
+ complete(&mcu->trng_completion);
+
+ return IRQ_HANDLED;
+}
+
+static int omnia_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+ struct omnia_mcu *mcu = (struct omnia_mcu *)rng->priv;
+ u8 reply[1 + OMNIA_CMD_TRNG_MAX_ENTROPY_LEN];
+ int err, bytes;
+
+ if (!wait && !completion_done(&mcu->trng_completion))
+ return 0;
+
+ do {
+ if (wait_for_completion_interruptible(&mcu->trng_completion))
+ return -ERESTARTSYS;
+
+ err = omnia_cmd_read(mcu->client,
+ OMNIA_CMD_TRNG_COLLECT_ENTROPY,
+ reply, sizeof(reply));
+ if (err)
+ return err;
+
+ bytes = min3(reply[0], max, OMNIA_CMD_TRNG_MAX_ENTROPY_LEN);
+ } while (wait && !bytes);
+
+ memcpy(data, &reply[1], bytes);
+
+ return bytes;
+}
+
+int omnia_mcu_register_trng(struct omnia_mcu *mcu)
+{
+ struct device *dev = &mcu->client->dev;
+ u8 irq_idx, dummy;
+ int irq, err;
+
+ if (!(mcu->features & OMNIA_FEAT_TRNG))
+ return 0;
+
+ irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
+ irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
+
+ /*
+ * If someone else cleared the TRNG interrupt but did not read the
+ * entropy, a new interrupt won't be generated, and entropy collection
+ * will be stuck. Ensure an interrupt will be generated by executing
+ * the collect entropy command (and discarding the result).
+ */
+ err = omnia_cmd_read(mcu->client, OMNIA_CMD_TRNG_COLLECT_ENTROPY,
+ &dummy, 1);
+ if (err)
+ return err;
+
+ init_completion(&mcu->trng_completion);
+
+ err = devm_request_threaded_irq(dev, irq, NULL, omnia_trng_irq_handler,
+ IRQF_ONESHOT, "turris-omnia-mcu-trng",
+ mcu);
+ if (err)
+ return dev_err_probe(dev, err, "Cannot request TRNG IRQ\n");
+
+ mcu->trng.name = "turris-omnia-mcu-trng";
+ mcu->trng.read = omnia_trng_read;
+ mcu->trng.priv = (unsigned long)mcu;
+
+ err = devm_hwrng_register(dev, &mcu->trng);
+ if (err)
+ return dev_err_probe(dev, err, "Cannot register TRNG\n");
+
+ return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index f54d17af457d..7d71890664b1 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -9,7 +9,9 @@
#define __TURRIS_OMNIA_MCU_H

#include <linux/bitops.h>
+#include <linux/completion.h>
#include <linux/gpio/driver.h>
+#include <linux/hw_random.h>
#include <linux/if_ether.h>
#include <linux/mutex.h>
#include <linux/rtc.h>
@@ -47,6 +49,10 @@ struct omnia_mcu {

/* MCU watchdog */
struct watchdog_device wdt;
+
+ /* true random number generator */
+ struct hwrng trng;
+ struct completion trng_completion;
};

int omnia_cmd_write_read(const struct i2c_client *client,
@@ -175,11 +181,13 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd,
return omnia_cmd_read(client, cmd, reply, sizeof(*reply));
}

+extern const u8 omnia_int_to_gpio_idx[32];
extern const struct attribute_group omnia_mcu_gpio_group;
extern const struct attribute_group omnia_mcu_poweroff_group;

int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
+int omnia_mcu_register_trng(struct omnia_mcu *mcu);
int omnia_mcu_register_watchdog(struct omnia_mcu *mcu);

#endif /* __TURRIS_OMNIA_MCU_H */
--
2.43.2


2024-05-10 10:23:18

by Marek Behún

[permalink] [raw]
Subject: [PATCH v10 7/9] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs

Add support for digital message signing with private key stored in the
MCU. Boards with MKL MCUs have a NIST256p ECDSA private key created
when manufactured. The private key is not readable from the MCU, but
MCU allows for signing messages with it and retrieving the public key.

As described in a similar commit 50524d787de3 ("firmware:
turris-mox-rwtm: support ECDSA signatures via debugfs"):
The optimal solution would be to register an akcipher provider via
kernel's crypto API, but crypto API does not yet support accessing
akcipher API from userspace (and probably won't for some time, see
https://www.spinics.net/lists/linux-crypto/msg38388.html).

Therefore we add support for accessing this signature generation
mechanism via debugfs for now, so that userspace can access it.

Signed-off-by: Marek Behún <[email protected]>
---
.../ABI/testing/debugfs-turris-omnia-mcu | 13 ++
.../sysfs-bus-i2c-devices-turris-omnia-mcu | 13 ++
MAINTAINERS | 1 +
drivers/platform/cznic/Kconfig | 2 +
drivers/platform/cznic/Makefile | 1 +
.../platform/cznic/turris-omnia-mcu-base.c | 45 +++-
.../platform/cznic/turris-omnia-mcu-debugfs.c | 208 ++++++++++++++++++
drivers/platform/cznic/turris-omnia-mcu.h | 23 ++
8 files changed, 305 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/debugfs-turris-omnia-mcu
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-debugfs.c

diff --git a/Documentation/ABI/testing/debugfs-turris-omnia-mcu b/Documentation/ABI/testing/debugfs-turris-omnia-mcu
new file mode 100644
index 000000000000..1665005c2dcd
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-turris-omnia-mcu
@@ -0,0 +1,13 @@
+What: /sys/kernel/debug/turris-omnia-mcu/do_sign
+Date: July 2024
+KernelVersion: 6.10
+Contact: Marek Behún <[email protected]>
+Description:
+
+ ======= ===========================================================
+ (Write) Message to sign with the ECDSA private key stored in MCU.
+ The message must be exactly 32 bytes long (since this is
+ intended to be a SHA-256 hash).
+ (Read) The resulting signature, 64 bytes. This contains the R and
+ S values of the ECDSA signature, both in big-endian format.
+ ======= ===========================================================
diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
index 5e2d8ec52374..42710b8c77ef 100644
--- a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
@@ -90,6 +90,19 @@ Description: (RO) Contains the microcontroller type (STM32, GD32, MKL).

Format: %s.

+What: /sys/bus/i2c/devices/<mcu_device>/public_key
+Date: July 2024
+KernelVersion: 6.10
+Contact: Marek Behún <[email protected]>
+Description: (RO) Contains board ECDSA public key.
+
+ Only available if MCU supports signing messages with the ECDSA
+ algorithm. If so, the board has a private key stored in the MCU
+ that was generated during manufacture and cannot be retrieved
+ from the MCU.
+
+ Format: %s.
+
What: /sys/bus/i2c/devices/<mcu_device>/reset_selector
Date: July 2024
KernelVersion: 6.10
diff --git a/MAINTAINERS b/MAINTAINERS
index ed95e9147c42..e60dbeb89857 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2139,6 +2139,7 @@ M: Marek Behún <[email protected]>
S: Maintained
W: https://www.turris.cz/
F: Documentation/ABI/testing/debugfs-moxtet
+F: Documentation/ABI/testing/debugfs-turris-omnia-mcu
F: Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
F: Documentation/ABI/testing/sysfs-bus-moxtet-devices
F: Documentation/ABI/testing/sysfs-firmware-turris-mox-rwtm
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index 6edac80d5fa3..152c866d63a6 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -29,6 +29,8 @@ config TURRIS_OMNIA_MCU
disabled) and the ability to configure wake up from this mode (via
rtcwake)
- true random number generator (if available on the MCU)
+ - ECDSA message signing with board private key (if available on the
+ MCU)
- MCU watchdog
- GPIO pins
- to get front button press events (the front button can be
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
index eae4c6b341ff..af9213928404 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -6,3 +6,4 @@ turris-omnia-mcu-y += turris-omnia-mcu-gpio.o
turris-omnia-mcu-y += turris-omnia-mcu-sys-off-wakeup.o
turris-omnia-mcu-y += turris-omnia-mcu-trng.o
turris-omnia-mcu-y += turris-omnia-mcu-watchdog.o
+turris-omnia-mcu-$(CONFIG_DEBUG_FS) += turris-omnia-mcu-debugfs.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index 1d4153a96526..316ce8200885 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -162,6 +162,16 @@ static ssize_t board_revision_show(struct device *dev,
}
static DEVICE_ATTR_RO(board_revision);

+static ssize_t public_key_show(struct device *dev, struct device_attribute *a,
+ char *buf)
+{
+ struct omnia_mcu *mcu = i2c_get_clientdata(to_i2c_client(dev));
+
+ return sysfs_emit(buf, "%*phN\n", OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN,
+ mcu->board_public_key);
+}
+static DEVICE_ATTR_RO(public_key);
+
static struct attribute *omnia_mcu_base_attrs[] = {
&dev_attr_fw_version_hash_application.attr,
&dev_attr_fw_version_hash_bootloader.attr,
@@ -171,6 +181,7 @@ static struct attribute *omnia_mcu_base_attrs[] = {
&dev_attr_serial_number.attr,
&dev_attr_first_mac_address.attr,
&dev_attr_board_revision.attr,
+ &dev_attr_public_key.attr,
NULL
};

@@ -185,6 +196,9 @@ static umode_t omnia_mcu_base_attrs_visible(struct kobject *kobj,
a == &dev_attr_board_revision.attr) &&
!(mcu->features & OMNIA_FEAT_BOARD_INFO))
return 0;
+ else if (a == &dev_attr_public_key.attr &&
+ !(mcu->features & OMNIA_FEAT_CRYPTO))
+ return 0;

return a->mode;
}
@@ -344,6 +358,24 @@ static int omnia_mcu_read_board_info(struct omnia_mcu *mcu)
return 0;
}

+static int omnia_mcu_read_public_key(struct omnia_mcu *mcu)
+{
+ u8 reply[1 + OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN];
+ int err;
+
+ err = omnia_cmd_read(mcu->client, OMNIA_CMD_CRYPTO_GET_PUBLIC_KEY,
+ reply, sizeof(reply));
+ if (err)
+ return err;
+
+ if (reply[0] != OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN)
+ return -EIO;
+
+ memcpy(mcu->board_public_key, &reply[1], sizeof(mcu->board_public_key));
+
+ return 0;
+}
+
static int omnia_mcu_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -372,6 +404,13 @@ static int omnia_mcu_probe(struct i2c_client *client)
"Cannot read board info\n");
}

+ if (mcu->features & OMNIA_FEAT_CRYPTO) {
+ err = omnia_mcu_read_public_key(mcu);
+ if (err)
+ return dev_err_probe(dev, err,
+ "Cannot read board public key\n");
+ }
+
err = omnia_mcu_register_sys_off_and_wakeup(mcu);
if (err)
return err;
@@ -384,7 +423,11 @@ static int omnia_mcu_probe(struct i2c_client *client)
if (err)
return err;

- return omnia_mcu_register_trng(mcu);
+ err = omnia_mcu_register_trng(mcu);
+ if (err)
+ return err;
+
+ return omnia_mcu_register_debugfs(mcu);
}

static const struct of_device_id of_omnia_mcu_match[] = {
diff --git a/drivers/platform/cznic/turris-omnia-mcu-debugfs.c b/drivers/platform/cznic/turris-omnia-mcu-debugfs.c
new file mode 100644
index 000000000000..ae92b41037cf
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-debugfs.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU ECDSA message signing via debugfs
+ *
+ * 2024 by Marek Behún <[email protected]>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/string.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "turris-omnia-mcu.h"
+
+#define OMNIA_CMD_CRYPTO_SIGN_MESSAGE_LEN 32
+
+enum {
+ SIGN_STATE_CLOSED = 0,
+ SIGN_STATE_OPEN = 1,
+ SIGN_STATE_REQUESTED = 2,
+ SIGN_STATE_COLLECTED = 3,
+};
+
+static irqreturn_t omnia_msg_signed_irq_handler(int irq, void *dev_id)
+{
+ u8 reply[1 + OMNIA_MCU_CRYPTO_SIGNATURE_LEN];
+ struct omnia_mcu *mcu = dev_id;
+ int err;
+
+ err = omnia_cmd_read(mcu->client, OMNIA_CMD_CRYPTO_COLLECT_SIGNATURE,
+ reply, sizeof(reply));
+ if (!err && reply[0] != OMNIA_MCU_CRYPTO_SIGNATURE_LEN)
+ err = -EIO;
+
+ guard(mutex)(&mcu->sign_lock);
+
+ if (mcu->sign_state == SIGN_STATE_REQUESTED) {
+ mcu->sign_err = err;
+ if (!err)
+ memcpy(mcu->signature, &reply[1],
+ OMNIA_MCU_CRYPTO_SIGNATURE_LEN);
+ mcu->sign_state = SIGN_STATE_COLLECTED;
+ complete(&mcu->msg_signed_completion);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int do_sign_open(struct inode *inode, struct file *file)
+{
+ struct omnia_mcu *mcu = inode->i_private;
+
+ guard(mutex)(&mcu->sign_lock);
+
+ /* do_sign is allowed to be opened only once */
+ if (mcu->sign_state != SIGN_STATE_CLOSED)
+ return -EBUSY;
+
+ mcu->sign_state = SIGN_STATE_OPEN;
+
+ file->private_data = mcu;
+
+ return nonseekable_open(inode, file);
+}
+
+static int do_sign_release(struct inode *inode, struct file *file)
+{
+ struct omnia_mcu *mcu = file->private_data;
+
+ guard(mutex)(&mcu->sign_lock);
+
+ mcu->sign_state = SIGN_STATE_CLOSED;
+
+ /* forget signature on release even if it was not read, for security */
+ memzero_explicit(mcu->signature, sizeof(mcu->signature));
+
+ return 0;
+}
+
+static ssize_t do_sign_read(struct file *file, char __user *buf, size_t len,
+ loff_t *ppos)
+{
+ struct omnia_mcu *mcu = file->private_data;
+
+ /* only allow read of one whole signature */
+ if (len != sizeof(mcu->signature))
+ return -EINVAL;
+
+ scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &mcu->sign_lock) {
+ if (mcu->sign_state != SIGN_STATE_REQUESTED &&
+ mcu->sign_state != SIGN_STATE_COLLECTED)
+ return -ENODATA;
+ }
+
+ if (wait_for_completion_interruptible(&mcu->msg_signed_completion))
+ return -ERESTARTSYS;
+
+ guard(mutex)(&mcu->sign_lock);
+
+ mcu->sign_state = SIGN_STATE_OPEN;
+
+ if (mcu->sign_err)
+ return mcu->sign_err;
+
+ if (copy_to_user(buf, mcu->signature, len))
+ return -EFAULT;
+
+ /* on read forget the signature, for security */
+ memzero_explicit(mcu->signature, sizeof(mcu->signature));
+
+ return len;
+}
+
+static ssize_t do_sign_write(struct file *file, const char __user *buf,
+ size_t len, loff_t *ppos)
+{
+ u8 cmd[1 + OMNIA_CMD_CRYPTO_SIGN_MESSAGE_LEN], reply;
+ struct omnia_mcu *mcu = file->private_data;
+ int err;
+
+ /*
+ * the input is a SHA-256 hash of a message to sign, so exactly
+ * 32 bytes have to be read
+ */
+ if (len != OMNIA_CMD_CRYPTO_SIGN_MESSAGE_LEN)
+ return -EINVAL;
+
+ cmd[0] = OMNIA_CMD_CRYPTO_SIGN_MESSAGE;
+
+ if (copy_from_user(&cmd[1], buf, len))
+ return -EFAULT;
+
+ guard(mutex)(&mcu->sign_lock);
+
+ if (mcu->sign_state != SIGN_STATE_OPEN)
+ return -EBUSY;
+
+ err = omnia_cmd_write_read(mcu->client, cmd, sizeof(cmd), &reply, 1);
+ if (err)
+ return err;
+
+ if (reply)
+ mcu->sign_state = SIGN_STATE_REQUESTED;
+
+ return reply ? len : -EBUSY;
+}
+
+static const struct file_operations do_sign_fops = {
+ .owner = THIS_MODULE,
+ .open = do_sign_open,
+ .read = do_sign_read,
+ .write = do_sign_write,
+ .release = do_sign_release,
+ .llseek = no_llseek,
+};
+
+static void omnia_debugfs_drop_dir(void *res)
+{
+ debugfs_remove_recursive(res);
+}
+
+int omnia_mcu_register_debugfs(struct omnia_mcu *mcu)
+{
+ struct device *dev = &mcu->client->dev;
+ struct dentry *root;
+ int irq, err;
+ u8 irq_idx;
+
+ if (!(mcu->features & OMNIA_FEAT_CRYPTO))
+ return 0;
+
+ irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_MESSAGE_SIGNED)];
+ irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
+ if (irq < 0)
+ return dev_err_probe(dev, irq,
+ "Cannot get MESSAGE_SIGNED IRQ\n");
+
+ err = devm_mutex_init(dev, &mcu->sign_lock);
+ if (err)
+ return err;
+
+ mcu->sign_state = 0;
+
+ init_completion(&mcu->msg_signed_completion);
+
+ err = devm_request_threaded_irq(dev, irq, NULL,
+ omnia_msg_signed_irq_handler,
+ IRQF_ONESHOT,
+ "turris-omnia-mcu-debugfs", mcu);
+ if (err)
+ return dev_err_probe(dev, err,
+ "Cannot request MESSAGE_SIGNED IRQ\n");
+
+ root = debugfs_create_dir("turris-omnia-mcu", NULL);
+ debugfs_create_file_unsafe("do_sign", 0600, root, mcu, &do_sign_fops);
+
+ return devm_add_action_or_reset(dev, omnia_debugfs_drop_dir, root);
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index 7d71890664b1..1b7a1512ae0e 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -21,6 +21,9 @@
#include <asm/byteorder.h>
#include <asm/unaligned.h>

+#define OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN (1 + 32)
+#define OMNIA_MCU_CRYPTO_SIGNATURE_LEN 64
+
struct i2c_client;

struct omnia_mcu {
@@ -32,6 +35,7 @@ struct omnia_mcu {
u64 board_serial_number;
u8 board_first_mac[ETH_ALEN];
u8 board_revision;
+ u8 board_public_key[OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN];

/* GPIO chip */
struct gpio_chip gc;
@@ -53,6 +57,16 @@ struct omnia_mcu {
/* true random number generator */
struct hwrng trng;
struct completion trng_completion;
+
+#ifdef CONFIG_DEBUG_FS
+ /* MCU ECDSA message signing via debugfs */
+ struct dentry *debugfs_root;
+ struct completion msg_signed_completion;
+ struct mutex sign_lock;
+ unsigned int sign_state;
+ u8 signature[OMNIA_MCU_CRYPTO_SIGNATURE_LEN];
+ int sign_err;
+#endif
};

int omnia_cmd_write_read(const struct i2c_client *client,
@@ -190,4 +204,13 @@ int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
int omnia_mcu_register_trng(struct omnia_mcu *mcu);
int omnia_mcu_register_watchdog(struct omnia_mcu *mcu);

+#ifdef CONFIG_DEBUG_FS
+int omnia_mcu_register_debugfs(struct omnia_mcu *mcu);
+#else
+static inline int omnia_mcu_register_debugfs(struct omnia_mcu *mcu)
+{
+ return 0;
+}
+#endif
+
#endif /* __TURRIS_OMNIA_MCU_H */
--
2.43.2


2024-05-10 10:53:13

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v10 7/9] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs

On Fri, May 10, 2024 at 12:18:17PM +0200, Marek Beh?n wrote:
> Add support for digital message signing with private key stored in the
> MCU. Boards with MKL MCUs have a NIST256p ECDSA private key created
> when manufactured. The private key is not readable from the MCU, but
> MCU allows for signing messages with it and retrieving the public key.
>
> As described in a similar commit 50524d787de3 ("firmware:
> turris-mox-rwtm: support ECDSA signatures via debugfs"):
> The optimal solution would be to register an akcipher provider via
> kernel's crypto API, but crypto API does not yet support accessing
> akcipher API from userspace (and probably won't for some time, see
> https://www.spinics.net/lists/linux-crypto/msg38388.html).
>
> Therefore we add support for accessing this signature generation
> mechanism via debugfs for now, so that userspace can access it.

Having a "real" user/kernel api in debugfs feels wrong here, why would
you not do this properly? On most, if not all, systems, debugfs is
locked down so you do not have access to it, as it is only there for
debugging. So how is a user supposed to use this feature if they can't
get access to it?

And debugfs files can be changed at any time, so how can you ensure that
your new api will always be there?

In other words, please solve this properly, do not just add a hack into
debugfs that no one can use as that is not a good idea.

thanks,

greg k-h

2024-05-10 11:36:43

by Marek Behún

[permalink] [raw]
Subject: Re: [PATCH v10 7/9] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs

On Fri, 10 May 2024 11:52:56 +0100
Greg Kroah-Hartman <[email protected]> wrote:

> On Fri, May 10, 2024 at 12:18:17PM +0200, Marek Behún wrote:
> > Add support for digital message signing with private key stored in the
> > MCU. Boards with MKL MCUs have a NIST256p ECDSA private key created
> > when manufactured. The private key is not readable from the MCU, but
> > MCU allows for signing messages with it and retrieving the public key.
> >
> > As described in a similar commit 50524d787de3 ("firmware:
> > turris-mox-rwtm: support ECDSA signatures via debugfs"):
> > The optimal solution would be to register an akcipher provider via
> > kernel's crypto API, but crypto API does not yet support accessing
> > akcipher API from userspace (and probably won't for some time, see
> > https://www.spinics.net/lists/linux-crypto/msg38388.html).
> >
> > Therefore we add support for accessing this signature generation
> > mechanism via debugfs for now, so that userspace can access it.
>
> Having a "real" user/kernel api in debugfs feels wrong here, why would
> you not do this properly? On most, if not all, systems, debugfs is
> locked down so you do not have access to it, as it is only there for
> debugging. So how is a user supposed to use this feature if they can't
> get access to it?
>
> And debugfs files can be changed at any time, so how can you ensure that
> your new api will always be there?
>
> In other words, please solve this properly, do not just add a hack into
> debugfs that no one can use as that is not a good idea.

Hi Greg,

this is the same thing we discussed 5 years ago, I wanted to implement
it via crypto's akcipher, but was refused due to
https://www.spinics.net/lists/linux-crypto/msg38388.html

I've then exposed this via debugfs in the turris-mox-rwtm driver 4
years ago, and we have supported this in our utility scripts, with the
plan that to reimplement it in the kernel via the correct ABI once
akcipher (or other ABI) is available to userspace, but AFAIK after 5
years this is still not the case :-(

If not debugfs and not akcipher, another option is to expose this via
sysfs, but that also doesn't seem right, and if I recall correctly you
also disapproved of this 5 years ago.

The last option would be to create another device, something like
/dev/turris-crypto for this. I wanted to avoid that and wait for
akcipher to be exposed do crypto since another /dev device must be
supported forever, while debugfs implementation can be removed once
this is supported via standardized ABI.

Do you have any suggestions?

Marek

2024-05-10 11:39:43

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v10 7/9] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs

On Fri, May 10, 2024 at 01:31:58PM +0200, Marek Beh?n wrote:
> On Fri, 10 May 2024 11:52:56 +0100
> Greg Kroah-Hartman <[email protected]> wrote:
>
> > On Fri, May 10, 2024 at 12:18:17PM +0200, Marek Beh?n wrote:
> > > Add support for digital message signing with private key stored in the
> > > MCU. Boards with MKL MCUs have a NIST256p ECDSA private key created
> > > when manufactured. The private key is not readable from the MCU, but
> > > MCU allows for signing messages with it and retrieving the public key.
> > >
> > > As described in a similar commit 50524d787de3 ("firmware:
> > > turris-mox-rwtm: support ECDSA signatures via debugfs"):
> > > The optimal solution would be to register an akcipher provider via
> > > kernel's crypto API, but crypto API does not yet support accessing
> > > akcipher API from userspace (and probably won't for some time, see
> > > https://www.spinics.net/lists/linux-crypto/msg38388.html).
> > >
> > > Therefore we add support for accessing this signature generation
> > > mechanism via debugfs for now, so that userspace can access it.
> >
> > Having a "real" user/kernel api in debugfs feels wrong here, why would
> > you not do this properly? On most, if not all, systems, debugfs is
> > locked down so you do not have access to it, as it is only there for
> > debugging. So how is a user supposed to use this feature if they can't
> > get access to it?
> >
> > And debugfs files can be changed at any time, so how can you ensure that
> > your new api will always be there?
> >
> > In other words, please solve this properly, do not just add a hack into
> > debugfs that no one can use as that is not a good idea.
>
> Hi Greg,
>
> this is the same thing we discussed 5 years ago, I wanted to implement
> it via crypto's akcipher, but was refused due to
> https://www.spinics.net/lists/linux-crypto/msg38388.html
>
> I've then exposed this via debugfs in the turris-mox-rwtm driver 4
> years ago, and we have supported this in our utility scripts, with the
> plan that to reimplement it in the kernel via the correct ABI once
> akcipher (or other ABI) is available to userspace, but AFAIK after 5
> years this is still not the case :-(
>
> If not debugfs and not akcipher, another option is to expose this via
> sysfs, but that also doesn't seem right, and if I recall correctly you
> also disapproved of this 5 years ago.

Yeah, sysfs is not ok for this either.

> The last option would be to create another device, something like
> /dev/turris-crypto for this. I wanted to avoid that and wait for
> akcipher to be exposed do crypto since another /dev device must be
> supported forever, while debugfs implementation can be removed once
> this is supported via standardized ABI.
>
> Do you have any suggestions?

Not really, I can't see the link above (no internet connection right
now) but this should just be fixed properly at the crypto subsystem
instead of these horrible debugfs hacks.

thanks,

greg k-h

2024-05-10 11:56:06

by Marek Behún

[permalink] [raw]
Subject: Re: [PATCH v10 7/9] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs

On Fri, 10 May 2024 12:37:04 +0100
Greg Kroah-Hartman <[email protected]> wrote:

> On Fri, May 10, 2024 at 01:31:58PM +0200, Marek Behún wrote:
> > On Fri, 10 May 2024 11:52:56 +0100
> > Greg Kroah-Hartman <[email protected]> wrote:
> >
> > > On Fri, May 10, 2024 at 12:18:17PM +0200, Marek Behún wrote:
> > > > Add support for digital message signing with private key stored in the
> > > > MCU. Boards with MKL MCUs have a NIST256p ECDSA private key created
> > > > when manufactured. The private key is not readable from the MCU, but
> > > > MCU allows for signing messages with it and retrieving the public key.
> > > >
> > > > As described in a similar commit 50524d787de3 ("firmware:
> > > > turris-mox-rwtm: support ECDSA signatures via debugfs"):
> > > > The optimal solution would be to register an akcipher provider via
> > > > kernel's crypto API, but crypto API does not yet support accessing
> > > > akcipher API from userspace (and probably won't for some time, see
> > > > https://www.spinics.net/lists/linux-crypto/msg38388.html).
> > > >
> > > > Therefore we add support for accessing this signature generation
> > > > mechanism via debugfs for now, so that userspace can access it.
> > >
> > > Having a "real" user/kernel api in debugfs feels wrong here, why would
> > > you not do this properly? On most, if not all, systems, debugfs is
> > > locked down so you do not have access to it, as it is only there for
> > > debugging. So how is a user supposed to use this feature if they can't
> > > get access to it?
> > >
> > > And debugfs files can be changed at any time, so how can you ensure that
> > > your new api will always be there?
> > >
> > > In other words, please solve this properly, do not just add a hack into
> > > debugfs that no one can use as that is not a good idea.
> >
> > Hi Greg,
> >
> > this is the same thing we discussed 5 years ago, I wanted to implement
> > it via crypto's akcipher, but was refused due to
> > https://www.spinics.net/lists/linux-crypto/msg38388.html
> >
> > I've then exposed this via debugfs in the turris-mox-rwtm driver 4
> > years ago, and we have supported this in our utility scripts, with the
> > plan that to reimplement it in the kernel via the correct ABI once
> > akcipher (or other ABI) is available to userspace, but AFAIK after 5
> > years this is still not the case :-(
> >
> > If not debugfs and not akcipher, another option is to expose this via
> > sysfs, but that also doesn't seem right, and if I recall correctly you
> > also disapproved of this 5 years ago.
>
> Yeah, sysfs is not ok for this either.
>
> > The last option would be to create another device, something like
> > /dev/turris-crypto for this. I wanted to avoid that and wait for
> > akcipher to be exposed do crypto since another /dev device must be
> > supported forever, while debugfs implementation can be removed once
> > this is supported via standardized ABI.
> >
> > Do you have any suggestions?
>
> Not really, I can't see the link above (no internet connection right
> now) but this should just be fixed properly at the crypto subsystem
> instead of these horrible debugfs hacks.
>
> thanks,
>
> greg k-h

The mail is from Herbert Xu and it says the following:

The akcipher kernel API is still in a state of flux. See the
recent work on ecrdsa for example which affected the RSA API.

Until that settles down I will not allow akcipher to be exported
through af_alg as that would commit us to that API forever.

Marek

2024-05-10 12:52:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v10 7/9] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs

On Fri, May 10, 2024 at 01:50:20PM +0200, Marek Beh?n wrote:
> On Fri, 10 May 2024 12:37:04 +0100
> Greg Kroah-Hartman <[email protected]> wrote:
>
> > On Fri, May 10, 2024 at 01:31:58PM +0200, Marek Beh?n wrote:
> > > On Fri, 10 May 2024 11:52:56 +0100
> > > Greg Kroah-Hartman <[email protected]> wrote:
> > >
> > > > On Fri, May 10, 2024 at 12:18:17PM +0200, Marek Beh?n wrote:
> > > > > Add support for digital message signing with private key stored in the
> > > > > MCU. Boards with MKL MCUs have a NIST256p ECDSA private key created
> > > > > when manufactured. The private key is not readable from the MCU, but
> > > > > MCU allows for signing messages with it and retrieving the public key.
> > > > >
> > > > > As described in a similar commit 50524d787de3 ("firmware:
> > > > > turris-mox-rwtm: support ECDSA signatures via debugfs"):
> > > > > The optimal solution would be to register an akcipher provider via
> > > > > kernel's crypto API, but crypto API does not yet support accessing
> > > > > akcipher API from userspace (and probably won't for some time, see
> > > > > https://www.spinics.net/lists/linux-crypto/msg38388.html).
> > > > >
> > > > > Therefore we add support for accessing this signature generation
> > > > > mechanism via debugfs for now, so that userspace can access it.
> > > >
> > > > Having a "real" user/kernel api in debugfs feels wrong here, why would
> > > > you not do this properly? On most, if not all, systems, debugfs is
> > > > locked down so you do not have access to it, as it is only there for
> > > > debugging. So how is a user supposed to use this feature if they can't
> > > > get access to it?
> > > >
> > > > And debugfs files can be changed at any time, so how can you ensure that
> > > > your new api will always be there?
> > > >
> > > > In other words, please solve this properly, do not just add a hack into
> > > > debugfs that no one can use as that is not a good idea.
> > >
> > > Hi Greg,
> > >
> > > this is the same thing we discussed 5 years ago, I wanted to implement
> > > it via crypto's akcipher, but was refused due to
> > > https://www.spinics.net/lists/linux-crypto/msg38388.html
> > >
> > > I've then exposed this via debugfs in the turris-mox-rwtm driver 4
> > > years ago, and we have supported this in our utility scripts, with the
> > > plan that to reimplement it in the kernel via the correct ABI once
> > > akcipher (or other ABI) is available to userspace, but AFAIK after 5
> > > years this is still not the case :-(
> > >
> > > If not debugfs and not akcipher, another option is to expose this via
> > > sysfs, but that also doesn't seem right, and if I recall correctly you
> > > also disapproved of this 5 years ago.
> >
> > Yeah, sysfs is not ok for this either.
> >
> > > The last option would be to create another device, something like
> > > /dev/turris-crypto for this. I wanted to avoid that and wait for
> > > akcipher to be exposed do crypto since another /dev device must be
> > > supported forever, while debugfs implementation can be removed once
> > > this is supported via standardized ABI.
> > >
> > > Do you have any suggestions?
> >
> > Not really, I can't see the link above (no internet connection right
> > now) but this should just be fixed properly at the crypto subsystem
> > instead of these horrible debugfs hacks.
> >
> > thanks,
> >
> > greg k-h
>
> The mail is from Herbert Xu and it says the following:
>
> The akcipher kernel API is still in a state of flux. See the
> recent work on ecrdsa for example which affected the RSA API.
>
> Until that settles down I will not allow akcipher to be exported
> through af_alg as that would commit us to that API forever.

5 years is a long time for "in a state of flux", perhaps work on getting
that fixed up now that things are settled down?

thanks,

greg k-h