docs: Update docs to be more explicit about sig validation changes for SAML responses.

saml/README.md

@@ -45,3 +45,16 @@ the requirements of the interoperable SAML
 
 You can find the full demo code in the [`saml/demo`](./saml/demo/main.go)
 package.
+
+## ParseResponse signature validation policy
+
+`ParseResponse` always validates any signatures that are present, but the
+required signature policy is controlled by options:
+
+- `ValidateResponseSignature()`: requires a valid signature on the top-level
+    SAML `Response`.
+- `ValidateAssertionSignature()`: if the response is signed and valid,
+    assertions are accepted as covered by that signature; if the response is not
+    signed, each assertion must be signed and valid.
+- Using both options requires a valid response signature and validates any
+    assertion signatures that are present.

saml/go.mod

@@ -1,16 +1,16 @@
 module github.com/hashicorp/cap/saml
 
-go 1.23.0
+go 1.25.0
 
 require (
-	github.com/beevik/etree v1.2.0
+	github.com/beevik/etree v1.6.0
 	github.com/crewjam/go-xmlsec v0.0.0-20200414151428-d2b1a58f7262
 	github.com/crewjam/saml v0.4.14
 	github.com/hashicorp/go-uuid v1.0.3
-	github.com/jonboulle/clockwork v0.4.0
-	github.com/russellhaering/gosaml2 v0.9.1
-	github.com/russellhaering/goxmldsig v1.4.0
-	github.com/stretchr/testify v1.8.4
+	github.com/jonboulle/clockwork v0.5.0
+	github.com/russellhaering/gosaml2 v0.11.0
+	github.com/russellhaering/goxmldsig v1.6.0
+	github.com/stretchr/testify v1.11.1
 )
 
 require (

saml/go.sum

@@ -1,7 +1,5 @@
-github.com/beevik/etree v1.1.0/go.mod h1:r8Aw8JqVegEf0w2fDnATrX9VpkMcyFeM0FhwO62wh+A=
-github.com/beevik/etree v1.2.0 h1:l7WETslUG/T+xOPs47dtd6jov2Ii/8/OjCldk5fYfQw=
-github.com/beevik/etree v1.2.0/go.mod h1:aiPf89g/1k3AShMVAzriilpcE4R/Vuor90y83zVZWFc=
-github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/beevik/etree v1.6.0 h1:u8Kwy8pp9D9XeITj2Z0XtA5qqZEmtJtuXZRQi+j03eE=
+github.com/beevik/etree v1.6.0/go.mod h1:bh4zJxiIr62SOf9pRzN7UUYaEDa9HEKafK25+sLc0Gc=
 github.com/crewjam/errset v0.0.0-20160219153700-f78d65de925c h1:dCJ9oZ0VgnzJHR5BjkSrwkXA1USu483qlxBd0u29P8s=
 github.com/crewjam/errset v0.0.0-20160219153700-f78d65de925c/go.mod h1:XhiWL7J86xoqJ8+x2OA+AM2l9skQP2DZ0UOXQYVg7uI=
 github.com/crewjam/go-xmlsec v0.0.0-20200414151428-d2b1a58f7262 h1:3V8RSsB1mxeAfxMb7lGSd0HlCHhc/ElJj1peaJMAkyk=
@@ -17,50 +15,36 @@ github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8=
 github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
-github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=
-github.com/jonboulle/clockwork v0.3.0/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=
-github.com/jonboulle/clockwork v0.4.0 h1:p4Cf1aMWXnXAUh8lVfewRBx1zaTSYKrKMF2g3ST4RZ4=
-github.com/jonboulle/clockwork v0.4.0/go.mod h1:xgRqUGwRcjKCO1vbZUEtSLrqKoPSsUpK7fnezOII0kc=
-github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
-github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
-github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
+github.com/jonboulle/clockwork v0.5.0 h1:Hyh9A8u51kptdkR+cqRpT1EebBwTn1oK9YfGYbdFz6I=
+github.com/jonboulle/clockwork v0.5.0/go.mod h1:3mZlmanh0g2NDKO5TWZVJAfofYk64M7XN3SzBPjZF60=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
-github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
-github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/mattermost/xml-roundtrip-validator v0.1.0 h1:RXbVD2UAl7A7nOTR4u7E3ILa4IbtvKBHw64LDsmu9hU=
 github.com/mattermost/xml-roundtrip-validator v0.1.0/go.mod h1:qccnGMcpgwcNaBnxqpJpWWUiPNr5H3O8eDgGV9gT5To=
 github.com/moov-io/signedxml v1.1.1 h1:TQ2fK4DRCYv7agH+z6RjtnBTmEyYMAztFzuHIPtUJpg=
 github.com/moov-io/signedxml v1.1.1/go.mod h1:p+b4f/Wo/qKyew8fHW8VZOgsILWylyvvjdE68egzbwc=
-github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
 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/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
-github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE=
 github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8=
 github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
-github.com/russellhaering/gosaml2 v0.9.1 h1:H/whrl8NuSoxyW46Ww5lKPskm+5K+qYLw9afqJ/Zef0=
-github.com/russellhaering/gosaml2 v0.9.1/go.mod h1:ja+qgbayxm+0mxBRLMSUuX3COqy+sb0RRhIGun/W2kc=
-github.com/russellhaering/goxmldsig v1.3.0/go.mod h1:gM4MDENBQf7M+V824SGfyIUVFWydB7n0KkEubVJl+Tw=
-github.com/russellhaering/goxmldsig v1.4.0 h1:8UcDh/xGyQiyrW+Fq5t8f+l2DLB1+zlhYzkPUJ7Qhys=
-github.com/russellhaering/goxmldsig v1.4.0/go.mod h1:gM4MDENBQf7M+V824SGfyIUVFWydB7n0KkEubVJl+Tw=
+github.com/russellhaering/gosaml2 v0.11.0 h1:wlWm7dWMrpJBzh0xEOZof70nVen4f/2BEF8ZXaidJ9o=
+github.com/russellhaering/gosaml2 v0.11.0/go.mod h1:GmL5LeCP7PBYzSkkFxtmHuRzC2eUZ/6JSLYQd5fzKK4=
+github.com/russellhaering/goxmldsig v1.6.0 h1:8fdWXEPh2k/NZNQBPFNoVfS3JmzS4ZprY/sAOpKQLks=
+github.com/russellhaering/goxmldsig v1.6.0/go.mod h1:TrnaquDcYxWXfJrOjeMBTX4mLBeYAqaHEyUeWPxZlBM=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
-github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
+github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
+github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 golang.org/x/crypto v0.36.0 h1:AnAEvhDddvBdpY+uR+MyHmuZzzNqXSe/GvuDeob5L34=
 golang.org/x/crypto v0.36.0/go.mod h1:Y4J0ReaxCR1IMaabaSMugxJES1EpwhBHhv2bDHklZvc=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
-gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=

saml/response.go

@@ -9,6 +9,7 @@ import (
 	"encoding/pem"
 	"fmt"
 	"regexp"
+	"slices"
 
 	"github.com/jonboulle/clockwork"
 	saml2 "github.com/russellhaering/gosaml2"
@@ -77,7 +78,8 @@ func InsecureSkipSignatureValidation() Option {
 	}
 }
 
-// ValidateResponseSignature enables signature validation to ensure the response is at least signed
+// ValidateResponseSignature requires a valid signature on the top-level SAML
+// response envelope.
 func ValidateResponseSignature() Option {
 	return func(o interface{}) {
 		if o, ok := o.(*parseResponseOptions); ok {
@@ -86,7 +88,14 @@ func ValidateResponseSignature() Option {
 	}
 }
 
-// ValidateAssertionSignature enables signature validation to ensure the assertion is at least signed
+// ValidateAssertionSignature enables assertion-signature policy checks.
+//
+// Behavior:
+//   - If the response envelope is signed and valid, assertions are accepted as
+//     covered by that signature. Any assertion signatures that are present must
+//     still validate.
+//   - If the response envelope is not signed, each assertion must be signed and
+//     valid.
 func ValidateAssertionSignature() Option {
 	return func(o interface{}) {
 		if o, ok := o.(*parseResponseOptions); ok {
@@ -101,6 +110,8 @@ func ValidateAssertionSignature() Option {
 // - InsecureSkipRequestIDValidation
 // - InsecureSkipAssertionConditionValidation
 // - InsecureSkipSignatureValidation
+// - ValidateResponseSignature
+// - ValidateAssertionSignature
 // - WithAssertionConsumerServiceURL
 // - WithClock
 func (sp *ServiceProvider) ParseResponse(
@@ -282,20 +293,34 @@ func parsePEMCertificate(cert []byte) (*x509.Certificate, error) {
 }
 
 func validateSignature(response *core.Response, op string, opts parseResponseOptions) error {
-	// validate child object assertions
-	for _, assert := range response.Assertions() {
-		// note: at one time func ip.ValidateEncodedResponse(...) above allows all signed or all unsigned
-		// assertions, and will give error if there is a mix of both. We are still looping on all assertions
-		// instead of retrieving signature for one assertion, so we do not depend on dependency implementation.
-		if !assert.SignatureValidated && opts.validateAssertionSignature {
-			return fmt.Errorf("%s: %w", op, ErrInvalidSignature)
+	switch {
+	case response.SignatureValidated:
+		// When decoding a signed response, the internal decoder will automatically
+		// verify the validaty of both the response signature and any assertions
+		// that are signed. Any unsigned assertions will be handled by the response
+		// envelope signature. Only when all of this is true will the internal
+		// decoder set the SignatureValidated field be set to true.
+		return nil
+	case !response.SignatureValidated && opts.validateResponseSignature:
+		// Our response envelope is not signed but we require it
+		return fmt.Errorf("%s: %w", op, ErrInvalidSignature)
+	case !response.SignatureValidated && opts.validateAssertionSignature:
+		// Our response envelope is not signed but we require that each assertion
+		// is signed. The internal decoder will have set SignatureValidated on
+		// each assertion in this code path. We can use to determine whether or not
+		// it is valid.
+		for assert := range slices.Values(response.Assertions()) {
+			// note: at one time func ip.ValidateEncodedResponse(...) above allows all signed or all unsigned
+			// assertions, and will give error if there is a mix of both. We are still looping on all assertions
+			// instead of retrieving signature for one assertion, so we do not depend on dependency implementation.
+			if !assert.SignatureValidated {
+				return fmt.Errorf("%s: %w", op, ErrInvalidSignature)
+			}
 		}
-	}
 
-	// validate root object response
-	if !response.SignatureValidated && opts.validateResponseSignature {
+		return nil
+	default:
+		// This ought to be unreachable but we'll catch here.
 		return fmt.Errorf("%s: %w", op, ErrInvalidSignature)
 	}
-
-	return nil
 }

saml/response_test.go

@@ -100,21 +100,28 @@ func TestServiceProvider_ParseResponse(t *testing.T) {
 			opts:      []saml.Option{saml.ValidateAssertionSignature()},
 			requestID: testRequestId,
 		},
+		{
+			name:      "success - with both options enabled of validating both signatures & with only response signed with no assertions signed",
+			sp:        testSp,
+			samlResp:  base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustResponseSigned()))),
+			opts:      []saml.Option{saml.ValidateResponseSignature(), saml.ValidateAssertionSignature()},
+			requestID: testRequestId,
+		},
 		{
 			name:            "missing signature",
 			sp:              testSp,
 			samlResp:        base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t))),
 			opts:            []saml.Option{},
 			requestID:       testRequestId,
-			wantErrContains: "response and/or assertions must be signed",
+			wantErrContains: "Missing signature referencing the top-level element",
 		},
 		{
-			name:            "error-invalid-signature - with both options enabled of validating both signatures & with only response signed",
+			name:            "error-invalid-signature - with both options enabled of validating both signatures & with only response signed and wrap attacked assertion",
 			sp:              testSp,
-			samlResp:        base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustResponseSigned()))),
+			samlResp:        base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithWrapAttackedAssertion()))),
 			opts:            []saml.Option{saml.ValidateResponseSignature(), saml.ValidateAssertionSignature()},
 			requestID:       testRequestId,
