From 57640cc7a9f69ae71ac446449ead377974f3c8b9 Mon Sep 17 00:00:00 2001 From: Stephen Augustus Date: Mon, 7 Dec 2020 22:01:28 -0500 Subject: [PATCH] connector/saml: Validate XML roundtrip data before processing request Signed-off-by: Stephen Augustus --- connector/saml/saml.go | 9 ++++++++- go.mod | 2 ++ go.sum | 4 ++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 29f5dd98..08ff6cbc 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -7,7 +7,6 @@ import ( "encoding/base64" "encoding/pem" "encoding/xml" - "errors" "fmt" "io/ioutil" "strings" @@ -15,6 +14,8 @@ import ( "time" "github.com/beevik/etree" + xrv "github.com/mattermost/xml-roundtrip-validator" + "github.com/pkg/errors" dsig "github.com/russellhaering/goxmldsig" "github.com/russellhaering/goxmldsig/etreeutils" @@ -287,6 +288,7 @@ func (p *provider) POSTData(s connector.Scopes, id string) (action, value string // // The steps taken are: // +// * Validate XML document does not contain malicious inputs. // * Verify signature on XML document (or verify sig on assertion elements). // * Verify various parts of the Assertion element. Conditions, audience, etc. // * Map the Assertion's attribute elements to user info. @@ -297,6 +299,11 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str return ident, fmt.Errorf("decode response: %v", err) } + byteReader := bytes.NewReader(rawResp) + if xrvErr := xrv.Validate(byteReader); xrvErr != nil { + return ident, errors.Wrap(xrvErr, "validating XML response") + } + // Root element is allowed to not be signed if the Assertion element is. rootElementSigned := true if p.validator != nil { diff --git a/go.mod b/go.mod index d92f8fbc..86a1e9c8 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,9 @@ require ( github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect github.com/kylelemons/godebug v1.1.0 github.com/lib/pq v1.3.0 + github.com/mattermost/xml-roundtrip-validator v0.0.0-20201204154048-1a8688af4cf1 github.com/mattn/go-sqlite3 v1.11.0 + github.com/pkg/errors v0.9.1 github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 // indirect github.com/prometheus/client_golang v1.4.0 github.com/russellhaering/goxmldsig v1.1.0 diff --git a/go.sum b/go.sum index 2ce312e9..ada7c048 100644 --- a/go.sum +++ b/go.sum @@ -186,6 +186,8 @@ github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+ github.com/lib/pq v1.3.0 h1:/qkRGz8zljWiDcFvgpwUpwIAPu3r07TDvs3Rws+o/pU= github.com/lib/pq v1.3.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= +github.com/mattermost/xml-roundtrip-validator v0.0.0-20201204154048-1a8688af4cf1 h1:D2uMrH5NnWgU7JdjiWjOg/n31gbPH6I0D3IbCrqVVYE= +github.com/mattermost/xml-roundtrip-validator v0.0.0-20201204154048-1a8688af4cf1/go.mod h1:qccnGMcpgwcNaBnxqpJpWWUiPNr5H3O8eDgGV9gT5To= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU= @@ -226,6 +228,8 @@ github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/9 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 h1:J9b7z+QKAmPf4YLrFg6oQUotqHQeUNWwkvo7jZp1GLU=