diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 7fad1cc3..8b55252a 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -17,6 +17,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/beevik/etree" dsig "github.com/russellhaering/goxmldsig" + "github.com/russellhaering/goxmldsig/etreeutils" "github.com/coreos/dex/connector" ) @@ -500,8 +501,9 @@ func verify(validator *dsig.ValidationContext, data []byte) (signed []byte, err verified = true doc.SetRoot(transformedResponse) } - assertion := response.SelectElement("Assertion") - if assertion == nil { + // Ensures xmlns are copied down to the assertion element when they are defined in the root + assertion, err := etreeutils.NSSelectOne(response, "urn:oasis:names:tc:SAML:2.0:assertion", "Assertion") + if err != nil { return nil, fmt.Errorf("response does not contain an Assertion element") } transformedAssertion, err := validator.Validate(assertion) diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go index a2bc49b2..cba0fe12 100644 --- a/connector/saml/saml_test.go +++ b/connector/saml/saml_test.go @@ -86,6 +86,10 @@ func TestVerify(t *testing.T) { runVerify(t, "testdata/okta-ca.pem", "testdata/okta-resp.xml", true) } +func TestVerifyUnsignedMessageAndSignedAssertionWithRootXmlNs(t *testing.T) { + runVerify(t, "testdata/oam-ca.pem", "testdata/oam-resp.xml", true) +} + func TestVerifySignedMessageAndUnsignedAssertion(t *testing.T) { runVerify(t, "testdata/idp-cert.pem", "testdata/idp-resp-signed-message.xml", true) } diff --git a/connector/saml/testdata/oam-ca.pem b/connector/saml/testdata/oam-ca.pem new file mode 100644 index 00000000..41645dda --- /dev/null +++ b/connector/saml/testdata/oam-ca.pem @@ -0,0 +1,13 @@ +-----BEGIN CERTIFICATE----- +MIIB/jCCAWegAwIBAgIBCjANBgkqhkiG9w0BAQQFADAkMSIwIAYDVQQDExlkZWFv +YW0tZGV2MDIuanBsLm5hc2EuZ292MB4XDTE2MDYzMDA0NTQxNloXDTI2MDYyODA0 +NTQxNlowJDEiMCAGA1UEAxMZZGVhb2FtLWRldjAyLmpwbC5uYXNhLmdvdjCBnzAN +BgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAht1N4lGdwUbl7YRyHwSCrnep6/e2I3+V +eue0pSA/DGn8OuR/udM8UCja5utqlqJdq200ox4b4Mpz0Jg9kMckALtKe+1DgeES +EIx9FpeuBdHlitYQNSbEr30HIG2nmeTOy4Vi5unBO54um3tNazcUTMA0/LJ6KQL8 +LeZSlB/IxwUCAwEAAaNAMD4wDAYDVR0TAQH/BAIwADAPBgNVHQ8BAf8EBQMDB9gA +MB0GA1UdDgQWBBRYo1YjfrNonauLzj6/AsueWFGSszANBgkqhkiG9w0BAQQFAAOB +gQACq7GHK/Zsg0+qC0WWa2ZjmOXE6Dqk/xuooG49QT7ihABs7k9U27Fw3xKF6MkC +7pca1FwT82eZK1N3XKKpZe7Flu1fMKt2o/XSiBkDjWwUcChVnwGsUBe8hJFwFqg7 +olNJn1kaVBJUqZIiXF9kS0d+1H55rStOd0CNXAzp9utr2A== +-----END CERTIFICATE----- diff --git a/connector/saml/testdata/oam-resp.xml b/connector/saml/testdata/oam-resp.xml new file mode 100644 index 00000000..99d14877 --- /dev/null +++ b/connector/saml/testdata/oam-resp.xml @@ -0,0 +1 @@ +https://deaoam-dev02.jpl.nasa.gov:14101/oam/fedhttps://deaoam-dev02.jpl.nasa.gov:14101/oam/fedz1HD/59hv6UOd5+jeG+ihaFWLgI=I99oG5kiOfIgbXYa21z/TOmzftTkFnXe9ObhBNSKit9kAhT93apYROqqXv4Ax96P144Ld7ERX1hgJsytK8LC2874Pk7QrSNm4zvW3x0D4GR4lM06CvJK/EhIur3TrCUJDPigvyP7TJitheCyBejwt0x0lqNP/OzR3tMbAIMRoho=pkieuJSAuthurn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport diff --git a/glide.lock b/glide.lock index 63ae90d2..93ac43c2 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: 4a3ac3a2a2b39357dc5de7ba296dbe20a97dd2fe13bc15a68e904e69afae2adb -updated: 2017-03-21T09:24:08.089284929-07:00 +hash: ba77a77f03b1750479aa4a61de59d7a545dc8bd5c71be10e1c2e9afed37e1925 +updated: 2017-03-24T11:03:21.332291207-07:00 imports: - name: github.com/beevik/etree version: 4cd0dd976db869f817248477718071a28e978df0 @@ -46,9 +46,10 @@ imports: subpackages: - cacheobject - name: github.com/russellhaering/goxmldsig - version: 51810e925e5fc495822fbddda8202f70a6e4a3f3 + version: eaac44c63fe007124f8f6255b09febc906784981 subpackages: - etreeutils + - types - name: github.com/Sirupsen/logrus version: d26492970760ca5d33129d2d799e34be5c4782eb - name: github.com/spf13/cobra diff --git a/glide.yaml b/glide.yaml index bde1eb5c..2af9964f 100644 --- a/glide.yaml +++ b/glide.yaml @@ -73,7 +73,7 @@ import: - package: golang.org/x/oauth2 version: 08c8d727d2392d18286f9f88ad775ad98f09ab33 subpackages: [] -# The oauth2 package only imports the appengine code when it's given a +# The oauth2 package only imports the appengine code when it's given a # specific build tags, but glide detects it anyway. # # https://github.com/golang/oauth2/blob/d5040cdd/client_appengine.go @@ -133,7 +133,7 @@ import: # XML signature validation for SAML connector - package: github.com/russellhaering/goxmldsig - version: 51810e925e5fc495822fbddda8202f70a6e4a3f3 + version: eaac44c63fe007124f8f6255b09febc906784981 - package: github.com/beevik/etree version: 4cd0dd976db869f817248477718071a28e978df0 - package: github.com/jonboulle/clockwork diff --git a/vendor/github.com/russellhaering/goxmldsig/canonicalize.go b/vendor/github.com/russellhaering/goxmldsig/canonicalize.go index 2a2c9227..34e1a581 100644 --- a/vendor/github.com/russellhaering/goxmldsig/canonicalize.go +++ b/vendor/github.com/russellhaering/goxmldsig/canonicalize.go @@ -2,7 +2,6 @@ package dsig import ( "sort" - "strings" "github.com/beevik/etree" "github.com/russellhaering/goxmldsig/etreeutils" @@ -15,28 +14,25 @@ type Canonicalizer interface { } type c14N10ExclusiveCanonicalizer struct { - InclusiveNamespaces map[string]struct{} + prefixList string } // MakeC14N10ExclusiveCanonicalizerWithPrefixList constructs an exclusive Canonicalizer // from a PrefixList in NMTOKENS format (a white space separated list). func MakeC14N10ExclusiveCanonicalizerWithPrefixList(prefixList string) Canonicalizer { - prefixes := strings.Fields(prefixList) - prefixSet := make(map[string]struct{}, len(prefixes)) - - for _, prefix := range prefixes { - prefixSet[prefix] = struct{}{} - } - return &c14N10ExclusiveCanonicalizer{ - InclusiveNamespaces: prefixSet, + prefixList: prefixList, } } // Canonicalize transforms the input Element into a serialized XML document in canonical form. func (c *c14N10ExclusiveCanonicalizer) Canonicalize(el *etree.Element) ([]byte, error) { - scope := make(map[string]c14nSpace) - return canonicalSerialize(excCanonicalPrep(el, scope, c.InclusiveNamespaces)) + err := etreeutils.TransformExcC14n(el, c.prefixList) + if err != nil { + return nil, err + } + + return canonicalSerialize(el) } func (c *c14N10ExclusiveCanonicalizer) Algorithm() AlgorithmID { @@ -75,94 +71,6 @@ type c14nSpace struct { const nsSpace = "xmlns" -// excCanonicalPrep accepts an *etree.Element and recursively transforms it into one -// which is ready for serialization to exclusive canonical form. Specifically this -// entails: -// -// 1. Stripping re-declarations of namespaces -// 2. Stripping unused namespaces -// 3. Sorting attributes into canonical order. -// -// NOTE(russell_h): Currently this function modifies the passed element. -func excCanonicalPrep(el *etree.Element, _nsAlreadyDeclared map[string]c14nSpace, inclusiveNamespaces map[string]struct{}) *etree.Element { - //Copy alreadyDeclared map (only contains namespaces) - nsAlreadyDeclared := make(map[string]c14nSpace, len(_nsAlreadyDeclared)) - for k := range _nsAlreadyDeclared { - nsAlreadyDeclared[k] = _nsAlreadyDeclared[k] - } - - //Track the namespaces used on the current element - nsUsedHere := make(map[string]struct{}) - - //Make sure to track the element namespace for the case: - // - if el.Space != "" { - nsUsedHere[el.Space] = struct{}{} - } - - toRemove := make([]string, 0, 0) - - for _, a := range el.Attr { - switch a.Space { - case nsSpace: - - //For simplicity, remove all xmlns attribues; to be added in one pass - //later. Otherwise, we need another map/set to track xmlns attributes - //that we left alone. - toRemove = append(toRemove, a.Space+":"+a.Key) - if _, ok := nsAlreadyDeclared[a.Key]; !ok { - //If we're not tracking ancestor state already for this namespace, add - //it to the map - nsAlreadyDeclared[a.Key] = c14nSpace{a: a, used: false} - } - - // This algorithm accepts a set of namespaces which should be treated - // in an inclusive fashion. Specifically that means we should keep the - // declaration of that namespace closest to the root of the tree. We can - // accomplish that be pretending it was used by this element. - _, inclusive := inclusiveNamespaces[a.Key] - if inclusive { - nsUsedHere[a.Key] = struct{}{} - } - - default: - //We only track namespaces, so ignore attributes without one. - if a.Space != "" { - nsUsedHere[a.Space] = struct{}{} - } - } - } - - //Remove all attributes so that we can add them with much-simpler logic - for _, attrK := range toRemove { - el.RemoveAttr(attrK) - } - - //For all namespaces used on the current element, declare them if they were - //not declared (and used) in an ancestor. - for k := range nsUsedHere { - spc := nsAlreadyDeclared[k] - //If previously unused, mark as used - if !spc.used { - el.Attr = append(el.Attr, spc.a) - spc.used = true - - //Assignment here is only to update the pre-existing `used` tracking value - nsAlreadyDeclared[k] = spc - } - } - - //Canonicalize all children, passing down the ancestor tracking map - for _, child := range el.ChildElements() { - excCanonicalPrep(child, nsAlreadyDeclared, inclusiveNamespaces) - } - - //Sort attributes lexicographically - sort.Sort(etreeutils.SortedAttrs(el.Attr)) - - return el.Copy() -} - // canonicalPrep accepts an *etree.Element and transforms it into one which is ready // for serialization into inclusive canonical form. Specifically this // entails: @@ -208,7 +116,7 @@ func canonicalPrep(el *etree.Element, seenSoFar map[string]struct{}) *etree.Elem func canonicalSerialize(el *etree.Element) ([]byte, error) { doc := etree.NewDocument() - doc.SetRoot(el) + doc.SetRoot(el.Copy()) doc.WriteSettings = etree.WriteSettings{ CanonicalAttrVal: true, diff --git a/vendor/github.com/russellhaering/goxmldsig/etreeutils/canonicalize.go b/vendor/github.com/russellhaering/goxmldsig/etreeutils/canonicalize.go new file mode 100644 index 00000000..9e6df954 --- /dev/null +++ b/vendor/github.com/russellhaering/goxmldsig/etreeutils/canonicalize.go @@ -0,0 +1,98 @@ +package etreeutils + +import ( + "sort" + "strings" + + "github.com/beevik/etree" +) + +// TransformExcC14n transforms the passed element into xml-exc-c14n form. +func TransformExcC14n(el *etree.Element, inclusiveNamespacesPrefixList string) error { + prefixes := strings.Fields(inclusiveNamespacesPrefixList) + prefixSet := make(map[string]struct{}, len(prefixes)) + + for _, prefix := range prefixes { + prefixSet[prefix] = struct{}{} + } + + err := transformExcC14n(DefaultNSContext, EmptyNSContext, el, prefixSet) + if err != nil { + return err + } + + return nil +} + +func transformExcC14n(ctx, declared NSContext, el *etree.Element, inclusiveNamespaces map[string]struct{}) error { + scope, err := ctx.SubContext(el) + if err != nil { + return err + } + + visiblyUtilizedPrefixes := map[string]struct{}{ + el.Space: struct{}{}, + } + + filteredAttrs := []etree.Attr{} + + // Filter out all namespace declarations + for _, attr := range el.Attr { + switch { + case attr.Space == xmlnsPrefix: + if _, ok := inclusiveNamespaces[attr.Key]; ok { + visiblyUtilizedPrefixes[attr.Key] = struct{}{} + } + + case attr.Space == defaultPrefix && attr.Key == xmlnsPrefix: + if _, ok := inclusiveNamespaces[defaultPrefix]; ok { + visiblyUtilizedPrefixes[defaultPrefix] = struct{}{} + } + + default: + if attr.Space != defaultPrefix { + visiblyUtilizedPrefixes[attr.Space] = struct{}{} + } + + filteredAttrs = append(filteredAttrs, attr) + } + } + + el.Attr = filteredAttrs + + declared = declared.Copy() + + // Declare all visibly utilized prefixes that are in-scope but haven't + // been declared in the canonicalized form yet. These might have been + // declared on this element but then filtered out above, or they might + // have been declared on an ancestor (before canonicalization) which + // didn't visibly utilize and thus had them removed. + for prefix := range visiblyUtilizedPrefixes { + // Skip redundant declarations - they have to already have the same + // value. + if declaredNamespace, ok := declared.prefixes[prefix]; ok { + if value, ok := scope.prefixes[prefix]; ok && declaredNamespace == value { + continue + } + } + + namespace, err := scope.LookupPrefix(prefix) + if err != nil { + return err + } + + el.Attr = append(el.Attr, declared.declare(prefix, namespace)) + } + + sort.Sort(SortedAttrs(el.Attr)) + + // Transform child elements + for _, child := range el.ChildElements() { + err := transformExcC14n(scope, declared, child, inclusiveNamespaces) + if err != nil { + return err + } + } + + return nil +} diff --git a/vendor/github.com/russellhaering/goxmldsig/etreeutils/namespace.go b/vendor/github.com/russellhaering/goxmldsig/etreeutils/namespace.go index 3034ff1a..45f3bea1 100644 --- a/vendor/github.com/russellhaering/goxmldsig/etreeutils/namespace.go +++ b/vendor/github.com/russellhaering/goxmldsig/etreeutils/namespace.go @@ -47,13 +47,38 @@ type NSContext struct { prefixes map[string]string } -func (ctx NSContext) SubContext(el *etree.Element) (NSContext, error) { - // The subcontext should inherit existing declared prefixes +func (ctx NSContext) Copy() NSContext { prefixes := make(map[string]string, len(ctx.prefixes)+4) for k, v := range ctx.prefixes { prefixes[k] = v } + return NSContext{prefixes: prefixes} +} + +func (ctx NSContext) declare(prefix, namespace string) etree.Attr { + ctx.prefixes[prefix] = namespace + + switch prefix { + case defaultPrefix: + return etree.Attr{ + Key: xmlnsPrefix, + Value: namespace, + } + + default: + return etree.Attr{ + Space: xmlnsPrefix, + Key: prefix, + Value: namespace, + } + } +} + +func (ctx NSContext) SubContext(el *etree.Element) (NSContext, error) { + // The subcontext should inherit existing declared prefixes + newCtx := ctx.Copy() + // Merge new namespace declarations on top of existing ones. for _, attr := range el.Attr { if attr.Space == xmlnsPrefix { @@ -69,7 +94,7 @@ func (ctx NSContext) SubContext(el *etree.Element) (NSContext, error) { return ctx, ErrReservedNamespace } - prefixes[attr.Key] = attr.Value + newCtx.declare(attr.Key, attr.Value) } else if attr.Space == defaultPrefix && attr.Key == xmlnsPrefix { // This attribute is a default namespace declaration @@ -78,11 +103,21 @@ func (ctx NSContext) SubContext(el *etree.Element) (NSContext, error) { return ctx, ErrInvalidDefaultNamespace } - prefixes[defaultPrefix] = attr.Value + newCtx.declare(defaultPrefix, attr.Value) } } - return NSContext{prefixes: prefixes}, nil + return newCtx, nil +} + +// Prefixes returns a copy of this context's prefix map. +func (ctx NSContext) Prefixes() map[string]string { + prefixes := make(map[string]string, len(ctx.prefixes)) + for k, v := range ctx.prefixes { + prefixes[k] = v + } + + return prefixes } // LookupPrefix attempts to find a declared namespace for the specified prefix. If the prefix @@ -98,7 +133,13 @@ func (ctx NSContext) LookupPrefix(prefix string) (string, error) { } } -func nsTraverse(ctx NSContext, el *etree.Element, handle func(NSContext, *etree.Element) error) error { +// NSIterHandler is a function which is invoked with a element and its surrounding +// NSContext during traversals. +type NSIterHandler func(NSContext, *etree.Element) error + +// NSTraverse traverses an element tree, invoking the passed handler for each element +// in the tree. +func NSTraverse(ctx NSContext, el *etree.Element, handle NSIterHandler) error { ctx, err := ctx.SubContext(el) if err != nil { return err @@ -111,7 +152,7 @@ func nsTraverse(ctx NSContext, el *etree.Element, handle func(NSContext, *etree. // Recursively traverse child elements. for _, child := range el.ChildElements() { - err := nsTraverse(ctx, child, handle) + err := NSTraverse(ctx, child, handle) if err != nil { return err } @@ -120,7 +161,9 @@ func nsTraverse(ctx NSContext, el *etree.Element, handle func(NSContext, *etree. return nil } -func detachWithNamespaces(ctx NSContext, el *etree.Element) (*etree.Element, error) { +// NSDetatch makes a copy of the passed element, and declares any namespaces in +// the passed context onto the new element before returning it. +func NSDetatch(ctx NSContext, el *etree.Element) (*etree.Element, error) { ctx, err := ctx.SubContext(el) if err != nil { return nil, err @@ -177,43 +220,52 @@ func detachWithNamespaces(ctx NSContext, el *etree.Element) (*etree.Element, err return el, nil } -// NSSelectOne conducts a depth-first search for an element with the specified namespace +// NSSelectOne behaves identically to NSSelectOneCtx, but uses DefaultNSContext as the +// surrounding context. +func NSSelectOne(el *etree.Element, namespace, tag string) (*etree.Element, error) { + return NSSelectOneCtx(DefaultNSContext, el, namespace, tag) +} + +// NSSelectOneCtx 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) { +func NSSelectOneCtx(ctx NSContext, 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) + err := NSFindIterateCtx(ctx, el, namespace, tag, func(ctx NSContext, el *etree.Element) error { + var err error + + found, err = NSDetatch(ctx, el) 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 + return ErrTraversalHalted }) - if err != nil && err != ErrTraversalHalted { + if err != nil { 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 { +// NSFindIterate behaves identically to NSFindIterateCtx, but uses DefaultNSContext +// as the surrounding context. +func NSFindIterate(el *etree.Element, namespace, tag string, handle NSIterHandler) error { + return NSFindIterateCtx(DefaultNSContext, el, namespace, tag, handle) +} + +// NSFindIterateCtx conducts a depth-first traversal searching for elements with the +// specified tag in the specified namespace. It uses the passed NSContext for prefix +// lookups. 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 NSFindIterateCtx(ctx NSContext, el *etree.Element, namespace, tag string, handle NSIterHandler) error { + err := NSTraverse(ctx, el, func(ctx NSContext, el *etree.Element) error { currentNS, err := ctx.LookupPrefix(el.Space) if err != nil { return err @@ -221,7 +273,7 @@ func NSFindIterate(el *etree.Element, namespace, tag string, handle func(*etree. // Base case, el is the sought after element. if currentNS == namespace && el.Tag == tag { - return handle(el) + return handle(ctx, el) } return nil @@ -234,12 +286,18 @@ func NSFindIterate(el *etree.Element, namespace, tag string, handle func(*etree. return nil } -// NSFindOne conducts a depth-first search for the specified element. If such an element -// is found a reference to it is returned. +// NSFindOne behaves identically to NSFindOneCtx, but uses DefaultNSContext for +// context. func NSFindOne(el *etree.Element, namespace, tag string) (*etree.Element, error) { + return NSFindOneCtx(DefaultNSContext, el, namespace, tag) +} + +// NSFindOneCtx conducts a depth-first search for the specified element. If such an element +// is found a reference to it is returned. +func NSFindOneCtx(ctx NSContext, el *etree.Element, namespace, tag string) (*etree.Element, error) { var found *etree.Element - err := NSFindIterate(el, namespace, tag, func(el *etree.Element) error { + err := NSFindIterateCtx(ctx, el, namespace, tag, func(ctx NSContext, el *etree.Element) error { found = el return ErrTraversalHalted }) @@ -250,3 +308,21 @@ func NSFindOne(el *etree.Element, namespace, tag string) (*etree.Element, error) return found, nil } + +// NSBuildParentContext recurses upward from an element in order to build an NSContext +// for its immediate parent. If the element has no parent DefaultNSContext +// is returned. +func NSBuildParentContext(el *etree.Element) (NSContext, error) { + parent := el.Parent() + if parent == nil { + return DefaultNSContext, nil + } + + ctx, err := NSBuildParentContext(parent) + + if err != nil { + return ctx, err + } + + return ctx.SubContext(parent) +} diff --git a/vendor/github.com/russellhaering/goxmldsig/etreeutils/sort.go b/vendor/github.com/russellhaering/goxmldsig/etreeutils/sort.go index f530b428..5871a391 100644 --- a/vendor/github.com/russellhaering/goxmldsig/etreeutils/sort.go +++ b/vendor/github.com/russellhaering/goxmldsig/etreeutils/sort.go @@ -2,6 +2,8 @@ package etreeutils import "github.com/beevik/etree" +// SortedAttrs provides sorting capabilities, compatible with XML C14N, on top +// of an []etree.Attr type SortedAttrs []etree.Attr func (a SortedAttrs) Len() int { @@ -13,17 +15,23 @@ func (a SortedAttrs) Swap(i, j int) { } 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 - } + // This is the best reference I've found on sort order: + // http://dst.lbl.gov/~ksb/Scratch/XMLC14N.html + // If attr j is a default namespace declaration, attr i may + // not be strictly "less" than it. if a[j].Space == defaultPrefix && a[j].Key == xmlnsPrefix { return false } + // Otherwise, if attr i is a default namespace declaration, it + // must be less than anything else. + if a[i].Space == defaultPrefix && a[i].Key == xmlnsPrefix { + return true + } + + // Next, namespace prefix declarations, sorted by prefix, come before + // anythign else. if a[i].Space == xmlnsPrefix { if a[j].Space == xmlnsPrefix { return a[i].Key < a[j].Key @@ -35,6 +43,21 @@ func (a SortedAttrs) Less(i, j int) bool { return false } + // Then come unprefixed attributes, sorted by key. + if a[i].Space == defaultPrefix { + if a[j].Space == defaultPrefix { + return a[i].Key < a[j].Key + } + return true + } + + if a[j].Space == defaultPrefix { + return false + } + + // Wow. We're still going. Finally, attributes in the same namespace should be + // sorted by key. Attributes in different namespaces should be sorted by the + // actual namespace (_not_ the prefix). For now just use the prefix. if a[i].Space == a[j].Space { return a[i].Key < a[j].Key } diff --git a/vendor/github.com/russellhaering/goxmldsig/etreeutils/unmarshal.go b/vendor/github.com/russellhaering/goxmldsig/etreeutils/unmarshal.go new file mode 100644 index 00000000..b1fecf85 --- /dev/null +++ b/vendor/github.com/russellhaering/goxmldsig/etreeutils/unmarshal.go @@ -0,0 +1,43 @@ +package etreeutils + +import ( + "encoding/xml" + + "github.com/beevik/etree" +) + +// NSUnmarshalElement unmarshals the passed etree Element into the value pointed to by +// v using encoding/xml in the context of the passed NSContext. If v implements +// ElementKeeper, SetUnderlyingElement will be called on v with a reference to el. +func NSUnmarshalElement(ctx NSContext, el *etree.Element, v interface{}) error { + detatched, err := NSDetatch(ctx, el) + if err != nil { + return err + } + + doc := etree.NewDocument() + doc.AddChild(detatched) + data, err := doc.WriteToBytes() + if err != nil { + return err + } + + err = xml.Unmarshal(data, v) + if err != nil { + return err + } + + switch v := v.(type) { + case ElementKeeper: + v.SetUnderlyingElement(el) + } + + return nil +} + +// ElementKeeper should be implemented by types which will be passed to +// UnmarshalElement, but wish to keep a reference +type ElementKeeper interface { + SetUnderlyingElement(*etree.Element) + UnderlyingElement() *etree.Element +} diff --git a/vendor/github.com/russellhaering/goxmldsig/sign.go b/vendor/github.com/russellhaering/goxmldsig/sign.go index b41364e4..82498098 100644 --- a/vendor/github.com/russellhaering/goxmldsig/sign.go +++ b/vendor/github.com/russellhaering/goxmldsig/sign.go @@ -11,6 +11,7 @@ import ( "fmt" "github.com/beevik/etree" + "github.com/russellhaering/goxmldsig/etreeutils" ) type SigningContext struct { @@ -133,15 +134,39 @@ func (ctx *SigningContext) constructSignature(el *etree.Element, enveloped bool) } sig.CreateAttr(xmlns, Namespace) + sig.AddChild(signedInfo) - sig.Child = append(sig.Child, signedInfo) + // When using xml-c14n11 (ie, non-exclusive canonicalization) the canonical form + // of the SignedInfo must declare all namespaces that are in scope at it's final + // enveloped location in the document. In order to do that, we're going to construct + // a series of cascading NSContexts to capture namespace declarations: - // Must propagate down the attributes to the 'SignedInfo' before digesting - for _, attr := range sig.Attr { - signedInfo.CreateAttr(attr.Space+":"+attr.Key, attr.Value) + // First get the context surrounding the element we are signing. + rootNSCtx, err := etreeutils.NSBuildParentContext(el) + if err != nil { + return nil, err } - digest, err := ctx.digest(signedInfo) + // Then capture any declarations on the element itself. + elNSCtx, err := rootNSCtx.SubContext(el) + if err != nil { + return nil, err + } + + // Followed by declarations on the Signature (which we just added above) + sigNSCtx, err := elNSCtx.SubContext(sig) + if err != nil { + return nil, err + } + + // Finally detatch the SignedInfo in order to capture all of the namespace + // declarations in the scope we've constructed. + detatchedSignedInfo, err := etreeutils.NSDetatch(sigNSCtx, signedInfo) + if err != nil { + return nil, err + } + + digest, err := ctx.digest(detatchedSignedInfo) if err != nil { return nil, err } diff --git a/vendor/github.com/russellhaering/goxmldsig/types/signature.go b/vendor/github.com/russellhaering/goxmldsig/types/signature.go new file mode 100644 index 00000000..2c7b1632 --- /dev/null +++ b/vendor/github.com/russellhaering/goxmldsig/types/signature.go @@ -0,0 +1,93 @@ +package types + +import ( + "encoding/xml" + + "github.com/beevik/etree" +) + +type InclusiveNamespaces struct { + XMLName xml.Name `xml:"http://www.w3.org/2001/10/xml-exc-c14n# InclusiveNamespaces"` + PrefixList string `xml:"PrefixList,attr"` +} + +type Transform struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# Transform"` + Algorithm string `xml:"Algorithm,attr"` + InclusiveNamespaces *InclusiveNamespaces `xml:"InclusiveNamespaces"` +} + +type Transforms struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# Transforms"` + Transforms []Transform `xml:"Transform"` +} + +type DigestMethod struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# DigestMethod"` + Algorithm string `xml:"Algorithm,attr"` +} + +type Reference struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# Reference"` + URI string `xml:"URI,attr"` + DigestValue string `xml:"DigestValue"` + DigestAlgo DigestMethod `xml:"DigestMethod"` + Transforms Transforms `xml:"Transforms"` +} + +type CanonicalizationMethod struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# CanonicalizationMethod"` + Algorithm string `xml:"Algorithm,attr"` +} + +type SignatureMethod struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# SignatureMethod"` + Algorithm string `xml:"Algorithm,attr"` +} + +type SignedInfo struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# SignedInfo"` + CanonicalizationMethod CanonicalizationMethod `xml:"CanonicalizationMethod"` + SignatureMethod SignatureMethod `xml:"SignatureMethod"` + References []Reference `xml:"Reference"` +} + +type SignatureValue struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# SignatureValue"` + Data string `xml:",chardata"` +} + +type KeyInfo struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# KeyInfo"` + X509Data X509Data `xml:"X509Data"` +} + +type X509Data struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# X509Data"` + X509Certificate X509Certificate `xml:"X509Certificate"` +} + +type X509Certificate struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# X509Certificate"` + Data string `xml:",chardata"` +} + +type Signature struct { + XMLName xml.Name `xml:"http://www.w3.org/2000/09/xmldsig# Signature"` + SignedInfo *SignedInfo `xml:"SignedInfo"` + SignatureValue *SignatureValue `xml:"SignatureValue"` + KeyInfo *KeyInfo `xml:"KeyInfo"` + el *etree.Element +} + +// SetUnderlyingElement will be called with a reference to the Element this Signature +// was unmarshaled from. +func (s *Signature) SetUnderlyingElement(el *etree.Element) { + s.el = el +} + +// UnderlyingElement returns a reference to the Element this signature was unmarshaled +// from, where applicable. +func (s *Signature) UnderlyingElement() *etree.Element { + return s.el +} diff --git a/vendor/github.com/russellhaering/goxmldsig/validate.go b/vendor/github.com/russellhaering/goxmldsig/validate.go index 250d9e78..7aa2d73e 100644 --- a/vendor/github.com/russellhaering/goxmldsig/validate.go +++ b/vendor/github.com/russellhaering/goxmldsig/validate.go @@ -11,6 +11,7 @@ import ( "github.com/beevik/etree" "github.com/russellhaering/goxmldsig/etreeutils" + "github.com/russellhaering/goxmldsig/types" ) var uriRegexp = regexp.MustCompile("^#[a-zA-Z_][\\w.-]*$") @@ -82,7 +83,12 @@ func recursivelyRemoveElement(tree, el *etree.Element) bool { // instead return a copy. Unfortunately copying the tree makes it difficult to // correctly locate the signature. I'm opting, for now, to simply mutate the root // parameter. -func (ctx *ValidationContext) transform(root, sig *etree.Element, transforms []*etree.Element) (*etree.Element, Canonicalizer, error) { +func (ctx *ValidationContext) transform( + el *etree.Element, + sig *types.Signature, + ref *types.Reference) (*etree.Element, Canonicalizer, error) { + transforms := ref.Transforms.Transforms + if len(transforms) != 2 { return nil, nil, errors.New("Expected Enveloped and C14N transforms") } @@ -90,25 +96,18 @@ func (ctx *ValidationContext) transform(root, sig *etree.Element, transforms []* var canonicalizer Canonicalizer for _, transform := range transforms { - algo := transform.SelectAttr(AlgorithmAttr) - if algo == nil { - return nil, nil, errors.New("Missing Algorithm attribute") - } + algo := transform.Algorithm - switch AlgorithmID(algo.Value) { + switch AlgorithmID(algo) { case EnvelopedSignatureAltorithmId: - if !recursivelyRemoveElement(root, sig) { + if !recursivelyRemoveElement(el, sig.UnderlyingElement()) { return nil, nil, errors.New("Error applying canonicalization transform: Signature not found") } case CanonicalXML10ExclusiveAlgorithmId: var prefixList string - ins := transform.FindElement(childPath("", InclusiveNamespacesTag)) - if ins != nil { - prefixListEl := ins.SelectAttr(PrefixListAttr) - if prefixListEl != nil { - prefixList = prefixListEl.Value - } + if transform.InclusiveNamespaces != nil { + prefixList = transform.InclusiveNamespaces.PrefixList } canonicalizer = MakeC14N10ExclusiveCanonicalizerWithPrefixList(prefixList) @@ -117,7 +116,7 @@ func (ctx *ValidationContext) transform(root, sig *etree.Element, transforms []* canonicalizer = MakeC14N11Canonicalizer() default: - return nil, nil, errors.New("Unknown Transform Algorithm: " + algo.Value) + return nil, nil, errors.New("Unknown Transform Algorithm: " + algo) } } @@ -125,7 +124,7 @@ func (ctx *ValidationContext) transform(root, sig *etree.Element, transforms []* return nil, nil, errors.New("Expected canonicalization transform") } - return root, canonicalizer, nil + return el, canonicalizer, nil } func (ctx *ValidationContext) digest(el *etree.Element, digestAlgorithmId string, canonicalizer Canonicalizer) ([]byte, error) { @@ -148,19 +147,16 @@ func (ctx *ValidationContext) digest(el *etree.Element, digestAlgorithmId string return hash.Sum(nil), nil } -func (ctx *ValidationContext) verifySignedInfo(signatureElement *etree.Element, canonicalizer Canonicalizer, signatureMethodId string, cert *x509.Certificate, sig []byte) error { +func (ctx *ValidationContext) verifySignedInfo(sig *types.Signature, canonicalizer Canonicalizer, signatureMethodId string, cert *x509.Certificate, decodedSignature []byte) error { + signatureElement := sig.UnderlyingElement() + signedInfo := signatureElement.FindElement(childPath(signatureElement.Space, SignedInfoTag)) if signedInfo == nil { return errors.New("Missing SignedInfo") } - // Any attributes from the 'Signature' element must be pushed down into the 'SignedInfo' element before it is canonicalized - for _, attr := range signatureElement.Attr { - signedInfo.CreateAttr(attr.Space+":"+attr.Key, attr.Value) - } - // Canonicalize the xml - canonical, err := canonicalizer.Canonicalize(signedInfo) + canonical, err := canonicalSerialize(signedInfo) if err != nil { return err } @@ -184,7 +180,7 @@ func (ctx *ValidationContext) verifySignedInfo(signatureElement *etree.Element, } // Verify that the private key matching the public key from the cert was what was used to sign the 'SignedInfo' and produce the 'SignatureValue' - err = rsa.VerifyPKCS1v15(pubKey, signatureAlgorithm, hashed[:], sig) + err = rsa.VerifyPKCS1v15(pubKey, signatureAlgorithm, hashed[:], decodedSignature) if err != nil { return err } @@ -192,65 +188,37 @@ func (ctx *ValidationContext) verifySignedInfo(signatureElement *etree.Element, return nil } -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 { - return nil, errors.New("Missing SignedInfo") +func (ctx *ValidationContext) validateSignature(el *etree.Element, sig *types.Signature, cert *x509.Certificate) (*etree.Element, error) { + idAttr := el.SelectAttr(DefaultIdAttr) + if idAttr == nil || idAttr.Value == "" { + return nil, errors.New("Missing ID attribute") } - reference := signedInfo.FindElement(childPath(sig.Space, ReferenceTag)) - if reference == nil { - return nil, errors.New("Missing Reference") - } + var ref *types.Reference - transforms := reference.FindElement(childPath(sig.Space, TransformsTag)) - if transforms == nil { - return nil, errors.New("Missing Transforms") - } - - uri := reference.SelectAttr("URI") - if uri == nil { - // TODO(russell_h): It is permissible to leave this out. We should be - // able to fall back to finding the referenced element some other way. - return nil, errors.New("Reference is missing URI attribute") - } - - // Get the element referenced in the 'SignedInfo' - referencedElement := el.FindElement(fmt.Sprintf("//[@%s='%s']", ctx.IdAttribute, uri.Value[1:])) - if referencedElement == nil { - return nil, errors.New("Unable to find referenced element: " + uri.Value) + // Find the first reference which references the top-level element + for _, _ref := range sig.SignedInfo.References { + if _ref.URI == "" || _ref.URI[1:] == idAttr.Value { + ref = &_ref + } } // Perform all transformations listed in the 'SignedInfo' // Basically, this means removing the 'SignedInfo' - transformed, canonicalizer, err := ctx.transform(referencedElement, sig, transforms.ChildElements()) + transformed, canonicalizer, err := ctx.transform(el, sig, ref) if err != nil { return nil, err } - digestMethod := reference.FindElement(childPath(sig.Space, DigestMethodTag)) - if digestMethod == nil { - return nil, errors.New("Missing DigestMethod") - } - - digestValue := reference.FindElement(childPath(sig.Space, DigestValueTag)) - if digestValue == nil { - return nil, errors.New("Missing DigestValue") - } - - digestAlgorithmAttr := digestMethod.SelectAttr(AlgorithmAttr) - if digestAlgorithmAttr == nil { - return nil, errors.New("Missing DigestMethod Algorithm attribute") - } + digestAlgorithm := ref.DigestAlgo.Algorithm // Digest the transformed XML and compare it to the 'DigestValue' from the 'SignedInfo' - digest, err := ctx.digest(transformed, digestAlgorithmAttr.Value, canonicalizer) + digest, err := ctx.digest(transformed, digestAlgorithm, canonicalizer) if err != nil { return nil, err } - decodedDigestValue, err := base64.StdEncoding.DecodeString(digestValue.Text()) + decodedDigestValue, err := base64.StdEncoding.DecodeString(ref.DigestValue) if err != nil { return nil, err } @@ -259,30 +227,15 @@ func (ctx *ValidationContext) validateSignature(el, sig *etree.Element, cert *x5 return nil, errors.New("Signature could not be verified") } - //Verify the signed info - signatureMethod := signedInfo.FindElement(childPath(sig.Space, SignatureMethodTag)) - if signatureMethod == nil { - return nil, errors.New("Missing SignatureMethod") - } - - signatureMethodAlgorithmAttr := signatureMethod.SelectAttr(AlgorithmAttr) - if digestAlgorithmAttr == nil { - return nil, errors.New("Missing SignatureMethod Algorithm attribute") - } - // Decode the 'SignatureValue' so we can compare against it - signatureValue := sig.FindElement(childPath(sig.Space, SignatureValueTag)) - if signatureValue == nil { - return nil, errors.New("Missing SignatureValue") - } - - decodedSignature, err := base64.StdEncoding.DecodeString(signatureValue.Text()) - + decodedSignature, err := base64.StdEncoding.DecodeString(sig.SignatureValue.Data) if err != nil { return nil, errors.New("Could not decode signature") } + // Actually verify the 'SignedInfo' was signed by a trusted source - err = ctx.verifySignedInfo(sig, canonicalizer, signatureMethodAlgorithmAttr.Value, cert, decodedSignature) + signatureMethod := sig.SignedInfo.SignatureMethod.Algorithm + err = ctx.verifySignedInfo(sig, canonicalizer, signatureMethod, cert, decodedSignature) if err != nil { return nil, err } @@ -300,37 +253,88 @@ func contains(roots []*x509.Certificate, cert *x509.Certificate) bool { } // findSignature searches for a Signature element referencing the passed root element. -func (ctx *ValidationContext) findSignature(el *etree.Element) (*etree.Element, error) { +func (ctx *ValidationContext) findSignature(el *etree.Element) (*types.Signature, error) { idAttr := el.SelectAttr(DefaultIdAttr) if idAttr == nil || idAttr.Value == "" { return nil, errors.New("Missing ID attribute") } - var signatureElement *etree.Element + var sig *types.Signature - err := etreeutils.NSFindIterate(el, Namespace, SignatureTag, func(sig *etree.Element) error { - signedInfo := sig.FindElement(childPath(sig.Space, SignedInfoTag)) - if signedInfo == nil { + // Traverse the tree looking for a Signature element + err := etreeutils.NSFindIterate(el, Namespace, SignatureTag, func(ctx etreeutils.NSContext, el *etree.Element) error { + + found := false + err := etreeutils.NSFindIterateCtx(ctx, el, Namespace, SignedInfoTag, + func(ctx etreeutils.NSContext, signedInfo *etree.Element) error { + // Ignore any SignedInfo that isn't an immediate descendent of Signature. + if signedInfo.Parent() != el { + return nil + } + + detachedSignedInfo, err := etreeutils.NSDetatch(ctx, signedInfo) + if err != nil { + return err + } + + c14NMethod := detachedSignedInfo.FindElement(childPath(detachedSignedInfo.Space, CanonicalizationMethodTag)) + if c14NMethod == nil { + return errors.New("missing CanonicalizationMethod on Signature") + } + + c14NAlgorithm := c14NMethod.SelectAttrValue(AlgorithmAttr, "") + + var canonicalSignedInfo *etree.Element + + switch AlgorithmID(c14NAlgorithm) { + case CanonicalXML10ExclusiveAlgorithmId: + err := etreeutils.TransformExcC14n(detachedSignedInfo, "") + if err != nil { + return err + } + + // NOTE: TransformExcC14n transforms the element in-place, + // while canonicalPrep isn't meant to. Once we standardize + // this behavior we can drop this, as well as the adding and + // removing of elements below. + canonicalSignedInfo = detachedSignedInfo + + case CanonicalXML11AlgorithmId: + canonicalSignedInfo = canonicalPrep(detachedSignedInfo, map[string]struct{}{}) + + default: + return fmt.Errorf("invalid CanonicalizationMethod on Signature: %s", c14NAlgorithm) + } + + el.RemoveChild(signedInfo) + el.AddChild(canonicalSignedInfo) + + found = true + + return etreeutils.ErrTraversalHalted + }) + if err != nil { + return err + } + + if !found { return errors.New("Missing SignedInfo") } - referenceElement := signedInfo.FindElement(childPath(sig.Space, ReferenceTag)) - if referenceElement == nil { - return errors.New("Missing Reference Element") + // Unmarshal the signature into a structured Signature type + _sig := &types.Signature{} + err = etreeutils.NSUnmarshalElement(ctx, el, _sig) + if err != nil { + return err } - uriAttr := referenceElement.SelectAttr(URIAttr) - if uriAttr == nil || uriAttr.Value == "" { - 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 = sig - return etreeutils.ErrTraversalHalted + // Traverse references in the signature to determine whether it has at least + // one reference to the top level element. If so, conclude the search. + for _, ref := range _sig.SignedInfo.References { + if ref.URI == "" || ref.URI[1:] == idAttr.Value { + sig = _sig + return etreeutils.ErrTraversalHalted + } } return nil @@ -340,14 +344,14 @@ func (ctx *ValidationContext) findSignature(el *etree.Element) (*etree.Element, return nil, err } - if signatureElement == nil { + if sig == nil { return nil, ErrMissingSignature } - return signatureElement, nil + return sig, nil } -func (ctx *ValidationContext) verifyCertificate(signatureElement *etree.Element) (*x509.Certificate, error) { +func (ctx *ValidationContext) verifyCertificate(sig *types.Signature) (*x509.Certificate, error) { now := ctx.Clock.Now() roots, err := ctx.CertificateStore.Certificates() @@ -357,17 +361,13 @@ func (ctx *ValidationContext) verifyCertificate(signatureElement *etree.Element) 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") + if sig.KeyInfo != nil { + // If the Signature includes KeyInfo, extract the certificate from there + if sig.KeyInfo.X509Data.X509Certificate.Data == "" { + return nil, errors.New("missing X509Certificate within KeyInfo") } - } else { - certData, err := base64.StdEncoding.DecodeString(x509Element.Text()) + + certData, err := base64.StdEncoding.DecodeString(sig.KeyInfo.X509Data.X509Certificate.Data) if err != nil { return nil, errors.New("Failed to parse certificate") } @@ -376,6 +376,13 @@ func (ctx *ValidationContext) verifyCertificate(signatureElement *etree.Element) if err != nil { return nil, err } + } else { + // If the Signature doesn't have KeyInfo, Use the root certificate if there is only one + if len(roots) == 1 { + cert = roots[0] + } else { + return nil, errors.New("Missing x509 Element") + } } // Verify that the certificate is one we trust