From c3af161be0cc94cd794c4c4d33de14b7785fd5c9 Mon Sep 17 00:00:00 2001 From: Fumiaki Kinoshita Date: Mon, 5 Aug 2024 20:34:29 +0900 Subject: [PATCH 1/2] Parse assertion signatures --- CHANGELOG.md | 1 + src/Network/Wai/SAML2/Assertion.hs | 9 ++++++-- src/Network/Wai/SAML2/Response.hs | 8 +++---- src/Network/Wai/SAML2/Validation.hs | 12 +++++----- tests/data/google.xml.expected | 34 +++++++++++++++-------------- tests/data/keycloak.xml.expected | 33 ++++++++++++++-------------- tests/data/okta.xml.expected | 33 ++++++++++++++-------------- 7 files changed, 70 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79b34ff..1f43da5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Replaced `x509Certificate` with `x509Certificates` in `IDPSSODescriptor` so that it may have more than one certificate ([#65](https://github.com/mbg/wai-saml2/pull/65) by [@fumieval](https://github.com/fumieval)) - Added `attributeValues` to `AssertionAttribute` in order to handle multiple attribute values with the same name ([#67](https://github.com/mbg/wai-saml2/pull/67) by [@fumieval](https://github.com/fumieval)) +- Support signed assertions, not just signed responses by ([#45](https://github.com/mbg/wai-saml2/pull/45) by [@fumieval](https://github.com/mbg/wai-saml2)) ## 0.6 diff --git a/src/Network/Wai/SAML2/Assertion.hs b/src/Network/Wai/SAML2/Assertion.hs index f4b7f87..8ae9f47 100644 --- a/src/Network/Wai/SAML2/Assertion.hs +++ b/src/Network/Wai/SAML2/Assertion.hs @@ -31,6 +31,7 @@ import Data.Time import Text.XML.Cursor import Network.Wai.SAML2.NameIDFormat +import Network.Wai.SAML2.Signature import Network.Wai.SAML2.XML -------------------------------------------------------------------------------- @@ -278,7 +279,9 @@ data Assertion = Assertion { -- | The authentication statement included in the assertion. assertionAuthnStatement :: !AuthnStatement, -- | The assertion's attribute statement. - assertionAttributeStatement :: !AttributeStatement + assertionAttributeStatement :: !AttributeStatement, + -- | The assertion's signature. + assertionSignature :: !(Maybe Signature) } deriving (Eq, Show) -- Reference [Assertion] @@ -306,7 +309,9 @@ instance FromXML Assertion where assertionAuthnStatement = authnStatement, assertionAttributeStatement = cursor $/ element (saml2Name "AttributeStatement") - >=> parseAttributeStatement + >=> parseAttributeStatement, + assertionSignature = listToMaybe $ + cursor $/ element (dsName "Signature") >=> parseXML } -------------------------------------------------------------------------------- diff --git a/src/Network/Wai/SAML2/Response.hs b/src/Network/Wai/SAML2/Response.hs index 68acca2..ff61fac 100644 --- a/src/Network/Wai/SAML2/Response.hs +++ b/src/Network/Wai/SAML2/Response.hs @@ -56,7 +56,7 @@ data Response = Response { -- | The status of the response. responseStatusCode :: !StatusCode, -- | The response signature. - responseSignature :: !Signature, + responseSignature :: !(Maybe Signature), -- | The unencrypted assertion. -- -- @since 0.4 @@ -88,9 +88,6 @@ instance FromXML Response where $/ element (saml2Name "EncryptedAssertion") ) >>= parseXML - signature <- oneOrFail "Signature is required" ( - cursor $/ element (dsName "Signature") ) >>= parseXML - pure Response{ responseDestination = T.concat $ attribute "Destination" cursor, responseId = T.concat $ attribute "ID" cursor, @@ -100,7 +97,8 @@ instance FromXML Response where responseIssuer = T.concat $ cursor $/ element (saml2Name "Issuer") &/ content, responseStatusCode = statusCode, - responseSignature = signature, + responseSignature = listToMaybe $ + (cursor $/ element (dsName "Signature")) >>= parseXML, responseAssertion = assertion, responseEncryptedAssertion = encAssertion } diff --git a/src/Network/Wai/SAML2/Validation.hs b/src/Network/Wai/SAML2/Validation.hs index 6fd4413..373c8c8 100644 --- a/src/Network/Wai/SAML2/Validation.hs +++ b/src/Network/Wai/SAML2/Validation.hs @@ -140,6 +140,10 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do Left err -> throwError $ CanonicalisationFailure err Right result -> pure result + signature <- case responseSignature samlResponse of + Just sig -> pure sig + Nothing -> throwError $ InvalidResponse $ userError "Response Signature is required" + -- 2. At this point we should dereference all elements identified by -- Reference elements inside the SignedInfo element. However, we do -- not currently do that and instead just assume that there is only @@ -149,8 +153,7 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do let documentId = responseId samlResponse let referenceId = referenceURI $ signedInfoReference - $ signatureInfo - $ responseSignature samlResponse + $ signatureInfo signature if documentId /= referenceId then throwError $ UnexpectedReference referenceId @@ -180,8 +183,7 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do $ BS.decodeLenient $ referenceDigestValue $ signedInfoReference - $ signatureInfo - $ responseSignature samlResponse + $ signatureInfo signature if Just documentHash /= referenceHash then throwError InvalidDigest @@ -191,7 +193,7 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do -- We need to check that the SignedInfo element has not been tampered -- with, which we do by checking the signature contained in the response; -- first: extract the signature data from the response - let sig = BS.decodeLenient $ signatureValue $ responseSignature samlResponse + let sig = BS.decodeLenient $ signatureValue signature -- using the IdP's public key and the canonicalised SignedInfo element, -- check that the signature is correct diff --git a/tests/data/google.xml.expected b/tests/data/google.xml.expected index c3dfbd1..fc5e628 100644 --- a/tests/data/google.xml.expected +++ b/tests/data/google.xml.expected @@ -13,22 +13,23 @@ Response MkStatusCode { statusCodeValue = Success , statusCodeSubordinate = Nothing } , responseSignature = - Signature - { signatureInfo = - SignedInfo - { signedInfoCanonicalisationMethod = C14N_EXC_1_0 - , signedInfoSignatureMethod = RSA_SHA256 - , signedInfoReference = - Reference - { referenceURI = "_b9917f3478ff3f776cb351547379d0bc" - , referenceDigestMethod = DigestSHA256 - , referenceDigestValue = - "z3YXe+aIiyAYbSGqxRiYPwTO38JNZad2WBgqinjiX8g=" - } - } - , signatureValue = - "S6IAb+4yYyNVIfnqM7ZIrFSKbzi0RgVqk6qNL0ehzFZhcaArqN/8S0oCDrrc+0B2ygCkpbDKt03/\nvNIxq862VYa1Y39n8w0/eIKc+m3beNS6e+n316o3LCtSTPvgC2kE96Jh44MOUX9z/KIYxXiPGgdi\nnnlxK6L1dZGrMKKBxEFiBJMhFI42gWADyrUz311PDImM+rVhINJT9NyD1i1G++PILLer/BlGwg5n\nwNQHmjt4pV3m+8zMMIrfLjZT/YX8s7+1Xn1Y3bMzo6PnsbvS7OX1YGuN4cPn9f4MLu5Vcn9lWo/r\n+zuZay/pS0mEqTmeOOMGk4BOseiU/hAra5NDgQ==" - } + Just + Signature + { signatureInfo = + SignedInfo + { signedInfoCanonicalisationMethod = C14N_EXC_1_0 + , signedInfoSignatureMethod = RSA_SHA256 + , signedInfoReference = + Reference + { referenceURI = "_b9917f3478ff3f776cb351547379d0bc" + , referenceDigestMethod = DigestSHA256 + , referenceDigestValue = + "z3YXe+aIiyAYbSGqxRiYPwTO38JNZad2WBgqinjiX8g=" + } + } + , signatureValue = + "S6IAb+4yYyNVIfnqM7ZIrFSKbzi0RgVqk6qNL0ehzFZhcaArqN/8S0oCDrrc+0B2ygCkpbDKt03/\nvNIxq862VYa1Y39n8w0/eIKc+m3beNS6e+n316o3LCtSTPvgC2kE96Jh44MOUX9z/KIYxXiPGgdi\nnnlxK6L1dZGrMKKBxEFiBJMhFI42gWADyrUz311PDImM+rVhINJT9NyD1i1G++PILLer/BlGwg5n\nwNQHmjt4pV3m+8zMMIrfLjZT/YX8s7+1Xn1Y3bMzo6PnsbvS7OX1YGuN4cPn9f4MLu5Vcn9lWo/r\n+zuZay/pS0mEqTmeOOMGk4BOseiU/hAra5NDgQ==" + } , responseAssertion = Just Assertion @@ -72,6 +73,7 @@ Response , authnStatementLocality = "" } , assertionAttributeStatement = [] + , assertionSignature = Nothing } , responseEncryptedAssertion = Nothing } \ No newline at end of file diff --git a/tests/data/keycloak.xml.expected b/tests/data/keycloak.xml.expected index 558125a..0e1add3 100644 --- a/tests/data/keycloak.xml.expected +++ b/tests/data/keycloak.xml.expected @@ -10,22 +10,23 @@ Response MkStatusCode { statusCodeValue = Success , statusCodeSubordinate = Nothing } , responseSignature = - Signature - { signatureInfo = - SignedInfo - { signedInfoCanonicalisationMethod = C14N_EXC_1_0 - , signedInfoSignatureMethod = RSA_SHA256 - , signedInfoReference = - Reference - { referenceURI = "ID_5b1d000b-3a5e-4dfe-aa4e-b7bf1e3efbfd" - , referenceDigestMethod = DigestSHA256 - , referenceDigestValue = - "/U47P3hsUf+tUyyglYF8M1u6lbVnHimCthtxusju4mo=" - } - } - , signatureValue = - "b9vgIBQ1yNvUYgNmfAyuQJXOJ68PMfRvNAZEa93tnzZXHPEsf7/F49xI6/mlYI/T9pDxYnFcfl7kPMxgz4ssvMjwUEgAR3G3ZrNv4gPMUPmbZnXe0KG8yU9AskK90ya/T11kQfI21cSlA8FrLPTGP2X97yErR10mIDvEJ/m5dWra95cGLx/ntjaSIqNJpVgpHhRxieS4Lw+zeWe/nVuznXQnb8VRhCq18ikL/u23+YhYT3ws3iXQssJ2BosX9JJt0O+X31sIHJIWHsxbI69NLJ782bVDDkI1PNF8MKoa8gSEiLsNSmp3SyXtMPzaRIBguksl9xbnmYmsJDQg6kFVlQ==" - } + Just + Signature + { signatureInfo = + SignedInfo + { signedInfoCanonicalisationMethod = C14N_EXC_1_0 + , signedInfoSignatureMethod = RSA_SHA256 + , signedInfoReference = + Reference + { referenceURI = "ID_5b1d000b-3a5e-4dfe-aa4e-b7bf1e3efbfd" + , referenceDigestMethod = DigestSHA256 + , referenceDigestValue = + "/U47P3hsUf+tUyyglYF8M1u6lbVnHimCthtxusju4mo=" + } + } + , signatureValue = + "b9vgIBQ1yNvUYgNmfAyuQJXOJ68PMfRvNAZEa93tnzZXHPEsf7/F49xI6/mlYI/T9pDxYnFcfl7kPMxgz4ssvMjwUEgAR3G3ZrNv4gPMUPmbZnXe0KG8yU9AskK90ya/T11kQfI21cSlA8FrLPTGP2X97yErR10mIDvEJ/m5dWra95cGLx/ntjaSIqNJpVgpHhRxieS4Lw+zeWe/nVuznXQnb8VRhCq18ikL/u23+YhYT3ws3iXQssJ2BosX9JJt0O+X31sIHJIWHsxbI69NLJ782bVDDkI1PNF8MKoa8gSEiLsNSmp3SyXtMPzaRIBguksl9xbnmYmsJDQg6kFVlQ==" + } , responseAssertion = Nothing , responseEncryptedAssertion = Just diff --git a/tests/data/okta.xml.expected b/tests/data/okta.xml.expected index aaa764e..ea4797e 100644 --- a/tests/data/okta.xml.expected +++ b/tests/data/okta.xml.expected @@ -10,22 +10,23 @@ Response MkStatusCode { statusCodeValue = Success , statusCodeSubordinate = Nothing } , responseSignature = - Signature - { signatureInfo = - SignedInfo - { signedInfoCanonicalisationMethod = C14N_EXC_1_0 - , signedInfoSignatureMethod = RSA_SHA256 - , signedInfoReference = - Reference - { referenceURI = "id36905492230634463108368061" - , referenceDigestMethod = DigestSHA256 - , referenceDigestValue = - "i300U58mYoywmpgMkq7AHOplar4B6ZwW++Ri3PUvGlc=" - } - } - , signatureValue = - "eKymDiZIxFd+oJCIhdHQpI+s2nQYFBVH7TBoZaqXizJdNnNPpNlKM5wds33zP6a72GWOL/2n8WRtW+xSnPTmw5PumEuUhWbO2EizfU/SdBL4HKxxJt0nntkDRhdIoHBi+9Gy80So4NGr82B2clnhpj74GW0Ko8A0bt2sHzdQ9NaJ5ru4Qvx0Fk/UCBAGAYobryjAc9Tb5qxxgHGMmPUHG5CprRYTINKZlGF1Jl88VSdTbOYmGjJ4b6GY8pLR2qLt4LErVPUSI4vEUuPFU2f/9/i8D39hzWtJOCzuaUZ5tdKU3Fyjsgoa7ooDfZVzb4howWhQ9zPUgwjZEd9tbW2EwA==" - } + Just + Signature + { signatureInfo = + SignedInfo + { signedInfoCanonicalisationMethod = C14N_EXC_1_0 + , signedInfoSignatureMethod = RSA_SHA256 + , signedInfoReference = + Reference + { referenceURI = "id36905492230634463108368061" + , referenceDigestMethod = DigestSHA256 + , referenceDigestValue = + "i300U58mYoywmpgMkq7AHOplar4B6ZwW++Ri3PUvGlc=" + } + } + , signatureValue = + "eKymDiZIxFd+oJCIhdHQpI+s2nQYFBVH7TBoZaqXizJdNnNPpNlKM5wds33zP6a72GWOL/2n8WRtW+xSnPTmw5PumEuUhWbO2EizfU/SdBL4HKxxJt0nntkDRhdIoHBi+9Gy80So4NGr82B2clnhpj74GW0Ko8A0bt2sHzdQ9NaJ5ru4Qvx0Fk/UCBAGAYobryjAc9Tb5qxxgHGMmPUHG5CprRYTINKZlGF1Jl88VSdTbOYmGjJ4b6GY8pLR2qLt4LErVPUSI4vEUuPFU2f/9/i8D39hzWtJOCzuaUZ5tdKU3Fyjsgoa7ooDfZVzb4howWhQ9zPUgwjZEd9tbW2EwA==" + } , responseAssertion = Nothing , responseEncryptedAssertion = Just From 39b31bcb5d2cac3232caa12ec6b30a879d1188e9 Mon Sep 17 00:00:00 2001 From: Fumiaki Kinoshita Date: Fri, 2 Aug 2024 13:21:45 +0900 Subject: [PATCH 2/2] Validate signed assertions --- src/Network/Wai/SAML2/Validation.hs | 103 +++++++++++++++++++++------- tests/Validation.hs | 3 +- 2 files changed, 81 insertions(+), 25 deletions(-) diff --git a/src/Network/Wai/SAML2/Validation.hs b/src/Network/Wai/SAML2/Validation.hs index 373c8c8..264c452 100644 --- a/src/Network/Wai/SAML2/Validation.hs +++ b/src/Network/Wai/SAML2/Validation.hs @@ -83,16 +83,10 @@ decodeResponse responseData = do Left err -> throwError $ InvalidResponse err Right samlResponse -> pure (responseXmlDoc, samlResponse) --- | 'validateSAMLResponse' @cfg doc response timestamp@ validates a decoded SAML2 --- response using the given @timestamp@. +-- | 'validateSAMLPreliminary' @cfg samlResponse@ validates the status code, destination, and issuer of a SAML2 response. -- --- @since 0.4 -validateSAMLResponse :: SAML2Config - -> XML.Document - -> Response - -> UTCTime - -> ExceptT SAML2Error IO Assertion -validateSAMLResponse cfg responseXmlDoc samlResponse now = do +validateSAMLPreliminary :: SAML2Config -> Response -> ExceptT SAML2Error IO () +validateSAMLPreliminary cfg samlResponse = do -- check that the response indicates success case statusCodeValue $ responseStatusCode samlResponse of @@ -118,6 +112,29 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do | issuer /= expectedIssuer -> throwError $ InvalidIssuer issuer _ -> pure () +-- | 'validateSAMLResponse' @cfg doc response timestamp@ validates a decoded SAML2 +-- response using the given @timestamp@. +-- +-- @since 0.4 +validateSAMLResponse :: SAML2Config + -> XML.Document + -> Response + -> UTCTime + -> ExceptT SAML2Error IO Assertion +validateSAMLResponse cfg responseXmlDoc samlResponse now = do + + validateSAMLPreliminary cfg samlResponse + + case responseSignature samlResponse of + Just _ -> validateSAMLResponseSignature cfg responseXmlDoc samlResponse now + Nothing -> validateSAMLAssertionSignature cfg responseXmlDoc samlResponse now + +validateSAMLResponseSignature :: SAML2Config + -> XML.Document + -> Response + -> UTCTime + -> ExceptT SAML2Error IO Assertion +validateSAMLResponseSignature cfg responseXmlDoc samlResponse now = do -- ***CORE VALIDATION*** -- See https://www.w3.org/TR/xmldsig-core1/#sec-CoreValidation -- @@ -126,20 +143,6 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do -- Signature element. This element contains signedInfo <- extractSignedInfo (XML.fromDocument responseXmlDoc) - -- construct a new XML document from the SignedInfo element and render - -- it into a textual representation - let doc = XML.Document (XML.Prologue [] Nothing []) signedInfo [] - let signedInfoXml = XML.renderLBS def doc - - -- canonicalise the textual representation of the SignedInfo element - let prefixList = extractPrefixList (XML.fromDocument doc) - signedInfoCanonResult <- liftIO $ try $ - canonicalise prefixList (LBS.toStrict signedInfoXml) - - normalisedSignedInfo <- case signedInfoCanonResult of - Left err -> throwError $ CanonicalisationFailure err - Right result -> pure result - signature <- case responseSignature samlResponse of Just sig -> pure sig Nothing -> throwError $ InvalidResponse $ userError "Response Signature is required" @@ -165,6 +168,34 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do -- the Signature element present). First remove the Signature element: let docMinusSignature = removeSignature responseXmlDoc + validateSAMLSignature ValidationContext{..} + +data ValidationContext = ValidationContext + { cfg :: !SAML2Config + , docMinusSignature :: !XML.Document + , now :: !UTCTime + , responseXmlDoc :: !XML.Document + , samlResponse :: !Response + , signature :: !Signature + , signedInfo :: !XML.Element + } + +validateSAMLSignature :: ValidationContext -> ExceptT SAML2Error IO Assertion +validateSAMLSignature ValidationContext{..} = do + -- construct a new XML document from the SignedInfo element and render + -- it into a textual representation + let doc = XML.Document (XML.Prologue [] Nothing []) signedInfo [] + let signedInfoXml = XML.renderLBS def doc + + -- canonicalise the textual representation of the SignedInfo element + let prefixList = extractPrefixList (XML.fromDocument doc) + signedInfoCanonResult <- liftIO $ try $ + canonicalise prefixList (LBS.toStrict signedInfoXml) + + normalisedSignedInfo <- case signedInfoCanonResult of + Left err -> throwError $ CanonicalisationFailure err + Right result -> pure result + -- then render the resulting document and canonicalise it let renderedXml = XML.renderLBS def docMinusSignature refCanonResult <- liftIO $ try $ canonicalise prefixList (LBS.toStrict renderedXml) @@ -235,6 +266,32 @@ validateSAMLResponse cfg responseXmlDoc samlResponse now = do -- all checks out, return the assertion pure assertion +validateSAMLAssertionSignature :: SAML2Config -> XML.Document -> Response -> UTCTime -> ExceptT SAML2Error IO Assertion +validateSAMLAssertionSignature cfg responseXmlDoc samlResponse now = do + assertion <- case responseAssertion samlResponse of + Just a -> pure a + _ -> throwError $ InvalidResponse $ userError "Assertion is required" + + signature <- case assertionSignature assertion of + Just a -> pure a + _ -> throwError $ InvalidResponse $ userError "Assertion signature is required" + + -- Obtain the XML node of the assertion for validation + assertionXml <- oneOrFail "Assertion is required" $ + XML.fromDocument responseXmlDoc XML.$/ XML.element (saml2Name "Assertion") + + signedInfo <- extractSignedInfo assertionXml + + docMinusSignature <- removeSignature <$> case XML.node assertionXml of + XML.NodeElement node -> pure XML.Document + { documentPrologue = XML.Prologue [] Nothing [] + , documentRoot = node + , documentEpilogue = [] + } + _ -> throwError $ InvalidResponse $ userError "Assertion is required" + + validateSAMLSignature ValidationContext{..} + -- | `decryptAssertion` @key encryptedAssertion@ decrypts the AES key in -- @encryptedAssertion@ using `key`, then decrypts the contents using -- the AES key. diff --git a/tests/Validation.hs b/tests/Validation.hs index 9a3dc7e..abf1713 100644 --- a/tests/Validation.hs +++ b/tests/Validation.hs @@ -11,7 +11,6 @@ import Network.Wai.SAML2 import Network.Wai.SAML2.Validation import System.FilePath import Test.Tasty -import Test.Tasty.ExpectedFailure import Test.Tasty.HUnit -- | Get a public key from a X.509 certificate @@ -46,7 +45,7 @@ tests :: TestTree tests = testGroup "Validate SAML2 Response" [ testCase "AzureAD signed response" $ run "azuread.crt" "2023-05-10T01:20:00Z" "azuread-signed-response.xml" - , expectFail $ testCase "AzureAD signed assertion" + , testCase "AzureAD signed assertion" $ run "azuread.crt" "2023-05-09T16:00:00Z" "azuread-signed-assertion.xml" , testCase "Okta with AttributeStatement" $ run "okta.crt" "2023-06-16T06:43:00.000Z" "okta-attributes.xml"