To reintroduce the sm2 algorithm, the patch set did the following:
 - Reintroduce the mpi library based on libgcrypt.
 - Reintroduce ec implementation to MPI library.
 - Rework sm2 algorithm.
 - Support verification of X.509 certificates.

Gu Bowen (4):
  Revert "Revert "lib/mpi: Extend the MPI library""
  Revert "Revert "lib/mpi: Introduce ec implementation to MPI library""
  crypto/sm2: Rework sm2 alg with sig_alg backend
  crypto/sm2: support SM2-with-SM3 verification of X.509 certificates

 certs/system_keyring.c                   |    8 +
 crypto/Kconfig                           |   18 +
 crypto/Makefile                          |    8 +
 crypto/asymmetric_keys/public_key.c      |    7 +
 crypto/asymmetric_keys/x509_public_key.c |   27 +-
 crypto/sm2.c                             |  492 +++++++
 crypto/sm2signature.asn1                 |    4 +
 crypto/testmgr.c                         |    6 +
 crypto/testmgr.h                         |   57 +
 include/crypto/sm2.h                     |   31 +
 include/keys/system_keyring.h            |   13 +
 include/linux/mpi.h                      |  170 +++
 lib/crypto/mpi/Makefile                  |    2 +
 lib/crypto/mpi/ec.c                      | 1507 ++++++++++++++++++++++
 lib/crypto/mpi/mpi-add.c                 |   50 +
 lib/crypto/mpi/mpi-bit.c                 |  143 ++
 lib/crypto/mpi/mpi-cmp.c                 |   46 +-
 lib/crypto/mpi/mpi-div.c                 |   29 +
 lib/crypto/mpi/mpi-internal.h            |   10 +
 lib/crypto/mpi/mpi-inv.c                 |  143 ++
 lib/crypto/mpi/mpi-mod.c                 |  144 +++
 lib/crypto/mpi/mpicoder.c                |  336 +++++
 lib/crypto/mpi/mpih-mul.c                |   25 +
 lib/crypto/mpi/mpiutil.c                 |  182 +++
 24 files changed, 3447 insertions(+), 11 deletions(-)
 create mode 100644 crypto/sm2.c
 create mode 100644 crypto/sm2signature.asn1
 create mode 100644 include/crypto/sm2.h
 create mode 100644 lib/crypto/mpi/ec.c
 create mode 100644 lib/crypto/mpi/mpi-inv.c

-- 
2.25.1

Hi,

On Mon, Jun 30, 2025 at 09:39:30PM +0800, Gu Bowen wrote:
> To reintroduce the sm2 algorithm, the patch set did the following:
>  - Reintroduce the mpi library based on libgcrypt.
>  - Reintroduce ec implementation to MPI library.
>  - Rework sm2 algorithm.
>  - Support verification of X.509 certificates.
> 
> Gu Bowen (4):
>   Revert "Revert "lib/mpi: Extend the MPI library""
>   Revert "Revert "lib/mpi: Introduce ec implementation to MPI library""
>   crypto/sm2: Rework sm2 alg with sig_alg backend
>   crypto/sm2: support SM2-with-SM3 verification of X.509 certificates

I am less than enthusiastic about this. Firstly, I'm kind of biased
against the whole "national flag algorithms" thing. But I don't know how
much weight that argument will have here. More importantly, however,
implementing this atop MPI sounds very bad. The more MPI we can get rid
of, the better.

Is MPI constant time? Usually the good way to implement EC algorithms
like this is to very carefully work out constant time (and fast!) field
arithmetic routines, verify their correctness, and then implement your
ECC atop that. At this point, there's *lots* of work out there on doing
fast verified ECC and a bunch of different frameworks for producing good
implementations. There are also other implementations out there you
could look at that people have presumably studied a lot. This is old
news. (In 3 minutes of scrolling around, I noticed that
count_leading_zeros() on a value is used as a loop index, for example.
Maybe fine, maybe not, I dunno; this stuff requires analysis.)

