linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] provide paes selftests
@ 2019-11-13 10:55 Harald Freudenberger
  2019-11-13 10:55 ` [PATCH 1/3] s390/pkey: Add support for key blob with clear key value Harald Freudenberger
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-13 10:55 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: ebiggers, heiko.carstens, gor, Harald Freudenberger

This patch series provides a reworked version of the paes
in-kernel cipher implementation together with the introdution
of in-kernel selftests for the ciphers implemented there.

Please note that the paes patch is developed against linux-next
respectively Herbert Xu's crypto tree as the fixes from Eric Biggers
which move the paes cipers from blkciper to skciper are a
requirement for the paes rework.

For more details about each patch please have a look into the
individual patch headers.

Harald Freudenberger (3):
  s390/pkey: Add support for key blob with clear key value
  s390/crypto: Rework on paes implementation
  crypto/testmgr: add selftests for paes-s390

 arch/s390/crypto/paes_s390.c         | 163 +++++++++----
 crypto/testmgr.c                     |  36 +++
 crypto/testmgr.h                     | 334 +++++++++++++++++++++++++++
 drivers/s390/crypto/pkey_api.c       |  60 ++++-
 drivers/s390/crypto/zcrypt_ccamisc.h |   1 +
 5 files changed, 545 insertions(+), 49 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] s390/pkey: Add support for key blob with clear key value
  2019-11-13 10:55 [PATCH 0/3] provide paes selftests Harald Freudenberger
@ 2019-11-13 10:55 ` Harald Freudenberger
  2019-11-13 10:55 ` [PATCH 2/3] s390/crypto: Rework on paes implementation Harald Freudenberger
  2019-11-13 10:55 ` [PATCH 3/3] crypto/testmgr: add selftests for paes-s390 Harald Freudenberger
  2 siblings, 0 replies; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-13 10:55 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: ebiggers, heiko.carstens, gor, Harald Freudenberger

This patch adds support for a new key blob format to the
pkey kernel module. The new key blob comprises a clear
key value together with key type information.

The implementation tries to derive an protected key
from the blob with the clear key value inside with
1) the PCKMO instruction. This may fail as the LPAR
   profile may disable this way.
2) Generate an CCA AES secure data key with exact the
   clear key value. This requires to have a working
   crypto card in CCA Coprocessor mode. Then derive
   an protected key from the CCA AES secure key again
   with the help of a working crypto card in CCA mode.
If both way fail, the transformation of the clear key
blob into a protected key will fail. For the PAES cipher
this would result in a failure at setkey() invocation.

A clear key value exposed in main memory is a security
risk. The intention of this new 'clear key blob' support
for pkey is to provide self-tests for the PAES cipher key
implementation. These known answer tests obviously need
to be run with well known key values. So with the clear
key blob format there is a way to provide knwon answer
tests together with an pkey clear key blob for the
in-kernel self tests done at cipher registration.

Reference-ID: SEC1918
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/s390/crypto/pkey_api.c       | 60 +++++++++++++++++++++++++---
 drivers/s390/crypto/zcrypt_ccamisc.h |  1 +
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/drivers/s390/crypto/pkey_api.c b/drivers/s390/crypto/pkey_api.c
index 9de3d46b3253..1f3dfcc837bf 100644
--- a/drivers/s390/crypto/pkey_api.c
+++ b/drivers/s390/crypto/pkey_api.c
@@ -71,6 +71,17 @@ struct protaeskeytoken {
 	u8  protkey[MAXPROTKEYSIZE]; /* the protected key blob */
 } __packed;
 
+/* inside view of a clear key token (type 0x00 version 0x02) */
+struct clearaeskeytoken {
+	u8  type;	 /* 0x00 for PAES specific key tokens */
+	u8  res0[3];
+	u8  version;	 /* 0x02 for clear AES key token */
+	u8  res1[3];
+	u32 keytype;	 /* key type, one of the PKEY_KEYTYPE values */
+	u32 len;	 /* bytes actually stored in clearkey[] */
+	u8  clearkey[0]; /* clear key value */
+} __packed;
+
 /*
  * Create a protected key from a clear key value.
  */
@@ -305,26 +316,63 @@ static int pkey_verifyprotkey(const struct pkey_protkey *protkey)
 static int pkey_nonccatok2pkey(const u8 *key, u32 keylen,
 			       struct pkey_protkey *protkey)
 {
+	int rc = -EINVAL;
 	struct keytoken_header *hdr = (struct keytoken_header *)key;
-	struct protaeskeytoken *t;
 
 	switch (hdr->version) {
-	case TOKVER_PROTECTED_KEY:
-		if (keylen != sizeof(struct protaeskeytoken))
-			return -EINVAL;
+	case TOKVER_PROTECTED_KEY: {
+		struct protaeskeytoken *t;
 
+		if (keylen != sizeof(struct protaeskeytoken))
+			goto out;
 		t = (struct protaeskeytoken *)key;
 		protkey->len = t->len;
 		protkey->type = t->keytype;
 		memcpy(protkey->protkey, t->protkey,
 		       sizeof(protkey->protkey));
+		rc = pkey_verifyprotkey(protkey);
+		break;
+	}
+	case TOKVER_CLEAR_KEY: {
+		struct clearaeskeytoken *t;
+		struct pkey_clrkey ckey;
+		struct pkey_seckey skey;
 
-		return pkey_verifyprotkey(protkey);
+		if (keylen < sizeof(struct clearaeskeytoken))
+			goto out;
+		t = (struct clearaeskeytoken *)key;
+		if (keylen != sizeof(*t) + t->len)
+			goto out;
+		if ((t->keytype == PKEY_KEYTYPE_AES_128 && t->len == 16)
+		    || (t->keytype == PKEY_KEYTYPE_AES_192 && t->len == 24)
+		    || (t->keytype == PKEY_KEYTYPE_AES_256 && t->len == 32))
+			memcpy(ckey.clrkey, t->clearkey, t->len);
+		else
+			goto out;
+		/* try direct way with the PCKMO instruction */
+		rc = pkey_clr2protkey(t->keytype, &ckey, protkey);
+		if (rc == 0)
+			break;
+		/* PCKMO failed, so try the CCA secure key way */
+		rc = cca_clr2seckey(0xFFFF, 0xFFFF, t->keytype,
+				    ckey.clrkey, skey.seckey);
+		if (rc == 0)
+			rc = pkey_skey2pkey(skey.seckey, protkey);
+		/* now we should really have an protected key */
+		if (rc == 0)
+			break;
+		DEBUG_ERR("%s unable to build protected key from clear",
+			  __func__);
+		break;
+	}
 	default:
 		DEBUG_ERR("%s unknown/unsupported non-CCA token version %d\n",
 			  __func__, hdr->version);
-		return -EINVAL;
+		rc = -EINVAL;
 	}
+
+out:
+	return rc;
 }
 
 /*
diff --git a/drivers/s390/crypto/zcrypt_ccamisc.h b/drivers/s390/crypto/zcrypt_ccamisc.h
index 77b6cc7b8f82..3a9876d5ab0e 100644
--- a/drivers/s390/crypto/zcrypt_ccamisc.h
+++ b/drivers/s390/crypto/zcrypt_ccamisc.h
@@ -19,6 +19,7 @@
 
 /* For TOKTYPE_NON_CCA: */
 #define TOKVER_PROTECTED_KEY	0x01 /* Protected key token */
+#define TOKVER_CLEAR_KEY	0x02 /* Clear key token */
 
 /* For TOKTYPE_CCA_INTERNAL: */
 #define TOKVER_CCA_AES		0x04 /* CCA AES key token */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] s390/crypto: Rework on paes implementation
  2019-11-13 10:55 [PATCH 0/3] provide paes selftests Harald Freudenberger
  2019-11-13 10:55 ` [PATCH 1/3] s390/pkey: Add support for key blob with clear key value Harald Freudenberger
@ 2019-11-13 10:55 ` Harald Freudenberger
  2019-11-22  8:13   ` Herbert Xu
  2019-11-13 10:55 ` [PATCH 3/3] crypto/testmgr: add selftests for paes-s390 Harald Freudenberger
  2 siblings, 1 reply; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-13 10:55 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: ebiggers, heiko.carstens, gor, Harald Freudenberger

There have been some findings during Eric Biggers rework of the
paes implementation which this patch tries to address:

A very minor finding within paes ctr where when the cpacf instruction
returns with only partially data en/decrytped the walk_done() was
mistakenly done with the all data counter.  Please note this can only
happen when the kmctr returns because the protected key became invalid
in the middle of the operation. And this is only with suspend and
resume on a system with different effective wrapping key.

Eric Biggers mentioned that the context struct within the tfm struct
may be shared among multiple kernel threads. So here now a rework
which uses a spinlock per context to protect the read and write of the
protected key blob value. The en/decrypt functions copy the protected
key(s) at the beginning into a param struct and do not work with the
protected key within the context any more. If the protected key in the
param struct becomes invalid, the key material is again converted to
protected key(s) and the context gets this update protected by the
spinlock. Race conditions are still possible and may result in writing
the very same protected key value more than once. So the spinlock
needs to make sure the protected key(s) within the context are
consistent updated.

