add gm algorithm support.

[chench246]

add gm algorithm support.

[kalvdans]

Some comments, I'm not very familiar with the source code yet, feel free to ignore them!

[kalvdans]

The patch looks good to me now! I don't have merge rights though. Thanks for your patience!

[chench246]

@AndreasFuchsTPM @williamcroberts Would you mind taking a look at this PR and sharing your feedback? Thanks!

[williamcroberts]

Overall looks really good, some minor style nits. Major item to work on is a test, we need a test in the test suite.

[chench246]

Overall looks really good, some minor style nits. Major item to work on is a test, we need a test in the test suite.

Thanks for the feedback. I’ve fixed the style issues in the latest revision — please take a look. I’ll also add the corresponding test cases to the test suite shortly.

[williamcroberts]

LGTM, just needs a test now. Thanks, nice code.

[chench246]

LGTM, just needs a test now. Thanks, nice code.

Sorry for the late reply. I’ve added some test cases in the latest revision. Could you please take another look? Thanks.

[williamcroberts]

Looks good, we'll see how the CI turns out

[chench246]

Sorry to bother you. I just wanted to check whether there has been any progress on the review of this PR. Also, from what I can see, the CI failures do not appear to be related to my changes.

[williamcroberts]

Sorry to bother you. I just wanted to check whether there has been any progress on the review of this PR. Also, from what I can see, the CI failures do not appear to be related to my changes.

Sorry, sick. Yeah, your PR seems fine. We need to invest some time into sorting the CI out and then taking this change. It's just a matter of time at this point.

[chench246]

Sorry to bother you. I just wanted to check whether there has been any progress on the review of this PR. Also, from what I can see, the CI failures do not appear to be related to my changes.

Sorry, sick. Yeah, your PR seems fine. We need to invest some time into sorting the CI out and then taking this change. It's just a matter of time at this point.

Sorry to hear that, hope you feel better soon. Thanks for the update.

[williamcroberts]

LGTM, @AndreasFuchsTPM what do you think?

[chench246]

Thanks for the LGTM. Gentle ping to follow up on this PR, as it has been open for a little over two months.
@AndreasFuchsTPM Could you please take a look when you have a chance?
I’d be happy to address any further comments if needed.

[chench246]

Is this still waiting for Andreas' review, or is there another maintainer who can make the final merge decision? @williamcroberts

[williamcroberts]

Is this still waiting for Andreas' review, or is there another maintainer who can make the final merge decision? @williamcroberts

I just sent an email to @AndreasFuchsTPM , I'd like to wait on him, but if he doesn't get to it in a week, I'll merge it.

[AndreasFuchsTPM]

Looks good in general, but CI is failing.
Unfortunately it does not show logs anymore.
Could you force-push something to trigger CI again and then fix the errors ?

[chench246]

Looks good in general, but CI is failing. Unfortunately it does not show logs anymore. Could you force-push something to trigger CI again and then fix the errors ?

Thanks. I force-pushed a new commit to retrigger CI.

It looks like the workflow was created, but it is in action_required state and no jobs were started:
https://github.com/tpm2-software/tpm2-tss-engine/actions/runs/28572942690

Could a maintainer please approve/run the workflow? Once it actually runs, I’ll check the logs and fix any issues related to this PR. @williamcroberts @AndreasFuchsTPM

[williamcroberts]

Looks good in general, but CI is failing. Unfortunately it does not show logs anymore. Could you force-push something to trigger CI again and then fix the errors ?

Thanks. I force-pushed a new commit to retrigger CI.

It looks like the workflow was created, but it is in action_required state and no jobs were started: https://github.com/tpm2-software/tpm2-tss-engine/actions/runs/28572942690

Could a maintainer please approve/run the workflow? Once it actually runs, I’ll check the logs and fix any issues related to this PR. @williamcroberts @AndreasFuchsTPM

Yeah I'll try and get to it in the next 5 days. Stay tuned.

Nevermind, I misread what you typed, approved.

