Merge pull request #867 from ericchiang/xml-validation

glide.yaml: update goxmldsig

glide.lock

-hash: fbef1f81a0f86f519714bbe568692a92709c446ef487a3fa9875e58f86e14430
-updated: 2017-03-08T10:31:06.364335442-08:00
+hash: 4a3ac3a2a2b39357dc5de7ba296dbe20a97dd2fe13bc15a68e904e69afae2adb
+updated: 2017-03-21T09:24:08.089284929-07:00
 imports:
 - name: github.com/beevik/etree
   version: 4cd0dd976db869f817248477718071a28e978df0
@@ -46,7 +46,9 @@ imports:
   subpackages:
   - cacheobject
 - name: github.com/russellhaering/goxmldsig
-  version: e2990269f42f6ddfea940870a0800a14acdb8c21
+  version: 51810e925e5fc495822fbddda8202f70a6e4a3f3
+  subpackages:
+  - etreeutils
 - name: github.com/Sirupsen/logrus
   version: d26492970760ca5d33129d2d799e34be5c4782eb
 - name: github.com/spf13/cobra

glide.yaml

@@ -133,7 +133,7 @@ import:
 
 # XML signature validation for SAML connector
 - package: github.com/russellhaering/goxmldsig
-  version: e2990269f42f6ddfea940870a0800a14acdb8c21
+  version: 51810e925e5fc495822fbddda8202f70a6e4a3f3
 - package: github.com/beevik/etree
   version: 4cd0dd976db869f817248477718071a28e978df0
 - package: github.com/jonboulle/clockwork

vendor/github.com/russellhaering/goxmldsig/canonicalize.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/beevik/etree"
+	"github.com/russellhaering/goxmldsig/etreeutils"
 )
 
 // Canonicalizer is an implementation of a canonicalization algorithm.
@@ -67,41 +68,6 @@ func composeAttr(space, key string) string {
 	return key
 }
 
-type attrsByKey []etree.Attr
-
-func (a attrsByKey) Len() int {
-	return len(a)
-}
-
-func (a attrsByKey) Swap(i, j int) {
-	a[i], a[j] = a[j], a[i]
-}
-
-func (a attrsByKey) Less(i, j int) bool {
-	// As I understand it: any "xmlns" attribute should come first, followed by any
-	// any "xmlns:prefix" attributes, presumably ordered by prefix. Lastly any other
-	// attributes in lexicographical order.
-	if a[i].Space == "" && a[i].Key == "xmlns" {
-		return true
-	}
-	if a[j].Space == "" && a[j].Key == "xmlns" {
-		return false
-	}
-
-	if a[i].Space == "xmlns" {
-		if a[j].Space == "xmlns" {
-			return a[i].Key < a[j].Key
-		}
-		return true
-	}
-
-	if a[j].Space == "xmlns" {
-		return false
-	}
-
-	return composeAttr(a[i].Space, a[i].Key) < composeAttr(a[j].Space, a[j].Key)
-}
-
 type c14nSpace struct {
 	a    etree.Attr
 	used bool
@@ -192,7 +158,7 @@ func excCanonicalPrep(el *etree.Element, _nsAlreadyDeclared map[string]c14nSpace
 	}
 
 	//Sort attributes lexicographically
-	sort.Sort(attrsByKey(el.Attr))
+	sort.Sort(etreeutils.SortedAttrs(el.Attr))
 
 	return el.Copy()
 }
@@ -215,7 +181,7 @@ func canonicalPrep(el *etree.Element, seenSoFar map[string]struct{}) *etree.Elem
 	}
 
 	ne := el.Copy()