The ctr page is now locked by a mutex instead of a spinlock. A similar
patch went into the aes_s390 code as a result of a complain "sleeping
function called from invalid context at ...algapi.h". See
commit 1c2c7029c008 ("s390/crypto: fix possible sleep during spinlock
aquired")' for more. The very same page is now allocated before
registration. As the selftest runs during registration the page needs
to be available before registration or the kernel would crash.

During testing with instrumented code another issue with the xts
en/decrypt function revealed. The retry cleared the running iv value
and thus let to wrong en/decrypted data.

Tested and verified with additional testcases via AF_ALG interface and
additional selftests within the kernel (which will be made available
as an additional patch to the crypto testmgr).

Reported-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 arch/s390/crypto/paes_s390.c | 163 ++++++++++++++++++++++++++---------
 1 file changed, 120 insertions(+), 43 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index c7119c617b6e..c1f1640a7c12 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/cpufeature.h>
 #include <linux/init.h>
+#include <linux/mutex.h>
 #include <linux/spinlock.h>
 #include <crypto/internal/skcipher.h>
 #include <crypto/xts.h>
@@ -32,11 +33,11 @@
  * is called. As paes can handle different kinds of key blobs
  * and padding is also possible, the limits need to be generous.
  */
-#define PAES_MIN_KEYSIZE 64
+#define PAES_MIN_KEYSIZE 32
 #define PAES_MAX_KEYSIZE 256
 
 static u8 *ctrblk;
-static DEFINE_SPINLOCK(ctrblk_lock);
+static DEFINE_MUTEX(ctrblk_lock);
 
 static cpacf_mask_t km_functions, kmc_functions, kmctr_functions;
 
@@ -82,17 +83,19 @@ static inline void _free_kb_keybuf(struct key_blob *kb)
 struct s390_paes_ctx {
 	struct key_blob kb;
 	struct pkey_protkey pk;
+	spinlock_t pk_lock;
 	unsigned long fc;
 };
 
 struct s390_pxts_ctx {
 	struct key_blob kb[2];
 	struct pkey_protkey pk[2];
+	spinlock_t pk_lock;
 	unsigned long fc;
 };
 
-static inline int __paes_convert_key(struct key_blob *kb,
-				     struct pkey_protkey *pk)
+static inline int __paes_keyblob2pkey(struct key_blob *kb,
+				      struct pkey_protkey *pk)
 {
 	int i, ret;
 
@@ -106,22 +109,18 @@ static inline int __paes_convert_key(struct key_blob *kb,
 	return ret;
 }
 
-static int __paes_set_key(struct s390_paes_ctx *ctx)
+static inline int __paes_convert_key(struct s390_paes_ctx *ctx)
 {
-	unsigned long fc;
+	struct pkey_protkey pkey;
 
-	if (__paes_convert_key(&ctx->kb, &ctx->pk))
+	if (__paes_keyblob2pkey(&ctx->kb, &pkey))
 		return -EINVAL;
 
-	/* Pick the correct function code based on the protected key type */
-	fc = (ctx->pk.type == PKEY_KEYTYPE_AES_128) ? CPACF_KM_PAES_128 :
-		(ctx->pk.type == PKEY_KEYTYPE_AES_192) ? CPACF_KM_PAES_192 :
-		(ctx->pk.type == PKEY_KEYTYPE_AES_256) ? CPACF_KM_PAES_256 : 0;
+	spin_lock_bh(&ctx->pk_lock);
+	memcpy(&ctx->pk, &pkey, sizeof(pkey));
+	spin_unlock_bh(&ctx->pk_lock);
 
-	/* Check if the function code is available */
-	ctx->fc = (fc && cpacf_test_func(&km_functions, fc)) ? fc : 0;
-
-	return ctx->fc ? 0 : -EINVAL;
+	return 0;
 }
 
 static int ecb_paes_init(struct crypto_skcipher *tfm)
@@ -129,6 +128,7 @@ static int ecb_paes_init(struct crypto_skcipher *tfm)
 	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
 
 	ctx->kb.key = NULL;
+	spin_lock_init(&ctx->pk_lock);
 
 	return 0;
 }
@@ -140,6 +140,24 @@ static void ecb_paes_exit(struct crypto_skcipher *tfm)
 	_free_kb_keybuf(&ctx->kb);
 }
 
+static inline int __ecb_paes_set_key(struct s390_paes_ctx *ctx)
+{
+	unsigned long fc;
+
+	if (__paes_convert_key(ctx))
+		return -EINVAL;
+
+	/* Pick the correct function code based on the protected key type */
+	fc = (ctx->pk.type == PKEY_KEYTYPE_AES_128) ? CPACF_KM_PAES_128 :
+		(ctx->pk.type == PKEY_KEYTYPE_AES_192) ? CPACF_KM_PAES_192 :
+		(ctx->pk.type == PKEY_KEYTYPE_AES_256) ? CPACF_KM_PAES_256 : 0;
+
+	/* Check if the function code is available */
+	ctx->fc = (fc && cpacf_test_func(&km_functions, fc)) ? fc : 0;
+
+	return ctx->fc ? 0 : -EINVAL;
+}
+
 static int ecb_paes_set_key(struct crypto_skcipher *tfm, const u8 *in_key,
 			    unsigned int key_len)
 {
@@ -151,7 +169,7 @@ static int ecb_paes_set_key(struct crypto_skcipher *tfm, const u8 *in_key,
 	if (rc)
 		return rc;
 
-	if (__paes_set_key(ctx)) {
+	if (__ecb_paes_set_key(ctx)) {
 		crypto_skcipher_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
 		return -EINVAL;
 	}
@@ -165,18 +183,31 @@ static int ecb_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 	struct skcipher_walk walk;
 	unsigned int nbytes, n, k;
 	int ret;
+	struct {
+		u8 key[MAXPROTKEYSIZE];
+	} param;
 
 	ret = skcipher_walk_virt(&walk, req, false);
+	if (ret)
+		return ret;
+
+	spin_lock_bh(&ctx->pk_lock);
+	memcpy(param.key, ctx->pk.protkey, MAXPROTKEYSIZE);
+	spin_unlock_bh(&ctx->pk_lock);
+
 	while ((nbytes = walk.nbytes) != 0) {
 		/* only use complete blocks */
 		n = nbytes & ~(AES_BLOCK_SIZE - 1);
-		k = cpacf_km(ctx->fc | modifier, ctx->pk.protkey,
+		k = cpacf_km(ctx->fc | modifier, &param,
 			     walk.dst.virt.addr, walk.src.virt.addr, n);
 		if (k)
 			ret = skcipher_walk_done(&walk, nbytes - k);
 		if (k < n) {
-			if (__paes_set_key(ctx) != 0)
+			if (__paes_convert_key(ctx))
 				return skcipher_walk_done(&walk, -EIO);
+			spin_lock_bh(&ctx->pk_lock);
+			memcpy(param.key, ctx->pk.protkey, MAXPROTKEYSIZE);
+			spin_unlock_bh(&ctx->pk_lock);
 		}
 	}
 	return ret;
@@ -214,6 +245,7 @@ static int cbc_paes_init(struct crypto_skcipher *tfm)
 	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
 
 	ctx->kb.key = NULL;
+	spin_lock_init(&ctx->pk_lock);
 
 	return 0;
 }
@@ -225,11 +257,11 @@ static void cbc_paes_exit(struct crypto_skcipher *tfm)
 	_free_kb_keybuf(&ctx->kb);
 }
 
-static int __cbc_paes_set_key(struct s390_paes_ctx *ctx)
+static inline int __cbc_paes_set_key(struct s390_paes_ctx *ctx)
 {
 	unsigned long fc;
 
-	if (__paes_convert_key(&ctx->kb, &ctx->pk))
+	if (__paes_convert_key(ctx))
 		return -EINVAL;
 
 	/* Pick the correct function code based on the protected key type */
@@ -276,8 +308,12 @@ static int cbc_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 	ret = skcipher_walk_virt(&walk, req, false);
 	if (ret)
 		return ret;
+
 	memcpy(param.iv, walk.iv, AES_BLOCK_SIZE);
+	spin_lock_bh(&ctx->pk_lock);
 	memcpy(param.key, ctx->pk.protkey, MAXPROTKEYSIZE);
+	spin_unlock_bh(&ctx->pk_lock);
+
 	while ((nbytes = walk.nbytes) != 0) {
 		/* only use complete blocks */
 		n = nbytes & ~(AES_BLOCK_SIZE - 1);
@@ -288,9 +324,11 @@ static int cbc_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 			ret = skcipher_walk_done(&walk, nbytes - k);
 		}
 		if (k < n) {
-			if (__cbc_paes_set_key(ctx) != 0)
+			if (__paes_convert_key(ctx))
 				return skcipher_walk_done(&walk, -EIO);
+			spin_lock_bh(&ctx->pk_lock);
 			memcpy(param.key, ctx->pk.protkey, MAXPROTKEYSIZE);
+			spin_unlock_bh(&ctx->pk_lock);
 		}
 	}
 	return ret;
@@ -330,6 +368,7 @@ static int xts_paes_init(struct crypto_skcipher *tfm)
 
 	ctx->kb[0].key = NULL;
 	ctx->kb[1].key = NULL;
+	spin_lock_init(&ctx->pk_lock);
 
 	return 0;
 }
@@ -342,12 +381,27 @@ static void xts_paes_exit(struct crypto_skcipher *tfm)
 	_free_kb_keybuf(&ctx->kb[1]);
 }
 
