diff mbox series

[v7,4/7] crypto: add ecc curve and expose them

Message ID 1611299395-675-5-git-send-email-yumeng18@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series add ECDH and CURVE25519 algorithms support for Kunpeng 930 | expand

Commit Message

Meng Yu Jan. 22, 2021, 7:09 a.m. UTC
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 <yumeng18@huawei.com>
Reviewed-by: Zaibo Xu <xuzaibo@huawei.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 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

Comments

Herbert Xu Jan. 28, 2021, 5:03 a.m. UTC | #1
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,
Ard Biesheuvel Jan. 28, 2021, 10:30 a.m. UTC | #2
On Thu, 28 Jan 2021 at 06:04, Herbert Xu <herbert@gondor.apana.org.au> 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?
Herbert Xu Jan. 28, 2021, 10:39 a.m. UTC | #3
On Thu, Jan 28, 2021 at 11:30:23AM +0100, Ard Biesheuvel wrote:
>
> But this also begs the question which ecdh-nist-pXXX implementations
> we actually need? Why are we exposing these curves in the first place?

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.

Thanks,
Stefan Berger Jan. 29, 2021, 2:49 a.m. UTC | #4
On 1/28/21 5:30 AM, Ard Biesheuvel wrote:
> On Thu, 28 Jan 2021 at 06:04, Herbert Xu <herbert@gondor.apana.org.au> 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
Herbert Xu Jan. 29, 2021, 3 a.m. UTC | #5
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,
Meng Yu Feb. 1, 2021, 3:45 a.m. UTC | #6
在 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!
Daniele Alessandrelli Feb. 1, 2021, 5:09 p.m. UTC | #7
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.
Herbert Xu Feb. 2, 2021, 5:13 a.m. UTC | #8
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,
Alessandrelli, Daniele Feb. 2, 2021, 9:27 a.m. UTC | #9
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?
Herbert Xu Feb. 2, 2021, 9:42 a.m. UTC | #10
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,
Alessandrelli, Daniele Feb. 2, 2021, 12:35 p.m. UTC | #11
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/20201217172101.381772-1-daniele.alessandrelli@linux.intel.com/

> 
> Cheers,
Saulo Alessandre de Lima Feb. 3, 2021, 6:03 p.m. UTC | #12
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 <saulo.alessandre@gmail.com>
Herbert Xu Feb. 4, 2021, 5:31 a.m. UTC | #13
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,
Herbert Xu Feb. 4, 2021, 5:41 a.m. UTC | #14
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,
Vitaly Chikunov Feb. 8, 2021, 6:35 a.m. UTC | #15
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?


> 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 <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Ard Biesheuvel Feb. 8, 2021, 6:47 a.m. UTC | #16
On Mon, 8 Feb 2021 at 07:37, Vitaly Chikunov <vt@altlinux.org> 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 <herbert@gondor.apana.org.au>
> > Home Page: http://gondor.apana.org.au/~herbert/
> > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Vitaly Chikunov Feb. 8, 2021, 9:27 p.m. UTC | #17
Ard,

On Mon, Feb 08, 2021 at 07:47:44AM +0100, Ard Biesheuvel wrote:
> On Mon, 8 Feb 2021 at 07:37, Vitaly Chikunov <vt@altlinux.org> 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
diff mbox series

Patch

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