| Message ID | 20230509170517.2637245-1-selva.nair@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Bugfix: dangling pointer passed to pkcs11-helper | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> The patch itself looks trivial ("just move the structure to the outer scope"). The interesting bit is "why" :-) - "set_pss_params()" is harmless (it just fills the data in the structure), but then mech.pParameter is set to &pss_params, and *this* is passed to pkcs11h_certificate_signAny_ex() a few lines down, outside the original scope. So - the patch makes sense, the bug is obvious in hindsight, and we also have a confirmation in #323 that the patch fixes a real problem "depending on compiler and OS", nasty. For testing, I have only used the GH Action builds - there's two instances that build with --enable-pkcs11 and run the tests (and I have no suitable setup locally). [==========] Running 3 test(s). Slot 0 has a free/uninitialized token. The token has been initialized and is reassigned to slot 379532672 [ RUN ] test_pkcs11_ids [ OK ] test_pkcs11_ids [ RUN ] test_tls_ctx_use_pkcs11 [ OK ] test_tls_ctx_use_pkcs11 [ RUN ] test_tls_ctx_use_pkcs11__management [ OK ] test_tls_ctx_use_pkcs11__management Found token (541bef49-4423-01c1-e7c6-600c169f3580) with matching token label. The token (softhsm2_tokens_Fi02IS/541bef49-4423-01c1-e7c6-600c169f3580) has been deleted. [==========] 3 test(s) run. [ PASSED ] 3 test(s). PASS: pkcs11_testdriver Your patch has been applied to the master branch. commit f4850745709c5b80ab7d09c03a86c5ceea6d10a2 (master) commit 7e4becb4cd8be7f0d5ff80cf80877ea152f99830 (release/2.6) Author: Selva Nair Date: Tue May 9 13:05:17 2023 -0400 Bugfix: dangling pointer passed to pkcs11-helper Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20230509170517.2637245-1-selva.nair@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26640.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/pkcs11_openssl.c b/src/openvpn/pkcs11_openssl.c index eee86e17..9b0ab39f 100644 --- a/src/openvpn/pkcs11_openssl.c +++ b/src/openvpn/pkcs11_openssl.c @@ -165,6 +165,7 @@ xkey_pkcs11h_sign(void *handle, unsigned char *sig, { pkcs11h_certificate_t cert = handle; CK_MECHANISM mech = {CKM_RSA_PKCS, NULL, 0}; /* default value */ + CK_RSA_PKCS_PSS_PARAMS pss_params = {0}; unsigned char buf[EVP_MAX_MD_SIZE]; size_t buflen; @@ -203,7 +204,6 @@ xkey_pkcs11h_sign(void *handle, unsigned char *sig, } else if (!strcmp(sigalg.padmode, "pss")) { - CK_RSA_PKCS_PSS_PARAMS pss_params = {0}; mech.mechanism = CKM_RSA_PKCS_PSS; if (!set_pss_params(&pss_params, sigalg, cert))