All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/8] crypto: AF_ALG support for KPP
@ 2017-04-18 23:03 Stephan Müller
  2017-04-18 23:04 ` [PATCH 1/8] crypto: AF_ALG -- add DH keygen / ssgen API Stephan Müller
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:03 UTC (permalink / raw)
  To: linux-crypto, keyrings

Hi,

This is an RFC to discuss whether and how to support access to the
KPP kernel crypto API (Diffie-Hellman and EC Diffie-Hellman) by
user space.

The patch set is only meant for discussion and not for production use.
Testing is only performed for the DH part via [1]. The DH testing shows
that the DH operation shows the same results as OpenSSL.

The patch set includes:

- addition of PKCS#3 support for DH to allow domain parameters to be set

- extension of the setsockopt and cmsg API

- addition of AF_ALG KPP interface where the implementation follows the
  exact same coding style as currently discussed for the memory management
  fix patch set for algif_skcipher and algif_aead -- this shall allow
  the immediate incorporation of patches that replace duplicated code with
  common service functions.

The patch 8 describes the different operations that are supported by AF_ALG
KPP. This support includes generation and retaining of the private key
inside the kernel. This private key would never be sent to user space.

I am aware and in fact supported development of the DH support in the keys
subsystem. The question will be raised whether the AF_ALG KPP interface is
superfluous in light of the keys DH support. My answer is that AF_ALG KPP is
orthogonal to the keys DH support. The keys DH support use case is that
the keys are managed inside the kernel and thus has the focus on the
key management. Contrary, AF_ALG KPP does not really focus on key management
but simply externalizes the DH/ECDH support found in the kernel including
hardware acceleration. User space is in full control of the key life cycle.
This way, AF_ALG could be used to complement user-space network protocol
implementations like TLS or IKE to offload the resource intense DH
calculations to accelerators.

A full description of the DH testing and how to compare it to OpenSSL
is present in [1] with the test/kcapi-main.c tool, function "kpp".

[1] https://github.com/smuellerDD/libkcapi/tree/kpp

Stephan Mueller (8):
  crypto: AF_ALG -- add DH keygen / ssgen API
  crypto: AF_ALG -- add DH param / ECDH curve setsockopt call
  crypto: AF_ALG - eliminate code duplication
  crypto: KPP - add API crypto_kpp_set_params
  crypto: DH - allow params and key to be set independently
  crypto: DH - add PKCS#3 parameter handling
  crypto: ecdh - constify key
  crypto: AF_ALG - add KPP support

 Documentation/crypto/api-kpp.rst |    2 +-
 crypto/Kconfig                   |   10 +
 crypto/Makefile                  |    8 +-
 crypto/af_alg.c                  |   17 +-
 crypto/algif_kpp.c               | 1230 ++++++++++++++++++++++++++++++++++++++
 crypto/dh.c                      |   21 +-
 crypto/dh_helper.c               |   53 +-
 crypto/dhparameter.asn1          |    4 +
 include/crypto/dh.h              |   21 +-
 include/crypto/ecdh.h            |    2 +-
 include/crypto/if_alg.h          |    2 +
 include/crypto/kpp.h             |   28 +
 include/uapi/linux/if_alg.h      |    4 +
 13 files changed, 1384 insertions(+), 18 deletions(-)
 create mode 100644 crypto/algif_kpp.c
 create mode 100644 crypto/dhparameter.asn1

-- 
2.9.3

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

* [PATCH 1/8] crypto: AF_ALG -- add DH keygen / ssgen API
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
@ 2017-04-18 23:04 ` Stephan Müller
  2017-04-18 23:05 ` [PATCH 2/8] crypto: AF_ALG -- add DH param / ECDH curve setsockopt call Stephan Müller
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:04 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

Add the flags for handling DH key generation and DH shared
secret generation.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 include/uapi/linux/if_alg.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index 02e6162..c48eeb6 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -41,5 +41,7 @@ struct af_alg_iv {
 #define ALG_OP_ENCRYPT			1
 #define ALG_OP_SIGN			2
 #define ALG_OP_VERIFY			3
+#define ALG_OP_KEYGEN			4
+#define ALG_OP_SSGEN			5
 
 #endif	/* _LINUX_IF_ALG_H */
-- 
2.9.3

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

* [PATCH 2/8] crypto: AF_ALG -- add DH param / ECDH curve setsockopt  call
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
  2017-04-18 23:04 ` [PATCH 1/8] crypto: AF_ALG -- add DH keygen / ssgen API Stephan Müller
@ 2017-04-18 23:05 ` Stephan Müller
  2017-04-18 23:05 ` [PATCH 3/8] crypto: AF_ALG - eliminate code duplication Stephan Müller
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:05 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

For supporting DH ciphers, user space must be able to set the
DH parameters. The patch adds a new setsockopt call for setting
these parameters.

Similarly, the ECDH curve information can be set by user space via the
newly added setsockopt call.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/af_alg.c             | 12 ++++++++++++
 include/crypto/if_alg.h     |  2 ++
 include/uapi/linux/if_alg.h |  2 ++
 3 files changed, 16 insertions(+)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 0a91404..a3210bf 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,18 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 
 		err = alg_setkey(sk, optval, optlen, type->setpubkey);
 		break;
+	case ALG_SET_DH_PARAMETERS:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+
+		err = alg_setkey(sk, optval, optlen, type->dhparams);
+		break;
+	case ALG_SET_ECDH_CURVE:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+
+		err = alg_setkey(sk, optval, optlen, type->ecdhcurve);
+		break;
 	case ALG_SET_AEAD_AUTHSIZE:
 		if (sock->state == SS_CONNECTED)
 			goto unlock;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 3c3ebe3..52dcefe 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -53,6 +53,8 @@ struct af_alg_type {
 	void (*release)(void *private);
 	int (*setkey)(void *private, const u8 *key, unsigned int keylen);
 	int (*setpubkey)(void *private, const u8 *key, unsigned int keylen);
+	int (*dhparams)(void *private, const u8 *param, unsigned int paramlen);
+	int (*ecdhcurve)(void *private, const u8 *param, unsigned int paramlen);
 	int (*accept)(void *private, struct sock *sk);
 	int (*accept_nokey)(void *private, struct sock *sk);
 	int (*setauthsize)(void *private, unsigned int authsize);
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index c48eeb6..fc9ed9f 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,8 @@ struct af_alg_iv {
 #define ALG_SET_AEAD_ASSOCLEN		4
 #define ALG_SET_AEAD_AUTHSIZE		5
 #define ALG_SET_PUBKEY			6
+#define ALG_SET_DH_PARAMETERS		7
+#define ALG_SET_ECDH_CURVE		8
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.9.3

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

* [PATCH 3/8] crypto: AF_ALG - eliminate code duplication
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
  2017-04-18 23:04 ` [PATCH 1/8] crypto: AF_ALG -- add DH keygen / ssgen API Stephan Müller
  2017-04-18 23:05 ` [PATCH 2/8] crypto: AF_ALG -- add DH param / ECDH curve setsockopt call Stephan Müller
@ 2017-04-18 23:05 ` Stephan Müller
  2017-04-18 23:06 ` [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params Stephan Müller
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:05 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

The handling function for setsockopt contains duplicated code which is
cleaned up with this patch. This patch does not change the functionality.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/af_alg.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a3210bf..f2b2865 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -247,34 +247,23 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 	if (level != SOL_ALG || !type)
 		goto unlock;
 
+	if (sock->state == SS_CONNECTED)
+		goto unlock;
+
 	switch (optname) {
 	case ALG_SET_KEY:
-		if (sock->state == SS_CONNECTED)
-			goto unlock;
-
 		err = alg_setkey(sk, optval, optlen, type->setkey);
 		break;
 	case ALG_SET_PUBKEY:
-		if (sock->state == SS_CONNECTED)
-			goto unlock;
-
 		err = alg_setkey(sk, optval, optlen, type->setpubkey);
 		break;
 	case ALG_SET_DH_PARAMETERS:
-		if (sock->state == SS_CONNECTED)
-			goto unlock;
-
 		err = alg_setkey(sk, optval, optlen, type->dhparams);
 		break;
 	case ALG_SET_ECDH_CURVE:
-		if (sock->state == SS_CONNECTED)
-			goto unlock;
-
 		err = alg_setkey(sk, optval, optlen, type->ecdhcurve);
 		break;
 	case ALG_SET_AEAD_AUTHSIZE:
-		if (sock->state == SS_CONNECTED)
-			goto unlock;
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
-- 
2.9.3

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

* [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
                   ` (2 preceding siblings ...)
  2017-04-18 23:05 ` [PATCH 3/8] crypto: AF_ALG - eliminate code duplication Stephan Müller
@ 2017-04-18 23:06 ` Stephan Müller
  2017-04-19 14:51   ` Benedetto, Salvatore
  2017-04-18 23:06 ` [PATCH 5/8] crypto: DH - allow params and key to be set independently Stephan Müller
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:06 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

KPP mechanisms like DH require a parameter set to be provided by the
caller. That parameter set may be provided by the crypto_kpp_set_secret
function. Yet, the parameters hare handled independently from the secret
key which implies that they should be able to be set independently from
the key.

The new API allows KPP mechanisms to register a callback allowing to set
such parameters.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 Documentation/crypto/api-kpp.rst |  2 +-
 include/crypto/kpp.h             | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/crypto/api-kpp.rst b/Documentation/crypto/api-kpp.rst
index 7d86ab9..7b2c0d4 100644
--- a/Documentation/crypto/api-kpp.rst
+++ b/Documentation/crypto/api-kpp.rst
@@ -11,7 +11,7 @@ Key-agreement Protocol Primitives (KPP) Cipher API
    :doc: Generic Key-agreement Protocol Primitives API
 
 .. kernel-doc:: include/crypto/kpp.h
-   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_secret crypto_kpp_generate_public_key crypto_kpp_compute_shared_secret crypto_kpp_maxsize
+   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_params crypto_kpp_set_secret crypto_kpp_generate_public_key crypto_kpp_compute_shared_secret crypto_kpp_maxsize
 
 Key-agreement Protocol Primitives (KPP) Cipher Request Handle
 -------------------------------------------------------------
diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
index ce8e1f7..5931364 100644
--- a/include/crypto/kpp.h
+++ b/include/crypto/kpp.h
@@ -51,6 +51,9 @@ struct crypto_kpp {
 /**
  * struct kpp_alg - generic key-agreement protocol primitives
  *
+ * @set_params:	Function allows the caller to set the parameters
+ *			separately from the key. The format of the parameters
+ *			is protocol specific.
  * @set_secret:		Function invokes the protocol specific function to
  *			store the secret private key along with parameters.
  *			The implementation knows how to decode thie buffer
@@ -74,6 +77,8 @@ struct crypto_kpp {
  * @base:		Common crypto API algorithm data structure
  */
 struct kpp_alg {
+	int (*set_params)(struct crypto_kpp *tfm, const void *buffer,
+			  unsigned int len);
 	int (*set_secret)(struct crypto_kpp *tfm, const void *buffer,
 			  unsigned int len);
 	int (*generate_public_key)(struct kpp_request *req);
@@ -259,6 +264,29 @@ struct kpp_secret {
 };
 
 /**
+ * crypto_kpp_set_params() - Set parameters needed for kpp operation
+ *
+ * Function invokes the specific kpp operation for a given alg.
+ *
+ * @tfm:	tfm handle
+ * @buffer:	Buffer holding the protocol specific representation of the
+ *		parameters (e.g. PKCS#3 DER for DH)
+ * @len:	Length of the parameter buffer.
+ *
+ * Return: zero on success; error code in case of error
+ */
+static inline int crypto_kpp_set_params(struct crypto_kpp *tfm,
+				        const void *buffer, unsigned int len)
+{
+	struct kpp_alg *alg = crypto_kpp_alg(tfm);
+
+	if (alg->set_params)
+		return alg->set_params(tfm, buffer, len);
+	else
+		return -EOPNOTSUPP;
+}
+
+/**
  * crypto_kpp_set_secret() - Invoke kpp operation
  *
  * Function invokes the specific kpp operation for a given alg.
-- 
2.9.3

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

* [PATCH 5/8] crypto: DH - allow params and key to be set independently
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
                   ` (3 preceding siblings ...)
  2017-04-18 23:06 ` [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params Stephan Müller
@ 2017-04-18 23:06 ` Stephan Müller
  2017-04-18 23:06 ` [PATCH 6/8] crypto: DH - add PKCS#3 parameter handling Stephan Müller
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:06 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

Parameters are handled independently from the secret key. Therefore,
this patch allows setting of the parameter independently from the secret
key. Before invoking the actual crypto operation, the code must now
check that the secret key and the parameters are all present.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/dh.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 87e3542..f7be48e 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -60,6 +60,10 @@ static int dh_check_params_length(unsigned int p_len)
 
 static int dh_set_params(struct dh_ctx *ctx, struct dh *params)
 {
+	/* If DH parameters are not given, do not check them. */
+	if (!params->p_size && !params->g_size)
+		return 0;
+
 	if (unlikely(!params->p || !params->g))
 		return -EINVAL;
 
@@ -111,7 +115,7 @@ static int dh_compute_value(struct kpp_request *req)
 	if (!val)
 		return -ENOMEM;
 
-	if (unlikely(!ctx->xa)) {
+	if (unlikely(!ctx->xa || !ctx->p || !ctx->g)) {
 		ret = -EINVAL;
 		goto err_free_val;
 	}
-- 
2.9.3

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

* [PATCH 6/8] crypto: DH - add PKCS#3 parameter handling
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
                   ` (4 preceding siblings ...)
  2017-04-18 23:06 ` [PATCH 5/8] crypto: DH - allow params and key to be set independently Stephan Müller
@ 2017-04-18 23:06 ` Stephan Müller
  2017-04-18 23:07 ` [PATCH 7/8] crypto: ecdh - constify key Stephan Müller
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:06 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

DH parameters are commonly handled in PKCS#3 ASN.1 DER encoded files.
The addition adds support for the parsing of such DER encoded parameter
sets. After parsing, the data is added to the DH context data structure.

This support allows using of parameter sets generated with the openssl
dhparam command.

Note, the privateValueLength parameter defined in PKCS#3 is not parsed
as it is not required for the DH operation.

The PKCS#3 parser is able to parse data created by other tools, such as:

        openssl dhparam -outform DER -out dhparam.der 2048

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/Makefile         |  7 ++++++-
 crypto/dh.c             | 15 ++++++++++++++
 crypto/dh_helper.c      | 53 ++++++++++++++++++++++++++++++++++++++++++++++---
 crypto/dhparameter.asn1 |  4 ++++
 include/crypto/dh.h     | 21 +++++++++++++++++---
 5 files changed, 93 insertions(+), 7 deletions(-)
 create mode 100644 crypto/dhparameter.asn1

diff --git a/crypto/Makefile b/crypto/Makefile
index 8a44057..a25cfdd 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -30,7 +30,12 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o
 obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
 obj-$(CONFIG_CRYPTO_KPP2) += kpp.o
 
-dh_generic-y := dh.o
+$(obj)/dhparameter-asn1.o: $(obj)/dhparameter-asn1.c $(obj)/dhparameter-asn1.h
+$(obj)/dh_helper.o: $(obj)/dhparameter-asn1.h
+clean-files += dhparameter-asn1.c dhparameter-asn1.h
+
+dh_generic-y := dhparameter-asn1.o
+dh_generic-y += dh.o
 dh_generic-y += dh_helper.o
 obj-$(CONFIG_CRYPTO_DH) += dh_generic.o
 ecdh_generic-y := ecc.o
diff --git a/crypto/dh.c b/crypto/dh.c
index f7be48e..a82dceb 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -83,6 +83,20 @@ static int dh_set_params(struct dh_ctx *ctx, struct dh *params)
 	return 0;
 }
 
+static int dh_set_params_pkcs3(struct crypto_kpp *tfm, const void *param,
+			       unsigned int param_len)
+{
+	struct dh_ctx *ctx = dh_get_ctx(tfm);
+	struct dh parsed_params;
+	int ret;
+
+	ret = dh_parse_params_pkcs3(&parsed_params, param, param_len);
+	if (ret)
+		return ret;
+
+	return dh_set_params(ctx, &parsed_params);
+}
+
 static int dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 			 unsigned int len)
 {
@@ -163,6 +177,7 @@ static void dh_exit_tfm(struct crypto_kpp *tfm)
 }
 
 static struct kpp_alg dh = {
+	.set_params = dh_set_params_pkcs3,
 	.set_secret = dh_set_secret,
 	.generate_public_key = dh_compute_value,
 	.compute_shared_secret = dh_compute_value,
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 02db76b..27bffc9 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -13,6 +13,7 @@
 #include <linux/string.h>
 #include <crypto/dh.h>
 #include <crypto/kpp.h>
+#include "dhparameter-asn1.h"
 
 #define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 3 * sizeof(int))
 
@@ -53,13 +54,22 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
 	if (len != crypto_dh_key_len(params))
 		return -EINVAL;
 
+	/* Prevention of out-of-bounds access in decode code path */
+	if ((!params->key && params->key_size) ||
+	    (!params->p && params->p_size) ||
+	    (!params->g && params->g_size))
+		return -EINVAL;
+
 	ptr = dh_pack_data(ptr, &secret, sizeof(secret));
 	ptr = dh_pack_data(ptr, &params->key_size, sizeof(params->key_size));
 	ptr = dh_pack_data(ptr, &params->p_size, sizeof(params->p_size));
 	ptr = dh_pack_data(ptr, &params->g_size, sizeof(params->g_size));
-	ptr = dh_pack_data(ptr, params->key, params->key_size);
-	ptr = dh_pack_data(ptr, params->p, params->p_size);
-	dh_pack_data(ptr, params->g, params->g_size);
+	if (params->key)
+		ptr = dh_pack_data(ptr, params->key, params->key_size);
+	if (params->p)
+		ptr = dh_pack_data(ptr, params->p, params->p_size);
+	if (params->g)
+		dh_pack_data(ptr, params->g, params->g_size);
 
 	return 0;
 }
@@ -93,3 +103,40 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(crypto_dh_decode_key);
+
+int dh_get_p(void *context, size_t hdrlen, unsigned char tag, const void *value,
+	     size_t vlen)
+{
+	struct dh *dh = context;
+
+	/* invalid key provided */
+	if (!value || !vlen)
+		return -EINVAL;
+
+	dh->p = value;
+	dh->p_size = vlen;
+
+	return 0;
+}
+
+int dh_get_g(void *context, size_t hdrlen, unsigned char tag,
+	     const void *value, size_t vlen)
+{
+	struct dh *dh = context;
+
+	/* invalid base provided */
+	if (!value || !dh->p_size || !vlen || vlen > dh->p_size)
+		return -EINVAL;
+
+	dh->g = value;
+	dh->g_size = vlen;
+
+	return 0;
+}
+
+int dh_parse_params_pkcs3(struct dh *dh, const void *param,
+			 unsigned int param_len)
+{
+	return asn1_ber_decoder(&dhparameter_decoder, dh, param, param_len);
+}
+EXPORT_SYMBOL_GPL(dh_parse_params_pkcs3);
diff --git a/crypto/dhparameter.asn1 b/crypto/dhparameter.asn1
new file mode 100644
index 0000000..e9107c8
--- /dev/null
+++ b/crypto/dhparameter.asn1
@@ -0,0 +1,4 @@
+DHParameter ::= SEQUENCE {
+	prime			INTEGER ({ dh_get_p }),
+	base			INTEGER ({ dh_get_g })
+}
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index 6b424ad..ba4988a 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -35,9 +35,9 @@
  * @g_size:	Size of DH generator G
  */
 struct dh {
-	void *key;
-	void *p;
-	void *g;
+	const void *key;
+	const void *p;
+	const void *g;
 	unsigned int key_size;
 	unsigned int p_size;
 	unsigned int g_size;
@@ -84,4 +84,19 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params);
  */
 int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params);
 
+/**
+ * dh_parse_params_pkcs3() - decodes the PKCS#3 BER encoded buffer of the DH
+ *			    parameters and stores in the provided struct dh,
+ *			    pointers to the raw p and g parameters as is, so
+ *			    that the caller can copy it or MPI parse it, etc.
+ *
+ * @dh:		struct dh representation
+ * @param:	DH parameters in BER format following PKCS#3
+ * @param_len:	length of parameter buffer
+ *
+ * Return:	0 on success or error code in case of error
+ */
+int dh_parse_params_pkcs3(struct dh *dh, const void *param,
+			  unsigned int param_len);
+
 #endif
-- 
2.9.3

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

* [PATCH 7/8] crypto: ecdh - constify key
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
                   ` (5 preceding siblings ...)
  2017-04-18 23:06 ` [PATCH 6/8] crypto: DH - add PKCS#3 parameter handling Stephan Müller
@ 2017-04-18 23:07 ` Stephan Müller
  2017-08-28 10:34     ` Tudor Ambarus
  2017-04-18 23:07 ` [PATCH 8/8] crypto: AF_ALG - add KPP support Stephan Müller
  2017-04-19 12:03 ` [RFC PATCH 0/8] crypto: AF_ALG support for KPP Tudor Ambarus
  8 siblings, 1 reply; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:07 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 include/crypto/ecdh.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
index 03a64f6..b5bb149 100644
--- a/include/crypto/ecdh.h
+++ b/include/crypto/ecdh.h
@@ -40,7 +40,7 @@
  */
 struct ecdh {
 	unsigned short curve_id;
-	char *key;
+	const char *key;
 	unsigned short key_size;
 };
 
-- 
2.9.3

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

* [PATCH 8/8] crypto: AF_ALG - add KPP support
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
                   ` (6 preceding siblings ...)
  2017-04-18 23:07 ` [PATCH 7/8] crypto: ecdh - constify key Stephan Müller
@ 2017-04-18 23:07 ` Stephan Müller
  2017-04-19 12:03 ` [RFC PATCH 0/8] crypto: AF_ALG support for KPP Tudor Ambarus
  8 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-18 23:07 UTC (permalink / raw)
  To: linux-crypto; +Cc: keyrings

The patch externalizes the KPP kernel crypto API to user space. This
allows user space to make use of Diffie-Hellman and EC Diffie-Hellman
operations.

The following operations are supported:

 * DH parameters formatted in PKCS#3 with ALG_SET_DH_PARAMETERS
   setsockopt. The call returns the buffer length user space must
   allocate for the shared secret / public key operation.

 * ECDH curve selection via ALG_SET_ECDH_CURVE setsockopt. The call
   returns the buffer length user space must allocate for the shared
   secret / public key operation.

 * The private key can be set with the ALG_SET_KEY setsockopt. It is
   permissible to provide a NULL key. In this case, the kernel uses the
   stdrng to generate a private key of appropriate size and sets the key
   with the TFM. The idea is that the caller can obtain the public key
   from the private key, exchange it with the peer, inject the peer's
   public key into the kernel and derive the shared secret. This way,
   the private key does not leave the kernel realm. The call returns the
   buffer length user space must allocate for the shared secret / public
   key operation.

 * The public key is obtained from the private key via the recvmsg
   operation. For this operation, no data sent to the kernel via sendmsg
   or sendpage is required.

 * The shared secret is obtained by providing the peer's public key via
   sendmsg / sendpage and reading the shared secret via recvmsg.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/Kconfig     |   10 +
 crypto/Makefile    |    1 +
 crypto/algif_kpp.c | 1230 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1241 insertions(+)
 create mode 100644 crypto/algif_kpp.c

diff --git a/crypto/Kconfig b/crypto/Kconfig
index aac4bc9..cf59f76 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1757,6 +1757,16 @@ config CRYPTO_USER_API_AEAD
 	  This option enables the user-spaces interface for AEAD
 	  cipher algorithms.
 
+config CRYPTO_USER_API_KPP
+	tristate "User-space interface for key protocol primitives algorithms"
+	depends on NET
+	select CRYPTO_KPP2
+	select CRYPTO_USER_API
+	help
+	  This option enables the user-spaces interface for key protocol
+	  primitives algorithms. This covers Diffie-Hellman and EC
+	  Diffie-Hellman
+
 config CRYPTO_HASH_INFO
 	bool
 
diff --git a/crypto/Makefile b/crypto/Makefile
index a25cfdd..45d861b 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
 obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
 obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
 obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
+obj-$(CONFIG_CRYPTO_USER_API_KPP) += algif_kpp.o
 
 #
 # generic algorithms and the async_tx api
diff --git a/crypto/algif_kpp.c b/crypto/algif_kpp.c
new file mode 100644
index 0000000..664b517
--- /dev/null
+++ b/crypto/algif_kpp.c
@@ -0,0 +1,1230 @@
+/*
+ * algif_kpp: User-space interface for key protocol primitives algorithms
+ *
+ * Copyright (C) 2017, Stephan Mueller <smueller@chronox.de>
+ *
+ * This file provides the user-space API for key protocol primitives.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. As part of the recvmsg operation, the processed
+ * TX buffers are extracted from the TX SGL into a separate SGL.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The extracted TX SGL parts are released together with
+ * the RX SGL release.
+ */
+
+#include <crypto/kpp.h>
+#include <crypto/dh.h>
+#include <crypto/ecdh.h>
+#include <crypto/rng.h>
+#include <crypto/scatterwalk.h>
+#include <crypto/if_alg.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/kernel.h>
+#include <linux/sched/signal.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <net/sock.h>
+
+struct kpp_tsgl {
+	struct list_head list;
+	unsigned int cur;		/* Last processed SG entry */
+	struct scatterlist sg[0];	/* Array of SGs forming the SGL */
+};
+
+struct kpp_rsgl {
+	struct af_alg_sgl sgl;
+	struct list_head list;
+	size_t sg_num_bytes;		/* Bytes of data in that SGL */
+};
+
+struct kpp_async_req {
+	struct kiocb *iocb;
+	struct sock *sk;
+
+	struct kpp_rsgl first_rsgl;	/* First RX SG */
+	struct list_head rsgl_list;	/* Track RX SGs */
+
+	struct scatterlist *tsgl;	/* priv. TX SGL of buffers to process */
+	unsigned int tsgl_entries;	/* number of entries in priv. TX SGL */
+
+	unsigned int areqlen;		/* Length of this data struct */
+	struct kpp_request req;		/* req ctx trails this struct */
+};
+
+struct kpp_tfm {
+	struct crypto_kpp *kpp;
+	bool has_key;
+
+#define KPP_NO_PARAMS	0
+#define KPP_DH_PARAMS	1
+#define KPP_ECDH_PARAMS	2
+	int has_params;		/* Type of KPP mechanism */
+};
+
+struct kpp_ctx {
+	struct list_head tsgl_list;	/* Link to TX SGL */
+
+	struct af_alg_completion completion;	/* sync work queue */
+
+	unsigned int inflight;	/* Outstanding AIO ops */
+	size_t used;		/* TX bytes sent to kernel */
+	size_t rcvused;		/* total RX bytes to be processed by kernel */
+
+	bool more;		/* More data to be expected? */
+	bool merge;		/* Merge new data into existing SG */
+	int op;			/* Crypto operation: enc, dec, sign, verify */
+
+	unsigned int len;	/* Length of allocated memory for this struct */
+};
+
+static DECLARE_WAIT_QUEUE_HEAD(kpp_aio_finish_wait);
+
+#define MAX_SGL_ENTS ((4096 - sizeof(struct kpp_tsgl)) / \
+		      sizeof(struct scatterlist) - 1)
+
+static inline int kpp_sndbuf(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+
+	return max_t(int, max_t(int, sk->sk_sndbuf & PAGE_MASK, PAGE_SIZE) -
+			  ctx->used, 0);
+}
+
+static inline bool kpp_writable(struct sock *sk)
+{
+	return PAGE_SIZE <= kpp_sndbuf(sk);
+}
+
+static inline int kpp_rcvbuf(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+
+	return max_t(int, max_t(int, sk->sk_rcvbuf & PAGE_MASK, PAGE_SIZE) -
+			  ctx->rcvused, 0);
+}
+
+static inline bool kpp_readable(struct sock *sk)
+{
+	return PAGE_SIZE <= kpp_rcvbuf(sk);
+}
+
+static int kpp_alloc_tsgl(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kpp_tsgl *sgl;
+	struct scatterlist *sg = NULL;
+
+	sgl = list_entry(ctx->tsgl_list.prev, struct kpp_tsgl, list);
+	if (!list_empty(&ctx->tsgl_list))
+		sg = sgl->sg;
+
+	if (!sg || sgl->cur >= MAX_SGL_ENTS) {
+		sgl = sock_kmalloc(sk, sizeof(*sgl) +
+				       sizeof(sgl->sg[0]) * (MAX_SGL_ENTS + 1),
+				   GFP_KERNEL);
+		if (!sgl)
+			return -ENOMEM;
+
+		sg_init_table(sgl->sg, MAX_SGL_ENTS + 1);
+		sgl->cur = 0;
+
+		if (sg)
+			sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
+
+		list_add_tail(&sgl->list, &ctx->tsgl_list);
+	}
+
+	return 0;
+}
+
+static unsigned int kpp_count_tsgl(struct sock *sk, size_t bytes)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kpp_tsgl *sgl, *tmp;
+	unsigned int i;
+	unsigned int sgl_count = 0;
+
+	if (!bytes)
+		return 0;
+
+	list_for_each_entry_safe(sgl, tmp, &ctx->tsgl_list, list) {
+		struct scatterlist *sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			sgl_count++;
+			if (sg[i].length >= bytes)
+				return sgl_count;
+
+			bytes -= sg[i].length;
+		}
+	}
+
+	return sgl_count;
+}
+
+static void kpp_pull_tsgl(struct sock *sk, size_t used,
+			       struct scatterlist *dst)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kpp_tsgl *sgl;
+	struct scatterlist *sg;
+	unsigned int i;
+
+	while (!list_empty(&ctx->tsgl_list)) {
+		sgl = list_first_entry(&ctx->tsgl_list, struct kpp_tsgl,
+				       list);
+		sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			size_t plen = min_t(size_t, used, sg[i].length);
+			struct page *page = sg_page(sg + i);
+
+			if (!page)
+				continue;
+
+			/*
+			 * Assumption: caller created kpp_count_tsgl(len)
+			 * SG entries in dst.
+			 */
+			if (dst)
+				sg_set_page(dst + i, page, plen, sg[i].offset);
+
+			sg[i].length -= plen;
+			sg[i].offset += plen;
+
+			used -= plen;
+			ctx->used -= plen;
+
+			if (sg[i].length)
+				return;
+
+			if (!dst)
+				put_page(page);
+			sg_assign_page(sg + i, NULL);
+		}
+
+		list_del(&sgl->list);
+		sock_kfree_s(sk, sgl, sizeof(*sgl) + sizeof(sgl->sg[0]) *
+						     (MAX_SGL_ENTS + 1));
+	}
+
+	if (!ctx->used)
+		ctx->merge = 0;
+}
+
+static void kpp_free_areq_sgl(struct kpp_async_req *areq)
+{
+	struct sock *sk = areq->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kpp_rsgl *rsgl, *tmp;
+	struct scatterlist *tsgl;
+	struct scatterlist *sg;
+	unsigned int i;
+
+	list_for_each_entry_safe(rsgl, tmp, &areq->rsgl_list, list) {
+		ctx->rcvused -= rsgl->sg_num_bytes;
+		af_alg_free_sg(&rsgl->sgl);
+		list_del(&rsgl->list);
+		if (rsgl != &areq->first_rsgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+	}
+
+	tsgl = areq->tsgl;
+	for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
+		if (!sg_page(sg))
+			continue;
+		put_page(sg_page(sg));
+	}
+
+	if (areq->tsgl && areq->tsgl_entries)
+		sock_kfree_s(sk, tsgl, areq->tsgl_entries * sizeof(*tsgl));
+}
+
+static int kpp_wait_for_wmem(struct sock *sk, unsigned flags)
+{
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);
+	int err = -ERESTARTSYS;
+	long timeout;
+
+	if (flags & MSG_DONTWAIT)
+		return -EAGAIN;
+
+	sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+
+	add_wait_queue(sk_sleep(sk), &wait);
+	for (;;) {
+		if (signal_pending(current))
+			break;
+		timeout = MAX_SCHEDULE_TIMEOUT;
+		if (sk_wait_event(sk, &timeout, kpp_writable(sk), &wait)) {
+			err = 0;
+			break;
+		}
+	}
+	remove_wait_queue(sk_sleep(sk), &wait);
+
+	return err;
+}
+
+static void kpp_wmem_wakeup(struct sock *sk)
+{
+	struct socket_wq *wq;
+
+	if (!kpp_writable(sk))
+		return;
+
+	rcu_read_lock();
+	wq = rcu_dereference(sk->sk_wq);
+	if (skwq_has_sleeper(wq))
+		wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
+							   POLLRDNORM |
+							   POLLRDBAND);
+	sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
+	rcu_read_unlock();
+}
+
+static int kpp_wait_for_data(struct sock *sk, unsigned flags)
+{
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	long timeout;
+	int err = -ERESTARTSYS;
+
+	if (flags & MSG_DONTWAIT) {
+		return -EAGAIN;
+	}
+
+	sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+
+	add_wait_queue(sk_sleep(sk), &wait);
+	for (;;) {
+		if (signal_pending(current))
+			break;
+		timeout = MAX_SCHEDULE_TIMEOUT;
+		if (sk_wait_event(sk, &timeout, ctx->used, &wait)) {
+			err = 0;
+			break;
+		}
+	}
+	remove_wait_queue(sk_sleep(sk), &wait);
+
+	sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+
+	return err;
+}
+
+static void kpp_data_wakeup(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct socket_wq *wq;
+
+	if (!ctx->used)
+		return;
+
+	rcu_read_lock();
+	wq = rcu_dereference(sk->sk_wq);
+	if (skwq_has_sleeper(wq))
+		wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
+							   POLLRDNORM |
+							   POLLRDBAND);
+	sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
+	rcu_read_unlock();
+}
+
+static int kpp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kpp_tsgl *sgl;
+	struct af_alg_control con = {};
+	int copied = 0;
+	int op = 0;
+	bool init = 0;
+	int err = 0;
+
+	if (msg->msg_controllen) {
+		err = af_alg_cmsg_send(msg, &con);
+		if (err)
+			return err;
+
+		init = 1;
+		switch (con.op) {
+		case ALG_OP_KEYGEN:
+		case ALG_OP_SSGEN:
+			op = con.op;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	lock_sock(sk);
+	if (!ctx->more && ctx->used) {
+		err =  -EINVAL;
+		goto unlock;
+	}
+
+	if (init)
+		ctx->op = op;
+
+	while (size) {
+		struct scatterlist *sg;
+		size_t len = size;
+		size_t plen;
+
+		/* use the existing memory in an allocated page */
+		if (ctx->merge) {
+			sgl = list_entry(ctx->tsgl_list.prev,
+					 struct kpp_tsgl, list);
+			sg = sgl->sg + sgl->cur - 1;
+			len = min_t(size_t, len,
+				    PAGE_SIZE - sg->offset - sg->length);
+			err = memcpy_from_msg(page_address(sg_page(sg)) +
+					      sg->offset + sg->length,
+					      msg, len);
+			if (err)
+				goto unlock;
+
+			sg->length += len;
+			ctx->merge = (sg->offset + sg->length) &
+				     (PAGE_SIZE - 1);
+
+			ctx->used += len;
+			copied += len;
+			size -= len;
+			continue;
+		}
+
+		if (!kpp_writable(sk)) {
+			err = kpp_wait_for_wmem(sk, msg->msg_flags);
+			if (err)
+				goto unlock;
+		}
+
+		/* allocate a new page */
+		len = min_t(size_t, size, kpp_sndbuf(sk));
+
+		err = kpp_alloc_tsgl(sk);
+		if (err)
+			goto unlock;
+
+		sgl = list_entry(ctx->tsgl_list.prev, struct kpp_tsgl,
+				 list);
+		sg = sgl->sg;
+		if (sgl->cur)
+			sg_unmark_end(sg + sgl->cur - 1);
+
+		do {
+			unsigned int i = sgl->cur;
+
+			plen = min_t(size_t, len, PAGE_SIZE);
+
+			sg_assign_page(sg + i, alloc_page(GFP_KERNEL));
+			if (!sg_page(sg + i)) {
+				err = -ENOMEM;
+				goto unlock;
+			}
+
+			err = memcpy_from_msg(page_address(sg_page(sg + i)),
+					      msg, plen);
+			if (err) {
+				__free_page(sg_page(sg + i));
+				sg_assign_page(sg + i, NULL);
+				goto unlock;
+			}
+
+			sg[i].length = plen;
+			len -= plen;
+			ctx->used += plen;
+			copied += plen;
+			size -= plen;
+			sgl->cur++;
+		} while (len && sgl->cur < MAX_SGL_ENTS);
+
+		if (!size)
+			sg_mark_end(sg + sgl->cur - 1);
+
+		ctx->merge = plen & (PAGE_SIZE - 1);
+	}
+
+	err = 0;
+
+	ctx->more = msg->msg_flags & MSG_MORE;
+
+unlock:
+	kpp_data_wakeup(sk);
+	release_sock(sk);
+
+	return copied ?: err;
+}
+
+static ssize_t kpp_sendpage(struct socket *sock, struct page *page,
+			    int offset, size_t size, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kpp_tsgl *sgl;
+	int err = -EINVAL;
+
+	if (flags & MSG_SENDPAGE_NOTLAST)
+		flags |= MSG_MORE;
+
+	lock_sock(sk);
+	if (!ctx->more && ctx->used)
+		goto unlock;
+
+	if (!size)
+		goto done;
+
+	if (!kpp_writable(sk)) {
+		err = kpp_wait_for_wmem(sk, flags);
+		if (err)
+			goto unlock;
+	}
+
+	err = kpp_alloc_tsgl(sk);
+	if (err)
+		goto unlock;
+
+	ctx->merge = 0;
+	sgl = list_entry(ctx->tsgl_list.prev, struct kpp_tsgl, list);
+
+	if (sgl->cur)
+		sg_unmark_end(sgl->sg + sgl->cur - 1);
+
+	sg_mark_end(sgl->sg + sgl->cur);
+
+	get_page(page);
+	sg_set_page(sgl->sg + sgl->cur, page, size, offset);
+	sgl->cur++;
+	ctx->used += size;
+
+done:
+	ctx->more = flags & MSG_MORE;
+unlock:
+	kpp_data_wakeup(sk);
+	release_sock(sk);
+
+	return err ?: size;
+}
+
+static void kpp_async_cb(struct crypto_async_request *_req, int err)
+{
+	struct kpp_async_req *areq = _req->data;
+	struct sock *sk = areq->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kiocb *iocb = areq->iocb;
+	unsigned int resultlen;
+
+	lock_sock(sk);
+
+	BUG_ON(!ctx->inflight);
+
+	/* Buffer size written by crypto operation. */
+	resultlen = areq->req.dst_len;
+
+	kpp_free_areq_sgl(areq);
+	sock_kfree_s(sk, areq, areq->areqlen);
+	__sock_put(sk);
+	ctx->inflight--;
+
+	iocb->ki_complete(iocb, err ? err : resultlen, 0);
+
+	release_sock(sk);
+
+	wake_up_interruptible(&kpp_aio_finish_wait);
+}
+
+static int _kpp_recvmsg(struct socket *sock, struct msghdr *msg,
+			     size_t ignored, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct sock *psk = ask->parent;
+	struct alg_sock *pask = alg_sk(psk);
+	struct kpp_ctx *ctx = ask->private;
+	struct kpp_tfm *kpp = pask->private;
+	struct crypto_kpp *tfm = kpp->kpp;
+
+	unsigned int areqlen = sizeof(struct kpp_async_req) +
+			       crypto_kpp_reqsize(tfm);
+	struct kpp_async_req *areq;
+	struct kpp_rsgl *last_rsgl = NULL;
+	int err = 0;
+	int maxsize;
+	size_t len = 0;
+	size_t used = 0;
+	unsigned int resultlen = 0;
+
+	maxsize = crypto_kpp_maxsize(tfm);
+	if (maxsize < 0)
+		return maxsize;
+
+	/* Allocate cipher request for current operation. */
+	areq = sock_kmalloc(sk, areqlen, GFP_KERNEL);
+	if (unlikely(!areq))
+		return -ENOMEM;
+	areq->areqlen = areqlen;
+	areq->sk = sk;
+	INIT_LIST_HEAD(&areq->rsgl_list);
+	areq->tsgl = NULL;
+	areq->tsgl_entries = 0;
+
+	/* convert iovecs of output buffers into RX SGL */
+	while ((len < maxsize) && msg_data_left(msg)) {
+		struct kpp_rsgl *rsgl;
+		size_t seglen;
+
+		/* limit the amount of readable buffers */
+		if (!kpp_readable(sk))
+			break;
+
+		if ((ctx->op == ALG_OP_SSGEN) && !ctx->used) {
+			err = kpp_wait_for_data(sk, flags);
+			if (err)
+				goto free;
+		}
+
+		seglen = min_t(size_t, (maxsize - len), msg_data_left(msg));
+
+		if (list_empty(&areq->rsgl_list)) {
+			rsgl = &areq->first_rsgl;
+		} else {
+			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
+			if (!rsgl) {
+				err = -ENOMEM;
+				goto free;
+			}
+		}
+
+		rsgl->sgl.npages = 0;
+		list_add_tail(&rsgl->list, &areq->rsgl_list);
+
+		/* make one iovec available as scatterlist */
+		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
+		if (err < 0)
+			goto free;
+
+		/* chain the new scatterlist with previous one */
+		if (last_rsgl)
+			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
+
+		last_rsgl = rsgl;
+		len += err;
+		ctx->rcvused += err;
+		rsgl->sg_num_bytes = err;
+		iov_iter_advance(&msg->msg_iter, err);
+	}
+
+	/* ensure output buffer is sufficiently large */
+	if (len < maxsize) {
+		err = -EMSGSIZE;
+		goto free;
+	}
+
+	/*
+	 * Create a per request TX SGL for this request which tracks the
+	 * SG entries from the global TX SGL.
+	 */
+	if (ctx->op == ALG_OP_SSGEN) {
+		used = ctx->used;
+
+		areq->tsgl_entries = kpp_count_tsgl(sk, used);
+		if (!areq->tsgl_entries)
+			areq->tsgl_entries = 1;
+		areq->tsgl = sock_kmalloc(
+				sk, sizeof(*areq->tsgl) * areq->tsgl_entries,
+				GFP_KERNEL);
+		if (!areq->tsgl) {
+			err = -ENOMEM;
+			goto free;
+		}
+		sg_init_table(areq->tsgl, areq->tsgl_entries);
+		kpp_pull_tsgl(sk, used, areq->tsgl);
+	}
+
+	/* Initialize the crypto operation */
+	kpp_request_set_input(&areq->req, areq->tsgl, used);
+	kpp_request_set_output(&areq->req, areq->first_rsgl.sgl.sg, len);
+	kpp_request_set_tfm(&areq->req, tfm);
+
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+		/* AIO operation */
+		areq->iocb = msg->msg_iocb;
+		kpp_request_set_callback(&areq->req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP,
+					      kpp_async_cb, areq);
+	} else
+		/* Synchronous operation */
+		kpp_request_set_callback(&areq->req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP |
+					      CRYPTO_TFM_REQ_MAY_BACKLOG,
+					      af_alg_complete,
+					      &ctx->completion);
+
+	switch (ctx->op) {
+	case ALG_OP_KEYGEN:
+		err = crypto_kpp_generate_public_key(&areq->req);
+		break;
+	case ALG_OP_SSGEN:
+		err = crypto_kpp_compute_shared_secret(&areq->req);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		goto free;
+	}
+
+	/* Wait for synchronous operation completion */
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb))
+		err = af_alg_wait_for_completion(err, &ctx->completion);
+
+	/* AIO operation in progress */
+	if (err == -EINPROGRESS) {
+		sock_hold(sk);
+		ctx->inflight++;
+		return -EIOCBQUEUED;
+	}
+
+	resultlen = areq->req.dst_len;
+
+free:
+	kpp_free_areq_sgl(areq);
+	if (areq)
+		sock_kfree_s(sk, areq, areqlen);
+
+	return err ? err : resultlen;
+}
+
+static int kpp_recvmsg(struct socket *sock, struct msghdr *msg,
+			    size_t ignored, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct sock *psk = ask->parent;
+	struct alg_sock *pask = alg_sk(psk);
+	struct kpp_tfm *kpp = pask->private;
+	struct crypto_kpp *tfm = kpp->kpp;
+
+	int ret = 0;
+
+	lock_sock(sk);
+
+	while (msg_data_left(msg)) {
+		int err = _kpp_recvmsg(sock, msg, ignored, flags);
+
+		/*
+		 * This error covers -EIOCBQUEUED which implies that we can
+		 * only handle one AIO request. If the caller wants to have
+		 * multiple AIO requests in parallel, he must make multiple
+		 * separate AIO calls.
+		 */
+		if (err < 0) {
+			ret = err;
+			goto out;
+		}
+		if (!err)
+			goto out;
+
+		ret += err;
+
+		/*
+		 * The caller must provide crypto_kpp_maxsize per request.
+		 * If he provides more, we conclude that multiple kpp
+		 * operations are requested.
+		 */
+		iov_iter_advance(&msg->msg_iter,
+				 crypto_kpp_maxsize(tfm) - err);
+	}
+
+out:
+
+	kpp_wmem_wakeup(sk);
+	release_sock(sk);
+	return ret;
+}
+
+static unsigned int kpp_poll(struct file *file, struct socket *sock,
+				  poll_table *wait)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+	unsigned int mask = 0;
+
+	sock_poll_wait(file, sk_sleep(sk), wait);
+
+	if (ctx->used)
+		mask |= POLLIN | POLLRDNORM;
+
+	if (kpp_writable(sk))
+		mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
+
+	return mask;
+}
+
+static struct proto_ops algif_kpp_ops = {
+	.family		=	PF_ALG,
+
+	.connect	=	sock_no_connect,
+	.socketpair	=	sock_no_socketpair,
+	.getname	=	sock_no_getname,
+	.ioctl		=	sock_no_ioctl,
+	.listen		=	sock_no_listen,
+	.shutdown	=	sock_no_shutdown,
+	.getsockopt	=	sock_no_getsockopt,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.setsockopt	=	sock_no_setsockopt,
+
+	.release	=	af_alg_release,
+	.sendmsg	=	kpp_sendmsg,
+	.sendpage	=	kpp_sendpage,
+	.recvmsg	=	kpp_recvmsg,
+	.poll		=	kpp_poll,
+};
+
+static int kpp_check_key(struct socket *sock)
+{
+	int err = 0;
+	struct sock *psk;
+	struct alg_sock *pask;
+	struct kpp_tfm *tfm;
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+
+	lock_sock(sk);
+	if (ask->refcnt)
+		goto unlock_child;
+
+	psk = ask->parent;
+	pask = alg_sk(ask->parent);
+	tfm = pask->private;
+
+	err = -ENOKEY;
+	lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
+	if (!tfm->has_key || (tfm->has_params == KPP_NO_PARAMS))
+		goto unlock;
+
+	if (!pask->refcnt++)
+		sock_hold(psk);
+
+	ask->refcnt = 1;
+	sock_put(psk);
+
+	err = 0;
+
+unlock:
+	release_sock(psk);
+unlock_child:
+	release_sock(sk);
+
+	return err;
+}
+
+static int kpp_sendmsg_nokey(struct socket *sock, struct msghdr *msg,
+				  size_t size)
+{
+	int err;
+
+	err = kpp_check_key(sock);
+	if (err)
+		return err;
+
+	return kpp_sendmsg(sock, msg, size);
+}
+
+static ssize_t kpp_sendpage_nokey(struct socket *sock, struct page *page,
+				       int offset, size_t size, int flags)
+{
+	int err;
+
+	err = kpp_check_key(sock);
+	if (err)
+		return err;
+
+	return kpp_sendpage(sock, page, offset, size, flags);
+}
+
+static int kpp_recvmsg_nokey(struct socket *sock, struct msghdr *msg,
+				  size_t ignored, int flags)
+{
+	int err;
+
+	err = kpp_check_key(sock);
+	if (err)
+		return err;
+
+	return kpp_recvmsg(sock, msg, ignored, flags);
+}
+
+static struct proto_ops algif_kpp_ops_nokey = {
+	.family		=	PF_ALG,
+
+	.connect	=	sock_no_connect,
+	.socketpair	=	sock_no_socketpair,
+	.getname	=	sock_no_getname,
+	.ioctl		=	sock_no_ioctl,
+	.listen		=	sock_no_listen,
+	.shutdown	=	sock_no_shutdown,
+	.getsockopt	=	sock_no_getsockopt,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.setsockopt	=	sock_no_setsockopt,
+
+	.release	=	af_alg_release,
+	.sendmsg	=	kpp_sendmsg_nokey,
+	.sendpage	=	kpp_sendpage_nokey,
+	.recvmsg	=	kpp_recvmsg_nokey,
+	.poll		=	kpp_poll,
+};
+
+static void *kpp_bind(const char *name, u32 type, u32 mask)
+{
+	struct kpp_tfm *tfm;
+	struct crypto_kpp *kpp;
+
+	tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+	if (!tfm)
+		return ERR_PTR(-ENOMEM);
+
+	kpp = crypto_alloc_kpp(name, type, mask);
+	if (IS_ERR(kpp)) {
+		kfree(tfm);
+		return ERR_CAST(kpp);
+	}
+
+	tfm->kpp = kpp;
+
+	return tfm;
+}
+
+static void kpp_release(void *private)
+{
+	struct kpp_tfm *tfm = private;
+	struct crypto_kpp *kpp = tfm->kpp;
+
+	crypto_free_kpp(kpp);
+	kfree(tfm);
+}
+
+static int kpp_dh_set_secret(struct crypto_kpp *tfm, struct dh *params)
+{
+	char *packed_key = NULL;
+	unsigned int packed_key_len;
+	int ret;
+
+	packed_key_len = crypto_dh_key_len(params);
+	packed_key = kmalloc(packed_key_len, GFP_KERNEL);
+	if (!packed_key)
+		return -ENOMEM;
+
+	ret = crypto_dh_encode_key(packed_key, packed_key_len, params);
+	if (ret)
+		goto out;
+
+	ret = crypto_kpp_set_secret(tfm, packed_key, packed_key_len);
+
+out:
+	kfree(packed_key);
+	return ret;
+}
+
+static int kpp_dh_set_privkey(struct crypto_kpp *tfm, const u8 *key,
+			      unsigned int keylen)
+{
+	struct dh params = {
+		.key = key,
+		.key_size = keylen,
+		.p = NULL,
+		.p_size = 0,
+		.g = NULL,
+		.g_size = 0,
+	};
+
+	return kpp_dh_set_secret(tfm, &params);
+}
+
+static int kpp_ecdh_set_secret(struct crypto_kpp *tfm, struct ecdh *params)
+{
+	char *packed_key = NULL;
+	unsigned int packed_key_len;
+	int ret;
+
+	packed_key_len = crypto_ecdh_key_len(params);
+	packed_key = kmalloc(packed_key_len, GFP_KERNEL);
+	if (!packed_key)
+		return -ENOMEM;
+
+	ret = crypto_ecdh_encode_key(packed_key, packed_key_len, params);
+	if (ret)
+		goto out;
+
+	ret = crypto_kpp_set_secret(tfm, packed_key, packed_key_len);
+
+out:
+	kfree(packed_key);
+	return ret;
+}
+
+static int kpp_ecdh_set_privkey(struct crypto_kpp *tfm, const u8 *key,
+				unsigned int keylen)
+{
+	struct ecdh params = {
+		.curve_id = 0,
+		.key = key,
+		.key_size = keylen,
+	};
+
+	return kpp_ecdh_set_secret(tfm, &params);
+}
+
+static int kpp_genkey(struct crypto_kpp *tfm, int (*set_secret)
+		(struct crypto_kpp *tfm, const u8 *key, unsigned int keylen))
+{
+	int err;
+	u8 *rndbuf = NULL;
+	int maxsize;
+
+	maxsize = crypto_kpp_maxsize(tfm);
+	if (maxsize < 0)
+		return maxsize;
+
+	/* Hard limit size to prevent caller to allocate arbitrary memory. */
+	if (maxsize > (16384 >> 3))
+		return -E2BIG;
+
+	rndbuf = kmalloc(maxsize, GFP_KERNEL);
+	if (!rndbuf)
+		return -ENOMEM;
+
+	if (crypto_get_default_rng()) {
+		err = -EFAULT;
+		goto out;
+	}
+
+	err = crypto_rng_get_bytes(crypto_default_rng, rndbuf, maxsize);
+	crypto_put_default_rng();
+	if (err)
+		goto out;
+
+	err = set_secret(tfm, rndbuf, maxsize);
+
+out:
+	kzfree(rndbuf);
+	return err;
+}
+
+static int kpp_setprivkey(void *private, const u8 *key, unsigned int keylen)
+{
+	struct kpp_tfm *kpp = private;
+	struct crypto_kpp *tfm = kpp->kpp;
+	int err;
+
+	if (kpp->has_params == KPP_NO_PARAMS)
+		return -ENOKEY;
+
+	if (!key || !keylen) {
+		/* Let kernel generate private key. */
+		switch (kpp->has_params) {
+		case KPP_DH_PARAMS:
+			err = kpp_genkey(tfm, kpp_dh_set_privkey);
+			break;
+		case KPP_ECDH_PARAMS:
+			err = kpp_genkey(tfm, kpp_ecdh_set_privkey);
+			break;
+		default:
+			err = -EFAULT;
+		}
+	} else {
+		/* Take user-provided private key. */
+		switch (kpp->has_params) {
+		case KPP_DH_PARAMS:
+			err = kpp_dh_set_privkey(tfm, key, keylen);
+			break;
+		case KPP_ECDH_PARAMS:
+			err = kpp_ecdh_set_privkey(tfm, key, keylen);
+			break;
+		default:
+			err = -EFAULT;
+		}
+	}
+
+	/* Return the maximum size of the kpp operation. */
+	if (!err) {
+		kpp->has_key = true;
+		err = crypto_kpp_maxsize(tfm);
+	} else
+		kpp->has_key = false;
+
+	return err;
+}
+
+static int kpp_dh_setparams_pkcs3(void *private, const u8 *params,
+				  unsigned int paramslen)
+{
+	struct kpp_tfm *kpp = private;
+	struct crypto_kpp *tfm = kpp->kpp;
+	int err;
+
+	/* If parameters were already set, disallow setting them again. */
+	if (kpp->has_params != KPP_NO_PARAMS)
+		return -EINVAL;
+
+	err = crypto_kpp_set_params(tfm, params, paramslen);
+	if (!err) {
+		kpp->has_params = KPP_DH_PARAMS;
+		/* Return the maximum size of the kpp operation. */
+		err = crypto_kpp_maxsize(tfm);
+	} else
+		kpp->has_params = KPP_NO_PARAMS;
+
+	return err;
+}
+
+static int kpp_ecdh_setcurve(void *private, const u8 *curveid,
+			     unsigned int curveidlen)
+{
+	struct kpp_tfm *kpp = private;
+	struct crypto_kpp *tfm = kpp->kpp;
+	int err;
+	struct ecdh params = {
+		.key = NULL,
+		.key_size = 0,
+	};
+
+	/* If parameters were already set, disallow setting them again. */
+	if (kpp->has_params != KPP_NO_PARAMS)
+		return -EINVAL;
+
+	if (curveidlen != sizeof(unsigned long))
+		return -EINVAL;
+
+	err = kstrtou16(curveid, 10, &params.curve_id);
+	if (err)
+		return err;
+
+	err = kpp_ecdh_set_secret(tfm, &params);
+	if (!err) {
+		kpp->has_params = KPP_ECDH_PARAMS;
+		/* Return the maximum size of the kpp operation. */
+		err = crypto_kpp_maxsize(tfm);
+	} else
+		kpp->has_params = KPP_NO_PARAMS;
+
+	return err;
+}
+
+static void kpp_sock_destruct(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct kpp_ctx *ctx = ask->private;
+
+	/* Suspend caller if AIO operations are in flight. */
+	wait_event_interruptible(kpp_aio_finish_wait,
+				 (ctx->inflight == 0));
+
+	kpp_pull_tsgl(sk, ctx->used, NULL);
+	sock_kfree_s(sk, ctx, ctx->len);
+	af_alg_release_parent(sk);
+}
+
+static int kpp_accept_parent_nokey(void *private, struct sock *sk)
+{
+	struct kpp_ctx *ctx;
+	struct alg_sock *ask = alg_sk(sk);
+	unsigned int len = sizeof(*ctx);
+
+	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	memset(ctx, 0, len);
+
+	INIT_LIST_HEAD(&ctx->tsgl_list);
+	ctx->len = len;
+	ctx->used = 0;
+	ctx->rcvused = 0;
+	ctx->more = 0;
+	ctx->merge = 0;
+	ctx->op = 0;
+	ctx->inflight = 0;
+	af_alg_init_completion(&ctx->completion);
+
+	ask->private = ctx;
+
+	sk->sk_destruct = kpp_sock_destruct;
+
+	return 0;
+}
+
+static int kpp_accept_parent(void *private, struct sock *sk)
+{
+	struct kpp_tfm *tfm = private;
+
+	if (!tfm->has_key || (tfm->has_params == KPP_NO_PARAMS))
+		return -ENOKEY;
+
+	return kpp_accept_parent_nokey(private, sk);
+}
+
+static const struct af_alg_type algif_type_kpp = {
+	.bind		=	kpp_bind,
+	.release	=	kpp_release,
+	.setkey		=	kpp_setprivkey,
+	.setpubkey	=	NULL,
+	.dhparams	=	kpp_dh_setparams_pkcs3,
+	.ecdhcurve	=	kpp_ecdh_setcurve,
+	.setauthsize	=	NULL,
+	.accept		=	kpp_accept_parent,
+	.accept_nokey	=	kpp_accept_parent_nokey,
+	.ops		=	&algif_kpp_ops,
+	.ops_nokey	=	&algif_kpp_ops_nokey,
+	.name		=	"kpp",
+	.owner		=	THIS_MODULE
+};
+
+static int __init algif_kpp_init(void)
+{
+	return af_alg_register_type(&algif_type_kpp);
+}
+
+static void __exit algif_kpp_exit(void)
+{
+	int err = af_alg_unregister_type(&algif_type_kpp);
+	BUG_ON(err);
+}
+
+module_init(algif_kpp_init);
+module_exit(algif_kpp_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
+MODULE_DESCRIPTION("Key protocol primitives kernel crypto API user space interface");
-- 
2.9.3

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

* Re: [RFC PATCH 0/8] crypto: AF_ALG support for KPP
  2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
                   ` (7 preceding siblings ...)
  2017-04-18 23:07 ` [PATCH 8/8] crypto: AF_ALG - add KPP support Stephan Müller
@ 2017-04-19 12:03 ` Tudor Ambarus
  2017-04-19 12:16   ` Stephan Müller
  8 siblings, 1 reply; 15+ messages in thread
From: Tudor Ambarus @ 2017-04-19 12:03 UTC (permalink / raw)
  To: Stephan Müller, linux-crypto, keyrings, herbert

Hi, Stephan, Herbert,

On 19.04.2017 02:03, Stephan Müller wrote:
> The patch 8 describes the different operations that are supported by AF_ALG
> KPP. This support includes generation and retaining of the private key
> inside the kernel. This private key would never be sent to user space.

There are crypto co-processors that are capable of generating and
retaining the private key inside the device without revealing it to
kernel. The private key will be further used to generate the public
key and the shared secret. Should we extend the KPP API to support this?

Thanks,
ta

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

* Re: [RFC PATCH 0/8] crypto: AF_ALG support for KPP
  2017-04-19 12:03 ` [RFC PATCH 0/8] crypto: AF_ALG support for KPP Tudor Ambarus
@ 2017-04-19 12:16   ` Stephan Müller
  0 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-19 12:16 UTC (permalink / raw)
  To: Tudor Ambarus; +Cc: linux-crypto, keyrings, herbert

Am Mittwoch, 19. April 2017, 14:03:35 CEST schrieb Tudor Ambarus:

Hi Tudor,

> Hi, Stephan, Herbert,
> 
> On 19.04.2017 02:03, Stephan Müller wrote:
> > The patch 8 describes the different operations that are supported by
> > AF_ALG
> > KPP. This support includes generation and retaining of the private key
> > inside the kernel. This private key would never be sent to user space.
> 
> There are crypto co-processors that are capable of generating and
> retaining the private key inside the device without revealing it to
> kernel. The private key will be further used to generate the public
> key and the shared secret. Should we extend the KPP API to support this?

The less software has access to secrets, the better. Thus, having such API 
would be good.

This of course assumes that the private key is generated with an appropriate 
RNG. As normal users cannot verify the RNG or the noise sources implemented in 
hardware, the choice of using such API to keep the private key inside the 
hardware should be left to the caller.

>From the AF_ALG KPP support side, I could imagine that that the algif_kpp code 
makes use of the hardware support unless the caller objects.

Ciao
Stephan

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

* RE: [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params
  2017-04-18 23:06 ` [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params Stephan Müller
@ 2017-04-19 14:51   ` Benedetto, Salvatore
  2017-04-20  7:29     ` Stephan Müller
  0 siblings, 1 reply; 15+ messages in thread
From: Benedetto, Salvatore @ 2017-04-19 14:51 UTC (permalink / raw)
  To: Stephan Müller, linux-crypto; +Cc: keyrings, Benedetto, Salvatore

Hi Stephan,

> -----Original Message-----
> From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> owner@vger.kernel.org] On Behalf Of Stephan Müller
> Sent: Wednesday, April 19, 2017 12:06 AM
> To: linux-crypto@vger.kernel.org
> Cc: keyrings@vger.kernel.org
> Subject: [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params
> 
> KPP mechanisms like DH require a parameter set to be provided by the caller.
> That parameter set may be provided by the crypto_kpp_set_secret function.
> Yet, the parameters hare handled independently from the secret key which
> implies that they should be able to be set independently from the key.
> 
> The new API allows KPP mechanisms to register a callback allowing to set
> such parameters.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>  Documentation/crypto/api-kpp.rst |  2 +-
>  include/crypto/kpp.h             | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/crypto/api-kpp.rst b/Documentation/crypto/api-
> kpp.rst
> index 7d86ab9..7b2c0d4 100644
> --- a/Documentation/crypto/api-kpp.rst
> +++ b/Documentation/crypto/api-kpp.rst
> @@ -11,7 +11,7 @@ Key-agreement Protocol Primitives (KPP) Cipher API
>     :doc: Generic Key-agreement Protocol Primitives API
> 
>  .. kernel-doc:: include/crypto/kpp.h
> -   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_secret
> crypto_kpp_generate_public_key crypto_kpp_compute_shared_secret
> crypto_kpp_maxsize
> +   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_params
> + crypto_kpp_set_secret crypto_kpp_generate_public_key
> + crypto_kpp_compute_shared_secret crypto_kpp_maxsize
> 
>  Key-agreement Protocol Primitives (KPP) Cipher Request Handle
>  -------------------------------------------------------------
> diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h index
> ce8e1f7..5931364 100644
> --- a/include/crypto/kpp.h
> +++ b/include/crypto/kpp.h
> @@ -51,6 +51,9 @@ struct crypto_kpp {
>  /**
>   * struct kpp_alg - generic key-agreement protocol primitives
>   *
> + * @set_params:	Function allows the caller to set the parameters
> + *			separately from the key. The format of the
> parameters
> + *			is protocol specific.
>   * @set_secret:		Function invokes the protocol specific
> function to
>   *			store the secret private key along with parameters.
>   *			The implementation knows how to decode thie
> buffer
> @@ -74,6 +77,8 @@ struct crypto_kpp {
>   * @base:		Common crypto API algorithm data structure
>   */
>  struct kpp_alg {
> +	int (*set_params)(struct crypto_kpp *tfm, const void *buffer,
> +			  unsigned int len);
>  	int (*set_secret)(struct crypto_kpp *tfm, const void *buffer,
>  			  unsigned int len);
>  	int (*generate_public_key)(struct kpp_request *req); @@ -259,6
> +264,29 @@ struct kpp_secret {  };
> 
>  /**
> + * crypto_kpp_set_params() - Set parameters needed for kpp operation
> + *
> + * Function invokes the specific kpp operation for a given alg.
> + *
> + * @tfm:	tfm handle
> + * @buffer:	Buffer holding the protocol specific representation of the
> + *		parameters (e.g. PKCS#3 DER for DH)
> + * @len:	Length of the parameter buffer.
> + *
> + * Return: zero on success; error code in case of error  */ static
> +inline int crypto_kpp_set_params(struct crypto_kpp *tfm,
> +				        const void *buffer, unsigned int len) {
> +	struct kpp_alg *alg = crypto_kpp_alg(tfm);
> +
> +	if (alg->set_params)
> +		return alg->set_params(tfm, buffer, len);
> +	else
> +		return -EOPNOTSUPP;
> +}
> +
> +/**
>   * crypto_kpp_set_secret() - Invoke kpp operation
>   *
>   * Function invokes the specific kpp operation for a given alg.
> --
> 2.9.3

I'm not really in favor of having two ways for setting the params.
As you are probably aware, PKCS3 and set_params was my intial
approach, but then Herbert suggested a lighter approach like rtnetlink
which I actually prefer.

Can't you expose that through AF_ALG?

Regards,
Salvatore

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

* Re: [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params
  2017-04-19 14:51   ` Benedetto, Salvatore
@ 2017-04-20  7:29     ` Stephan Müller
  0 siblings, 0 replies; 15+ messages in thread
From: Stephan Müller @ 2017-04-20  7:29 UTC (permalink / raw)
  To: Benedetto, Salvatore; +Cc: linux-crypto, keyrings

Am Mittwoch, 19. April 2017, 16:51:54 CEST schrieb Benedetto, Salvatore:

Hi Salvatore,

> Hi Stephan,
> 
> > -----Original Message-----
> > From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> > owner@vger.kernel.org] On Behalf Of Stephan Müller
> > Sent: Wednesday, April 19, 2017 12:06 AM
> > To: linux-crypto@vger.kernel.org
> > Cc: keyrings@vger.kernel.org
> > Subject: [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params
> > 
> > KPP mechanisms like DH require a parameter set to be provided by the
> > caller. That parameter set may be provided by the crypto_kpp_set_secret
> > function. Yet, the parameters hare handled independently from the secret
> > key which implies that they should be able to be set independently from
> > the key.
> > 
> > The new API allows KPP mechanisms to register a callback allowing to set
> > such parameters.
> > 
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > 
> >  Documentation/crypto/api-kpp.rst |  2 +-
> >  include/crypto/kpp.h             | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/crypto/api-kpp.rst b/Documentation/crypto/api-
> > kpp.rst
> > index 7d86ab9..7b2c0d4 100644
> > --- a/Documentation/crypto/api-kpp.rst
> > +++ b/Documentation/crypto/api-kpp.rst
> > @@ -11,7 +11,7 @@ Key-agreement Protocol Primitives (KPP) Cipher API
> > 
> >     :doc: Generic Key-agreement Protocol Primitives API
> >  
> >  .. kernel-doc:: include/crypto/kpp.h
> > 
> > -   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_secret
> > crypto_kpp_generate_public_key crypto_kpp_compute_shared_secret
> > crypto_kpp_maxsize
> > +   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_params
> > + crypto_kpp_set_secret crypto_kpp_generate_public_key
> > + crypto_kpp_compute_shared_secret crypto_kpp_maxsize
> > 
> >  Key-agreement Protocol Primitives (KPP) Cipher Request Handle
> >  -------------------------------------------------------------
> > 
> > diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h index
> > ce8e1f7..5931364 100644
> > --- a/include/crypto/kpp.h
> > +++ b/include/crypto/kpp.h
> > @@ -51,6 +51,9 @@ struct crypto_kpp {
> > 
> >  /**
> >  
> >   * struct kpp_alg - generic key-agreement protocol primitives
> >   *
> > 
> > + * @set_params:	Function allows the caller to set the parameters
> > + *			separately from the key. The format of the
> > parameters
> > + *			is protocol specific.
> > 
> >   * @set_secret:		Function invokes the protocol specific
> > 
> > function to
> > 
> >   *			store the secret private key along with parameters.
> >   *			The implementation knows how to decode thie
> > 
> > buffer
> > @@ -74,6 +77,8 @@ struct crypto_kpp {
> > 
> >   * @base:		Common crypto API algorithm data structure
> >   */
> >  
> >  struct kpp_alg {
> > 
> > +	int (*set_params)(struct crypto_kpp *tfm, const void *buffer,
> > +			  unsigned int len);
> > 
> >  	int (*set_secret)(struct crypto_kpp *tfm, const void *buffer,
> >  	
> >  			  unsigned int len);
> >  	
> >  	int (*generate_public_key)(struct kpp_request *req); @@ -259,6
> > 
> > +264,29 @@ struct kpp_secret {  };
> > 
> >  /**
> > 
> > + * crypto_kpp_set_params() - Set parameters needed for kpp operation
> > + *
> > + * Function invokes the specific kpp operation for a given alg.
> > + *
> > + * @tfm:	tfm handle
> > + * @buffer:	Buffer holding the protocol specific representation of the
> > + *		parameters (e.g. PKCS#3 DER for DH)
> > + * @len:	Length of the parameter buffer.
> > + *
> > + * Return: zero on success; error code in case of error  */ static
> > +inline int crypto_kpp_set_params(struct crypto_kpp *tfm,
> > +				        const void *buffer, unsigned int len) {
> > +	struct kpp_alg *alg = crypto_kpp_alg(tfm);
> > +
> > +	if (alg->set_params)
> > +		return alg->set_params(tfm, buffer, len);
> > +	else
> > +		return -EOPNOTSUPP;
> > +}
> > +
> > +/**
> > 
> >   * crypto_kpp_set_secret() - Invoke kpp operation
> >   *
> >   * Function invokes the specific kpp operation for a given alg.
> > 
> > --
> > 2.9.3
> 
> I'm not really in favor of having two ways for setting the params.

I am in full agreement. But setting the key together with the parameters is 
not good, IMHO. Every other crypto lib implements a separate setting.

> As you are probably aware, PKCS3 and set_params was my intial
> approach, but then Herbert suggested a lighter approach like rtnetlink
> which I actually prefer.

I was not aware of that. Considering that PKCS#3 or X9.42 are the common 
formats for setting parameters, I am not in favor of "inventing" a special 
format just for the kernel user space interface. If we would have such special 
format, user space would need to add a conversion step from a common to the 
kernel-special format.

You see with https://github.com/smuellerDD/libkcapi/blob/kpp/test/kcapi-main.c 
starting at line 2500 a full description how the OpenSSL-generated DH 
parameters can be used directly by the AF_ALG interface.

I have no concern to use rtnetlink for in-kernel use cases.
> 
> Can't you expose that through AF_ALG?

As mentioned, exposing rtnetlink means that we have to have an ASN.1-based 
converter from PKCS#3 to rtnetlink. As the kernel already has an ASN.1 parser 
and the additional code to use PKCS#3 in the kernel is very minimal compared 
to adding a ASN.1 parser in user space, I opted for implementing PKCS#3 in 
kernel space.
> 
> Regards,
> Salvatore


Ciao
Stephan

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

* Re: [PATCH 7/8] crypto: ecdh - constify key
  2017-04-18 23:07 ` [PATCH 7/8] crypto: ecdh - constify key Stephan Müller
@ 2017-08-28 10:34     ` Tudor Ambarus
  0 siblings, 0 replies; 15+ messages in thread
From: Tudor Ambarus @ 2017-08-28 10:34 UTC (permalink / raw)
  To: Stephan Müller, linux-crypto; +Cc: keyrings



On 04/19/2017 02:07 AM, Stephan Müller wrote:
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>   include/crypto/ecdh.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
> index 03a64f6..b5bb149 100644
> --- a/include/crypto/ecdh.h
> +++ b/include/crypto/ecdh.h
> @@ -40,7 +40,7 @@
>    */
>   struct ecdh {
>   	unsigned short curve_id;
> -	char *key;
> +	const char *key;
>   	unsigned short key_size;
>   };
>   
> 

I just came across this and remembered that Stephan already
made a patch, so:

Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com>

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

* Re: [PATCH 7/8] crypto: ecdh - constify key
@ 2017-08-28 10:34     ` Tudor Ambarus
  0 siblings, 0 replies; 15+ messages in thread
From: Tudor Ambarus @ 2017-08-28 10:34 UTC (permalink / raw)
  To: Stephan Müller, linux-crypto; +Cc: keyrings



On 04/19/2017 02:07 AM, Stephan Müller wrote:
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>   include/crypto/ecdh.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
> index 03a64f6..b5bb149 100644
> --- a/include/crypto/ecdh.h
> +++ b/include/crypto/ecdh.h
> @@ -40,7 +40,7 @@
>    */
>   struct ecdh {
>   	unsigned short curve_id;
> -	char *key;
> +	const char *key;
>   	unsigned short key_size;
>   };
>   
> 

I just came across this and remembered that Stephan already
made a patch, so:

Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com>

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

end of thread, other threads:[~2017-08-28 10:34 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 23:03 [RFC PATCH 0/8] crypto: AF_ALG support for KPP Stephan Müller
2017-04-18 23:04 ` [PATCH 1/8] crypto: AF_ALG -- add DH keygen / ssgen API Stephan Müller
2017-04-18 23:05 ` [PATCH 2/8] crypto: AF_ALG -- add DH param / ECDH curve setsockopt call Stephan Müller
2017-04-18 23:05 ` [PATCH 3/8] crypto: AF_ALG - eliminate code duplication Stephan Müller
2017-04-18 23:06 ` [PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params Stephan Müller
2017-04-19 14:51   ` Benedetto, Salvatore
2017-04-20  7:29     ` Stephan Müller
2017-04-18 23:06 ` [PATCH 5/8] crypto: DH - allow params and key to be set independently Stephan Müller
2017-04-18 23:06 ` [PATCH 6/8] crypto: DH - add PKCS#3 parameter handling Stephan Müller
2017-04-18 23:07 ` [PATCH 7/8] crypto: ecdh - constify key Stephan Müller
2017-08-28 10:34   ` Tudor Ambarus
2017-08-28 10:34     ` Tudor Ambarus
2017-04-18 23:07 ` [PATCH 8/8] crypto: AF_ALG - add KPP support Stephan Müller
2017-04-19 12:03 ` [RFC PATCH 0/8] crypto: AF_ALG support for KPP Tudor Ambarus
2017-04-19 12:16   ` Stephan Müller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.