2021-01-22 07:15:32

by Meng Yu

[permalink] [raw]
Subject: [PATCH v7 0/7] add ECDH and CURVE25519 algorithms support for Kunpeng 930

1. Add some new elliptic curve parameters definitions, and reorder
ECC 'Curves IDs';
2. Add interface to get elliptic curve by curve_id in
"include/crypto/ecc_curve.h" by curve_id;
3. Add ECDH and CURVE25519 algorithms support for Kunpeng 930.

v6->v7:
- patch #4: add function interface to expose elliptic curve parameters
- patch #4: eliminate warning by 'kernel test robot'
- patch #5: add function interface to expose curve25519 parameters

v5->v6:
- patch #1: add a new patch (the first patch), which is the "depend on" patch before

v4->v5:
- patch #4: delete P-128 and P-320 curve, as the few using case in the kernel

v3 -> v4:
- patch #3: add new, move ecc_curve params to "include/crypto"

v2 -> v3:
- patch #5: fix sparse warnings
- patch #5: add 'CRYPTO_LIB_CURVE25519_GENERIC' in 'Kconfig'

v1 -> v2:
- patch #5: delete `curve25519_null_point'


Hui Tang (1):
crypto: hisilicon/hpre - add some updates to adapt to Kunpeng 930

Meng Yu (6):
crypto: hisilicon/hpre - add version adapt to new algorithms
crypto: hisilicon/hpre - add algorithm type
crypto: add ecc curve and expose them
crypto: add curve 25519 and expose them
crypto: hisilicon/hpre - add 'ECDH' algorithm
crypto: hisilicon/hpre - add 'CURVE25519' algorithm

crypto/ecc.c | 22 +-
crypto/ecc.h | 37 +-
crypto/ecc_curve_defs.h | 163 +++++-
crypto/testmgr.h | 12 +-
drivers/crypto/hisilicon/Kconfig | 1 +
drivers/crypto/hisilicon/hpre/hpre.h | 25 +-
drivers/crypto/hisilicon/hpre/hpre_crypto.c | 861 +++++++++++++++++++++++++++-
drivers/crypto/hisilicon/hpre/hpre_main.c | 105 ++--
drivers/crypto/hisilicon/qm.c | 4 +-
drivers/crypto/hisilicon/qm.h | 4 +-
drivers/crypto/hisilicon/sec2/sec.h | 4 +-
drivers/crypto/hisilicon/sec2/sec_crypto.c | 4 +-
drivers/crypto/hisilicon/sec2/sec_crypto.h | 4 +-
drivers/crypto/hisilicon/zip/zip.h | 4 +-
drivers/crypto/hisilicon/zip/zip_crypto.c | 4 +-
include/crypto/ecc_curve.h | 60 ++
include/crypto/ecdh.h | 5 +-
17 files changed, 1191 insertions(+), 128 deletions(-)
create mode 100644 include/crypto/ecc_curve.h

--
2.8.1


2021-01-22 07:16:23

by Meng Yu

[permalink] [raw]
Subject: [PATCH v7 3/7] crypto: hisilicon/hpre - add algorithm type

Algorithm type is brought in to get hardware HPRE queue
to support different algorithms.

Signed-off-by: Meng Yu <[email protected]>
Reviewed-by: Zaibo Xu <[email protected]>
---
drivers/crypto/hisilicon/hpre/hpre.h | 10 +++++++++-
drivers/crypto/hisilicon/hpre/hpre_crypto.c | 12 ++++++------
drivers/crypto/hisilicon/hpre/hpre_main.c | 11 +++++++++--
3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/hisilicon/hpre/hpre.h b/drivers/crypto/hisilicon/hpre/hpre.h
index cc50f23..02193e1 100644
--- a/drivers/crypto/hisilicon/hpre/hpre.h
+++ b/drivers/crypto/hisilicon/hpre/hpre.h
@@ -10,6 +10,14 @@
#define HPRE_PF_DEF_Q_NUM 64
#define HPRE_PF_DEF_Q_BASE 0

+/*
+ * type used in qm sqc DW6.
+ * 0 - Algorithm which has been supported in V2, like RSA, DH and so on;
+ * 1 - ECC algorithm in V3.
+ */
+#define HPRE_V2_ALG_TYPE 0
+#define HPRE_V3_ECC_ALG_TYPE 1
+
enum {
HPRE_CLUSTER0,
HPRE_CLUSTER1,
@@ -92,7 +100,7 @@ struct hpre_sqe {
__le32 rsvd1[_HPRE_SQE_ALIGN_EXT];
};

-struct hisi_qp *hpre_create_qp(void);
+struct hisi_qp *hpre_create_qp(u8 type);
int hpre_algs_register(struct hisi_qm *qm);
void hpre_algs_unregister(struct hisi_qm *qm);

diff --git a/drivers/crypto/hisilicon/hpre/hpre_crypto.c b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
index d89b2f5..712bea9 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_crypto.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
@@ -152,12 +152,12 @@ static void hpre_rm_req_from_ctx(struct hpre_asym_request *hpre_req)
}
}