-			wantErrContains: "invalid signature",
+			wantErrContains: "Signature could not be verified",
 		},
 		{
 			name:            "error-invalid-signature - with both options enabled of validating both signatures & with only assertion signed",
@@ -132,14 +139,6 @@ func TestServiceProvider_ParseResponse(t *testing.T) {
 			requestID:       testRequestId,
 			wantErrContains: "invalid signature",
 		},
-		{
-			name:            "error-invalid-signature - with option of validate assertion signature & with just response signed",
-			sp:              testSp,
-			samlResp:        base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustResponseSigned()))),
-			opts:            []saml.Option{saml.ValidateAssertionSignature()},
-			requestID:       testRequestId,
-			wantErrContains: "invalid signature",
-		},
 		{
 			name:     "err-assertion-missing-attribute-stmt",
 			sp:       testSp,
@@ -498,7 +497,7 @@ NBp9UZome44qZAYH1iqrpmmjsfI9pJItsgWu3kXPjhSfj1AJGR1l9JGvJrHki1iHTA==</ds:X509Cer
 			</saml2:Subject>
 			<saml2:Conditions NotBefore="2023-08-25T14:32:53.680Z" NotOnOrAfter="2023-08-25T14:37:53.680Z">
 				<saml2:AudienceRestriction>
-					<! -- audience is intentionally invalid -->. 
+					<! -- audience is intentionally invalid -->.
 					<saml2:Audience>http://saml.julz/invalid-audience</saml2:Audience>
 				</saml2:AudienceRestriction>
 			</saml2:Conditions>

saml/test/provider.go

@@ -431,9 +431,10 @@ func (p *TestProvider) parseRequestPost(request string) *core.AuthnRequest {
 }
 
 type responseOptions struct {
-	signResponseElem  bool
-	signAssertionElem bool
-	expired           bool
+	signResponseElem        bool
+	signAssertionElem       bool
+	wrapAttackAssertionElem bool
+	expired                 bool
 }
 
 type ResponseOption func(*responseOptions)
@@ -458,6 +459,13 @@ func WithResponseAndAssertionSigned() ResponseOption {
 	}
 }
 