-static int __xts_paes_set_key(struct s390_pxts_ctx *ctx)
+static inline int __xts_paes_convert_key(struct s390_pxts_ctx *ctx)
+{
+	struct pkey_protkey pkey0, pkey1;
+
+	if (__paes_keyblob2pkey(&ctx->kb[0], &pkey0) ||
+	    __paes_keyblob2pkey(&ctx->kb[1], &pkey1))
+		return -EINVAL;
+
+	spin_lock_bh(&ctx->pk_lock);
+	memcpy(&ctx->pk[0], &pkey0, sizeof(pkey0));
+	memcpy(&ctx->pk[1], &pkey1, sizeof(pkey1));
+	spin_unlock_bh(&ctx->pk_lock);
+
+	return 0;
+}
+
+static inline int __xts_paes_set_key(struct s390_pxts_ctx *ctx)
 {
 	unsigned long fc;
 
-	if (__paes_convert_key(&ctx->kb[0], &ctx->pk[0]) ||
-	    __paes_convert_key(&ctx->kb[1], &ctx->pk[1]))
+	if (__xts_paes_convert_key(ctx))
 		return -EINVAL;
 
 	if (ctx->pk[0].type != ctx->pk[1].type)
@@ -425,15 +479,17 @@ static int xts_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 	ret = skcipher_walk_virt(&walk, req, false);
 	if (ret)
 		return ret;
+
 	keylen = (ctx->pk[0].type == PKEY_KEYTYPE_AES_128) ? 48 : 64;
 	offset = (ctx->pk[0].type == PKEY_KEYTYPE_AES_128) ? 16 : 0;
-retry:
+
 	memset(&pcc_param, 0, sizeof(pcc_param));
 	memcpy(pcc_param.tweak, walk.iv, sizeof(pcc_param.tweak));
+	spin_lock_bh(&ctx->pk_lock);
 	memcpy(pcc_param.key + offset, ctx->pk[1].protkey, keylen);
-	cpacf_pcc(ctx->fc, pcc_param.key + offset);
-
 	memcpy(xts_param.key + offset, ctx->pk[0].protkey, keylen);
+	spin_unlock_bh(&ctx->pk_lock);
+	cpacf_pcc(ctx->fc, pcc_param.key + offset);
 	memcpy(xts_param.init, pcc_param.xts, 16);
 
 	while ((nbytes = walk.nbytes) != 0) {
@@ -444,11 +500,15 @@ static int xts_paes_crypt(struct skcipher_request *req, unsigned long modifier)
 		if (k)
 			ret = skcipher_walk_done(&walk, nbytes - k);
 		if (k < n) {
-			if (__xts_paes_set_key(ctx) != 0)
+			if (__xts_paes_convert_key(ctx))
 				return skcipher_walk_done(&walk, -EIO);
-			goto retry;
+			spin_lock_bh(&ctx->pk_lock);
+			memcpy(xts_param.key + offset,
+			       ctx->pk[0].protkey, keylen);
+			spin_unlock_bh(&ctx->pk_lock);
 		}
 	}
+
 	return ret;
 }
 
@@ -485,6 +545,7 @@ static int ctr_paes_init(struct crypto_skcipher *tfm)
 	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
 
 	ctx->kb.key = NULL;
+	spin_lock_init(&ctx->pk_lock);
 
 	return 0;
 }
@@ -496,11 +557,11 @@ static void ctr_paes_exit(struct crypto_skcipher *tfm)
 	_free_kb_keybuf(&ctx->kb);
 }
 
