[PATCH 0/2] crypto: caam - add tagged keys functionality and tk transformations for skcipher

[PATCH 0/2] crypto: caam - add tagged keys functionality and tk transformations for skcipher

[Iuliana Prodan]

Tagged keys are keys that contain metadata indicating what
they are and how to handle them using the new added tag_object API.
A tag object represents the metadata (or simply a header/configuration)
and the actual data (e.g. black key) obtained from hardware.
Patch #2 adds support, for tagged keys, to skcipher algorithms by
adding new transformations, with tk prefix to distinguish
between plaintext and tagged keys.

The tk_ transformations can be used directly by their name:
struct sockaddr_alg sa = {
.salg_family = AF_ALG,
.salg_type = "skcipher", /* this selects the symmetric cipher /
.salg_name = "tk(cbc(aes))" / this is the cipher name */
};
or for dm-crypt, e.g. using dmsetup:
dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
sector_size:512".

tk_ transformations will know how to handle tagged keys, by loading the
proper settings for KEY command.

Iuliana Prodan (2):
  crypto: caam - add tag object functionality
  crypto: caam - support tagged keys for skcipher algorithms

 drivers/crypto/caam/Kconfig        |   9 +++
 drivers/crypto/caam/Makefile       |   1 +
 drivers/crypto/caam/caamalg.c      | 107 ++++++++++++++++++++++++++++--
 drivers/crypto/caam/caamalg_desc.c |  28 ++++++--
 drivers/crypto/caam/desc.h         |   4 +-
 drivers/crypto/caam/desc_constr.h  |   4 ++
 drivers/crypto/caam/tag_object.c   | 129 +++++++++++++++++++++++++++++++++++++
 drivers/crypto/caam/tag_object.h   |  99 ++++++++++++++++++++++++++++
 8 files changed, 372 insertions(+), 9 deletions(-)
 create mode 100644 drivers/crypto/caam/tag_object.c
 create mode 100644 drivers/crypto/caam/tag_object.h

-- 
2.1.0

[PATCH 1/2] crypto: caam - add tag object functionality

[Iuliana Prodan]

A tag object represents the metadata (or simply a header/configuration)
and the actual data (e.g. black key) obtained from hardware.
Add functionality to tag an object with metadata:
- validate metadata: check tag object header;
- retrieve metadata: get tag object header configuration, black key
configuration or tag object data.

This API expects that the object (the actual data) from a tag object
to be a buffer (defined by address and size).

Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
---
 drivers/crypto/caam/Kconfig      |   9 +++
 drivers/crypto/caam/Makefile     |   1 +
 drivers/crypto/caam/desc.h       |   4 +-
 drivers/crypto/caam/tag_object.c | 129 +++++++++++++++++++++++++++++++++++++++
 drivers/crypto/caam/tag_object.h |  99 ++++++++++++++++++++++++++++++
 5 files changed, 241 insertions(+), 1 deletion(-)
 create mode 100644 drivers/crypto/caam/tag_object.c
 create mode 100644 drivers/crypto/caam/tag_object.h

diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
index bc35aa0..73368d8 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -149,6 +149,15 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API
 	  Selecting this will register the SEC4 hardware rng to
 	  the hw_random API for supplying the kernel entropy pool.
 
+config CRYPTO_DEV_FSL_CAAM_TK_API
+	bool "Register tagged key cryptography implementations with Crypto API"
+	depends on CRYPTO_DEV_FSL_CAAM_CRYPTO_API
+	help
+	  Selecting this will register algorithms supporting tagged key.
+
+	  Tagged keys are keys that contain metadata indicating what
+	  they are and how to handle them.
+
 endif # CRYPTO_DEV_FSL_CAAM_JR
 
 endif # CRYPTO_DEV_FSL_CAAM
diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
index 68d5cc0..192a88e 100644
--- a/drivers/crypto/caam/Makefile
+++ b/drivers/crypto/caam/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API_DESC) += caamhash_desc.o
 
 caam-y := ctrl.o
 caam_jr-y := jr.o key_gen.o
+caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API) += tag_object.o
 caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API) += caamalg.o
 caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o
 caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
diff --git a/drivers/crypto/caam/desc.h b/drivers/crypto/caam/desc.h
index e134709..3001a8d 100644
--- a/drivers/crypto/caam/desc.h
+++ b/drivers/crypto/caam/desc.h
@@ -152,7 +152,7 @@
  * with the TDKEK if TK is set
  */
 #define KEY_ENC			0x00400000
-
+#define KEY_ENC_OFFSET		22
 /*
  * No Write Back - Do not allow key to be FIFO STOREd
  */
@@ -162,11 +162,13 @@
  * Enhanced Encryption of Key
  */
 #define KEY_EKT			0x00100000
+#define KEY_EKT_OFFSET		20
 
 /*
  * Encrypted with Trusted Key
  */
 #define KEY_TK			0x00008000