-	sort.Sort(attrsByKey(ne.Attr))
+	sort.Sort(etreeutils.SortedAttrs(ne.Attr))
 	if len(ne.Attr) != 0 {
 		for _, attr := range ne.Attr {
 			if attr.Space != nsSpace {

vendor/github.com/russellhaering/goxmldsig/etreeutils/namespace.go

+package etreeutils
+
+import (
+	"errors"
+
+	"fmt"
+
+	"sort"
+
+	"github.com/beevik/etree"
+)
+
+const (
+	defaultPrefix = ""
+	xmlnsPrefix   = "xmlns"
+	xmlPrefix     = "xml"
+
+	XMLNamespace   = "http://www.w3.org/XML/1998/namespace"
+	XMLNSNamespace = "http://www.w3.org/2000/xmlns/"
+)
+
+var (
+	DefaultNSContext = NSContext{
+		prefixes: map[string]string{
+			defaultPrefix: XMLNamespace,
+			xmlPrefix:     XMLNamespace,
+			xmlnsPrefix:   XMLNSNamespace,
+		},
+	}
+
+	EmptyNSContext = NSContext{}
+
+	ErrReservedNamespace       = errors.New("disallowed declaration of reserved namespace")
+	ErrInvalidDefaultNamespace = errors.New("invalid default namespace declaration")
+	ErrTraversalHalted         = errors.New("traversal halted")
+)
+
+type ErrUndeclaredNSPrefix struct {
+	Prefix string
+}
+
+func (e ErrUndeclaredNSPrefix) Error() string {
+	return fmt.Sprintf("undeclared namespace prefix: '%s'", e.Prefix)
+}
+
+type NSContext struct {
+	prefixes map[string]string
+}
+
+func (ctx NSContext) SubContext(el *etree.Element) (NSContext, error) {
+	// The subcontext should inherit existing declared prefixes
+	prefixes := make(map[string]string, len(ctx.prefixes)+4)
+	for k, v := range ctx.prefixes {
+		prefixes[k] = v
+	}
+
+	// Merge new namespace declarations on top of existing ones.
+	for _, attr := range el.Attr {
+		if attr.Space == xmlnsPrefix {
+			// This attribute is a namespace declaration of the form "xmlns:<prefix>"
+
+			// The 'xml' namespace may only be re-declared with the name 'http://www.w3.org/XML/1998/namespace'
+			if attr.Key == xmlPrefix && attr.Value != XMLNamespace {
+				return ctx, ErrReservedNamespace
+			}
+
+			// The 'xmlns' namespace may not be re-declared
+			if attr.Key == xmlnsPrefix {
+				return ctx, ErrReservedNamespace
+			}
+
+			prefixes[attr.Key] = attr.Value
+		} else if attr.Space == defaultPrefix && attr.Key == xmlnsPrefix {
+			// This attribute is a default namespace declaration
+
+			// The xmlns namespace value may not be declared as the default namespace
+			if attr.Value == XMLNSNamespace {
+				return ctx, ErrInvalidDefaultNamespace
+			}
+
+			prefixes[defaultPrefix] = attr.Value
+		}
+	}
+
+	return NSContext{prefixes: prefixes}, nil
+}
+
+// LookupPrefix attempts to find a declared namespace for the specified prefix. If the prefix
+// is an empty string this will be the default namespace for this context. If the prefix is
+// undeclared in this context an ErrUndeclaredNSPrefix will be returned.
+func (ctx NSContext) LookupPrefix(prefix string) (string, error) {
+	if namespace, ok := ctx.prefixes[prefix]; ok {
+		return namespace, nil
+	}
+
+	return "", ErrUndeclaredNSPrefix{
+		Prefix: prefix,
+	}
+}
+
+func nsTraverse(ctx NSContext, el *etree.Element, handle func(NSContext, *etree.Element) error) error {
+	ctx, err := ctx.SubContext(el)
+	if err != nil {
+		return err
+	}
+
+	err = handle(ctx, el)
+	if err != nil {
+		return err
+	}
+
+	// Recursively traverse child elements.
+	for _, child := range el.ChildElements() {
+		err := nsTraverse(ctx, child, handle)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func detachWithNamespaces(ctx NSContext, el *etree.Element) (*etree.Element, error) {
+	ctx, err := ctx.SubContext(el)
+	if err != nil {
+		return nil, err
+	}
+
+	el = el.Copy()
+
+	// Build a new attribute list
+	attrs := make([]etree.Attr, 0, len(el.Attr))
+
+	// First copy over anything that isn't a namespace declaration
+	for _, attr := range el.Attr {
+		if attr.Space == xmlnsPrefix {
+			continue
+		}
+
+		if attr.Space == defaultPrefix && attr.Key == xmlnsPrefix {
+			continue
+		}
+
+		attrs = append(attrs, attr)
+	}
+
+	// Append all in-context namespace declarations
+	for prefix, namespace := range ctx.prefixes {
+		// Skip the implicit "xml" and "xmlns" prefix declarations
+		if prefix == xmlnsPrefix || prefix == xmlPrefix {
+			continue
+		}
+
+		// Also skip declararing the default namespace as XMLNamespace
+		if prefix == defaultPrefix && namespace == XMLNamespace {
+			continue
+		}
+
+		if prefix != defaultPrefix {
+			attrs = append(attrs, etree.Attr{
+				Space: xmlnsPrefix,
+				Key:   prefix,
+				Value: namespace,
+			})
+		} else {
+			attrs = append(attrs, etree.Attr{
+				Key:   xmlnsPrefix,
+				Value: namespace,
+			})
+		}
+	}
+
+	sort.Sort(SortedAttrs(attrs))
+
+	el.Attr = attrs
+
+	return el, nil
+}
+
+// NSSelectOne conducts a depth-first search for an element with the specified namespace
+// and tag. If such an element is found, a new *etree.Element is returned which is a
+// copy of the found element, but with all in-context namespace declarations attached
+// to the element as attributes.
+func NSSelectOne(el *etree.Element, namespace, tag string) (*etree.Element, error) {
+	var found *etree.Element
+
+	err := nsTraverse(DefaultNSContext, el, func(ctx NSContext, el *etree.Element) error {
+		currentNS, err := ctx.LookupPrefix(el.Space)
+		if err != nil {
+			return err
+		}
+
+		// Base case, el is the sought after element.
+		if currentNS == namespace && el.Tag == tag {
+			found, err = detachWithNamespaces(ctx, el)
+			return ErrTraversalHalted
+		}
+
+		return nil
+	})
+
+	if err != nil && err != ErrTraversalHalted {
+		return nil, err
+	}
+
+	return found, nil
+}
+
+// NSFindIterate conducts a depth-first traversal searching for elements with the
+// specified tag in the specified namespace. For each such element, the passed
+// handler function is invoked. If the handler function returns an error
+// traversal is immediately halted. If the error returned by the handler is
+// ErrTraversalHalted then nil will be returned by NSFindIterate. If any other
+// error is returned by the handler, that error will be returned by NSFindIterate.
+func NSFindIterate(el *etree.Element, namespace, tag string, handle func(*etree.Element) error) error {
+	err := nsTraverse(DefaultNSContext, el, func(ctx NSContext, el *etree.Element) error {
+		currentNS, err := ctx.LookupPrefix(el.Space)
+		if err != nil {
+			return err
+		}
+
+		// Base case, el is the sought after element.
+		if currentNS == namespace && el.Tag == tag {
+			return handle(el)
+		}
+
+		return nil
+	})
+
+	if err != nil && err != ErrTraversalHalted {
+		return err
+	}
+
+	return nil
+}
+
+// NSFindOne conducts a depth-first search for the specified element. If such an element
+// is found a reference to it is returned.
+func NSFindOne(el *etree.Element, namespace, tag string) (*etree.Element, error) {
+	var found *etree.Element
+
+	err := NSFindIterate(el, namespace, tag, func(el *etree.Element) error {
+		found = el
+		return ErrTraversalHalted
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return found, nil
+}

vendor/github.com/russellhaering/goxmldsig/etreeutils/sort.go

+package etreeutils
+
+import "github.com/beevik/etree"
+
+type SortedAttrs []etree.Attr
+
+func (a SortedAttrs) Len() int {
+	return len(a)
+}
+
+func (a SortedAttrs) Swap(i, j int) {
+	a[i], a[j] = a[j], a[i]
+}
+
+func (a SortedAttrs) Less(i, j int) bool {
+	// As I understand it: any "xmlns" attribute should come first, followed by any
+	// any "xmlns:prefix" attributes, presumably ordered by prefix. Lastly any other
+	// attributes in lexicographical order.
+	if a[i].Space == defaultPrefix && a[i].Key == xmlnsPrefix {
+		return true
+	}
+
+	if a[j].Space == defaultPrefix && a[j].Key == xmlnsPrefix {
+		return false
+	}
+
+	if a[i].Space == xmlnsPrefix {
+		if a[j].Space == xmlnsPrefix {
+			return a[i].Key < a[j].Key
+		}
+		return true
+	}
+
+	if a[j].Space == xmlnsPrefix {
+		return false
+	}
+
+	if a[i].Space == a[j].Space {
+		return a[i].Key < a[j].Key
+	}
+
+	return a[i].Space < a[j].Space
+}

vendor/github.com/russellhaering/goxmldsig/sign.go

@@ -89,7 +89,7 @@ func (ctx *SigningContext) constructSignedInfo(el *etree.Element, enveloped bool
 	// /SignedInfo/Reference
 	reference := ctx.createNamespacedElement(signedInfo, ReferenceTag)
 
-	dataId := el.SelectAttrValue(DefaultIdAttr, "")
+	dataId := el.SelectAttrValue(ctx.IdAttribute, "")
 	if dataId == "" {
 		return nil, errors.New("Missing data ID")
 	}

vendor/github.com/russellhaering/goxmldsig/validate.go

@@ -5,16 +5,22 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"encoding/base64"
-	"encoding/pem"
 	"errors"
 	"fmt"
 	"regexp"
 
 	"github.com/beevik/etree"
+	"github.com/russellhaering/goxmldsig/etreeutils"
 )
 
 var uriRegexp = regexp.MustCompile("^#[a-zA-Z_][\\w.-]*$")
 
+var (
+	// ErrMissingSignature indicates that no enveloped signature was found referencing
+	// the top level element passed for signature verification.
+	ErrMissingSignature = errors.New("Missing signature referencing the top-level element")
+)
+
 type ValidationContext struct {
 	CertificateStore X509CertificateStore
 	IdAttribute      string
@@ -186,21 +192,7 @@ func (ctx *ValidationContext) verifySignedInfo(signatureElement *etree.Element,
 	return nil
 }
 
-func (ctx *ValidationContext) validateSignature(el *etree.Element, cert *x509.Certificate) (*etree.Element, error) {
-	el = el.Copy()
-
-	// Verify the document minus the signedInfo against the 'DigestValue'
-	// Find the 'Signature' element
-	sig := el.FindElement(SignatureTag)
-
-	if sig == nil {
-		return nil, errors.New("Missing Signature")
-	}
-
-	if !inNamespace(sig, Namespace) {
-		return nil, errors.New("Signature element is in the wrong namespace")
-	}
-
+func (ctx *ValidationContext) validateSignature(el, sig *etree.Element, cert *x509.Certificate) (*etree.Element, error) {
 	// Get the 'SignedInfo' element
 	signedInfo := sig.FindElement(childPath(sig.Space, SignedInfoTag))
 	if signedInfo == nil {
@@ -224,10 +216,6 @@ func (ctx *ValidationContext) validateSignature(el *etree.Element, cert *x509.Ce
 		return nil, errors.New("Reference is missing URI attribute")
 	}
 
-	if !uriRegexp.MatchString(uri.Value) {
-		return nil, errors.New("Invalid URI: " + uri.Value)
-	}
-
 	// Get the element referenced in the 'SignedInfo'
 	referencedElement := el.FindElement(fmt.Sprintf("//[@%s='%s']", ctx.IdAttribute, uri.Value[1:]))
 	if referencedElement == nil {
@@ -311,67 +299,83 @@ func contains(roots []*x509.Certificate, cert *x509.Certificate) bool {
 	return false
 }
 
-func (ctx *ValidationContext) verifyCertificate(el *etree.Element) (*x509.Certificate, error) {
-	now := ctx.Clock.Now()
-	el = el.Copy()
-
+// findSignature searches for a Signature element referencing the passed root element.
+func (ctx *ValidationContext) findSignature(el *etree.Element) (*etree.Element, error) {
 	idAttr := el.SelectAttr(DefaultIdAttr)
 	if idAttr == nil || idAttr.Value == "" {
 		return nil, errors.New("Missing ID attribute")
 	}
 
-	signatureElements := el.FindElements("//" + SignatureTag)
 	var signatureElement *etree.Element
 
-	// Find the Signature element that references the whole Response element
-	for _, e := range signatureElements {
-		e2 := e.Copy()
-
-		signedInfo := e2.FindElement(childPath(e2.Space, SignedInfoTag))
+	err := etreeutils.NSFindIterate(el, Namespace, SignatureTag, func(sig *etree.Element) error {
+		signedInfo := sig.FindElement(childPath(sig.Space, SignedInfoTag))
 		if signedInfo == nil {
-			return nil, errors.New("Missing SignedInfo")
+			return errors.New("Missing SignedInfo")
 		}
 
-		referenceElement := signedInfo.FindElement(childPath(e2.Space, ReferenceTag))
+		referenceElement := signedInfo.FindElement(childPath(sig.Space, ReferenceTag))
 		if referenceElement == nil {
-			return nil, errors.New("Missing Reference Element")
+			return errors.New("Missing Reference Element")
 		}
 
 		uriAttr := referenceElement.SelectAttr(URIAttr)
 		if uriAttr == nil || uriAttr.Value == "" {
-			return nil, errors.New("Missing URI attribute")
+			return errors.New("Missing URI attribute")
+		}
+
+		if !uriRegexp.MatchString(uriAttr.Value) {
+			return errors.New("Invalid URI: " + uriAttr.Value)
 		}
 
 		if uriAttr.Value[1:] == idAttr.Value {
-			signatureElement = e
-			break
+			signatureElement = sig
+			return etreeutils.ErrTraversalHalted
 		}
+
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
 	}
 
 	if signatureElement == nil {
-		return nil, errors.New("Missing signature referencing the top-level element")
+		return nil, ErrMissingSignature
 	}
 
-	// Get the x509 element from the signature
-	x509Element := signatureElement.FindElement("//" + childPath(signatureElement.Space, X509CertificateTag))
-	if x509Element == nil {
-		return nil, errors.New("Missing x509 Element")
-	}
+	return signatureElement, nil
+}
 
-	x509Text := "-----BEGIN CERTIFICATE-----\n" + x509Element.Text() + "\n-----END CERTIFICATE-----"
-	block, _ := pem.Decode([]byte(x509Text))
-	if block == nil {
-		return nil, errors.New("Failed to parse certificate PEM")
-	}
+func (ctx *ValidationContext) verifyCertificate(signatureElement *etree.Element) (*x509.Certificate, error) {
+	now := ctx.Clock.Now()
 
-	cert, err := x509.ParseCertificate(block.Bytes)
+	roots, err := ctx.CertificateStore.Certificates()
 	if err != nil {
 		return nil, err
 	}
 
-	roots, err := ctx.CertificateStore.Certificates()
-	if err != nil {
-		return nil, err
+	var cert *x509.Certificate
+
+	// Get the x509 element from the signature
+	x509Element := signatureElement.FindElement("//" + childPath(signatureElement.Space, X509CertificateTag))
+	if x509Element == nil {
+		// Use root certificate if there is only one and it is not contained in signatureElement
+		if len(roots) == 1 {
+			cert = roots[0]
+		} else {
+			return nil, errors.New("Missing x509 Element")
+		}
+	} else {
+		certData, err := base64.StdEncoding.DecodeString(x509Element.Text())
+		if err != nil {
+			return nil, errors.New("Failed to parse certificate")
+		}
+
+		cert, err = x509.ParseCertificate(certData)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	// Verify that the certificate is one we trust
@@ -386,12 +390,21 @@ func (ctx *ValidationContext) verifyCertificate(el *etree.Element) (*x509.Certif
 	return cert, nil
 }
 
+// Validate verifies that the passed element contains a valid enveloped signature
+// matching a currently-valid certificate in the context's CertificateStore.
 func (ctx *ValidationContext) Validate(el *etree.Element) (*etree.Element, error) {
-	cert, err := ctx.verifyCertificate(el)
+	// Make a copy of the element to avoid mutating the one we were passed.
+	el = el.Copy()
+
+	sig, err := ctx.findSignature(el)
+	if err != nil {
+		return nil, err
+	}
 
+	cert, err := ctx.verifyCertificate(sig)
 	if err != nil {
 		return nil, err
 	}
 
-	return ctx.validateSignature(el, cert)
+	return ctx.validateSignature(el, sig, cert)
 }