-static int __ctr_paes_set_key(struct s390_paes_ctx *ctx)
+static inline int __ctr_paes_set_key(struct s390_paes_ctx *ctx)
 {
 	unsigned long fc;
 
-	if (__paes_convert_key(&ctx->kb, &ctx->pk))
+	if (__paes_convert_key(ctx))
 		return -EINVAL;
 
 	/* Pick the correct function code based on the protected key type */
@@ -556,45 +617,61 @@ static int ctr_paes_crypt(struct skcipher_request *req)
 	struct skcipher_walk walk;
 	unsigned int nbytes, n, k;
 	int ret, locked;
-
-	locked = spin_trylock(&ctrblk_lock);
+	struct {
+		u8 key[MAXPROTKEYSIZE];
+	} param;
 
 	ret = skcipher_walk_virt(&walk, req, false);
+	if (ret)
+		return ret;
+
+	spin_lock_bh(&ctx->pk_lock);
+	memcpy(param.key, ctx->pk.protkey, MAXPROTKEYSIZE);
+	spin_unlock_bh(&ctx->pk_lock);
+
+	locked = mutex_trylock(&ctrblk_lock);
+
 	while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
 		n = AES_BLOCK_SIZE;
 		if (nbytes >= 2*AES_BLOCK_SIZE && locked)
 			n = __ctrblk_init(ctrblk, walk.iv, nbytes);
 		ctrptr = (n > AES_BLOCK_SIZE) ? ctrblk : walk.iv;
-		k = cpacf_kmctr(ctx->fc, ctx->pk.protkey, walk.dst.virt.addr,
+		k = cpacf_kmctr(ctx->fc, &param, walk.dst.virt.addr,
 				walk.src.virt.addr, n, ctrptr);
 		if (k) {
 			if (ctrptr == ctrblk)
 				memcpy(walk.iv, ctrptr + k - AES_BLOCK_SIZE,
 				       AES_BLOCK_SIZE);
 			crypto_inc(walk.iv, AES_BLOCK_SIZE);
-			ret = skcipher_walk_done(&walk, nbytes - n);
+			ret = skcipher_walk_done(&walk, nbytes - k);
 		}
 		if (k < n) {
-			if (__ctr_paes_set_key(ctx) != 0) {
+			if (__paes_convert_key(ctx)) {
 				if (locked)
-					spin_unlock(&ctrblk_lock);
+					mutex_unlock(&ctrblk_lock);
 				return skcipher_walk_done(&walk, -EIO);
 			}
+			spin_lock_bh(&ctx->pk_lock);
+			memcpy(param.key, ctx->pk.protkey, MAXPROTKEYSIZE);
+			spin_unlock_bh(&ctx->pk_lock);
 		}
 	}
 	if (locked)
-		spin_unlock(&ctrblk_lock);
+		mutex_unlock(&ctrblk_lock);
 	/*
 	 * final block may be < AES_BLOCK_SIZE, copy only nbytes
 	 */
 	if (nbytes) {
 		while (1) {
-			if (cpacf_kmctr(ctx->fc, ctx->pk.protkey, buf,
+			if (cpacf_kmctr(ctx->fc, &param, buf,
 					walk.src.virt.addr, AES_BLOCK_SIZE,
 					walk.iv) == AES_BLOCK_SIZE)
 				break;
-			if (__ctr_paes_set_key(ctx) != 0)
+			if (__paes_convert_key(ctx))
 				return skcipher_walk_done(&walk, -EIO);
+			spin_lock_bh(&ctx->pk_lock);
+			memcpy(param.key, ctx->pk.protkey, MAXPROTKEYSIZE);
+			spin_unlock_bh(&ctx->pk_lock);
 		}
 		memcpy(walk.dst.virt.addr, buf, nbytes);
 		crypto_inc(walk.iv, AES_BLOCK_SIZE);
@@ -674,14 +751,14 @@ static int __init paes_s390_init(void)
 	if (cpacf_test_func(&kmctr_functions, CPACF_KMCTR_PAES_128) ||
 	    cpacf_test_func(&kmctr_functions, CPACF_KMCTR_PAES_192) ||
 	    cpacf_test_func(&kmctr_functions, CPACF_KMCTR_PAES_256)) {
-		ret = crypto_register_skcipher(&ctr_paes_alg);
-		if (ret)
-			goto out_err;
 		ctrblk = (u8 *) __get_free_page(GFP_KERNEL);
 		if (!ctrblk) {
 			ret = -ENOMEM;
 			goto out_err;
 		}
+		ret = crypto_register_skcipher(&ctr_paes_alg);
+		if (ret)
+			goto out_err;
 	}
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] crypto/testmgr: add selftests for paes-s390
  2019-11-13 10:55 [PATCH 0/3] provide paes selftests Harald Freudenberger
  2019-11-13 10:55 ` [PATCH 1/3] s390/pkey: Add support for key blob with clear key value Harald Freudenberger
  2019-11-13 10:55 ` [PATCH 2/3] s390/crypto: Rework on paes implementation Harald Freudenberger
@ 2019-11-13 10:55 ` Harald Freudenberger
  2019-11-22  8:16   ` Herbert Xu
  2020-02-13  7:40   ` [PATCH] " Harald Freudenberger
  2 siblings, 2 replies; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-13 10:55 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: ebiggers, heiko.carstens, gor, Harald Freudenberger

This patch adds selftests for the s390 specific protected key
AES (PAES) cipher implementations:
  * cbc-paes-s390
  * ctr-paes-s390
  * ecb-paes-s390
  * xts-paes-s390
PAES is an AES cipher but with encrypted ('protected') key
material. So here come ordinary AES enciphered data values
but with a special key format understood by the PAES
implementation.

The testdata definitons and testlist entries are surrounded
by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
make any sense on non s390 platforms or without the PAES
cipher implementation.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 crypto/testmgr.c |  36 +++++
 crypto/testmgr.h | 334 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 370 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 82084f6d84b6..6bda67bdc662 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4104,6 +4104,15 @@ static const struct alg_test_desc alg_test_descs[] = {
 			.cipher = __VECS(tf_cbc_tv_template)
 		},
 	}, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+		.alg = "cbc-paes-s390",
+		.fips_allowed = 1,
+		.test = alg_test_skcipher,
+		.suite = {
+			.cipher = __VECS(paes_cbc_tv_template)
+		}
+	}, {
+#endif
 		.alg = "cbcmac(aes)",
 		.fips_allowed = 1,
 		.test = alg_test_hash,
@@ -4252,6 +4261,15 @@ static const struct alg_test_desc alg_test_descs[] = {
 			.cipher = __VECS(tf_ctr_tv_template)
 		}
 	}, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+		.alg = "ctr-paes-s390",
+		.fips_allowed = 1,
+		.test = alg_test_skcipher,
+		.suite = {
+			.cipher = __VECS(paes_ctr_tv_template)
+		}
+	}, {
+#endif
 		.alg = "cts(cbc(aes))",
 		.test = alg_test_skcipher,
 		.fips_allowed = 1,
@@ -4538,6 +4556,15 @@ static const struct alg_test_desc alg_test_descs[] = {
 			.cipher = __VECS(xtea_tv_template)
 		}
 	}, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+		.alg = "ecb-paes-s390",
+		.fips_allowed = 1,
+		.test = alg_test_skcipher,
+		.suite = {
+			.cipher = __VECS(paes_ecb_tv_template)
+		}
+	}, {
+#endif
 		.alg = "ecdh",
 		.test = alg_test_kpp,
 		.fips_allowed = 1,
@@ -5109,6 +5136,15 @@ static const struct alg_test_desc alg_test_descs[] = {
 			.cipher = __VECS(tf_xts_tv_template)
 		}
 	}, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+		.alg = "xts-paes-s390",
+		.fips_allowed = 1,
+		.test = alg_test_skcipher,
+		.suite = {
+			.cipher = __VECS(paes_xts_tv_template)
+		}
+	}, {
+#endif
 		.alg = "xts4096(paes)",
 		.test = alg_test_null,
 		.fips_allowed = 1,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index d1d89101f1b5..14c594e8fbed 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -31942,4 +31942,338 @@ static const struct aead_testvec essiv_hmac_sha256_aes_cbc_tv_temp[] = {
 	},
 };
 
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+static const struct cipher_testvec paes_cbc_tv_template[] = {
+	{
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20",
+		.klen	= 16 + 16,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\x29\x4a\x3a\x21\x51\xbf\x9e\x57"
+			  "\xd2\x79\x5b\xb1\x91\x4f\x54\x59",
+		.ptext	= "\x48\x61\x74\x20\x64\x65\x72\x20"
+			  "\x61\x6c\x74\x65\x20\x48\x65\x78",
+		.ctext	= "\x29\x4a\x3a\x21\x51\xbf\x9e\x57"
+			  "\xd2\x79\x5b\xb1\x91\x4f\x54\x59",
+		.len	= 16,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20",
+		.klen	= 16 + 16,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\x6c\xfd\xd0\xce\x12\x53\xf5\x01"
+			  "\x9d\x87\xe6\xb1\x32\x72\xfd\x31",
+		.ptext	= "\x65\x6e\x6d\x65\x69\x73\x74\x65"
+			  "\x72\x0a\x53\x69\x63\x68\x20\x64"
+			  "\x6f\x63\x68\x20\x65\x69\x6e\x6d"
+			  "\x61\x6c\x20\x77\x65\x67\x62\x65",
+		.ctext	= "\x58\xd0\x61\x98\x33\xf5\xee\x3e"
+			  "\xcd\xe8\x2f\x8b\x05\x30\x3d\xf0"
+			  "\x6c\xfd\xd0\xce\x12\x53\xf5\x01"
+			  "\x9d\x87\xe6\xb1\x32\x72\xfd\x31",
+		.len	= 32,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x02\x00\x00\x00\x18"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x76\x6f\x6e\x20\x47\x6f\x65\x74",
+		.klen	= 16 + 24,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\xfa\xce\xf8\x56\x00\x06\x07\xd0"
+			  "\x80\xca\xd1\x92\x8b\x72\x3a\x37",
+		.ptext	= "\x67\x65\x62\x65\x6e\x21\x0a\x55"
+			  "\x6e\x64\x20\x6e\x75\x6e\x20\x73"
+			  "\x6f\x6c\x6c\x65\x6e\x20\x73\x65"
+			  "\x69\x6e\x65\x20\x47\x65\x69\x73"
+			  "\x74\x65\x72\x0a\x41\x75\x63\x68"
+			  "\x20\x6e\x61\x63\x68\x20\x6d\x65",
+		.ctext	= "\xf9\x64\x0b\x55\x99\x95\xbb\x84"
+			  "\xd2\x4e\x12\x63\xff\xde\x1d\x34"
+			  "\x39\xdd\x05\x33\x70\x8d\x43\x3f"
+			  "\xf8\xf2\x52\x00\xbd\xce\x35\x73"
+			  "\xfa\xce\xf8\x56\x00\x06\x07\xd0"
+			  "\x80\xca\xd1\x92\x8b\x72\x3a\x37",
+		.len	= 48,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x03\x00\x00\x00\x20"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x76\x6f\x6e\x20\x47\x6f\x65\x74"
+			  "\x68\x65\x0a\x00\x00\x00\x00\x00",
+		.klen	= 16 + 32,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\x2b\xea\x25\x71\x32\x2b\xe7\x5b"
+			  "\xdd\x59\xb9\xfc\x3b\x34\x50\x62",
+		.ptext	= "\x69\x6e\x65\x6d\x20\x57\x69\x6c"
+			  "\x6c\x65\x6e\x20\x6c\x65\x62\x65"
+			  "\x6e\x2e\x0a\x53\x65\x69\x6e\x65"
+			  "\x20\x57\x6f\x72\x74\x20\x75\x6e"
+			  "\x64\x20\x57\x65\x72\x6b\x65\x0a"
+			  "\x4d\x65\x72\x6b\x74\x20\x69\x63"
+			  "\x68\x20\x75\x6e\x64\x20\x64\x65"
+			  "\x6e\x20\x42\x72\x61\x75\x63\x68",
+		.ctext	= "\x04\x5b\x92\x57\xe4\xec\x01\xb8"
+			  "\x92\xd9\xba\x28\x1e\xe9\xc0\xbd"
+			  "\x9b\xa2\x08\xc2\xe1\x59\x34\xd9"
+			  "\xc8\x30\x66\xfb\x28\x06\xb0\xac"
+			  "\x37\x18\x1d\x82\xb5\xd4\x0c\xa0"
+			  "\x80\x40\x2c\x51\x5d\x07\xc6\xe9"
+			  "\x2b\xea\x25\x71\x32\x2b\xe7\x5b"
+			  "\xdd\x59\xb9\xfc\x3b\x34\x50\x62",
+		.len	= 64,
+	},
+};
+
+static const struct cipher_testvec paes_ctr_tv_template[] = {
+	{
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20",
+		.klen	= 16 + 16,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x01",
+		.ptext	= "\x2c\x0a\x55\x6e\x64\x20\x6d\x69"
+			  "\x74\x20\x47\x65\x69\x73\x74\x65",
+		.ctext	= "\x10\xfd\x9c\x75\x74\x7d\xa1\x49"
+			  "\xef\xc9\x90\xa8\x24\x43\x8d\xee",
+		.len	= 16,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20",
+		.klen	= 16 + 16,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x02",
+		.ptext	= "\x73\x73\x74\xc3\xa4\x72\x6b\x65"
+			  "\x0a\x54\x75\x20\x69\x63\x68\x20"
+			  "\x57\x75\x6e\x64\x65\x72\x20\x61"
+			  "\x75\x63\x68\x2e\x0a\x57\x61\x6c",
+		.ctext	= "\x4f\x84\xbd\xd8\xb4\x2f\xa7\x45"
+			  "\x91\xbd\xa2\xed\x24\x53\x91\xab"
+			  "\x93\xbf\xe3\xce\xca\x97\x10\xaf"
+			  "\x01\x50\x8a\xf8\x9f\x6c\x9c\x6c",
+		.len	= 32,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x02\x00\x00\x00\x18"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x76\x6f\x6e\x20\x47\x6f\x65\x74",
+		.klen	= 16 + 24,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x03",
+		.ptext	= "\x6c\x65\x21\x20\x77\x61\x6c\x6c"
+			  "\x65\x0a\x4d\x61\x6e\x63\x68\x65"
+			  "\x20\x53\x74\x72\x65\x63\x6b\x65"
+			  "\x2c\x0a\x44\x61\xc3\x9f\x2c\x20"
+			  "\x7a\x75\x6d\x20\x5a\x77\x65\x63"
+			  "\x6b\x65\x2c\x0a\x57\x61\x73\x73",
+		.ctext	= "\xfa\x8a\x69\xe9\xb6\x0e\x33\x38"
+			  "\x7a\x9b\x3a\x1d\x0e\xad\xb9\xc5"
+			  "\x17\xc2\x8c\x5c\x0a\xa7\x6f\xf2"
+			  "\x22\x81\xdd\x72\xf0\x69\x4a\xea"
+			  "\x20\xb8\x8d\x3f\x4a\xf1\x96\xd9"
+			  "\x45\x96\x87\x64\xae\xa4\x1c\xe0",
+		.len	= 48,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x03\x00\x00\x00\x20"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x76\x6f\x6e\x20\x47\x6f\x65\x74"
+			  "\x68\x65\x0a\x00\x00\x00\x00\x00",
+		.klen	= 16 + 32,
+		.iv	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x00",
+		.iv_out	= "\x5a\x61\x75\x62\x65\x72\x6c\x65"
+			  "\x68\x72\x6c\x69\x6e\x67\x00\x04",
+		.ptext	= "\x65\x72\x20\x66\x6c\x69\x65\xc3"
+			  "\x9f\x65\x0a\x55\x6e\x64\x20\x6d"
+			  "\x69\x74\x20\x72\x65\x69\x63\x68"
+			  "\x65\x6d\x2c\x20\x76\x6f\x6c\x6c"
+			  "\x65\x6d\x20\x53\x63\x68\x77\x61"
+			  "\x6c\x6c\x65\x0a\x5a\x75\x20\x64"
+			  "\x65\x6d\x20\x42\x61\x64\x65\x20"
+			  "\x73\x69\x63\x68\x20\x65\x72\x67",
+		.ctext	= "\x7d\x37\x30\x05\x86\x89\x6c\x68"
+			  "\xb9\xb1\xda\x71\xc7\x4d\x8e\x64"
+			  "\x0b\x46\xdf\x0c\xd2\x25\x1f\x8f"
+			  "\xd6\xc2\xd7\x35\x98\x6b\x31\x67"
+			  "\xc0\x46\x69\x34\xc2\x3f\x2a\x13"
+			  "\xc2\x4a\xbf\x5c\xe0\x6f\x92\x38"
+			  "\x7b\xa5\x46\xc0\x70\x96\xa9\x09"
+			  "\xe4\x03\x10\xbc\x28\xe7\xe9\xf8",
+		.len	= 64,
+	},
+};
+
+static const struct cipher_testvec paes_ecb_tv_template[] = {
+	{
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20",
+		.klen	= 16 + 16,
+		.ptext	= "\x69\x65\xc3\x9f\x65\x2e\x0a\x55"
+			  "\x6e\x64\x20\x6e\x75\x6e\x20\x6b",
+		.ctext	= "\x22\xb1\xc6\x20\x52\x27\x69\xbc"
+			  "\xac\x96\x6b\x39\xe7\x5f\x3c\x5e",
+		.len	= 16,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20",
+		.klen	= 16 + 16,
+		.ptext	= "\x6f\x6d\x6d\x2c\x20\x64\x75\x20"
+			  "\x61\x6c\x74\x65\x72\x20\x42\x65"
+			  "\x73\x65\x6e\x2c\x0a\x4e\x69\x6d"
+			  "\x6d\x20\x64\x69\x65\x20\x73\x63",
+		.ctext	= "\x74\x37\x97\x02\x47\xe8\x3f\xfd"
+			  "\x06\xd4\x7f\x92\xe6\x86\xd5\xdd"
+			  "\x20\x05\x7b\xa8\x5a\xe4\x55\x68"
+			  "\xfe\x4e\x7e\x01\x47\x92\xc5\x9c",
+		.len	= 32,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x02\x00\x00\x00\x18"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x76\x6f\x6e\x20\x47\x6f\x65\x74",
+		.klen	= 16 + 24,
+		.ptext	= "\x68\x6c\x65\x63\x68\x74\x65\x6e"
+			  "\x20\x4c\x75\x6d\x70\x65\x6e\x68"
+			  "\xc3\xbc\x6c\x6c\x65\x6e\x21\x0a"
+			  "\x42\x69\x73\x74\x20\x73\x63\x68"
+			  "\x6f\x6e\x20\x6c\x61\x6e\x67\x65"
+			  "\x20\x4b\x6e\x65\x63\x68\x74\x20",
+		.ctext	= "\x2c\xd6\xa7\x25\x94\x16\x47\xb0"
+			  "\xad\x9f\x05\x17\x72\xa5\x5a\x82"
+			  "\xcd\x0f\x01\xaa\x39\xd6\x52\x1e"
+			  "\x8d\x3d\x87\x0e\xdd\x67\xa0\xb7"
+			  "\xbc\x9e\xd6\xf4\x11\xad\x94\x21"
+			  "\xd2\x95\x44\xca\x76\xfd\x75\xc2",
+		.len	= 48,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x03\x00\x00\x00\x20"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x76\x6f\x6e\x20\x47\x6f\x65\x74"
+			  "\x68\x65\x0a\x00\x00\x00\x00\x00",
+		.klen	= 16 + 32,
+		.ptext	= "\x67\x65\x77\x65\x73\x65\x6e\x3a"
+			  "\x0a\x4e\x75\x6e\x20\x65\x72\x66"
+			  "\xc3\xbc\x6c\x6c\x65\x20\x6d\x65"
+			  "\x69\x6e\x65\x6e\x20\x57\x69\x6c"
+			  "\x6c\x65\x6e\x21\x0a\x41\x75\x66"
+			  "\x20\x7a\x77\x65\x69\x20\x42\x65"
+			  "\x69\x6e\x65\x6e\x20\x73\x74\x65"
+			  "\x68\x65\x2c\x0a\x4f\x62\x65\x6e",
+		.ctext	= "\x93\x33\xa3\xb0\xa5\xda\x3d\x2b"
+			  "\xf1\xde\x19\xb4\xe5\xa6\x89\xa9"
+			  "\x3f\xaa\x25\x69\x2c\x54\xd1\x0d"
+			  "\xaa\x6b\xd2\x91\xfb\x6d\xb7\x6c"
+			  "\xad\xa6\xf5\x0a\x20\x24\x5a\x1e"
+			  "\x41\x01\xc4\x45\x4a\x4b\xf9\x26"
+			  "\xf0\xb3\x42\xfb\x5a\xe4\x55\x4c"
+			  "\xea\x2b\xd8\x31\x60\x6e\xef\xe9",
+		.len	= 64,
+	},
+};
+
+static const struct cipher_testvec paes_xts_tv_template[] = {
+	{
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x44\x65\x72\x20\x5a\x61\x75\x62"
+			  "\x65\x72\x6c\x65\x68\x72\x6c\x69",
+		.klen	= 2 * (16 + 16),
+		.iv	= "\x31\x37\x34\x39\x2d\x31\x38\x33"
+			  "\x32\x00\x00\x00\x00\x00\x00\x00",
+		.ptext	= "\x20\x73\x65\x69\x20\x65\x69\x6e"
+			  "\x20\x4b\x6f\x70\x66\x2c\x0a\x45",
+		.ctext	= "\x53\xc8\x2f\x3f\x95\x06\x19\x08"
+			  "\xc5\xe0\xf4\x4a\xeb\x00\x50\xd9",
+		.len	= 16,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x01\x00\x00\x00\x10"
+			  "\x44\x65\x72\x20\x5a\x61\x75\x62"
+			  "\x65\x72\x6c\x65\x68\x72\x6c\x69",
+		.klen	= 2 * (16 + 16),
+		.iv	= "\x31\x37\x34\x39\x2d\x31\x38\x33"
+			  "\x32\x00\x00\x00\x00\x00\x00\x00",
+		.ptext	= "\x69\x6c\x65\x20\x6e\x75\x6e\x20"
+			  "\x75\x6e\x64\x20\x67\x65\x68\x65"
+			  "\x0a\x4d\x69\x74\x20\x64\x65\x6d"
+			  "\x20\x57\x61\x73\x73\x65\x72\x74",
+		.ctext	= "\xe4\x08\xdc\x74\xba\xa2\xb2\x09"
+			  "\xe4\x1a\x34\x81\xb6\x65\xe7\x7a"
+			  "\x51\xe3\x88\x36\xb8\xb1\x23\xfe"
+			  "\x83\x21\xd8\x2e\x9d\x78\xfb\x06",
+		.len	= 32,
+	}, {
+		.key	= "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x03\x00\x00\x00\x20"
+			  "\x4a\x6f\x68\x61\x6e\x6e\x20\x57"
+			  "\x6f\x6c\x66\x67\x61\x6e\x67\x20"
+			  "\x76\x6f\x6e\x20\x47\x6f\x65\x74"
+			  "\x68\x65\x00\x00\x00\x00\x00\x00"
+			  "\x00\x00\x00\x00\x02\x00\x00\x00"
+			  "\x00\x00\x00\x03\x00\x00\x00\x20"
+			  "\x44\x65\x72\x20\x5a\x61\x75\x62"
+			  "\x65\x72\x6c\x65\x68\x72\x6c\x69"
+			  "\x6e\x67\x00\x00\x00\x00\x00\x00"
+			  "\x00\x00\x00\x00\x00\x00\x00\x00",
+		.klen	= 2 * (16 + 32),
+		.iv	= "\x31\x37\x34\x39\x2d\x31\x38\x33"
+			  "\x32\x00\x00\x00\x00\x00\x00\x00",
+		.ptext	= "\x6f\x70\x66\x21\x0a\x57\x61\x6c"
+			  "\x6c\x65\x21\x20\x77\x61\x6c\x6c"
+			  "\x65\x0a\x4d\x61\x6e\x63\x68\x65"
+			  "\x20\x53\x74\x72\x65\x63\x6b\x65"
+			  "\x2c\x0a\x44\x61\xc3\x9f\x2c\x20"
+			  "\x7a\x75\x6d\x20\x5a\x77\x65\x63"
+			  "\x6b\x65\x2c\x0a\x57\x61\x73\x73"
+			  "\x65\x72\x20\x66\x6c\x69\x65\xc3",
+		.ctext	= "\x04\x02\x88\xed\xe0\xce\x13\xad"
+			  "\x9b\x9d\x7f\x10\xdf\x7d\x7b\xc5"
+			  "\xf0\xab\x35\x07\x1a\xfa\xdd\x33"
+			  "\xf1\x10\x52\x1e\x1f\x6b\x41\x3a"
+			  "\xef\xae\xbb\xd9\xf0\x1d\x1b\x7b"
+			  "\x2c\x77\xee\x18\xe4\x51\x18\xc5"
+			  "\x99\xb6\x83\x3b\x5f\xae\x14\x6e"
+			  "\xc9\x22\x9c\x0a\xaa\x2f\x0a\xe1",
+		.len	= 64,
+	},
+};
+#endif /* CONFIG_CRYPTO_PAES_S390 */
+
 #endif	/* _CRYPTO_TESTMGR_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] s390/crypto: Rework on paes implementation
  2019-11-13 10:55 ` [PATCH 2/3] s390/crypto: Rework on paes implementation Harald Freudenberger
@ 2019-11-22  8:13   ` Herbert Xu
  2019-11-22  9:54     ` Harald Freudenberger
  0 siblings, 1 reply; 18+ messages in thread
From: Herbert Xu @ 2019-11-22  8:13 UTC (permalink / raw)
  To: Harald Freudenberger; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On Wed, Nov 13, 2019 at 11:55:22AM +0100, Harald Freudenberger wrote:
>
> @@ -129,6 +128,7 @@ static int ecb_paes_init(struct crypto_skcipher *tfm)
>  	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
>  
>  	ctx->kb.key = NULL;
> +	spin_lock_init(&ctx->pk_lock);

This makes no sense.  The context is per-tfm, and each tfm should
have a single key at any time.  The synchronisation of setkey vs.
crypto operations is left to the user of the tfm, not the
implementor.

So why do you need this spin lock at all?

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] crypto/testmgr: add selftests for paes-s390
  2019-11-13 10:55 ` [PATCH 3/3] crypto/testmgr: add selftests for paes-s390 Harald Freudenberger
@ 2019-11-22  8:16   ` Herbert Xu
  2019-11-22  9:11     ` Harald Freudenberger
  2020-01-31 11:06     ` Harald Freudenberger
  2020-02-13  7:40   ` [PATCH] " Harald Freudenberger
  1 sibling, 2 replies; 18+ messages in thread
From: Herbert Xu @ 2019-11-22  8:16 UTC (permalink / raw)
  To: Harald Freudenberger; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On Wed, Nov 13, 2019 at 11:55:23AM +0100, Harald Freudenberger wrote:
> This patch adds selftests for the s390 specific protected key
> AES (PAES) cipher implementations:
>   * cbc-paes-s390
>   * ctr-paes-s390
>   * ecb-paes-s390
>   * xts-paes-s390
> PAES is an AES cipher but with encrypted ('protected') key
> material. So here come ordinary AES enciphered data values
> but with a special key format understood by the PAES
> implementation.
> 
> The testdata definitons and testlist entries are surrounded
> by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
> make any sense on non s390 platforms or without the PAES
> cipher implementation.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> ---
>  crypto/testmgr.c |  36 +++++
>  crypto/testmgr.h | 334 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 370 insertions(+)

So with your cleartext work, I gather that you can now supply
arbitrary keys to paes? If so my preferred method of testing it
would be to add a paes-specific tester function that massaged the
existing aes vectors into the format required by paes so you
get exactly the same testing coverage as plain aes.

Is this possible?

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] crypto/testmgr: add selftests for paes-s390
  2019-11-22  8:16   ` Herbert Xu
@ 2019-11-22  9:11     ` Harald Freudenberger
  2019-11-26  8:59       ` Herbert Xu
  2020-01-31 11:06     ` Harald Freudenberger
  1 sibling, 1 reply; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-22  9:11 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On 22.11.19 09:16, Herbert Xu wrote:
> On Wed, Nov 13, 2019 at 11:55:23AM +0100, Harald Freudenberger wrote:
>> This patch adds selftests for the s390 specific protected key
>> AES (PAES) cipher implementations:
>>   * cbc-paes-s390
>>   * ctr-paes-s390
>>   * ecb-paes-s390
>>   * xts-paes-s390
>> PAES is an AES cipher but with encrypted ('protected') key
>> material. So here come ordinary AES enciphered data values
>> but with a special key format understood by the PAES
>> implementation.
>>
>> The testdata definitons and testlist entries are surrounded
>> by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
>> make any sense on non s390 platforms or without the PAES
>> cipher implementation.
>>
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>> ---
>>  crypto/testmgr.c |  36 +++++
>>  crypto/testmgr.h | 334 +++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 370 insertions(+)
> So with your cleartext work, I gather that you can now supply
> arbitrary keys to paes? If so my preferred method of testing it
> would be to add a paes-specific tester function that massaged the
> existing aes vectors into the format required by paes so you
> get exactly the same testing coverage as plain aes.
>
> Is this possible?
>
> Thanks,
Thanks for your feedback.
I thought about this too. But it would require to implement own versions of
alg_test_skcipher() and test_skcipher() and test_skcipher_vs_generic_impl()
and that's a lot of complicated code unique for paes within testmgr.c
I'd like to avoid.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] s390/crypto: Rework on paes implementation
  2019-11-22  8:13   ` Herbert Xu
@ 2019-11-22  9:54     ` Harald Freudenberger
  2019-11-22 10:42       ` Herbert Xu
  0 siblings, 1 reply; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-22  9:54 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On 22.11.19 09:13, Herbert Xu wrote:
> On Wed, Nov 13, 2019 at 11:55:22AM +0100, Harald Freudenberger wrote:
>> @@ -129,6 +128,7 @@ static int ecb_paes_init(struct crypto_skcipher *tfm)
>>  	struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm);
>>  
>>  	ctx->kb.key = NULL;
>> +	spin_lock_init(&ctx->pk_lock);
> This makes no sense.  The context is per-tfm, and each tfm should
> have a single key at any time.  The synchronisation of setkey vs.
> crypto operations is left to the user of the tfm, not the
> implementor.
>
> So why do you need this spin lock at all?
>
> Cheers,
The setkey() sets the base key material (usually a secure key) to an
tfm instance. From this key a 'protected key' (pkey) is derived which
may get invalid at any time and may need to get re-derived from the
base key material.
An tfm instance may be shared, so the context where the pkey is
stored into is also shared. So when a pkey gets invalid there is a need
to update the pkey value within the context struct. This update needs
to be done atomic as another thread may concurrently use this pkey
value. That's all what this spinlock does. Make sure read and write
operations on the pkey within the context are atomic.
It is still possible that two threads copy the pkey, try to use it, find out
that it is invalid and needs refresh, re-derive and both update the pkey
memory serialized by the spinlock. But this is no issue. The spinlock
makes sure the stored pkey is always a consistent pkey (which may
be valid or invalid but not corrupted).

Hope this makes it clear

Thanks


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] s390/crypto: Rework on paes implementation
  2019-11-22  9:54     ` Harald Freudenberger
@ 2019-11-22 10:42       ` Herbert Xu
  2019-11-22 13:38         ` Harald Freudenberger
  0 siblings, 1 reply; 18+ messages in thread
From: Herbert Xu @ 2019-11-22 10:42 UTC (permalink / raw)
  To: Harald Freudenberger; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On Fri, Nov 22, 2019 at 10:54:50AM +0100, Harald Freudenberger wrote:
> The setkey() sets the base key material (usually a secure key) to an
> tfm instance. From this key a 'protected key' (pkey) is derived which
> may get invalid at any time and may need to get re-derived from the
> base key material.
> An tfm instance may be shared, so the context where the pkey is
> stored into is also shared. So when a pkey gets invalid there is a need
> to update the pkey value within the context struct. This update needs
> to be done atomic as another thread may concurrently use this pkey
> value. That's all what this spinlock does. Make sure read and write
> operations on the pkey within the context are atomic.
> It is still possible that two threads copy the pkey, try to use it, find out
> that it is invalid and needs refresh, re-derive and both update the pkey
> memory serialized by the spinlock. But this is no issue. The spinlock
> makes sure the stored pkey is always a consistent pkey (which may
> be valid or invalid but not corrupted).

OK.  Can you give me a bit more background info on how often
this is likely to happen? I mean it happened every time you
might as well not store the protected key in the tfm at all.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] s390/crypto: Rework on paes implementation
  2019-11-22 10:42       ` Herbert Xu
@ 2019-11-22 13:38         ` Harald Freudenberger
  2019-11-22 14:07           ` Herbert Xu
  0 siblings, 1 reply; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-22 13:38 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On 22.11.19 11:42, Herbert Xu wrote:
> On Fri, Nov 22, 2019 at 10:54:50AM +0100, Harald Freudenberger wrote:
>> The setkey() sets the base key material (usually a secure key) to an
>> tfm instance. From this key a 'protected key' (pkey) is derived which
>> may get invalid at any time and may need to get re-derived from the
>> base key material.
>> An tfm instance may be shared, so the context where the pkey is
>> stored into is also shared. So when a pkey gets invalid there is a need
>> to update the pkey value within the context struct. This update needs
>> to be done atomic as another thread may concurrently use this pkey
>> value. That's all what this spinlock does. Make sure read and write
>> operations on the pkey within the context are atomic.
>> It is still possible that two threads copy the pkey, try to use it, find out
>> that it is invalid and needs refresh, re-derive and both update the pkey
>> memory serialized by the spinlock. But this is no issue. The spinlock
>> makes sure the stored pkey is always a consistent pkey (which may
>> be valid or invalid but not corrupted).
> OK.  Can you give me a bit more background info on how often
> this is likely to happen? I mean it happened every time you
> might as well not store the protected key in the tfm at all.
>
> Thanks,
The pkey is in fact a encrypted key + a verification pattern for the
encrypted key used. It gets invalid when this encryption key changes.
The encryption key changes when the LPAR is re-activated so for
example on suspend/resume or an Linux running as kvm guest
gets relocated. So this happens very rarely.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] s390/crypto: Rework on paes implementation
  2019-11-22 13:38         ` Harald Freudenberger
@ 2019-11-22 14:07           ` Herbert Xu
  2019-11-22 14:45             ` Harald Freudenberger
  0 siblings, 1 reply; 18+ messages in thread
From: Herbert Xu @ 2019-11-22 14:07 UTC (permalink / raw)
  To: Harald Freudenberger; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On Fri, Nov 22, 2019 at 02:38:30PM +0100, Harald Freudenberger wrote:
>
> The pkey is in fact a encrypted key + a verification pattern for the
> encrypted key used. It gets invalid when this encryption key changes.
> The encryption key changes when the LPAR is re-activated so for
> example on suspend/resume or an Linux running as kvm guest
> gets relocated. So this happens very rarely.

I see.  Is there any way of you finding out that the key has been
invalidated apart from trying out the crypto and having it fail?

Ideally you'd have a global counter that gets incremented everytime
an invalidation occurs.  You can then regenerate your key if its
generation counter differs from the current global counter.

Also when the crypto fails due to an invalid key you're currently
calling skcipher_walk_done with zero.  This is wrong as the done
function must be called with a positive value or an error.  In
some cases this can cause a crash in scatterwalk.

IOW you should just repeat the crypto operation after regenerating
the key rather than looping around again.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] s390/crypto: Rework on paes implementation
  2019-11-22 14:07           ` Herbert Xu
