URL: https://github.com/SSSD/sssd/pull/843 Author: sumit-bose Title: #843: p11_child: prefer better digest function if card supports it Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/843/head:pr843 git checkout pr843
From 48138736900bf6cdce0d61b10eb987fb6bd8335e Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Tue, 2 Jul 2019 17:11:50 +0200 Subject: [PATCH 1/2] p11_child: prefer better digest function if card supports it To improve FIPS compliance and security in general p11_child now checks which message digest functions (hashes) are support for RSA keys and tries to use the highest bit length supported. For EC keys sha512 is used unconditionally. Related to https://pagure.io/SSSD/sssd/issue/4039 --- src/p11_child/p11_child_openssl.c | 87 +++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 5 deletions(-) diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c index 75e4bef8e1..a5d694e0a7 100644 --- a/src/p11_child/p11_child_openssl.c +++ b/src/p11_child/p11_child_openssl.c @@ -1097,7 +1097,75 @@ static int rs_to_seq(TALLOC_CTX *mem_ctx, CK_BYTE *rs_sig, CK_ULONG rs_sig_len, return EOK; } +static CK_RV get_preferred_rsa_mechanism(TALLOC_CTX *mem_ctx, + CK_FUNCTION_LIST *module, + CK_SLOT_ID slot_id, + CK_MECHANISM_TYPE *preferred_mechanism, + const EVP_MD **preferred_evp_md) +{ + CK_ULONG count; + CK_MECHANISM_TYPE *mechanism_list = NULL; + CK_RV rv; + size_t c; + size_t m; + struct prefs { + CK_MECHANISM_TYPE mech; + const char *mech_name; + const EVP_MD *evp_md; + const char *md_name; + } prefs[] = { + { CKM_SHA512_RSA_PKCS, "CKM_SHA512_RSA_PKCS", EVP_sha512(), "sha512" }, + { CKM_SHA384_RSA_PKCS, "CKM_SHA384_RSA_PKCS", EVP_sha384(), "sha384" }, + { CKM_SHA256_RSA_PKCS, "CKM_SHA256_RSA_PKCS", EVP_sha256(), "sha256" }, + { CKM_SHA224_RSA_PKCS, "CKM_SHA224_RSA_PKCS", EVP_sha224(), "sha224" }, + { CKM_SHA1_RSA_PKCS, "CKM_SHA1_RSA_PKCS", EVP_sha1(), "sha1" }, + { 0, NULL } + }; + + *preferred_mechanism = CKM_SHA1_RSA_PKCS; + *preferred_evp_md = EVP_sha1(); + + rv = module->C_GetMechanismList(slot_id, NULL, &count); + if (rv == CKR_OK && count > 0) { + mechanism_list = talloc_size(mem_ctx, + count * sizeof(CK_MECHANISM_TYPE)); + if (mechanism_list != NULL) { + rv = module->C_GetMechanismList(slot_id, mechanism_list, &count); + if (rv == CKR_OK) { + if (DEBUG_IS_SET(SSSDBG_TRACE_ALL)) { + for (m = 0; m < count; m++) { + DEBUG(SSSDBG_TRACE_ALL, "Found mechanism [%lu].\n", + mechanism_list[m]); + } + } + for (c = 0; prefs[c].mech != 0; c++) { + for (m = 0; m < count; m++) { + if (prefs[c].mech == mechanism_list[m]) { + *preferred_mechanism = prefs[c].mech; + *preferred_evp_md = prefs[c].evp_md; + DEBUG(SSSDBG_FUNC_DATA, + "Using PKCS#11 mechanism [%lu][%s] and " + "local message digest [%s].\n", + *preferred_mechanism, prefs[c].mech_name, + prefs[c].md_name); + break; + } + } + if (m != count) { + break; + } + } + } + } + } + + talloc_free(mechanism_list); + + return rv; +} + static int sign_data(CK_FUNCTION_LIST *module, CK_SESSION_HANDLE session, + CK_SLOT_ID slot_id, struct cert_list *cert) { CK_OBJECT_CLASS key_class = CKO_PRIVATE_KEY; @@ -1108,6 +1176,7 @@ static int sign_data(CK_FUNCTION_LIST *module, CK_SESSION_HANDLE session, {CKA_ID, NULL, 0} }; CK_MECHANISM mechanism = { CK_UNAVAILABLE_INFORMATION, NULL, 0 }; + CK_MECHANISM_TYPE preferred_mechanism; CK_OBJECT_HANDLE priv_key_object; CK_ULONG object_count; CK_BYTE random_value[128]; @@ -1157,15 +1226,23 @@ static int sign_data(CK_FUNCTION_LIST *module, CK_SESSION_HANDLE session, switch (get_key_type(module, session, priv_key_object)) { case CKK_RSA: - DEBUG(SSSDBG_TRACE_ALL, "Found RSA key using CKM_SHA1_RSA_PKCS.\n"); - mechanism.mechanism = CKM_SHA1_RSA_PKCS; - evp_md = EVP_sha1(); + rv = get_preferred_rsa_mechanism(cert, module, slot_id, + &preferred_mechanism, &evp_md); + if (rv != CKR_OK) { + DEBUG(SSSDBG_OP_FAILURE, "get_preferred_rsa_mechanism failed, " + "using default CKM_SHA1_RSA_PKCS.\n"); + preferred_mechanism = CKM_SHA1_RSA_PKCS; + evp_md = EVP_sha1(); + } + DEBUG(SSSDBG_TRACE_ALL, "Found RSA key using mechanism [%lu].\n", + preferred_mechanism); + mechanism.mechanism = preferred_mechanism; card_does_hash = true; break; case CKK_EC: DEBUG(SSSDBG_TRACE_ALL, "Found ECC key using CKM_ECDSA.\n"); mechanism.mechanism = CKM_ECDSA; - evp_md = EVP_sha1(); + evp_md = EVP_sha512(); card_does_hash = false; break; case CK_UNAVAILABLE_INFORMATION: @@ -1661,7 +1738,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx, goto done; } - ret = sign_data(module, session, cert_list); + ret = sign_data(module, session, slot_id, cert_list); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sign_data failed.\n"); ret = EACCES; From e15c3cd9fbb5bfb8a66ab68cf2853203b8286edb Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Tue, 2 Jul 2019 21:58:15 +0200 Subject: [PATCH 2/2] p11_child: fix memory leak EVP_MD_CTX_create() was called without matching EVP_MD_CTX_destroy(). --- src/p11_child/p11_child_openssl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c index a5d694e0a7..fb2ebbb1cb 100644 --- a/src/p11_child/p11_child_openssl.c +++ b/src/p11_child/p11_child_openssl.c @@ -1187,7 +1187,7 @@ static int sign_data(CK_FUNCTION_LIST *module, CK_SESSION_HANDLE session, CK_RV rv; CK_RV rv_f; EVP_PKEY *cert_pub_key = NULL; - EVP_MD_CTX *md_ctx; + EVP_MD_CTX *md_ctx = NULL; int ret; const EVP_MD *evp_md = NULL; CK_BYTE *hash_val = NULL; @@ -1357,6 +1357,7 @@ static int sign_data(CK_FUNCTION_LIST *module, CK_SESSION_HANDLE session, ret = EOK; done: + EVP_MD_CTX_destroy(md_ctx); talloc_free(signature); EVP_PKEY_free(cert_pub_key);
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org