Skip to content

Commit

Permalink
implement query signature verification
Browse files Browse the repository at this point in the history 
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 <ieva.vasiljeva@grafana.com>
  • Loading branch information
@Jguer @IevaVasiljeva
Jguer and IevaVasiljeva committed Jan 2, 2023
1 parent 2d6a792 commit 0002d15
Show file tree
Hide file tree
Showing 4 changed files with 374 additions and 15 deletions.
35 changes: 20 additions & 15 deletions service_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(),
}
Expand All @@ -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
Expand All @@ -1553,6 +1557,7 @@ func (sp *ServiceProvider) ValidateLogoutResponseRedirect(queryParameterData str
if err := sp.validateLogoutResponse(&resp); err != nil {
return err
}

return nil
}

Expand Down
141 changes: 141 additions & 0 deletions service_provider_signed.go
View file
Original file line number Diff line number Diff line change
@@ -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
}
117 changes: 117 additions & 0 deletions service_provider_signed_test.go
View file
Original file line number Diff line number Diff line change
@@ -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")
}

0 comments on commit 0002d15

Please sign in to comment.