@ 2019-11-22 14:45             ` Harald Freudenberger
  0 siblings, 0 replies; 18+ messages in thread
From: Harald Freudenberger @ 2019-11-22 14:45 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On 22.11.19 15:07, Herbert Xu wrote:
> On Fri, Nov 22, 2019 at 02:38:30PM +0100, Harald Freudenberger wrote:
>> The pkey is in fact a encrypted key + a verification pattern for the
>> encrypted key used. It gets invalid when this encryption key changes.
>> The encryption key changes when the LPAR is re-activated so for
>> example on suspend/resume or an Linux running as kvm guest
>> gets relocated. So this happens very rarely.
> I see.  Is there any way of you finding out that the key has been
> invalidated apart from trying out the crypto and having it fail?
No. By using the pkey for a crypto operation the hardware
checks the verification pattern and if there is a mismatch
it simple rejects the operation. Theoretically such an operation
can only partly complete and then a pkey could get invalid.
I have no way to check if the pkey is still valid before the
cpacf instruction call.
>
> Ideally you'd have a global counter that gets incremented everytime
> an invalidation occurs.  You can then regenerate your key if its
> generation counter differs from the current global counter.
>
> Also when the crypto fails due to an invalid key you're currently
> calling skcipher_walk_done with zero.  This is wrong as the done
> function must be called with a positive value or an error.  In
> some cases this can cause a crash in scatterwalk.
>
> IOW you should just repeat the crypto operation after regenerating
> the key rather than looping around again.
That's right. I'll try to rework the functions this way to
avoid calling skciper_walk_done with 0.

Thanks
>
> Cheers,


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] crypto/testmgr: add selftests for paes-s390
  2019-11-22  9:11     ` Harald Freudenberger
