[RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

[RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

[Nayna Jain]

PowerVM provides an isolated Platform Keystore(PKS) storage allocation
for each partition with individually managed access controls to store
sensitive information securely. Linux Kernel can access this storage by
interfacing with hypervisor using a new set of hypervisor calls. 

PowerVM guest secure boot intend to use Platform Keystore for the
purpose of storing public keys. Secure boot requires public keys to
be able to verify the grub and boot kernel. To allow authenticated
 manipulation of keys, it supports variables to store key authorities
- PK/KEK and code signing keys - db. It also supports denied list to
disallow booting even if signed with valid key. This is done via
denied list database - dbx or sbat. These variables would be stored in
PKS, and are managed and controlled by firmware.

The purpose of this patchset is to add support for users to
read/write/add/delete variables required for secure boot on PowerVM.

Nayna Jain (2):
  pseries: define driver for Platform Keystore
  pseries: define sysfs interface to expose PKS variables

 Documentation/ABI/testing/sysfs-pksvar        |  77 +++
 arch/powerpc/include/asm/hvcall.h             |  13 +-
 arch/powerpc/include/asm/pks.h                |  84 +++
 arch/powerpc/platforms/pseries/Kconfig        |  17 +
 arch/powerpc/platforms/pseries/Makefile       |   2 +
 arch/powerpc/platforms/pseries/pks.c          | 494 ++++++++++++++++++
 arch/powerpc/platforms/pseries/pksvar-sysfs.c | 356 +++++++++++++
 7 files changed, 1042 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-pksvar
 create mode 100644 arch/powerpc/include/asm/pks.h
 create mode 100644 arch/powerpc/platforms/pseries/pks.c
 create mode 100644 arch/powerpc/platforms/pseries/pksvar-sysfs.c

-- 
2.27.0

[RFC PATCH 1/2] pseries: define driver for Platform Keystore

[Nayna Jain]

PowerVM provides an isolated Platform Keystore(PKS) storage allocation
for each partition with individually managed access controls to store
sensitive information securely. It provides a new set of hypervisor
calls for Linux kernel to access PKS storage.

Define PKS driver using H_CALL interface to access PKS storage.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/include/asm/hvcall.h       |  13 +-
 arch/powerpc/include/asm/pks.h          |  84 ++++
 arch/powerpc/platforms/pseries/Kconfig  |  10 +
 arch/powerpc/platforms/pseries/Makefile |   1 +
 arch/powerpc/platforms/pseries/pks.c    | 494 ++++++++++++++++++++++++
 5 files changed, 601 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/include/asm/pks.h
 create mode 100644 arch/powerpc/platforms/pseries/pks.c

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 9bcf345cb208..08108dcf8677 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -97,6 +97,7 @@
 #define H_OP_MODE	-73
 #define H_COP_HW	-74
 #define H_STATE		-75
+#define H_IN_USE	-77
 #define H_UNSUPPORTED_FLAG_START	-256
 #define H_UNSUPPORTED_FLAG_END		-511
 #define H_MULTI_THREADS_ACTIVE	-9005
@@ -321,9 +322,19 @@
 #define H_SCM_UNBIND_ALL        0x3FC
 #define H_SCM_HEALTH            0x400
 #define H_SCM_PERFORMANCE_STATS 0x418
+#define H_PKS_GET_CONFIG	0x41C
+#define H_PKS_SET_PASSWORD	0x420
+#define H_PKS_GEN_PASSWORD	0x424
+#define H_PKS_GET_OBJECT_LABELS 0x428
+#define H_PKS_WRITE_OBJECT	0x42C
+#define H_PKS_GEN_KEY		0x430
+#define H_PKS_READ_OBJECT	0x434
+#define H_PKS_REMOVE_OBJECT	0x438
+#define H_PKS_CONFIRM_OBJECT_FLUSHED	0x43C
 #define H_RPT_INVALIDATE	0x448
 #define H_SCM_FLUSH		0x44C
-#define MAX_HCALL_OPCODE	H_SCM_FLUSH
+#define H_PKS_SB_SIGNED_UPDATE	0x454
+#define MAX_HCALL_OPCODE	H_PKS_SB_SIGNED_UPDATE
 
 /* Scope args for H_SCM_UNBIND_ALL */
 #define H_UNBIND_SCOPE_ALL (0x1)
diff --git a/arch/powerpc/include/asm/pks.h b/arch/powerpc/include/asm/pks.h
new file mode 100644
index 000000000000..ef6f541d75d3
--- /dev/null
+++ b/arch/powerpc/include/asm/pks.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain
+ *
+ * Platform keystore for pseries.
+ */
+#ifndef _PSERIES_PKS_H
+#define _PSERIES_PKS_H
+
+
+#include <linux/types.h>
+#include <linux/list.h>
+
+struct pks_var {
+	char *prefix;
+	u8 *name;
+	u16 namelen;
+	u32 policy;
+	u16 datalen;
+	u8 *data;
+};
+
+struct pks_var_name {
+	u16 namelen;
+	u8  *name;
+};
+
+struct pks_var_name_list {
+	u32 varcount;
+	struct pks_var_name *varlist;
+};
+
+struct pks_config {
+	u8 version;
+	u8 flags;
+	u32 rsvd0;
+	u16 maxpwsize;
+	u16 maxobjlabelsize;
+	u16 maxobjsize;
+	u32 totalsize;
+	u32 usedspace;
+	u32 supportedpolicies;
+	u64 rsvd1;
+} __packed;
+
+/**
+ * Successful return from this API  implies PKS is available.
+ * This is used to initialize kernel driver and user interfaces.
+ */
+extern struct pks_config *pks_get_config(void);
+
+/**
+ * Returns all the var names for this prefix.
+ * This only returns name list. If the caller needs data, it has to specifically
+ * call read for the required var name.
+ */
+int pks_get_var_ids_for_type(char *prefix, struct pks_var_name_list *list);
+
+/**
+ * Writes the specified var and its data to PKS.
+ * Any caller of PKS driver should present a valid prefix type for their
+ * variable. This is an exception only for signed variables exposed via
+ * sysfs which do not have any prefixes.
+ * The prefix should always start with '/'. For eg. '/sysfs'.
+ */
+extern int pks_write_var(struct pks_var var);
+
+/**
+ * Writes the specified signed var and its data to PKS.
+ */
+extern int pks_update_signed_var(struct pks_var var);
+
+/**
+ * Removes the specified var and its data from PKS.
+ */
+extern int pks_remove_var(char *prefix, struct pks_var_name vname);
+
+/**
+ * Returns the data for the specified variable.
+ */
+extern int pks_read_var(struct pks_var *var);
+
+#endif
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 2e57391e0778..32d0df84e611 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -147,6 +147,16 @@ config IBMEBUS
 	help
 	  Bus device driver for GX bus based adapters.
 
+config PSERIES_PKS
+	depends on PPC_PSERIES
+	tristate "Support for the Platform Key Storage"
+	help
+	  PowerVM provides an isolated Platform Keystore(PKS) storage
+	  allocation for each partition with individually managed
+	  access controls to store sensitive information securely. Select
+	  this config to enable operating system interface to hypervisor to
+	  access this space.
+
 config PAPR_SCM
 	depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
 	tristate "Support for the PAPR Storage Class Memory interface"
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 41d8aee98da4..83eb665a742f 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PPC_VAS)		+= vas.o
 
 obj-$(CONFIG_ARCH_HAS_CC_PLATFORM)	+= cc_platform.o
+obj-$(CONFIG_PSERIES_PKS)      += pks.o
diff --git a/arch/powerpc/platforms/pseries/pks.c b/arch/powerpc/platforms/pseries/pks.c
new file mode 100644
index 000000000000..df9334a4fb89
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/pks.c
@@ -0,0 +1,494 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * POWER platform keystore
+ * Copyright (C) 2010 IBM Corporation
+ *
+ * This pseries platform device driver provides access to
+ * variables stored in platform keystore.
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <asm/hvcall.h>
+#include <asm/firmware.h>
+#include <linux/slab.h>
+#include <asm/pks.h>
+#include <asm/unaligned.h>
+#include <asm/machdep.h>
+#include <linux/string.h>
+
+#define MODULE_VERS "1.0"
+#define MODULE_NAME "pseries-pks"
+
+static bool configset;
+static struct pks_config *config;
+
+struct pks_var_name_one {
+	struct pks_var_name var;
+	struct list_head link;
+};
+
+LIST_HEAD(pks_var_name_list);
+
+static u64 labelcount;
+
+struct pks_auth {
+	u8 version;
+	u8 consumer;
+	__be64 rsvd0;
+	__be32 rsvd1;
+	__be16 passwordlength;
+	u8 password[32];
+} __attribute__ ((packed, aligned(16)));
+
+static struct pks_auth auth;
+
+static int pseries_status_to_err(int rc)
+{
+	int err;
+
+	switch (rc) {
+	case H_SUCCESS:
+		err = 0;
+		break;
+	case H_FUNCTION:
+		err = -ENXIO;
+		break;
+	case H_P2:
+	case H_P3:
+	case H_P4:
+	case H_P5:
+	case H_P6:
+		err = -EINVAL;
+		break;
+	case H_NOT_FOUND:
+		err = -ENOENT;
+		break;
+	case H_BUSY:
+		err = -EBUSY;
+		break;
+	case H_AUTHORITY:
+		err = -EPERM;
+		break;
+	case H_NO_MEM:
+		err = -ENOMEM;
+		break;
+	case H_RESOURCE:
+		err = -EEXIST;
+		break;
+	case H_TOO_BIG:
+		err = -EFBIG;
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static int pks_gen_password(u8 *password[])
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	u8 consumer = 0x3;
+	int rc;
+
+	rc = plpar_hcall(H_PKS_GEN_PASSWORD,
+			retbuf,
+			consumer,
+			0,
+			virt_to_phys(*password),
+			config->maxpwsize);
+
+	return pseries_status_to_err(rc);
+}
+
+static int construct_auth(void)
+{
+	int rc = 0;
+	u8 *password;
+
+	auth.version = 1;
+	auth.consumer = 0x3;
+	auth.rsvd0 = 0;
+	auth.rsvd1 = 0;
+	auth.passwordlength = cpu_to_be16(config->maxpwsize);
+	password = kzalloc(config->maxpwsize, GFP_KERNEL);
+	if (!password)
+		return -ENOMEM;
+
+	rc = pks_gen_password(&password);
+	if (rc) {
+		if (rc == H_IN_USE) {
+			rc = 0;
+		} else {
+			pr_err("Failed setting password\n");
+			rc = pseries_status_to_err(rc);
+			goto err;
+		}
+	}
+	memcpy(auth.password, password, config->maxpwsize);
+
+err:
+	kfree(password);
+	return rc;
+}
+
+static bool validate_name(char *name)
+{
+	int i = 0;
+
+	for (i = 0; i < strlen(name); i++) {
+		if (!isalnum(name[i]) && (name[i] != '-')
+				      && (name[i] != '_')) {
+			pr_err("invalid name, should only contain alphanumeric,hyphen(-) or underscore(_)\n");
+			return false;
+		}
+	}
+
+	return true;
+}
+
+static int construct_label(char *prefix, u8 *name, u16 namelen, u8 **label)
+{
+	int varlen;
+
+	if (!label)
+		return -EINVAL;
+
+	if (!prefix) {
+		*label = kzalloc(namelen, GFP_KERNEL);
+		if (!*label)
+			return -ENOMEM;
+		memcpy(*label, name, namelen);
+	} else {
+		varlen = strlen(prefix) + namelen + 1;
+		*label = kzalloc(varlen, GFP_KERNEL);
+		if (!*label)
+			return -ENOMEM;
+
+		memcpy(*label, prefix, strlen(prefix));
+		(*label)[strlen(prefix)] = '/';
+		memcpy(*label + strlen(prefix) + 1, name, namelen);
+	}
+
+	return 0;
+}
+
+static int _pks_get_config(void)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	size_t size = sizeof(struct pks_config);
+
+	config = kzalloc(size, GFP_KERNEL);
+	if (!config)
+		return -ENOMEM;
+
+	rc = plpar_hcall(H_PKS_GET_CONFIG,
+			retbuf,
+			virt_to_phys(config),
+			size);
+
+	if (rc != H_SUCCESS)
+		return pseries_status_to_err(rc);
+
+	config->rsvd0 = be32_to_cpu(config->rsvd0);
+	config->maxpwsize = be16_to_cpu(config->maxpwsize);
+	config->maxobjlabelsize = be16_to_cpu(config->maxobjlabelsize);
+	config->maxobjsize = be16_to_cpu(config->maxobjsize);
+	config->totalsize = be32_to_cpu(config->totalsize);
+	config->usedspace =  be32_to_cpu(config->usedspace);
+	config->supportedpolicies =  be32_to_cpu(config->supportedpolicies);
+	config->rsvd1 = be64_to_cpu(config->rsvd1);
+
+	configset = true;
+
+	return rc;
+}
+
+static int get_objectlabels(void)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc = 0;
+	u16 bufsize = 1024;
+	u8 buf[1024];
+	int i;
+	int index;
+	u16 labelsize = 0;
+	u64 continuetoken = 0;
+	u64 count;
+	struct pks_var_name_one *vname = NULL;
+
+	do {
+		rc = plpar_hcall(H_PKS_GET_OBJECT_LABELS,
+				retbuf,
+				virt_to_phys(&auth),
+				continuetoken,
+				virt_to_phys(buf),
+				bufsize);
+
+		if (rc) {
+			rc = pseries_status_to_err(rc);
+			goto err;
+		}
+
+		count =  retbuf[0];
+		continuetoken = retbuf[1];
+		index = 0;
+		for (i = 0; i < count; i++) {
+			labelsize = be16_to_cpu(*(__be16 *)(&buf[index]));
+			vname = kzalloc(sizeof(struct pks_var_name_one),
+					GFP_KERNEL);
+			vname->var.namelen = labelsize;
+			vname->var.name = kzalloc(labelsize, GFP_KERNEL);
+			if (!vname->var.name) {
+				rc = -ENOMEM;
+				goto err;
+			}
+			index = index + 2;
+			memcpy(vname->var.name, buf + index, labelsize);
+			list_add(&vname->link, &pks_var_name_list);
+			index =  index + labelsize;
+		}
+		labelcount = labelcount + count;
+		pr_info("Total number of variables are %llu\n", labelcount);
+	} while (continuetoken != 0);
+err:
+	return rc;
+}
+
+int pks_get_var_ids_for_type(char *prefix, struct pks_var_name_list *list)
+{
+	int count = 0;
+	int idx = 0;
+	struct pks_var_name_one *vname = NULL;
+	u8 *name;
+	u16 namelen;
+
+	list_for_each_entry(vname, &pks_var_name_list, link) {
+		name = vname->var.name;
+		if (((!prefix) && (name[0] == '/'))
+		   || (prefix && (strncmp(name, prefix, strlen(prefix)))))
+			continue;
+		count++;
+	}
+
+	list->varcount = count;
+	list->varlist = kcalloc(count, sizeof(list->varlist), GFP_KERNEL);
+	if (!list->varlist)
+		return -ENOMEM;
+
+	list_for_each_entry(vname, &pks_var_name_list, link) {
+		name = (char *)vname->var.name;
+		if (((!prefix) && (name[0] == '/'))
+		   || (prefix && (strncmp(name, prefix, strlen(prefix)))))
+			continue;
+
+		if (!prefix)
+			namelen = vname->var.namelen;
+		else {
+			name = name + strlen(prefix) + 1;
+			namelen = strlen(name) + 1;
+		}
+		pr_debug("var is %s of size %d\n", name, namelen);
+
+		list->varlist[idx].namelen = namelen;
+		list->varlist[idx].name = kzalloc(namelen, GFP_KERNEL);
+		memcpy(list->varlist[idx].name, name, namelen);
+		idx++;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(pks_get_var_ids_for_type);
+
+int pks_update_signed_var(struct pks_var var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u8 *label;
+	u16 varlen;
+	u8 *data = var.data;
+
+	if (var.prefix)
+		return -EINVAL;
+
+	if (!validate_name(var.name))
+		return -EINVAL;
+
+	rc = construct_label(var.prefix, var.name, var.namelen, &label);
+	if (rc)
+		return rc;
+
+	pr_info("Label to be written is %s of size %d\n", label, varlen);
+	varlen = strlen(label) + 1;
+	rc = plpar_hcall(H_PKS_SB_SIGNED_UPDATE,
+			retbuf,
+			virt_to_phys(&auth),
+			virt_to_phys(label),
+			varlen,
+			var.policy,
+			virt_to_phys(data),
+			var.datalen);
+
+	kfree(label);
+
+	return pseries_status_to_err(rc);
+}
+EXPORT_SYMBOL(pks_update_signed_var);
+
+int pks_write_var(struct pks_var var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u8 *label;
+	u16 varlen;
+	u8 *data = var.data;
+
+	if ((!var.prefix) || (var.prefix[0] != '/'))
+		return -EINVAL;
+
+	if (!validate_name(var.name))
+		return -EINVAL;
+
+	rc = construct_label(var.prefix, var.name, var.namelen, &label);
+	if (rc)
+		return rc;
+
+	pr_info("Label to be written is %s of size %d\n", label, varlen);
+	varlen = strlen(label) + 1;
+	rc = plpar_hcall(H_PKS_WRITE_OBJECT,
+			retbuf,
+			virt_to_phys(&auth),
+			virt_to_phys(label),
+			varlen,
+			var.policy,
+			virt_to_phys(data),
+			var.datalen);
+
+	kfree(label);
+
+	return pseries_status_to_err(rc);
+}
+EXPORT_SYMBOL(pks_write_var);
+
+int pks_remove_var(char *prefix, struct pks_var_name vname)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u8 *label;
+	u16 varlen;
+
+	rc = construct_label(prefix, vname.name, vname.namelen, &label);
+	if (rc)
+		return rc;
+
+	varlen = strlen(label) + 1;
+	pr_info("Label to be removed is %s of size %d\n", label, varlen);
+	rc = plpar_hcall(H_PKS_REMOVE_OBJECT,
+			retbuf,
+			virt_to_phys(&auth),
+			virt_to_phys(label),
+			varlen);
+
+	kfree(label);
+
+	return pseries_status_to_err(rc);
+}
+EXPORT_SYMBOL(pks_remove_var);
+
+
+int pks_read_var(struct pks_var *var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u16 outlen = config->maxobjsize;
+	u8 *label;
+	u8 *out;
+	u16 varlen;
+
+	rc = construct_label(var->prefix, var->name, var->namelen, &label);
+	if (rc)
+		return rc;
+
+	varlen = strlen(label) + 1;
+	pr_info("Label to be read %s of size %d\n", label, varlen);
+	out = kzalloc(outlen, GFP_KERNEL);
+	if (!out)
+		return -ENOMEM;
+
+	rc = plpar_hcall(H_PKS_READ_OBJECT,
+			retbuf,
+			virt_to_phys(&auth),
+			virt_to_phys(label),
+			varlen,
+			virt_to_phys(out),
+			outlen);
+
+	if (rc != H_SUCCESS) {
+		pr_err("Failed to read %d\n", rc);
+		rc = pseries_status_to_err(rc);
+		goto err;
+	}
+
+	var->datalen = retbuf[0];
+	var->policy = retbuf[1];
+
+	var->data = kzalloc(var->datalen, GFP_KERNEL);
+	if (!var->data) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	memcpy(var->data, out, var->datalen);
+err:
+	kfree(out);
+	kfree(label);
+
+	return rc;
+}
+EXPORT_SYMBOL(pks_read_var);
+
+struct pks_config *pks_get_config(void)
+{
+
+	if (!configset) {
+		if (_pks_get_config())
+			return NULL;
+	}
+
+	return config;
+}
+EXPORT_SYMBOL(pks_get_config);
+
+int __init pseries_pks_init(void)
+{
+	int rc = 0;
+	struct pks_var_name_one *vname = NULL;
+
+	rc = _pks_get_config();
+
+	if (rc) {
+		pr_err("Error initializing pks\n");
+		return rc;
+	}
+
+	rc = construct_auth();
+	if (rc)
+		return rc;
+
+	rc = get_objectlabels();
+	if (rc) {
+		pr_err("Getting object labels failed. Error initializing pks\n");
+		return rc;
+	}
+
+	list_for_each_entry(vname, &pks_var_name_list, link)
+		pr_info("name is %s\n", vname->var.name);
+
+	return rc;
+}
+arch_initcall(pseries_pks_init);
-- 
2.27.0

[RFC PATCH 2/2] pseries: define sysfs interface to expose PKS variables

[Nayna Jain]

PowerVM guest secure boot intend to use Platform Keystore(PKS) for the
purpose of storing public keys to verify digital signature.

Define sysfs interface to expose PKS variables to userspace to allow
read/write/add/delete operations. Each variable is shown as a read/write
attribute file. The size of the file represents the size of the current
content of the variable.

create_var and delete_var attribute files are always present which allow
users to create/delete variables. These are write only attributes.The
design has tried to be compliant with sysfs semantic to represent single
value per attribute. Thus, rather than mapping a complete data structure
representation to create_var, it only accepts a single formatted string
to create an empty variable.

The sysfs interface is designed such as to expose PKS configuration
properties, operating system variables and firmware variables.
Current version exposes configuration and operating system variables.
The support for exposing firmware variables will be added in the future
version.

Example of pksvar sysfs interface:

# cd /sys/firmware/pksvar/
# ls
config  os

# cd config

# ls -ltrh
total 0
-r--r--r-- 1 root root 64K Jan 21 17:55 version
-r--r--r-- 1 root root 64K Jan 21 17:55 used_space
-r--r--r-- 1 root root 64K Jan 21 17:55 total_size
-r--r--r-- 1 root root 64K Jan 21 17:55 supported_policies
-r--r--r-- 1 root root 64K Jan 21 17:55 max_object_size
-r--r--r-- 1 root root 64K Jan 21 17:55 max_object_label_size
-r--r--r-- 1 root root 64K Jan 21 17:55 flags

# cd os

# ls -ltrh
total 0
-rw------- 1 root root 104 Jan 21 17:56 var4
-rw------- 1 root root 104 Jan 21 17:56 var3
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
-rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
--w------- 1 root root 64K Jan 21 17:56 delete_var
--w------- 1 root root 64K Jan 21 17:56 create_var

1. Read variable

# hexdump -C GLOBAL_db
00000000  00 00 00 00 a1 59 c0 a5  e4 94 a7 4a 87 b5 ab 15  |.....Y.....J....|
00000010  5c 2b f0 72 3f 03 00 00  00 00 00 00 23 03 00 00  |\+.r?.......#...|
....
00000330  02 a8 e8 ed 0f 20 60 3f  40 04 7c a8 91 21 37 eb  |..... `?@.|..!7.|
00000340  f3 f1 4e                                          |..N|
00000343

2. Write variable

cat /tmp/data.bin > <variable_name>

3. Create variable

# echo "var1" > create_var
# ls -ltrh
total 0
-rw------- 1 root root 104 Jan 21 17:56 var4
-rw------- 1 root root 104 Jan 21 17:56 var3
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
-rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
--w------- 1 root root 64K Jan 21 17:56 delete_var
--w------- 1 root root 64K Jan 21 17:57 create_var
-rw------- 1 root root   0 Jan 21 17:57 var1.tmp

Current design creates a zero size temporary variable. This implies
it is not yet persisted to PKS. Only once data is written to newly
created temporary variable and if it is successfully stored in the
PKS, that the variable is permanent. The temporary variable will get
removed on reboot. The code currently doesn't remove .tmp suffix
immediately when persisted. The future version will fix this.

To avoid the additional .tmp semantic, alternative option is to consider
any zero size variable as temporary variable. This option is under
evaluation. This would avoid any runtime sysfs magic to replace .tmp
variable with real variable.

Also, the formatted string to pass to create_var will have following
format in the future version:
<variable_name>:<comma-separated-policy strings>

4. Delete variable
# echo "var3" > delete_var
# ls -ltrh
total 0
-rw------- 1 root root 104 Jan 21 17:56 var4
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
-rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
-rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
--w------- 1 root root 64K Jan 21 17:57 create_var
-rw------- 1 root root   0 Jan 21 17:57 var1.tmp
--w------- 1 root root 64K Jan 21 17:58 delete_var

The var3 file is removed at runtime, if variable is successfully
removed from the PKS storage.

NOTE: We are evaluating two design for userspace interface: using the
sysfs or defining a new filesystem based. Any feedback on this sysfs based
approach would be highly appreciated. We have tried to follow one value
per attribute semantic. If that or any other semantics aren't followed
properly, please let us know.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-pksvar        |  77 ++++
 arch/powerpc/platforms/pseries/Kconfig        |   7 +
 arch/powerpc/platforms/pseries/Makefile       |   1 +
 arch/powerpc/platforms/pseries/pksvar-sysfs.c | 356 ++++++++++++++++++
 4 files changed, 441 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-pksvar
 create mode 100644 arch/powerpc/platforms/pseries/pksvar-sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-pksvar b/Documentation/ABI/testing/sysfs-pksvar
new file mode 100644
index 000000000000..fe5327f5bd70
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-pksvar
@@ -0,0 +1,77 @@
+What:		/sys/firmware/pksvar
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This directory is created if the POWER firmware supports
+		Platform Keystore (PKS). It exposes interface for reading/writing
+		variables which are persisted in PKS.
+
+What:		/sys/firmware/pksvar/config
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This directory lists the configuration properties of Platform
+		Keystore.
+
+What:		/sys/firmware/pksvar/os
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This directory lists all the variables owned by operating system
+		users and stored in Platform Keystore.
+
+What:		/sys/firmware/pksvar/config/version
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	A value indicating version of Firmware Platform Keystore.
+
+What:		/sys/firmware/pksvar/config/used_space
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	The value specifying amount of space used in Platform Keystore.
+
+What:		/sys/firmware/pksvar/config/total_size
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	The value specifying total size of Platform Keystore.
+
+What:		/sys/firmware/pksvar/config/supported_policies
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	The value specifying policies supported by Platform Keystore semantics.
+
+What:		/sys/firmware/pksvar/config/max_object_size
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	The value specifies the maximum object size that can be supported.
+
+What:		/sys/firmware/pksvar/config/max_object_label_size
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:    The value specifies the maximum object label size that is supported.
+
+What:		/sys/firmware/pksvar/config/flags
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:    The value specifies the flags supported by Platform Keystore.
+
+What:		/sys/firmware/pksvar/os/<attribute_file>
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:    A read-write file indicating the variable stored in the Platform Keystore.
+		The size of the file represents the size of the actual data stored for
+		this variable in PKS.
+
+What:		/sys/firmware/pksvar/os/create_var
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:    A write only file used to create new variables. The user should write
+		formatted string containing name to this file. It creates at runtime
+		zero size read-write <variable-name>.tmp temporary attribute file. The
+		temporary variable name is not persisted to Platform Keystore until
+		data is updated to it.
+
+What:		/sys/firmware/pksvar/os/delete_var
+Date:		August 2022
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:    A write only file used to delete existing variable. The user should
+		write variable name to this file. If the variable is successfully
+		deleted from Platform Keystore, the corresponding attribute file is
+		also removed at runtime.
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 32d0df84e611..9310876d201d 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -157,6 +157,13 @@ config PSERIES_PKS
 	  this config to enable operating system interface to hypervisor to
 	  access this space.
 
+config PSERIES_PKS_SYSFS
+	depends on PSERIES_PKS
+	tristate "Support for sysfs interface the Platform Key Storage"
+	help
+	  Enable sysfs based user interace to add/delete/modify variables
+	  stored in Platform Keystore.
+
 config PAPR_SCM
 	depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
 	tristate "Support for the PAPR Storage Class Memory interface"
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 83eb665a742f..27f9aafbd08b 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_PPC_VAS)		+= vas.o
 
 obj-$(CONFIG_ARCH_HAS_CC_PLATFORM)	+= cc_platform.o
 obj-$(CONFIG_PSERIES_PKS)      += pks.o
+obj-$(CONFIG_PSERIES_PKS_SYSFS) += pksvar-sysfs.o
diff --git a/arch/powerpc/platforms/pseries/pksvar-sysfs.c b/arch/powerpc/platforms/pseries/pksvar-sysfs.c
new file mode 100644
index 000000000000..2e53daaf6e9f
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/pksvar-sysfs.c
@@ -0,0 +1,356 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation <nayna@linux.ibm.com>
+ *
+ * This code exposes variables stored in Platform Keystore via sysfs
+ */
+
+#define pr_fmt(fmt) "pksvar-sysfs: " fmt
+
+#include <linux/slab.h>
+#include <linux/compat.h>
+#include <linux/string.h>
+#include <linux/of.h>
+#include <asm/pks.h>
+
+static struct kobject *pks_kobj;
+static struct kobject *prop_kobj;
+static struct kobject *os_kobj;
+
+static struct pks_config *config;
+
+struct osvar_sysfs_attr {
+	struct bin_attribute bin_attr;
+	struct list_head node;
+};
+
+static LIST_HEAD(osvar_sysfs_list);
+
+
+static ssize_t osvar_sysfs_read(struct file *file, struct kobject *kobj,
+				struct bin_attribute *bin_attr, char *buf,
+				loff_t off, size_t count)
+{
+	struct pks_var var;
+	char *out;
+	u32 outlen;
+	int rc;
+
+	var.name = (char *)bin_attr->attr.name;
+	var.namelen = strlen(var.name) + 1;
+	var.prefix = NULL;
+	rc = pks_read_var(&var);
+	if (rc) {
+		pr_err("Error reading object %d\n", rc);
+		return rc;
+	}
+
+	outlen = sizeof(var.policy) + var.datalen;
+	out = kzalloc(outlen, GFP_KERNEL);
+	memcpy(out, &var.policy, sizeof(var.policy));
+	memcpy(out + sizeof(var.policy), var.data, var.datalen);
+
+	count = outlen;
+	memcpy(buf, out, outlen);
+
+	kfree(out);
+	return count;
+}
+
+static ssize_t osvar_sysfs_write(struct file *file, struct kobject *kobj,
+				 struct bin_attribute *bin_attr, char *buf,
+				 loff_t off, size_t count)
+{
+	struct pks_var *var = NULL;
+	int rc = 0;
+	char *p;
+	char *name = (char *)bin_attr->attr.name;
+	struct osvar_sysfs_attr *osvar_sysfs = NULL;
+
+	list_for_each_entry(osvar_sysfs, &osvar_sysfs_list, node) {
+		if (strncmp(name, osvar_sysfs->bin_attr.attr.name,
+			    strlen(name)) == 0) {
+			var = osvar_sysfs->bin_attr.private;
+			break;
+		}
+	}
+
+	p = strsep(&name, ".");
+
+	var->datalen = count;
+	var->data = kzalloc(count, GFP_KERNEL);
+	if (!var->data)
+		return -ENOMEM;
+
+	memcpy(var->data, buf, count);
+	var->name = p;
+	var->namelen = strlen(p) + 1;
+
+	pr_info("var %s of length %d to be written\n", var->name, var->namelen);
+	var->prefix = NULL;
+	rc = pks_update_signed_var(*var);
+
+	if (rc) {
+		pr_err(" write failed with rc is %d\n", rc);
+		var->datalen = 0;
+		count = rc;
+		goto err;
+	}
+
+err:
+	kfree(var->data);
+	return count;
+}
+
+
+
+static ssize_t version_show(struct kobject *kobj, struct kobj_attribute *attr,
+			    char *buf)
+{
+	return sprintf(buf, "%d\n", config->version);
+}
+
+static ssize_t flags_show(struct kobject *kobj, struct kobj_attribute *attr,
+			  char *buf)
+{
+	return sprintf(buf, "%02x\n", config->flags);
+}
+
+static ssize_t max_object_label_size_show(struct kobject *kobj,
+					  struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", config->maxobjlabelsize);
+}
+
+static ssize_t max_object_size_show(struct kobject *kobj,
+				    struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", config->maxobjsize);
+}
+
+static ssize_t total_size_show(struct kobject *kobj,
+			       struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", config->totalsize);
+}
+
+static ssize_t used_space_show(struct kobject *kobj,
+			       struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", config->usedspace);
+}
+
+static ssize_t supported_policies_show(struct kobject *kobj,
+				       struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", config->supportedpolicies);
+}
+
+static ssize_t create_var_store(struct kobject *kobj,
+				struct kobj_attribute *attr,
+				const char *buf, size_t count)
+{
+	int rc;
+	struct pks_var *var;
+	char *suffix = ".tmp";
+	char *name;
+	u16 namelen = 0;
+	struct osvar_sysfs_attr *osvar_sysfs = NULL;
+
+	namelen = count + strlen(suffix);
+	name = kzalloc(namelen, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+	memcpy(name, buf, count-1);
+	memcpy(name + (count-1), suffix, strlen(suffix));
+	name[namelen] = '\0';
+
+	pr_debug("var %s of length %d to be added\n", name, namelen);
+
+	osvar_sysfs = kzalloc(sizeof(struct osvar_sysfs_attr), GFP_KERNEL);
+	if (!osvar_sysfs) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	var = kzalloc(sizeof(struct pks_var), GFP_KERNEL);
+
+	if (!var) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	var->name = name;
+	var->namelen = namelen;
+	var->prefix = NULL;
+	var->policy = 0;
+
+	sysfs_bin_attr_init(&osvar_sysfs->bin_attr);
+	osvar_sysfs->bin_attr.private = var;
+	osvar_sysfs->bin_attr.attr.name = name;
+	osvar_sysfs->bin_attr.attr.mode = 0600;
+	osvar_sysfs->bin_attr.size = 0;
+	osvar_sysfs->bin_attr.read = osvar_sysfs_read;
+	osvar_sysfs->bin_attr.write = osvar_sysfs_write;
+
+	rc = sysfs_create_bin_file(os_kobj,
+			&osvar_sysfs->bin_attr);
+	if (rc)
+		goto err;
+
+	list_add_tail(&osvar_sysfs->node, &osvar_sysfs_list);
+	rc = count;
+err:
+	return rc;
+}
+
+static ssize_t delete_var_store(struct kobject *kobj,
+				struct kobj_attribute *attr,
+				const char *buf, size_t count)
+{
+	int rc;
+	struct pks_var_name vname;
+	struct osvar_sysfs_attr *osvar_sysfs = NULL;
+
+	vname.name = kzalloc(count, GFP_KERNEL);
+	if (!vname.name)
+		return -ENOMEM;
+
+	memcpy(vname.name, buf, count-1);
+	vname.name[count] = '\0';
+	vname.namelen = count;
+
+	pr_debug("var %s of length %lu to be deleted\n", buf, count);
+
+	rc = pks_remove_var(NULL, vname);
+
+	if (!rc) {
+		list_for_each_entry(osvar_sysfs, &osvar_sysfs_list, node) {
+			if (strncmp(vname.name, osvar_sysfs->bin_attr.attr.name,
+				    strlen(vname.name)) == 0) {
+				list_del(&osvar_sysfs->node);
+				sysfs_remove_bin_file(os_kobj, &osvar_sysfs->bin_attr);
+				break;
+			}
+		}
+		rc = count;
+	}
+
+	return rc;
+}
+
+static struct kobj_attribute version_attr = __ATTR_RO(version);
+static struct kobj_attribute flags_attr = __ATTR_RO(flags);
+static struct kobj_attribute max_object_label_size_attr = __ATTR_RO(max_object_label_size);
+static struct kobj_attribute max_object_size_attr = __ATTR_RO(max_object_size);
+static struct kobj_attribute total_size_attr = __ATTR_RO(total_size);
+static struct kobj_attribute used_space_attr = __ATTR_RO(used_space);
+static struct kobj_attribute supported_policies_attr = __ATTR_RO(supported_policies);
+static struct kobj_attribute create_var_attr = __ATTR_WO(create_var);
+static struct kobj_attribute delete_var_attr = __ATTR_WO(delete_var);
+
+static int __init pks_sysfs_prop_load(void)
+{
+	int rc;
+
+	config = pks_get_config();
+	if (!config)
+		return -ENODEV;
+
+	rc = sysfs_create_file(prop_kobj, &version_attr.attr);
+	rc = sysfs_create_file(prop_kobj, &flags_attr.attr);
+	rc = sysfs_create_file(prop_kobj, &max_object_label_size_attr.attr);
+	rc = sysfs_create_file(prop_kobj, &max_object_size_attr.attr);
+	rc = sysfs_create_file(prop_kobj, &total_size_attr.attr);
+	rc = sysfs_create_file(prop_kobj, &used_space_attr.attr);
+	rc = sysfs_create_file(prop_kobj, &supported_policies_attr.attr);
+
+	return 0;
+}
+
+static int __init pks_sysfs_os_load(void)
+{
+	struct osvar_sysfs_attr *osvar_sysfs = NULL;
+	struct pks_var_name_list namelist;
+	struct pks_var *var;
+	int rc;
+	int i;
+
+	rc = sysfs_create_file(os_kobj, &create_var_attr.attr);
+	rc = sysfs_create_file(os_kobj, &delete_var_attr.attr);
+	rc = pks_get_var_ids_for_type(NULL, &namelist);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < namelist.varcount; i++) {
+		var = kzalloc(sizeof(struct pks_var), GFP_KERNEL);
+		var->name = namelist.varlist[i].name;
+		var->namelen = namelist.varlist[i].namelen;
+		var->prefix = NULL;
+		rc = pks_read_var(var);
+		if (rc) {
+			pr_err("Error %d reading object %s\n", rc, var->name);
+			continue;
+		}
+
+		osvar_sysfs = kzalloc(sizeof(struct osvar_sysfs_attr), GFP_KERNEL);
+		if (!osvar_sysfs) {
+			rc = -ENOMEM;
+			break;
+		}
+
+		sysfs_bin_attr_init(&osvar_sysfs->bin_attr);
+		osvar_sysfs->bin_attr.private = var;
+		osvar_sysfs->bin_attr.attr.name = namelist.varlist[i].name;
+		osvar_sysfs->bin_attr.attr.mode = 0600;
+		osvar_sysfs->bin_attr.size = var->datalen;
+		osvar_sysfs->bin_attr.read = osvar_sysfs_read;
+		osvar_sysfs->bin_attr.write = osvar_sysfs_write;
+
+		rc = sysfs_create_bin_file(os_kobj,
+				&osvar_sysfs->bin_attr);
+
+		if (rc)
+			continue;
+
+		list_add_tail(&osvar_sysfs->node, &osvar_sysfs_list);
+	}
+
+	return rc;
+}
+
+static int pks_sysfs_init(void)
+{
+	int rc;
+
+	pks_kobj = kobject_create_and_add("pksvar", firmware_kobj);
+	if (!pks_kobj) {
+		pr_err("pksvar: Failed to create pks kobj\n");
+		return -ENOMEM;
+	}
+
+	prop_kobj = kobject_create_and_add("config", pks_kobj);
+	if (!prop_kobj) {
+		pr_err("secvar: config kobject registration failed.\n");
+		kobject_put(pks_kobj);
+		return -ENOMEM;
+	}
+
+	rc = pks_sysfs_prop_load();
+	if (rc)
+		return rc;
+
+	os_kobj = kobject_create_and_add("os", pks_kobj);
+	if (!os_kobj) {
+		pr_err("pksvar: os kobject registration failed.\n");
+		kobject_put(os_kobj);
+		return -ENOMEM;
+	}
+
+	rc = pks_sysfs_os_load();
+	if (rc)
+		return rc;
+
+	return 0;
+}
+late_initcall(pks_sysfs_init);
-- 
2.27.0

Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

[Greg KH]

On Fri, Jan 21, 2022 at 07:56:35PM -0500, Nayna Jain wrote:
> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
> for each partition with individually managed access controls to store
> sensitive information securely. Linux Kernel can access this storage by
> interfacing with hypervisor using a new set of hypervisor calls. 
> 
> PowerVM guest secure boot intend to use Platform Keystore for the
> purpose of storing public keys. Secure boot requires public keys to
> be able to verify the grub and boot kernel. To allow authenticated
>  manipulation of keys, it supports variables to store key authorities
> - PK/KEK and code signing keys - db. It also supports denied list to
> disallow booting even if signed with valid key. This is done via
> denied list database - dbx or sbat. These variables would be stored in
> PKS, and are managed and controlled by firmware.
> 
> The purpose of this patchset is to add support for users to
> read/write/add/delete variables required for secure boot on PowerVM.

Ok, this is like the 3rd or 4th different platform-specific proposal for
this type of functionality.  I think we need to give up on
platform-specific user/kernel apis on this (random sysfs/securityfs
files scattered around the tree), and come up with a standard place for
all of this.

Please work with the other developers of the other drivers for this to
make this unified so that userspace has a chance to use this in a sane
manner.

thanks,

greg k-h

Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

[Daniel Axtens]

Hi Greg,

> Ok, this is like the 3rd or 4th different platform-specific proposal for
> this type of functionality.  I think we need to give up on
> platform-specific user/kernel apis on this (random sysfs/securityfs
> files scattered around the tree), and come up with a standard place for
> all of this.

I agree that we do have a number of platforms exposing superficially
similar functionality. Indeed, back in 2019 I had a crack at a unified
approach: [1] [2].

Looking back at it now, I am not sure it ever would have worked because
the semantics of the underlying firmware stores are quite
different. Here are the ones I know about:

 - OpenPower/PowerNV Secure Variables:
 
   * Firmware semantics:
     - flat variable space
     - variables are fixed in firmware, can neither be created nor
        destroyed
     - variable names are ASCII
     - no concept of policy/attributes
     
   * Current kernel interface semantics:
     - names are case sensitive
     - directory per variable     


 - (U)EFI variables:
 
   * Firmware semantics:
     - flat variable space
     - variables can be created/destroyed but the semantics are fiddly
        [3]
     - variable names are UTF-16 + UUID
     - variables have 32-bit attributes

   * efivarfs interface semantics:
     - file per variable
     - attributes are the first 4 bytes of the file
     - names are partially case-insensitive (UUID part) and partially
        case-sensitive ('name' part)

   * sysfs interface semantics (as used by CONFIG_GOOGLE_SMI)
     - directory per variable
     - attributes are a separate sysfs file
     - to create a variable you write a serialised structure to
        `/sys/firmware/efi/vars/new_var`, to delete a var you write
        to `.../del_var`
     - names are case-sensitive including the UUID


 - PowerVM Partition Key Store Variables:
 
   * Firmware semantics:
     - _not_ a flat space, there are 3 domains ("consumers"): firmware,
        bootloader and OS (not yet supported by the patch set)
     - variables can be created and destroyed but the semantics are
        fiddly and fiddly in different ways to UEFI [4]
     - variable names are arbitrary byte strings: the hypervisor permits
        names to contain nul and /.
     - variables have 32-bit attributes ("policy") that don't align with
        UEFI attributes

   * No stable kernel interface yet

Even if we could come up with some stable kernel interface features
(e.g. decide if we want file per variable vs directory per variable), I
don't know how easy it would be to deal with the underlying semantic
differences - I think userspace would still need substantial
per-platform knowledge.

Or have I misunderstood what you're asking for? (If you want them all to
live under /sys/firmware, these ones all already do...)

Kind regards,
Daniel


[1]: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-May/190735.html
[2]: discussion continues at https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-June/191365.html
[3]: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-June/191496.html

[4]: An unsigned variable cannot be updated, it can only be deleted
     (unless it was created with the immutable policy) and then
     re-created. A signed variable, on the other hand, can be updated
     and the only way to delete it is to submit a validly signed empty
     update.

Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

[Greg KH]

On Mon, Jan 24, 2022 at 11:25:17AM +1100, Daniel Axtens wrote:
> Hi Greg,
> 
> > Ok, this is like the 3rd or 4th different platform-specific proposal for
> > this type of functionality.  I think we need to give up on
> > platform-specific user/kernel apis on this (random sysfs/securityfs
> > files scattered around the tree), and come up with a standard place for
> > all of this.
> 
> I agree that we do have a number of platforms exposing superficially
> similar functionality. Indeed, back in 2019 I had a crack at a unified
> approach: [1] [2].
> 
> Looking back at it now, I am not sure it ever would have worked because
> the semantics of the underlying firmware stores are quite
> different. Here are the ones I know about:
> 
>  - OpenPower/PowerNV Secure Variables:
>  
>    * Firmware semantics:
>      - flat variable space
>      - variables are fixed in firmware, can neither be created nor
>         destroyed
>      - variable names are ASCII
>      - no concept of policy/attributes
>      
>    * Current kernel interface semantics:
>      - names are case sensitive
>      - directory per variable     
> 
> 
>  - (U)EFI variables:
>  
>    * Firmware semantics:
>      - flat variable space
>      - variables can be created/destroyed but the semantics are fiddly
>         [3]
>      - variable names are UTF-16 + UUID
>      - variables have 32-bit attributes
> 
>    * efivarfs interface semantics:
>      - file per variable
>      - attributes are the first 4 bytes of the file
>      - names are partially case-insensitive (UUID part) and partially
>         case-sensitive ('name' part)
> 
>    * sysfs interface semantics (as used by CONFIG_GOOGLE_SMI)
>      - directory per variable
>      - attributes are a separate sysfs file
>      - to create a variable you write a serialised structure to
>         `/sys/firmware/efi/vars/new_var`, to delete a var you write
>         to `.../del_var`
>      - names are case-sensitive including the UUID
> 
> 
>  - PowerVM Partition Key Store Variables:
>  
>    * Firmware semantics:
>      - _not_ a flat space, there are 3 domains ("consumers"): firmware,
>         bootloader and OS (not yet supported by the patch set)
>      - variables can be created and destroyed but the semantics are
>         fiddly and fiddly in different ways to UEFI [4]
>      - variable names are arbitrary byte strings: the hypervisor permits
>         names to contain nul and /.
>      - variables have 32-bit attributes ("policy") that don't align with
>         UEFI attributes
> 
>    * No stable kernel interface yet
> 
> Even if we could come up with some stable kernel interface features
> (e.g. decide if we want file per variable vs directory per variable), I
> don't know how easy it would be to deal with the underlying semantic
> differences - I think userspace would still need substantial
> per-platform knowledge.
> 
> Or have I misunderstood what you're asking for? (If you want them all to
> live under /sys/firmware, these ones all already do...)

I want them to be unified in some way, right now there are lots of
proposals for the same type of thing, but in different places (sysfs,
securityfs, somewhere else), and with different names.

Please work together.

thanks,

greg k-h

Re: [RFC PATCH 0/2] powerpc/pseries: add support for local secure storage called Platform Keystore(PKS)

[Dave Hansen]

On 1/21/22 16:56, Nayna Jain wrote:
> Nayna Jain (2):
>   pseries: define driver for Platform Keystore
>   pseries: define sysfs interface to expose PKS variables

Hi Folks,

There another feature that we might want to consider in the naming here:

> https://lore.kernel.org/all/20220127175505.851391-1-ira.weiny@intel.com/

Protection Keys for Supervisor pages is also called PKS.  It's also not
entirely impossible that powerpc might want to start using this code at
some point, just like what happened with the userspace protection keys[1].

I don't think it's the end of the world either way, but it might save a
hapless user or kernel developer some confusion if we can avoid
including two "PKS" features in the kernel.  I just wanted to make sure
we were aware of the other's existence. :)

1.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/include/asm/pkeys.h

Re: [RFC PATCH 2/2] pseries: define sysfs interface to expose PKS variables

[Dov Murik]

Hi Nayna,


On 22/01/2022 2:56, Nayna Jain wrote:
> PowerVM guest secure boot intend to use Platform Keystore(PKS) for the
> purpose of storing public keys to verify digital signature.
> 
> Define sysfs interface to expose PKS variables to userspace to allow
> read/write/add/delete operations. Each variable is shown as a read/write
> attribute file. The size of the file represents the size of the current
> content of the variable.
> 
> create_var and delete_var attribute files are always present which allow
> users to create/delete variables. These are write only attributes.The
> design has tried to be compliant with sysfs semantic to represent single
> value per attribute. Thus, rather than mapping a complete data structure
> representation to create_var, it only accepts a single formatted string
> to create an empty variable.
> 
> The sysfs interface is designed such as to expose PKS configuration
> properties, operating system variables and firmware variables.
> Current version exposes configuration and operating system variables.
> The support for exposing firmware variables will be added in the future
> version.
> 
> Example of pksvar sysfs interface:
> 
> # cd /sys/firmware/pksvar/
> # ls
> config  os
> 
> # cd config
> 
> # ls -ltrh
> total 0
> -r--r--r-- 1 root root 64K Jan 21 17:55 version
> -r--r--r-- 1 root root 64K Jan 21 17:55 used_space
> -r--r--r-- 1 root root 64K Jan 21 17:55 total_size
> -r--r--r-- 1 root root 64K Jan 21 17:55 supported_policies
> -r--r--r-- 1 root root 64K Jan 21 17:55 max_object_size
> -r--r--r-- 1 root root 64K Jan 21 17:55 max_object_label_size
> -r--r--r-- 1 root root 64K Jan 21 17:55 flags
> 
> # cd os
> 
> # ls -ltrh
> total 0
> -rw------- 1 root root 104 Jan 21 17:56 var4
> -rw------- 1 root root 104 Jan 21 17:56 var3
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
> -rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
> --w------- 1 root root 64K Jan 21 17:56 delete_var
> --w------- 1 root root 64K Jan 21 17:56 create_var
> 
> 1. Read variable
> 
> # hexdump -C GLOBAL_db
> 00000000  00 00 00 00 a1 59 c0 a5  e4 94 a7 4a 87 b5 ab 15  |.....Y.....J....|
> 00000010  5c 2b f0 72 3f 03 00 00  00 00 00 00 23 03 00 00  |\+.r?.......#...|
> ....
> 00000330  02 a8 e8 ed 0f 20 60 3f  40 04 7c a8 91 21 37 eb  |..... `?@.|..!7.|
> 00000340  f3 f1 4e                                          |..N|
> 00000343
> 
> 2. Write variable
> 
> cat /tmp/data.bin > <variable_name>
> 
> 3. Create variable
> 
> # echo "var1" > create_var

It would be easier to understand if the user could create a new variable
like a regular new file, something like:

# cat /tmp/data.bin > var1

but I understand there are also comma-seperated-policy-strings which
should go somewhere; not sure how this fits (or if there are other
examples for similar interfaces in other sysfs parts).



> # ls -ltrh
> total 0
> -rw------- 1 root root 104 Jan 21 17:56 var4
> -rw------- 1 root root 104 Jan 21 17:56 var3
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
> -rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
> --w------- 1 root root 64K Jan 21 17:56 delete_var
> --w------- 1 root root 64K Jan 21 17:57 create_var
> -rw------- 1 root root   0 Jan 21 17:57 var1.tmp
> 
> Current design creates a zero size temporary variable. This implies
> it is not yet persisted to PKS. Only once data is written to newly
> created temporary variable and if it is successfully stored in the
> PKS, that the variable is permanent. The temporary variable will get
> removed on reboot. The code currently doesn't remove .tmp suffix
> immediately when persisted. The future version will fix this.
> 
> To avoid the additional .tmp semantic, alternative option is to consider
> any zero size variable as temporary variable. This option is under
> evaluation. This would avoid any runtime sysfs magic to replace .tmp
> variable with real variable.
> 
> Also, the formatted string to pass to create_var will have following
> format in the future version:
> <variable_name>:<comma-separated-policy strings>
> 
> 4. Delete variable
> # echo "var3" > delete_var

If it's possible here, I think it would be easier to understand (and
use) if you implement unlink(), so deleting var3 would be:

# rm var3

(and then there's no need for the special 'delete_var' entry.)


-Dov

> # ls -ltrh
> total 0
> -rw------- 1 root root 104 Jan 21 17:56 var4
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
> -rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
> --w------- 1 root root 64K Jan 21 17:57 create_var
> -rw------- 1 root root   0 Jan 21 17:57 var1.tmp
> --w------- 1 root root 64K Jan 21 17:58 delete_var
> 
> The var3 file is removed at runtime, if variable is successfully
> removed from the PKS storage.
> 
> NOTE: We are evaluating two design for userspace interface: using the
> sysfs or defining a new filesystem based. Any feedback on this sysfs based
> approach would be highly appreciated. We have tried to follow one value
> per attribute semantic. If that or any other semantics aren't followed
> properly, please let us know.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  Documentation/ABI/testing/sysfs-pksvar        |  77 ++++
>  arch/powerpc/platforms/pseries/Kconfig        |   7 +
>  arch/powerpc/platforms/pseries/Makefile       |   1 +
>  arch/powerpc/platforms/pseries/pksvar-sysfs.c | 356 ++++++++++++++++++
>  4 files changed, 441 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-pksvar
>  create mode 100644 arch/powerpc/platforms/pseries/pksvar-sysfs.c
>