-static struct hisi_qp *hpre_get_qp_and_start(void)
+static struct hisi_qp *hpre_get_qp_and_start(u8 type)
{
struct hisi_qp *qp;
int ret;

- qp = hpre_create_qp();
+ qp = hpre_create_qp(type);
if (!qp) {
pr_err("Can not create hpre qp!\n");
return ERR_PTR(-ENODEV);
@@ -422,11 +422,11 @@ static void hpre_alg_cb(struct hisi_qp *qp, void *resp)
req->cb(ctx, resp);
}

-static int hpre_ctx_init(struct hpre_ctx *ctx)
+static int hpre_ctx_init(struct hpre_ctx *ctx, u8 type)
{
struct hisi_qp *qp;

- qp = hpre_get_qp_and_start();
+ qp = hpre_get_qp_and_start(type);
if (IS_ERR(qp))
return PTR_ERR(qp);

@@ -674,7 +674,7 @@ static int hpre_dh_init_tfm(struct crypto_kpp *tfm)
{
struct hpre_ctx *ctx = kpp_tfm_ctx(tfm);

- return hpre_ctx_init(ctx);
+ return hpre_ctx_init(ctx, HPRE_V2_ALG_TYPE);
}

static void hpre_dh_exit_tfm(struct crypto_kpp *tfm)
@@ -1100,7 +1100,7 @@ static int hpre_rsa_init_tfm(struct crypto_akcipher *tfm)
return PTR_ERR(ctx->rsa.soft_tfm);
}

- ret = hpre_ctx_init(ctx);
+ ret = hpre_ctx_init(ctx, HPRE_V2_ALG_TYPE);
if (ret)
crypto_free_akcipher(ctx->rsa.soft_tfm);

diff --git a/drivers/crypto/hisilicon/hpre/hpre_main.c b/drivers/crypto/hisilicon/hpre/hpre_main.c
index 52827b0..d3ec3b4 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_main.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_main.c
@@ -223,13 +223,20 @@ static u32 vfs_num;
module_param_cb(vfs_num, &vfs_num_ops, &vfs_num, 0444);
MODULE_PARM_DESC(vfs_num, "Number of VFs to enable(1-63), 0(default)");

-struct hisi_qp *hpre_create_qp(void)
+struct hisi_qp *hpre_create_qp(u8 type)
{
int node = cpu_to_node(smp_processor_id());
struct hisi_qp *qp = NULL;
int ret;

- ret = hisi_qm_alloc_qps_node(&hpre_devices, 1, 0, node, &qp);
+ if (type != HPRE_V2_ALG_TYPE && type != HPRE_V3_ECC_ALG_TYPE)
+ return NULL;
+
+ /*
+ * type: 0 - RSA/DH. algorithm supported in V2,
+ * 1 - ECC algorithm in V3.
+ */
+ ret = hisi_qm_alloc_qps_node(&hpre_devices, 1, type, node, &qp);
if (!ret)
return qp;

--
2.8.1

2021-01-22 07:17:07

by Meng Yu

[permalink] [raw]
Subject: [PATCH v7 4/7] crypto: add ecc curve and expose them

1. Add ecc curves(P224, P384, P521) for ECDH;
2. Reorder ECC 'Curves ID' in 'include/crypto/ecdh.h', and
modify 'curve_id' used in 'testmgr.h';
3. Add function 'ecc_get_curve_param' in 'include/crypto/ecc_curve.h' for
users, so everyone in the kernel tree can easily get ecc curve params;

Signed-off-by: Meng Yu <[email protected]>
Reviewed-by: Zaibo Xu <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
crypto/ecc.c | 15 ++++-
crypto/ecc.h | 37 +----------
crypto/ecc_curve_defs.h | 152 ++++++++++++++++++++++++++++++++++++++-------
crypto/testmgr.h | 12 ++--
include/crypto/ecc_curve.h | 53 ++++++++++++++++
include/crypto/ecdh.h | 5 +-
6 files changed, 207 insertions(+), 67 deletions(-)
create mode 100644 include/crypto/ecc_curve.h

diff --git a/crypto/ecc.c b/crypto/ecc.c
index c80aa25..cfa1dc3 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -24,6 +24,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

+#include <crypto/ecc_curve.h>
#include <linux/module.h>
#include <linux/random.h>
#include <linux/slab.h>
@@ -42,14 +43,24 @@ typedef struct {
u64 m_high;
} uint128_t;

+/* Returns CURVE if get curve succssful, NULL otherwise */
+const struct ecc_curve *ecc_get_curve_by_id(unsigned int curve_id)
+{
+ if (curve_id >= ECC_CURVE_NIST_P192 && curve_id <= ECC_CURVE_NIST_P521)
+ return &ecc_curve_list[curve_id - 1];
+
+ return NULL;
+}
+EXPORT_SYMBOL(ecc_get_curve_by_id);
+
static inline const struct ecc_curve *ecc_get_curve(unsigned int curve_id)
{
switch (curve_id) {
/* In FIPS mode only allow P256 and higher */
case ECC_CURVE_NIST_P192:
- return fips_enabled ? NULL : &nist_p192;
+ return fips_enabled ? NULL : ecc_get_curve_by_id(curve_id);
case ECC_CURVE_NIST_P256:
- return &nist_p256;
+ return ecc_get_curve_by_id(curve_id);
default:
return NULL;
}
diff --git a/crypto/ecc.h b/crypto/ecc.h
index d4e546b..38a81d4 100644
--- a/crypto/ecc.h
+++ b/crypto/ecc.h
@@ -26,6 +26,8 @@
#ifndef _CRYPTO_ECC_H
#define _CRYPTO_ECC_H

+#include <crypto/ecc_curve.h>
+
/* One digit is u64 qword. */
#define ECC_CURVE_NIST_P192_DIGITS 3
#define ECC_CURVE_NIST_P256_DIGITS 4
@@ -33,44 +35,9 @@

#define ECC_DIGITS_TO_BYTES_SHIFT 3

-/**
- * struct ecc_point - elliptic curve point in affine coordinates
- *
- * @x: X coordinate in vli form.
- * @y: Y coordinate in vli form.
- * @ndigits: Length of vlis in u64 qwords.
- */
-struct ecc_point {
- u64 *x;
- u64 *y;
- u8 ndigits;
-};
-
#define ECC_POINT_INIT(x, y, ndigits) (struct ecc_point) { x, y, ndigits }

/**
- * struct ecc_curve - definition of elliptic curve
- *
- * @name: Short name of the curve.
- * @g: Generator point of the curve.
- * @p: Prime number, if Barrett's reduction is used for this curve
- * pre-calculated value 'mu' is appended to the @p after ndigits.
- * Use of Barrett's reduction is heuristically determined in
- * vli_mmod_fast().
- * @n: Order of the curve group.
- * @a: Curve parameter a.
- * @b: Curve parameter b.
- */
-struct ecc_curve {
- char *name;
- struct ecc_point g;
- u64 *p;
- u64 *n;
- u64 *a;
- u64 *b;
-};
-
-/**
* ecc_is_key_valid() - Validate a given ECDH private key
*
* @curve_id: id representing the curve to use
diff --git a/crypto/ecc_curve_defs.h b/crypto/ecc_curve_defs.h
index 69be6c7..b81e580 100644
--- a/crypto/ecc_curve_defs.h
+++ b/crypto/ecc_curve_defs.h
@@ -15,18 +15,20 @@ static u64 nist_p192_a[] = { 0xFFFFFFFFFFFFFFFCull, 0xFFFFFFFFFFFFFFFEull,
0xFFFFFFFFFFFFFFFFull };
static u64 nist_p192_b[] = { 0xFEB8DEECC146B9B1ull, 0x0FA7E9AB72243049ull,
0x64210519E59C80E7ull };
-static struct ecc_curve nist_p192 = {
- .name = "nist_192",
- .g = {
- .x = nist_p192_g_x,
- .y = nist_p192_g_y,
- .ndigits = 3,
- },
- .p = nist_p192_p,
- .n = nist_p192_n,
- .a = nist_p192_a,
- .b = nist_p192_b
-};
+
+/* NIST P-224 */
+static u64 nist_p224_g_x[] = { 0x343280D6115C1D21ull, 0x4A03C1D356C21122ull,
+ 0x6BB4BF7F321390B9ull, 0xB70E0CBDull };
+static u64 nist_p224_g_y[] = { 0x44d5819985007e34ull, 0xcd4375a05a074764ull,
+ 0xb5f723fb4c22dfe6ull, 0xbd376388ull };
+static u64 nist_p224_p[] = { 0x0000000000000001ull, 0xFFFFFFFF00000000ull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFull };
+static u64 nist_p224_n[] = { 0x13DD29455C5C2A3Dull, 0xFFFF16A2E0B8F03Eull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFull };
+static u64 nist_p224_a[] = { 0xFFFFFFFFFFFFFFFEull, 0xFFFFFFFEFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFull };
+static u64 nist_p224_b[] = { 0x270B39432355FFB4ull, 0x5044B0B7D7BFD8BAull,
+ 0x0C04B3ABF5413256ull, 0xB4050A85ull };

/* NIST P-256: a = p - 3 */
static u64 nist_p256_g_x[] = { 0xF4A13945D898C296ull, 0x77037D812DEB33A0ull,
@@ -41,17 +43,121 @@ static u64 nist_p256_a[] = { 0xFFFFFFFFFFFFFFFCull, 0x00000000FFFFFFFFull,
0x0000000000000000ull, 0xFFFFFFFF00000001ull };
static u64 nist_p256_b[] = { 0x3BCE3C3E27D2604Bull, 0x651D06B0CC53B0F6ull,
0xB3EBBD55769886BCull, 0x5AC635D8AA3A93E7ull };
-static struct ecc_curve nist_p256 = {
- .name = "nist_256",
- .g = {
- .x = nist_p256_g_x,
- .y = nist_p256_g_y,
- .ndigits = 4,
- },
- .p = nist_p256_p,
- .n = nist_p256_n,
- .a = nist_p256_a,
- .b = nist_p256_b
+
+
+/* NIST P-384: a = p - 3 */
+static u64 nist_p384_g_x[] = { 0x3A545E3872760AB7ull, 0x5502F25DBF55296Cull,
+ 0x59F741E082542A38ull, 0x6E1D3B628BA79B98ull,
+ 0x8EB1C71EF320AD74ull, 0xAA87CA22BE8B0537ull};
+static u64 nist_p384_g_y[] = { 0x7A431D7C90EA0E5Full, 0x0A60B1CE1D7E819Dull,
+ 0xE9DA3113B5F0B8C0ull, 0xF8F41DBD289A147Cull,
+ 0x5D9E98BF9292DC29ull, 0x3617DE4A96262C6Full};
+static u64 nist_p384_p[] = { 0x00000000FFFFFFFFull, 0xFFFFFFFF00000000ull,
+ 0xFFFFFFFFFFFFFFFEull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull};
+static u64 nist_p384_n[] = { 0xECEC196ACCC52973ull, 0x581A0DB248B0A77Aull,
+ 0xC7634D81F4372DDFull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull};
+static u64 nist_p384_a[] = { 0x00000000FFFFFFFCull, 0xFFFFFFFF00000000ull,
+ 0xFFFFFFFFFFFFFFFEull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull};
+static u64 nist_p384_b[] = { 0x2A85C8EDD3EC2AEFull, 0xC656398D8A2ED19Dull,
+ 0x0314088F5013875Aull, 0x181D9C6EFE814112ull,
+ 0x988E056BE3F82D19ull, 0xB3312FA7E23EE7E4ull};
+
+/* NIST P-521: a = p - 3 */
+static u64 nist_p521_g_x[] = { 0xF97E7E31C2E5BD66ull, 0x3348B3C1856A429Bull,
+ 0xFE1DC127A2FFA8DEull, 0xA14B5E77EFE75928ull,
+ 0xF828AF606B4D3DBAull, 0x9C648139053FB521ull,
+ 0x9E3ECB662395B442ull, 0x858E06B70404E9CDull,
+ 0x00C6ull };
+static u64 nist_p521_g_y[] = { 0x88be94769fd16650ull, 0x353c7086a272c240ull,
+ 0xc550b9013fad0761ull, 0x97ee72995ef42640ull,
+ 0x17afbd17273e662cull, 0x98f54449579b4468ull,
+ 0x5c8a5fb42c7d1bd9ull, 0x39296a789a3bc004ull,
+ 0x0118ull };
+static u64 nist_p521_p[] = {0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0x01FFull };
+static u64 nist_p521_n[] = { 0xBB6FB71E91386409ull, 0x3BB5C9B8899C47AEull,
+ 0x7FCC0148F709A5D0ull, 0x51868783BF2F966Bull,
+ 0xFFFFFFFFFFFFFFFAull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0x01FFull };
+static u64 nist_p521_a[] = { 0xFFFFFFFFFFFFFFFCull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFFull,
+ 0x01FFull };
+static u64 nist_p521_b[] = { 0xEF451FD46B503F00ull, 0x3573DF883D2C34F1ull,
+ 0x1652C0BD3BB1BF07ull, 0x56193951EC7E937Bull,
+ 0xB8B489918EF109E1ull, 0xA2DA725B99B315F3ull,
+ 0x929A21A0B68540EEull, 0x953EB9618E1C9A1Full,
+ 0x0051ull };
+
+/**
+ * 'ecc_curve_list' is ordered as 'Curves IDs'
+ * defined in "include/crypto/ecdh.h"
+ */
+static const struct ecc_curve ecc_curve_list[] = {
+ {
+ .name = "nist_192",
+ .g = {
+ .x = nist_p192_g_x,
+ .y = nist_p192_g_y,
+ .ndigits = 3,
+ },
+ .p = nist_p192_p,
+ .n = nist_p192_n,
+ .a = nist_p192_a,
+ .b = nist_p192_b,
+ }, {
+ .name = "nist_224",
+ .g = {
+ .x = nist_p224_g_x,
+ .y = nist_p224_g_y,
+ .ndigits = 4,
+ },
+ .p = nist_p224_p,
+ .n = nist_p224_n,
+ .a = nist_p224_a,
+ .b = nist_p224_b,
+ }, {
+ .name = "nist_256",
+ .g = {
+ .x = nist_p256_g_x,
+ .y = nist_p256_g_y,
+ .ndigits = 4,
+ },
+ .p = nist_p256_p,
+ .n = nist_p256_n,
+ .a = nist_p256_a,
+ .b = nist_p256_b,
+ }, {
+ .name = "nist_384",
+ .g = {
+ .x = nist_p384_g_x,
+ .y = nist_p384_g_y,
+ .ndigits = 6,
+ },
+ .p = nist_p384_p,
+ .n = nist_p384_n,
+ .a = nist_p384_a,
+ .b = nist_p384_b,
+ }, {
+ .name = "nist_521",
+ .g = {
+ .x = nist_p521_g_x,
+ .y = nist_p521_g_y,
+ .ndigits = 9,
+ },
+ .p = nist_p521_p,
+ .n = nist_p521_n,
+ .a = nist_p521_a,
+ .b = nist_p521_b,
+ }
};

#endif
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 8c83811..7fe0fb9 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -2307,12 +2307,12 @@ static const struct kpp_testvec ecdh_tv_template[] = {
#ifdef __LITTLE_ENDIAN
"\x02\x00" /* type */
"\x28\x00" /* len */
- "\x02\x00" /* curve_id */
+ "\x03\x00" /* curve_id */
"\x20\x00" /* key_size */
#else
"\x00\x02" /* type */
"\x00\x28" /* len */
- "\x00\x02" /* curve_id */
+ "\x00\x03" /* curve_id */
"\x00\x20" /* key_size */
#endif
"\x24\xd1\x21\xeb\xe5\xcf\x2d\x83"
@@ -2351,24 +2351,24 @@ static const struct kpp_testvec ecdh_tv_template[] = {
#ifdef __LITTLE_ENDIAN
"\x02\x00" /* type */
"\x08\x00" /* len */
- "\x02\x00" /* curve_id */
+ "\x03\x00" /* curve_id */
"\x00\x00", /* key_size */
#else
"\x00\x02" /* type */
"\x00\x08" /* len */
- "\x00\x02" /* curve_id */
+ "\x00\x03" /* curve_id */
"\x00\x00", /* key_size */
#endif
.b_secret =
#ifdef __LITTLE_ENDIAN
"\x02\x00" /* type */
"\x28\x00" /* len */
- "\x02\x00" /* curve_id */
+ "\x03\x00" /* curve_id */
"\x20\x00" /* key_size */
#else
"\x00\x02" /* type */
"\x00\x28" /* len */
- "\x00\x02" /* curve_id */
+ "\x00\x03" /* curve_id */
"\x00\x20" /* key_size */
#endif
"\x24\xd1\x21\xeb\xe5\xcf\x2d\x83"
diff --git a/include/crypto/ecc_curve.h b/include/crypto/ecc_curve.h
new file mode 100644
index 0000000..a3adf1e
--- /dev/null
+++ b/include/crypto/ecc_curve.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2021 HiSilicon Limited. */
+
+#ifndef _CRYTO_ECC_CURVE_H
+#define _CRYTO_ECC_CURVE_H
+
+#include <linux/types.h>
+
+/**
+ * struct ecc_point - elliptic curve point in affine coordinates
+ *
+ * @x: X coordinate in vli form.
+ * @y: Y coordinate in vli form.
+ * @ndigits: Length of vlis in u64 qwords.
+ */
+struct ecc_point {
+ u64 *x;
+ u64 *y;
+ u8 ndigits;
+};
+
+/**
+ * struct ecc_curve - definition of elliptic curve
+ *
+ * @name: Short name of the curve.
+ * @g: Generator point of the curve.
+ * @p: Prime number, if Barrett's reduction is used for this curve
+ * pre-calculated value 'mu' is appended to the @p after ndigits.
+ * Use of Barrett's reduction is heuristically determined in
+ * vli_mmod_fast().
+ * @n: Order of the curve group.
+ * @a: Curve parameter a.
+ * @b: Curve parameter b.
+ */
+struct ecc_curve {
+ char *name;
+ struct ecc_point g;
+ u64 *p;
+ u64 *n;
+ u64 *a;
+ u64 *b;
+};
+
+/**
+ * ecc_get_curve_by_id() - get elliptic curve;
+ * @curve_id: Curves IDs:
+ * defined in "include/crypto/ecc_curve.h";
+ *
+ * Returns curve if get curve succssful, NULL otherwise
+ */
+const struct ecc_curve *ecc_get_curve_by_id(unsigned int curve_id);
+
+#endif
\ No newline at end of file
diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
index a5b805b..741d18a 100644
--- a/include/crypto/ecdh.h
+++ b/include/crypto/ecdh.h
@@ -24,7 +24,10 @@

/* Curves IDs */
#define ECC_CURVE_NIST_P192 0x0001
-#define ECC_CURVE_NIST_P256 0x0002
+#define ECC_CURVE_NIST_P224 0x0002
+#define ECC_CURVE_NIST_P256 0x0003
+#define ECC_CURVE_NIST_P384 0x0004
+#define ECC_CURVE_NIST_P521 0x0005

/**
* struct ecdh - define an ECDH private key
--
2.8.1

2021-01-22 07:17:25

by Meng Yu

[permalink] [raw]
Subject: [PATCH v7 2/7] crypto: hisilicon/hpre - add some updates to adapt to Kunpeng 930

From: Hui Tang <[email protected]>

HPRE of Kunpeng 930 is updated on cluster numbers and configurations
of Kunpeng 920 HPRE, so we try to update this driver to make it running
okay on both chips.

Signed-off-by: Hui Tang <[email protected]>
Signed-off-by: Meng Yu <[email protected]>
Reviewed-by: Zaibo Xu <[email protected]>
---
drivers/crypto/hisilicon/hpre/hpre.h | 8 ++-
drivers/crypto/hisilicon/hpre/hpre_main.c | 93 +++++++++++++++++++++----------
2 files changed, 68 insertions(+), 33 deletions(-)

diff --git a/drivers/crypto/hisilicon/hpre/hpre.h b/drivers/crypto/hisilicon/hpre/hpre.h
index e784712..cc50f23 100644
--- a/drivers/crypto/hisilicon/hpre/hpre.h
+++ b/drivers/crypto/hisilicon/hpre/hpre.h
@@ -14,8 +14,7 @@ enum {
HPRE_CLUSTER0,
HPRE_CLUSTER1,
HPRE_CLUSTER2,
- HPRE_CLUSTER3,
- HPRE_CLUSTERS_NUM,
+ HPRE_CLUSTER3
};

enum hpre_ctrl_dbgfs_file {
@@ -36,7 +35,10 @@ enum hpre_dfx_dbgfs_file {
HPRE_DFX_FILE_NUM
};

-#define HPRE_DEBUGFS_FILE_NUM (HPRE_DEBUG_FILE_NUM + HPRE_CLUSTERS_NUM - 1)
+#define HPRE_CLUSTERS_NUM_V2 (HPRE_CLUSTER3 + 1)
+#define HPRE_CLUSTERS_NUM_V3 1
+#define HPRE_CLUSTERS_NUM_MAX HPRE_CLUSTERS_NUM_V2
+#define HPRE_DEBUGFS_FILE_NUM (HPRE_DEBUG_FILE_NUM + HPRE_CLUSTERS_NUM_MAX - 1)

struct hpre_debugfs_file {
int index;
diff --git a/drivers/crypto/hisilicon/hpre/hpre_main.c b/drivers/crypto/hisilicon/hpre/hpre_main.c
index ad8b691..52827b0 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_main.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_main.c
@@ -30,6 +30,8 @@
#define HPRE_BD_ARUSR_CFG 0x301030
#define HPRE_BD_AWUSR_CFG 0x301034
#define HPRE_TYPES_ENB 0x301038
+#define HPRE_RSA_ENB BIT(0)
+#define HPRE_ECC_ENB BIT(1)
#define HPRE_DATA_RUSER_CFG 0x30103c
#define HPRE_DATA_WUSER_CFG 0x301040
#define HPRE_INT_MASK 0x301400
@@ -74,7 +76,8 @@
#define HPRE_QM_AXI_CFG_MASK 0xffff
#define HPRE_QM_VFG_AX_MASK 0xff
#define HPRE_BD_USR_MASK 0x3
-#define HPRE_CLUSTER_CORE_MASK 0xf
+#define HPRE_CLUSTER_CORE_MASK_V2 0xf
+#define HPRE_CLUSTER_CORE_MASK_V3 0xff

#define HPRE_AM_OOO_SHUTDOWN_ENB 0x301044
#define HPRE_AM_OOO_SHUTDOWN_ENABLE BIT(0)
@@ -87,6 +90,11 @@
#define HPRE_QM_PM_FLR BIT(11)
#define HPRE_QM_SRIOV_FLR BIT(12)

+#define HPRE_CLUSTERS_NUM(qm) \
+ (((qm)->ver >= QM_HW_V3) ? HPRE_CLUSTERS_NUM_V3 : HPRE_CLUSTERS_NUM_V2)
+#define HPRE_CLUSTER_CORE_MASK(qm) \
+ (((qm)->ver >= QM_HW_V3) ? HPRE_CLUSTER_CORE_MASK_V3 :\
+ HPRE_CLUSTER_CORE_MASK_V2)
#define HPRE_VIA_MSI_DSM 1
#define HPRE_SQE_MASK_OFFSET 8
#define HPRE_SQE_MASK_LEN 24
@@ -276,8 +284,40 @@ static int hpre_cfg_by_dsm(struct hisi_qm *qm)
return 0;
}

+static int hpre_set_cluster(struct hisi_qm *qm)
+{
+ u32 cluster_core_mask = HPRE_CLUSTER_CORE_MASK(qm);
+ u8 clusters_num = HPRE_CLUSTERS_NUM(qm);
+ struct device *dev = &qm->pdev->dev;
+ unsigned long offset;
+ u32 val = 0;
+ int ret, i;
+
+ for (i = 0; i < clusters_num; i++) {
+ offset = i * HPRE_CLSTR_ADDR_INTRVL;
+
+ /* clusters initiating */
+ writel(cluster_core_mask,
+ HPRE_ADDR(qm, offset + HPRE_CORE_ENB));
+ writel(0x1, HPRE_ADDR(qm, offset + HPRE_CORE_INI_CFG));
+ ret = readl_relaxed_poll_timeout(HPRE_ADDR(qm, offset +
+ HPRE_CORE_INI_STATUS), val,
+ ((val & cluster_core_mask) ==
+ cluster_core_mask),
+ HPRE_REG_RD_INTVRL_US,
+ HPRE_REG_RD_TMOUT_US);
+ if (ret) {
+ dev_err(dev,
+ "cluster %d int st status timeout!\n", i);
+ return -ETIMEDOUT;
+ }
+ }
+
+ return 0;
+}
+
/*
- * For Hi1620, we shoul disable FLR triggered by hardware (BME/PM/SRIOV).
+ * For Kunpeng 920, we shoul disable FLR triggered by hardware (BME/PM/SRIOV).
* Or it may stay in D3 state when we bind and unbind hpre quickly,
* as it does FLR triggered by hardware.
*/
@@ -295,9 +335,8 @@ static void disable_flr_of_bme(struct hisi_qm *qm)
static int hpre_set_user_domain_and_cache(struct hisi_qm *qm)
{
struct device *dev = &qm->pdev->dev;
- unsigned long offset;
- int ret, i;
u32 val;
+ int ret;

writel(HPRE_QM_USR_CFG_MASK, HPRE_ADDR(qm, QM_ARUSER_M_CFG_ENABLE));
writel(HPRE_QM_USR_CFG_MASK, HPRE_ADDR(qm, QM_AWUSER_M_CFG_ENABLE));
@@ -308,7 +347,12 @@ static int hpre_set_user_domain_and_cache(struct hisi_qm *qm)
val |= BIT(HPRE_TIMEOUT_ABNML_BIT);
writel_relaxed(val, HPRE_ADDR(qm, HPRE_QM_ABNML_INT_MASK));

- writel(0x1, HPRE_ADDR(qm, HPRE_TYPES_ENB));
+ if (qm->ver >= QM_HW_V3)
+ writel(HPRE_RSA_ENB | HPRE_ECC_ENB,
+ HPRE_ADDR(qm, HPRE_TYPES_ENB));
+ else
+ writel(HPRE_RSA_ENB, HPRE_ADDR(qm, HPRE_TYPES_ENB));
+
writel(HPRE_QM_VFG_AX_MASK, HPRE_ADDR(qm, HPRE_VFG_AXCACHE));
writel(0x0, HPRE_ADDR(qm, HPRE_BD_ENDIAN));
writel(0x0, HPRE_ADDR(qm, HPRE_INT_MASK));
@@ -333,37 +377,25 @@ static int hpre_set_user_domain_and_cache(struct hisi_qm *qm)
return -ETIMEDOUT;
}

- for (i = 0; i < HPRE_CLUSTERS_NUM; i++) {
- offset = i * HPRE_CLSTR_ADDR_INTRVL;
-
- /* clusters initiating */
- writel(HPRE_CLUSTER_CORE_MASK,
- HPRE_ADDR(qm, offset + HPRE_CORE_ENB));
- writel(0x1, HPRE_ADDR(qm, offset + HPRE_CORE_INI_CFG));
- ret = readl_relaxed_poll_timeout(HPRE_ADDR(qm, offset +
- HPRE_CORE_INI_STATUS), val,
- ((val & HPRE_CLUSTER_CORE_MASK) ==
- HPRE_CLUSTER_CORE_MASK),
- HPRE_REG_RD_INTVRL_US,
- HPRE_REG_RD_TMOUT_US);
- if (ret) {
- dev_err(dev,
- "cluster %d int st status timeout!\n", i);
- return -ETIMEDOUT;
- }
- }
-
- ret = hpre_cfg_by_dsm(qm);
+ ret = hpre_set_cluster(qm);
if (ret)
- dev_err(dev, "acpi_evaluate_dsm err.\n");
+ return -ETIMEDOUT;

- disable_flr_of_bme(qm);
+ /* This setting is only needed by Kunpeng 920. */
+ if (qm->ver == QM_HW_V2) {
+ ret = hpre_cfg_by_dsm(qm);
+ if (ret)
+ dev_err(dev, "acpi_evaluate_dsm err.\n");
+
+ disable_flr_of_bme(qm);
+ }

return ret;
}

static void hpre_cnt_regs_clear(struct hisi_qm *qm)
{
+ u8 clusters_num = HPRE_CLUSTERS_NUM(qm);
unsigned long offset;
int i;

@@ -372,7 +404,7 @@ static void hpre_cnt_regs_clear(struct hisi_qm *qm)
writel(0x0, qm->io_base + QM_DFX_DB_CNT_VF);

/* clear clusterX/cluster_ctrl */
- for (i = 0; i < HPRE_CLUSTERS_NUM; i++) {
+ for (i = 0; i < clusters_num; i++) {
offset = HPRE_CLSTR_BASE + i * HPRE_CLSTR_ADDR_INTRVL;
writel(0x0, qm->io_base + offset + HPRE_CLUSTER_INQURY);
}
@@ -671,13 +703,14 @@ static int hpre_pf_comm_regs_debugfs_init(struct hisi_qm *qm)

static int hpre_cluster_debugfs_init(struct hisi_qm *qm)
{
+ u8 clusters_num = HPRE_CLUSTERS_NUM(qm);
struct device *dev = &qm->pdev->dev;
char buf[HPRE_DBGFS_VAL_MAX_LEN];
struct debugfs_regset32 *regset;
struct dentry *tmp_d;
int i, ret;

- for (i = 0; i < HPRE_CLUSTERS_NUM; i++) {
+ for (i = 0; i < clusters_num; i++) {
ret = snprintf(buf, HPRE_DBGFS_VAL_MAX_LEN, "cluster%d", i);
if (ret < 0)
return -EINVAL;
--
2.8.1

2021-01-28 05:07:05

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Fri, Jan 22, 2021 at 03:09:52PM +0800, Meng Yu wrote:
> 1. Add ecc curves(P224, P384, P521) for ECDH;

OK I think this is getting unwieldy.

In light of the fact that we already have hardware that supports
a specific subset of curves, I think perhaps it would be better
to move the curve ID from the key into the algorithm name instead.

IOW, instead of allocating ecdh, you would allocate ecdh-nist-pXXX.

Any comments?

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

2021-01-28 10:32:55

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Thu, 28 Jan 2021 at 06:04, Herbert Xu <[email protected]> wrote:
>
> On Fri, Jan 22, 2021 at 03:09:52PM +0800, Meng Yu wrote:
> > 1. Add ecc curves(P224, P384, P521) for ECDH;
>
> OK I think this is getting unwieldy.
>
> In light of the fact that we already have hardware that supports
> a specific subset of curves, I think perhaps it would be better
> to move the curve ID from the key into the algorithm name instead.
>
> IOW, instead of allocating ecdh, you would allocate ecdh-nist-pXXX.
>
> Any comments?
>

Agreed. Bluetooth appears to be the only in-kernel user at the moment,
and it is hard coded to use p256, so it can be easily updated.

But this also begs the question which ecdh-nist-pXXX implementations
we actually need? Why are we exposing these curves in the first place?

2021-01-29 02:52:49

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On 1/28/21 5:30 AM, Ard Biesheuvel wrote:
> On Thu, 28 Jan 2021 at 06:04, Herbert Xu <[email protected]> wrote:
>> On Fri, Jan 22, 2021 at 03:09:52PM +0800, Meng Yu wrote:
>>> 1. Add ecc curves(P224, P384, P521) for ECDH;
>> OK I think this is getting unwieldy.
>>
>> In light of the fact that we already have hardware that supports
>> a specific subset of curves, I think perhaps it would be better
>> to move the curve ID from the key into the algorithm name instead.
>>
>> IOW, instead of allocating ecdh, you would allocate ecdh-nist-pXXX.
>>
>> Any comments?
>>
> Agreed. Bluetooth appears to be the only in-kernel user at the moment,
> and it is hard coded to use p256, so it can be easily updated.
>
> But this also begs the question which ecdh-nist-pXXX implementations
> we actually need? Why are we exposing these curves in the first place?

In the patch series that I just submitted I would like to expose at
least NIST p256 to users. Fedora 34 is working to add signatures for
files to rpms for the Integrity Measurment Architecture (IMA) to be able
to enforce signatures on executables, libraries etc. The signatures are
written out into security.ima extended attribute upon rpm installation.
IMA accesses keys on a keyring to verify these file signatures. RSA
signatures are much larger, so elliptic curve signatures are much better
in terms of storage size needed for storing them in rpms. They add like
1% of size. Fedora is using a NIST P256 key.

Besides that Fedora and RHEL support only these curves here in openssl
(Ubuntu supports a lot more):

$ openssl ecparam -list_curves
  secp224r1 : NIST/SECG curve over a 224 bit prime field
  secp256k1 : SECG curve over a 256 bit prime field
  secp384r1 : NIST/SECG curve over a 384 bit prime field
  secp521r1 : NIST/SECG curve over a 521 bit prime field
  prime256v1: X9.62/SECG curve over a 256 bit prime field

So from that perspective it makes a lot of sense to support some of
these and allow users to load them into the kernel.


In my patch series I initially had registered the akciphers under the
names ecc-nist-p192 and ecc-nist-p256 but now, in V4, joined them
together as 'ecdsa'. This may be too generic for a name. Maybe it should
be called ecsda-nist for the NIST family.

   Stefan

2021-01-29 03:02:21

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Thu, Jan 28, 2021 at 09:49:41PM -0500, Stefan Berger wrote:
>
> In my patch series I initially had registered the akciphers under the names
> ecc-nist-p192 and ecc-nist-p256 but now, in V4, joined them together as
> 'ecdsa'. This may be too generic for a name. Maybe it should be called
> ecsda-nist for the NIST family.

What I'm proposing is specifying the curve in the name as well, i.e.,
ecdsa-nist-p192 instead of just ecdsa or ecdsa-nist.

This simplifies the task of handling hardware that only supports a
subset of curves.

There is a parallel discussion of exactly what curves we should
support in the kernel. Personally if there is a user in the kernel
for it then I'm happy to see it added. In your specific case, as
long as your use of the algorithm in x509 is accepted then I don't
have any problems with adding support in the Crypto API.

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

2021-02-01 03:47:03

by Meng Yu

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them



?? 2021/1/28 13:03, Herbert Xu д??:
> On Fri, Jan 22, 2021 at 03:09:52PM +0800, Meng Yu wrote:
>> 1. Add ecc curves(P224, P384, P521) for ECDH;
>
> OK I think this is getting unwieldy.
>
> In light of the fact that we already have hardware that supports
> a specific subset of curves, I think perhaps it would be better
> to move the curve ID from the key into the algorithm name instead.
>
> IOW, instead of allocating ecdh, you would allocate ecdh-nist-pXXX.
>
> Any comments?
>
> Thanks,
>

Yes??It is a good idea, thank you!

2021-02-01 17:13:34

by Daniele Alessandrelli

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Thu, 2021-01-28 at 21:39 +1100, Herbert Xu wrote:
> Once they're distinct algorithms, we can then make sure that only
> the ones that are used in the kernel is added, even if some hardware
> may support more curves.

I like the idea of having different algorithms names (ecdh-nist-
pXXX) for different curves, but I'm not fully convinced by the above
statement.

What's the downside of letting device drivers enable all the curves
supported by the HW (with the exception of obsolete curves /
algorithms), even if there is (currently) no user of such curves in the
kernel? Code size and maintainability?

I think that once there is support for certain curves, it's more likely
that drivers / modules using them will appear.

Also, even if there are no in-tree users, there might be a few out-of-
tree ones.

2021-02-02 05:16:40

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Mon, Feb 01, 2021 at 05:09:41PM +0000, Daniele Alessandrelli wrote:
> What's the downside of letting device drivers enable all the curves
> supported by the HW (with the exception of obsolete curves /
> algorithms), even if there is (currently) no user of such curves in the
> kernel? Code size and maintainability?

The issue is that we always require a software implementation for
any given hardware algorithm. As otherwise kernel users cannot
rely on the algorithm to work.

Of course we don't want to add every single algorithm out there
to the kernel so that's why require there to be an actual in-kernel
user before adding a given algorithm.

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

2021-02-02 09:28:54

by Alessandrelli, Daniele

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Tue, 2021-02-02 at 16:13 +1100, Herbert Xu wrote:
> The issue is that we always require a software implementation for
> any given hardware algorithm. As otherwise kernel users cannot
> rely on the algorithm to work.

I understand. This sounds very reasonable to me.

> Of course we don't want to add every single algorithm out there
> to the kernel so that's why require there to be an actual in-kernel
> user before adding a given algorithm.

I see. Just to clarify: does the in-kernel user requirement also apply
to the case when the author of a device driver also provides the
software implementation for the new algorithms supported by device
driver / HW?

2021-02-02 09:45:35

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Tue, Feb 02, 2021 at 09:27:33AM +0000, Alessandrelli, Daniele wrote:
>
> I see. Just to clarify: does the in-kernel user requirement also apply
> to the case when the author of a device driver also provides the
> software implementation for the new algorithms supported by device
> driver / HW?

Yes we need an actual user. For example, if your algorithm is used
by the Security Subsystem (IMA) that would be sufficient.

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

2021-02-02 12:39:11

by Alessandrelli, Daniele

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Tue, 2021-02-02 at 20:42 +1100, Herbert Xu wrote:
> On Tue, Feb 02, 2021 at 09:27:33AM +0000, Alessandrelli, Daniele
> wrote:
> > I see. Just to clarify: does the in-kernel user requirement also
> > apply
> > to the case when the author of a device driver also provides the
> > software implementation for the new algorithms supported by device
> > driver / HW?
>
> Yes we need an actual user. For example, if your algorithm is used
> by the Security Subsystem (IMA) that would be sufficient.

Understood, thanks!

Unrelated question: I have my Keem Bay OCS ECC patchset [1] almost
ready for re-submission. Should I go ahead or should I wait for the
final decision about using 'ecdh-nist-pXXX' in place of 'ecdh'?

Also, I guess I'll have to drop P-384 support from the driver, since
the are no in-kernel user AFAIK.

[1] https://lore.kernel.org/linux-crypto/[email protected]/

>
> Cheers,

2021-02-03 18:06:11

by Saulo Alessandre de Lima

[permalink] [raw]
Subject: [PATCH v7 4/7] crypto: add ecc curve and expose them

On 28/01/2021 02:03, Herbert Xu wrote:
> On Fri, Jan 22, 2021 at 03:09:52PM +0800, Meng Yu wrote:
>> 1. Add ecc curves(P224, P384, P521) for ECDH;
>
> OK I think this is getting unwieldy.
>
> In light of the fact that we already have hardware that supports
> a specific subset of curves, I think perhaps it would be better
> to move the curve ID from the key into the algorithm name instead.
>
I think I understand you, I'm not using ECDH at the moment, but IMHO maybe
we could use enum OID of oid_registry.h as curve ID and eliminate the
duplicate ECC_CURVE_NIST_{...} from ecdh.h. Or perhaps put another param
in struct ecc_curve with OID, because the name already exists.

> IOW, instead of allocating ecdh, you would allocate ecdh-nist-pXXX.
>
> Any comments?

I recently sent a patch for the ECDSA signature verification, that use
the NIST-P curves to check elf32 binary modules and signatures in
about 450k T-DRE voting machines, in the Brazilian Elections across
the country. I put the other curves because we started testing them
(P256, P384) for speed measurement, but we ended up using P521 in our
production version since 2017 in the 4.9.xxx kernel, and now in 5.4.xxx.

In this patch I'm using akcipher allocate like ecdsa(sha1,sha256,...),
because the ecdsa algo is generic, and using the curve name and ndigits
inside vli_mmod_fast to discover the curve, but I agree the correct way
would be allocate ecdsa-nist-p521(sha1,...) and have all params for the
curve inside strut ecc_curve, remembering that we have anothers curves
incoming, like Edwards.

regards,
--
Email: Saulo Alessandre <[email protected]>

2021-02-04 05:33:20

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Tue, Feb 02, 2021 at 12:35:26PM +0000, Alessandrelli, Daniele wrote:
>
> Unrelated question: I have my Keem Bay OCS ECC patchset [1] almost
> ready for re-submission. Should I go ahead or should I wait for the
> final decision about using 'ecdh-nist-pXXX' in place of 'ecdh'?

If we agree on going down this route, then the first step is to
convert the existing ecdh generic algorithm and its users to this
scheme to ensure no regressions.

After that then you can add your driver.

PS I just noticed that we already have one driver implementing
ecdh, atmel so it too would need to be converted before we take
on any new drivers for ecdh.

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

2021-02-04 05:42:43

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Wed, Feb 03, 2021 at 03:03:44PM -0300, Saulo Alessandre wrote:
>
> In this patch I'm using akcipher allocate like ecdsa(sha1,sha256,...),
> because the ecdsa algo is generic, and using the curve name and ndigits
> inside vli_mmod_fast to discover the curve, but I agree the correct way
> would be allocate ecdsa-nist-p521(sha1,...) and have all params for the
> curve inside strut ecc_curve, remembering that we have anothers curves
> incoming, like Edwards.

I'm not sure whether we really should encode hash into the algorithm
name. This may be something that we can move into setkey instead.

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

2021-02-08 06:50:30

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

On Mon, 8 Feb 2021 at 07:37, Vitaly Chikunov <[email protected]> wrote:
>
> Herbert,
>
> On Fri, Jan 29, 2021 at 02:00:04PM +1100, Herbert Xu wrote:
> > On Thu, Jan 28, 2021 at 09:49:41PM -0500, Stefan Berger wrote:
> > >
> > > In my patch series I initially had registered the akciphers under the names
> > > ecc-nist-p192 and ecc-nist-p256 but now, in V4, joined them together as
> > > 'ecdsa'. This may be too generic for a name. Maybe it should be called
> > > ecsda-nist for the NIST family.
> >
> > What I'm proposing is specifying the curve in the name as well, i.e.,
> > ecdsa-nist-p192 instead of just ecdsa or ecdsa-nist.
> >
> > This simplifies the task of handling hardware that only supports a
> > subset of curves.
>
> So, if some implementation supports multiple curves (like EC-RDSA
> currently supports 5 curves), it should add 5 ecrdsa-{a,b,c,..}
> algorithms with actually the same top level implementation?
> Right?
>

Yes. The only difference will be the init() function, which can be
used to set the TFM properties that define which curve is being used.
The other routines can be generic, and refer to those properties if
the behavior is curve-specific.


>
> > There is a parallel discussion of exactly what curves we should
> > support in the kernel. Personally if there is a user in the kernel
> > for it then I'm happy to see it added. In your specific case, as
> > long as your use of the algorithm in x509 is accepted then I don't
> > have any problems with adding support in the Crypto API.
> >
> > Cheers,
> > --
> > Email: Herbert Xu <[email protected]>
> > Home Page: http://gondor.apana.org.au/~herbert/
> > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-02-08 21:31:02

by Vitaly Chikunov

[permalink] [raw]
Subject: Re: [PATCH v7 4/7] crypto: add ecc curve and expose them

Ard,

On Mon, Feb 08, 2021 at 07:47:44AM +0100, Ard Biesheuvel wrote:
> On Mon, 8 Feb 2021 at 07:37, Vitaly Chikunov <[email protected]> wrote:
> >
> > Herbert,
> >
> > On Fri, Jan 29, 2021 at 02:00:04PM +1100, Herbert Xu wrote:
> > > On Thu, Jan 28, 2021 at 09:49:41PM -0500, Stefan Berger wrote:
> > > >
> > > > In my patch series I initially had registered the akciphers under the names
> > > > ecc-nist-p192 and ecc-nist-p256 but now, in V4, joined them together as
> > > > 'ecdsa'. This may be too generic for a name. Maybe it should be called
> > > > ecsda-nist for the NIST family.
> > >
> > > What I'm proposing is specifying the curve in the name as well, i.e.,
> > > ecdsa-nist-p192 instead of just ecdsa or ecdsa-nist.
> > >
> > > This simplifies the task of handling hardware that only supports a
> > > subset of curves.
> >
> > So, if some implementation supports multiple curves (like EC-RDSA
> > currently supports 5 curves), it should add 5 ecrdsa-{a,b,c,..}
> > algorithms with actually the same top level implementation?
> > Right?
> >
>
> Yes. The only difference will be the init() function, which can be
> used to set the TFM properties that define which curve is being used.
> The other routines can be generic, and refer to those properties if
> the behavior is curve-specific.

Thanks. This may be good!

JFYI. There is possible non-hardware accelerated implementations
for ECC algorithms which (perhaps) may never go to the kernel source,
because they are generated code. For example
https://gitlab.com/nisec/ecckiila