@ 2019-11-26  8:59       ` Herbert Xu
  0 siblings, 0 replies; 18+ messages in thread
From: Herbert Xu @ 2019-11-26  8:59 UTC (permalink / raw)
  To: Harald Freudenberger; +Cc: linux-crypto, ebiggers, heiko.carstens, gor

On Fri, Nov 22, 2019 at 10:11:30AM +0100, Harald Freudenberger wrote:
>
> I thought about this too. But it would require to implement own versions of
> alg_test_skcipher() and test_skcipher() and test_skcipher_vs_generic_impl()
> and that's a lot of complicated code unique for paes within testmgr.c
> I'd like to avoid.

I don't think you have to do test_skcipher_vs_generic_impl right
away.  Just supporting the normal test vectors should be the same
as your current patch with the advantage that any changes to the
generic test vectors will be picked up automatically for paes.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] crypto/testmgr: add selftests for paes-s390
  2019-11-22  8:16   ` Herbert Xu
  2019-11-22  9:11     ` Harald Freudenberger
@ 2020-01-31 11:06     ` Harald Freudenberger
  2020-02-10  7:19       ` Harald Freudenberger
  1 sibling, 1 reply; 18+ messages in thread
From: Harald Freudenberger @ 2020-01-31 11:06 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, heiko.carstens, Vasily Gorbik

On 22.11.19 09:16, Herbert Xu wrote:
> On Wed, Nov 13, 2019 at 11:55:23AM +0100, Harald Freudenberger wrote:
>> This patch adds selftests for the s390 specific protected key
>> AES (PAES) cipher implementations:
>>   * cbc-paes-s390
>>   * ctr-paes-s390
>>   * ecb-paes-s390
>>   * xts-paes-s390
>> PAES is an AES cipher but with encrypted ('protected') key
>> material. So here come ordinary AES enciphered data values
>> but with a special key format understood by the PAES
>> implementation.
>>
>> The testdata definitons and testlist entries are surrounded
>> by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
>> make any sense on non s390 platforms or without the PAES
>> cipher implementation.
>>
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>> ---
>>  crypto/testmgr.c |  36 +++++
>>  crypto/testmgr.h | 334 +++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 370 insertions(+)
> So with your cleartext work, I gather that you can now supply
> arbitrary keys to paes? If so my preferred method of testing it
> would be to add a paes-specific tester function that massaged the
> existing aes vectors into the format required by paes so you
> get exactly the same testing coverage as plain aes.
>
> Is this possible?
>
> Thanks,


So here is now a reworked version of the paes selftest invocation within the testmanager code.
I picked your suggestions and now the paes ciphers are able to deal with plain aes key values
and so can benefit from the generic aes testcases.
Please note, this patch needs as a prerequirement some other patches which enable the
base functionality in the zcyrpt device driver and the pkey kernel module. These patches
will come with the next s390 subsystem merge for the 5.6 development kernel. If you agree
to this patch, then Vasily will push this patch with the s390 subsystem together as part of the patch
series.

Thanks and here is the patch for the testmanager:

============================================================

From fb82ea49910b8cde33ca7286c8855c0326e78177 Mon Sep 17 00:00:00 2001
From: Harald Freudenberger <freude@linux.ibm.com>
Date: Wed, 22 Jan 2020 14:43:23 +0100
Subject: [PATCH] crypto/testmgr: enable selftests for paes-s390 ciphers

This patch enables the selftests for the s390 specific protected key
AES (PAES) cipher implementations:
  * cbc-paes-s390
  * ctr-paes-s390
  * ecb-paes-s390
  * xts-paes-s390
PAES is an AES cipher but with encrypted ('protected') key
material. However, the paes ciphers are able to derive an protected
key from clear key material with the help of the pkey kernel module.

So this patch now enables the generic AES tests for the paes
ciphers. Under the hood the setkey() functions rearrange the clear key
values as clear key token and so the pkey kernel module is able to
provide protected key blobs from the given clear key values. The
derived protected key blobs are then used within the paes cipers and
should produce the very same results as the generic AES implementation
with the clear key values.

The s390-paes cipher testlist entries are surrounded
by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
make any sense on non s390 platforms or without the PAES
cipher implementation.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com>
Acked-by: Vasily Gorbik <gor@linux.ibm.com>
---
 crypto/testmgr.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 82513b6b0abd..6c4a98102825 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4156,6 +4156,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(tf_cbc_tv_template)
         },
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "cbc-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_cbc_tv_template)
+        }
+    }, {
+#endif
         .alg = "cbcmac(aes)",
         .fips_allowed = 1,
         .test = alg_test_hash,
@@ -4304,6 +4313,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(tf_ctr_tv_template)
         }
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "ctr-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_ctr_tv_template)
+        }
+    }, {
+#endif
         .alg = "cts(cbc(aes))",
         .test = alg_test_skcipher,
         .fips_allowed = 1,
@@ -4596,6 +4614,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(xtea_tv_template)
         }
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "ecb-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_tv_template)
+        }
+    }, {
+#endif
         .alg = "ecdh",
         .test = alg_test_kpp,
         .fips_allowed = 1,
@@ -5167,6 +5194,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(tf_xts_tv_template)
         }
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "xts-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_xts_tv_template)
+        }
+    }, {
+#endif
         .alg = "xts4096(paes)",
         .test = alg_test_null,
         .fips_allowed = 1,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] crypto/testmgr: add selftests for paes-s390
  2020-01-31 11:06     ` Harald Freudenberger
