diff --git a/etreeutils/unmarshal.go b/etreeutils/unmarshal.go index b1fecf8..3e6e0a0 100644 --- a/etreeutils/unmarshal.go +++ b/etreeutils/unmarshal.go @@ -9,7 +9,7 @@ import ( // 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 { +func NSUnmarshalElement(ctx NSContext, root, el *etree.Element, v interface{}) error { detatched, err := NSDetatch(ctx, el) if err != nil { return err @@ -29,7 +29,7 @@ func NSUnmarshalElement(ctx NSContext, el *etree.Element, v interface{}) error { switch v := v.(type) { case ElementKeeper: - v.SetUnderlyingElement(el) + v.SetUnderlyingElement(root, el) } return nil @@ -38,6 +38,6 @@ func NSUnmarshalElement(ctx NSContext, el *etree.Element, v interface{}) error { // ElementKeeper should be implemented by types which will be passed to // UnmarshalElement, but wish to keep a reference type ElementKeeper interface { - SetUnderlyingElement(*etree.Element) + SetUnderlyingElement(root, el *etree.Element) UnderlyingElement() *etree.Element } diff --git a/main_test.go b/main_test.go index 0e709c3..0838a98 100644 --- a/main_test.go +++ b/main_test.go @@ -81,3 +81,53 @@ func TestManifestExample(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, manifest) } + +func TestRecursiveSigning(t *testing.T) { + + // Generate a key and self-signed certificate for signing + randomKeyStore := RandomKeyStoreForTest() + ctx := NewDefaultSigningContext(randomKeyStore) + + test := []byte{0x45, 0xf1, 0xab, 0xd7, 0x8a, 0x6f, 0x92, 0xe6, 0xa4, 0xb6, 0x8e, 0xba, 0x8f, 0xe7, 0x91, 0x96, 0xe0, 0xb2, 0x16, 0xd6, 0x0b, 0x82, 0x1b, 0x00, 0x45, 0xfa, 0xb8, 0xad, 0xd4, 0xfa, 0xff, 0xf9} + + sig := ctx.CreateSignature("id1234") + err := ctx.AddManifestRef(sig, "package", crypto.SHA256, test) + require.NoError(t, err) + + // Sign the signature + signed, err := ctx.Sign(sig) + require.NoError(t, err) + + list := &etree.Element{Tag: "Signatures"} + list.AddChild(signed) + + // create second layer + signed, err = ctx.SignEnveloped(list) + require.NoError(t, err) + + // Validate + _, certData, err := ctx.KeyStore.GetKeyPair() + require.NoError(t, err) + + cert, err := x509.ParseCertificate(certData) + require.NoError(t, err) + + // Construct a signing context with one or more roots of trust. + vctx := NewDefaultValidationContext(&MemoryX509CertificateStore{ + Roots: []*x509.Certificate{cert}, + }) + + // It is important to only use the returned validated element. + // See: https://www.w3.org/TR/xmldsig-bestpractices/#check-what-is-signed + manifest, err := vctx.ValidateManifest(signed) + + require.NoError(t, err) + require.NotEmpty(t, manifest) + require.Equal(t, len(manifest.References), 1) + + hash, digest, err := vctx.DecodeRef(&manifest.References[0]) + + require.NoError(t, err) + require.Equal(t, digest, test) + require.Equal(t, hash, crypto.SHA256) +} diff --git a/types/signature.go b/types/signature.go index d9f73fb..a0cb7a9 100644 --- a/types/signature.go +++ b/types/signature.go @@ -84,13 +84,17 @@ type Signature struct { SignatureValue *SignatureValue `xml:"SignatureValue"` KeyInfo *KeyInfo `xml:"KeyInfo"` el *etree.Element + path []int RootRef *Reference } // SetUnderlyingElement will be called with a reference to the Element this Signature // was unmarshaled from. -func (s *Signature) SetUnderlyingElement(el *etree.Element) { +func (s *Signature) SetUnderlyingElement(root, el *etree.Element) { s.el = el + // map the path to the passed signature relative to the passed root, in order + // to enable removal of the signature by an enveloped signature transform + s.path = mapPathToElement(root, el) } // UnderlyingElement returns a reference to the Element this signature was unmarshaled @@ -98,3 +102,47 @@ func (s *Signature) SetUnderlyingElement(el *etree.Element) { func (s *Signature) UnderlyingElement() *etree.Element { return s.el } + +func (s *Signature) RemoveUnderlyingElement(el *etree.Element) bool { + return removeElementAtPath(el, s.path) +} + +func mapPathToElement(tree, el *etree.Element) []int { + for i, child := range tree.Child { + if child == el { + return []int{i} + } + } + + for i, child := range tree.Child { + if childElement, ok := child.(*etree.Element); ok { + childPath := mapPathToElement(childElement, el) + if childPath != nil { + return append([]int{i}, childPath...) + } + } + } + + return nil +} + +func removeElementAtPath(el *etree.Element, path []int) bool { + + if len(path) == 0 { + return false + } + + if path[0] < len(el.Child) { + + if len(path) == 1 { + el.RemoveChildAt(path[0]) + return true + } + + childElement, ok := el.Child[path[0]].(*etree.Element) + if ok { + return removeElementAtPath(childElement, path[1:]) + } + } + return false +} diff --git a/types/signature_test.go b/types/signature_test.go new file mode 100644 index 0000000..09bd0a8 --- /dev/null +++ b/types/signature_test.go @@ -0,0 +1,28 @@ +package types + +import ( + "testing" + + "github.com/austdev/goxmldsig/etreeutils" + + "github.com/beevik/etree" + "github.com/stretchr/testify/require" +) + +func TestMapPathAndRemove(t *testing.T) { + doc := etree.NewDocument() + err := doc.ReadFromString(``) + require.NoError(t, err) + + el, err := etreeutils.NSFindOne(doc.Root(), "x", "RemoveMe") + require.NoError(t, err) + require.NotNil(t, el) + + path := mapPathToElement(doc.Root(), el) + removed := removeElementAtPath(doc.Root(), path) + require.True(t, removed) + + el, err = etreeutils.NSFindOne(doc.Root(), "x", "RemoveMe") + require.NoError(t, err) + require.Nil(t, el) +} diff --git a/validate.go b/validate.go index 57f5c49..6832f8f 100644 --- a/validate.go +++ b/validate.go @@ -79,46 +79,6 @@ func childPath(space, tag string) string { } } -func mapPathToElement(tree, el *etree.Element) []int { - for i, child := range tree.Child { - if child == el { - return []int{i} - } - } - - for i, child := range tree.Child { - if childElement, ok := child.(*etree.Element); ok { - childPath := mapPathToElement(childElement, el) - if childPath != nil { - return append([]int{i}, childPath...) - } - } - } - - return nil -} - -func removeElementAtPath(el *etree.Element, path []int) bool { - - if len(path) == 0 { - return false - } - - if path[0] < len(el.Child) { - - if len(path) == 1 { - el.RemoveChildAt(path[0]) - return true - } - - childElement, ok := el.Child[path[0]].(*etree.Element) - if ok { - return removeElementAtPath(childElement, path[1:]) - } - } - return false -} - // Transform returns a new element equivalent to the passed root el, but with // the set of transformations described by the ref applied. // @@ -133,11 +93,6 @@ func (ctx *ValidationContext) transform( } transforms := ref.Transforms.Transforms - // map the path to the passed signature relative to the passed root, in - // order to enable removal of the signature by an enveloped signature - // transform - signaturePath := mapPathToElement(el, sig.UnderlyingElement()) - var canonicalizer Canonicalizer for _, transform := range transforms { @@ -146,7 +101,7 @@ func (ctx *ValidationContext) transform( switch AlgorithmID(algo) { case EnvelopedSignatureAltorithmId: el = el.Copy() // make a copy of the passed root - if !removeElementAtPath(el, signaturePath) { + if !sig.RemoveUnderlyingElement(el) { return nil, nil, fmt.Errorf("%w: error applying canonicalization transform: Signature not found", ErrInvalidSignature) } @@ -407,7 +362,7 @@ func (ctx *ValidationContext) findSignature(root *etree.Element) (*types.Signatu // Unmarshal the signature into a structured Signature type _sig := &types.Signature{} - err = etreeutils.NSUnmarshalElement(ctx, signatureEl, _sig) + err = etreeutils.NSUnmarshalElement(ctx, root, signatureEl, _sig) if err != nil { return err } @@ -591,12 +546,10 @@ func (ctx *ValidationContext) Validate(el *etree.Element) (*etree.Element, error // Validate verifies that the passed element contains a valid signatures // matching a currently-valid certificate in the context's CertificateStore. func (ctx *ValidationContext) ValidateManifest(el *etree.Element) (*types.Manifest, error) { - // Make a copy of the element to avoid mutating the one we were passed. - el = el.Copy() for { - - sig, err := ctx.findSignature(el) + // Make a copy of the element to avoid mutating the one we were passed. + sig, err := ctx.findSignature(el.Copy()) if err != nil { return nil, err } diff --git a/validate_test.go b/validate_test.go index 7516376..7de7438 100644 --- a/validate_test.go +++ b/validate_test.go @@ -286,24 +286,6 @@ func TestValidateWithModifiedAndSignatureEdited(t *testing.T) { require.Error(t, err) } -func TestMapPathAndRemove(t *testing.T) { - doc := etree.NewDocument() - err := doc.ReadFromString(``) - require.NoError(t, err) - - el, err := etreeutils.NSFindOne(doc.Root(), "x", "RemoveMe") - require.NoError(t, err) - require.NotNil(t, el) - - path := mapPathToElement(doc.Root(), el) - removed := removeElementAtPath(doc.Root(), path) - require.True(t, removed) - - el, err = etreeutils.NSFindOne(doc.Root(), "x", "RemoveMe") - require.NoError(t, err) - require.Nil(t, el) -} - const ( validExample = `http://www.okta.com/exkrfkzzb7NyB3UeP0h7LwRDkrPmsTcUa++BIS5VJIANUlZN7zzdtjLfxfLAWds=UyjNRj9ZFbhApPhWEuVG26yACVqd25uyRKalSpp6XCdjrqKjI8Fmx7Q/IFkk5M755cxyFCQGttxThR6IPBk4Kp5OG2qGKXNHt7OQ8mumSLqWZpBJbmzNIKyG3nWlFoLVCoWPtBTd2gZM0aHOQp1JKa1birFBp2NofkEXbLeghZQ2YfCc4m8qgpZW5k/Itc0P/TVIkvPInjdSMyjm/ql4FUDO8cMkExJNR/i+GElW8cfnniWGcDPSiOqfIjLEDvZouXC7F1v5Wa0SmIxg7NJUTB+g6yrDN15VDq3KbHHTMlZXOZTXON2mBZOj5cwyyd4uX3aGSmYQiy/CGqBdqxrW2A==MIIDnjCCAoagAwIBAgIGAXHxS90vMA0GCSqGSIb3DQEBCwUAMIGPMQswCQYDVQQGEwJVUzETMBEG A1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzENMAsGA1UECgwET2t0YTEU