[chench246]

Looks good in general, but CI is failing. Unfortunately it does not show logs anymore. Could you force-push something to trigger CI again and then fix the errors ?

Thanks. I force-pushed a new commit to retrigger CI.
It looks like the workflow was created, but it is in action_required state and no jobs were started: https://github.com/tpm2-software/tpm2-tss-engine/actions/runs/28572942690
Could a maintainer please approve/run the workflow? Once it actually runs, I’ll check the logs and fix any issues related to this PR. @williamcroberts @AndreasFuchsTPM

Yeah I'll try and get to it in the next 5 days. Stay tuned.

Nevermind, I misread what you typed, approved.

Thanks for approving and rerunning CI.

From the job summary, the failures do not appear to be caused by this PR:

• whitespace-check passed.
• multi-arch (ubuntu-20.04.arm32v7, 3.0.x, 4.0) failed in Setup QEMU, before checkout/building this patch.
• build-test (fedora-32, 3.0.x, 4.0, gcc) failed inside tpm2-software/ci/runCI@main.
• The same QEMU/Fedora failure pattern was already present in previous CI runs for this PR.

Based on the failing steps this looks like an existing CI/environment issue rather than an issue introduced by this patch.

[williamcroberts]

Thanks for approving and rerunning CI.

From the job summary, the failures do not appear to be caused by this PR:

• whitespace-check passed.
• multi-arch (ubuntu-20.04.arm32v7, 3.0.x, 4.0) failed in Setup QEMU, before checkout/building this patch.
• build-test (fedora-32, 3.0.x, 4.0, gcc) failed inside tpm2-software/ci/runCI@main.
• The same QEMU/Fedora failure pattern was already present in previous CI runs for this PR.

Based on the failing steps this looks like an existing CI/environment issue rather than an issue introduced by this patch.

Yeah my understanding as well. Which now back to my original comment, give me 5 days, I'll pull it locally and try it. Then one of the maintainers, myself included, needs to look at the CI stuff.

[AndreasFuchsTPM]

CI triggers a lot of errors:

../src/tpm2-tss-engine-rsa.c: In function 'rsa_pkey_copy':
../src/tpm2-tss-engine-rsa.c:645:34: warning: passing argument 2 of 'digest_sign_copy' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  645 |     return digest_sign_copy(dst, src);
      |                                  ^~~
../src/tpm2-tss-engine-ecc.c: In function 'ecdsa_pkey_copy':
../src/tpm2-tss-engine-ecc.c:428:34: warning: passing argument 2 of 'digest_sign_copy' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  428 |     return digest_sign_copy(dst, src);
      |                                  ^~~
In file included from ../src/tpm2-tss-engine-rsa.c:38:
../src/tpm2-tss-engine-common.h:195:51: note: expected 'EVP_PKEY_CTX *' {aka 'struct evp_pkey_ctx_st *'} but argument is of type 'const EVP_PKEY_CTX *' {aka 'const struct evp_pkey_ctx_st *'}
  195 | digest_sign_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src);
      |                                     ~~~~~~~~~~~~~~^~~
In file included from ../src/tpm2-tss-engine-ecc.c:45:
../src/tpm2-tss-engine-common.h:195:51: note: expected 'EVP_PKEY_CTX *' {aka 'struct evp_pkey_ctx_st *'} but argument is of type 'const EVP_PKEY_CTX *' {aka 'const struct evp_pkey_ctx_st *'}
  195 | digest_sign_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src);
      |                                     ~~~~~~~~~~~~~~^~~
../src/tpm2-tss-engine-rsa.c: In function 'init_rsa':
../src/tpm2-tss-engine-rsa.c:802:46: warning: passing argument 2 of 'EVP_PKEY_meth_get_copy' from incompatible pointer type [-Wincompatible-pointer-types]
  802 |     EVP_PKEY_meth_get_copy(pkey_rsa_methods, &rsa_pkey_orig_copy);
      |                                              ^~~~~~~~~~~~~~~~~~~
      |                                              |
      |                                              int (**)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *) {aka int (**)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)}
