From ea6eee2a0b0a8ed3c3d2f60653dc2f958ad355ab Mon Sep 17 00:00:00 2001 From: jguer Date: Wed, 20 Jul 2022 17:32:57 +0200 Subject: [PATCH 1/9] implement query signature verification add support for sha1 & sha512 add tests use query sign in redirect implement review feedback - Return error if signature is unsupported - wrap errors Co-authored-by: Ieva Co-authored-by: Orgad Shaneh --- service_provider.go | 35 ++++---- service_provider_signed.go | 141 ++++++++++++++++++++++++++++++++ service_provider_signed_test.go | 117 ++++++++++++++++++++++++++ testdata/SP_IDPMetadata_signing | 96 ++++++++++++++++++++++ 4 files changed, 374 insertions(+), 15 deletions(-) create mode 100644 service_provider_signed.go create mode 100644 service_provider_signed_test.go create mode 100644 testdata/SP_IDPMetadata_signing diff --git a/service_provider.go b/service_provider.go index 3eac33f7..415b90e0 100644 --- a/service_provider.go +++ b/service_provider.go @@ -259,29 +259,24 @@ func (req *AuthnRequest) Redirect(relayState string, sp *ServiceProvider) (*url. rv, _ := url.Parse(req.Destination) // We can't depend on Query().set() as order matters for signing + reqString := string(w.Bytes()) query := rv.RawQuery if len(query) > 0 { - query += "&SAMLRequest=" + url.QueryEscape(string(w.Bytes())) + query += "&" + string(samlRequest) + "=" + url.QueryEscape(reqString) } else { - query += "SAMLRequest=" + url.QueryEscape(string(w.Bytes())) + query += string(samlRequest) + "=" + url.QueryEscape(reqString) } if relayState != "" { query += "&RelayState=" + relayState } if len(sp.SignatureMethod) > 0 { - query += "&SigAlg=" + url.QueryEscape(sp.SignatureMethod) - signingContext, err := GetSigningContext(sp) - - if err != nil { - return nil, err + var errSig error + query, errSig = sp.signQuery(samlRequest, query, reqString, relayState) + if errSig != nil { + return nil, errSig } - sig, err := signingContext.SignString(query) - if err != nil { - return nil, err - } - query += "&Signature=" + url.QueryEscape(base64.StdEncoding.EncodeToString(sig)) } rv.RawQuery = query @@ -1456,8 +1451,9 @@ func (sp *ServiceProvider) nameIDFormat() string { // ValidateLogoutResponseRequest validates the LogoutResponse content from the request func (sp *ServiceProvider) ValidateLogoutResponseRequest(req *http.Request) error { - if data := req.URL.Query().Get("SAMLResponse"); data != "" { - return sp.ValidateLogoutResponseRedirect(data) + query := req.URL.Query() + if data := query.Get("SAMLResponse"); data != "" { + return sp.ValidateLogoutResponseRedirect(query) } err := req.ParseForm() @@ -1512,7 +1508,8 @@ func (sp *ServiceProvider) ValidateLogoutResponseForm(postFormData string) error // // 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 { +func (sp *ServiceProvider) ValidateLogoutResponseRedirect(query url.Values) error { + queryParameterData := query.Get("SAMLResponse") retErr := &InvalidResponseError{ Now: TimeNow(), } @@ -1534,6 +1531,13 @@ func (sp *ServiceProvider) ValidateLogoutResponseRedirect(queryParameterData str return err } + if query.Get("Signature") != "" && query.Get("SigAlg") != "" { + if err := sp.validateQuerySig(query); err != nil { + retErr.PrivateErr = err + return retErr + } + } + doc := etree.NewDocument() if err := doc.ReadFromBytes(rawResponseBuf); err != nil { retErr.PrivateErr = err @@ -1553,6 +1557,7 @@ func (sp *ServiceProvider) ValidateLogoutResponseRedirect(queryParameterData str if err := sp.validateLogoutResponse(&resp); err != nil { return err } + return nil } diff --git a/service_provider_signed.go b/service_provider_signed.go new file mode 100644 index 00000000..d8b02f06 --- /dev/null +++ b/service_provider_signed.go @@ -0,0 +1,141 @@ +package saml + +import ( + "crypto" + "crypto/rsa" + "crypto/sha1" // #nosec G505 + "crypto/sha256" + "crypto/sha512" + "crypto/x509" + "encoding/base64" + "errors" + "fmt" + "net/url" + + dsig "github.com/russellhaering/goxmldsig" +) + +type reqType string + +const ( + samlRequest reqType = "SAMLRequest" + samlResponse reqType = "SAMLResponse" +) + +var ( + // ErrInvalidQuerySignature is returned when the query signature is invalid + ErrInvalidQuerySignature = errors.New("invalid query signature") + // ErrNoQuerySignature is returned when the query does not contain a signature + ErrNoQuerySignature = errors.New("query Signature or SigAlg not found") +) + +// Sign Query with the SP private key. +// Returns provided query with the SigAlg and Signature parameters added. +func (sp *ServiceProvider) signQuery(reqT reqType, query, body, relayState string) (string, error) { + signingContext, err := GetSigningContext(sp) + + // Encode Query as standard demands. query.Encode() is not standard compliant + toHash := string(reqT) + "=" + url.QueryEscape(body) + if relayState != "" { + toHash += "&RelayState=" + url.QueryEscape(relayState) + } + + toHash += "&SigAlg=" + url.QueryEscape(sp.SignatureMethod) + + if err != nil { + return "", err + } + + sig, err := signingContext.SignString(toHash) + if err != nil { + return "", err + } + + query += "&SigAlg=" + url.QueryEscape(sp.SignatureMethod) + query += "&Signature=" + url.QueryEscape(base64.StdEncoding.EncodeToString(sig)) + + return query, nil +} + +// validateSig validation of the signature of the Redirect Binding in query values +// Query is valid if return is nil +func (sp *ServiceProvider) validateQuerySig(query url.Values) error { + sig := query.Get("Signature") + alg := query.Get("SigAlg") + if sig == "" || alg == "" { + return ErrNoQuerySignature + } + + certs, err := sp.getIDPSigningCerts() + if err != nil { + return err + } + + respType := "" + if query.Get("SAMLResponse") != "" { + respType = "SAMLResponse" + } else if query.Get("SAMLRequest") != "" { + respType = "SAMLRequest" + } else { + return fmt.Errorf("No SAMLResponse or SAMLRequest found in query") + } + + // Encode Query as standard demands. + // query.Encode() is not standard compliant + // as query encoding order matters + res := respType + "=" + url.QueryEscape(query.Get(respType)) + + relayState := query.Get("RelayState") + if relayState != "" { + res += "&RelayState=" + url.QueryEscape(relayState) + } + + res += "&SigAlg=" + url.QueryEscape(alg) + + // Signature is base64 encoded + sigBytes, err := base64.StdEncoding.DecodeString(sig) + if err != nil { + return fmt.Errorf("failed to decode signature: %w", err) + } + + var ( + hashed []byte + hashAlg crypto.Hash + sigAlg x509.SignatureAlgorithm + ) + + // Hashed Query + switch alg { + case dsig.RSASHA256SignatureMethod: + hashed256 := sha256.Sum256([]byte(res)) + hashed = hashed256[:] + hashAlg = crypto.SHA256 + sigAlg = x509.SHA256WithRSA + case dsig.RSASHA512SignatureMethod: + hashed512 := sha512.Sum512([]byte(res)) + hashed = hashed512[:] + hashAlg = crypto.SHA512 + sigAlg = x509.SHA512WithRSA + case dsig.RSASHA1SignatureMethod: + hashed1 := sha1.Sum([]byte(res)) // #nosec G401 + hashed = hashed1[:] + hashAlg = crypto.SHA1 + sigAlg = x509.SHA1WithRSA + default: + return fmt.Errorf("unsupported signature algorithm: %s", alg) + } + + // validate signature + for _, cert := range certs { + // verify cert is RSA + if cert.SignatureAlgorithm != sigAlg { + continue + } + + if err := rsa.VerifyPKCS1v15(cert.PublicKey.(*rsa.PublicKey), hashAlg, hashed, sigBytes); err == nil { + return nil + } + } + + return ErrInvalidQuerySignature +} diff --git a/service_provider_signed_test.go b/service_provider_signed_test.go new file mode 100644 index 00000000..a45b8cbd --- /dev/null +++ b/service_provider_signed_test.go @@ -0,0 +1,117 @@ +package saml + +import ( + "crypto/rsa" + "encoding/base64" + "encoding/xml" + "net/url" + "testing" + + dsig "github.com/russellhaering/goxmldsig" + "gotest.tools/assert" + "gotest.tools/golden" +) + +// Given a SAMLRequest query string, sign the query and validate signature +// Using same Cert for SP and IdP in order to test +func TestSigningAndValidation(t *testing.T) { + type testCase struct { + desc string + relayState string + requestType reqType + wantErr bool + wantRawQuery string + } + + testCases := []testCase{ + { + desc: "validate signature of SAMLRequest with relayState", + relayState: "AAAAAAAAAAAA", + requestType: samlRequest, + wantRawQuery: "SAMLRequest=PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6NTc6MDlaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAuZXhhbXBsZS5jb20vc2FtbC9zc28iIEFzc2VydGlvbkNvbnN1bWVyU2VydmljZVVSTD0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9zYW1sMi9hY3MiIFByb3RvY29sQmluZGluZz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmJpbmRpbmdzOkhUVFAtUE9TVCI%2BPHNhbWw6SXNzdWVyIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6ZW50aXR5Ij5odHRwczovL3NwLmV4YW1wbGUuY29tL3NhbWwyL21ldGFkYXRhPC9zYW1sOklzc3Vlcj48c2FtbHA6TmFtZUlEUG9saWN5IEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50IiBBbGxvd0NyZWF0ZT0idHJ1ZSIvPjwvc2FtbHA6QXV0aG5SZXF1ZXN0Pg%3D%3D&RelayState=AAAAAAAAAAAA&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=zWAF4S%2FIs7tfmEriOsT5Fm8EFOGS3iCq6OxP5i7hM%2BMPwAoXwdDz6fKH8euS1gQ3sGOZBdHD588FZLvnO1OeCxLaEsxHMVKsAZSZFLBmPPwqB6e%2B84cCwX2szOeoMROaR%2B36mdoBDRQz36JIvyBBG%2FND9x41k%2FGQuAuwk%2B9fkuE%3D", + }, + { + desc: "validate signature of SAML request without relay state", + relayState: "", + requestType: samlRequest, + wantRawQuery: "SAMLRequest=PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6NTc6MDlaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAuZXhhbXBsZS5jb20vc2FtbC9zc28iIEFzc2VydGlvbkNvbnN1bWVyU2VydmljZVVSTD0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9zYW1sMi9hY3MiIFByb3RvY29sQmluZGluZz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmJpbmRpbmdzOkhUVFAtUE9TVCI%2BPHNhbWw6SXNzdWVyIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6ZW50aXR5Ij5odHRwczovL3NwLmV4YW1wbGUuY29tL3NhbWwyL21ldGFkYXRhPC9zYW1sOklzc3Vlcj48c2FtbHA6TmFtZUlEUG9saWN5IEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50IiBBbGxvd0NyZWF0ZT0idHJ1ZSIvPjwvc2FtbHA6QXV0aG5SZXF1ZXN0Pg%3D%3D&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=HDdoHJSdkYh9%2BmE7RZ1LXcsAWIMJ6LuzKJgwLxH%2BQ4sKFlh8b5moFuQ%2B7rPEwoTcg9SjgCGV5rW9v8PrSU7WGKcLfAbeVwXWyU94ghjFZHEj%2BFCDpsfTD750ZPAPVnhVr0GogFZZ7c%2BEWX4NAqL4CYxDvsg56o%2BpOjw62G%2FyPDc%3D", + }, + { + desc: "validate signature of SAML response with relay state", + relayState: "AAAAAAAAAAAA", + requestType: samlResponse, + wantRawQuery: "SAMLResponse=PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6NTc6MDlaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAuZXhhbXBsZS5jb20vc2FtbC9zc28iIEFzc2VydGlvbkNvbnN1bWVyU2VydmljZVVSTD0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9zYW1sMi9hY3MiIFByb3RvY29sQmluZGluZz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmJpbmRpbmdzOkhUVFAtUE9TVCI%2BPHNhbWw6SXNzdWVyIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6ZW50aXR5Ij5odHRwczovL3NwLmV4YW1wbGUuY29tL3NhbWwyL21ldGFkYXRhPC9zYW1sOklzc3Vlcj48c2FtbHA6TmFtZUlEUG9saWN5IEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50IiBBbGxvd0NyZWF0ZT0idHJ1ZSIvPjwvc2FtbHA6QXV0aG5SZXF1ZXN0Pg%3D%3D&RelayState=AAAAAAAAAAAA&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=JDeiWfLgV7SZqgqU64wgtAHS%2FqtF2c3c%2B9g1vdfRHn03tm5jrgsvJtIYg1BD8HoejCoyruH3xgDz1i2qqecVcUiAdaVgVvhn0JWJ%2BzeN9YpUFTEQ4Ah1pwezlSArzuz5esgYzSkemViox313HePWZ%2Fd0FAmtdXuGHA8O0Lp%2F4Ws%3D", + }, + } + + idpMetadata := golden.Get(t, "SP_IDPMetadata_signing") + s := ServiceProvider{ + Key: mustParsePrivateKey(golden.Get(t, "idp_key.pem")).(*rsa.PrivateKey), + Certificate: mustParseCertificate(golden.Get(t, "idp_cert.pem")), + MetadataURL: mustParseURL("https://15661444.ngrok.io/saml2/metadata"), + AcsURL: mustParseURL("https://15661444.ngrok.io/saml2/acs"), + SignatureMethod: dsig.RSASHA1SignatureMethod, + } + + err := xml.Unmarshal(idpMetadata, &s.IDPMetadata) + idpCert, err := s.getIDPSigningCerts() + + assert.Check(t, err == nil) + assert.Check(t, + s.Certificate.Issuer.CommonName == idpCert[0].Issuer.CommonName, "expected %s, got %s", + s.Certificate.Issuer.CommonName, idpCert[0].Issuer.CommonName) + + req := golden.Get(t, "idp_authn_request.xml") + reqString := base64.StdEncoding.EncodeToString(req) + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + relayState := tc.relayState + + rawQuery := string(tc.requestType) + "=" + url.QueryEscape(reqString) + + if relayState != "" { + rawQuery += "&RelayState=" + relayState + } + + rawQuery, err = s.signQuery(tc.requestType, rawQuery, reqString, relayState) + assert.NilError(t, err, "error signing query: %s", err) + + assert.Equal(t, tc.wantRawQuery, rawQuery) + + query, err := url.ParseQuery(rawQuery) + assert.NilError(t, err, "error parsing query: %s", err) + + err = s.validateQuerySig(query) + assert.NilError(t, err, "error validating query: %s", err) + }) + } +} + +// Given a raw query with an unsupported signature method, the signature should be rejected. +func TestInvalidSignatureAlgorithm(t *testing.T) { + rawQuery := "SAMLRequest=PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6NTc6MDlaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAuZXhhbXBsZS5jb20vc2FtbC9zc28iIEFzc2VydGlvbkNvbnN1bWVyU2VydmljZVVSTD0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9zYW1sMi9hY3MiIFByb3RvY29sQmluZGluZz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmJpbmRpbmdzOkhUVFAtUE9TVCI%2BPHNhbWw6SXNzdWVyIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6ZW50aXR5Ij5odHRwczovL3NwLmV4YW1wbGUuY29tL3NhbWwyL21ldGFkYXRhPC9zYW1sOklzc3Vlcj48c2FtbHA6TmFtZUlEUG9saWN5IEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50IiBBbGxvd0NyZWF0ZT0idHJ1ZSIvPjwvc2FtbHA6QXV0aG5SZXF1ZXN0Pg%3D%3D&RelayState=AAAAAAAAAAAA&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha384&Signature=zWAF4S%2FIs7tfmEriOsT5Fm8EFOGS3iCq6OxP5i7hM%2BMPwAoXwdDz6fKH8euS1gQ3sGOZBdHD588FZLvnO1OeCxLaEsxHMVKsAZSZFLBmPPwqB6e%2B84cCwX2szOeoMROaR%2B36mdoBDRQz36JIvyBBG%2FND9x41k%2FGQuAuwk%2B9fkuE%3D" + + idpMetadata := golden.Get(t, "SP_IDPMetadata_signing") + s := ServiceProvider{ + Key: mustParsePrivateKey(golden.Get(t, "idp_key.pem")).(*rsa.PrivateKey), + Certificate: mustParseCertificate(golden.Get(t, "idp_cert.pem")), + MetadataURL: mustParseURL("https://15661444.ngrok.io/saml2/metadata"), + AcsURL: mustParseURL("https://15661444.ngrok.io/saml2/acs"), + SignatureMethod: dsig.RSASHA1SignatureMethod, + } + + err := xml.Unmarshal(idpMetadata, &s.IDPMetadata) + idpCert, err := s.getIDPSigningCerts() + + assert.Check(t, err == nil) + assert.Check(t, + s.Certificate.Issuer.CommonName == idpCert[0].Issuer.CommonName, "expected %s, got %s", + s.Certificate.Issuer.CommonName, idpCert[0].Issuer.CommonName) + + query, err := url.ParseQuery(rawQuery) + assert.NilError(t, err, "error parsing query: %s", err) + + err = s.validateQuerySig(query) + assert.Error(t, err, "unsupported signature algorithm: http://www.w3.org/2000/09/xmldsig#rsa-sha384") +} diff --git a/testdata/SP_IDPMetadata_signing b/testdata/SP_IDPMetadata_signing new file mode 100644 index 00000000..58793ea9 --- /dev/null +++ b/testdata/SP_IDPMetadata_signing @@ -0,0 +1,96 @@ + + + + + + + + + + + + + + + testshib.org + + TestShib Test IdP + TestShib IdP. Use this as a source of attributes + for your test SP. + https://www.testshib.org/testshibtwo.jpg + + + + + + MIIB7zCCAVgCCQDFzbKIp7b3MTANBgkqhkiG9w0BAQUFADA8MQswCQYDVQQGEwJV + UzELMAkGA1UECAwCR0ExDDAKBgNVBAoMA2ZvbzESMBAGA1UEAwwJbG9jYWxob3N0 + MB4XDTEzMTAwMjAwMDg1MVoXDTE0MTAwMjAwMDg1MVowPDELMAkGA1UEBhMCVVMx + CzAJBgNVBAgMAkdBMQwwCgYDVQQKDANmb28xEjAQBgNVBAMMCWxvY2FsaG9zdDCB + nzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA1PMHYmhZj308kWLhZVT4vOulqx/9 + ibm5B86fPWwUKKQ2i12MYtz07tzukPymisTDhQaqyJ8Kqb/6JjhmeMnEOdTvSPmH + O8m1ZVveJU6NoKRn/mP/BD7FW52WhbrUXLSeHVSKfWkNk6S4hk9MV9TswTvyRIKv + Rsw0X/gfnqkroJcCAwEAATANBgkqhkiG9w0BAQUFAAOBgQCMMlIO+GNcGekevKgk + akpMdAqJfs24maGb90DvTLbRZRD7Xvn1MnVBBS9hzlXiFLYOInXACMW5gcoRFfeT + QLSouMM8o57h0uKjfTmuoWHLQLi6hnF+cvCsEFiJZ4AbF+DgmO6TarJ8O05t8zvn + OwJlNCASPZRH/JmF8tX0hoHuAQ== + + + + + + + + + + + + urn:mace:shibboleth:1.0:nameIdentifier + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + + + + + + + + + + + + MIIB7zCCAVgCCQDFzbKIp7b3MTANBgkqhkiG9w0BAQUFADA8MQswCQYDVQQGEwJV + UzELMAkGA1UECAwCR0ExDDAKBgNVBAoMA2ZvbzESMBAGA1UEAwwJbG9jYWxob3N0 + MB4XDTEzMTAwMjAwMDg1MVoXDTE0MTAwMjAwMDg1MVowPDELMAkGA1UEBhMCVVMx + CzAJBgNVBAgMAkdBMQwwCgYDVQQKDANmb28xEjAQBgNVBAMMCWxvY2FsaG9zdDCB + nzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA1PMHYmhZj308kWLhZVT4vOulqx/9 + ibm5B86fPWwUKKQ2i12MYtz07tzukPymisTDhQaqyJ8Kqb/6JjhmeMnEOdTvSPmH + O8m1ZVveJU6NoKRn/mP/BD7FW52WhbrUXLSeHVSKfWkNk6S4hk9MV9TswTvyRIKv + Rsw0X/gfnqkroJcCAwEAATANBgkqhkiG9w0BAQUFAAOBgQCMMlIO+GNcGekevKgk + akpMdAqJfs24maGb90DvTLbRZRD7Xvn1MnVBBS9hzlXiFLYOInXACMW5gcoRFfeT + QLSouMM8o57h0uKjfTmuoWHLQLi6hnF+cvCsEFiJZ4AbF+DgmO6TarJ8O05t8zvn + OwJlNCASPZRH/JmF8tX0hoHuAQ== + + + + + + + + + + + + urn:mace:shibboleth:1.0:nameIdentifier + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + + + TestShib Two Identity Provider + TestShib Two + http://www.testshib.org/testshib-two/ + + + Nate + Klingenstein + ndk@internet2.edu + + \ No newline at end of file From dae376ee009af19e0345424be52bf97bb048eb56 Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Sat, 17 May 2025 23:20:19 +0100 Subject: [PATCH 2/9] fix: improve SAML signature validation for redirect binding - Removes the signature validation using SAMLResponse payload for HTTP-Redirect binding - Fix the Signature validation failures for ADFS because of decode/encode while reconstructing the sign data --- service_provider.go | 18 ++--- service_provider_signed.go | 118 +++++++++++++++++--------------- service_provider_signed_test.go | 30 ++++---- service_provider_test.go | 13 ++++ util.go | 21 ++++++ util_test.go | 35 ++++++++++ 6 files changed, 154 insertions(+), 81 deletions(-) create mode 100644 util_test.go diff --git a/service_provider.go b/service_provider.go index dc9ab7af..7df18eea 100644 --- a/service_provider.go +++ b/service_provider.go @@ -306,9 +306,9 @@ func (r *AuthnRequest) Redirect(relayState string, sp *ServiceProvider) (*url.UR // We can't depend on Query().set() as order matters for signing query := rv.RawQuery if len(query) > 0 { - query += "&" + string(samlRequest) + "=" + url.QueryEscape(requestStr.String()) + query += "&" + string(SAMLRequest) + "=" + url.QueryEscape(requestStr.String()) } else { - query += string(samlRequest) + "=" + url.QueryEscape(requestStr.String()) + query += string(SAMLRequest) + "=" + url.QueryEscape(requestStr.String()) } if relayState != "" { @@ -316,7 +316,7 @@ func (r *AuthnRequest) Redirect(relayState string, sp *ServiceProvider) (*url.UR } if len(sp.SignatureMethod) > 0 { var errSig error - query, errSig = sp.signQuery(samlRequest, query, requestStr.String(), relayState) + query, errSig = sp.signQuery(SAMLRequest, query, requestStr.String(), relayState) if errSig != nil { return nil, errSig } @@ -1620,7 +1620,7 @@ func (sp *ServiceProvider) nameIDFormat() string { func (sp *ServiceProvider) ValidateLogoutResponseRequest(req *http.Request) error { query := req.URL.Query() if data := query.Get("SAMLResponse"); data != "" { - return sp.ValidateLogoutResponseRedirect(query) + return sp.ValidateLogoutResponseRedirect(req) } err := req.ParseForm() @@ -1672,7 +1672,8 @@ func (sp *ServiceProvider) ValidateLogoutResponseForm(postFormData string) error // // 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(query url.Values) error { +func (sp *ServiceProvider) ValidateLogoutResponseRedirect(r *http.Request) error { + query := r.URL.Query() queryParameterData := query.Get("SAMLResponse") retErr := &InvalidResponseError{ Now: TimeNow(), @@ -1696,7 +1697,7 @@ func (sp *ServiceProvider) ValidateLogoutResponseRedirect(query url.Values) erro } if query.Get("Signature") != "" && query.Get("SigAlg") != "" { - if err := sp.validateQuerySig(query); err != nil { + if err := sp.validateRedirectBindingSignature(r); err != nil { retErr.PrivateErr = err return retErr } @@ -1708,11 +1709,6 @@ func (sp *ServiceProvider) ValidateLogoutResponseRedirect(query url.Values) erro return retErr } - if err := sp.validateSignature(doc.Root()); err != nil { - retErr.PrivateErr = err - return retErr - } - var resp LogoutResponse if err := unmarshalElement(doc.Root(), &resp); err != nil { retErr.PrivateErr = err diff --git a/service_provider_signed.go b/service_provider_signed.go index d8b02f06..b1fe6865 100644 --- a/service_provider_signed.go +++ b/service_provider_signed.go @@ -10,16 +10,20 @@ import ( "encoding/base64" "errors" "fmt" + "net/http" "net/url" dsig "github.com/russellhaering/goxmldsig" ) -type reqType string +type queryParam string const ( - samlRequest reqType = "SAMLRequest" - samlResponse reqType = "SAMLResponse" + SAMLRequest queryParam = "SAMLRequest" + SAMLResponse queryParam = "SAMLResponse" + SigAlg queryParam = "SigAlg" + Signature queryParam = "Signature" + RelayState queryParam = "RelayState" ) var ( @@ -31,7 +35,7 @@ var ( // Sign Query with the SP private key. // Returns provided query with the SigAlg and Signature parameters added. -func (sp *ServiceProvider) signQuery(reqT reqType, query, body, relayState string) (string, error) { +func (sp *ServiceProvider) signQuery(reqT queryParam, query, body, relayState string) (string, error) { signingContext, err := GetSigningContext(sp) // Encode Query as standard demands. query.Encode() is not standard compliant @@ -57,85 +61,87 @@ func (sp *ServiceProvider) signQuery(reqT reqType, query, body, relayState strin return query, nil } -// validateSig validation of the signature of the Redirect Binding in query values +// validateRedirectBindingSignature validation of the signature of the Redirect Binding in query values // Query is valid if return is nil -func (sp *ServiceProvider) validateQuerySig(query url.Values) error { - sig := query.Get("Signature") - alg := query.Get("SigAlg") +// URL encoding could be done uppercase or lowercase and in addition, can be done following RFC 3986 or not. +// Based on that, if an entity sign using a url enconding mechanism that differs the way another entity decode it, will cause Signature validation issues. +// In order to avoid it the RawQuery is used to retrieve the original query parameters. +// Re doing encoding/decoding of the query parameter are failing especially in ADFS Single Logouts. +func (sp *ServiceProvider) validateRedirectBindingSignature(r *http.Request) error { + rawQuery := r.URL.RawQuery + + // Extract and validate required query params + sig := getRawQueryParam(rawQuery, string(Signature)) + alg := getRawQueryParam(rawQuery, string(SigAlg)) if sig == "" || alg == "" { return ErrNoQuerySignature } + // Get the IDP public certificates certs, err := sp.getIDPSigningCerts() if err != nil { return err } - respType := "" - if query.Get("SAMLResponse") != "" { - respType = "SAMLResponse" - } else if query.Get("SAMLRequest") != "" { - respType = "SAMLRequest" + // Determine whether we're dealing with a response or request + var paramType, paramValue string + if val := getRawQueryParam(rawQuery, string(SAMLResponse)); val != "" { + paramType, paramValue = string(SAMLResponse), val + } else if val := getRawQueryParam(rawQuery, string(SAMLRequest)); val != "" { + paramType, paramValue = string(SAMLRequest), val } else { - return fmt.Errorf("No SAMLResponse or SAMLRequest found in query") + return fmt.Errorf("no SAMLResponse or SAMLRequest found in query") } - // Encode Query as standard demands. - // query.Encode() is not standard compliant - // as query encoding order matters - res := respType + "=" + url.QueryEscape(query.Get(respType)) - - relayState := query.Get("RelayState") - if relayState != "" { - res += "&RelayState=" + url.QueryEscape(relayState) + // Reconstruct the signed payload (already URL-encoded in RawQuery, so no need to encode/escape it again) + signedData := fmt.Sprintf("%s=%s", paramType, paramValue) + if relay := getRawQueryParam(rawQuery, string(RelayState)); relay != "" { + signedData += "&RelayState=" + relay } + signedData += "&SigAlg=" + alg - res += "&SigAlg=" + url.QueryEscape(alg) - - // Signature is base64 encoded - sigBytes, err := base64.StdEncoding.DecodeString(sig) + // Decode signature from base64, here we have to decode query param value before base64 decoding + sigBytes, err := base64.StdEncoding.DecodeString(r.URL.Query().Get(string(Signature))) if err != nil { return fmt.Errorf("failed to decode signature: %w", err) } - var ( - hashed []byte - hashAlg crypto.Hash - sigAlg x509.SignatureAlgorithm - ) - - // Hashed Query - switch alg { - case dsig.RSASHA256SignatureMethod: - hashed256 := sha256.Sum256([]byte(res)) - hashed = hashed256[:] - hashAlg = crypto.SHA256 - sigAlg = x509.SHA256WithRSA - case dsig.RSASHA512SignatureMethod: - hashed512 := sha512.Sum512([]byte(res)) - hashed = hashed512[:] - hashAlg = crypto.SHA512 - sigAlg = x509.SHA512WithRSA - case dsig.RSASHA1SignatureMethod: - hashed1 := sha1.Sum([]byte(res)) // #nosec G401 - hashed = hashed1[:] - hashAlg = crypto.SHA1 - sigAlg = x509.SHA1WithRSA - default: - return fmt.Errorf("unsupported signature algorithm: %s", alg) + // Determine hashing algorithm + hashAlg, sigAlg, hashed, err := computeSignatureHash(r.URL.Query().Get(string(SigAlg)), []byte(signedData)) + if err != nil { + return err } - // validate signature + // Attempt verification with each valid certificate for _, cert := range certs { - // verify cert is RSA if cert.SignatureAlgorithm != sigAlg { continue } - - if err := rsa.VerifyPKCS1v15(cert.PublicKey.(*rsa.PublicKey), hashAlg, hashed, sigBytes); err == nil { - return nil + pubKey, ok := cert.PublicKey.(*rsa.PublicKey) + if !ok { + continue + } + if err := rsa.VerifyPKCS1v15(pubKey, hashAlg, hashed, sigBytes); err == nil { + return nil // ✅ Signature verified } } return ErrInvalidQuerySignature } + +// computeSignatureHash computes the signature hash for the given algorithm and data. +func computeSignatureHash(alg string, data []byte) (crypto.Hash, x509.SignatureAlgorithm, []byte, error) { + switch alg { + case dsig.RSASHA256SignatureMethod: + h := sha256.Sum256(data) + return crypto.SHA256, x509.SHA256WithRSA, h[:], nil + case dsig.RSASHA512SignatureMethod: + h := sha512.Sum512(data) + return crypto.SHA512, x509.SHA512WithRSA, h[:], nil + case dsig.RSASHA1SignatureMethod: + h := sha1.Sum(data) // #nosec G401 + return crypto.SHA1, x509.SHA1WithRSA, h[:], nil + default: + return 0, 0, nil, fmt.Errorf("unsupported signature algorithm: %s", alg) + } +} diff --git a/service_provider_signed_test.go b/service_provider_signed_test.go index a45b8cbd..25270684 100644 --- a/service_provider_signed_test.go +++ b/service_provider_signed_test.go @@ -4,6 +4,7 @@ import ( "crypto/rsa" "encoding/base64" "encoding/xml" + "net/http" "net/url" "testing" @@ -18,8 +19,7 @@ func TestSigningAndValidation(t *testing.T) { type testCase struct { desc string relayState string - requestType reqType - wantErr bool + requestType queryParam wantRawQuery string } @@ -27,19 +27,19 @@ func TestSigningAndValidation(t *testing.T) { { desc: "validate signature of SAMLRequest with relayState", relayState: "AAAAAAAAAAAA", - requestType: samlRequest, + requestType: SAMLRequest, wantRawQuery: "SAMLRequest=PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6NTc6MDlaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAuZXhhbXBsZS5jb20vc2FtbC9zc28iIEFzc2VydGlvbkNvbnN1bWVyU2VydmljZVVSTD0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9zYW1sMi9hY3MiIFByb3RvY29sQmluZGluZz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmJpbmRpbmdzOkhUVFAtUE9TVCI%2BPHNhbWw6SXNzdWVyIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6ZW50aXR5Ij5odHRwczovL3NwLmV4YW1wbGUuY29tL3NhbWwyL21ldGFkYXRhPC9zYW1sOklzc3Vlcj48c2FtbHA6TmFtZUlEUG9saWN5IEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50IiBBbGxvd0NyZWF0ZT0idHJ1ZSIvPjwvc2FtbHA6QXV0aG5SZXF1ZXN0Pg%3D%3D&RelayState=AAAAAAAAAAAA&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=zWAF4S%2FIs7tfmEriOsT5Fm8EFOGS3iCq6OxP5i7hM%2BMPwAoXwdDz6fKH8euS1gQ3sGOZBdHD588FZLvnO1OeCxLaEsxHMVKsAZSZFLBmPPwqB6e%2B84cCwX2szOeoMROaR%2B36mdoBDRQz36JIvyBBG%2FND9x41k%2FGQuAuwk%2B9fkuE%3D", }, { desc: "validate signature of SAML request without relay state", relayState: "", - requestType: samlRequest, + requestType: SAMLRequest, wantRawQuery: "SAMLRequest=PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6NTc6MDlaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAuZXhhbXBsZS5jb20vc2FtbC9zc28iIEFzc2VydGlvbkNvbnN1bWVyU2VydmljZVVSTD0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9zYW1sMi9hY3MiIFByb3RvY29sQmluZGluZz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmJpbmRpbmdzOkhUVFAtUE9TVCI%2BPHNhbWw6SXNzdWVyIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6ZW50aXR5Ij5odHRwczovL3NwLmV4YW1wbGUuY29tL3NhbWwyL21ldGFkYXRhPC9zYW1sOklzc3Vlcj48c2FtbHA6TmFtZUlEUG9saWN5IEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50IiBBbGxvd0NyZWF0ZT0idHJ1ZSIvPjwvc2FtbHA6QXV0aG5SZXF1ZXN0Pg%3D%3D&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=HDdoHJSdkYh9%2BmE7RZ1LXcsAWIMJ6LuzKJgwLxH%2BQ4sKFlh8b5moFuQ%2B7rPEwoTcg9SjgCGV5rW9v8PrSU7WGKcLfAbeVwXWyU94ghjFZHEj%2BFCDpsfTD750ZPAPVnhVr0GogFZZ7c%2BEWX4NAqL4CYxDvsg56o%2BpOjw62G%2FyPDc%3D", }, { desc: "validate signature of SAML response with relay state", relayState: "AAAAAAAAAAAA", - requestType: samlResponse, + requestType: SAMLResponse, wantRawQuery: "SAMLResponse=PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6NTc6MDlaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAuZXhhbXBsZS5jb20vc2FtbC9zc28iIEFzc2VydGlvbkNvbnN1bWVyU2VydmljZVVSTD0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9zYW1sMi9hY3MiIFByb3RvY29sQmluZGluZz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmJpbmRpbmdzOkhUVFAtUE9TVCI%2BPHNhbWw6SXNzdWVyIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6ZW50aXR5Ij5odHRwczovL3NwLmV4YW1wbGUuY29tL3NhbWwyL21ldGFkYXRhPC9zYW1sOklzc3Vlcj48c2FtbHA6TmFtZUlEUG9saWN5IEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50IiBBbGxvd0NyZWF0ZT0idHJ1ZSIvPjwvc2FtbHA6QXV0aG5SZXF1ZXN0Pg%3D%3D&RelayState=AAAAAAAAAAAA&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=JDeiWfLgV7SZqgqU64wgtAHS%2FqtF2c3c%2B9g1vdfRHn03tm5jrgsvJtIYg1BD8HoejCoyruH3xgDz1i2qqecVcUiAdaVgVvhn0JWJ%2BzeN9YpUFTEQ4Ah1pwezlSArzuz5esgYzSkemViox313HePWZ%2Fd0FAmtdXuGHA8O0Lp%2F4Ws%3D", }, } @@ -54,6 +54,10 @@ func TestSigningAndValidation(t *testing.T) { } err := xml.Unmarshal(idpMetadata, &s.IDPMetadata) + if err != nil { + t.Fatal(err) + } + idpCert, err := s.getIDPSigningCerts() assert.Check(t, err == nil) @@ -79,10 +83,8 @@ func TestSigningAndValidation(t *testing.T) { assert.Equal(t, tc.wantRawQuery, rawQuery) - query, err := url.ParseQuery(rawQuery) - assert.NilError(t, err, "error parsing query: %s", err) - - err = s.validateQuerySig(query) + req, _ := http.NewRequest("GET", "http://example.com?"+rawQuery, nil) + err = s.validateRedirectBindingSignature(req) assert.NilError(t, err, "error validating query: %s", err) }) } @@ -102,16 +104,16 @@ func TestInvalidSignatureAlgorithm(t *testing.T) { } err := xml.Unmarshal(idpMetadata, &s.IDPMetadata) - idpCert, err := s.getIDPSigningCerts() + assert.Check(t, err == nil) + idpCert, err := s.getIDPSigningCerts() assert.Check(t, err == nil) + assert.Check(t, s.Certificate.Issuer.CommonName == idpCert[0].Issuer.CommonName, "expected %s, got %s", s.Certificate.Issuer.CommonName, idpCert[0].Issuer.CommonName) - query, err := url.ParseQuery(rawQuery) - assert.NilError(t, err, "error parsing query: %s", err) - - err = s.validateQuerySig(query) + req, _ := http.NewRequest("GET", "http://example.com?"+rawQuery, nil) + err = s.validateRedirectBindingSignature(req) assert.Error(t, err, "unsupported signature algorithm: http://www.w3.org/2000/09/xmldsig#rsa-sha384") } diff --git a/service_provider_test.go b/service_provider_test.go index 35103d6b..aa2fc23c 100644 --- a/service_provider_test.go +++ b/service_provider_test.go @@ -2037,3 +2037,16 @@ func TestSPInvalidResponses(t *testing.T) { assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr, "cannot validate signature on Assertion: x509: malformed certificate")) } + +func TestValidateLogoutResponseRequest(t *testing.T) { + t.Run("Should fail invalid base64 SAMLResponse", func(t *testing.T) { + sp := ServiceProvider{} + err := sp.ValidateLogoutResponseRedirect(&http.Request{ + URL: &url.URL{ + RawQuery: "SAMLResponse=invalid", + }, + }) + + assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr, "unable to parse base64: illegal base64 data at input byte 4")) + }) +} \ No newline at end of file diff --git a/util.go b/util.go index eda053ee..649c2f35 100644 --- a/util.go +++ b/util.go @@ -3,6 +3,7 @@ package saml import ( "crypto/rand" "io" + "strings" "time" dsig "github.com/russellhaering/goxmldsig" @@ -30,3 +31,23 @@ func randomBytes(n int) []byte { } return rv } + +// getRawQueryParam extracts the raw (URL-encoded) value of the given query parameter from the raw query string. +// This avoids parsing via url.Values to preserve parameter order and encoding. +func getRawQueryParam(rawQuery, name string) string { + prefix := name + "=" + start := strings.Index(rawQuery, prefix) + if start == -1 { + return "" + } + + // Move index to the start of the value + start += len(prefix) + + // Find the end of the value (next '&' or end of string) + end := strings.Index(rawQuery[start:], "&") + if end == -1 { + return rawQuery[start:] + } + return rawQuery[start : start+end] +} \ No newline at end of file diff --git a/util_test.go b/util_test.go new file mode 100644 index 00000000..700df679 --- /dev/null +++ b/util_test.go @@ -0,0 +1,35 @@ +package saml + +import ( + "net/url" + "testing" + + "gotest.tools/assert" +) + +func TestGetOriginalQueryParam(t *testing.T) { + t.Run("query param not found", func(t *testing.T) { + assert.Equal(t, getRawQueryParam("", "test"), "") + }) + + t.Run("query param found", func(t *testing.T) { + assert.Equal(t, getRawQueryParam("test=1", "test"), "1") + }) + + t.Run("query param found with &", func(t *testing.T) { + assert.Equal(t, getRawQueryParam("test=1&test2=2", "test"), "1") + }) + + t.Run("query param found with & and other params", func(t *testing.T) { + assert.Equal(t, getRawQueryParam("test=1&test2=2&test3=3", "test"), "1") + }) + + t.Run("query param found with encoded samlresponse", func(t *testing.T) { + url, err := url.Parse("/saml/logoutResponse/bkwiatm923wdmnkssrwx?SAMLResponse=fVLBbqQwDP0VlDuEEEJKxCCt2stI7WWn6mEvK5OYLiokCIcyn7%2fMzFaruVSKojzLz37PTkMwjbN5Du9hjT%2bR5uAJk%2bPTgf2uXC50p3Uq%2byJPS3iw6YOSkApV6r4QQqEuWPKGCw3BH1iR5Sw5Eq149BTBxz2UFyrNVSr0q1Amr%2faTVZX%2bxZInpDh4iFfmnxhnMpw7%2fMQxzLhkn10484syPt4J493HNkCc6kJubvIfRMt23rv6r4TXcGCDS62TUNquQOxkUaNynUNRiVKIvnOu68GCA1fu8s%2fT6Mlch3Bg6%2bJNABrIeJiQTLTm9OPl2ezWzLyEGGwYWdtcTS436vckIMLlYpK1XyYpUrYN3oWNMo%2bRuxp6KXqVSq10WuL%2bqqWuL1cvrayghpI3%2fNazbW7rOkWIK92jx%2bAweYNxxe817bvZs81ptRaJGG%2bb66T%2fF%2f0H779E%2bxc%3d&Signature=pJQ9%2ff1UbS88s5T5FUCsE9Alej9rmAdlrd64m4RUKTf5WON3v3H7wI1sMfDyfYQDXZYCtz%2b0txQ7ai9xGxoJIXIMEVKCAQXatdL04shef1DPJ%2f7hk5FtGQviOLeoSAk6fRRw72iLMcZb9m7q89wS4F6Si1VCSS4%2bBOqZvRu6qdHVW496nNh8t5RlfPfuix7XpmWaEpoGEqM3Jl06AqaWyYn5V0K6LIHevCLl6D2QDkeTl5QT6VhiHcFFC%2b%2bYZBTTfMlet6cF6u7IuHhCowk73Jm4goFBStG%2b3gd%2bIxLOez%2bfoCqlX8i68KBqKhktOqJHKGzsTPZ3MEJNSedGl507wQ%3d%3d&SigAlg=http%3a%2f%2fwww.w3.org%2f2001%2f04%2fxmldsig-more%23rsa-sha256#") + assert.NilError(t, err) + + assert.Equal(t, getRawQueryParam(url.RawQuery, "SAMLResponse"), "fVLBbqQwDP0VlDuEEEJKxCCt2stI7WWn6mEvK5OYLiokCIcyn7%2fMzFaruVSKojzLz37PTkMwjbN5Du9hjT%2bR5uAJk%2bPTgf2uXC50p3Uq%2byJPS3iw6YOSkApV6r4QQqEuWPKGCw3BH1iR5Sw5Eq149BTBxz2UFyrNVSr0q1Amr%2faTVZX%2bxZInpDh4iFfmnxhnMpw7%2fMQxzLhkn10484syPt4J493HNkCc6kJubvIfRMt23rv6r4TXcGCDS62TUNquQOxkUaNynUNRiVKIvnOu68GCA1fu8s%2fT6Mlch3Bg6%2bJNABrIeJiQTLTm9OPl2ezWzLyEGGwYWdtcTS436vckIMLlYpK1XyYpUrYN3oWNMo%2bRuxp6KXqVSq10WuL%2bqqWuL1cvrayghpI3%2fNazbW7rOkWIK92jx%2bAweYNxxe817bvZs81ptRaJGG%2bb66T%2fF%2f0H779E%2bxc%3d") + assert.Equal(t, getRawQueryParam(url.RawQuery, "Signature"), "pJQ9%2ff1UbS88s5T5FUCsE9Alej9rmAdlrd64m4RUKTf5WON3v3H7wI1sMfDyfYQDXZYCtz%2b0txQ7ai9xGxoJIXIMEVKCAQXatdL04shef1DPJ%2f7hk5FtGQviOLeoSAk6fRRw72iLMcZb9m7q89wS4F6Si1VCSS4%2bBOqZvRu6qdHVW496nNh8t5RlfPfuix7XpmWaEpoGEqM3Jl06AqaWyYn5V0K6LIHevCLl6D2QDkeTl5QT6VhiHcFFC%2b%2bYZBTTfMlet6cF6u7IuHhCowk73Jm4goFBStG%2b3gd%2bIxLOez%2bfoCqlX8i68KBqKhktOqJHKGzsTPZ3MEJNSedGl507wQ%3d%3d") + assert.Equal(t, getRawQueryParam(url.RawQuery, "SigAlg"), "http%3a%2f%2fwww.w3.org%2f2001%2f04%2fxmldsig-more%23rsa-sha256") + }) +} \ No newline at end of file From cd4a5c333c4a2cf6d8fd1293e387134a2a658039 Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Sat, 17 May 2025 23:34:47 +0100 Subject: [PATCH 3/9] lint --- service_provider_signed.go | 2 +- service_provider_test.go | 2 +- util.go | 2 +- util_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/service_provider_signed.go b/service_provider_signed.go index b1fe6865..677e4faa 100644 --- a/service_provider_signed.go +++ b/service_provider_signed.go @@ -64,7 +64,7 @@ func (sp *ServiceProvider) signQuery(reqT queryParam, query, body, relayState st // validateRedirectBindingSignature validation of the signature of the Redirect Binding in query values // Query is valid if return is nil // URL encoding could be done uppercase or lowercase and in addition, can be done following RFC 3986 or not. -// Based on that, if an entity sign using a url enconding mechanism that differs the way another entity decode it, will cause Signature validation issues. +// Based on that, if an entity sign using a url encoding mechanism that differs the way another entity decode it, will cause Signature validation issues. // In order to avoid it the RawQuery is used to retrieve the original query parameters. // Re doing encoding/decoding of the query parameter are failing especially in ADFS Single Logouts. func (sp *ServiceProvider) validateRedirectBindingSignature(r *http.Request) error { diff --git a/service_provider_test.go b/service_provider_test.go index aa2fc23c..4111b1c8 100644 --- a/service_provider_test.go +++ b/service_provider_test.go @@ -2049,4 +2049,4 @@ func TestValidateLogoutResponseRequest(t *testing.T) { assert.Check(t, is.Error(err.(*InvalidResponseError).PrivateErr, "unable to parse base64: illegal base64 data at input byte 4")) }) -} \ No newline at end of file +} diff --git a/util.go b/util.go index 649c2f35..8916f033 100644 --- a/util.go +++ b/util.go @@ -50,4 +50,4 @@ func getRawQueryParam(rawQuery, name string) string { return rawQuery[start:] } return rawQuery[start : start+end] -} \ No newline at end of file +} diff --git a/util_test.go b/util_test.go index 700df679..adec54da 100644 --- a/util_test.go +++ b/util_test.go @@ -32,4 +32,4 @@ func TestGetOriginalQueryParam(t *testing.T) { assert.Equal(t, getRawQueryParam(url.RawQuery, "Signature"), "pJQ9%2ff1UbS88s5T5FUCsE9Alej9rmAdlrd64m4RUKTf5WON3v3H7wI1sMfDyfYQDXZYCtz%2b0txQ7ai9xGxoJIXIMEVKCAQXatdL04shef1DPJ%2f7hk5FtGQviOLeoSAk6fRRw72iLMcZb9m7q89wS4F6Si1VCSS4%2bBOqZvRu6qdHVW496nNh8t5RlfPfuix7XpmWaEpoGEqM3Jl06AqaWyYn5V0K6LIHevCLl6D2QDkeTl5QT6VhiHcFFC%2b%2bYZBTTfMlet6cF6u7IuHhCowk73Jm4goFBStG%2b3gd%2bIxLOez%2bfoCqlX8i68KBqKhktOqJHKGzsTPZ3MEJNSedGl507wQ%3d%3d") assert.Equal(t, getRawQueryParam(url.RawQuery, "SigAlg"), "http%3a%2f%2fwww.w3.org%2f2001%2f04%2fxmldsig-more%23rsa-sha256") }) -} \ No newline at end of file +} From 7dca39b791e2f5b146392cb36eb22aa4b2c4e1ec Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Sat, 17 May 2025 23:36:31 +0100 Subject: [PATCH 4/9] lint --- service_provider_signed.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/service_provider_signed.go b/service_provider_signed.go index 677e4faa..153904b2 100644 --- a/service_provider_signed.go +++ b/service_provider_signed.go @@ -19,11 +19,16 @@ import ( type queryParam string const ( - SAMLRequest queryParam = "SAMLRequest" + // SAMLRequest query param + SAMLRequest queryParam = "SAMLRequest" + // SAMLResponse query param SAMLResponse queryParam = "SAMLResponse" - SigAlg queryParam = "SigAlg" - Signature queryParam = "Signature" - RelayState queryParam = "RelayState" + // SigAlg query param + SigAlg queryParam = "SigAlg" + // Signature query param + Signature queryParam = "Signature" + // RelayState query param + RelayState queryParam = "RelayState" ) var ( From 1f5b00792414d4dac43eb292e525a01c1d8219fd Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Sun, 18 May 2025 12:08:56 +0100 Subject: [PATCH 5/9] feat: Validate LogoutRequest for IdP initiated Single Logouts --- service_provider.go | 118 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/service_provider.go b/service_provider.go index 7df18eea..906abc1e 100644 --- a/service_provider.go +++ b/service_provider.go @@ -1738,6 +1738,124 @@ func (sp *ServiceProvider) validateLogoutResponse(resp *LogoutResponse) error { return nil } +// ValidateLogoutRequest validates the LogoutRequest content from the request +func (sp *ServiceProvider) ValidateLogoutRequest(req *http.Request) error { + query := req.URL.Query() + if data := query.Get(string(SAMLRequest)); data != "" { + return sp.ValidateLogoutRequestRedirect(req) + } + + err := req.ParseForm() + if err != nil { + return fmt.Errorf("validateLogoutRequest: unable to parse form: %v", err) + } + + return sp.ValidateLogoutRequestForm(req.PostForm.Get(string(SAMLRequest))) +} + +// ValidateLogoutRequestRedirect returns a nil error if the logout request is valid. This is used for the HTTP Redirect binding. +func (sp *ServiceProvider) ValidateLogoutRequestRedirect(r *http.Request) error { + query := r.URL.Query() + queryParameterData := query.Get(string(SAMLRequest)) + retErr := &InvalidResponseError{ + Now: TimeNow(), + } + + rawRequestBuf, err := base64.StdEncoding.DecodeString(queryParameterData) + if err != nil { + retErr.PrivateErr = fmt.Errorf("validateLogoutRequestRedirect: unable to parse base64: %s", err) + return retErr + } + retErr.Response = string(rawRequestBuf) + + gr, err := io.ReadAll(newSaferFlateReader(bytes.NewBuffer(rawRequestBuf))) + if err != nil { + retErr.PrivateErr = err + return retErr + } + + if err := xrv.Validate(bytes.NewReader(gr)); err != nil { + return fmt.Errorf("validateLogoutRequestRedirect: response contains invalid XML: %s", err) + } + + if query.Get("Signature") != "" && query.Get("SigAlg") != "" { + if err := sp.validateRedirectBindingSignature(r); err != nil { + retErr.PrivateErr = err + return retErr + } + } + + doc := etree.NewDocument() + if err := doc.ReadFromBytes(gr); err != nil { + retErr.PrivateErr = err + return retErr + } + + var req LogoutRequest + if err := unmarshalElement(doc.Root(), &req); err != nil { + retErr.PrivateErr = err + return retErr + } + + return sp.validateLogoutRequest(&req) +} + +// ValidateLogoutRequestForm returns a nil error if the logout request is valid. This is used for the HTTP POST binding. +func (sp *ServiceProvider) ValidateLogoutRequestForm(postFormData string) error { + retErr := &InvalidResponseError{ + Now: TimeNow(), + } + + rawRequestBuf, err := base64.StdEncoding.DecodeString(postFormData) + if err != nil { + retErr.PrivateErr = fmt.Errorf("unable to parse base64: %s", err) + return retErr + } + retErr.Response = string(rawRequestBuf) + + if err := xrv.Validate(bytes.NewReader(rawRequestBuf)); err != nil { + return fmt.Errorf("logout request contains invalid XML: %s", err) + } + + doc := etree.NewDocument() + if err := doc.ReadFromBytes(rawRequestBuf); err != nil { + retErr.PrivateErr = err + return retErr + } + + if err := sp.validateSignature(doc.Root()); err != nil { + retErr.PrivateErr = err + return retErr + } + + var req LogoutRequest + if err := unmarshalElement(doc.Root(), &req); err != nil { + retErr.PrivateErr = err + return retErr + } + + return sp.validateLogoutRequest(&req) +} + +// validateLogoutRequest validates the LogoutRequest fields. Returns a nil error if the LogoutRequest is valid. +// This checks the destination, issue instant, and issuer. +func (sp *ServiceProvider) validateLogoutRequest(req *LogoutRequest) error { + if req.Destination != sp.SloURL.String() { + return fmt.Errorf("`Destination` does not match SloURL (expected %q)", sp.SloURL.String()) + } + + now := time.Now() + if req.IssueInstant.Add(MaxIssueDelay).Before(now) { + return fmt.Errorf("issueInstant expired at %s", req.IssueInstant.Add(MaxIssueDelay)) + } + if req.Issuer.Value != sp.IDPMetadata.EntityID { + return fmt.Errorf("issuer does not match the IDP metadata (expected %q)", sp.IDPMetadata.EntityID) + } + + return nil +} + + func firstSet(a, b string) string { if a == "" { return b From 947be3ce725795e1d79c198ccb07aeb24197998e Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Mon, 19 May 2025 14:06:38 +0100 Subject: [PATCH 6/9] fix: add missing Signature and SigAlg query params with single logout HTTTP-Redirect binding request --- service_provider.go | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/service_provider.go b/service_provider.go index 906abc1e..5fc0024e 100644 --- a/service_provider.go +++ b/service_provider.go @@ -1358,7 +1358,6 @@ func (sp *ServiceProvider) SignLogoutRequest(req *LogoutRequest) error { // MakeLogoutRequest produces a new LogoutRequest object for idpURL. func (sp *ServiceProvider) MakeLogoutRequest(idpURL, nameID string) (*LogoutRequest, error) { - req := LogoutRequest{ ID: fmt.Sprintf("id-%x", randomBytes(20)), IssueInstant: TimeNow(), @@ -1375,11 +1374,7 @@ func (sp *ServiceProvider) MakeLogoutRequest(idpURL, nameID string) (*LogoutRequ SPNameQualifier: sp.Metadata().EntityID, }, } - if sp.SignatureMethod != "" { - if err := sp.SignLogoutRequest(&req); err != nil { - return nil, err - } - } + return &req, nil } @@ -1391,11 +1386,12 @@ func (sp *ServiceProvider) MakeRedirectLogoutRequest(nameID, relayState string) if err != nil { return nil, err } - return req.Redirect(relayState), nil + + return req.Redirect(relayState, sp) } // Redirect returns a URL suitable for using the redirect binding with the request -func (r *LogoutRequest) Redirect(relayState string) *url.URL { +func (r *LogoutRequest) Redirect(relayState string, sp *ServiceProvider) (*url.URL, error) { w := &bytes.Buffer{} w1 := base64.NewEncoder(base64.StdEncoding, w) w2, _ := flate.NewWriter(w1, 9) @@ -1413,14 +1409,29 @@ func (r *LogoutRequest) Redirect(relayState string) *url.URL { rv, _ := url.Parse(r.Destination) - query := rv.Query() - query.Set("SAMLRequest", w.String()) + // We can't depend on Query().set() as order matters for signing + query := rv.RawQuery + if len(query) > 0 { + query += "&" + string(SAMLRequest) + "=" + url.QueryEscape(w.String()) + } else { + query += string(SAMLRequest) + "=" + url.QueryEscape(w.String()) + } + if relayState != "" { - query.Set("RelayState", relayState) + query += "&RelayState=" + relayState } - rv.RawQuery = query.Encode() - return rv + if sp.SignatureMethod != "" { + var err error + query, err = sp.signQuery(SAMLRequest, query, w.String(), relayState) + if err != nil { + return nil, fmt.Errorf("logout request - redirect binding - failed to sign query: %v", err) + } + } + + rv.RawQuery = query + + return rv, nil } // MakePostLogoutRequest creates a SAML authentication request using @@ -1431,6 +1442,13 @@ func (sp *ServiceProvider) MakePostLogoutRequest(nameID, relayState string) ([]b if err != nil { return nil, err } + + if sp.SignatureMethod != "" { + if err := sp.SignLogoutRequest(req); err != nil { + return nil, err + } + } + return req.Post(relayState), nil } From 8c5d566f1aaf80b1fdfcb00c91daee9ddb1eeff3 Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Mon, 19 May 2025 14:35:14 +0100 Subject: [PATCH 7/9] lint --- service_provider.go | 1 - 1 file changed, 1 deletion(-) diff --git a/service_provider.go b/service_provider.go index 5fc0024e..1b30bd06 100644 --- a/service_provider.go +++ b/service_provider.go @@ -1873,7 +1873,6 @@ func (sp *ServiceProvider) validateLogoutRequest(req *LogoutRequest) error { return nil } - func firstSet(a, b string) string { if a == "" { return b From 21f11b511b18f4e12abd1936532aaf07a78df49e Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Tue, 27 May 2025 16:10:25 +0100 Subject: [PATCH 8/9] fix: returns LogoutRequest after validation --- service_provider.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/service_provider.go b/service_provider.go index 1b30bd06..d93ad843 100644 --- a/service_provider.go +++ b/service_provider.go @@ -1757,22 +1757,22 @@ func (sp *ServiceProvider) validateLogoutResponse(resp *LogoutResponse) error { } // ValidateLogoutRequest validates the LogoutRequest content from the request -func (sp *ServiceProvider) ValidateLogoutRequest(req *http.Request) error { +func (sp *ServiceProvider) ValidateLogoutRequest(req *http.Request) (*LogoutRequest, error) { query := req.URL.Query() if data := query.Get(string(SAMLRequest)); data != "" { - return sp.ValidateLogoutRequestRedirect(req) + return sp.ValidateLogoutRequestRedirect(req) } err := req.ParseForm() if err != nil { - return fmt.Errorf("validateLogoutRequest: unable to parse form: %v", err) + return nil, fmt.Errorf("validateLogoutRequest: unable to parse form: %v", err) } return sp.ValidateLogoutRequestForm(req.PostForm.Get(string(SAMLRequest))) } // ValidateLogoutRequestRedirect returns a nil error if the logout request is valid. This is used for the HTTP Redirect binding. -func (sp *ServiceProvider) ValidateLogoutRequestRedirect(r *http.Request) error { +func (sp *ServiceProvider) ValidateLogoutRequestRedirect(r *http.Request) (*LogoutRequest, error) { query := r.URL.Query() queryParameterData := query.Get(string(SAMLRequest)) retErr := &InvalidResponseError{ @@ -1782,44 +1782,44 @@ func (sp *ServiceProvider) ValidateLogoutRequestRedirect(r *http.Request) error rawRequestBuf, err := base64.StdEncoding.DecodeString(queryParameterData) if err != nil { retErr.PrivateErr = fmt.Errorf("validateLogoutRequestRedirect: unable to parse base64: %s", err) - return retErr + return nil, retErr } retErr.Response = string(rawRequestBuf) gr, err := io.ReadAll(newSaferFlateReader(bytes.NewBuffer(rawRequestBuf))) if err != nil { retErr.PrivateErr = err - return retErr + return nil, retErr } if err := xrv.Validate(bytes.NewReader(gr)); err != nil { - return fmt.Errorf("validateLogoutRequestRedirect: response contains invalid XML: %s", err) + return nil, fmt.Errorf("validateLogoutRequestRedirect: response contains invalid XML: %s", err) } if query.Get("Signature") != "" && query.Get("SigAlg") != "" { if err := sp.validateRedirectBindingSignature(r); err != nil { retErr.PrivateErr = err - return retErr + return nil, retErr } } doc := etree.NewDocument() if err := doc.ReadFromBytes(gr); err != nil { retErr.PrivateErr = err - return retErr + return nil, retErr } var req LogoutRequest if err := unmarshalElement(doc.Root(), &req); err != nil { retErr.PrivateErr = err - return retErr + return nil, retErr } - return sp.validateLogoutRequest(&req) + return &req, sp.validateLogoutRequest(&req) } // ValidateLogoutRequestForm returns a nil error if the logout request is valid. This is used for the HTTP POST binding. -func (sp *ServiceProvider) ValidateLogoutRequestForm(postFormData string) error { +func (sp *ServiceProvider) ValidateLogoutRequestForm(postFormData string) (*LogoutRequest, error) { retErr := &InvalidResponseError{ Now: TimeNow(), } @@ -1827,32 +1827,32 @@ func (sp *ServiceProvider) ValidateLogoutRequestForm(postFormData string) error rawRequestBuf, err := base64.StdEncoding.DecodeString(postFormData) if err != nil { retErr.PrivateErr = fmt.Errorf("unable to parse base64: %s", err) - return retErr + return nil, retErr } retErr.Response = string(rawRequestBuf) if err := xrv.Validate(bytes.NewReader(rawRequestBuf)); err != nil { - return fmt.Errorf("logout request contains invalid XML: %s", err) + return nil, fmt.Errorf("logout request contains invalid XML: %s", err) } doc := etree.NewDocument() if err := doc.ReadFromBytes(rawRequestBuf); err != nil { retErr.PrivateErr = err - return retErr + return nil, retErr } if err := sp.validateSignature(doc.Root()); err != nil { retErr.PrivateErr = err - return retErr + return nil, retErr } var req LogoutRequest if err := unmarshalElement(doc.Root(), &req); err != nil { retErr.PrivateErr = err - return retErr + return nil, retErr } - return sp.validateLogoutRequest(&req) + return &req, sp.validateLogoutRequest(&req) } // validateLogoutRequest validates the LogoutRequest fields. Returns a nil error if the LogoutRequest is valid. From 54244ce036b496e4d639e8801b5ead84ce48b2d7 Mon Sep 17 00:00:00 2001 From: "sandeep.gs" Date: Tue, 27 May 2025 16:15:31 +0100 Subject: [PATCH 9/9] lint --- service_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service_provider.go b/service_provider.go index d93ad843..81df91d6 100644 --- a/service_provider.go +++ b/service_provider.go @@ -1760,7 +1760,7 @@ func (sp *ServiceProvider) validateLogoutResponse(resp *LogoutResponse) error { func (sp *ServiceProvider) ValidateLogoutRequest(req *http.Request) (*LogoutRequest, error) { query := req.URL.Query() if data := query.Get(string(SAMLRequest)); data != "" { - return sp.ValidateLogoutRequestRedirect(req) + return sp.ValidateLogoutRequestRedirect(req) } err := req.ParseForm()