On the other hand, maybe you don't care because you only implement
verification, not signing, so all info is public? If so, the fact that
you don't care about CT should probably be made pretty visible. But
either way, you should still be concerned with having an actually good &
correct implementation of which you feel strongly about the correctness.

Secondly, the MPI stuff you're proposing here adds a 25519 and 448
implementation, and support for weierstrauss, montgomery, and edwards,
and... surely you don't need all of this for SM-2. Why add all this
unused code? Presumably because you don't really understand or "own" all
of the code that you're proposing to add. And that gives me a lot of
hesitation, because somebody is going to have to maintain this, and if
the person sending patches with it isn't fully on top of it, we're not
off to a good start.

Lastly, just to nip in the bud the argument, "but weierstrauss is all
the same, so why not just have one library to do all possible
weierstrauss curves?" -- the fact that this series reintroduces the
removed "generic EC library" indicates there's actually not another user
of it, even before we get into questions of whether it's a good idea.

Jason

Hi,

On 7/3/2025 9:14 PM, Jason A. Donenfeld wrote:
> Hi,
> 
> On Mon, Jun 30, 2025 at 09:39:30PM +0800, Gu Bowen wrote:
>> To reintroduce the sm2 algorithm, the patch set did the following:
>>   - Reintroduce the mpi library based on libgcrypt.
>>   - Reintroduce ec implementation to MPI library.
>>   - Rework sm2 algorithm.
>>   - Support verification of X.509 certificates.
>>
>> Gu Bowen (4):
>>    Revert "Revert "lib/mpi: Extend the MPI library""
>>    Revert "Revert "lib/mpi: Introduce ec implementation to MPI library""
>>    crypto/sm2: Rework sm2 alg with sig_alg backend
>>    crypto/sm2: support SM2-with-SM3 verification of X.509 certificates
> 
> I am less than enthusiastic about this. Firstly, I'm kind of biased
> against the whole "national flag algorithms" thing. But I don't know how
> much weight that argument will have here. More importantly, however,
> implementing this atop MPI sounds very bad. The more MPI we can get rid
> of, the better.
> 
> Is MPI constant time? Usually the good way to implement EC algorithms
> like this is to very carefully work out constant time (and fast!) field
> arithmetic routines, verify their correctness, and then implement your
> ECC atop that. At this point, there's *lots* of work out there on doing
> fast verified ECC and a bunch of different frameworks for producing good
> implementations. There are also other implementations out there you
> could look at that people have presumably studied a lot. This is old
> news. (In 3 minutes of scrolling around, I noticed that
> count_leading_zeros() on a value is used as a loop index, for example.
> Maybe fine, maybe not, I dunno; this stuff requires analysis.)

Actually, I wasn't very familiar with MPI in the past. Previously, the 
implementation of sm2 was done through MPI, so I used it as well. 
Perhaps I could try using the ecc algorithm in the kernel.

> On the other hand, maybe you don't care because you only implement
> verification, not signing, so all info is public? If so, the fact that
> you don't care about CT should probably be made pretty visible. But
> either way, you should still be concerned with having an actually good &
> correct implementation of which you feel strongly about the correctness.
> 
> Secondly, the MPI stuff you're proposing here adds a 25519 and 448
> implementation, and support for weierstrauss, montgomery, and edwards,
> and... surely you don't need all of this for SM-2. Why add all this
> unused code? Presumably because you don't really understand or "own" all
> of the code that you're proposing to add. And that gives me a lot of
> hesitation, because somebody is going to have to maintain this, and if
> the person sending patches with it isn't fully on top of it, we're not
> off to a good start.
> 
> Lastly, just to nip in the bud the argument, "but weierstrauss is all
> the same, so why not just have one library to do all possible
> weierstrauss curves?" -- the fact that this series reintroduces the
> removed "generic EC library" indicates there's actually not another user
> of it, even before we get into questions of whether it's a good idea.

Thank you for your advice, it has been very beneficial for me as I just 
started participating in the community. I will try to implement the 
functionality with more robust code and only submit parts that I fully 
understand.

Best Regards，
Guber