In file included from /usr/include/openssl/pem.h:16,
                 from /usr/include/openssl/ui.h:19,
                 from /usr/include/openssl/engine.h:24,
                 from ../src/tpm2-tss-engine-rsa.c:34:
/usr/include/openssl/evp.h:1549:36: note: expected 'int (**)(EVP_PKEY_CTX *, EVP_PKEY_CTX *)' {aka 'int (**)(struct evp_pkey_ctx_st *, struct evp_pkey_ctx_st *)'} but argument is of type 'int (**)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *)' {aka 'int (**)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)'}
 1549 |                             int (**pcopy) (EVP_PKEY_CTX *dst,
      |                             ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
 1550 |                                            EVP_PKEY_CTX *src));
      |                                            ~~~~~~~~~~~~~~~~~~
../src/tpm2-tss-engine-ecc.c: In function 'init_ecc':
../src/tpm2-tss-engine-ecc.c:1025:46: warning: passing argument 2 of 'EVP_PKEY_meth_get_copy' from incompatible pointer type [-Wincompatible-pointer-types]
 1025 |     EVP_PKEY_meth_get_copy(pkey_ecc_methods, &ecdsa_pkey_orig_copy);
      |                                              ^~~~~~~~~~~~~~~~~~~~~
      |                                              |
      |                                              int (**)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *) {aka int (**)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)}
In file included from /usr/include/openssl/pem.h:16,
                 from /usr/include/openssl/ui.h:19,
                 from /usr/include/openssl/engine.h:24,
                 from ../src/tpm2-tss-engine-ecc.c:34:
/usr/include/openssl/evp.h:1549:36: note: expected 'int (**)(EVP_PKEY_CTX *, EVP_PKEY_CTX *)' {aka 'int (**)(struct evp_pkey_ctx_st *, struct evp_pkey_ctx_st *)'} but argument is of type 'int (**)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *)' {aka 'int (**)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)'}
 1549 |                             int (**pcopy) (EVP_PKEY_CTX *dst,
      |                             ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
 1550 |                                            EVP_PKEY_CTX *src));
      |                                            ~~~~~~~~~~~~~~~~~~
../src/tpm2-tss-engine-rsa.c:805:46: warning: passing argument 2 of 'EVP_PKEY_meth_set_copy' from incompatible pointer type [-Wincompatible-pointer-types]
  805 |     EVP_PKEY_meth_set_copy(pkey_rsa_methods, rsa_pkey_copy);
      |                                              ^~~~~~~~~~~~~
      |                                              |
      |                                              int (*)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *) {aka int (*)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)}
../src/tpm2-tss-engine-ecc.c:1028:46: warning: passing argument 2 of 'EVP_PKEY_meth_set_copy' from incompatible pointer type [-Wincompatible-pointer-types]
 1028 |     EVP_PKEY_meth_set_copy(pkey_ecc_methods, ecdsa_pkey_copy);
      |                                              ^~~~~~~~~~~~~~~
      |                                              |
      |                                              int (*)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *) {aka int (*)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)}
In file included from /usr/include/openssl/pem.h:16,
                 from /usr/include/openssl/ui.h:19,
                 from /usr/include/openssl/engine.h:24,
                 from ../src/tpm2-tss-engine-rsa.c:34:
/usr/include/openssl/evp.h:1430:35: note: expected 'int (*)(EVP_PKEY_CTX *, EVP_PKEY_CTX *)' {aka 'int (*)(struct evp_pkey_ctx_st *, struct evp_pkey_ctx_st *)'} but argument is of type 'int (*)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *)' {aka 'int (*)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)'}
 1430 |                             int (*copy) (EVP_PKEY_CTX *dst,
      |                             ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
 1431 |                                          EVP_PKEY_CTX *src));
      |                                          ~~~~~~~~~~~~~~~~~~
