From 8b8fd22a168860c5034822472d1fb5745f8fa0f5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Wed, 16 Jun 2021 10:18:30 +0200
Subject: [PATCH] Fix lasso_query_sign HMAC other than SHA1 (#54037)
The switch clause was using SHA1 digests for all digest types when
signing. This obviously breaks verifying the signatures if HMAC-SHAXXX
is used and XXX is something else than 1.
---
lasso/xml/tools.c | 35 +++++++++++++++++++++++------------
tests/login_tests_saml2.c | 6 +++---
2 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/lasso/xml/tools.c b/lasso/xml/tools.c
index 96d88a2c4..290fd55f2 100644
--- a/lasso/xml/tools.c
+++ b/lasso/xml/tools.c
@@ -594,22 +594,20 @@ lasso_query_sign(char *query, LassoSignatureContext context)
sigret_size = DSA_size(dsa);
break;
case LASSO_SIGNATURE_METHOD_HMAC_SHA1:
+ md = EVP_sha1();
+ sigret_size = EVP_MD_size(md);
+ break;
case LASSO_SIGNATURE_METHOD_HMAC_SHA256:
+ md = EVP_sha256();
+ sigret_size = EVP_MD_size(md);
+ break;
case LASSO_SIGNATURE_METHOD_HMAC_SHA384:
+ md = EVP_sha384();
+ sigret_size = EVP_MD_size(md);
+ break;
case LASSO_SIGNATURE_METHOD_HMAC_SHA512:
- if ((rc = lasso_get_hmac_key(key, (void**)&hmac_key,
- &hmac_key_length))) {
- message(G_LOG_LEVEL_CRITICAL, "Failed to get hmac key (%s)", lasso_strerror(rc));
- goto done;
- }
- g_assert(hmac_key);
- md = EVP_sha1();
+ md = EVP_sha512();
sigret_size = EVP_MD_size(md);
- /* key should be at least 128 bits long */
- if (hmac_key_length < 16) {
- critical("HMAC key should be at least 128 bits long");
- goto done;
- }
break;
default:
g_assert_not_reached();
@@ -645,6 +643,19 @@ lasso_query_sign(char *query, LassoSignatureContext context)
case LASSO_SIGNATURE_METHOD_HMAC_SHA256:
case LASSO_SIGNATURE_METHOD_HMAC_SHA384:
case LASSO_SIGNATURE_METHOD_HMAC_SHA512:
+ if ((rc = lasso_get_hmac_key(key, (void**)&hmac_key,
+ &hmac_key_length))) {
+ message(G_LOG_LEVEL_CRITICAL, "Failed to get hmac key (%s)", lasso_strerror(rc));
+ goto done;
+ }
+ g_assert(hmac_key);
+
+ /* key should be at least 128 bits long */
+ if (hmac_key_length < 16) {
+ critical("HMAC key should be at least 128 bits long");
+ goto done;
+ }
+
HMAC(md, hmac_key, hmac_key_length, (unsigned char *)new_query,
strlen(new_query), sigret, &siglen);
status = 1;
diff --git a/tests/login_tests_saml2.c b/tests/login_tests_saml2.c
index e331c07a7..e1d78b5b1 100644
--- a/tests/login_tests_saml2.c
+++ b/tests/login_tests_saml2.c
@@ -981,7 +981,7 @@ sso_initiated_by_sp(LassoServer *idp_context, LassoServer *sp_context, SsoCallba
lasso_release_gobject(sp_login_context);
}
-START_TEST(test07_sso_sp_with_hmac_sha1_signatures)
+START_TEST(test07_sso_sp_with_hmac_sha256_signatures)
{
LassoServer *idp_context = NULL;
LassoServer *sp_context = NULL;
@@ -990,7 +990,7 @@ START_TEST(test07_sso_sp_with_hmac_sha1_signatures)
/* Create the shared key */
key = lasso_key_new_for_signature_from_memory("xxxxxxxxxxxxxxxx", 16,
- NULL, LASSO_SIGNATURE_METHOD_HMAC_SHA1, NULL);
+ NULL, LASSO_SIGNATURE_METHOD_HMAC_SHA256, NULL);
check_true(LASSO_IS_KEY(key));
/* Create an IdP context for IdP initiated SSO with provider metadata 1 */
@@ -1640,7 +1640,7 @@ login_saml2_suite()
tcase_add_test(tc_spSloSoap, test04_sso_then_slo_soap);
tcase_add_test(tc_idpKeyRollover, test05_sso_idp_with_key_rollover);
tcase_add_test(tc_spKeyRollover, test06_sso_sp_with_key_rollover);
- tcase_add_test(tc_hmacSignature, test07_sso_sp_with_hmac_sha1_signatures);
+ tcase_add_test(tc_hmacSignature, test07_sso_sp_with_hmac_sha256_signatures);
tcase_add_test(tc_spLogin, test08_test_authnrequest_flags);
tcase_add_test(tc_ecp, test09_ecp);
tcase_add_test(tc_ecp, test10_ecp);
--
2.26.3