From d5504ab1c78a96c716db8e0d2be9e14449f3f9d2 Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:22:40 +0200 Subject: [PATCH 1/2] security: fix InResponseTo request-correlation bypass --- saml/response.go | 8 ++++++ saml/response_test.go | 60 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/saml/response.go b/saml/response.go index f42c9c09..27a9764c 100644 --- a/saml/response.go +++ b/saml/response.go @@ -181,6 +181,14 @@ func (sp *ServiceProvider) ParseResponse( // there isn't a subject, but we've left this here since it's a // required for our implementation as well. return nil, fmt.Errorf("%s: %w", op, ErrMissingSubject) + case assert.Subject.SubjectConfirmation != nil && + assert.Subject.SubjectConfirmation.SubjectConfirmationData != nil && + assert.Subject.SubjectConfirmation.SubjectConfirmationData.InResponseTo != requestID: + return nil, fmt.Errorf( + "SubjectConfirmationData.InResponseTo (%s) doesn't match the expected requestID (%s)", + assert.Subject.SubjectConfirmation.SubjectConfirmationData.InResponseTo, + requestID, + ) case assert.AttributeStatement == nil: return nil, fmt.Errorf("%s: %w", op, ErrMissingAttributeStmt) } diff --git a/saml/response_test.go b/saml/response_test.go index 2e2d84ec..fe654b5c 100644 --- a/saml/response_test.go +++ b/saml/response_test.go @@ -6,6 +6,7 @@ package saml_test import ( "encoding/base64" "fmt" + "strings" "testing" "time" @@ -824,3 +825,62 @@ NBp9UZome44qZAYH1iqrpmmjsfI9pJItsgWu3kXPjhSfj1AJGR1l9JGvJrHki1iHTA== ` + +// TestServiceProvider_ParseResponseAcceptsSignedAssertionWithTamperedUnsignedResponseInResponseTo +// verifies that SAML responses with stale assertions cannot correlate to fresh +// requests. +func TestServiceProvider_ParseResponseAcceptsSignedAssertionWithTamperedUnsignedResponseInResponseTo(t *testing.T) { + tp := testprovider.StartTestProvider(t) + defer tp.Close() + + const ( + entityID = "http://hashicorp-cap.test" + acsURL = "http://hashicorp-cap.test/saml/acs" + oldRequestID = "test-request-id" + freshRequestID = "fresh-request-id" + ) + + cfg, err := saml.NewConfig(entityID, acsURL, fmt.Sprintf("%s/saml/metadata", tp.ServerURL())) + require.NoError(t, err) + sp, err := saml.NewServiceProvider(cfg) + require.NoError(t, err) + + // The test provider emits an unsigned Response containing a signed + // Assertion. Both the Response and Assertion are initially bound to + // oldRequestID. + raw := tp.SamlResponse(t, testprovider.WithJustAssertionSigned()) + + // Mutate exactly the first InResponseTo occurrence: the unsigned Response + // envelope. The signed Assertion remains cryptographically bound to + // oldRequestID. + require.Equal(t, 2, strings.Count(raw, `InResponseTo="`+oldRequestID+`"`)) + tampered := strings.Replace(raw, `InResponseTo="`+oldRequestID+`"`, `InResponseTo="`+freshRequestID+`"`, 1) + require.Contains(t, tampered, `InResponseTo="`+oldRequestID+`"`) + require.Contains(t, tampered, `InResponseTo="`+freshRequestID+`"`) + + encoded := base64.StdEncoding.EncodeToString([]byte(tampered)) + + for _, tc := range []struct { + name string + opts []saml.Option + }{ + { + name: "default signature policy", + }, + { + name: "ValidateAssertionSignature", + opts: []saml.Option{saml.ValidateAssertionSignature()}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := sp.ParseResponse(encoded, freshRequestID, tc.opts...) + require.Error(t, err) + require.ErrorContains(t, err, "doesn't match the expected requestID") + }) + } + + t.Run("ValidateResponseSignature rejects unsigned envelope", func(t *testing.T) { + _, err := sp.ParseResponse(encoded, freshRequestID, saml.ValidateResponseSignature()) + require.Error(t, err) + }) +} From 235ebec35e8da027b23655e74ccd60dcd043cdd0 Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:36:35 +0200 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e7afc71..fe9be0f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Canonical reference for changes, improvements, and bugfixes for cap. ## Next +* saml: fix InResponseTo request-correlation bypass ([PR #192](https://github.com/hashicorp/cap/pull/192)) + ## 0.13.0 * feat (jwt): add optional callback to dynamically fetch key sets in validator ([PR #187](https://github.com/hashicorp/cap/pull/187))