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