Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EVP_PKEY_fromdata_init fails in sign_init when using openssl 3 + pkcs11 engine #830

Open
ricardosalveti opened this issue Mar 14, 2023 · 2 comments

Comments

@ricardosalveti
Copy link

With openssl/openssl#20463 I'm able to use openssl 3 + pkcs11 engine + tpm2-pkcs11, but I always get an error message in sign_init (when performing the normal sign operation) in EVP_PKEY_fromdata_init:

ERROR: EVP_PKEY_fromdata_init: %s: error:03000096:digital envelope routines::operation not supported for this keytype

This error message does not cause a failure, and sign works correctly later on, but I'm trying to understand if this is indeed expected or if an issue that is not necessarily being handled in the tpm2-pkcs11 codebase.

Looking at https://github.com/tpm2-software/tpm2-pkcs11/blob/master/src/lib/ssl_util.c#L219 I see that the error is being handled, but later on the function returns CKR_OK, allowing the logic in common_init to proceed. From what it looks like not having pkey set is ok for sign, since it seems it is only used for verify (which would explain why sign still works fine).

Investigating a bit further I was able to find a post in the openssl ml where this same issue is discussed (https://www.mail-archive.com/[email protected]/msg90614.html), and it seems that EVP_PKEY_fromdata only works when provider is used instead, which would indeed explain the error.

As I'm still getting familiar with the codebase, should we skip setting up pkey when sign is used instead? I was also able to confirm that verify is working correctly with openssl + engine + pkcs11 + tpm2-pkcs11.

@ricardosalveti
Copy link
Author

A quick patch like below is enough to get rid of the failure and sign/verify works ok with it:

diff --git a/src/lib/sign.c b/src/lib/sign.c
index 20494ea..e78c304 100644
--- a/src/lib/sign.c
+++ b/src/lib/sign.c
@@ -36,7 +36,7 @@ struct sign_opdata {
     const EVP_MD *md;
 };

-static sign_opdata *sign_opdata_new(mdetail *mdtl, CK_MECHANISM_PTR mechanism, tobject *tobj) {
+static sign_opdata *sign_opdata_new(operation op, mdetail *mdtl, CK_MECHANISM_PTR mechanism, tobject *tobj) {

     int padding = 0;
     CK_RV rv = mech_get_padding(mdtl,
@@ -74,9 +74,11 @@ static sign_opdata *sign_opdata_new(mdetail *mdtl, CK_MECHANISM_PTR mechanism, t
     }

     EVP_PKEY *pkey = NULL;
-    rv = ssl_util_attrs_to_evp(tobj->attrs, &pkey);
-    if (rv != CKR_OK) {
-        return NULL;
+    if (op != operation_sign) {
+        rv = ssl_util_attrs_to_evp(tobj->attrs, &pkey);
+        if (rv != CKR_OK) {
+            return NULL;
+        }
     }

     sign_opdata *opdata = calloc(1, sizeof(sign_opdata));
@@ -226,7 +228,7 @@ static CK_RV common_init(operation op, session_ctx *ctx, CK_MECHANISM_PTR mechan
         }
     }

-    sign_opdata *opdata = sign_opdata_new(tok->mdtl,
+    sign_opdata *opdata = sign_opdata_new(op, tok->mdtl,
             mechanism, tobj);
     if (!opdata) {
         tpm_opdata_free(&tpm_opdata);

But wanted to understand better if this is indeed the right approach.

@fdub
Copy link

fdub commented Jun 25, 2024

For me, using the openssl utility from the command-line, signing with an RSA key through the pkcs11 engine always leads to a 'Host memory error'.
20E025B97F000000:error:41800002:PKCS#11 module:ERR_CKR_error:Host memory error:p11_rsa.c:120:

I could step through the code and see that the error described above is causing it. When applying the proposed patch, everything works fine.

I would greatly appreciate a solution for this without having to apply a patch that hasn't been reviewed and approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants