CVE-2020-27846

@@ -10,6 +10,7 @@ require (
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/jonboulle/clockwork v0.2.1 // indirect
 	github.com/kr/pretty v0.2.1
+	github.com/mattermost/xml-roundtrip-validator v0.0.0-00010101000000-000000000000
 	github.com/pkg/errors v0.8.1 // indirect
 	github.com/russellhaering/goxmldsig v1.1.0
 	github.com/stretchr/testify v1.6.1

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/beevik/etree"
+	xrv "github.com/mattermost/xml-roundtrip-validator"
 	dsig "github.com/russellhaering/goxmldsig"
 
 	"github.com/crewjam/saml/logger"
@@ -359,13 +360,18 @@ func NewIdpAuthnRequest(idp *IdentityProvider, r *http.Request) (*IdpAuthnReques
 	default:
 		return nil, fmt.Errorf("method not allowed")
 	}
+
 	return req, nil
 }
 
 // Validate checks that the authentication request is valid and assigns
 // the AuthnRequest and Metadata properties. Returns a non-nil error if the
 // request is not valid.
 func (req *IdpAuthnRequest) Validate() error {
+	if err := xrv.Validate(bytes.NewReader(req.RequestBuffer)); err != nil {
+		return err
+	}
+
 	if err := xml.Unmarshal(req.RequestBuffer, &req.Request); err != nil {
 		return err
 	}

@@ -1,13 +1,16 @@
 package saml
 
 import (
+	"bytes"
+	"compress/flate"
 	"crypto"
 	"crypto/rsa"
 	"crypto/x509"
 	"encoding/base64"
 	"encoding/pem"
 	"encoding/xml"
 	"fmt"
+	"io/ioutil"
 	"math/rand"
 	"net/http"
 	"net/http/httptest"
@@ -232,15 +235,41 @@ func TestIDPHTTPCanHandleMetadataRequest(t *testing.T) {
 func TestIDPHTTPCanHandleSSORequest(t *testing.T) {
 	test := NewIdentifyProviderTest()
 	w := httptest.NewRecorder()
-	r, _ := http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&SAMLRequest=lJJBayoxFIX%2FypC9JhnU5wszAz7lgWCLaNtFd5fMbQ1MkmnunVb%2FfUfbUqEgdhs%2BTr5zkmLW8S5s8KVD4mzvm0Cl6FIwEciRCeCRDFuznd2sTD5Upk2Ro42NyGZEmNjFMI%2BBOo9pi%2BnVWbzfrEqxY27JSEntEPfg2waHNnpJ4JtcgiWRLfoLXYBjwDfu6p%2B8JIoiWy5K4eqBUipXIzVRUwXKKtRK53qkJ3qqQVuNPUjU4TIQQ%2BBS5EqPBzofKH2ntBn%2FMervo8jWnyX%2BuVC78FwKkT1gopNKX1JUxSklXTMIfM0gsv8xeeDL%2BPGk7%2FF0Qg0GdnwQ1cW5PDLUwFDID6uquO1Dlot1bJw9%2FPLRmia%2BzRMCYyk4dSiq6205QSDXOxfy3KAq5Pkvqt4DAAD%2F%2Fw%3D%3D", nil)
+
+	const validRequest = `lJJBayoxFIX%2FypC9JhnU5wszAz7lgWCLaNtFd5fMbQ1MkmnunVb%2FfUfbUqEgdhs%2BTr5zkmLW8S5s8KVD4mzvm0Cl6FIwEciRCeCRDFuznd2sTD5Upk2Ro42NyGZEmNjFMI%2BBOo9pi%2BnVWbzfrEqxY27JSEntEPfg2waHNnpJ4JtcgiWRLfoLXYBjwDfu6p%2B8JIoiWy5K4eqBUipXIzVRUwXKKtRK53qkJ3qqQVuNPUjU4TIQQ%2BBS5EqPBzofKH2ntBn%2FMervo8jWnyX%2BuVC78FwKkT1gopNKX1JUxSklXTMIfM0gsv8xeeDL%2BPGk7%2FF0Qg0GdnwQ1cW5PDLUwFDID6uquO1Dlot1bJw9%2FPLRmia%2BzRMCYyk4dSiq6205QSDXOxfy3KAq5Pkvqt4DAAD%2F%2Fw%3D%3D`
+
+	r, _ := http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&"+
+		"SAMLRequest="+validRequest, nil)
 	test.IDP.Handler().ServeHTTP(w, r)
 	assert.Equal(t, http.StatusOK, w.Code)
 
 	// rejects requests that are invalid
 	w = httptest.NewRecorder()
-	r, _ = http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&SAMLRequest=PEF1dGhuUmVxdWVzdA%3D%3D", nil)
+	r, _ = http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&"+
+		"SAMLRequest=PEF1dGhuUmVxdWVzdA%3D%3D", nil)
 	test.IDP.Handler().ServeHTTP(w, r)
 	assert.Equal(t, http.StatusBadRequest, w.Code)
+
+	// rejects requests that contain malformed XML
+	{
+		a, _ := url.QueryUnescape(validRequest)
+		b, _ := base64.StdEncoding.DecodeString(a)
+		c, _ := ioutil.ReadAll(flate.NewReader(bytes.NewReader(b)))
+		d := bytes.Replace(c, []byte("<AuthnRequest"), []byte("<AuthnRequest ::foo=\"bar\""), 1)
+		f := bytes.Buffer{}
+		e, _ := flate.NewWriter(&f, flate.DefaultCompression)
+		e.Write(d)
+		e.Close()
+		g := base64.StdEncoding.EncodeToString(f.Bytes())
+		invalidRequest := url.QueryEscape(g)
+
+		w = httptest.NewRecorder()
+		r, _ = http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&"+
+			"SAMLRequest="+invalidRequest, nil)
+		test.IDP.Handler().ServeHTTP(w, r)
+		assert.Equal(t, http.StatusBadRequest, w.Code)
+	}
+
 }
 
 func TestIDPCanHandleRequestWithNewSession(t *testing.T) {

@@ -1,12 +1,13 @@
 package samlidp
 
 import (
+	"bytes"
+	"encoding/xml"
 	"errors"
+	"io"
 	"io/ioutil"
 
-	"encoding/xml"
-
-	"io"
+	xrv "github.com/mattermost/xml-roundtrip-validator"
 
 	"github.com/crewjam/saml"
 )
@@ -20,19 +21,20 @@ func randomBytes(n int) []byte {
 }
 
 func getSPMetadata(r io.Reader) (spMetadata *saml.EntityDescriptor, err error) {
-	var bytes []byte
-
-	if bytes, err = ioutil.ReadAll(r); err != nil {
+	var data []byte
+	if data, err = ioutil.ReadAll(r); err != nil {
 		return nil, err
 	}
 
 	spMetadata = &saml.EntityDescriptor{}
+	if err := xrv.Validate(bytes.NewBuffer(data)); err != nil {
+		return nil, err
+	}
 
-	if err := xml.Unmarshal(bytes, &spMetadata); err != nil {
+	if err := xml.Unmarshal(data, &spMetadata); err != nil {
 		if err.Error() == "expected element type <EntityDescriptor> but have <EntitiesDescriptor>" {
 			entities := &saml.EntitiesDescriptor{}
-
-			if err := xml.Unmarshal(bytes, &entities); err != nil {
+			if err := xml.Unmarshal(data, &entities); err != nil {
 				return nil, err
 			}
 

@@ -0,0 +1,22 @@
+package samlidp
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetSPMetadata(t *testing.T) {
+	good := "" +
+		"<EntityDescriptor xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\" validUntil=\"2013-03-10T00:32:19.104Z\" cacheDuration=\"PT1H\" entityID=\"http://localhost:5000/e087a985171710fb9fb30f30f41384f9/saml2/metadata/\">\n" +
+		"</EntityDescriptor>"
+	_, err := getSPMetadata(strings.NewReader(good))
+	assert.NoError(t, err)
+
+	bad := "" +
+		"<EntityDescriptor xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\" ::attr=\"foo\" validUntil=\"2013-03-10T00:32:19.104Z\" cacheDuration=\"PT1H\" entityID=\"http://localhost:5000/e087a985171710fb9fb30f30f41384f9/saml2/metadata/\">\n" +
+		"</EntityDescriptor>"
+	_, err = getSPMetadata(strings.NewReader(bad))
+	assert.EqualError(t, err, "validator: in token starting at 1:1: roundtrip error: expected {{ EntityDescriptor} [{{ xmlns} urn:oasis:names:tc:SAML:2.0:metadata} {{ :attr} foo} {{ validUntil} 2013-03-10T00:32:19.104Z} {{ cacheDuration} PT1H} {{ entityID} http://localhost:5000/e087a985171710fb9fb30f30f41384f9/saml2/metadata/}]}, observed {{ EntityDescriptor} [{{ xmlns} urn:oasis:names:tc:SAML:2.0:metadata} {{ attr} foo} {{ validUntil} 2013-03-10T00:32:19.104Z} {{ cacheDuration} PT1H} {{ entityID} http://localhost:5000/e087a985171710fb9fb30f30f41384f9/saml2/metadata/}]}")
+}

@@ -1,6 +1,7 @@
 package samlsp
 
 import (
+	"bytes"
 	"context"
 	"encoding/xml"
 	"errors"
@@ -9,6 +10,7 @@ import (
 	"net/url"
 
 	"github.com/crewjam/httperr"
+	xrv "github.com/mattermost/xml-roundtrip-validator"
 
 	"github.com/crewjam/saml"
 )
@@ -20,6 +22,11 @@ import (
 // <EntityDescriptor>.
 func ParseMetadata(data []byte) (*saml.EntityDescriptor, error) {
 	entity := &saml.EntityDescriptor{}
+
+	if err := xrv.Validate(bytes.NewBuffer(data)); err != nil {
+		return nil, err
+	}
+
 	err := xml.Unmarshal(data, entity)
 
 	// this comparison is ugly, but it is how the error is generated in encoding/xml

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -25,3 +26,19 @@ func TestFetchMetadata(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "https://idp.testshib.org/idp/shibboleth", md.EntityID)
 }
+
+func TestFetchMetadataRejectsInvalid(t *testing.T) {
+	test := NewMiddlewareTest()
+	test.IDPMetadata = strings.Replace(test.IDPMetadata, "<EntityDescriptor ", "<EntityDescriptor ::foo=\"bar\"", -1)
+
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, "/metadata", r.URL.String())
+		fmt.Fprint(w, test.IDPMetadata)
+	}))
+
+	fmt.Println(testServer.URL + "/metadata")
+	u, _ := url.Parse(testServer.URL + "/metadata")
+	md, err := FetchMetadata(context.Background(), testServer.Client(), *u)
+	assert.EqualError(t, err, "validator: in token starting at 2:1: roundtrip error: expected {{ EntityDescriptor} [{{ :foo} bar} {{ xmlns} urn:oasis:names:tc:SAML:2.0:metadata} {{xmlns ds} http://www.w3.org/2000/09/xmldsig#} {{xmlns mdalg} urn:oasis:names:tc:SAML:metadata:algsupport} {{xmlns mdui} urn:oasis:names:tc:SAML:metadata:ui} {{xmlns shibmd} urn:mace:shibboleth:metadata:1.0} {{xmlns xsi} http://www.w3.org/2001/XMLSchema-instance} {{ Name} urn:mace:shibboleth:testshib:two} {{ entityID} https://idp.testshib.org/idp/shibboleth}]}, observed {{ EntityDescriptor} [{{ foo} bar} {{ xmlns} urn:oasis:names:tc:SAML:2.0:metadata} {{xmlns ds} http://www.w3.org/2000/09/xmldsig#} {{xmlns mdalg} urn:oasis:names:tc:SAML:metadata:algsupport} {{xmlns mdui} urn:oasis:names:tc:SAML:metadata:ui} {{xmlns shibmd} urn:mace:shibboleth:metadata:1.0} {{xmlns xsi} http://www.w3.org/2001/XMLSchema-instance} {{ Name} urn:mace:shibboleth:testshib:two} {{ entityID} https://idp.testshib.org/idp/shibboleth} {{ entityID} https://idp.testshib.org/idp/shibboleth}]}")
+	assert.Nil(t, md)
+}

@@ -11,11 +11,14 @@ import (
 	"errors"
 	"fmt"
 	"html/template"
+	"io/ioutil"
 	"net/http"
 	"net/url"
 	"regexp"
 	"time"
 
+	xrv "github.com/mattermost/xml-roundtrip-validator"
+
 	"github.com/beevik/etree"
 	dsig "github.com/russellhaering/goxmldsig"
 	"github.com/russellhaering/goxmldsig/etreeutils"
@@ -553,9 +556,15 @@ func (sp *ServiceProvider) ParseXMLResponse(decodedResponseXML []byte, possibleR
 		Response: string(decodedResponseXML),
 	}
 
+	// ensure that the response XML is well formed before we parse it
+	if err := xrv.Validate(bytes.NewReader(decodedResponseXML)); err != nil {
+		retErr.PrivateErr = fmt.Errorf("invalid xml: %s", err)
+		return nil, retErr
+	}
+
 	// do some validation first before we decrypt
 	resp := Response{}
-	if err := xml.Unmarshal([]byte(decodedResponseXML), &resp); err != nil {
+	if err := xml.Unmarshal(decodedResponseXML, &resp); err != nil {
 		retErr.PrivateErr = fmt.Errorf("cannot unmarshal response: %s", err)
 		return nil, retErr
 	}
@@ -659,6 +668,12 @@ func (sp *ServiceProvider) ParseXMLResponse(decodedResponseXML []byte, possibleR
 		}
 		retErr.Response = string(plaintextAssertion)
 
+		// TODO(ross): add test case for this
+		if err := xrv.Validate(bytes.NewReader(plaintextAssertion)); err != nil {
+			retErr.PrivateErr = fmt.Errorf("plaintext response contains invalid XML: %s", err)
+			return nil, retErr
+		}
+
 		doc = etree.NewDocument()
 		if err := doc.ReadFromBytes(plaintextAssertion); err != nil {
 			retErr.PrivateErr = fmt.Errorf("cannot parse plaintext response %v", err)
@@ -673,6 +688,8 @@ func (sp *ServiceProvider) ParseXMLResponse(decodedResponseXML []byte, possibleR
 		}
 
 		assertion = &Assertion{}
+		// Note: plaintextAssertion is known to be safe to parse because
+		// plaintextAssertion is unmodified from when xrv.Validate() was called above.
 		if err := xml.Unmarshal(plaintextAssertion, assertion); err != nil {
 			retErr.PrivateErr = err
 			return nil, retErr
@@ -1001,8 +1018,12 @@ func (sp *ServiceProvider) ValidateLogoutResponseForm(postFormData string) error
 		return fmt.Errorf("unable to parse base64: %s", err)
 	}
 
-	var resp LogoutResponse
+	// TODO(ross): add test case for this (SLO does not have tests right now)
+	if err := xrv.Validate(bytes.NewReader(rawResponseBuf)); err != nil {
+		return fmt.Errorf("response contains invalid XML: %s", err)
+	}
 
+	var resp LogoutResponse
 	if err := xml.Unmarshal(rawResponseBuf, &resp); err != nil {
 		return fmt.Errorf("cannot unmarshal response: %s", err)
 	}
@@ -1034,9 +1055,16 @@ func (sp *ServiceProvider) ValidateLogoutResponseRedirect(queryParameterData str
 		return fmt.Errorf("unable to parse base64: %s", err)
 	}
 
-	gr := flate.NewReader(bytes.NewBuffer(rawResponseBuf))
+	gr, err := ioutil.ReadAll(flate.NewReader(bytes.NewBuffer(rawResponseBuf)))
+	if err != nil {
+		return err
+	}
+
+	if err := xrv.Validate(bytes.NewReader(gr)); err != nil {
+		return err
+	}
 
-	decoder := xml.NewDecoder(gr)
+	decoder := xml.NewDecoder(bytes.NewReader(gr))
 
 	var resp LogoutResponse
 
@@ -1050,7 +1078,7 @@ func (sp *ServiceProvider) ValidateLogoutResponseRedirect(queryParameterData str
 	}
 
 	doc := etree.NewDocument()
-	if _, err := doc.ReadFrom(gr); err != nil {
+	if _, err := doc.ReadFrom(bytes.NewReader(gr)); err != nil {
 		return err
 	}
 

@@ -897,6 +897,81 @@ PUkfbaYHQGP6IS0lzeCeDX0wab3qRoh7/jJt5/BR8Iwf</ds:X509Certificate>
 	}
 }
 
+
+func TestSPRejectsMalformedResponse(t *testing.T) {
+	test := NewServiceProviderTest()
+	// An actual response from google
+	TimeNow = func() time.Time {
+		rv, _ := time.Parse("Mon Jan 2 15:04:05 UTC 2006", "Tue Jan 5 16:55:39 UTC 2016")
+		return rv
+	}
+	Clock = dsig.NewFakeClockAt(TimeNow())
+	SamlResponse := "PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+PHNhbWwycDpSZXNwb25zZSB4bWxuczpzYW1sMnA9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpwcm90b2NvbCIgRGVzdGluYXRpb249Imh0dHBzOi8vMjllZTZkMmUubmdyb2suaW8vc2FtbC9hY3MiIElEPSJfZmMxNDFkYjI4NGViMzA5ODYwNTM1MWJkZTRkOWJlNTkiIEluUmVzcG9uc2VUbz0iaWQtZmQ0MTlhNWFiMDQ3MjY0NTQyN2Y4ZTA3ZDg3YTNhNWRkMGIyZTlhNiIgSXNzdWVJbnN0YW50PSIyMDE2LTAxLTA1VDE2OjU1OjM5LjM0OFoiIFZlcnNpb249IjIuMCI+PHNhbWwyOklzc3VlciB4bWxuczpzYW1sMj0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmFzc2VydGlvbiI+aHR0cHM6Ly9hY2NvdW50cy5nb29nbGUuY29tL28vc2FtbDI/aWRwaWQ9QzAyZGZsMXIxPC9zYW1sMjpJc3N1ZXI+PGRzOlNpZ25hdHVyZSB4bWxuczpkcz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnIyI+PGRzOlNpZ25lZEluZm8+PGRzOkNhbm9uaWNhbGl6YXRpb25NZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzEwL3htbC1leGMtYzE0biMiLz48ZHM6U2lnbmF0dXJlTWV0aG9kIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8wNC94bWxkc2lnLW1vcmUjcnNhLXNoYTI1NiIvPjxkczpSZWZlcmVuY2UgVVJJPSIjX2ZjMTQxZGIyODRlYjMwOTg2MDUzNTFiZGU0ZDliZTU5Ij48ZHM6VHJhbnNmb3Jtcz48ZHM6VHJhbnNmb3JtIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnI2VudmVsb3BlZC1zaWduYXR1cmUiLz48ZHM6VHJhbnNmb3JtIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8xMC94bWwtZXhjLWMxNG4jIi8+PC9kczpUcmFuc2Zvcm1zPjxkczpEaWdlc3RNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzA0L3htbGVuYyNzaGEyNTYiLz48ZHM6RGlnZXN0VmFsdWU+bHRNRUJLRzRZNVNLeERScUxHR2xFSGtPd3hla3dQOStybnA2WEtqdkJxVT08L2RzOkRpZ2VzdFZhbHVlPjwvZHM6UmVmZXJlbmNlPjwvZHM6U2lnbmVkSW5mbz48ZHM6U2lnbmF0dXJlVmFsdWU+SFBVV0pmYTlqdVdiKy9wZ0YrQklsc2pycE40NkE0RUNiT3hNdXhmWEFRUCtrMU5KMG9EdTJKYk1pZHpmclJBRkRHMjZaNjZWQWtkcwpBRmYwVFgzMWxvVjdaU0tGS0lVY0tuaFlXTHFuUTZLbmRydnJLbzF5UUhzUkdUNzJoVjl3SWdqTFRTZm5FV3QvOEMxaERQQi96R0txClhXZ3VvNFFHYlZUeVBoVVh3eEFzRmxBNjFDdkE5Q1pzU2xpeHBaY2pOVjUyQmMydzI5RUNRNStBcHZGWjVqRU1EN1JiQTVpMzdBbmgKUVBCeVYrZXo4ZU9Yc0hvQlhsR0drTjlDR201MFR6djZ3TW12WkdkT2pKWlhvRWZGUTA4UFJwbE9DQWpxSjM3QnhpWitLZWtUaE1KYgorelowcG1yeWR2V3lONEMzNWcycGVueGw2QUtxYnhMaXlJUkVaZz09PC9kczpTaWduYXR1cmVWYWx1ZT48ZHM6S2V5SW5mbz48ZHM6WDUwOURhdGE+PGRzOlg1MDlTdWJqZWN0TmFtZT5TVD1DYWxpZm9ybmlhLEM9VVMsT1U9R29vZ2xlIEZvciBXb3JrLENOPUdvb2dsZSxMPU1vdW50YWluIFZpZXcsTz1Hb29nbGUgSW5jLjwvZHM6WDUwOVN1YmplY3ROYW1lPjxkczpYNTA5Q2VydGlmaWNhdGU+TUlJRGREQ0NBbHlnQXdJQkFnSUdBVklTbElsWU1BMEdDU3FHU0liM0RRRUJDd1VBTUhzeEZEQVNCZ05WQkFvVEMwZHZiMmRzWlNCSgpibU11TVJZd0ZBWURWUVFIRXcxTmIzVnVkR0ZwYmlCV2FXVjNNUTh3RFFZRFZRUURFd1pIYjI5bmJHVXhHREFXQmdOVkJBc1REMGR2CmIyZHNaU0JHYjNJZ1YyOXlhekVMTUFrR0ExVUVCaE1DVlZNeEV6QVJCZ05WQkFnVENrTmhiR2xtYjNKdWFXRXdIaGNOTVRZd01UQTEKTVRZeE56UTVXaGNOTWpFd01UQXpNVFl4TnpRNVdqQjdNUlF3RWdZRFZRUUtFd3RIYjI5bmJHVWdTVzVqTGpFV01CUUdBMVVFQnhNTgpUVzkxYm5SaGFXNGdWbWxsZHpFUE1BMEdBMVVFQXhNR1IyOXZaMnhsTVJnd0ZnWURWUVFMRXc5SGIyOW5iR1VnUm05eUlGZHZjbXN4CkN6QUpCZ05WQkFZVEFsVlRNUk13RVFZRFZRUUlFd3BEWVd4cFptOXlibWxoTUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEEKTUlJQkNnS0NBUUVBbVVmTVVQeEhTWS9aWVo4OGZVR0FsaFVQNE5pN3pqNTR2c3JzUERBNFVoUWlSZUVEUnVuTjFxM09Ic1NoUm9uZwpnZDRMdkE4My9lLzNwbS9WNjBSNnZ5TWZqM1ovSUdXWStlWjk3RUpVdmprdHQrVlJvQWkyNm9lWTlaVzZTODV5YXB2QTNpdWhFd0lRCk9jdVBtMU9xUlEweVE0c1VEK1d0TC9RU21sWXZEUDVUSzFkNndoVGlzTnNLU3FlRlpDYi9zOU9YMDFVZXhXMUJ1RE9MZVZ0MHJDVzEKa1JOY0JCTERtZDRobkRQMFNWcTduTGhORllYajJFYTZXc3lSQUl2Y2hhVUd5K0ltYTJva1htOTVZZTlrbjhlMTE4aS81clJleUtDbQpCbHNrTWtOYUE0S1dLdklRbTNEZGpnT05nRWQwSXZLRXh5THdZN2E1L0pJVXZCaGI5UUlEQVFBQk1BMEdDU3FHU0liM0RRRUJDd1VBCkE0SUJBUUFVRExNbkhwemZwNFNoZEJxQ3JlVzQ4ZjhyVTk0cTJxTXdyVStXNkRrT3JHSlRBU1ZHUzlSaWIvTUtBaVJZT21xbGFxRVkKTlA1N3BDckUvblJCNUZWZEUrQWxTeC9mUjNraHNRM3pmLzRkWXMyMVN2R2YrT2FzOTlYRWJXZlYwT21QTVltM0lyU0NPQkVWMzF3aAo0MXFSYzVRTG5SK1h1dE5QYlNCTit0bitnaVJDTEdDQkxlODFvVnc0ZlJHUWJna2Q4N3JmTE95M0c2MzBJNnMvSjVmZUZGVVQ4ZDdoCjltcE9lT3FMQ1ByS3BxK3dJM2FEM2xmNG1YcUtJRE5pSEhSb05sNjdBTlB1L04zZk5VMUhwbFZ0dnJvVnBpTnA4N2ZyZ2RsS1RFY2cKUFVrZmJhWUhRR1A2SVMwbHplQ2VEWDB3YWIzcVJvaDcvakp0NS9CUjhJd2Y8L2RzOlg1MDlDZXJ0aWZpY2F0ZT48L2RzOlg1MDlEYXRhPjwvZHM6S2V5SW5mbz48L2RzOlNpZ25hdHVyZT48c2FtbDJwOlN0YXR1cz48c2FtbDJwOlN0YXR1c0NvZGUgVmFsdWU9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpzdGF0dXM6U3VjY2VzcyIvPjwvc2FtbDJwOlN0YXR1cz48c2FtbDI6QXNzZXJ0aW9uIHhtbG5zOnNhbWwyPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiBJRD0iXzllNzY0OTUyZTZhMjYxZTE5NDA5YTM4MjU1ODEwMzNkIiBJc3N1ZUluc3RhbnQ9IjIwMTYtMDEtMDVUMTY6NTU6MzkuMzQ4WiIgVmVyc2lvbj0iMi4wIj48c2FtbDI6SXNzdWVyPmh0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbS9vL3NhbWwyP2lkcGlkPUMwMmRmbDFyMTwvc2FtbDI6SXNzdWVyPjxzYW1sMjpTdWJqZWN0PjxzYW1sMjpOYW1lSUQ+cm9zc0BvY3RvbGFicy5pbzwvc2FtbDI6TmFtZUlEPjxzYW1sMjpTdWJqZWN0Q29uZmlybWF0aW9uIE1ldGhvZD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmNtOmJlYXJlciI+PHNhbWwyOlN1YmplY3RDb25maXJtYXRpb25EYXRhIEluUmVzcG9uc2VUbz0iaWQtZmQ0MTlhNWFiMDQ3MjY0NTQyN2Y4ZTA3ZDg3YTNhNWRkMGIyZTlhNiIgTm90T25PckFmdGVyPSIyMDE2LTAxLTA1VDE3OjAwOjM5LjM0OFoiIFJlY2lwaWVudD0iaHR0cHM6Ly8yOWVlNmQyZS5uZ3Jvay5pby9zYW1sL2FjcyIvPjwvc2FtbDI6U3ViamVjdENvbmZpcm1hdGlvbj48L3NhbWwyOlN1YmplY3Q+PHNhbWwyOkNvbmRpdGlvbnMgTm90QmVmb3JlPSIyMDE2LTAxLTA1VDE2OjUwOjM5LjM0OFoiIE5vdE9uT3JBZnRlcj0iMjAxNi0wMS0wNVQxNzowMDozOS4zNDhaIj48c2FtbDI6QXVkaWVuY2VSZXN0cmljdGlvbj48c2FtbDI6QXVkaWVuY2U+aHR0cHM6Ly8yOWVlNmQyZS5uZ3Jvay5pby9zYW1sL21ldGFkYXRhPC9zYW1sMjpBdWRpZW5jZT48L3NhbWwyOkF1ZGllbmNlUmVzdHJpY3Rpb24+PC9zYW1sMjpDb25kaXRpb25zPjxzYW1sMjpBdHRyaWJ1dGVTdGF0ZW1lbnQ+PHNhbWwyOkF0dHJpYnV0ZSBOYW1lPSJwaG9uZSIvPjxzYW1sMjpBdHRyaWJ1dGUgTmFtZT0iYWRkcmVzcyIvPjxzYW1sMjpBdHRyaWJ1dGUgTmFtZT0iam9iVGl0bGUiLz48c2FtbDI6QXR0cmlidXRlIE5hbWU9ImZpcnN0TmFtZSI+PHNhbWwyOkF0dHJpYnV0ZVZhbHVlIHhtbG5zOnhzPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxL1hNTFNjaGVtYSIgeG1sbnM6eHNpPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxL1hNTFNjaGVtYS1pbnN0YW5jZSIgeHNpOnR5cGU9InhzOmFueVR5cGUiPlJvc3M8L3NhbWwyOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDI6QXR0cmlidXRlPjxzYW1sMjpBdHRyaWJ1dGUgTmFtZT0ibGFzdE5hbWUiPjxzYW1sMjpBdHRyaWJ1dGVWYWx1ZSB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czphbnlUeXBlIj5LaW5kZXI8L3NhbWwyOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDI6QXR0cmlidXRlPjwvc2FtbDI6QXR0cmlidXRlU3RhdGVtZW50PjxzYW1sMjpBdXRoblN0YXRlbWVudCBBdXRobkluc3RhbnQ9IjIwMTYtMDEtMDVUMTY6NTU6MzguMDAwWiIgU2Vzc2lvbkluZGV4PSJfOWU3NjQ5NTJlNmEyNjFlMTk0MDlhMzgyNTU4MTAzM2QiPjxzYW1sMjpBdXRobkNvbnRleHQ+PHNhbWwyOkF1dGhuQ29udGV4dENsYXNzUmVmPnVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphYzpjbGFzc2VzOnVuc3BlY2lmaWVkPC9zYW1sMjpBdXRobkNvbnRleHRDbGFzc1JlZj48L3NhbWwyOkF1dGhuQ29udGV4dD48L3NhbWwyOkF1dGhuU3RhdGVtZW50Pjwvc2FtbDI6QXNzZXJ0aW9uPjwvc2FtbDJwOlJlc3BvbnNlPg=="
+	test.IDPMetadata = `<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://accounts.google.com/o/saml2?idpid=C02dfl1r1" validUntil="2021-01-03T16:17:49.000Z">
+  <md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+    <md:KeyDescriptor use="signing">
+      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+        <ds:X509Data>
+          <ds:X509Certificate>MIIDdDCCAlygAwIBAgIGAVISlIlYMA0GCSqGSIb3DQEBCwUAMHsxFDASBgNVBAoTC0dvb2dsZSBJ
+bmMuMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MQ8wDQYDVQQDEwZHb29nbGUxGDAWBgNVBAsTD0dv
+b2dsZSBGb3IgV29yazELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWEwHhcNMTYwMTA1
+MTYxNzQ5WhcNMjEwMTAzMTYxNzQ5WjB7MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEWMBQGA1UEBxMN
+TW91bnRhaW4gVmlldzEPMA0GA1UEAxMGR29vZ2xlMRgwFgYDVQQLEw9Hb29nbGUgRm9yIFdvcmsx
+CzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
+MIIBCgKCAQEAmUfMUPxHSY/ZYZ88fUGAlhUP4Ni7zj54vsrsPDA4UhQiReEDRunN1q3OHsShRong
+gd4LvA83/e/3pm/V60R6vyMfj3Z/IGWY+eZ97EJUvjktt+VRoAi26oeY9ZW6S85yapvA3iuhEwIQ
+OcuPm1OqRQ0yQ4sUD+WtL/QSmlYvDP5TK1d6whTisNsKSqeFZCb/s9OX01UexW1BuDOLeVt0rCW1
+kRNcBBLDmd4hnDP0SVq7nLhNFYXj2Ea6WsyRAIvchaUGy+Ima2okXm95Ye9kn8e118i/5rReyKCm
+BlskMkNaA4KWKvIQm3DdjgONgEd0IvKExyLwY7a5/JIUvBhb9QIDAQABMA0GCSqGSIb3DQEBCwUA
+A4IBAQAUDLMnHpzfp4ShdBqCreW48f8rU94q2qMwrU+W6DkOrGJTASVGS9Rib/MKAiRYOmqlaqEY
+NP57pCrE/nRB5FVdE+AlSx/fR3khsQ3zf/4dYs21SvGf+Oas99XEbWfV0OmPMYm3IrSCOBEV31wh
+41qRc5QLnR+XutNPbSBN+tn+giRCLGCBLe81oVw4fRGQbgkd87rfLOy3G630I6s/J5feFFUT8d7h
+9mpOeOqLCPrKpq+wI3aD3lf4mXqKIDNiHHRoNl67ANPu/N3fNU1HplVtvroVpiNp87frgdlKTEcg
+PUkfbaYHQGP6IS0lzeCeDX0wab3qRoh7/jJt5/BR8Iwf</ds:X509Certificate>
+        </ds:X509Data>
+      </ds:KeyInfo>
+    </md:KeyDescriptor>
+    <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://accounts.google.com/o/saml2/idp?idpid=C02dfl1r1"/>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://accounts.google.com/o/saml2/idp?idpid=C02dfl1r1"/>
+  </md:IDPSSODescriptor>
+</md:EntityDescriptor>`
+
+	s := ServiceProvider{
+		Key:         test.Key,
+		Certificate: test.Certificate,
+		MetadataURL: mustParseURL("https://29ee6d2e.ngrok.io/saml/metadata"),
+		AcsURL:      mustParseURL("https://29ee6d2e.ngrok.io/saml/acs"),
+		IDPMetadata: &EntityDescriptor{},
+	}
+	err := xml.Unmarshal([]byte(test.IDPMetadata), &s.IDPMetadata)
+	assert.NoError(t, err)
+
+	// this is a valid response
+	{
+		req := http.Request{PostForm: url.Values{}}
+		req.PostForm.Set("SAMLResponse", SamlResponse)
+		assertion, err := s.ParseResponse(&req, []string{"id-fd419a5ab0472645427f8e07d87a3a5dd0b2e9a6"})
+		assert.NoError(t, err)
+		assert.Equal(t, "ross@octolabs.io", assertion.Subject.NameID.Value)
+	}
+
+	// this is a valid response but with a comment injected
+	{
+		x, _ := base64.StdEncoding.DecodeString(SamlResponse)
+		y := strings.Replace(string(x), "<saml2p:Response", "<saml2p:Response ::foo=\"bar\"", 1)
+		SamlResponse = base64.StdEncoding.EncodeToString([]byte(y))
+
+		req := http.Request{PostForm: url.Values{}}
+		req.PostForm.Set("SAMLResponse", SamlResponse)
+		assertion, err := s.ParseResponse(&req, []string{"id-fd419a5ab0472645427f8e07d87a3a5dd0b2e9a6"})
+		assert.EqualError(t, err.(*InvalidResponseError).PrivateErr,
+			"invalid xml: validator: in token starting at 1:55: roundtrip error: expected {{saml2p Response} [{{ :foo} bar} {{xmlns saml2p} urn:oasis:names:tc:SAML:2.0:protocol} {{ Destination} https://29ee6d2e.ngrok.io/saml/acs} {{ ID} _fc141db284eb3098605351bde4d9be59} {{ InResponseTo} id-fd419a5ab0472645427f8e07d87a3a5dd0b2e9a6} {{ IssueInstant} 2016-01-05T16:55:39.348Z} {{ Version} 2.0}]}, observed {{ Response} [{{ xmlns} saml2p} {{ foo} bar} {{xmlns saml2p} urn:oasis:names:tc:SAML:2.0:protocol} {{ Destination} https://29ee6d2e.ngrok.io/saml/acs} {{ ID} _fc141db284eb3098605351bde4d9be59} {{ InResponseTo} id-fd419a5ab0472645427f8e07d87a3a5dd0b2e9a6} {{ IssueInstant} 2016-01-05T16:55:39.348Z} {{ Version} 2.0} {{ Version} 2.0}]}")
+		assert.Nil(t, assertion)
+	}
+}
+
 func TestSPCanParseResponse(t *testing.T) {
 	test := NewServiceProviderTest()
 	s := ServiceProvider{