Package: release.debian.org
Severity: normal
User: release.debian....@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 <fpet...@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 <bdauver...@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 via email to