Bug#989358: unblock: lasso/2.6.1-3
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
Reply to: