From 19ffa92a8e132fa7c5713f1abe48513201cebe80 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Thu, 31 Aug 2023 20:43:46 +0200 Subject: [PATCH 1/3] [bugfix] The fn:parse-xml-fragment on the samlp:Response returns a document-node(element(samlp:Response)) and not an element(samlp:response), the code was previously look for an element and not a document-node. This fix now extracts the SAML Response element from the XML Document correctly --- content/exsaml.xqm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/content/exsaml.xqm b/content/exsaml.xqm index 160b7d5..90e780c 100644 --- a/content/exsaml.xqm +++ b/content/exsaml.xqm @@ -216,7 +216,7 @@ declare function exsaml:process-saml-response-post($cid as xs:string) { let $saml-resp := request:get-parameter("SAMLResponse", "error") let $resp := - if ($saml-resp = "error") + if ($saml-resp = "error" or fn:empty($resp/samlp:Response)) then $saml-resp else @@ -238,7 +238,7 @@ declare function exsaml:process-saml-response-post($cid as xs:string) { try { - let $res := exsaml:validate-saml-response($cid, $resp) + let $res := exsaml:validate-saml-response($cid, $resp/samlp:Response) return if (xs:integer($res/@res) lt 0) then @@ -270,8 +270,8 @@ declare function exsaml:process-saml-response-post($cid as xs:string) { If SAML success, this is basically username and group membership. IF SAML fail, pass enough info to allow meaningful error messages. :) let $auth := - - {exsaml:fetch-saml-attribute-values($cid, $exsaml:group-attr, $resp/saml:Assertion) ! {.}} + + {exsaml:fetch-saml-attribute-values($cid, $exsaml:group-attr, $resp/samlp:Response/saml:Assertion) ! {.}} (: create SAML user if not exists yet :) @@ -306,11 +306,11 @@ declare function exsaml:process-saml-response-post($cid as xs:string) { : Validate a SAML response message. : : @param $cid An id used for correlating log messages. - : @param $resp the XML document containing the SAML Response. + : @param $resp the XML element containing the SAML Response. : : @return an element indicating the result of the validation. :) -declare %private function exsaml:validate-saml-response($cid as xs:string, $resp as node()) as element(exsaml:funcret) { +declare %private function exsaml:validate-saml-response($cid as xs:string, $resp as element(samlp:Response)) as element(exsaml:funcret) { let $log := exsaml:log("info", $cid, "validate-saml-response") let $as := $resp/saml:Assertion From 18f466dacd6e25cc5077bf6194ac64513c046b3d Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Thu, 31 Aug 2023 20:44:15 +0200 Subject: [PATCH 2/3] [bugfix] Add the correct type signature for the saml:Assertion --- content/exsaml.xqm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/exsaml.xqm b/content/exsaml.xqm index 90e780c..5ddc751 100644 --- a/content/exsaml.xqm +++ b/content/exsaml.xqm @@ -356,7 +356,7 @@ declare %private function exsaml:validate-saml-response($cid as xs:string, $resp : : @return an element indicating the result of the validation. :) -declare %private function exsaml:validate-saml-assertion($cid as xs:string, $assertion as item()) as element(exsaml:funcret) { +declare %private function exsaml:validate-saml-assertion($cid as xs:string, $assertion as element(saml:Assertion)) as element(exsaml:funcret) { if (empty($assertion)) then let $log := exsaml:log("info", $cid, "Error: Empty Assertion") From 99dd9932d9099dcdfbd4d0e1b0397bdde842ed4f Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Thu, 31 Aug 2023 21:10:18 +0200 Subject: [PATCH 3/3] [bugfix] Values in the saml:Response are optional according to the OASIS SAML v2.0 Core specification, see: http://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf --- content/exsaml.xqm | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/content/exsaml.xqm b/content/exsaml.xqm index 5ddc751..14b3cbc 100644 --- a/content/exsaml.xqm +++ b/content/exsaml.xqm @@ -313,8 +313,9 @@ declare function exsaml:process-saml-response-post($cid as xs:string) { declare %private function exsaml:validate-saml-response($cid as xs:string, $resp as element(samlp:Response)) as element(exsaml:funcret) { let $log := exsaml:log("info", $cid, "validate-saml-response") - let $as := $resp/saml:Assertion - let $sig := $resp/ds:Signature + let $as as element(saml:Assertion)? := $resp/saml:Assertion + let $sig as element(ds:Signature)? := $resp/ds:Signature + let $reqid as xs:string? := $resp/@InResponseTo ! xs:string(.) return (: check SAML response status. there are ~20 failure codes, check @@ -338,6 +339,11 @@ declare %private function exsaml:validate-saml-response($cid as xs:string, $resp (: else if (boolean($sig) and not(exsaml:verify-response-signature($cid, $sig))) then :) (: :) + (: verify Response/@InResponseTo is present in the SAML response :) + else if (fn:exists($reqid) and not(exsaml:check-authnreqid($reqid))) + then + + (: must contain at least one assertion :) else if (empty($as)) then @@ -365,10 +371,10 @@ declare %private function exsaml:validate-saml-assertion($cid as xs:string, $ass else let $log := exsaml:log("info", $cid, "validate-saml-assertion: " || fn:serialize($assertion)) - let $sig := $assertion/ds:Signature - let $subj-confirm-data := $assertion/saml:Subject/saml:SubjectConfirmation/saml:SubjectConfirmationData - let $conds := $assertion/saml:Conditions - let $reqid := $subj-confirm-data/@InResponseTo + let $sig as element(ds:Signature)? := $assertion/ds:Signature + let $subj-confirm-data as element(saml:SubjectConfirmationData)? := $assertion/saml:Subject/saml:SubjectConfirmation/saml:SubjectConfirmationData + let $conds as element(saml:Conditions)? := $assertion/saml:Conditions + let $reqid as xs:string? := $subj-confirm-data/@InResponseTo ! xs:string(.) return (: check that "Issuer" is the expected IDP. Not stricty required by @@ -388,12 +394,12 @@ declare %private function exsaml:validate-saml-assertion($cid as xs:string, $ass (: maybe verify SubjectConfirmation/@Method :) (: verify SubjectConfirmationData/@Recipient is SP URL ($sp-uri) :) - else if (not($subj-confirm-data/@Recipient = $exsaml:sp-uri)) + else if (fn:exists($subj-confirm-data/@Recipient) and not($subj-confirm-data/@Recipient = $exsaml:sp-uri)) then (: verify SubjectConfirmationData/@NotOnOrAfter is not later than now :) - else if (xs:dateTime(fn:current-dateTime()) ge xs:dateTime($subj-confirm-data/@NotOnOrAfter)) + else if (fn:exists($subj-confirm-data/@NotOnOrAfter) and xs:dateTime(fn:current-dateTime()) ge xs:dateTime($subj-confirm-data/@NotOnOrAfter)) then