In file included from /usr/include/openssl/pem.h:16,
                 from /usr/include/openssl/ui.h:19,
                 from /usr/include/openssl/engine.h:24,
                 from ../src/tpm2-tss-engine-ecc.c:34:
/usr/include/openssl/evp.h:1430:35: note: expected 'int (*)(EVP_PKEY_CTX *, EVP_PKEY_CTX *)' {aka 'int (*)(struct evp_pkey_ctx_st *, struct evp_pkey_ctx_st *)'} but argument is of type 'int (*)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *)' {aka 'int (*)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)'}
 1430 |                             int (*copy) (EVP_PKEY_CTX *dst,
      |                             ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
 1431 |                                          EVP_PKEY_CTX *src));
      |                                          ~~~~~~~~~~~~~~~~~~
  CCLD     libtpm2tss.la
  CCLD     tpm2tss-genkey
++ nproc
+ make -j4 check
make  test/error_tpm2-tss-engine-common test/tpm2-tss-engine-common
make[1]: Entering directory '/workspace/tpm2-tss-engine/build'
  CC       src/test_error_tpm2_tss_engine_common-tpm2-tss-engine.o
  CC       src/test_error_tpm2_tss_engine_common-tpm2-tss-engine-common.o
  CC       src/test_error_tpm2_tss_engine_common-tpm2-tss-engine-digest-sign.o
  CC       src/test_error_tpm2_tss_engine_common-tpm2-tss-engine-err.o
  CC       src/test_error_tpm2_tss_engine_common-tpm2-tss-engine-ecc.o
  CC       src/test_error_tpm2_tss_engine_common-tpm2-tss-engine-rand.o
  CC       src/test_error_tpm2_tss_engine_common-tpm2-tss-engine-rsa.o
../src/tpm2-tss-engine-ecc.c: In function 'ecdsa_pkey_copy':
../src/tpm2-tss-engine-ecc.c:428:34: warning: passing argument 2 of 'digest_sign_copy' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  428 |     return digest_sign_copy(dst, src);
      |                                  ^~~
In file included from ../src/tpm2-tss-engine-ecc.c:45:
../src/tpm2-tss-engine-common.h:195:51: note: expected 'EVP_PKEY_CTX *' {aka 'struct evp_pkey_ctx_st *'} but argument is of type 'const EVP_PKEY_CTX *' {aka 'const struct evp_pkey_ctx_st *'}
  195 | digest_sign_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src);
      |                                     ~~~~~~~~~~~~~~^~~
../src/tpm2-tss-engine-ecc.c: In function 'init_ecc':
../src/tpm2-tss-engine-ecc.c:1025:46: warning: passing argument 2 of 'EVP_PKEY_meth_get_copy' from incompatible pointer type [-Wincompatible-pointer-types]
 1025 |     EVP_PKEY_meth_get_copy(pkey_ecc_methods, &ecdsa_pkey_orig_copy);
      |                                              ^~~~~~~~~~~~~~~~~~~~~
      |                                              |
      |                                              int (**)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *) {aka int (**)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)}
In file included from /usr/include/openssl/pem.h:16,
                 from /usr/include/openssl/ui.h:19,
                 from /usr/include/openssl/engine.h:24,
      |                             ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
 1431 |                                          EVP_PKEY_CTX *src));
      |                                          ~~~~~~~~~~~~~~~~~~
../src/tpm2-tss-engine-rsa.c: In function 'rsa_pkey_copy':
../src/tpm2-tss-engine-rsa.c:645:34: warning: passing argument 2 of 'digest_sign_copy' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  645 |     return digest_sign_copy(dst, src);
      |                                  ^~~
In file included from ../src/tpm2-tss-engine-rsa.c:38:
../src/tpm2-tss-engine-common.h:195:51: note: expected 'EVP_PKEY_CTX *' {aka 'struct evp_pkey_ctx_st *'} but argument is of type 'const EVP_PKEY_CTX *' {aka 'const struct evp_pkey_ctx_st *'}
  195 | digest_sign_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src);
      |                                     ~~~~~~~~~~~~~~^~~
