From 0b7ecb6c3c15e8312a23b30925117300293b7bdc Mon Sep 17 00:00:00 2001 From: Jian Qiu Date: Tue, 8 Sep 2026 20:04:56 +0800 Subject: [PATCH] fix(identity): check SAML assertion issuer, audience, and optional signature (#25596) Co-authored-by: Qiu Jian Co-authored-by: Cursor --- pkg/apis/identity/saml.go | 3 + pkg/keystone/driver/saml/class.go | 11 + pkg/keystone/driver/saml/saml.go | 38 +++ pkg/util/samlutils/sp/idp.go | 14 +- pkg/util/samlutils/sp/sp.go | 18 +- pkg/util/samlutils/sp/validate.go | 453 +++++++++++++++++++++++++ pkg/util/samlutils/sp/validate_test.go | 267 +++++++++++++++ 7 files changed, 798 insertions(+), 6 deletions(-) create mode 100644 pkg/util/samlutils/sp/validate.go create mode 100644 pkg/util/samlutils/sp/validate_test.go diff --git a/pkg/apis/identity/saml.go b/pkg/apis/identity/saml.go index ce6d760ef8..3713917d5a 100644 --- a/pkg/apis/identity/saml.go +++ b/pkg/apis/identity/saml.go @@ -34,6 +34,9 @@ type SIdpAttributeOptions struct { type SSAMLIdpBaseConfigOptions struct { AllowIdpInit *bool `json:"allow_idp_init"` + // VerifySignature, when true, checks the SAML response signature with SigningCert. + VerifySignature *bool `json:"verify_signature"` + SigningCert string `json:"signing_cert"` } type SSAMLIdpConfigOptions struct { diff --git a/pkg/keystone/driver/saml/class.go b/pkg/keystone/driver/saml/class.go index 142703f252..cd96c2c2eb 100644 --- a/pkg/keystone/driver/saml/class.go +++ b/pkg/keystone/driver/saml/class.go @@ -17,6 +17,7 @@ package saml import ( "context" "net/url" + "strings" "yunion.io/x/jsonutils" "yunion.io/x/pkg/errors" @@ -27,6 +28,7 @@ import ( "yunion.io/x/onecloud/pkg/keystone/driver/utils" "yunion.io/x/onecloud/pkg/keystone/models" "yunion.io/x/onecloud/pkg/mcclient" + "yunion.io/x/onecloud/pkg/util/samlutils/sp" ) type SSAMLDriverClass struct{} @@ -117,6 +119,15 @@ func (self *SSAMLDriverClass) ValidateConfig(ctx context.Context, userCred mccli return tconf, errors.Wrapf(httperrors.ErrDuplicateResource, "entity_id %s has been registered", conf.EntityId) } } + verifySignature := conf.VerifySignature != nil && *conf.VerifySignature + if verifySignature && len(strings.TrimSpace(conf.SigningCert)) == 0 { + return tconf, errors.Wrap(httperrors.ErrInputParameter, "empty signing_cert") + } + if len(strings.TrimSpace(conf.SigningCert)) > 0 { + if _, err = sp.ParseCertificates(conf.SigningCert); err != nil { + return tconf, errors.Wrap(httperrors.ErrInputParameter, "invalid signing_cert") + } + } conf.SIdpAttributeOptions, err = utils.ValidateConfig(ctx, conf.SIdpAttributeOptions, userCred) if err != nil { return tconf, errors.Wrap(err, "ValidateConfig") diff --git a/pkg/keystone/driver/saml/saml.go b/pkg/keystone/driver/saml/saml.go index 7eed960f28..f179a2aad7 100644 --- a/pkg/keystone/driver/saml/saml.go +++ b/pkg/keystone/driver/saml/saml.go @@ -16,8 +16,10 @@ package saml import ( "context" + "crypto/rsa" "encoding/base64" "fmt" + "os" "yunion.io/x/jsonutils" "yunion.io/x/log" @@ -29,9 +31,11 @@ import ( "yunion.io/x/onecloud/pkg/httperrors" "yunion.io/x/onecloud/pkg/keystone/driver" "yunion.io/x/onecloud/pkg/keystone/models" + "yunion.io/x/onecloud/pkg/keystone/options" "yunion.io/x/onecloud/pkg/keystone/saml" "yunion.io/x/onecloud/pkg/mcclient" "yunion.io/x/onecloud/pkg/util/samlutils/sp" + "yunion.io/x/onecloud/pkg/util/seclib2" ) // SAML 2.0 Service Provider Driver @@ -125,6 +129,29 @@ func (self *SSAMLDriver) Authenticate(ctx context.Context, ident mcclient.SAuthe return nil, errors.Wrap(httperrors.ErrInvalidCredential, "SAML auth unsuccess") } + verifyOpts := sp.SAMLVerifyOptions{ + IdpEntityId: self.samlConfig.EntityId, + SpEntityId: saml.SAMLInstance().GetEntityId(), + VerifySignature: self.samlConfig.VerifySignature != nil && *self.samlConfig.VerifySignature, + } + if verifyOpts.VerifySignature { + certs, err := sp.ParseCertificates(self.samlConfig.SigningCert) + if err != nil { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "idp signing certificate") + } + verifyOpts.Certs = certs + if resp.EncryptedAssertion != nil { + verifyOpts.DecryptKey, err = loadSPPrivateKey() + if err != nil { + return nil, errors.Wrap(err, "load SP private key") + } + } + } + err = sp.VerifySAMLResponse(samlRespBytes, resp, verifyOpts) + if err != nil { + return nil, err + } + attrs := resp.FetchAttribtues() var domainId, domainName, usrId, usrName string @@ -169,3 +196,14 @@ func (self *SSAMLDriver) Sync(ctx context.Context) error { func (self *SSAMLDriver) Probe(ctx context.Context) error { return nil } + +func loadSPPrivateKey() (*rsa.PrivateKey, error) { + if len(options.Options.SslKeyfile) == 0 { + return nil, errors.Wrap(httperrors.ErrInputParameter, "Missing ssl-keyfile") + } + privData, err := os.ReadFile(options.Options.SslKeyfile) + if err != nil { + return nil, errors.Wrap(err, "ReadFile ssl-keyfile") + } + return seclib2.DecodePrivateKey(privData) +} diff --git a/pkg/util/samlutils/sp/idp.go b/pkg/util/samlutils/sp/idp.go index 56077914ed..74ee02acd3 100644 --- a/pkg/util/samlutils/sp/idp.go +++ b/pkg/util/samlutils/sp/idp.go @@ -15,6 +15,7 @@ package sp import ( + "crypto/x509" "net/url" "yunion.io/x/pkg/errors" @@ -26,6 +27,7 @@ import ( type SSAMLIdentityProvider struct { entityId string redirectSsoUrl string + signingCerts []x509.Certificate } func NewSAMLIdp(entityId, redirectSsoUrl string) *SSAMLIdentityProvider { @@ -41,13 +43,23 @@ func NewSAMLIdpFromDescriptor(desc samlutils.EntityDescriptor) (*SSAMLIdentityPr return nil, errors.Wrap(httperrors.ErrInputParameter, "missing IDPSSODescriptor") } redirectSsoUrl := findSSOUrl(desc, samlutils.BINDING_HTTP_REDIRECT) - return NewSAMLIdp(entityId, redirectSsoUrl), nil + idp := NewSAMLIdp(entityId, redirectSsoUrl) + idp.signingCerts = CertificatesFromIdpDescriptor(desc) + return idp, nil } func (idp *SSAMLIdentityProvider) GetEntityId() string { return idp.entityId } +func (idp *SSAMLIdentityProvider) SetSigningCerts(certs []x509.Certificate) { + idp.signingCerts = append([]x509.Certificate{}, certs...) +} + +func (idp *SSAMLIdentityProvider) GetSigningCerts() []x509.Certificate { + return idp.signingCerts +} + func findSSOUrl(desc samlutils.EntityDescriptor, binding string) string { for _, v := range desc.IDPSSODescriptor.SingleSignOnServices { if v.Binding == binding { diff --git a/pkg/util/samlutils/sp/sp.go b/pkg/util/samlutils/sp/sp.go index 33d17cdd7e..82288f8504 100644 --- a/pkg/util/samlutils/sp/sp.go +++ b/pkg/util/samlutils/sp/sp.go @@ -233,20 +233,28 @@ func (sp *SSAMLSpInstance) processAssertionConsumer(ctx context.Context, w http. return errors.Wrap(err, "saml.UnmarshalResponse") } - /*_, err = samlutils.ValidateXML(string(samlRespBytes)) - if err != nil { - return errors.Wrap(err, "ValidateXML") - }*/ - if !samlResp.IsSuccess() { return errors.Wrapf(httperrors.ErrInvalidCredential, "SAML authenticate fail: %s", samlResp.Status.StatusCode.Value) } idp := sp.getIdentityProvider(samlResp.Issuer.Issuer) + if idp == nil && samlResp.Assertion != nil { + idp = sp.getIdentityProvider(samlResp.Assertion.Issuer.Issuer) + } if idp == nil { return errors.Wrapf(httperrors.ErrResourceNotFound, "issuer %s not found", samlResp.Issuer.Issuer) } + err = VerifySAMLResponse(samlRespBytes, samlResp, SAMLVerifyOptions{ + IdpEntityId: idp.GetEntityId(), + SpEntityId: sp.saml.GetEntityId(), + Recipient: sp.getAssertionConsumerUrl(), + Certs: idp.GetSigningCerts(), + }) + if err != nil { + return err + } + result := SSAMLAssertionConsumeResult{} if samlResp.InResponseTo != nil { diff --git a/pkg/util/samlutils/sp/validate.go b/pkg/util/samlutils/sp/validate.go new file mode 100644 index 0000000000..486abb5c38 --- /dev/null +++ b/pkg/util/samlutils/sp/validate.go @@ -0,0 +1,453 @@ +// Copyright 2019 Yunion +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sp + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "crypto/rsa" + "crypto/sha1" + "crypto/x509" + "encoding/base64" + "encoding/pem" + "encoding/xml" + "hash" + "strings" + "time" + "unicode" + + "github.com/beevik/etree" + "github.com/ma314smith/signedxml" + + "yunion.io/x/pkg/errors" + "yunion.io/x/pkg/util/samlutils" + "yunion.io/x/pkg/util/timeutils" + + "yunion.io/x/onecloud/pkg/httperrors" +) + +const defaultSAMLClockSkew = 5 * time.Minute + +type SAMLVerifyOptions struct { + IdpEntityId string + SpEntityId string + Recipient string + Certs []x509.Certificate + DecryptKey *rsa.PrivateKey + Now time.Time + ClockSkew time.Duration + VerifySignature bool +} + +func ParseCertificates(certStr string) ([]x509.Certificate, error) { + certStr = strings.TrimSpace(certStr) + if len(certStr) == 0 { + return nil, errors.Wrap(httperrors.ErrInputParameter, "empty signing_cert") + } + + certs := make([]x509.Certificate, 0) + rest := []byte(certStr) + for { + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + break + } + if block.Type != "CERTIFICATE" { + continue + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, errors.Wrap(err, "parse certificate") + } + certs = append(certs, *cert) + } + if len(certs) > 0 { + return certs, nil + } + + der := []byte(certStr) + if decoded, err := base64.StdEncoding.DecodeString(stripAllSpace(certStr)); err == nil && len(decoded) > 0 { + der = decoded + } + cert, err := x509.ParseCertificate(der) + if err != nil { + return nil, errors.Wrap(httperrors.ErrInputParameter, "invalid signing_cert") + } + return []x509.Certificate{*cert}, nil +} + +func CertificatesFromIdpDescriptor(desc samlutils.EntityDescriptor) []x509.Certificate { + if desc.IDPSSODescriptor == nil { + return nil + } + certs := make([]x509.Certificate, 0) + for _, kd := range desc.IDPSSODescriptor.KeyDescriptors { + if len(kd.Use) > 0 && kd.Use != samlutils.KEY_USE_SIGNING { + continue + } + if kd.KeyInfo.X509Data == nil { + continue + } + parsed, err := ParseCertificates(kd.KeyInfo.X509Data.X509Certificate.Cert) + if err != nil { + continue + } + certs = append(certs, parsed...) + } + return certs +} + +func VerifySAMLResponse(xmlBytes []byte, resp *samlutils.Response, opts SAMLVerifyOptions) error { + if resp == nil { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing SAML response") + } + if len(strings.TrimSpace(opts.IdpEntityId)) == 0 { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing IdP entity id") + } + if len(strings.TrimSpace(opts.SpEntityId)) == 0 { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing SP entity id") + } + + if opts.VerifySignature { + if len(opts.Certs) == 0 { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing IdP signing certificate") + } + signedXML, err := signedDocument(xmlBytes, resp, opts.DecryptKey) + if err != nil { + return err + } + referenced, err := validateXMLWithCertificates(signedXML, opts.Certs) + if err != nil { + return err + } + if err := bindSignedAssertion(resp, referenced); err != nil { + return err + } + } + + if err := checkIssuers(resp, opts.IdpEntityId); err != nil { + return err + } + if err := checkAudience(resp, opts.SpEntityId); err != nil { + return err + } + if err := checkConditions(resp, opts); err != nil { + return err + } + if err := checkDestination(resp, opts.Recipient); err != nil { + return err + } + return nil +} + +func signedDocument(xmlBytes []byte, resp *samlutils.Response, decryptKey *rsa.PrivateKey) (string, error) { + if hasEnvelopedSignature(xmlBytes) { + return string(xmlBytes), nil + } + if resp.EncryptedAssertion == nil { + return "", errors.Wrap(httperrors.ErrInvalidCredential, "unsigned SAML response") + } + if decryptKey == nil { + return "", errors.Wrap(httperrors.ErrInvalidCredential, "unsigned SAML response") + } + plain, err := decryptEncryptedAssertion(resp.EncryptedAssertion, decryptKey) + if err != nil { + return "", errors.Wrap(err, "decrypt assertion") + } + if !hasEnvelopedSignature(plain) { + return "", errors.Wrap(httperrors.ErrInvalidCredential, "unsigned SAML assertion") + } + return string(plain), nil +} + +func hasEnvelopedSignature(xmlBytes []byte) bool { + doc := etree.NewDocument() + if err := doc.ReadFromBytes(xmlBytes); err != nil { + return false + } + return doc.FindElement(".//Signature") != nil +} + +func validateXMLWithCertificates(signed string, certs []x509.Certificate) ([]string, error) { + validator, err := signedxml.NewValidator(signed) + if err != nil { + return nil, errors.Wrap(err, "signedxml.NewValidator") + } + validator.Certificates = append([]x509.Certificate{}, certs...) + referenced, err := validator.ValidateReferences() + if err != nil { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "invalid SAML signature") + } + if len(referenced) == 0 { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "invalid SAML signature") + } + return referenced, nil +} + +func bindSignedAssertion(resp *samlutils.Response, referenced []string) error { + if resp.Assertion == nil || len(strings.TrimSpace(resp.Assertion.ID)) == 0 { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing assertion") + } + wantId := strings.TrimSpace(resp.Assertion.ID) + for _, refXML := range referenced { + doc := etree.NewDocument() + if err := doc.ReadFromString(refXML); err != nil { + continue + } + el := findElementByID(doc, wantId) + if el == nil { + continue + } + if !strings.EqualFold(el.Tag, "Assertion") { + continue + } + frag := etree.NewDocument() + frag.SetRoot(el.Copy()) + xmlStr, err := frag.WriteToString() + if err != nil { + return errors.Wrap(err, "write signed assertion") + } + assertion := samlutils.Assertion{} + if err := xml.Unmarshal([]byte(xmlStr), &assertion); err != nil { + return errors.Wrap(err, "unmarshal signed assertion") + } + resp.Assertion = &assertion + return nil + } + return errors.Wrap(httperrors.ErrInvalidCredential, "assertion is not signed") +} + +func findElementByID(doc *etree.Document, id string) *etree.Element { + if doc.Root() == nil || len(id) == 0 { + return nil + } + if doc.Root().SelectAttrValue("ID", "") == id { + return doc.Root() + } + return doc.FindElement(".//[@ID='" + id + "']") +} + +func checkIssuers(resp *samlutils.Response, idpEntityId string) error { + want := strings.TrimSpace(idpEntityId) + if resp.Assertion == nil { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing assertion") + } + got := strings.TrimSpace(resp.Assertion.Issuer.Issuer) + if got != want { + return errors.Wrap(httperrors.ErrInvalidCredential, "issuer mismatch") + } + if len(strings.TrimSpace(resp.Issuer.Issuer)) > 0 && strings.TrimSpace(resp.Issuer.Issuer) != want { + return errors.Wrap(httperrors.ErrInvalidCredential, "issuer mismatch") + } + return nil +} + +func checkAudience(resp *samlutils.Response, spEntityId string) error { + if resp.Assertion == nil { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing assertion") + } + want := strings.TrimSpace(spEntityId) + restrictions := resp.Assertion.Conditions.AudienceRestrictions + if len(restrictions) == 0 { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing audience") + } + for _, restriction := range restrictions { + if strings.TrimSpace(restriction.Audience.Value) == want { + return nil + } + } + return errors.Wrap(httperrors.ErrInvalidCredential, "audience mismatch") +} + +func checkConditions(resp *samlutils.Response, opts SAMLVerifyOptions) error { + now := opts.Now + if now.IsZero() { + now = time.Now().UTC() + } + skew := opts.ClockSkew + if skew <= 0 { + skew = defaultSAMLClockSkew + } + if resp.Assertion == nil { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing assertion") + } + + hasExpiry := false + cond := resp.Assertion.Conditions + if err := checkTimeWindow(cond.NotBefore, cond.NotOnOrAfter, now, skew, &hasExpiry); err != nil { + return err + } + scd := resp.Assertion.Subject.SubjectConfirmation.SubjectConfirmationData + if err := checkTimeWindow(scd.NotBefore, scd.NotOnOrAfter, now, skew, &hasExpiry); err != nil { + return err + } + if !hasExpiry { + return errors.Wrap(httperrors.ErrInvalidCredential, "missing NotOnOrAfter") + } + if len(opts.Recipient) > 0 && len(strings.TrimSpace(scd.Recipient)) > 0 && + strings.TrimSpace(scd.Recipient) != strings.TrimSpace(opts.Recipient) { + return errors.Wrap(httperrors.ErrInvalidCredential, "recipient mismatch") + } + return nil +} + +func checkDestination(resp *samlutils.Response, recipient string) error { + if len(recipient) == 0 || len(strings.TrimSpace(resp.Destination)) == 0 { + return nil + } + if strings.TrimSpace(resp.Destination) != strings.TrimSpace(recipient) { + return errors.Wrap(httperrors.ErrInvalidCredential, "destination mismatch") + } + return nil +} + +func checkTimeWindow(notBefore *string, notOnOrAfter string, now time.Time, skew time.Duration, hasExpiry *bool) error { + if notBefore != nil && len(strings.TrimSpace(*notBefore)) > 0 { + t, err := parseSAMLTime(*notBefore) + if err != nil { + return errors.Wrap(httperrors.ErrInvalidCredential, "invalid NotBefore") + } + if now.Add(skew).Before(t) { + return errors.Wrap(httperrors.ErrInvalidCredential, "assertion not yet valid") + } + } + if len(strings.TrimSpace(notOnOrAfter)) == 0 { + return nil + } + t, err := parseSAMLTime(notOnOrAfter) + if err != nil { + return errors.Wrap(httperrors.ErrInvalidCredential, "invalid NotOnOrAfter") + } + *hasExpiry = true + if !now.Add(-skew).Before(t) { + return errors.Wrap(httperrors.ErrInvalidCredential, "assertion expired") + } + return nil +} + +func parseSAMLTime(s string) (time.Time, error) { + s = strings.TrimSpace(s) + if t, err := timeutils.ParseTimeStr(s); err == nil { + return t, nil + } + if t, err := time.Parse(time.RFC3339, s); err == nil { + return t, nil + } + if t, err := time.Parse(time.RFC3339Nano, s); err == nil { + return t, nil + } + return time.Time{}, errors.Wrap(httperrors.ErrInvalidCredential, "invalid time") +} + +func stripAllSpace(s string) string { + return strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, s) +} + +func decryptEncryptedAssertion(enc *samlutils.EncryptedAssertion, privateKey *rsa.PrivateKey) ([]byte, error) { + if enc == nil || privateKey == nil { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "missing encrypted assertion") + } + data := enc.EncryptedData + cipherText, err := base64.StdEncoding.DecodeString(strings.TrimSpace(data.CipherData.CipherValue.Value)) + if err != nil { + return nil, errors.Wrap(err, "decode encrypted data") + } + if data.KeyInfo.EncryptedKey == nil { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "missing encrypted key") + } + key, err := decryptEncryptedKey(*data.KeyInfo.EncryptedKey, privateKey) + if err != nil { + return nil, err + } + switch data.EncryptionMethod.Algorithm { + case "http://www.w3.org/2001/04/xmlenc#aes128-cbc", + "http://www.w3.org/2001/04/xmlenc#aes192-cbc", + "http://www.w3.org/2001/04/xmlenc#aes256-cbc": + plain, err := decryptAesCbc(key, cipherText) + if err != nil { + return nil, err + } + return stripPKCS7(plain), nil + default: + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "unsupported encryption algorithm") + } +} + +func decryptEncryptedKey(key samlutils.EncryptedKey, privateKey *rsa.PrivateKey) ([]byte, error) { + cipherText, err := base64.StdEncoding.DecodeString(strings.TrimSpace(key.CipherData.CipherValue.Value)) + if err != nil { + return nil, errors.Wrap(err, "decode encrypted key") + } + if key.EncryptionMethod.Algorithm != "http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p" { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "unsupported key encryption") + } + var shaAlg hash.Hash = sha1.New() + if key.EncryptionMethod.DigestMethod != nil && + len(key.EncryptionMethod.DigestMethod.Algorithm) > 0 && + key.EncryptionMethod.DigestMethod.Algorithm != "http://www.w3.org/2000/09/xmldsig#sha1" { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "unsupported key digest") + } + plaintext, err := rsa.DecryptOAEP(shaAlg, rand.Reader, privateKey, cipherText, nil) + if err != nil { + return nil, errors.Wrap(err, "decrypt key") + } + return plaintext, nil +} + +func decryptAesCbc(key []byte, secret []byte) ([]byte, error) { + if len(secret) < aes.BlockSize { + return nil, errors.Wrap(httperrors.ErrInvalidCredential, "invalid encrypted assertion") + } + c, err := aes.NewCipher(key) + if err != nil { + return nil, errors.Wrap(err, "aes.NewCipher") + } + decrypter := cipher.NewCBCDecrypter(c, secret[0:aes.BlockSize]) + data := make([]byte, len(secret)-aes.BlockSize) + copy(data, secret[aes.BlockSize:]) + decrypter.CryptBlocks(data, data) + return data, nil +} + +func stripPKCS7(data []byte) []byte { + if len(data) == 0 { + return data + } + pad := int(data[len(data)-1]) + if pad <= 0 || pad > aes.BlockSize || pad > len(data) { + return bytesTrimRightNull(data) + } + for i := 0; i < pad; i++ { + if int(data[len(data)-1-i]) != pad { + return bytesTrimRightNull(data) + } + } + return data[:len(data)-pad] +} + +func bytesTrimRightNull(data []byte) []byte { + i := len(data) + for i > 0 && data[i-1] == 0 { + i-- + } + return data[:i] +} diff --git a/pkg/util/samlutils/sp/validate_test.go b/pkg/util/samlutils/sp/validate_test.go new file mode 100644 index 0000000000..eb4e23143f --- /dev/null +++ b/pkg/util/samlutils/sp/validate_test.go @@ -0,0 +1,267 @@ +// Copyright 2019 Yunion +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sp + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "encoding/xml" + "math/big" + "strings" + "testing" + "time" + + "yunion.io/x/pkg/util/samlutils" +) + +const ( + testIdpEntityId = "https://idp.example.com/saml" + testSpEntityId = "https://sp.example.com" + testACS = "https://sp.example.com/acs" +) + +func TestParseCertificates(t *testing.T) { + _, cert, pemStr := mustGenCert(t) + parsed, err := ParseCertificates(pemStr) + if err != nil { + t.Fatalf("ParseCertificates pem: %v", err) + } + if len(parsed) != 1 || parsed[0].SerialNumber.Cmp(cert.SerialNumber) != 0 { + t.Fatalf("unexpected pem cert") + } + + raw := strings.TrimSpace(strings.ReplaceAll(pemStr, "-----BEGIN CERTIFICATE-----", "")) + raw = strings.TrimSpace(strings.ReplaceAll(raw, "-----END CERTIFICATE-----", "")) + parsed, err = ParseCertificates(raw) + if err != nil { + t.Fatalf("ParseCertificates raw base64: %v", err) + } + if len(parsed) != 1 { + t.Fatalf("expected 1 cert, got %d", len(parsed)) + } + + if _, err := ParseCertificates(""); err == nil { + t.Fatalf("empty cert should fail") + } + if _, err := ParseCertificates("not-a-cert"); err == nil { + t.Fatalf("invalid cert should fail") + } +} + +func TestVerifySAMLResponse(t *testing.T) { + key, cert, pemStr := mustGenCert(t) + _, otherCert, _ := mustGenCert(t) + + signed, resp := mustSignedResponse(t, key, pemStr, testIdpEntityId, testSpEntityId, time.Now().UTC()) + unsigned, unsignedResp := mustUnsignedResponse(t, testIdpEntityId, testSpEntityId, time.Now().UTC()) + + baseOpts := SAMLVerifyOptions{ + IdpEntityId: testIdpEntityId, + SpEntityId: testSpEntityId, + Recipient: testACS, + Now: time.Now().UTC(), + } + if err := VerifySAMLResponse([]byte(unsigned), unsignedResp, baseOpts); err != nil { + t.Fatalf("unsigned response should pass without signature check: %v", err) + } + if err := VerifySAMLResponse([]byte(signed), resp, baseOpts); err != nil { + t.Fatalf("signed response should pass without signature check: %v", err) + } + + wrongIssuer, wrongIssuerResp := mustUnsignedResponse(t, "https://evil.example.com", testSpEntityId, time.Now().UTC()) + if err := VerifySAMLResponse([]byte(wrongIssuer), wrongIssuerResp, baseOpts); err == nil { + t.Fatalf("wrong issuer should fail") + } + + wrongAud, wrongAudResp := mustUnsignedResponse(t, testIdpEntityId, "https://other-sp.example.com", time.Now().UTC()) + if err := VerifySAMLResponse([]byte(wrongAud), wrongAudResp, baseOpts); err == nil { + t.Fatalf("wrong audience should fail") + } + + expired, expiredResp := mustUnsignedResponse(t, testIdpEntityId, testSpEntityId, time.Now().UTC()) + if err := VerifySAMLResponse([]byte(expired), expiredResp, SAMLVerifyOptions{ + IdpEntityId: testIdpEntityId, + SpEntityId: testSpEntityId, + Now: time.Now().UTC().Add(time.Hour), + }); err == nil { + t.Fatalf("expired assertion should fail") + } + + sigOpts := SAMLVerifyOptions{ + IdpEntityId: testIdpEntityId, + SpEntityId: testSpEntityId, + Recipient: testACS, + Certs: []x509.Certificate{*cert}, + Now: time.Now().UTC(), + VerifySignature: true, + } + if err := VerifySAMLResponse([]byte(signed), cloneResponse(t, signed), sigOpts); err != nil { + t.Fatalf("valid signed response: %v", err) + } + if err := VerifySAMLResponse([]byte(unsigned), unsignedResp, sigOpts); err == nil { + t.Fatalf("unsigned response should fail when signature check is enabled") + } + + if err := VerifySAMLResponse([]byte(signed), cloneResponse(t, signed), SAMLVerifyOptions{ + IdpEntityId: testIdpEntityId, + SpEntityId: testSpEntityId, + Certs: []x509.Certificate{*otherCert}, + Now: time.Now().UTC(), + VerifySignature: true, + }); err == nil { + t.Fatalf("wrong IdP certificate should fail") + } + + attackerKey, attackerCert, attackerPem := mustGenCert(t) + attackerSigned, attackerResp := mustSignedResponse(t, attackerKey, attackerPem, testIdpEntityId, testSpEntityId, time.Now().UTC()) + if err := VerifySAMLResponse([]byte(attackerSigned), attackerResp, SAMLVerifyOptions{ + IdpEntityId: testIdpEntityId, + SpEntityId: testSpEntityId, + Certs: []x509.Certificate{*cert}, + Now: time.Now().UTC(), + VerifySignature: true, + }); err == nil { + t.Fatalf("embedded attacker certificate should not be trusted") + } + _ = attackerCert +} + +func TestVerifySAMLResponseRejectsUnsignedAssertionWithSignedWrapper(t *testing.T) { + key, cert, pemStr := mustGenCert(t) + signed, _ := mustSignedResponse(t, key, pemStr, testIdpEntityId, testSpEntityId, time.Now().UTC()) + + var signedResp samlutils.Response + if err := xml.Unmarshal([]byte(signed), &signedResp); err != nil { + t.Fatalf("unmarshal signed: %v", err) + } + unsigned, unsignedResp := mustUnsignedResponse(t, testIdpEntityId, testSpEntityId, time.Now().UTC()) + _ = unsigned + unsignedResp.Assertion.ID = "_unsigned-assertion" + signedResp.Assertion = unsignedResp.Assertion + + opts := SAMLVerifyOptions{ + IdpEntityId: testIdpEntityId, + SpEntityId: testSpEntityId, + Certs: []x509.Certificate{*cert}, + Now: time.Now().UTC(), + VerifySignature: true, + } + if err := VerifySAMLResponse([]byte(signed), &signedResp, opts); err == nil { + t.Fatalf("unsigned substituted assertion should fail") + } +} + +func mustGenCert(t *testing.T) (*rsa.PrivateKey, *x509.Certificate, string) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("GenerateKey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: "idp.example.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + pemStr := string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})) + return key, cert, pemStr +} + +func mustUnsignedResponse(t *testing.T, idpEntityId, spEntityId string, now time.Time) (string, *samlutils.Response) { + t.Helper() + resp := samlutils.NewResponse(samlutils.SSAMLResponseInput{ + IssuerEntityId: idpEntityId, + RequestEntityId: spEntityId, + AssertionConsumerServiceURL: testACS, + SSAMLSpInitiatedLoginData: samlutils.SSAMLSpInitiatedLoginData{ + NameId: "user1", + NameIdFormat: samlutils.NAME_ID_FORMAT_UNSPEC, + AudienceRestriction: spEntityId, + Attributes: []samlutils.SSAMLResponseAttribute{ + {Name: "uid", Values: []string{"user1"}}, + }, + }, + }) + xmlBytes, err := xml.Marshal(&resp) + if err != nil { + t.Fatalf("xml.Marshal: %v", err) + } + out := samlutils.Response{} + if err := xml.Unmarshal(xmlBytes, &out); err != nil { + t.Fatalf("xml.Unmarshal: %v", err) + } + return string(xmlBytes), &out +} + +func mustSignedResponse(t *testing.T, key *rsa.PrivateKey, certPEM, idpEntityId, spEntityId string, now time.Time) (string, *samlutils.Response) { + t.Helper() + _ = now + block, _ := pem.Decode([]byte(certPEM)) + certB64 := "" + if block != nil { + certB64 = string(pem.EncodeToMemory(block)) + certB64 = strings.TrimPrefix(certB64, "-----BEGIN CERTIFICATE-----") + certB64 = strings.TrimSuffix(strings.TrimSpace(certB64), "-----END CERTIFICATE-----") + certB64 = strings.TrimSpace(certB64) + } + resp := samlutils.NewResponse(samlutils.SSAMLResponseInput{ + IssuerEntityId: idpEntityId, + RequestEntityId: spEntityId, + AssertionConsumerServiceURL: testACS, + IssuerCertString: certB64, + SSAMLSpInitiatedLoginData: samlutils.SSAMLSpInitiatedLoginData{ + NameId: "user1", + NameIdFormat: samlutils.NAME_ID_FORMAT_UNSPEC, + AudienceRestriction: spEntityId, + Attributes: []samlutils.SSAMLResponseAttribute{ + {Name: "uid", Values: []string{"user1"}}, + }, + }, + }) + xmlBytes, err := xml.Marshal(&resp) + if err != nil { + t.Fatalf("xml.Marshal: %v", err) + } + signed, err := samlutils.SignXML(string(xmlBytes), key) + if err != nil { + t.Fatalf("SignXML: %v", err) + } + out := samlutils.Response{} + if err := xml.Unmarshal([]byte(signed), &out); err != nil { + t.Fatalf("xml.Unmarshal signed: %v", err) + } + return signed, &out +} + +func cloneResponse(t *testing.T, xmlStr string) *samlutils.Response { + t.Helper() + out := samlutils.Response{} + if err := xml.Unmarshal([]byte(xmlStr), &out); err != nil { + t.Fatalf("xml.Unmarshal: %v", err) + } + return &out +}