@ 2020-02-10  7:19       ` Harald Freudenberger
  2020-02-11  2:38         ` Herbert Xu
  0 siblings, 1 reply; 18+ messages in thread
From: Harald Freudenberger @ 2020-02-10  7:19 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, heiko.carstens, Vasily Gorbik

Hello Herbert

sorry for my pressing ... but I received questions from a distro about if they can pick
this. And the pre requirement is to have this upstream accepted. So will you accept this
and it will appear in the 5.6 kernel or do you want me to do some rework ?

Thanks

On 31.01.20 12:06, Harald Freudenberger wrote:
> On 22.11.19 09:16, Herbert Xu wrote:
>> On Wed, Nov 13, 2019 at 11:55:23AM +0100, Harald Freudenberger wrote:
>>> This patch adds selftests for the s390 specific protected key
>>> AES (PAES) cipher implementations:
>>>   * cbc-paes-s390
>>>   * ctr-paes-s390
>>>   * ecb-paes-s390
>>>   * xts-paes-s390
>>> PAES is an AES cipher but with encrypted ('protected') key
>>> material. So here come ordinary AES enciphered data values
>>> but with a special key format understood by the PAES
>>> implementation.
>>>
>>> The testdata definitons and testlist entries are surrounded
>>> by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
>>> make any sense on non s390 platforms or without the PAES
>>> cipher implementation.
>>>
>>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>>> ---
>>>  crypto/testmgr.c |  36 +++++
>>>  crypto/testmgr.h | 334 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 370 insertions(+)
>> So with your cleartext work, I gather that you can now supply
>> arbitrary keys to paes? If so my preferred method of testing it
>> would be to add a paes-specific tester function that massaged the
>> existing aes vectors into the format required by paes so you
>> get exactly the same testing coverage as plain aes.
>>
>> Is this possible?
>>
>> Thanks,
>
> So here is now a reworked version of the paes selftest invocation within the testmanager code.
> I picked your suggestions and now the paes ciphers are able to deal with plain aes key values
> and so can benefit from the generic aes testcases.
> Please note, this patch needs as a prerequirement some other patches which enable the
> base functionality in the zcyrpt device driver and the pkey kernel module. These patches
> will come with the next s390 subsystem merge for the 5.6 development kernel. If you agree
> to this patch, then Vasily will push this patch with the s390 subsystem together as part of the patch
> series.
>
> Thanks and here is the patch for the testmanager:
>
> ============================================================
>
> From fb82ea49910b8cde33ca7286c8855c0326e78177 Mon Sep 17 00:00:00 2001
> From: Harald Freudenberger <freude@linux.ibm.com>
> Date: Wed, 22 Jan 2020 14:43:23 +0100
> Subject: [PATCH] crypto/testmgr: enable selftests for paes-s390 ciphers
>
> This patch enables the selftests for the s390 specific protected key
> AES (PAES) cipher implementations:
>   * cbc-paes-s390
>   * ctr-paes-s390
>   * ecb-paes-s390
>   * xts-paes-s390
> PAES is an AES cipher but with encrypted ('protected') key
> material. However, the paes ciphers are able to derive an protected
> key from clear key material with the help of the pkey kernel module.
>
> So this patch now enables the generic AES tests for the paes
> ciphers. Under the hood the setkey() functions rearrange the clear key
> values as clear key token and so the pkey kernel module is able to
> provide protected key blobs from the given clear key values. The
> derived protected key blobs are then used within the paes cipers and
> should produce the very same results as the generic AES implementation
> with the clear key values.
>
> The s390-paes cipher testlist entries are surrounded
> by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
> make any sense on non s390 platforms or without the PAES
> cipher implementation.
>
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com>
> Acked-by: Vasily Gorbik <gor@linux.ibm.com>
> ---
>  crypto/testmgr.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index 82513b6b0abd..6c4a98102825 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -4156,6 +4156,15 @@ static const struct alg_test_desc alg_test_descs[] = {
>              .cipher = __VECS(tf_cbc_tv_template)
>          },
>      }, {
> +#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
> +        .alg = "cbc-paes-s390",
> +        .fips_allowed = 1,
> +        .test = alg_test_skcipher,
> +        .suite = {
> +            .cipher = __VECS(aes_cbc_tv_template)
> +        }
> +    }, {
> +#endif
>          .alg = "cbcmac(aes)",
>          .fips_allowed = 1,
>          .test = alg_test_hash,
> @@ -4304,6 +4313,15 @@ static const struct alg_test_desc alg_test_descs[] = {
>              .cipher = __VECS(tf_ctr_tv_template)
>          }
>      }, {
> +#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
> +        .alg = "ctr-paes-s390",
> +        .fips_allowed = 1,
> +        .test = alg_test_skcipher,
> +        .suite = {
> +            .cipher = __VECS(aes_ctr_tv_template)
> +        }
> +    }, {
> +#endif
>          .alg = "cts(cbc(aes))",
>          .test = alg_test_skcipher,
>          .fips_allowed = 1,
> @@ -4596,6 +4614,15 @@ static const struct alg_test_desc alg_test_descs[] = {
>              .cipher = __VECS(xtea_tv_template)
>          }
>      }, {
> +#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
> +        .alg = "ecb-paes-s390",
> +        .fips_allowed = 1,
> +        .test = alg_test_skcipher,
> +        .suite = {
> +            .cipher = __VECS(aes_tv_template)
> +        }
> +    }, {
> +#endif
>          .alg = "ecdh",
>          .test = alg_test_kpp,
>          .fips_allowed = 1,
> @@ -5167,6 +5194,15 @@ static const struct alg_test_desc alg_test_descs[] = {
>              .cipher = __VECS(tf_xts_tv_template)
>          }
>      }, {
> +#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
> +        .alg = "xts-paes-s390",
> +        .fips_allowed = 1,
> +        .test = alg_test_skcipher,
> +        .suite = {
> +            .cipher = __VECS(aes_xts_tv_template)
> +        }
> +    }, {
> +#endif
>          .alg = "xts4096(paes)",
>          .test = alg_test_null,
>          .fips_allowed = 1,


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] crypto/testmgr: add selftests for paes-s390
  2020-02-10  7:19       ` Harald Freudenberger
@ 2020-02-11  2:38         ` Herbert Xu
  0 siblings, 0 replies; 18+ messages in thread
