diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 8ef434b6..9dc4bb44 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -341,22 +341,30 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str } // Root element is allowed to not be signed if the Assertion element is. - rootElementSigned := true + var rawSignedAssertion []byte + var rawSignedResponse []byte + if p.validator != nil { - rawResp, rootElementSigned, err = verifyResponseSig(p.validator, rawResp) + // if raw assertion must be + rawSignedAssertion, rawSignedResponse, err = verifyResponseSig(p.validator, rawResp) if err != nil { return ident, fmt.Errorf("verify signature: %v", err) } - } + } else { + // no validator? I'm rejecting it for now, because it's unsafe. - var resp response - if err := xml.Unmarshal(rawResp, &resp); err != nil { - return ident, fmt.Errorf("unmarshal response: %v", err) } // If the root element isn't signed, there's no reason to inspect these // elements. They're not verified. - if rootElementSigned { + + // we got a signed response, let's carry about business logic checks on it + if rawSignedResponse != nil { + var resp response + if err := xml.Unmarshal(rawSignedResponse, &resp); err != nil { + return ident, fmt.Errorf("unmarshal response: %v", err) + } + if p.ssoIssuer != "" && resp.Issuer != nil && resp.Issuer.Issuer != p.ssoIssuer { return ident, fmt.Errorf("expected Issuer value %s, got %s", p.ssoIssuer, resp.Issuer.Issuer) } @@ -382,11 +390,17 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str } } - assertion := resp.Assertion - if assertion == nil { + if rawSignedAssertion == nil { return ident, fmt.Errorf("response did not contain an assertion") } + // assertion, use the rawSignedAssertion + + var assertion assertion + if err := xml.Unmarshal(rawSignedAssertion, &assertion); err != nil { + return ident, fmt.Errorf("unmarshal response: %v", err) + } + // Subject is usually optional, but we need it for the user ID, so complain // if it's not present. subject := assertion.Subject @@ -646,44 +660,68 @@ func (p *provider) validateConditions(conditions *conditions) error { // verifyResponseSig attempts to verify the signature of a SAML response or // the assertion. +// Using the signed contents from the signature, it attempts to obtain both an assertion and if the response is signed +// the response // -// If the root element is properly signed, this method returns it. +// If the root response is signed: returns the first assertion, along with the root response // // The SAML spec requires supporting responses where the root element is -// unverified, but the sub elements are signed. In these cases, -// this method returns rootVerified=false to indicate that the -// elements should be trusted, but all other elements MUST be ignored. +// unverified, but the sub elements are signed. +// In these cases, the method returns the assertion, however, the signedResponse will be nil // // Note: we still don't support multiple tags. If there are // multiple present this code will only process the first. -func verifyResponseSig(validator *dsig.ValidationContext, data []byte) (signed []byte, rootVerified bool, err error) { +func verifyResponseSig(validator *dsig.ValidationContext, data []byte) (signedAssertion []byte, signedResponse []byte, err error) { doc := etree.NewDocument() if err = doc.ReadFromBytes(data); err != nil { - return nil, false, fmt.Errorf("parse document: %v", err) + return nil, nil, fmt.Errorf("parse document: %v", err) } response := doc.Root() if response == nil { - return nil, false, fmt.Errorf("parse document: empty root") + return nil, nil, fmt.Errorf("parse document: empty root") } + // transformedResponse is signed, show return value is parsed solely from transformedResponse transformedResponse, err := validator.Validate(response) if err == nil { - // Root element is verified, return it. - doc.SetRoot(transformedResponse) - signed, err = doc.WriteToBytes() - return signed, true, err + // signedResponse is the serialization of ONLY transformedResponse + isolatedDoc := etree.NewDocument() + isolatedDoc.SetRoot(transformedResponse) + signedResponse, err := isolatedDoc.WriteToBytes() + if err != nil || signedResponse == nil { + return nil, nil, fmt.Errorf("serialize response document: %v", err) + } + + // signedAssertionElement part of transformedResponse + signedAssertionElement, err := etreeutils.NSSelectOne(transformedResponse, "urn:oasis:names:tc:SAML:2.0:assertion", "Assertion") + if err != nil || signedAssertionElement == nil { + return nil, nil, fmt.Errorf("response does not contain an Assertion element") + } + + assertionIsolatedDoc := etree.NewDocument() + assertionIsolatedDoc.SetRoot(signedAssertionElement) + signedAssertion, err := assertionIsolatedDoc.WriteToBytes() + if err != nil || signedAssertion == nil { + return nil, nil, fmt.Errorf("serialize assertion node: %v", err) + } + return signedAssertion, signedResponse, nil } + // Case 2: Assertion signed + // Ensures xmlns are copied down to the assertion element when they are defined in the root // // TODO: Only select from child elements of the root. assertion, err := etreeutils.NSSelectOne(response, "urn:oasis:names:tc:SAML:2.0:assertion", "Assertion") if err != nil || assertion == nil { - return nil, false, fmt.Errorf("response does not contain an Assertion element") + return nil, nil, fmt.Errorf("response does not contain an Assertion element") } + + // transformedAssertion is signed + transformedAssertion, err := validator.Validate(assertion) if err != nil { - return nil, false, fmt.Errorf("response does not contain a valid signature element: %v", err) + return nil, nil, fmt.Errorf("response does not contain a valid signature element: %v", err) } // Verified an assertion but not the response. Can't trust any child elements, @@ -692,12 +730,13 @@ func verifyResponseSig(validator *dsig.ValidationContext, data []byte) (signed [ response.RemoveChild(el) } - // We still return the full element, even though it's unverified - // because the element is not a valid XML document on its own. - // It still requires the root element to define things like namespaces. - response.AddChild(transformedAssertion) - signed, err = doc.WriteToBytes() - return signed, false, err + newDoc := etree.NewDocument() + newDoc.SetRoot(transformedAssertion) + signedAssertion, err = newDoc.WriteToBytes() + if err != nil || signedAssertion == nil { + return nil, nil, fmt.Errorf("serialize signed assertion: %v", err) + } + return signedAssertion, nil, nil } // before determines if a given time is before the current time, with an