+func WithWrapAttackedAssertion() ResponseOption {
+	return func(o *responseOptions) {
+		o.signResponseElem = true
+		o.wrapAttackAssertionElem = true
+	}
+}
+
 func WithJustAssertionSigned() ResponseOption {
 	return func(o *responseOptions) {
 		o.signAssertionElem = true
@@ -584,6 +592,27 @@ func (p *TestProvider) SamlResponse(t *testing.T, opts ...ResponseOption) string
 		}
 	}
 
+	if opt.wrapAttackAssertionElem {
+		signCtx := dsig.NewDefaultSigningContext(p.keystore)
+		wrapCtx := dsig.NewDefaultSigningContext(dsig.RandomKeyStoreForTest())
+
+		// Sign the signature with a mismatched context but wrap it with the correct
+		// signature.
+		responseEl := doc.SelectElement("Response")
+		for _, assert := range responseEl.FindElements("Assertion") {
+			signedAssert, err := wrapCtx.SignEnveloped(assert)
+			r.NoError(err)
+
+			// replace signed assert object
+			responseEl.RemoveChildAt(assert.Index())
+			responseEl.AddChild(signedAssert)
+		}
+
+		signed, err := signCtx.SignEnveloped(doc.Root())
+		r.NoError(err)
+		doc.SetRoot(signed)
+	}
+
 	result, err := doc.WriteToString()
 	r.NoError(err)