crewjam/saml go library is vulnerable to signature bypass via multiple Assertion elements

@@ -3,6 +3,7 @@ package saml
 import (
 	"bytes"
 	"compress/flate"
+	"context"
 	"crypto/rsa"
 	"crypto/tls"
 	"crypto/x509"
@@ -17,9 +18,8 @@ import (
 	"regexp"
 	"time"
 
-	xrv "github.com/mattermost/xml-roundtrip-validator"
-
 	"github.com/beevik/etree"
+	xrv "github.com/mattermost/xml-roundtrip-validator"
 	dsig "github.com/russellhaering/goxmldsig"
 	"github.com/russellhaering/goxmldsig/etreeutils"
 
@@ -592,97 +592,79 @@ func (e ErrBadStatus) Error() string {
 	return e.Status
 }
 
-func responseIsSigned(response *etree.Element) (bool, error) {
-	signatureElement, err := findChild(response, "http://www.w3.org/2000/09/xmldsig#", "Signature")
-	if err != nil {
-		return false, err
+// ParseResponse extracts the SAML IDP response received in req, resolves
+// artifacts when necessary, validates it, and returns the verified assertion.
+func (sp *ServiceProvider) ParseResponse(req *http.Request, possibleRequestIDs []string) (*Assertion, error) {
+	if artifactID := req.Form.Get("SAMLart"); artifactID != "" {
+		return sp.handleArtifactRequest(req.Context(), artifactID, possibleRequestIDs)
 	}
-	return signatureElement != nil, nil
+	return sp.parseResponseHTTP(req, possibleRequestIDs)
 }
 
-// validateDestination validates the Destination attribute.
-// If the response is signed, the Destination is required to be present.
-func (sp *ServiceProvider) validateDestination(response *etree.Element, responseDom *Response) error {
-	signed, err := responseIsSigned(response)
+func (sp *ServiceProvider) handleArtifactRequest(ctx context.Context, artifactID string, possibleRequestIDs []string) (*Assertion, error) {
+	retErr := &InvalidResponseError{Now: TimeNow()}
+
+	artifactResolveRequest, err := sp.MakeArtifactResolveRequest(artifactID)
 	if err != nil {
-		return err
+		retErr.PrivateErr = fmt.Errorf("Cannot generate artifact resolution request: %s", err)
+		return nil, retErr
 	}
 
-	// Compare if the response is signed OR the Destination is provided.
-	// (Even if the response is not signed, if the Destination is set it must match.)
-	if signed || responseDom.Destination != "" {
-		if responseDom.Destination != sp.AcsURL.String() {
-			return fmt.Errorf("`Destination` does not match AcsURL (expected %q, actual %q)", sp.AcsURL.String(), responseDom.Destination)
-		}
+	requestBody, err := elementToBytes(artifactResolveRequest.SoapRequest())
+	if err != nil {
+		retErr.PrivateErr = err
+		return nil, retErr
 	}
 
-	return nil
-}
-
-// ParseResponse extracts the SAML IDP response received in req, resolves
-// artifacts when necessary, validates it, and returns the verified assertion.
-func (sp *ServiceProvider) ParseResponse(req *http.Request, possibleRequestIDs []string) (*Assertion, error) {
-	now := TimeNow()
+	req, err := http.NewRequestWithContext(ctx, "POST", sp.GetArtifactBindingLocation(SOAPBinding),
+		bytes.NewReader(requestBody))
+	if err != nil {
+		retErr.PrivateErr = err
+		return nil, retErr
+	}
 
-	var assertion *Assertion
+	httpClient := sp.HTTPClient
+	if httpClient == nil {
+		httpClient = http.DefaultClient
+	}
+	response, err := httpClient.Do(req)
+	if err != nil {
+		retErr.PrivateErr = fmt.Errorf("cannot resolve artifact: %s", err)
+		return nil, retErr
+	}
+	defer response.Body.Close()
+	if response.StatusCode != 200 {
+		retErr.PrivateErr = fmt.Errorf("Error during artifact resolution: HTTP status %d (%s)", response.StatusCode, response.Status)
+		return nil, retErr
+	}
+	responseBody, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		retErr.PrivateErr = fmt.Errorf("Error during artifact resolution: %s", err)
+		return nil, retErr
+	}
+	assertion, err := sp.ParseXMLArtifactResponse(responseBody, possibleRequestIDs, artifactResolveRequest.ID)
+	if err != nil {
+		return nil, err
+	}
+	return assertion, nil
+}
 
+func (sp *ServiceProvider) parseResponseHTTP(req *http.Request, possibleRequestIDs []string) (*Assertion, error) {
 	retErr := &InvalidResponseError{
-		Now:      now,
-		Response: req.PostForm.Get("SAMLResponse"),
+		Now: TimeNow(),
 	}
 
-	if req.Form.Get("SAMLart") != "" {
-		retErr.Response = req.Form.Get("SAMLart")
-
-		req, err := sp.MakeArtifactResolveRequest(req.Form.Get("SAMLart"))
-		if err != nil {
-			retErr.PrivateErr = fmt.Errorf("Cannot generate artifact resolution request: %s", err)
-			return nil, retErr
-		}
-
-		doc := etree.NewDocument()
-		doc.SetRoot(req.SoapRequest())
-
-		var requestBuffer bytes.Buffer
-		doc.WriteTo(&requestBuffer)
-		client := sp.HTTPClient
-		if client == nil {
-			client = http.DefaultClient
-		}
-		response, err := client.Post(sp.GetArtifactBindingLocation(SOAPBinding), "text/xml", &requestBuffer)
-		if err != nil {
-			retErr.PrivateErr = fmt.Errorf("Error during artifact resolution: %s", err)
-			return nil, retErr
-		}
-		defer response.Body.Close()
-		if response.StatusCode != 200 {
-			retErr.PrivateErr = fmt.Errorf("Error during artifact resolution: HTTP status %d (%s)", response.StatusCode, response.Status)
-			return nil, retErr
-		}
-		rawResponseBuf, err := ioutil.ReadAll(response.Body)
-		if err != nil {
-			retErr.PrivateErr = fmt.Errorf("Error during artifact resolution: %s", err)
-			return nil, retErr
-		}
-		assertion, err = sp.ParseXMLArtifactResponse(rawResponseBuf, possibleRequestIDs, req.ID)
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		rawResponseBuf, err := base64.StdEncoding.DecodeString(req.PostForm.Get("SAMLResponse"))
-		if err != nil {
-			retErr.PrivateErr = fmt.Errorf("cannot parse base64: %s", err)
-			return nil, retErr
-		}
-		retErr.Response = string(rawResponseBuf)
-		assertion, err = sp.ParseXMLResponse(rawResponseBuf, possibleRequestIDs)
-		if err != nil {
-			return nil, err
-		}
+	rawResponseBuf, err := base64.StdEncoding.DecodeString(req.PostForm.Get("SAMLResponse"))
+	if err != nil {
+		retErr.PrivateErr = fmt.Errorf("cannot parse base64: %s", err)
+		return nil, retErr
 	}
 
+	assertion, err := sp.ParseXMLResponse(rawResponseBuf, possibleRequestIDs)
+	if err != nil {
+		return nil, err
+	}
 	return assertion, nil
-
 }
 
 // ParseXMLArtifactResponse validates the SAML Artifact resolver response
@@ -695,83 +677,95 @@ func (sp *ServiceProvider) ParseResponse(req *http.Request, possibleRequestIDs [
 // properties are useful in describing which part of the parsing process
 // failed. However, to discourage inadvertent disclosure the diagnostic
 // information, the Error() method returns a static string.
-func (sp *ServiceProvider) ParseXMLArtifactResponse(decodedResponseXML []byte, possibleRequestIDs []string, artifactRequestID string) (*Assertion, error) {
+func (sp *ServiceProvider) ParseXMLArtifactResponse(soapResponseXML []byte, possibleRequestIDs []string, artifactRequestID string) (*Assertion, error) {
 	now := TimeNow()
-	//var err error
 	retErr := &InvalidResponseError{
+		Response: string(soapResponseXML),
 		Now:      now,
-		Response: string(decodedResponseXML),
 	}
 
-	// ensure that the response XML is well formed before we parse it
-	if err := xrv.Validate(bytes.NewReader(decodedResponseXML)); err != nil {
+	// ensure that the response XML is well-formed before we parse it
+	if err := xrv.Validate(bytes.NewReader(soapResponseXML)); err != nil {
 		retErr.PrivateErr = fmt.Errorf("invalid xml: %s", err)
 		return nil, retErr
 	}
 
-	envelope := &struct {
-		XMLName xml.Name `xml:"http://schemas.xmlsoap.org/soap/envelope/ Envelope"`
-		Body    struct {
-			ArtifactResponse ArtifactResponse
-		} `xml:"http://schemas.xmlsoap.org/soap/envelope/ Body"`
-	}{}
-	if err := xml.Unmarshal(decodedResponseXML, &envelope); err != nil {
+	doc := etree.NewDocument()
+	if err := doc.ReadFromBytes(soapResponseXML); err != nil {
 		retErr.PrivateErr = fmt.Errorf("cannot unmarshal response: %s", err)
 		return nil, retErr
 	}
-
-	resp := envelope.Body.ArtifactResponse
-
-	// Validate ArtifactResponse
-	if resp.InResponseTo != artifactRequestID {
-		retErr.PrivateErr = fmt.Errorf("`InResponseTo` does not match the artifact request ID (expected %v)", artifactRequestID)
+	if doc.Root().NamespaceURI() != "http://schemas.xmlsoap.org/soap/envelope/" ||
+		doc.Root().Tag != "Envelope" {
+		retErr.PrivateErr = fmt.Errorf("expected a SOAP Envelope")
 		return nil, retErr
 	}
-	if resp.IssueInstant.Add(MaxIssueDelay).Before(now) {
-		retErr.PrivateErr = fmt.Errorf("response IssueInstant expired at %s", resp.IssueInstant.Add(MaxIssueDelay))
-		return nil, retErr
-	}
-	if resp.Issuer != nil && resp.Issuer.Value != sp.IDPMetadata.EntityID {
-		retErr.PrivateErr = fmt.Errorf("response Issuer does not match the IDP metadata (expected %q)", sp.IDPMetadata.EntityID)
-		return nil, retErr
-	}
-	if resp.Status.StatusCode.Value != StatusSuccess {
-		retErr.PrivateErr = ErrBadStatus{Status: resp.Status.StatusCode.Value}
+
+	soapBodyEl, err := findOneChild(doc.Root(), "http://schemas.xmlsoap.org/soap/envelope/", "Body")
+	if err != nil {
+		retErr.PrivateErr = err
 		return nil, retErr
 	}
 
-	doc := etree.NewDocument()
-	if err := doc.ReadFromBytes(decodedResponseXML); err != nil {
+	artifactResponseEl, err := findOneChild(soapBodyEl, "urn:oasis:names:tc:SAML:2.0:protocol", "ArtifactResponse")
+	if err != nil {
 		retErr.PrivateErr = err
 		return nil, retErr
 	}
 
-	artifactEl := doc.FindElement("Envelope/Body/ArtifactResponse")
-	if artifactEl == nil {
-		retErr.PrivateErr = fmt.Errorf("missing ArtifactResponse")
-		return nil, retErr
+	return sp.parseArtifactResponse(artifactResponseEl, possibleRequestIDs, artifactRequestID, now)
+}
+
+func (sp *ServiceProvider) parseArtifactResponse(artifactResponseEl *etree.Element, possibleRequestIDs []string, artifactRequestID string, now time.Time) (*Assertion, error) {
+	retErr := &InvalidResponseError{
+		Now:      now,
+		Response: elementToString(artifactResponseEl),
+	}
+
+	{
+		var artifactResponse ArtifactResponse
+		if err := unmarshalElement(artifactResponseEl, &artifactResponse); err != nil {
+			retErr.PrivateErr = err
+			return nil, retErr
+		}
+		if artifactResponse.InResponseTo != artifactRequestID {
+			retErr.PrivateErr = fmt.Errorf("`InResponseTo` does not match the artifact request ID (expected %s)", artifactRequestID)
+			return nil, retErr
+		}
+		if artifactResponse.IssueInstant.Add(MaxIssueDelay).Before(now) {
+			retErr.PrivateErr = fmt.Errorf("response IssueInstant expired at %s", artifactResponse.IssueInstant.Add(MaxIssueDelay))
+			return nil, retErr
+		}
+		if artifactResponse.Issuer != nil && artifactResponse.Issuer.Value != sp.IDPMetadata.EntityID {
+			retErr.PrivateErr = fmt.Errorf("response Issuer does not match the IDP metadata (expected %q)", sp.IDPMetadata.EntityID)
+			return nil, retErr
+		}
+		if artifactResponse.Status.StatusCode.Value != StatusSuccess {
+			retErr.PrivateErr = ErrBadStatus{Status: artifactResponse.Status.StatusCode.Value}
+			return nil, retErr
+		}
 	}
-	responseEl := doc.FindElement("Envelope/Body/ArtifactResponse/Response")
-	if responseEl == nil {
-		retErr.PrivateErr = fmt.Errorf("missing inner Response")
+
+	var signatureRequirement signatureRequirement
+	sigErr := sp.validateSignature(artifactResponseEl)
+	if sigErr == nil {
+		signatureRequirement = signatureNotRequired
+	} else if sigErr == errSignatureElementNotPresent {
+		signatureRequirement = signatureRequired
+	} else {
+		retErr.PrivateErr = sigErr
 		return nil, retErr
 	}
 
-	haveSignature := false
-	var err error
-	if err = sp.validateArtifactSigned(artifactEl); err != nil && err.Error() != "either the Response or Assertion must be signed" {
+	responseEl, err := findOneChild(artifactResponseEl, "urn:oasis:names:tc:SAML:2.0:protocol", "Response")
+	if err != nil {
 		retErr.PrivateErr = err
 		return nil, retErr
 	}
-	if err == nil {
-		haveSignature = true
-	}
-	assertion, updatedResponse, err := sp.validateXMLResponse(&resp.Response, responseEl, possibleRequestIDs, now, !haveSignature)
+
+	assertion, err := sp.parseResponse(responseEl, possibleRequestIDs, now, signatureRequirement)
 	if err != nil {
 		retErr.PrivateErr = err
-		if updatedResponse != nil {
-			retErr.Response = *updatedResponse
-		}
 		return nil, retErr
 	}
 
@@ -797,150 +791,213 @@ func (sp *ServiceProvider) ParseXMLResponse(decodedResponseXML []byte, possibleR
 		Response: string(decodedResponseXML),
 	}
 
-	// ensure that the response XML is well formed before we parse it
+	// 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(decodedResponseXML, &resp); err != nil {
-		retErr.PrivateErr = fmt.Errorf("cannot unmarshal response: %s", err)
-		return nil, retErr
-	}
-
 	doc := etree.NewDocument()
 	if err := doc.ReadFromBytes(decodedResponseXML); err != nil {
 		retErr.PrivateErr = err
 		return nil, retErr
 	}
 
-	assertion, updatedResponse, err := sp.validateXMLResponse(&resp, doc.Root(), possibleRequestIDs, now, true)
+	assertion, err := sp.parseResponse(doc.Root(), possibleRequestIDs, now, signatureRequired)
 	if err != nil {
 		retErr.PrivateErr = err
-		if updatedResponse != nil {
-			retErr.Response = *updatedResponse
-		}
 		return nil, retErr
 	}
 
 	return assertion, nil
 }
 
+type signatureRequirement int
+
+const (
+	signatureRequired signatureRequirement = iota
+	signatureNotRequired
+)
+
 // validateXMLResponse validates the SAML IDP response and returns
 // the verified assertion.
 //
 // This function handles decrypting the message, verifying the digital
 // signature on the assertion, and verifying that the specified conditions
 // and properties are met.
-func (sp *ServiceProvider) validateXMLResponse(resp *Response, responseEl *etree.Element, possibleRequestIDs []string, now time.Time, needSig bool) (*Assertion, *string, error) {
-	var err error
-	var updatedResponse *string
-	if err := sp.validateDestination(responseEl, resp); err != nil {
-		return nil, updatedResponse, err
+func (sp *ServiceProvider) parseResponse(responseEl *etree.Element, possibleRequestIDs []string, now time.Time, signatureRequirement signatureRequirement) (*Assertion, error) {
+	var responseSignatureErr error
+	var responseHasSignature bool
+	if signatureRequirement == signatureRequired {
+		responseSignatureErr = sp.validateSignature(responseEl)
+		if responseSignatureErr != errSignatureElementNotPresent {
+			responseHasSignature = true
+		}
+
+		// Note: we're deferring taking action on the signature validation until after we've
+		// processed the request attributes, because certain test cases seem to require this mis-feature.
+		// TODO(ross): adjust the test cases so that we can abort here if the Response signature is invalid.
 	}
 
-	requestIDvalid := false
+	// validate request attributes
+	{
+		var response Response
+		if err := unmarshalElement(responseEl, &response); err != nil {
+			return nil, fmt.Errorf("cannot unmarshal response: %v", err)
+		}
 
-	if sp.AllowIDPInitiated {
-		requestIDvalid = true
-	} else {
-		for _, possibleRequestID := range possibleRequestIDs {
-			if resp.InResponseTo == possibleRequestID {
-				requestIDvalid = true
+		// If the response is *not* signed, the Destination may be omitted.
+		if responseHasSignature || response.Destination != "" {
+			if response.Destination != sp.AcsURL.String() {
+				return nil, fmt.Errorf("`Destination` does not match AcsURL (expected %q, actual %q)", sp.AcsURL.String(), response.Destination)
 			}
 		}
-	}
-
-	if !requestIDvalid {
-		return nil, updatedResponse, fmt.Errorf("`InResponseTo` does not match any of the possible request IDs (expected %v)", possibleRequestIDs)
-	}
 
-	if resp.IssueInstant.Add(MaxIssueDelay).Before(now) {
-		return nil, updatedResponse, fmt.Errorf("response IssueInstant expired at %s", resp.IssueInstant.Add(MaxIssueDelay))
-	}
-	if resp.Issuer != nil && resp.Issuer.Value != sp.IDPMetadata.EntityID {
-		return nil, updatedResponse, fmt.Errorf("response Issuer does not match the IDP metadata (expected %q)", sp.IDPMetadata.EntityID)
-	}
-	if resp.Status.StatusCode.Value != StatusSuccess {
-		return nil, updatedResponse, ErrBadStatus{Status: resp.Status.StatusCode.Value}
-	}
-
-	var assertion *Assertion
-	if resp.EncryptedAssertion == nil {
-		// TODO(ross): verify that the namespace is urn:oasis:names:tc:SAML:2.0:protocol
-		if responseEl.Tag != "Response" {
-			return nil, updatedResponse, fmt.Errorf("expected to find a response object, not %s", responseEl.Tag)
+		requestIDvalid := false
+		if sp.AllowIDPInitiated {
+			requestIDvalid = true
+		} else {
+			for _, possibleRequestID := range possibleRequestIDs {
+				if response.InResponseTo == possibleRequestID {
+					requestIDvalid = true
+				}
+			}
+		}
+		if !requestIDvalid {
+			return nil, fmt.Errorf("`InResponseTo` does not match any of the possible request IDs (expected %v)", possibleRequestIDs)
 		}
 
-		if err = sp.validateSigned(responseEl); err != nil && !(!needSig && err.Error() == "either the Response or Assertion must be signed") {
-			return nil, updatedResponse, err
+		if response.IssueInstant.Add(MaxIssueDelay).Before(now) {
+			return nil, fmt.Errorf("response IssueInstant expired at %s", response.IssueInstant.Add(MaxIssueDelay))
+		}
+		if response.Issuer != nil && response.Issuer.Value != sp.IDPMetadata.EntityID {
+			return nil, fmt.Errorf("response Issuer does not match the IDP metadata (expected %q)", sp.IDPMetadata.EntityID)
 		}
+		if response.Status.StatusCode.Value != StatusSuccess {
+			return nil, ErrBadStatus{Status: response.Status.StatusCode.Value}
+		}
+	}
 
-		assertion = resp.Assertion
+	if signatureRequirement == signatureRequired {
+		if responseSignatureErr == nil {
+			// since the request has a signature, none of the Assertions need one
+			signatureRequirement = signatureNotRequired
+		} else if responseSignatureErr == errSignatureElementNotPresent {
+			// the request has no signature, so assertions must be signed
+			signatureRequirement = signatureRequired // nop
+		} else {
+			return nil, responseSignatureErr
+		}
 	}
 
-	// decrypt the response
-	if resp.EncryptedAssertion != nil {
-		// encrypted assertions are part of the signature
-		// before decrypting the response verify that
-		responseSigned, err := responseIsSigned(responseEl)
+	var errs []error
+	var assertions []Assertion
+
+	// look for encrypted assertions
+	{
+		encryptedAssertionEls, err := findChildren(responseEl, "urn:oasis:names:tc:SAML:2.0:assertion", "EncryptedAssertion")
 		if err != nil {
-			return nil, updatedResponse, err
+			return nil, err
 		}
-		if responseSigned {
-			if err := sp.validateSigned(responseEl); err != nil {
-				return nil, updatedResponse, err
+		for _, encryptedAssertionEl := range encryptedAssertionEls {
+			assertion, err := sp.parseEncryptedAssertion(encryptedAssertionEl, possibleRequestIDs, now, signatureRequirement)
+			if err != nil {
+				errs = append(errs, err)
+				continue
 			}
+			assertions = append(assertions, *assertion)
 		}
+	}
 
-		var key interface{} = sp.Key
-		keyEl := responseEl.FindElement("//EncryptedAssertion/EncryptedKey")
-		if keyEl != nil {
-			key, err = xmlenc.Decrypt(sp.Key, keyEl)
+	// look for plaintext assertions
+	{
+		assertionEls, err := findChildren(responseEl, "urn:oasis:names:tc:SAML:2.0:assertion", "Assertion")
+		if err != nil {
+			return nil, err
+		}
+		for _, assertionEl := range assertionEls {
+			assertion, err := sp.parseAssertion(assertionEl, possibleRequestIDs, now, signatureRequirement)
 			if err != nil {
-				return nil, updatedResponse, fmt.Errorf("failed to decrypt key from response: %s", err)
+				errs = append(errs, err)
+				continue
 			}
+			assertions = append(assertions, *assertion)
 		}
+	}
 
-		el := responseEl.FindElement("//EncryptedAssertion/EncryptedData")
-		plaintextAssertion, err := xmlenc.Decrypt(key, el)
-		if err != nil {
-			return nil, updatedResponse, fmt.Errorf("failed to decrypt response: %s", err)
+	if len(assertions) == 0 {
+		if len(errs) > 0 {
+			return nil, errs[0]
 		}
-		updatedResponse = new(string)
-		*updatedResponse = string(plaintextAssertion)
+		return nil, fmt.Errorf("expected at least one valid Assertion, none found")
+	}
 
-		// TODO(ross): add test case for this
-		if err := xrv.Validate(bytes.NewReader(plaintextAssertion)); err != nil {
-			return nil, updatedResponse, fmt.Errorf("plaintext response contains invalid XML: %s", err)
-		}
+	// if we have at least one assertion, return the first one. It is almost universally true that valid responses
+	// contain only one assertion. This is less that fully correct, but we didn't realize that there could be more
+	// than one assertion at the time of establishing the public interface of ParseXMLResponse(), so for compatability
+	// we return the first one.
+	return &assertions[0], nil
+}
 
-		doc := etree.NewDocument()
-		if err := doc.ReadFromBytes(plaintextAssertion); err != nil {
-			return nil, updatedResponse, fmt.Errorf("cannot parse plaintext response %v", err)
-		}
+func (sp *ServiceProvider) parseEncryptedAssertion(encryptedAssertionEl *etree.Element, possibleRequestIDs []string, now time.Time, signatureRequirement signatureRequirement) (*Assertion, error) {
+	assertionEl, err := sp.decryptElement(encryptedAssertionEl)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decrypt EncryptedAssertion: %v", err)
+	}
+	return sp.parseAssertion(assertionEl, possibleRequestIDs, now, signatureRequirement)
+}
 
-		// the decrypted assertion may be signed too
-		// otherwise, a signed response is sufficient
-		if err := sp.validateSigned(doc.Root()); err != nil && !((responseSigned || !needSig) && err.Error() == "either the Response or Assertion must be signed") {
-			return nil, updatedResponse, err
+func (sp *ServiceProvider) decryptElement(encryptedEl *etree.Element) (*etree.Element, error) {
+	encryptedDataEl, err := findOneChild(encryptedEl, "http://www.w3.org/2001/04/xmlenc#", "EncryptedData")
+	if err != nil {
+		return nil, err
+	}
+
+	var key interface{} = sp.Key
+	keyEl := encryptedEl.FindElement("./EncryptedKey")
+	if keyEl != nil {
+		var err error
+		key, err = xmlenc.Decrypt(sp.Key, keyEl)
+		if err != nil {
+			return nil, fmt.Errorf("failed to decrypt key from response: %s", err)
 		}
+	}
+
+	plaintextEl, err := xmlenc.Decrypt(key, encryptedDataEl)
+	if err != nil {
+		return nil, err
+	}
 
-		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 {
-			return nil, updatedResponse, err
+	if err := xrv.Validate(bytes.NewReader(plaintextEl)); err != nil {
+		return nil, fmt.Errorf("plaintext response contains invalid XML: %s", err)
+	}
+
+	doc := etree.NewDocument()
+	if err := doc.ReadFromBytes(plaintextEl); err != nil {
+		return nil, fmt.Errorf("cannot parse plaintext response %v", err)
+	}
+	return doc.Root(), nil
+}
+
+func (sp *ServiceProvider) parseAssertion(assertionEl *etree.Element, possibleRequestIDs []string, now time.Time, signatureRequirement signatureRequirement) (*Assertion, error) {
+	if signatureRequirement == signatureRequired {
+		sigErr := sp.validateSignature(assertionEl)
+		if sigErr != nil {
+			return nil, sigErr
 		}
 	}
 
-	if err := sp.validateAssertion(assertion, possibleRequestIDs, now); err != nil {
-		return nil, updatedResponse, fmt.Errorf("assertion invalid: %s", err)
+	// parse the assertion we just validated
+	var assertion Assertion
+	if err := unmarshalElement(assertionEl, &assertion); err != nil {
+		return nil, err
 	}
 
-	return assertion, updatedResponse, nil
+	if err := sp.validateAssertion(&assertion, possibleRequestIDs, now); err != nil {
+		return nil, err
+	}
+
+	return &assertion, nil
 }
 
 // validateAssertion checks that the conditions specified in assertion match
@@ -1012,117 +1069,21 @@ func (sp *ServiceProvider) validateAssertion(assertion *Assertion, possibleReque
 	return nil
 }
 
-func findChild(parentEl *etree.Element, childNS string, childTag string) (*etree.Element, error) {
-	for _, childEl := range parentEl.ChildElements() {
-		if childEl.Tag != childTag {
-			continue
-		}
-
-		ctx, err := etreeutils.NSBuildParentContext(childEl)
-		if err != nil {
-			return nil, err
-		}
-		ctx, err = ctx.SubContext(childEl)
-		if err != nil {
-			return nil, err
-		}
-
-		ns, err := ctx.LookupPrefix(childEl.Space)
-		if err != nil {
-			return nil, fmt.Errorf("[%s]:%s cannot find prefix %s: %v", childNS, childTag, childEl.Space, err)
-		}
-		if ns != childNS {
-			continue
-		}
-
-		return childEl, nil
-	}
-	return nil, nil
-}
-
-// validateArtifactSigned returns a nil error iff each of the signatures on the ArtifactResponse, Response
-// and Assertion elements are valid and there is at least one signature.
-func (sp *ServiceProvider) validateArtifactSigned(artifactEl *etree.Element) error {
-	haveSignature := false
+var errSignatureElementNotPresent = errors.New("Signature element not present")
 
-	sigEl, err := findChild(artifactEl, "http://www.w3.org/2000/09/xmldsig#", "Signature")
-	if err != nil {
-		return err
-	}
-	if sigEl != nil {
-		if err = sp.validateSignature(artifactEl); err != nil {
-			return fmt.Errorf("cannot validate signature on Response: %v", err)
-		}
-		haveSignature = true
-	}
-
-	responseEl, err := findChild(artifactEl, "urn:oasis:names:tc:SAML:2.0:protocol", "Response")
-	if err != nil {
-		return err
-	}
-	if responseEl != nil {
-		err = sp.validateSigned(responseEl)
-		if err != nil && err.Error() != "either the Response or Assertion must be signed" {
-			return err
-		}
-		if err == nil {
-			haveSignature = true // guaranteed by validateSigned
-		}
-	}
-
-	if !haveSignature {
-		return errors.New("either the ArtifactResponse, Response or Assertion must be signed")
-	}
-	return nil
-}
-
-// validateSigned returns a nil error iff each of the signatures on the Response and Assertion elements
-// are valid and there is at least one signature.
-func (sp *ServiceProvider) validateSigned(responseEl *etree.Element) error {
-	haveSignature := false
-
-	// Some SAML responses have the signature on the Response object, and some on the Assertion
-	// object, and some on both. We will require that at least one signature be present and that
-	// all signatures be valid
-	sigEl, err := findChild(responseEl, "http://www.w3.org/2000/09/xmldsig#", "Signature")
-	if err != nil {
-		return err
-	}
-	if sigEl != nil {
-		if err = sp.validateSignature(responseEl); err != nil {
-			return fmt.Errorf("cannot validate signature on Response: %v", err)
-		}
-		haveSignature = true
-	}
-
-	assertionEl, err := findChild(responseEl, "urn:oasis:names:tc:SAML:2.0:assertion", "Assertion")
+// validateSignature returns nil iff the Signature embedded in the element is valid
+func (sp *ServiceProvider) validateSignature(el *etree.Element) error {
+	sigEl, err := findChild(el, "http://www.w3.org/2000/09/xmldsig#", "Signature")
 	if err != nil {
 		return err
 	}
-	if assertionEl != nil {
-		sigEl, err := findChild(assertionEl, "http://www.w3.org/2000/09/xmldsig#", "Signature")
-		if err != nil {
-			return err
-		}
-		if sigEl != nil {
-			if err = sp.validateSignature(assertionEl); err != nil {
-				return fmt.Errorf("cannot validate signature on Response: %v", err)
-			}
-			haveSignature = true
-		}
-	}
-
-	if !haveSignature {
-		return errors.New("either the Response or Assertion must be signed")
+	if sigEl == nil {
+		return errSignatureElementNotPresent
 	}
-	return nil
-}
 
-// validateSignature returns nill iff the Signature embedded in the element is valid
-func (sp *ServiceProvider) validateSignature(el *etree.Element) error {
 	certs, err := sp.getIDPSigningCerts()
 	if err != nil {
-		return err
+		return fmt.Errorf("cannot validate signature on %s: %v", el.Tag, err)
 	}
 
 	certificateStore := dsig.MemoryX509CertificateStore{
@@ -1154,23 +1115,26 @@ func (sp *ServiceProvider) validateSignature(el *etree.Element) error {
 
 	ctx, err := etreeutils.NSBuildParentContext(el)
 	if err != nil {
-		return err
+		return fmt.Errorf("cannot validate signature on %s: %v", el.Tag, err)
 	}
 	ctx, err = ctx.SubContext(el)
 	if err != nil {
-		return err
+		return fmt.Errorf("cannot validate signature on %s: %v", el.Tag, err)
 	}
 	el, err = etreeutils.NSDetatch(ctx, el)
 	if err != nil {
-		return err
+		return fmt.Errorf("cannot validate signature on %s: %v", el.Tag, err)
 	}
 
 	if sp.SignatureVerifier != nil {
 		return sp.SignatureVerifier.VerifySignature(validationContext, el)
 	}
 
-	_, err = validationContext.Validate(el)
-	return err
+	if _, err := validationContext.Validate(el); err != nil {
+		return fmt.Errorf("cannot validate signature on %s: %v", el.Tag, err)
+	}
+
+	return nil
 }
 
 // SignLogoutRequest adds the `Signature` element to the `LogoutRequest`.
@@ -1497,73 +1461,90 @@ func (sp *ServiceProvider) ValidateLogoutResponseRequest(req *http.Request) erro
 
 // ValidateLogoutResponseForm returns a nil error if the logout response is valid.
 func (sp *ServiceProvider) ValidateLogoutResponseForm(postFormData string) error {
+	retErr := &InvalidResponseError{
+		Now: TimeNow(),
+	}
+
 	rawResponseBuf, err := base64.StdEncoding.DecodeString(postFormData)
 	if err != nil {
-		return fmt.Errorf("unable to parse base64: %s", err)
+		retErr.PrivateErr = fmt.Errorf("unable to parse base64: %s", err)
+		return retErr
 	}
+	retErr.Response = string(rawResponseBuf)
 
 	// 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)
+	doc := etree.NewDocument()
+	if err := doc.ReadFromBytes(rawResponseBuf); err != nil {
+		retErr.PrivateErr = err
+		return retErr
 	}
 
-	if err := sp.validateLogoutResponse(&resp); err != nil {
-		return err
+	if err := sp.validateSignature(doc.Root()); err != nil {
+		retErr.PrivateErr = err
+		return retErr
 	}
 
-	doc := etree.NewDocument()
-	if err := doc.ReadFromBytes(rawResponseBuf); err != nil {
+	var resp LogoutResponse
+	if err := unmarshalElement(doc.Root(), &resp); err != nil {
+		retErr.PrivateErr = err
+		return retErr
+	}
+	if err := sp.validateLogoutResponse(&resp); err != nil {
 		return err
 	}
-
-	responseEl := doc.Root()
-	return sp.validateSigned(responseEl)
+	return nil
 }
 
 // ValidateLogoutResponseRedirect returns a nil error if the logout response is valid.
 //
 // URL Binding appears to be gzip / flate encoded
 // See https://www.oasis-open.org/committees/download.php/20645/sstc-saml-tech-overview-2%200-draft-10.pdf  6.6
 func (sp *ServiceProvider) ValidateLogoutResponseRedirect(queryParameterData string) error {
+	retErr := &InvalidResponseError{
+		Now: TimeNow(),
+	}
+
 	rawResponseBuf, err := base64.StdEncoding.DecodeString(queryParameterData)
 	if err != nil {
-		return fmt.Errorf("unable to parse base64: %s", err)
+		retErr.PrivateErr = fmt.Errorf("unable to parse base64: %s", err)
+		return retErr
 	}
+	retErr.Response = string(rawResponseBuf)
 
 	gr, err := ioutil.ReadAll(flate.NewReader(bytes.NewBuffer(rawResponseBuf)))
 	if err != nil {
-		return err
+		retErr.PrivateErr = err
+		return retErr
 	}
 
 	if err := xrv.Validate(bytes.NewReader(gr)); err != nil {
 		return err
 	}
 
-	decoder := xml.NewDecoder(bytes.NewReader(gr))
-
-	var resp LogoutResponse
-
-	err = decoder.Decode(&resp)
-	if err != nil {
-		return fmt.Errorf("unable to flate decode: %s", err)
+	doc := etree.NewDocument()
+	if err := doc.ReadFromBytes(rawResponseBuf); err != nil {
+		retErr.PrivateErr = err
+		return retErr
 	}
 
-	if err := sp.validateLogoutResponse(&resp); err != nil {
-		return err
+	if err := sp.validateSignature(doc.Root()); err != nil {
+		retErr.PrivateErr = err
+		return retErr
 	}
 
-	doc := etree.NewDocument()
-	if _, err := doc.ReadFrom(bytes.NewReader(gr)); err != nil {
+	var resp LogoutResponse
+	if err := unmarshalElement(doc.Root(), &resp); err != nil {
+		retErr.PrivateErr = err
+		return retErr
+	}
+	if err := sp.validateLogoutResponse(&resp); err != nil {
 		return err
 	}
-
-	responseEl := doc.Root()
-	return sp.validateSigned(responseEl)
+	return nil
 }
 
 // validateLogoutResponse validates the LogoutResponse fields. Returns a nil error if the LogoutResponse is valid.
@@ -1592,3 +1573,101 @@ func firstSet(a, b string) string {
 	}
 	return a
 }
+
+// findChildren returns all the elements matching childNS/childTag that are direct children of parentEl.
+func findChildren(parentEl *etree.Element, childNS string, childTag string) ([]*etree.Element, error) {
+	var rv []*etree.Element
+	for _, childEl := range parentEl.ChildElements() {
+		if childEl.Tag != childTag {
+			continue
+		}
+
+		ctx, err := etreeutils.NSBuildParentContext(childEl)
+		if err != nil {
+			return nil, err
+		}
+		ctx, err = ctx.SubContext(childEl)
+		if err != nil {
+			return nil, err
+		}
+
+		ns, err := ctx.LookupPrefix(childEl.Space)
+		if err != nil {
+			return nil, fmt.Errorf("[%s]:%s cannot find prefix %s: %v", childNS, childTag, childEl.Space, err)
+		}
+		if ns != childNS {
+			continue
+		}
+
+		rv = append(rv, childEl)
+	}
+
+	return rv, nil
+}
+
+// findOneChild finds the specified child element. Returns an error if the element doesn't exist.
+func findOneChild(parentEl *etree.Element, childNS string, childTag string) (*etree.Element, error) {
+	children, err := findChildren(parentEl, childNS, childTag)
+	if err != nil {
+		return nil, err
+	}
+	switch len(children) {
+	case 0:
+		return nil, fmt.Errorf("cannot find %s:%s element", childNS, childTag)
+	case 1:
+		return children[0], nil
+	default:
+		return nil, fmt.Errorf("expected exactly one %s:%s element", childNS, childTag)
+	}
+}
+
+// findChild finds the specified child element. Returns (nil, nil) of the element doesn't exist.
+func findChild(parentEl *etree.Element, childNS string, childTag string) (*etree.Element, error) {
+	children, err := findChildren(parentEl, childNS, childTag)
+	if err != nil {
+		return nil, err
+	}
+	switch len(children) {
+	case 0:
+		return nil, nil
+	case 1:
+		return children[0], nil
+	default:
+		return nil, fmt.Errorf("expected at most one %s:%s element", childNS, childTag)
+	}
+}
+
+func elementToBytes(el *etree.Element) ([]byte, error) {
+	namespaces := map[string]string{}
+	for _, childEl := range el.FindElements("//*") {
+		ns := childEl.NamespaceURI()
+		if ns != "" {
+			namespaces[childEl.Space] = ns
+		}
+	}
+
+	doc := etree.NewDocument()
+	doc.SetRoot(el.Copy())
+	for space, uri := range namespaces {
+		doc.Root().CreateAttr("xmlns:"+space, uri)
+	}
+
+	return doc.WriteToBytes()
+}
+
+// unmarshalElement serializes el into v by serializing el and then parsing it with xml.Unmarshal.
+func unmarshalElement(el *etree.Element, v interface{}) error {
+	buf, err := elementToBytes(el)
+	if err != nil {
+		return err
+	}
+	return xml.Unmarshal(buf, v)
+}
+
+func elementToString(el *etree.Element) string {
+	buf, err := elementToBytes(el)
+	if err != nil {
+		return ""
+	}
+	return string(buf)
+}

@@ -125,12 +125,12 @@ func TestSPInvalidResponses(t *testing.T) {
 	req.PostForm.Set("SAMLResponse", base64.StdEncoding.EncodeToString(test.SamlResponse))
 	_, err = s.ParseResponse(&req, []string{"id-9e61753d64e928af5a7a341a97f420c9"})
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: cannot parse certificate: illegal base64 data at input byte 4"))
+		"cannot validate signature on Assertion: cannot parse certificate: illegal base64 data at input byte 4"))
 
 	s.IDPMetadata.IDPSSODescriptors[0].KeyDescriptors[0].KeyInfo.X509Data.X509Certificates[0].Data = "aW52YWxpZA=="
 	req.PostForm.Set("SAMLResponse", base64.StdEncoding.EncodeToString(test.SamlResponse))
 	_, err = s.ParseResponse(&req, []string{"id-9e61753d64e928af5a7a341a97f420c9"})
 
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: x509: malformed certificate"))
+		"cannot validate signature on Assertion: x509: malformed certificate"))
 }

@@ -985,7 +985,7 @@ func TestServiceProviderMismatchedDestinationsWithSignaturePresent(t *testing.T)
 
 	req := http.Request{PostForm: url.Values{}}
 	s.AcsURL = mustParseURL("https://wrong/saml2/acs")
-	bytes, _ := addSignatureToDocument(test.responseDom()).WriteToBytes()
+	bytes, _ := test.responseDom().WriteToBytes()
 	req.PostForm.Set("SAMLResponse", base64.StdEncoding.EncodeToString(bytes))
 	_, err = s.ParseResponse(&req, []string{"id-9e61753d64e928af5a7a341a97f420c9"})
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
@@ -1087,11 +1087,20 @@ func TestSPInvalidAssertions(t *testing.T) {
 	err := xml.Unmarshal(test.IDPMetadata, &s.IDPMetadata)
 	assert.Check(t, err)
 
-	req := http.Request{PostForm: url.Values{}}
-	req.PostForm.Set("SAMLResponse", base64.StdEncoding.EncodeToString(test.SamlResponse))
-	s.IDPMetadata.IDPSSODescriptors[0].KeyDescriptors[0].KeyInfo.X509Data.X509Certificates[0].Data = "invalid"
-	_, err = s.ParseResponse(&req, []string{"id-9e61753d64e928af5a7a341a97f420c9"})
-	assertionBuf := []byte(err.(*InvalidResponseError).Response)
+	// HACK: decrypt response without verifying assertions
+	var assertionBuf []byte
+	{
+		doc := etree.NewDocument()
+		assert.Check(t, doc.ReadFromBytes(test.SamlResponse))
+		encryptedEL := doc.Root().FindElement("//EncryptedAssertion")
+		assertionEl, err := s.decryptElement(encryptedEL)
+		assert.Check(t, err)
+
+		doc = etree.NewDocument()
+		doc.SetRoot(assertionEl)
+		assertionBuf, err = doc.WriteToBytes()
+		assert.Check(t, err)
+	}
 
 	assertion := Assertion{}
 	err = xml.Unmarshal(assertionBuf, &assertion)
@@ -1234,9 +1243,13 @@ func TestXswPermutationThreeIsRejected(t *testing.T) {
 	req := http.Request{PostForm: url.Values{}}
 	req.PostForm.Set("SAMLResponse", string(respStr))
 	_, err = s.ParseResponse(&req, []string{"ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"})
-	// Because this permutation contains an unsigned assertion as child of the response
-	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"either the Response or Assertion must be signed"))
+
+	// This response contains two assertions. The first is missing a Signature element. The second is
+	// signed by a certificate that is not yet valid at the time of issue.
+	//
+	// When no assertions are valid, we return the first error encountered, which in this case is that
+	// there is no Signature on the element.
+	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr, "Signature element not present"))
 }
 
 func TestXswPermutationFourIsRejected(t *testing.T) {
@@ -1262,9 +1275,11 @@ func TestXswPermutationFourIsRejected(t *testing.T) {
 	req := http.Request{PostForm: url.Values{}}
 	req.PostForm.Set("SAMLResponse", string(respStr))
 	_, err = s.ParseResponse(&req, []string{"ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"})
-	// Because this permutation contains an unsigned assertion as child of the response
-	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"either the Response or Assertion must be signed"))
+
+	// This permutation contains a signed assertion embedded within an unsigned assertion.
+	// I'm pretty sure this is just not allowed, so we properly decide that there are no
+	// signed assertions at all.
+	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr, "Signature element not present"))
 }
 
 func TestXswPermutationFiveIsRejected(t *testing.T) {
@@ -1291,7 +1306,7 @@ func TestXswPermutationFiveIsRejected(t *testing.T) {
 	req.PostForm.Set("SAMLResponse", string(respStr))
 	_, err = s.ParseResponse(&req, []string{"ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"})
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: Missing signature referencing the top-level element"))
+		"cannot validate signature on Assertion: Missing signature referencing the top-level element"))
 }
 
 func TestXswPermutationSixIsRejected(t *testing.T) {
@@ -1318,7 +1333,7 @@ func TestXswPermutationSixIsRejected(t *testing.T) {
 	req.PostForm.Set("SAMLResponse", string(respStr))
 	_, err = s.ParseResponse(&req, []string{"ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"})
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: Missing signature referencing the top-level element"))
+		"cannot validate signature on Assertion: Missing signature referencing the top-level element"))
 }
 
 func TestXswPermutationSevenIsRejected(t *testing.T) {
@@ -1349,7 +1364,7 @@ func TestXswPermutationSevenIsRejected(t *testing.T) {
 	_, err = s.ParseResponse(&req, []string{"ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"})
 	//It's the assertion signature that can't be verified. The error message is generic and always mentions Response
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: Signature could not be verified"))
+		"cannot validate signature on Assertion: Signature could not be verified"))
 }
 
 func TestXswPermutationEightIsRejected(t *testing.T) {
@@ -1380,7 +1395,7 @@ func TestXswPermutationEightIsRejected(t *testing.T) {
 	_, err = s.ParseResponse(&req, []string{"ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"})
 	//It's the assertion signature that can't be verified. The error message is generic and always mentions Response
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: Signature could not be verified"))
+		"cannot validate signature on Assertion: Signature could not be verified"))
 }
 
 func TestXswPermutationNineIsRejected(t *testing.T) {
@@ -1411,7 +1426,7 @@ func TestXswPermutationNineIsRejected(t *testing.T) {
 	_, err = s.ParseResponse(&req, []string{"ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"})
 	//It's the assertion signature that can't be verified. The error message is generic and always mentions Response
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: Missing signature referencing the top-level element"))
+		"cannot validate signature on Assertion: Missing signature referencing the top-level element"))
 }
 
 func TestSPRealWorldKeyInfoHasRSAPublicKeyNotX509Cert(t *testing.T) {
@@ -1749,7 +1764,7 @@ func TestParseBadXMLArtifactResponse(t *testing.T) {
 
 	assertion, err = sp.ParseXMLArtifactResponse(samlResponse, possibleReqIDs, reqID)
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"cannot validate signature on Response: Cert is not valid at this time"))
+		"cannot validate signature on ArtifactResponse: Cert is not valid at this time"))
 	assert.Check(t, is.Nil(assertion))
 	Clock = dsig.NewFakeClockAt(TimeNow())
 
@@ -1769,6 +1784,32 @@ func TestParseBadXMLArtifactResponse(t *testing.T) {
 	sp.Key = mustParsePrivateKey(golden.Get(t, "key_2017.pem")).(*rsa.PrivateKey)
 	assertion, err = sp.ParseXMLArtifactResponse(samlResponse, possibleReqIDs, reqID)
 	assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr,
-		"failed to decrypt response: certificate does not match provided key"))
+		"failed to decrypt EncryptedAssertion: certificate does not match provided key"))
 	assert.Check(t, is.Nil(assertion))
 }
+
+func TestMultipleAssertions(t *testing.T) {
+	idpMetadata := golden.Get(t, "TestSPRealWorldKeyInfoHasRSAPublicKeyNotX509Cert_idp_metadata")
+	respStr := golden.Get(t, "TestSPMultipleAssertions")
+	TimeNow = func() time.Time {
+		rv, _ := time.Parse("Mon Jan 2 15:04:05 MST 2006", "Fri Apr 21 13:12:51 UTC 2017")
+		return rv
+	}
+	Clock = dsig.NewFakeClockAt(TimeNow())
+	s := ServiceProvider{
+		Key:         mustParsePrivateKey(golden.Get(t, "key_2017.pem")).(*rsa.PrivateKey),
+		Certificate: mustParseCertificate(golden.Get(t, "cert_2017.pem")),
+		MetadataURL: mustParseURL("https://preview.docrocket-ross.test.octolabs.io/saml/metadata"),
+		AcsURL:      mustParseURL("https://preview.docrocket-ross.test.octolabs.io/saml/acs"),
+		IDPMetadata: &EntityDescriptor{},
+	}
+	err := xml.Unmarshal(idpMetadata, &s.IDPMetadata)
+	assert.Check(t, err)
+
+	req := http.Request{PostForm: url.Values{}}
+	req.PostForm.Set("SAMLResponse", base64.StdEncoding.EncodeToString(respStr))
+	profile, err := s.ParseResponse(&req, []string{"id-3992f74e652d89c3cf1efd6c7e472abaac9bc917"})
+
+	assert.Check(t, err)
+	assert.Check(t, profile.Subject.NameID.Value != "admin@evil.com")
+}

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?><saml2p:Response xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol" Destination="https://preview.docrocket-ross.test.octolabs.io/saml/acs" ID="28338c8c-39ab-4b94-bcdc-46f68f99d962" InResponseTo="id-3992f74e652d89c3cf1efd6c7e472abaac9bc917" IssueInstant="2017-04-21T13:12:50.830Z" Version="2.0"><saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">https://idp.secureworks.com/SAML2</saml2:Issuer>
+<saml2p:Status><saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/><saml2p:StatusMessage>Authentication success.</saml2p:StatusMessage></saml2p:Status><saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" ID="e5afbcaa-be69-4b41-ac48-2f23538accdb" IssueInstant="2017-04-21T13:12:50.830Z" Version="2.0"><saml2:Issuer>https://idp.secureworks.com/SAML2</saml2:Issuer><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/><ds:Reference URI="#e5afbcaa-be69-4b41-ac48-2f23538accdb"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ds:DigestValue>BMN0lUblP0gYGcw2PCyhwFZzkxY=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>F/2aaOQ3J/S6ULUd+gAuIclVueHEC2UfmtO2eR2oYb/YXub9E22yZe7eQgj2wdhYOvacVXN28QJJJG+K3Njwvi6b7mqf+T8N1YwaJW1fYAm28ayg4dEOTjHnjbRMZ6L+3cZPmPcFyE+edhCHEMnTLSqSvBnSyc1cwGdO9PmfWmt6PzUwf2nr2P5577Yc1FEQ9OtTx7ugWN3iPmjtLeTcpZfIDQX9+gSsh0KT+t61uWaYz+PJhtKnZQFeyr3uIxBTxv4wQ90FnmE4PiDvMksin5CDMfiMwd7pn7rNbk4EVHiDgSMkY6P4h8eWQwiqglOrQSZZr4BJgCoUbcNfZCq/7A==</ds:SignatureValue><ds:KeyInfo><ds:KeyValue><ds:RSAKeyValue><ds:Modulus>zZlTNJ+QcTp2yGH1ECXO3ry4GHhcs1CW3I6GPiPvtO+P6lyWxYdQd2RK/Hk9Kap6qpm/qom0rTwb
+FU2I67Y2JdQ3T5QBJjGHbGHU1uMxVWkhJluoa0Lpm381zNCJTZp8PetoB8dnIGua9y1aL75v04CG
+TzJ14I9/sW+apTkWj7xVQXutvVKETdn4kAy+L33HpriZjQNlcuAbqQj6OWsN4tGkLvNFZT40jQzp
+/8/tOQE6n2+zn3I8hUePwjPQROUmCeK86CkF0yVCPQ/vOTsC00Uaeu/SPOUu5ot+/75NPyE8w5Ry
+DgefdDXhYNmeuQtwGtcu/FI66atQMNTDoChXJQ==</ds:Modulus><ds:Exponent>AQAB</ds:Exponent></ds:RSAKeyValue></ds:KeyValue></ds:KeyInfo></ds:Signature><saml2:Subject><saml2:NameID>rkinder@secureworks.com</saml2:NameID><saml2:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml2:SubjectConfirmationData InResponseTo="id-3992f74e652d89c3cf1efd6c7e472abaac9bc917" NotBefore="2017-04-21T13:12:50.830Z" NotOnOrAfter="2017-04-21T13:17:50.830Z" Recipient="https://preview.docrocket-ross.test.octolabs.io/saml/acs"/></saml2:SubjectConfirmation></saml2:Subject><saml2:Conditions NotBefore="2017-04-21T13:12:50.830Z" NotOnOrAfter="2017-04-21T13:17:50.830Z"><saml2:AudienceRestriction><saml2:Audience>https://preview.docrocket-ross.test.octolabs.io/saml/metadata</saml2:Audience></saml2:AudienceRestriction></saml2:Conditions><saml2:AuthnStatement AuthnInstant="2017-04-21T13:12:50.830Z" SessionIndex="undefined"><saml2:AuthnContext><saml2:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml2:AuthnContextClassRef></saml2:AuthnContext></saml2:AuthnStatement></saml2:Assertion>
+<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" ID="e5afbcaa-be69-4b41-ac48-2f23538accdb" IssueInstant="2017-04-21T13:12:50.830Z" Version="2.0"><saml2:Issuer>https://idp.secureworks.com/SAML2</saml2:Issuer>
+<saml2:Subject><saml2:NameID>admin@evil.com</saml2:NameID><saml2:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml2:SubjectConfirmationData InResponseTo="id-3992f74e652d89c3cf1efd6c7e472abaac9bc917" NotBefore="2017-04-21T13:12:50.830Z" NotOnOrAfter="2017-04-21T13:17:50.830Z" Recipient="https://preview.docrocket-ross.test.octolabs.io/saml/acs"/></saml2:SubjectConfirmation></saml2:Subject><saml2:Conditions NotBefore="2017-04-21T13:12:50.830Z" NotOnOrAfter="2017-04-21T13:17:50.830Z"><saml2:AudienceRestriction><saml2:Audience>https://preview.docrocket-ross.test.octolabs.io/saml/metadata</saml2:Audience></saml2:AudienceRestriction></saml2:Conditions><saml2:AuthnStatement AuthnInstant="2017-04-21T13:12:50.830Z" SessionIndex="undefined"><saml2:AuthnContext><saml2:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml2:AuthnContextClassRef></saml2:AuthnContext></saml2:AuthnStatement></saml2:Assertion></saml2p:Response>