On Thu, Jul 03, 2025 at 03:14:52PM +0200, Jason A. Donenfeld wrote:
> Hi,
> 
> On Mon, Jun 30, 2025 at 09:39:30PM +0800, Gu Bowen wrote:
> > To reintroduce the sm2 algorithm, the patch set did the following:
> >  - Reintroduce the mpi library based on libgcrypt.
> >  - Reintroduce ec implementation to MPI library.
> >  - Rework sm2 algorithm.
> >  - Support verification of X.509 certificates.
> > 
> > Gu Bowen (4):
> >   Revert "Revert "lib/mpi: Extend the MPI library""
> >   Revert "Revert "lib/mpi: Introduce ec implementation to MPI library""
> >   crypto/sm2: Rework sm2 alg with sig_alg backend
> >   crypto/sm2: support SM2-with-SM3 verification of X.509 certificates
> 
> I am less than enthusiastic about this. Firstly, I'm kind of biased
> against the whole "national flag algorithms" thing. But I don't know how
> much weight that argument will have here. More importantly, however,
> implementing this atop MPI sounds very bad. The more MPI we can get rid
> of, the better.
> 
> Is MPI constant time? Usually the good way to implement EC algorithms
> like this is to very carefully work out constant time (and fast!) field
> arithmetic routines, verify their correctness, and then implement your
> ECC atop that. At this point, there's *lots* of work out there on doing
> fast verified ECC and a bunch of different frameworks for producing good
> implementations. There are also other implementations out there you
> could look at that people have presumably studied a lot. This is old
> news. (In 3 minutes of scrolling around, I noticed that
> count_leading_zeros() on a value is used as a loop index, for example.
> Maybe fine, maybe not, I dunno; this stuff requires analysis.)
> 
> On the other hand, maybe you don't care because you only implement
> verification, not signing, so all info is public? If so, the fact that
> you don't care about CT should probably be made pretty visible. But
> either way, you should still be concerned with having an actually good &
> correct implementation of which you feel strongly about the correctness.
> 
> Secondly, the MPI stuff you're proposing here adds a 25519 and 448
> implementation, and support for weierstrauss, montgomery, and edwards,
> and... surely you don't need all of this for SM-2. Why add all this
> unused code? Presumably because you don't really understand or "own" all
> of the code that you're proposing to add. And that gives me a lot of
> hesitation, because somebody is going to have to maintain this, and if
> the person sending patches with it isn't fully on top of it, we're not
> off to a good start.
> 
> Lastly, just to nip in the bud the argument, "but weierstrauss is all
> the same, so why not just have one library to do all possible
> weierstrauss curves?" -- the fact that this series reintroduces the
> removed "generic EC library" indicates there's actually not another user
> of it, even before we get into questions of whether it's a good idea.

I went looking for reference implementations and came across this
"GmSSL" project and located:

https://github.com/guanzhi/GmSSL/blob/master/src/sm2_sign.c#L271
which uses some routines from
https://github.com/guanzhi/GmSSL/blob/master/src/sm2_z256.c

I have no idea what the deal actually is here -- is this any good? has
anybody looked at it? is it a random github? -- but it certainly
_resembles_ something more comfortable than the MPI code. Who knows, it
could be terrible, but you get the idea.

Jason

