fix(identity): check SAML assertion issuer, audience, and optional signature (#25596)

Co-authored-by: Qiu Jian <qiujian@yunionyun.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

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 {

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")

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)
+}

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 {

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 {

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]
+}

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
+}