+#define KEY_TK_OFFSET		15
 
 /*
  * KDEST - Key Destination: 0 - class key register,
diff --git a/drivers/crypto/caam/tag_object.c b/drivers/crypto/caam/tag_object.c
new file mode 100644
index 00000000..55f41e9
--- /dev/null
+++ b/drivers/crypto/caam/tag_object.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/*
+ * Copyright 2018-2020 NXP
+ */
+
+#include <linux/export.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+
+#include "tag_object.h"
+#include "desc.h"
+
+/**
+ * is_black_key -	Check if the tag object header is a black key
+ * @header:		The tag object header configuration
+ *
+ * Return:		True if is a black key, false otherwise
+ */
+bool is_black_key(const struct header_conf *header)
+{
+	u32 type = header->type;
+	/* Check type and color bitfields from tag object type */
+	return (type & (BIT(TAG_OBJ_COLOR_OFFSET) |
+			BIT(TAG_OBJ_TYPE_OFFSET))) == BIT(TAG_OBJ_COLOR_OFFSET);
+}
+EXPORT_SYMBOL(is_black_key);
+
+/**
+ * is_valid_header_conf - Check if the header configuration is valid
+ * @header:		The header configuration
+ *
+ * Return:		True if the header of the tag object configuration,
+ *			has the TAG_OBJECT_MAGIC number and a valid type,
+ *			false otherwise
+ */
+bool is_valid_header_conf(const struct header_conf *header)
+{
+	return (header->_magic_number == TAG_OBJECT_MAGIC);
+}
+
+/**
+ * get_tag_object_header_conf - Retrieve the address of tag object
+ *				header configuration
+ * @buffer:			Buffer containing the tag object
+ * @size:			The size of buffer
+ * @header:			Returned tag object header configuration
+ *
+ * Return:			'0' on success, error code otherwise
+ */
+int get_tag_object_header_conf(const void *buffer, size_t size,
+			       struct header_conf **header)
+{
+	bool valid;
+
+	/* Retrieve the tag object */
+	struct tagged_object *tag_obj = (struct tagged_object *)buffer;
+
+	/* Check if one can retrieve the tag object header configuration */
+	if (size < TAG_OVERHEAD_SIZE)
+		return -EINVAL;
+
+	/* Check tag object header configuration */
+	valid = is_valid_header_conf(&tag_obj->header);
+
+	/* Retrieve the tag object header configuration address */
+	*header = &tag_obj->header;
+
+	return valid ? 0 : -EINVAL;
+}
+EXPORT_SYMBOL(get_tag_object_header_conf);
+
+/**
+ * get_key_conf -	Retrieve the key configuration,
+ *			meaning the length of the black key and
+ *			the KEY command parameters needed for CAAM
+ * @header:		The tag object header configuration
+ * @real_len:		Key length
+ * @load_param:		Load parameters for KEY command:
+ *			- indicator for encrypted keys: plaintext or black
+ *			- indicator for encryption mode: AES-ECB or AES-CCM
+ *			- indicator for encryption keys: JDKEK or TDKEK
+ */
+void get_key_conf(const struct header_conf *header,
+		  u32 *real_len, u32 *load_param)
+{
+	*real_len = header->real_len;
+	/* Based on the color of the key, set key encryption bit (ENC) */
+	*load_param = ((header->type >> TAG_OBJ_COLOR_OFFSET) &
+		       TAG_OBJ_COLOR_MASK) << KEY_ENC_OFFSET;
+	/*
+	 * For red keys, the TK and EKT bits are ignored.
+	 * So we set them anyway, to be valid when the key is black.
+	 */
+	*load_param |= ((header->type >> TAG_OBJ_TK_OFFSET) &
+			 TAG_OBJ_TK_MASK) << KEY_TK_OFFSET;
+	*load_param |= ((header->type >> TAG_OBJ_EKT_OFFSET) &
+			 TAG_OBJ_EKT_MASK) << KEY_EKT_OFFSET;
+}
+EXPORT_SYMBOL(get_key_conf);
+
+/**
+ * get_tagged_data -	Retrieve the address of the data and size
+ *			of the tagged object
+ * @tagged_object:		Pointer to tag object
+ * @tagged_object_size:		The tagged object size
+ * @data:			Returned the address of the data from
+ *				the tagged object
+ * @data_size:			Returned the size of the data from the
+ *				tagged object
+ *
+ * Return:			'0' on success, error code otherwise
+ */
+int get_tagged_data(const void *tagged_object, size_t tagged_object_size,
+		    const void **data, u32 *data_size)
+{
+	/* Retrieve the tag object */
+	struct tagged_object *tag_obj = (struct tagged_object *)tagged_object;
+	/* Check if one can retrieve the data from the tagged object */
+	if (tagged_object_size < TAG_OVERHEAD_SIZE)
+		return -EINVAL;
+
+	/* Retrieve the address of the data/object from the tagged object */
+	*data = &tag_obj->object;
+	/* Retrieve the size of the data from the tagged object */
+	*data_size = tagged_object_size - TAG_OVERHEAD_SIZE;
+
+	return 0;
+}
+EXPORT_SYMBOL(get_tagged_data);
diff --git a/drivers/crypto/caam/tag_object.h b/drivers/crypto/caam/tag_object.h
new file mode 100644
index 00000000..9950c02
--- /dev/null
+++ b/drivers/crypto/caam/tag_object.h
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright 2018-2020 NXP
+ */
+
+#ifndef _TAG_OBJECT_H_
+#define _TAG_OBJECT_H_
+
+#include <linux/types.h>
+#include <linux/bitops.h>
+
+/**
+ * Magic number to identify the tag object structure
+ * 0x54 = 'T'
+ * 0x61 = 'a'
+ * 0x67 = 'g'
+ * 0x4f = 'O'
+ */
+#define TAG_OBJECT_MAGIC	0x5461674f
+#define TAG_MIN_SIZE		(2 * sizeof(struct header_conf))
+#define TAG_OVERHEAD_SIZE	sizeof(struct header_conf)
+
+/*
+ * Tag object type is a bitfield:
+ *
+ * EKT:	Encrypted Key Type (AES-ECB or AES-CCM)
+ * TK:	Trusted Key (use Job Descriptor Key Encryption Key (JDKEK)
+ *	or Trusted Descriptor Key Encryption Key (TDKEK) to
+ *	decrypt the key to be loaded into a Key Register).
+ *
+ *| Denomination | Security state | Memory  | EKT | TK    | Type | Color |
+ *| ------------ | -------------- | ------- | --- | ----- | ---- | ----- |
+ *| bit(s)       | 5-6            | 4       | 3   | 2     | 1    | 0     |
+ *| option 0     | non-secure     | general | ECB | JDKEK | key  | red   |
+ *| option 1     | secure         | secure  | CCM | TDKEK | blob | black |
+ *| option 2     | trusted        |         |     |       |      |       |
+ *
+ * CAAM supports two different Black Key encapsulation schemes,
+ * one intended for quick decryption (uses AES-ECB encryption),
+ * and another intended for high assurance (uses AES-CCM encryption).
+ *
+ * CAAM implements both Trusted and normal (non-Trusted) Black Keys,
+ * which are encrypted with different key-encryption keys.
+ * Both Trusted and normal Descriptors are allowed to encrypt or decrypt
+ * normal Black Keys, but only Trusted Descriptors are allowed to
+ * encrypt or decrypt Trusted Black Keys.
+ */
+#define TAG_OBJ_COLOR_OFFSET		0
+#define TAG_OBJ_COLOR_MASK		0x1
+#define TAG_OBJ_TYPE_OFFSET		1
+#define TAG_OBJ_TK_OFFSET		2
+#define TAG_OBJ_TK_MASK			0x1
+#define TAG_OBJ_EKT_OFFSET		3
+#define TAG_OBJ_EKT_MASK		0x1
+#define TAG_OBJ_MEM_OFFSET		4
+#define TAG_OBJ_SEC_STATE_OFFSET	5
+
+/**
+ * struct header_conf - Header configuration structure, which represents
+ *			the metadata (or simply a header) applied to the
+ *			actual data (e.g. black key)
+ * @_magic_number     : A magic number to identify the structure
+ * @version           : The version of the data contained (e.g. tag object)
+ * @type              : The type of data contained (e.g. black key, blob, etc.)
+ * @real_len          : Length of the object to be loaded by CAAM
+ */
+struct header_conf {
+	u32 _magic_number;
+	u32 version;
+	u32 type;
+	u32 real_len;
+};
+
+/**
+ * struct tagged_object - Tag object structure, which represents the metadata
+ *                        (or simply a header) and the actual data
+ *                        (e.g. black key) obtained from hardware
+ * @tag                 : The configuration of the data (e.g. header)
+ * @object              : The actual data (e.g. black key)
+ */
+struct tagged_object {
+	struct header_conf header;
+	char object;
+};
+
+bool is_black_key(const struct header_conf * const header);
+
+bool is_valid_header_conf(const struct header_conf *header);
+
+int get_tag_object_header_conf(const void *buffer, size_t buffer_size,
+			       struct header_conf **header);
+
+void get_key_conf(const struct header_conf *header,
+		  u32 *real_len, u32 *load_param);
+
+int get_tagged_data(const void *buffer, size_t buffer_size,
+		    const void **data, u32 *data_size);
+
+#endif /* _TAG_OBJECT_H_ */
-- 
2.1.0

