From b07b16cf83c4171d16da4d85608cb827f183cd79 Mon Sep 17 00:00:00 2001 From: Ross Kinder Date: Sat, 14 Oct 2023 10:18:46 -0400 Subject: [PATCH] Merge pull request from GHSA-267v-3v32-g6q5 --- metadata.go | 107 ++++++++++++++++++ metadata_test.go | 8 ++ ...esUrlSchemeForProtocolBinding_metadata.xml | 1 + 3 files changed, 116 insertions(+) create mode 100644 testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml diff --git a/metadata.go b/metadata.go index fb39a7b0..006a9e67 100644 --- a/metadata.go +++ b/metadata.go @@ -2,6 +2,8 @@ package saml import ( "encoding/xml" + "fmt" + "net/url" "time" "github.com/beevik/etree" @@ -19,6 +21,9 @@ const HTTPArtifactBinding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact" // SOAPBinding is the official URN for the SOAP binding (transport) const SOAPBinding = "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" +// SOAPBindingV1 is the URN for the SOAP binding in SAML 1.0 +const SOAPBindingV1 = "urn:oasis:names:tc:SAML:1.0:bindings:SOAP-binding" + // EntitiesDescriptor represents the SAML object of the same name. // // See http://docs.oasis-open.org/security/saml/v2.0/saml-metadata-2.0-os.pdf §2.3.1 @@ -188,6 +193,76 @@ type Endpoint struct { ResponseLocation string `xml:"ResponseLocation,attr,omitempty"` } +func checkEndpointLocation(binding string, location string) (string, error) { + // Within the SAML standard, the complex type EndpointType describes a + // SAML protocol binding endpoint at which a SAML entity can be sent + // protocol messages. In particular, the location of an endpoint type is + // defined as follows in the Metadata for the OASIS Security Assertion + // Markup Language (SAML) V2.0 - 2.2.2 Complex Type EndpointType: + // + // Location [Required] A required URI attribute that specifies the + // location of the endpoint. The allowable syntax of this URI depends + // on the protocol binding. + switch binding { + case HTTPPostBinding, + HTTPRedirectBinding, + HTTPArtifactBinding, + SOAPBinding, + SOAPBindingV1: + locationURL, err := url.Parse(location) + if err != nil { + return "", fmt.Errorf("invalid url %q: %w", location, err) + } + switch locationURL.Scheme { + case "http", "https": + // ok + default: + return "", fmt.Errorf("invalid url scheme %q for binding %q", + locationURL.Scheme, binding) + } + default: + // We don't know what form location should take, but the protocol + // requires that we validate its syntax. + // + // In practice, lots of metadata contains random bindings, for example + // "urn:mace:shibboleth:1.0:profiles:AuthnRequest" from our own test suite. + // + // We can't fail, but we also can't allow a location parameter whose syntax we + // cannot verify. The least-bad course of action here is to set location to + // and empty string, and hope the caller doesn't care need it. + location = "" + } + + return location, nil +} + +// UnmarshalXML implements xml.Unmarshaler +func (m *Endpoint) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + type Alias Endpoint + aux := &struct { + *Alias + }{ + Alias: (*Alias)(m), + } + if err := d.DecodeElement(aux, &start); err != nil { + return err + } + + var err error + m.Location, err = checkEndpointLocation(m.Binding, m.Location) + if err != nil { + return err + } + if m.ResponseLocation != "" { + m.ResponseLocation, err = checkEndpointLocation(m.Binding, m.ResponseLocation) + if err != nil { + return err + } + } + + return nil +} + // IndexedEndpoint represents the SAML IndexedEndpointType object. // // See http://docs.oasis-open.org/security/saml/v2.0/saml-metadata-2.0-os.pdf §2.2.3 @@ -199,6 +274,38 @@ type IndexedEndpoint struct { IsDefault *bool `xml:"isDefault,attr"` } +// UnmarshalXML implements xml.Unmarshaler +func (m *IndexedEndpoint) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + type Alias IndexedEndpoint + aux := &struct { + *Alias + }{ + Alias: (*Alias)(m), + } + if err := d.DecodeElement(aux, &start); err != nil { + return err + } + + var err error + m.Location, err = checkEndpointLocation(m.Binding, m.Location) + if err != nil { + return err + } + if m.ResponseLocation != nil { + responseLocation, err := checkEndpointLocation(m.Binding, *m.ResponseLocation) + if err != nil { + return err + } + if responseLocation != "" { + m.ResponseLocation = &responseLocation + } else { + m.ResponseLocation = nil + } + } + + return nil +} + // SSODescriptor represents the SAML complex type SSODescriptor // // See http://docs.oasis-open.org/security/saml/v2.0/saml-metadata-2.0-os.pdf §2.4.2 diff --git a/metadata_test.go b/metadata_test.go index cf1a4659..92c7daed 100644 --- a/metadata_test.go +++ b/metadata_test.go @@ -165,3 +165,11 @@ cvCsEFiJZ4AbF+DgmO6TarJ8O05t8zvnOwJlNCASPZRH/JmF8tX0hoHuAQ==`, assert.Check(t, err) golden.Assert(t, string(buf), "TestCanProduceSPMetadata_expected") } + +func TestMetadataValidatesUrlSchemeForProtocolBinding(t *testing.T) { + buf := golden.Get(t, "TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml") + + metadata := EntityDescriptor{} + err := xml.Unmarshal(buf, &metadata) + assert.Error(t, err, "invalid url scheme \"javascript\" for binding \"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\"") +} diff --git a/testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml b/testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml new file mode 100644 index 00000000..8273c6b0 --- /dev/null +++ b/testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml @@ -0,0 +1 @@ +Required attributes \ No newline at end of file