[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#989358: marked as done (unblock: lasso/2.6.1-3)



Your message dated Tue, 01 Jun 2021 22:36:18 +0000
with message-id <E1loCzi-0003gb-Sk@respighi.debian.org>
and subject line unblock lasso
has caused the Debian Bug report #989358,
regarding unblock: lasso/2.6.1-3
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
989358: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=989358
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please unblock package lasso

Fix for CVE-2021-28091 that is being released today; the library would fail
to properly check signature on responses with multiple assertions. The fix is
a single commit from upstream repository, that applied cleanly to the currently
packaged version.
  https://git.entrouvert.org/lasso.git/commit/?id=ea7e5efe9741e1b1787a58af16cb15b40c23be5a
  https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-28091 (not available yet)
  https://security-tracker.debian.org/tracker/CVE-2021-28091


[ Reason ]
Fix for security issue (CVE-2021-28091).

[ Impact ]
Current lasso package (2.6.1-2) has a security issue.

[ Tests ]
I built and ran a package locally and deployed the patch on $dayjob test
infrastructure (with a package with the patch but built for buster); I then
ran basic single sign-on tests. The fix has also been tested by external
parties during the embargo period.

[ Risks ]
The patch modifies a single function and its flow is easy enough to follow;
also for most uses SAML messages only contains a single assertion and the code
doesn't change behaviour in this case.

[ Checklist ]
  [x] all changes are documented in the d/changelog
  [×] I reviewed all changes and I approve them
  [×] attach debdiff against the package in testing

[ Other info ]
Thank you,

unblock lasso/2.6.1-3
diff -Nru lasso-2.6.1/debian/changelog lasso-2.6.1/debian/changelog
--- lasso-2.6.1/debian/changelog	2020-12-31 15:41:40.000000000 +0100
+++ lasso-2.6.1/debian/changelog	2021-06-01 14:43:55.000000000 +0200
@@ -1,3 +1,12 @@
+lasso (2.6.1-3) unstable; urgency=high
+
+  * debian/patches/0001-Fix-signature-checking-on-unsigned-response-with-mul.patch,
+    taken from upstream repository,
+    * CVE-2021-28091: Signature checking on unsigned response with multiple
+      assertions
+
+ -- Frederic Peters <fpeters@debian.org>  Tue, 01 Jun 2021 14:43:55 +0200
+
 lasso (2.6.1-2) unstable; urgency=medium
 
   * debian/control: add gtk-doc-tools to build-depends, required when running
diff -Nru lasso-2.6.1/debian/patches/0001-Fix-signature-checking-on-unsigned-response-with-mul.patch lasso-2.6.1/debian/patches/0001-Fix-signature-checking-on-unsigned-response-with-mul.patch
--- lasso-2.6.1/debian/patches/0001-Fix-signature-checking-on-unsigned-response-with-mul.patch	1970-01-01 01:00:00.000000000 +0100
+++ lasso-2.6.1/debian/patches/0001-Fix-signature-checking-on-unsigned-response-with-mul.patch	2021-06-01 14:41:51.000000000 +0200
@@ -0,0 +1,182 @@
+From ea7e5efe9741e1b1787a58af16cb15b40c23be5a Mon Sep 17 00:00:00 2001
+From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
+Date: Mon, 8 Mar 2021 11:33:26 +0100
+Subject: Fix signature checking on unsigned response with multiple assertions
+
+CVE-2021-28091 : when AuthnResponse messages are not signed (which is
+permitted by the specifiation), all assertion's signatures should be
+checked, but currently after the first signed assertion is checked all
+following assertions are accepted without checking their signature, and
+the last one is considered the main assertion.
+
+This patch :
+* check signatures from all assertions if the message is not signed,
+* refuse messages with assertion from different issuers than the one on
+  the message, to prevent assertion bundling event if they are signed.
+---
+ lasso/saml-2.0/login.c | 102 +++++++++++++++++++++++++++++------------
+ 1 file changed, 73 insertions(+), 29 deletions(-)
+
+diff --git a/lasso/saml-2.0/login.c b/lasso/saml-2.0/login.c
+index 0d4bb1da..cf62c1cc 100644
+--- a/lasso/saml-2.0/login.c
++++ b/lasso/saml-2.0/login.c
+@@ -1257,7 +1257,11 @@ lasso_saml20_login_check_assertion_signature(LassoLogin *login,
+ 	original_node = lasso_node_get_original_xmlnode(LASSO_NODE(assertion));
+ 	goto_cleanup_if_fail_with_rc(original_node, LASSO_PROFILE_ERROR_CANNOT_VERIFY_SIGNATURE);
+ 
+-	rc = profile->signature_status = lasso_provider_verify_saml_signature(remote_provider, original_node, NULL);
++	/* Shouldn't set the profile->signature_status here as we're only
++	 * checking the assertion signature.
++	 * Instead, we'll set the status after all the assertions are iterated.
++	 */
++	rc = lasso_provider_verify_saml_signature(remote_provider, original_node, NULL);
+ 
+ #define log_verify_assertion_signature_error(msg) \
+ 			message(G_LOG_LEVEL_WARNING, "Could not verify signature of assertion" \
+@@ -1282,18 +1286,6 @@ cleanup:
+ 	return rc;
+ }
+ 
+-static gboolean
+-_lasso_check_assertion_issuer(LassoSaml2Assertion *assertion, const gchar *provider_id)
+-{
+-	if (! LASSO_SAML2_ASSERTION(assertion) || ! provider_id)
+-		return FALSE;
+-
+-	if (! assertion->Issuer || ! assertion->Issuer->content)
+-		return FALSE;
+-
+-	return lasso_strisequal(assertion->Issuer->content,provider_id);
+-}
+-
+ static gint
+ _lasso_saml20_login_decrypt_assertion(LassoLogin *login, LassoSamlp2Response *samlp2_response)
+ {
+@@ -1358,11 +1350,23 @@ _lasso_saml20_login_decrypt_assertion(LassoLogin *login, LassoSamlp2Response *sa
+ 	return 0;
+ }
+ 
++/* Verify that an assertion comes from a designated Issuer */
++static gboolean
++_lasso_check_assertion_issuer(LassoSaml2Assertion *assertion, const gchar *provider_id)
++{
++	if (! LASSO_SAML2_ASSERTION(assertion) || ! provider_id)
++		return FALSE;
++	if (! assertion->Issuer || ! assertion->Issuer->content)
++		return FALSE;
++	return lasso_strisequal(assertion->Issuer->content,provider_id);
++}
++
+ static gint
+ lasso_saml20_login_process_response_status_and_assertion(LassoLogin *login)
+ {
+ 	LassoSamlp2StatusResponse *response;
+ 	LassoSamlp2Response *samlp2_response = NULL;
++	LassoSaml2Assertion *last_assertion = NULL;
+ 	LassoProfile *profile;
+ 	char *status_value;
+ 	lasso_error_t rc = 0;
+@@ -1404,34 +1408,62 @@ lasso_saml20_login_process_response_status_and_assertion(LassoLogin *login)
+ 
+ 	/* Decrypt all EncryptedAssertions */
+ 	_lasso_saml20_login_decrypt_assertion(login, samlp2_response);
+-	/* traverse all assertions */
+-	goto_cleanup_if_fail_with_rc (samlp2_response->Assertion != NULL,
+-			LASSO_PROFILE_ERROR_MISSING_ASSERTION);
+ 
++	/* Check there is at least one assertion */
++	goto_cleanup_if_fail_with_rc (samlp2_response->Assertion != NULL, LASSO_PROFILE_ERROR_MISSING_ASSERTION);
++
++	/* In case of verify_hint as 'FORCE', if there's no response signature,
++	 * we reject.
++	 * In case of 'MAYBE', if response signature is present and valid, or
++	 * not present, then we proceed with checking assertion signature(s).
++	 * In any case, if there's a response signature and it's not valid,
++	 * we reject.
++	*/
+ 	verify_hint = lasso_profile_get_signature_verify_hint(profile);
++	if (profile->signature_status == LASSO_DS_ERROR_SIGNATURE_NOT_FOUND) {
++		if (verify_hint == LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE) {
++			goto_cleanup_with_rc(profile->signature_status);
++		}
++	} else if (profile->signature_status != 0) {
++		goto_cleanup_with_rc(profile->signature_status);
++	}
+ 
+ 	lasso_foreach_full_begin(LassoSaml2Assertion*, assertion, it, samlp2_response->Assertion);
+ 		LassoSaml2Subject *subject = NULL;
+ 
+-		lasso_assign_gobject (login->private_data->saml2_assertion, assertion);
++		/* All Assertions MUST come from the same issuer as the Response. */
++		if (! _lasso_check_assertion_issuer(assertion, profile->remote_providerID)) {
++			goto_cleanup_with_rc(LASSO_PROFILE_ERROR_INVALID_ISSUER);
++		}
+ 
+-		/* If signature has already been verified on the message, and assertion has the same
+-		 * issuer as the message, the assertion is covered. So no need to verify a second
+-		 * time */
+-		if (profile->signature_status != 0 
+-			|| ! _lasso_check_assertion_issuer(assertion,
+-				profile->remote_providerID)
+-			|| verify_hint == LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE) {
++		if (profile->signature_status != 0) {
++			/* When response signature is not present */
++			if (verify_hint == LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE) {
++				assertion_signature_status =
++					lasso_saml20_login_check_assertion_signature(login, assertion);
++				if (assertion_signature_status) {
++					goto_cleanup_with_rc(assertion_signature_status);
++				}
++			}
++		} else {
++			/* response signature is present and valid */
+ 			assertion_signature_status = lasso_saml20_login_check_assertion_signature(login,
+-					assertion);
+-			/* If signature validation fails, it is the return code for this function */
++							assertion);
+ 			if (assertion_signature_status) {
+-				rc = LASSO_PROFILE_ERROR_CANNOT_VERIFY_SIGNATURE;
++				/* assertion signature is not valid or not present */
++				if (verify_hint == LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE) {
++					/* In case of FORCE, we reject right away */
++					goto_cleanup_with_rc(assertion_signature_status);
++				} else if (verify_hint == LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE) {
++					/* In case of MAYBE, if assertion signature is present and invalid, then we reject */
++					if (assertion_signature_status != LASSO_DS_ERROR_SIGNATURE_NOT_FOUND) {
++						goto_cleanup_with_rc(assertion_signature_status);
++					}
++				}
+ 			}
+ 		}
+-
+ 		lasso_extract_node_or_fail(subject, assertion->Subject, SAML2_SUBJECT,
+-				LASSO_PROFILE_ERROR_MISSING_SUBJECT);
++			LASSO_PROFILE_ERROR_MISSING_SUBJECT);
+ 
+ 		/* Verify Subject->SubjectConfirmationData->InResponseTo */
+ 		if (login->private_data->request_id) {
+@@ -1446,8 +1478,20 @@ lasso_saml20_login_process_response_status_and_assertion(LassoLogin *login)
+ 		/** Handle nameid */
+ 		lasso_check_good_rc(lasso_saml20_profile_process_name_identifier_decryption(profile,
+ 					&subject->NameID, &subject->EncryptedID));
++
++		last_assertion = assertion;
+ 	lasso_foreach_full_end();
+ 
++	/* set the profile signature status only after all the signatures are
++	 * verified.
++	 */
++	profile->signature_status = rc;
++
++	/* set the default assertion to the last one */
++	if (last_assertion) {
++		lasso_assign_gobject (login->private_data->saml2_assertion, last_assertion);
++	}
++
+ 	switch (verify_hint) {
+ 		case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_FORCE:
+ 		case LASSO_PROFILE_SIGNATURE_VERIFY_HINT_MAYBE:
+-- 
+2.32.0.rc2
+
diff -Nru lasso-2.6.1/debian/patches/series lasso-2.6.1/debian/patches/series
--- lasso-2.6.1/debian/patches/series	1970-01-01 01:00:00.000000000 +0100
+++ lasso-2.6.1/debian/patches/series	2021-06-01 14:41:59.000000000 +0200
@@ -0,0 +1 @@
+0001-Fix-signature-checking-on-unsigned-response-with-mul.patch

--- End Message ---
--- Begin Message ---
Unblocked.

--- End Message ---

Reply to: