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