Skip to content

Commit

Permalink
Fixed a bug in XML canonicalisation causing a digest mismatch on Okta…
Browse files Browse the repository at this point in the history
… when attributes are present

When parsing a SAML response, it has been inappropriately stripping `xmlns:xs="http://www.w3.org/2001/XMLSchema"` attribute in saml2:Assertion. This was causing a discrepancy between Okta's digest and our digest (but only when AttributeStatement is present).

This change fixes the problem by setting `psRetainNamespaces = True` and adding "xs" to the list of allowed prefixes for c14n.

Special thanks to @hiroqn for figuring this out
  • Loading branch information
fumieval committed Jun 22, 2023
1 parent 98461e4 commit 60c924f
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Changelog for `wai-saml2`

- Fixed a bug in XML canonicalisation causing a digest mismatch on Okta when assertion attributes are present (special thanks to @hiroqn) ([#51](https://github.com/mbg/wai-saml2/pull/51) by [@fumieval](https://github.com/fumieval))

## 0.4

- Split `validateResponse` into `decodeResponse` and `validateSAMLResponse` ([#31](https://github.com/mbg/wai-saml2/pull/31) by [@fumieval](https://github.com/fumieval))
Expand Down
2 changes: 1 addition & 1 deletion src/Network/Wai/SAML2/C14N.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import Text.XML.C14N

-- | 'canonicalise' @xml@ produces a canonical representation of @xml@.
canonicalise :: BS.ByteString -> IO BS.ByteString
canonicalise xml = c14n c14nOpts c14n_exclusive_1_0 [] False Nothing xml
canonicalise xml = c14n c14nOpts c14n_exclusive_1_0 ["xs"] False Nothing xml

-- | The options we want to use for canonicalisation of XML documents.
c14nOpts :: [CInt]
Expand Down
2 changes: 1 addition & 1 deletion src/Network/Wai/SAML2/Validation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ decodeResponse responseData = do

-- try to parse the XML document; throw an exception if it is not
-- a valid XML document
responseXmlDoc <- case XML.parseLBS def (LBS.fromStrict resXmlDocData) of
responseXmlDoc <- case XML.parseLBS parseSettings (LBS.fromStrict resXmlDocData) of
Left err -> throwError $ InvalidResponseXml err
Right responseXmlDoc -> pure responseXmlDoc

Expand Down
8 changes: 7 additions & 1 deletion src/Network/Wai/SAML2/XML.hs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ module Network.Wai.SAML2.XML (

-- * XML parsing
FromXML(..),
oneOrFail
oneOrFail,
parseSettings
) where

--------------------------------------------------------------------------------
Expand Down Expand Up @@ -100,3 +101,8 @@ oneOrFail err [] = fail err
oneOrFail _ (x:_) = pure x

--------------------------------------------------------------------------------

-- | It is important to retain namespaces in order to calculate the hash of the canonicalised XML correctly.
-- see: https://stackoverflow.com/questions/69252831/saml-2-0-digest-value-calculation-in-saml-assertion
parseSettings :: ParseSettings
parseSettings = def { psRetainNamespaces = True }

0 comments on commit 60c924f

Please sign in to comment.