tpm2-software / tpm2-pkcs11

A PKCS#11 interface for TPM2 hardware
https://tpm2-software.github.io
Other
265 stars 106 forks source link

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

Open ricardosalveti opened 1 year ago

ricardosalveti commented 1 year ago

With https://github.com/openssl/openssl/pull/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/openssl-users@openssl.org/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 commented 1 year ago

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 commented 3 weeks ago

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.