Re: [PATCH 1/2] crypto: caam - add tag object functionality

[Horia Geantă]

On 7/13/2020 1:05 AM, Iuliana Prodan wrote:
> A tag object represents the metadata (or simply a header/configuration)
> and the actual data (e.g. black key) obtained from hardware.
> Add functionality to tag an object with metadata:
> - validate metadata: check tag object header;
> - retrieve metadata: get tag object header configuration, black key
> configuration or tag object data.
> 
> This API expects that the object (the actual data) from a tag object
> to be a buffer (defined by address and size).
> 
> Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>

> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> ---
>  drivers/crypto/caam/Kconfig      |   9 +++
>  drivers/crypto/caam/Makefile     |   1 +
>  drivers/crypto/caam/desc.h       |   4 +-
>  drivers/crypto/caam/tag_object.c | 129 +++++++++++++++++++++++++++++++++++++++
>  drivers/crypto/caam/tag_object.h |  99 ++++++++++++++++++++++++++++++
>  5 files changed, 241 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/crypto/caam/tag_object.c
>  create mode 100644 drivers/crypto/caam/tag_object.h
> 
> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
> index bc35aa0..73368d8 100644
> --- a/drivers/crypto/caam/Kconfig
> +++ b/drivers/crypto/caam/Kconfig
> @@ -149,6 +149,15 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API
>  	  Selecting this will register the SEC4 hardware rng to
>  	  the hw_random API for supplying the kernel entropy pool.
>  
> +config CRYPTO_DEV_FSL_CAAM_TK_API
> +	bool "Register tagged key cryptography implementations with Crypto API"
> +	depends on CRYPTO_DEV_FSL_CAAM_CRYPTO_API
> +	help
> +	  Selecting this will register algorithms supporting tagged key.
> +
> +	  Tagged keys are keys that contain metadata indicating what
> +	  they are and how to handle them.
> +
Let's keep config options at minimum, we've got plenty of them already.

Please get rid of this entry (and the associated ifdeffery etc.).

>  endif # CRYPTO_DEV_FSL_CAAM_JR
>  
>  endif # CRYPTO_DEV_FSL_CAAM
> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
> index 68d5cc0..192a88e 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API_DESC) += caamhash_desc.o
>  
>  caam-y := ctrl.o
>  caam_jr-y := jr.o key_gen.o
> +caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API) += tag_object.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API) += caamalg.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
> diff --git a/drivers/crypto/caam/desc.h b/drivers/crypto/caam/desc.h
> index e134709..3001a8d 100644
> --- a/drivers/crypto/caam/desc.h
> +++ b/drivers/crypto/caam/desc.h
> @@ -152,7 +152,7 @@
>   * with the TDKEK if TK is set
>   */
>  #define KEY_ENC			0x00400000
> -
> +#define KEY_ENC_OFFSET		22
Missing empty line b/w groups of defines.

>  /*
>   * No Write Back - Do not allow key to be FIFO STOREd
>   */
> @@ -162,11 +162,13 @@
>   * Enhanced Encryption of Key
>   */
>  #define KEY_EKT			0x00100000
> +#define KEY_EKT_OFFSET		20
>  
>  /*
>   * Encrypted with Trusted Key
>   */
>  #define KEY_TK			0x00008000
> +#define KEY_TK_OFFSET		15
>  
>  /*
>   * KDEST - Key Destination: 0 - class key register,
> diff --git a/drivers/crypto/caam/tag_object.c b/drivers/crypto/caam/tag_object.c
> new file mode 100644
> index 00000000..55f41e9
> --- /dev/null
> +++ b/drivers/crypto/caam/tag_object.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> +/*
> + * Copyright 2018-2020 NXP
> + */
> +
> +#include <linux/export.h>
> +#include <linux/string.h>
Why is this include needed?

> +#include <linux/errno.h>
> +
> +#include "tag_object.h"
> +#include "desc.h"
> +
> +/**
> + * is_black_key -	Check if the tag object header is a black key
> + * @header:		The tag object header configuration
> + *
> + * Return:		True if is a black key, false otherwise
> + */
> +bool is_black_key(const struct header_conf *header)
> +{
> +	u32 type = header->type;
> +	/* Check type and color bitfields from tag object type */
> +	return (type & (BIT(TAG_OBJ_COLOR_OFFSET) |
> +			BIT(TAG_OBJ_TYPE_OFFSET))) == BIT(TAG_OBJ_COLOR_OFFSET);
> +}
> +EXPORT_SYMBOL(is_black_key);
Exported symbols should be named such that the probability of
collision with others is neglijible.

> +
> +/**
> + * is_valid_header_conf - Check if the header configuration is valid
> + * @header:		The header configuration
> + *
> + * Return:		True if the header of the tag object configuration,
> + *			has the TAG_OBJECT_MAGIC number and a valid type,
> + *			false otherwise
> + */
> +bool is_valid_header_conf(const struct header_conf *header)
> +{
> +	return (header->_magic_number == TAG_OBJECT_MAGIC);
> +}
Let's not "publish" internal functions in tag_object.h.

> +
> +/**
> + * get_tag_object_header_conf - Retrieve the address of tag object
> + *				header configuration
> + * @buffer:			Buffer containing the tag object
> + * @size:			The size of buffer
> + * @header:			Returned tag object header configuration
> + *
> + * Return:			'0' on success, error code otherwise
> + */
> +int get_tag_object_header_conf(const void *buffer, size_t size,
> +			       struct header_conf **header)
> +{
> +	bool valid;
> +
> +	/* Retrieve the tag object */
> +	struct tagged_object *tag_obj = (struct tagged_object *)buffer;
> +
> +	/* Check if one can retrieve the tag object header configuration */
> +	if (size < TAG_OVERHEAD_SIZE)
> +		return -EINVAL;
> +
> +	/* Check tag object header configuration */
> +	valid = is_valid_header_conf(&tag_obj->header);
> +
> +	/* Retrieve the tag object header configuration address */
> +	*header = &tag_obj->header;
> +
> +	return valid ? 0 : -EINVAL;
> +}
> +EXPORT_SYMBOL(get_tag_object_header_conf);
> +
> +/**
> + * get_key_conf -	Retrieve the key configuration,
> + *			meaning the length of the black key and
> + *			the KEY command parameters needed for CAAM
> + * @header:		The tag object header configuration
> + * @real_len:		Key length
> + * @load_param:		Load parameters for KEY command:
> + *			- indicator for encrypted keys: plaintext or black
> + *			- indicator for encryption mode: AES-ECB or AES-CCM
> + *			- indicator for encryption keys: JDKEK or TDKEK
> + */
> +void get_key_conf(const struct header_conf *header,
> +		  u32 *real_len, u32 *load_param)
> +{
> +	*real_len = header->real_len;
> +	/* Based on the color of the key, set key encryption bit (ENC) */
> +	*load_param = ((header->type >> TAG_OBJ_COLOR_OFFSET) &
> +		       TAG_OBJ_COLOR_MASK) << KEY_ENC_OFFSET;
> +	/*
> +	 * For red keys, the TK and EKT bits are ignored.
> +	 * So we set them anyway, to be valid when the key is black.
> +	 */
> +	*load_param |= ((header->type >> TAG_OBJ_TK_OFFSET) &
> +			 TAG_OBJ_TK_MASK) << KEY_TK_OFFSET;
> +	*load_param |= ((header->type >> TAG_OBJ_EKT_OFFSET) &
> +			 TAG_OBJ_EKT_MASK) << KEY_EKT_OFFSET;
> +}
> +EXPORT_SYMBOL(get_key_conf);
> +
> +/**
> + * get_tagged_data -	Retrieve the address of the data and size
> + *			of the tagged object
> + * @tagged_object:		Pointer to tag object
> + * @tagged_object_size:		The tagged object size
> + * @data:			Returned the address of the data from
> + *				the tagged object
> + * @data_size:			Returned the size of the data from the
> + *				tagged object
> + *
> + * Return:			'0' on success, error code otherwise
> + */
> +int get_tagged_data(const void *tagged_object, size_t tagged_object_size,
> +		    const void **data, u32 *data_size)
> +{
> +	/* Retrieve the tag object */
> +	struct tagged_object *tag_obj = (struct tagged_object *)tagged_object;
> +	/* Check if one can retrieve the data from the tagged object */
> +	if (tagged_object_size < TAG_OVERHEAD_SIZE)
> +		return -EINVAL;
> +
> +	/* Retrieve the address of the data/object from the tagged object */
> +	*data = &tag_obj->object;
> +	/* Retrieve the size of the data from the tagged object */
> +	*data_size = tagged_object_size - TAG_OVERHEAD_SIZE;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(get_tagged_data);
> diff --git a/drivers/crypto/caam/tag_object.h b/drivers/crypto/caam/tag_object.h
> new file mode 100644
> index 00000000..9950c02
> --- /dev/null
> +++ b/drivers/crypto/caam/tag_object.h
> @@ -0,0 +1,99 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
> +/*
> + * Copyright 2018-2020 NXP
> + */
> +
> +#ifndef _TAG_OBJECT_H_
> +#define _TAG_OBJECT_H_
> +
> +#include <linux/types.h>
> +#include <linux/bitops.h>
> +
> +/**
> + * Magic number to identify the tag object structure
> + * 0x54 = 'T'
> + * 0x61 = 'a'
> + * 0x67 = 'g'
> + * 0x4f = 'O'
> + */
> +#define TAG_OBJECT_MAGIC	0x5461674f
> +#define TAG_MIN_SIZE		(2 * sizeof(struct header_conf))
> +#define TAG_OVERHEAD_SIZE	sizeof(struct header_conf)
> +
> +/*
> + * Tag object type is a bitfield:
header_conf.type

> + *
> + * EKT:	Encrypted Key Type (AES-ECB or AES-CCM)
> + * TK:	Trusted Key (use Job Descriptor Key Encryption Key (JDKEK)
> + *	or Trusted Descriptor Key Encryption Key (TDKEK) to
> + *	decrypt the key to be loaded into a Key Register).
> + *
> + *| Denomination | Security state | Memory  | EKT | TK    | Type | Color |
> + *| ------------ | -------------- | ------- | --- | ----- | ---- | ----- |
> + *| bit(s)       | 5-6            | 4       | 3   | 2     | 1    | 0     |
> + *| option 0     | non-secure     | general | ECB | JDKEK | key  | red   |
> + *| option 1     | secure         | secure  | CCM | TDKEK | blob | black |
> + *| option 2     | trusted        |         |     |       |      |       |
> + *
> + * CAAM supports two different Black Key encapsulation schemes,
> + * one intended for quick decryption (uses AES-ECB encryption),
> + * and another intended for high assurance (uses AES-CCM encryption).
> + *
> + * CAAM implements both Trusted and normal (non-Trusted) Black Keys,
> + * which are encrypted with different key-encryption keys.
> + * Both Trusted and normal Descriptors are allowed to encrypt or decrypt
> + * normal Black Keys, but only Trusted Descriptors are allowed to
> + * encrypt or decrypt Trusted Black Keys.
> + */
> +#define TAG_OBJ_COLOR_OFFSET		0
> +#define TAG_OBJ_COLOR_MASK		0x1
> +#define TAG_OBJ_TYPE_OFFSET		1
> +#define TAG_OBJ_TK_OFFSET		2
> +#define TAG_OBJ_TK_MASK			0x1
> +#define TAG_OBJ_EKT_OFFSET		3
> +#define TAG_OBJ_EKT_MASK		0x1
> +#define TAG_OBJ_MEM_OFFSET		4
> +#define TAG_OBJ_SEC_STATE_OFFSET	5
Values for masks should be already shifted.
That's the idiom we use in the driver, and that's what's mostly used
throughout the kernel.

> +
> +/**
> + * struct header_conf - Header configuration structure, which represents
> + *			the metadata (or simply a header) applied to the
> + *			actual data (e.g. black key)
> + * @_magic_number     : A magic number to identify the structure
> + * @version           : The version of the data contained (e.g. tag object)
> + * @type              : The type of data contained (e.g. black key, blob, etc.)
> + * @real_len          : Length of the object to be loaded by CAAM
> + */
> +struct header_conf {
> +	u32 _magic_number;
> +	u32 version;
Version is currently not used in the implementation.
I assume this is intended for allowing a compatibility scheme.

Have you give it a thought how would it work?
For example, what happens in case a new field has to be added in the header?

> +	u32 type;
> +	u32 real_len;
> +};
> +
> +/**
> + * struct tagged_object - Tag object structure, which represents the metadata
> + *                        (or simply a header) and the actual data
> + *                        (e.g. black key) obtained from hardware
> + * @tag                 : The configuration of the data (e.g. header)
> + * @object              : The actual data (e.g. black key)
> + */
> +struct tagged_object {
> +	struct header_conf header;
> +	char object;
Should this be aligned?
In case the object is not copied, it will be DMA-read by the device
from this location.

Also let's make it clear this field is just a tag / alias
to the structure offset, and it's size is variable.

Horia

[PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Iuliana Prodan]

Tagged keys are keys that contain metadata indicating what
they are and how to handle them using tag_object API.

Add support, for tagged keys, to skcipher algorithms by
adding new transformations, with _tk_ prefix to distinguish
between plaintext and tagged keys.

For job descriptors a new option (key_cmd_opt) was added for KEY command.
Tagged keys can be loaded using only a KEY command with ENC=1
and the proper setting of the EKT bit. The EKT bit in the
KEY command indicates which encryption algorithm (AES-ECB or
AES-CCM) should be used to decrypt the key. These options will be kept in
key_cmd_opt.

The tk_ transformations can be used directly by their name:
struct sockaddr_alg sa = {
    .salg_family = AF_ALG,
    .salg_type = "skcipher", /* this selects the symmetric cipher */
    .salg_name = "tk(cbc(aes))" /* this is the cipher name */
};
or for dm-crypt, e.g. using dmsetup:
dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
sector_size:512".

Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
---
 drivers/crypto/caam/caamalg.c      | 107 +++++++++++++++++++++++++++++++++++--
 drivers/crypto/caam/caamalg_desc.c |  28 ++++++++--
 drivers/crypto/caam/desc_constr.h  |   4 ++
 3 files changed, 131 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index b2f9882..9e4206f 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -3,7 +3,7 @@
  * caam - Freescale FSL CAAM support for crypto API
  *
  * Copyright 2008-2011 Freescale Semiconductor, Inc.
- * Copyright 2016-2019 NXP
+ * Copyright 2016-2020 NXP
  *
  * Based on talitos crypto API driver.
  *
@@ -58,6 +58,10 @@
 #include "caamalg_desc.h"
 #include <crypto/engine.h>
 
+#ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API
+#include "tag_object.h"
+#endif /* CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API */
+
 /*
  * crypto alg
  */
@@ -84,6 +88,7 @@ struct caam_alg_entry {
 	bool rfc3686;
 	bool geniv;
 	bool nodkp;
+	bool support_tagged_key;
 };
 
 struct caam_aead_alg {
@@ -736,9 +741,16 @@ static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
 	print_hex_dump_debug("key in @"__stringify(__LINE__)": ",
 			     DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
 
-	ctx->cdata.keylen = keylen;
-	ctx->cdata.key_virt = key;
-	ctx->cdata.key_inline = true;
+	/*
+	 * If the algorithm has support for tagged key,
+	 * this is already set in tk_skcipher_setkey().
+	 * Otherwise, set here the algorithm details.
+	 */
+	if (!alg->caam.support_tagged_key) {
+		ctx->cdata.keylen = keylen;
+		ctx->cdata.key_virt = key;
+		ctx->cdata.key_inline = true;
+	}
 
 	/* skcipher_encrypt shared descriptor */
 	desc = ctx->sh_desc_enc;
@@ -816,6 +828,56 @@ static int arc4_skcipher_setkey(struct crypto_skcipher *skcipher,
 	return skcipher_setkey(skcipher, key, keylen, 0);
 }
 
+#ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API
+static int tk_skcipher_setkey(struct crypto_skcipher *skcipher,
+			      const u8 *key, unsigned int keylen)
+{
+	struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
+	struct device *jrdev = ctx->jrdev;
+	struct header_conf *header;
+	int ret;
+
+	ctx->cdata.keylen = keylen;
+	ctx->cdata.key_virt = key;
+	ctx->cdata.key_inline = true;
+
+	/* Retrieve the address of the tag object configuration */
+	ret = get_tag_object_header_conf(ctx->cdata.key_virt,
+					 ctx->cdata.keylen, &header);
+	if (ret) {
+		dev_err(jrdev,
+			"unable to get tag object header configuration\n");
+		return ret;
+	}
+
+	/* Check if the tag object header is a black key */
+	if (!is_black_key(header)) {
+		dev_err(jrdev,
+			"tagged key provided is not a black key\n");
+		return -EINVAL;
+	}
+
+	/* Retrieve the black key configuration */
+	get_key_conf(header,
+		     &ctx->cdata.key_real_len,
+		     &ctx->cdata.key_cmd_opt);
+
+	/*
+	 * Retrieve the address of the data
+	 * and size of the tagged object
+	 */
+	ret = get_tagged_data(ctx->cdata.key_virt, ctx->cdata.keylen,
+			      &ctx->cdata.key_virt, &ctx->cdata.keylen);
+	if (ret) {
+		dev_err(jrdev,
+			"unable to get data from tagged object\n");
+		return ret;
+	}
+
+	return skcipher_setkey(skcipher, key, keylen, 0);
+}
+#endif /* CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API */
+
 static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
 			       const u8 *key, unsigned int keylen)
 {
@@ -1833,6 +1895,25 @@ static struct caam_skcipher_alg driver_algs[] = {
 		},
 		.caam.class1_alg_type = OP_ALG_ALGSEL_AES | OP_ALG_AAI_CBC,
 	},
+#ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API
+	{
+		.skcipher = {
+			.base = {
+				.cra_name = "tk(cbc(aes))",
+				.cra_driver_name = "tk-cbc-aes-caam",
+				.cra_blocksize = AES_BLOCK_SIZE,
+			},
+			.setkey = tk_skcipher_setkey,
+			.encrypt = skcipher_encrypt,
+			.decrypt = skcipher_decrypt,
+			.min_keysize = TAG_MIN_SIZE,
+			.max_keysize = CAAM_MAX_KEY_SIZE,
+			.ivsize = AES_BLOCK_SIZE,
+		},
+		.caam.class1_alg_type = OP_ALG_ALGSEL_AES | OP_ALG_AAI_CBC,
+		.caam.support_tagged_key = true,
+	},
+#endif /* CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API */
 	{
 		.skcipher = {
 			.base = {
@@ -1952,6 +2033,24 @@ static struct caam_skcipher_alg driver_algs[] = {
 		},
 		.caam.class1_alg_type = OP_ALG_ALGSEL_AES | OP_ALG_AAI_ECB,
 	},
+#ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API
+	{
+		.skcipher = {
+			.base = {
+				.cra_name = "tk(ecb(aes))",
+				.cra_driver_name = "tk-ecb-aes-caam",
+				.cra_blocksize = AES_BLOCK_SIZE,
+			},
+			.setkey = tk_skcipher_setkey,
+			.encrypt = skcipher_encrypt,
+			.decrypt = skcipher_decrypt,
+			.min_keysize = TAG_MIN_SIZE,
+			.max_keysize = CAAM_MAX_KEY_SIZE,
+		},
+		.caam.class1_alg_type = OP_ALG_ALGSEL_AES | OP_ALG_AAI_ECB,
+		.caam.support_tagged_key = true,
+	},
+#endif /* CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API */
 	{
 		.skcipher = {
 			.base = {
diff --git a/drivers/crypto/caam/caamalg_desc.c b/drivers/crypto/caam/caamalg_desc.c
index d6c5818..447f7a5 100644
--- a/drivers/crypto/caam/caamalg_desc.c
+++ b/drivers/crypto/caam/caamalg_desc.c
@@ -1389,8 +1389,18 @@ void cnstr_shdsc_skcipher_encap(u32 * const desc, struct alginfo *cdata,
 				   JUMP_COND_SHRD);
 
 	/* Load class1 key only */
-	append_key_as_imm(desc, cdata->key_virt, cdata->keylen,
-			  cdata->keylen, CLASS_1 | KEY_DEST_CLASS_REG);
+	if (IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API) &&
+	    cdata->key_cmd_opt)
+		/*
+		 * Black keys can be loaded using only a KEY command
+		 * with ENC=1 and the proper setting of the EKT bit.
+		 */
+		append_key_as_imm(desc, cdata->key_virt, cdata->keylen,
+				  cdata->key_real_len, CLASS_1 |
+				  KEY_DEST_CLASS_REG | cdata->key_cmd_opt);
+	else
+		append_key_as_imm(desc, cdata->key_virt, cdata->keylen,
+				  cdata->keylen, CLASS_1 | KEY_DEST_CLASS_REG);
 
 	/* Load nonce into CONTEXT1 reg */
 	if (is_rfc3686) {
@@ -1464,8 +1474,18 @@ void cnstr_shdsc_skcipher_decap(u32 * const desc, struct alginfo *cdata,
 				   JUMP_COND_SHRD);
 
 	/* Load class1 key only */
-	append_key_as_imm(desc, cdata->key_virt, cdata->keylen,
-			  cdata->keylen, CLASS_1 | KEY_DEST_CLASS_REG);
+	if (IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM_TK_API) &&
+	    cdata->key_cmd_opt)
+		/*
+		 * Black keys can be loaded using only a KEY command
+		 * with ENC=1 and the proper setting of the EKT bit.
+		 */
+		append_key_as_imm(desc, cdata->key_virt, cdata->keylen,
+				  cdata->key_real_len, CLASS_1 |
+				  KEY_DEST_CLASS_REG | cdata->key_cmd_opt);
+	else
+		append_key_as_imm(desc, cdata->key_virt, cdata->keylen,
+				  cdata->keylen, CLASS_1 | KEY_DEST_CLASS_REG);
 
 	/* Load nonce into CONTEXT1 reg */
 	if (is_rfc3686) {
diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index 62ce642..93b2ab0 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -500,6 +500,8 @@ do { \
  * @key_virt: virtual address where algorithm key resides
  * @key_inline: true - key can be inlined in the descriptor; false - key is
  *              referenced by the descriptor
+ * @key_real_len: size of the key to be loaded by the CAAM
+ * @key_cmd_opt: optional parameters for KEY command
  */
 struct alginfo {
 	u32 algtype;
@@ -508,6 +510,8 @@ struct alginfo {
 	dma_addr_t key_dma;
 	const void *key_virt;
 	bool key_inline;
+	u32 key_real_len;
+	u32 key_cmd_opt;
 };
 
 /**
-- 
2.1.0

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Herbert Xu]

On Mon, Jul 13, 2020 at 01:05:36AM +0300, Iuliana Prodan wrote:
> Tagged keys are keys that contain metadata indicating what
> they are and how to handle them using tag_object API.
> 
> Add support, for tagged keys, to skcipher algorithms by
> adding new transformations, with _tk_ prefix to distinguish
> between plaintext and tagged keys.
> 
> For job descriptors a new option (key_cmd_opt) was added for KEY command.
> Tagged keys can be loaded using only a KEY command with ENC=1
> and the proper setting of the EKT bit. The EKT bit in the
> KEY command indicates which encryption algorithm (AES-ECB or
> AES-CCM) should be used to decrypt the key. These options will be kept in
> key_cmd_opt.
> 
> The tk_ transformations can be used directly by their name:
> struct sockaddr_alg sa = {
>     .salg_family = AF_ALG,
>     .salg_type = "skcipher", /* this selects the symmetric cipher */
>     .salg_name = "tk(cbc(aes))" /* this is the cipher name */
> };
> or for dm-crypt, e.g. using dmsetup:
> dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
> crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
> sector_size:512".
> 
> Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>

Can this use the existing paes name instead of tk as done in
other drivers?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Horia Geantă]

On 7/16/2020 10:36 AM, Herbert Xu wrote:
> On Mon, Jul 13, 2020 at 01:05:36AM +0300, Iuliana Prodan wrote:
>> Tagged keys are keys that contain metadata indicating what
>> they are and how to handle them using tag_object API.
>>
>> Add support, for tagged keys, to skcipher algorithms by
>> adding new transformations, with _tk_ prefix to distinguish
>> between plaintext and tagged keys.
>>
>> For job descriptors a new option (key_cmd_opt) was added for KEY command.
>> Tagged keys can be loaded using only a KEY command with ENC=1
>> and the proper setting of the EKT bit. The EKT bit in the
>> KEY command indicates which encryption algorithm (AES-ECB or
>> AES-CCM) should be used to decrypt the key. These options will be kept in
>> key_cmd_opt.
>>
>> The tk_ transformations can be used directly by their name:
>> struct sockaddr_alg sa = {
>>     .salg_family = AF_ALG,
>>     .salg_type = "skcipher", /* this selects the symmetric cipher */
>>     .salg_name = "tk(cbc(aes))" /* this is the cipher name */
>> };
>> or for dm-crypt, e.g. using dmsetup:
>> dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
>> crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
>> sector_size:512".
>>
>> Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> 
> Can this use the existing paes name instead of tk as done in
> other drivers?
> 
This patch set adds support only for some AES-based algorithms.
However, going further the plan is to add all keyed algorithms
supported by caam.

Thus I wouldn't tie the name to AES.

Possible alternatives would be:
pk - protected keys
tk - with "t" standing for "trusted" instead of "tagged"

Wrt. "trusted", I am not sure this term should strictly be tied
to a TPM or not.

Thanks,
Horia

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Herbert Xu]

On Thu, Jul 16, 2020 at 01:35:51PM +0300, Horia Geantă wrote:
>
> This patch set adds support only for some AES-based algorithms.
> However, going further the plan is to add all keyed algorithms
> supported by caam.
> 
> Thus I wouldn't tie the name to AES.

Yes but it's still exactly the same underlying feature as paes.
So I don't want to have two ways of doing the same thing in the
Crypto API.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Horia Geantă]

On 7/16/2020 2:53 PM, Herbert Xu wrote:
> On Thu, Jul 16, 2020 at 01:35:51PM +0300, Horia Geantă wrote:
>>
>> This patch set adds support only for some AES-based algorithms.
>> However, going further the plan is to add all keyed algorithms
>> supported by caam.
>>
>> Thus I wouldn't tie the name to AES.
> 
> Yes but it's still exactly the same underlying feature as paes.
> So I don't want to have two ways of doing the same thing in the
> Crypto API.
> 
So instead of tk(cbc(aes)) use paes(cbc(aes) or cbc(paes)?

How would this work for hmac(sha512),
paes(hmac(sha512)) or hmac(psha512), or even phmac(sha512)?

Thanks,
Horia

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Herbert Xu]

On Thu, Jul 16, 2020 at 03:07:50PM +0300, Horia Geantă wrote:
>
> So instead of tk(cbc(aes)) use paes(cbc(aes) or cbc(paes)?

Well if we're following the existing paes model then it'd be
cbc(paes).

> How would this work for hmac(sha512),
> paes(hmac(sha512)) or hmac(psha512), or even phmac(sha512)?

Perhaps hmac(psha512).

The point is whatever scheme you come up with has to be consistent
across all drivers.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Van Leeuwen, Pascal]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of Herbert Xu
> Sent: Thursday, July 16, 2020 2:20 PM
> To: Horia Geantă <horia.geanta@nxp.com>
> Cc: Iuliana Prodan <iuliana.prodan@nxp.com>; Aymen Sghaier <aymen.sghaier@nxp.com>; David S. Miller <davem@davemloft.net>;
> Silvano Di Ninno <silvano.dininno@nxp.com>; Franck Lenormand <franck.lenormand@nxp.com>; linux-crypto@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms
>
> <<< External Email >>>
> On Thu, Jul 16, 2020 at 03:07:50PM +0300, Horia Geantă wrote:
> >
> > So instead of tk(cbc(aes)) use paes(cbc(aes) or cbc(paes)?
>
> Well if we're following the existing paes model then it'd be
> cbc(paes).
>
> > How would this work for hmac(sha512),
> > paes(hmac(sha512)) or hmac(psha512), or even phmac(sha512)?
>
> Perhaps hmac(psha512).
>
That would make no sense though, as sha512 does not involve any keys ...
It's the HMAC part that needs the keys. So phmac(sha512) then?

> The point is whatever scheme you come up with has to be consistent
> across all drivers.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Pascal van Leeuwen
Silicon IP Architect Multi-Protocol Engines, Rambus Security
Rambus ROTW Holding BV
+31-73 6581953

Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
Please be so kind to update your e-mail address book with my new e-mail address.


** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **

Rambus Inc.<http://www.rambus.com>

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Herbert Xu]

On Thu, Jul 16, 2020 at 12:24:49PM +0000, Van Leeuwen, Pascal wrote:
>
> That would make no sense though, as sha512 does not involve any keys ...
> It's the HMAC part that needs the keys. So phmac(sha512) then?

You're right, that would be phmac(...).

But the point is we don't want each driver to do its own thing
so whatever scheme we pick should be applicable to all drivers.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Richard Weinberger]

On Mon, Jul 13, 2020 at 12:09 AM Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
>
> Tagged keys are keys that contain metadata indicating what
> they are and how to handle them using tag_object API.
>
> Add support, for tagged keys, to skcipher algorithms by
> adding new transformations, with _tk_ prefix to distinguish
> between plaintext and tagged keys.
>
> For job descriptors a new option (key_cmd_opt) was added for KEY command.
> Tagged keys can be loaded using only a KEY command with ENC=1
> and the proper setting of the EKT bit. The EKT bit in the
> KEY command indicates which encryption algorithm (AES-ECB or
> AES-CCM) should be used to decrypt the key. These options will be kept in
> key_cmd_opt.
>
> The tk_ transformations can be used directly by their name:
> struct sockaddr_alg sa = {
>     .salg_family = AF_ALG,
>     .salg_type = "skcipher", /* this selects the symmetric cipher */
>     .salg_name = "tk(cbc(aes))" /* this is the cipher name */
> };
> or for dm-crypt, e.g. using dmsetup:
> dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
> crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
> sector_size:512".

How to use it with cryptsetup?
I'm asking because it is not clear to me why you are not implementing
a new kernel key type (KEYS subsystem)
to utilize tagged keys.
Many tools already support the keyctl userspace interface (cryptsetup,
fscrypt, ...).

-- 
Thanks,
//richard

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Richard Weinberger]

On Thu, Jul 16, 2020 at 4:12 PM Richard Weinberger
<richard.weinberger@gmail.com> wrote:
>
> On Mon, Jul 13, 2020 at 12:09 AM Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
> >
> > Tagged keys are keys that contain metadata indicating what
> > they are and how to handle them using tag_object API.
> >
> > Add support, for tagged keys, to skcipher algorithms by
> > adding new transformations, with _tk_ prefix to distinguish
> > between plaintext and tagged keys.
> >
> > For job descriptors a new option (key_cmd_opt) was added for KEY command.
> > Tagged keys can be loaded using only a KEY command with ENC=1
> > and the proper setting of the EKT bit. The EKT bit in the
> > KEY command indicates which encryption algorithm (AES-ECB or
> > AES-CCM) should be used to decrypt the key. These options will be kept in
> > key_cmd_opt.
> >
> > The tk_ transformations can be used directly by their name:
> > struct sockaddr_alg sa = {
> >     .salg_family = AF_ALG,
> >     .salg_type = "skcipher", /* this selects the symmetric cipher */
> >     .salg_name = "tk(cbc(aes))" /* this is the cipher name */
> > };
> > or for dm-crypt, e.g. using dmsetup:
> > dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
> > crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
> > sector_size:512".
>
> How to use it with cryptsetup?
> I'm asking because it is not clear to me why you are not implementing
> a new kernel key type (KEYS subsystem)
> to utilize tagged keys.
> Many tools already support the keyctl userspace interface (cryptsetup,
> fscrypt, ...).

*friendly ping*

-- 
Thanks,
//richard

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Horia Geantă]

On 9/14/2020 9:38 AM, Richard Weinberger wrote:
> On Thu, Jul 16, 2020 at 4:12 PM Richard Weinberger
> <richard.weinberger@gmail.com> wrote:
>>
>> On Mon, Jul 13, 2020 at 12:09 AM Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
>>>
>>> Tagged keys are keys that contain metadata indicating what
>>> they are and how to handle them using tag_object API.
>>>
>>> Add support, for tagged keys, to skcipher algorithms by
>>> adding new transformations, with _tk_ prefix to distinguish
>>> between plaintext and tagged keys.
>>>
>>> For job descriptors a new option (key_cmd_opt) was added for KEY command.
>>> Tagged keys can be loaded using only a KEY command with ENC=1
>>> and the proper setting of the EKT bit. The EKT bit in the
>>> KEY command indicates which encryption algorithm (AES-ECB or
>>> AES-CCM) should be used to decrypt the key. These options will be kept in
>>> key_cmd_opt.
>>>
>>> The tk_ transformations can be used directly by their name:
>>> struct sockaddr_alg sa = {
>>>     .salg_family = AF_ALG,
>>>     .salg_type = "skcipher", /* this selects the symmetric cipher */
>>>     .salg_name = "tk(cbc(aes))" /* this is the cipher name */
>>> };
>>> or for dm-crypt, e.g. using dmsetup:
>>> dmsetup -v create encrypted --table "0 $(blockdev --getsz /dev/mmcblk2p10)
>>> crypt capi:tk(cbc(aes))-plain :32:logon:seckey 0 /dev/mmcblk2p10 0 1
>>> sector_size:512".
>>
>> How to use it with cryptsetup?
>> I'm asking because it is not clear to me why you are not implementing
>> a new kernel key type (KEYS subsystem)
>> to utilize tagged keys.
>> Many tools already support the keyctl userspace interface (cryptsetup,
>> fscrypt, ...).
> 
> *friendly ping*
> 
We didn't include the key management part in this series,
just the crypto API support for algorithms with protected keys,
to get early feedback.

Wrt. key management:
The NXP vendor / downstream kernel (to be included in i.MX BSP Q3 release)
will have support for protected keys generation.
Besides this, a dedicated ioctl-based interface will allow userspace to
generate and export these keys. After this, user can use standard keyctl
to add a key (as user / logon type) in the keyring, such that it would be
available to dm-crypt.

We know that adding new ioctls is frowned upon, so before trying to upstream
the ioctl-based solution the plan is checking the feasibility of
extending keyctl as David Howells suggested:
https://lore.kernel.org/lkml/8060.1533226481@warthog.procyon.org.uk
(Note the difference b/w adding new key type - which was rejected -
and a key "subtype extension".)

Horia

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "horia geanta" <horia.geanta@nxp.com>
>>> How to use it with cryptsetup?
>>> I'm asking because it is not clear to me why you are not implementing
>>> a new kernel key type (KEYS subsystem)
>>> to utilize tagged keys.
>>> Many tools already support the keyctl userspace interface (cryptsetup,
>>> fscrypt, ...).
>> 
>> *friendly ping*
>> 
> We didn't include the key management part in this series,
> just the crypto API support for algorithms with protected keys,
> to get early feedback.
> 
> Wrt. key management:
> The NXP vendor / downstream kernel (to be included in i.MX BSP Q3 release)
> will have support for protected keys generation.
> Besides this, a dedicated ioctl-based interface will allow userspace to
> generate and export these keys. After this, user can use standard keyctl
> to add a key (as user / logon type) in the keyring, such that it would be
> available to dm-crypt.
> 
> We know that adding new ioctls is frowned upon, so before trying to upstream
> the ioctl-based solution the plan is checking the feasibility of
> extending keyctl as David Howells suggested:
> https://lore.kernel.org/lkml/8060.1533226481@warthog.procyon.org.uk
> (Note the difference b/w adding new key type - which was rejected -
> and a key "subtype extension".)

We have also a keyctl based patch series which should go upstream.
Since we also added a new keytype, it got rejected so far.

Do you have git repo with the WIP patches available?
Not that we do the work twice. :-)
Our patch series also supports DCP beside of CAAM.

Thanks,
//richard

Re: [PATCH 2/2] crypto: caam - support tagged keys for skcipher algorithms

[Horia Geantă]

On 9/16/2020 12:50 AM, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "horia geanta" <horia.geanta@nxp.com>
>>>> How to use it with cryptsetup?
>>>> I'm asking because it is not clear to me why you are not implementing
>>>> a new kernel key type (KEYS subsystem)
>>>> to utilize tagged keys.
>>>> Many tools already support the keyctl userspace interface (cryptsetup,
>>>> fscrypt, ...).
>>>
>>> *friendly ping*
>>>
>> We didn't include the key management part in this series,
>> just the crypto API support for algorithms with protected keys,
>> to get early feedback.
>>
>> Wrt. key management:
>> The NXP vendor / downstream kernel (to be included in i.MX BSP Q3 release)
>> will have support for protected keys generation.
>> Besides this, a dedicated ioctl-based interface will allow userspace to
>> generate and export these keys. After this, user can use standard keyctl
>> to add a key (as user / logon type) in the keyring, such that it would be
>> available to dm-crypt.
>>
>> We know that adding new ioctls is frowned upon, so before trying to upstream
>> the ioctl-based solution the plan is checking the feasibility of
>> extending keyctl as David Howells suggested:
>> https://lore.kernel.org/lkml/8060.1533226481@warthog.procyon.org.uk
>> (Note the difference b/w adding new key type - which was rejected -
>> and a key "subtype extension".)
> 
> We have also a keyctl based patch series which should go upstream.
> Since we also added a new keytype, it got rejected so far.
> 
Could you please point me to the discussion?

> Do you have git repo with the WIP patches available?
> Not that we do the work twice. :-)
Unfortunately we haven't developed any code yet.

> Our patch series also supports DCP beside of CAAM.
> 
By looking at the DCP capabilities, I assume the OTP key that is copied
in the key RAM at boot time is used as KEK.

If you don't mind sharing, I could review the code.

Thanks,
Horia