From: Herbert Xu @ 2020-02-11  2:38 UTC (permalink / raw)
  To: Harald Freudenberger; +Cc: linux-crypto, heiko.carstens, Vasily Gorbik

On Mon, Feb 10, 2020 at 08:19:25AM +0100, Harald Freudenberger wrote:
> Hello Herbert
> 
> sorry for my pressing ... but I received questions from a distro about if they can pick
> this. And the pre requirement is to have this upstream accepted. So will you accept this
> and it will appear in the 5.6 kernel or do you want me to do some rework ?

Hi Harald:

It's too late for 5.6 I'm afraid.  However, if you can get them
submitted right away then 5.7 is certainly not a problem.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] crypto/testmgr: add selftests for paes-s390
  2019-11-13 10:55 ` [PATCH 3/3] crypto/testmgr: add selftests for paes-s390 Harald Freudenberger
  2019-11-22  8:16   ` Herbert Xu
@ 2020-02-13  7:40   ` Harald Freudenberger
  2020-02-13  8:39     ` Herbert Xu
  1 sibling, 1 reply; 18+ messages in thread
From: Harald Freudenberger @ 2020-02-13  7:40 UTC (permalink / raw)
  To: linux-crypto, Herbert Xu; +Cc: Heiko Carstens, Vasily Gorbik

This patch enables the selftests for the s390 specific protected key
AES (PAES) cipher implementations:
  * cbc-paes-s390
  * ctr-paes-s390
  * ecb-paes-s390
  * xts-paes-s390
PAES is an AES cipher but with encrypted ('protected') key
material. However, the paes ciphers are able to derive an protected
key from clear key material with the help of the pkey kernel module.

So this patch now enables the generic AES tests for the paes
ciphers. Under the hood the setkey() functions rearrange the clear key
values as clear key token and so the pkey kernel module is able to
provide protected key blobs from the given clear key values. The
derived protected key blobs are then used within the paes cipers and
should produce the very same results as the generic AES implementation
with the clear key values.

The s390-paes cipher testlist entries are surrounded
by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
make any sense on non s390 platforms or without the PAES
cipher implementation.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
---
 crypto/testmgr.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 82513b6b0abd..6c4a98102825 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4156,6 +4156,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(tf_cbc_tv_template)
         },
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "cbc-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_cbc_tv_template)
+        }
+    }, {
+#endif
         .alg = "cbcmac(aes)",
         .fips_allowed = 1,
         .test = alg_test_hash,
@@ -4304,6 +4313,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(tf_ctr_tv_template)
         }
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "ctr-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_ctr_tv_template)
+        }
+    }, {
+#endif
         .alg = "cts(cbc(aes))",
         .test = alg_test_skcipher,
         .fips_allowed = 1,
@@ -4596,6 +4614,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(xtea_tv_template)
         }
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "ecb-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_tv_template)
+        }
+    }, {
+#endif
         .alg = "ecdh",
         .test = alg_test_kpp,
         .fips_allowed = 1,
@@ -5167,6 +5194,15 @@ static const struct alg_test_desc alg_test_descs[] = {
             .cipher = __VECS(tf_xts_tv_template)
         }
     }, {
+#if IS_ENABLED(CONFIG_CRYPTO_PAES_S390)
+        .alg = "xts-paes-s390",
+        .fips_allowed = 1,
+        .test = alg_test_skcipher,
+        .suite = {
+            .cipher = __VECS(aes_xts_tv_template)
+        }
+    }, {
+#endif
         .alg = "xts4096(paes)",
         .test = alg_test_null,
         .fips_allowed = 1,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] crypto/testmgr: add selftests for paes-s390
  2020-02-13  7:40   ` [PATCH] " Harald Freudenberger
@ 2020-02-13  8:39     ` Herbert Xu
  0 siblings, 0 replies; 18+ messages in thread
From: Herbert Xu @ 2020-02-13  8:39 UTC (permalink / raw)
  To: Harald Freudenberger; +Cc: linux-crypto, Heiko Carstens, Vasily Gorbik

On Thu, Feb 13, 2020 at 08:40:06AM +0100, Harald Freudenberger wrote:
> This patch enables the selftests for the s390 specific protected key
> AES (PAES) cipher implementations:
>   * cbc-paes-s390
>   * ctr-paes-s390
>   * ecb-paes-s390
>   * xts-paes-s390
> PAES is an AES cipher but with encrypted ('protected') key
> material. However, the paes ciphers are able to derive an protected
> key from clear key material with the help of the pkey kernel module.
> 
> So this patch now enables the generic AES tests for the paes
> ciphers. Under the hood the setkey() functions rearrange the clear key
> values as clear key token and so the pkey kernel module is able to
> provide protected key blobs from the given clear key values. The
> derived protected key blobs are then used within the paes cipers and
> should produce the very same results as the generic AES implementation
> with the clear key values.
> 
> The s390-paes cipher testlist entries are surrounded
> by #if IS_ENABLED(CONFIG_CRYPTO_PAES_S390) because they don't
> make any sense on non s390 platforms or without the PAES
> cipher implementation.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
> ---
>  crypto/testmgr.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-02-13  8:39 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-13 10:55 [PATCH 0/3] provide paes selftests Harald Freudenberger
2019-11-13 10:55 ` [PATCH 1/3] s390/pkey: Add support for key blob with clear key value Harald Freudenberger
2019-11-13 10:55 ` [PATCH 2/3] s390/crypto: Rework on paes implementation Harald Freudenberger
2019-11-22  8:13   ` Herbert Xu
2019-11-22  9:54     ` Harald Freudenberger
2019-11-22 10:42       ` Herbert Xu
2019-11-22 13:38         ` Harald Freudenberger
2019-11-22 14:07           ` Herbert Xu
2019-11-22 14:45             ` Harald Freudenberger
2019-11-13 10:55 ` [PATCH 3/3] crypto/testmgr: add selftests for paes-s390 Harald Freudenberger
2019-11-22  8:16   ` Herbert Xu
2019-11-22  9:11     ` Harald Freudenberger
2019-11-26  8:59       ` Herbert Xu
2020-01-31 11:06     ` Harald Freudenberger
2020-02-10  7:19       ` Harald Freudenberger
2020-02-11  2:38         ` Herbert Xu
2020-02-13  7:40   ` [PATCH] " Harald Freudenberger
2020-02-13  8:39     ` Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).