../src/tpm2-tss-engine-rsa.c: In function 'init_rsa':
../src/tpm2-tss-engine-rsa.c:802:46: warning: passing argument 2 of 'EVP_PKEY_meth_get_copy' from incompatible pointer type [-Wincompatible-pointer-types]
  802 |     EVP_PKEY_meth_get_copy(pkey_rsa_methods, &rsa_pkey_orig_copy);
      |                                              ^~~~~~~~~~~~~~~~~~~
      |                                              |
      |                                              int (**)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *) {aka int (**)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)}
In file included from /usr/include/openssl/pem.h:16,
                 from /usr/include/openssl/ui.h:19,
                 from /usr/include/openssl/engine.h:24,
                 from ../src/tpm2-tss-engine-rsa.c:34:
/usr/include/openssl/evp.h:1549:36: note: expected 'int (**)(EVP_PKEY_CTX *, EVP_PKEY_CTX *)' {aka 'int (**)(struct evp_pkey_ctx_st *, struct evp_pkey_ctx_st *)'} but argument is of type 'int (**)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *)' {aka 'int (**)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)'}
 1549 |                             int (**pcopy) (EVP_PKEY_CTX *dst,
      |                             ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
 1550 |                                            EVP_PKEY_CTX *src));
      |                                            ~~~~~~~~~~~~~~~~~~
../src/tpm2-tss-engine-rsa.c:805:46: warning: passing argument 2 of 'EVP_PKEY_meth_set_copy' from incompatible pointer type [-Wincompatible-pointer-types]
  805 |     EVP_PKEY_meth_set_copy(pkey_rsa_methods, rsa_pkey_copy);
      |                                              ^~~~~~~~~~~~~
      |                                              |
      |                                              int (*)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *) {aka int (*)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)}
In file included from /usr/include/openssl/pem.h:16,
                 from /usr/include/openssl/ui.h:19,
                 from /usr/include/openssl/engine.h:24,
                 from ../src/tpm2-tss-engine-rsa.c:34:
/usr/include/openssl/evp.h:1430:35: note: expected 'int (*)(EVP_PKEY_CTX *, EVP_PKEY_CTX *)' {aka 'int (*)(struct evp_pkey_ctx_st *, struct evp_pkey_ctx_st *)'} but argument is of type 'int (*)(EVP_PKEY_CTX *, const EVP_PKEY_CTX *)' {aka 'int (*)(struct evp_pkey_ctx_st *, const struct evp_pkey_ctx_st *)'}
 1430 |                             int (*copy) (EVP_PKEY_CTX *dst,
      |                             ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
 1431 |                                          EVP_PKEY_CTX *src));
      |                                          ~~~~~~~~~~~~~~~~~~
  CCLD     test/error_tpm2-tss-engine-common
  CCLD     test/tpm2-tss-engine-common

[williamcroberts]

CI triggers a lot of errors:

../src/tpm2-tss-engine-rsa.c: In function 'rsa_pkey_copy':
../src/tpm2-tss-engine-rsa.c:645:34: warning: passing argument 2 of 'digest_sign_copy' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  645 |     return digest_sign_copy(dst, src);

> ```

@AndreasFuchsTPM , yes lot's of errors, but it's not all (any) of their patch per say. I just grabbed the fedora-32 container and it won't build and run with the CI scripts with a bunch of these errors. So I/We need to fix the CI and get it working over here again. So for now, I was going to build and collect all the warnings, apply the patch, rebuild and diff the warnings. If it was good, I push it, and then in the next week or so fix the ci.

Fix looked simple, patch here: #301
So lets see if this fixes the CI and then we can rebase these changes on top.

We also need to consider what the lifespan of this project looks like with OpenSSL 3 being the standard for the most part right now. Figure out when to EOL it and try to get another release out the door?

[williamcroberts]

Ok, rebased onto a working CI master branch now, lets see what happens (fingers crossed).