On Thu, Jul 3, 2025 at 3:29 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Thu, Jul 03, 2025 at 03:14:52PM +0200, Jason A. Donenfeld wrote:
> > Hi,
> >
> > On Mon, Jun 30, 2025 at 09:39:30PM +0800, Gu Bowen wrote:
> > > To reintroduce the sm2 algorithm, the patch set did the following:
> > >  - Reintroduce the mpi library based on libgcrypt.
> > >  - Reintroduce ec implementation to MPI library.
> > >  - Rework sm2 algorithm.
> > >  - Support verification of X.509 certificates.
> > >
> > > Gu Bowen (4):
> > >   Revert "Revert "lib/mpi: Extend the MPI library""
> > >   Revert "Revert "lib/mpi: Introduce ec implementation to MPI library""
> > >   crypto/sm2: Rework sm2 alg with sig_alg backend
> > >   crypto/sm2: support SM2-with-SM3 verification of X.509 certificates
> >
> > I am less than enthusiastic about this. Firstly, I'm kind of biased
> > against the whole "national flag algorithms" thing. But I don't know how
> > much weight that argument will have here. More importantly, however,
> > implementing this atop MPI sounds very bad. The more MPI we can get rid
> > of, the better.
> >
> > Is MPI constant time? Usually the good way to implement EC algorithms
> > like this is to very carefully work out constant time (and fast!) field
> > arithmetic routines, verify their correctness, and then implement your
> > ECC atop that. At this point, there's *lots* of work out there on doing
> > fast verified ECC and a bunch of different frameworks for producing good
> > implementations. There are also other implementations out there you
> > could look at that people have presumably studied a lot. This is old
> > news. (In 3 minutes of scrolling around, I noticed that
> > count_leading_zeros() on a value is used as a loop index, for example.
> > Maybe fine, maybe not, I dunno; this stuff requires analysis.)
> >
> > On the other hand, maybe you don't care because you only implement
> > verification, not signing, so all info is public? If so, the fact that
> > you don't care about CT should probably be made pretty visible. But
> > either way, you should still be concerned with having an actually good &
> > correct implementation of which you feel strongly about the correctness.
> >
> > Secondly, the MPI stuff you're proposing here adds a 25519 and 448
> > implementation, and support for weierstrauss, montgomery, and edwards,
> > and... surely you don't need all of this for SM-2. Why add all this
> > unused code? Presumably because you don't really understand or "own" all
> > of the code that you're proposing to add. And that gives me a lot of
> > hesitation, because somebody is going to have to maintain this, and if
> > the person sending patches with it isn't fully on top of it, we're not
> > off to a good start.
> >
> > Lastly, just to nip in the bud the argument, "but weierstrauss is all
> > the same, so why not just have one library to do all possible
> > weierstrauss curves?" -- the fact that this series reintroduces the
> > removed "generic EC library" indicates there's actually not another user
> > of it, even before we get into questions of whether it's a good idea.
>
> I went looking for reference implementations and came across this
> "GmSSL" project and located:
>
> https://github.com/guanzhi/GmSSL/blob/master/src/sm2_sign.c#L271
> which uses some routines from
> https://github.com/guanzhi/GmSSL/blob/master/src/sm2_z256.c
>
> I have no idea what the deal actually is here -- is this any good? has
> anybody looked at it? is it a random github? -- but it certainly
> _resembles_ something more comfortable than the MPI code. Who knows, it
> could be terrible, but you get the idea.

One thing to keep in mind with this project (and other projects) is
license compatibility with GPLv2 (I don't think the above project is
compatible)

>
> Jason

On Mon, Jun 30, 2025 at 09:39:30PM +0800, Gu Bowen wrote:
> To reintroduce the sm2 algorithm, the patch set did the following:
>  - Reintroduce the mpi library based on libgcrypt.
>  - Reintroduce ec implementation to MPI library.
>  - Rework sm2 algorithm.
>  - Support verification of X.509 certificates.

Remind me, why did we remove these?

regards,
dan carpenter

Hi,

On 7/1/2025 3:41 AM, Dan Carpenter wrote:
> On Mon, Jun 30, 2025 at 09:39:30PM +0800, Gu Bowen wrote:
>> To reintroduce the sm2 algorithm, the patch set did the following:
>>   - Reintroduce the mpi library based on libgcrypt.
>>   - Reintroduce ec implementation to MPI library.
>>   - Rework sm2 algorithm.
>>   - Support verification of X.509 certificates.
> 
> Remind me, why did we remove these?
> 

At first, the process of calculating the digest with the SM2 certificate
was coupled with the signature verification process, and this 
unreasonable situation was corrected with commit e5221fa6a355 ("KEYS: 
asymmetric: Move sm2 code into x509_public_key "). However, this commit 
also caused SM2 to be unable to verify secondary certificates due to its 
special implementation. This issue was not resolved, which led to the 
removal of the sm2 algorithm.