Sort signatures by keys IDs for deterministic signature order

.gitignore

@@ -0,0 +1 @@
+.DS_Store

client/testdata/go-tuf/consistent-snapshot-false/1/repository/2.root.json

@@ -1,12 +1,12 @@
 {
 	"signatures": [
-		{
-			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
-			"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
-		},
 		{
 			"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
 			"sig": "20e91b55c995989b270091b714347b15169285cb636ef05d68d49ed7b7a96ebfda13898e0d9ef928c382873b9dba90dca492dbf705d56a4b293adaaed574340f"
+		},
+		{
+			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
+			"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
 		}
 	],
 	"signed": {

client/testdata/go-tuf/consistent-snapshot-false/1/repository/root.json

@@ -1,12 +1,12 @@
 {
 	"signatures": [
-		{
-			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
-			"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
-		},
 		{
 			"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
 			"sig": "20e91b55c995989b270091b714347b15169285cb636ef05d68d49ed7b7a96ebfda13898e0d9ef928c382873b9dba90dca492dbf705d56a4b293adaaed574340f"
+		},
+		{
+			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
+			"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
 		}
 	],
 	"signed": {

client/testdata/go-tuf/consistent-snapshot-false/1/repository/snapshot.json

@@ -2,7 +2,7 @@
 	"signatures": [
 		{
 			"keyid": "289e5a9e71afd7909326aa4caea92f7557ee0e2283d8c31f0c3401ce67248a45",
-			"sig": "9a90d837dd4f3f4020777108de0ea9a06ea1d5e4693247a21feecf8ab5855e356dc0ef909086b664b9535cde8c74d9130cff8faa488d97f0cd2922f61d42f309"
+			"sig": "f832b8fd3e3a2ea65e35d8fe11861d9e252bef6c1a1775e39b4453d93314e0fc6c1b81b819af93ecd1433f707d06e026794e1dd3eccb1f5df5ed99dc3f13d30f"
 		}
 	],
 	"signed": {
@@ -11,7 +11,7 @@
 		"meta": {
 			"root.json": {
 				"hashes": {
-					"sha512": "db01435dcd98422dc54771acf9a0091ac2f6e223ad953dc665261b155bf84fdf4a3f306d197185f0a39fbc7f14ef2165c1aa609b2103dff49c473253f449ee02"
+					"sha512": "6b79dd15ae5aed0b96b5eb6226b27ba9b77f33bdac90a8da9b749120d312cd0240bd254cb8569daa777a7668edfee80681ee988f57416586bf1f7b04eefa4dc0"
 				},
 				"length": 2408,
 				"version": 2

client/testdata/go-tuf/consistent-snapshot-false/1/repository/timestamp.json

@@ -2,7 +2,7 @@
 	"signatures": [
 		{
 			"keyid": "aa3255b4e8e17e566d2bdbea0e5842978f9fa1d2fa9ec76ae76b146164acbfc8",
-			"sig": "bee62f48a1383aadad3ff47ef1eeed4f28158c903c5eee2cfc55b2d4d30f6b665f943e2d2b8f5aa81a67a3ad9edb3b4a4b625cda2f7b7c2806efc7e343758b02"
+			"sig": "5543ad32130a134921cf82d44575ad4b72a276297d6f8eafca116476d7e62b397e75e596f485517afb6c0a74e92f8ff96e1d24a1a341f8c68ceff06363386d07"
 		}
 	],
 	"signed": {
@@ -11,7 +11,7 @@
 		"meta": {
 			"snapshot.json": {
 				"hashes": {
-					"sha512": "f8fd7a63c597491397c7b155c39286c5be15127e0570ecefbb4cf3b6a166e0c35e5ef103e48650d44aedf93ab5d26f91465c970b1c0d0aa75aaefb4d38ec38d7"
+					"sha512": "d5ca4e4060b044075d38132891de10ba7ed9024f0a709674bd76b8c6270afabc16da2a449f5fd8740aebcca7c9828eb7fb28c917aa290a0cf1fc9c0daed879e7"
 				},
 				"length": 847,
 				"version": 2

client/testdata/go-tuf/consistent-snapshot-true/1/repository/2.root.json

@@ -1,12 +1,12 @@
 {
 	"signatures": [
-		{
-			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
-			"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
-		},
 		{
 			"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
 			"sig": "7ac71619b21fe3a076fbbf3e17f92177c5374f005c32f1818e7c92eee107e3c726ccf906e800878035a9caf7679610147d72cb515cd76164b08b5c8bf93c0f0e"
+		},
+		{
+			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
+			"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
 		}
 	],
 	"signed": {

client/testdata/go-tuf/consistent-snapshot-true/1/repository/2.snapshot.json

@@ -2,7 +2,7 @@
 	"signatures": [
 		{
 			"keyid": "289e5a9e71afd7909326aa4caea92f7557ee0e2283d8c31f0c3401ce67248a45",
-			"sig": "25c7977a662d4cd511eb348284a0420eb62e654f90e10338e0653bc2f7428af8033818ba2901734956112bdcf95ebfd9f86070c6ed75ae4c3a21ebf7098b1f04"
+			"sig": "a9aa74e87f6ee1719d0c2cb3a37c2ff63bee3648bff572377b9169833012ed50b6d5a4604e668e200600ed6ef7c622e998e44580d4358aa638ab767e5de90509"
 		}
 	],
 	"signed": {
@@ -11,7 +11,7 @@
 		"meta": {
 			"root.json": {
 				"hashes": {
-					"sha512": "645840e3191d531617ebab3de88e77461f1ff673cd6c857f23f87860e61ab6d80880933bb2b4f353dd89894c84b3497e3e02d791eaaab3750f4beabbba763c7a"
+					"sha512": "51cd60db1c77e80b7d4553bff37eba869eb7ff9e3eb2726f6e550f4209ca8bde123a74f04cd42b0ac3a34af92f32e313ff8d5b64488f3b46c4c4fc4b836fc003"
 				},
 				"length": 2407,
 				"version": 2

client/testdata/go-tuf/consistent-snapshot-true/1/repository/root.json

@@ -1,12 +1,12 @@
 {
 	"signatures": [
-		{
-			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
-			"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
-		},
 		{
 			"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
 			"sig": "7ac71619b21fe3a076fbbf3e17f92177c5374f005c32f1818e7c92eee107e3c726ccf906e800878035a9caf7679610147d72cb515cd76164b08b5c8bf93c0f0e"
+		},
+		{
+			"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
+			"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
 		}
 	],
 	"signed": {

client/testdata/go-tuf/consistent-snapshot-true/1/repository/snapshot.json

@@ -2,7 +2,7 @@
 	"signatures": [
 		{
 			"keyid": "289e5a9e71afd7909326aa4caea92f7557ee0e2283d8c31f0c3401ce67248a45",
-			"sig": "25c7977a662d4cd511eb348284a0420eb62e654f90e10338e0653bc2f7428af8033818ba2901734956112bdcf95ebfd9f86070c6ed75ae4c3a21ebf7098b1f04"
+			"sig": "a9aa74e87f6ee1719d0c2cb3a37c2ff63bee3648bff572377b9169833012ed50b6d5a4604e668e200600ed6ef7c622e998e44580d4358aa638ab767e5de90509"
 		}
 	],
 	"signed": {
@@ -11,7 +11,7 @@
 		"meta": {
 			"root.json": {
 				"hashes": {
-					"sha512": "645840e3191d531617ebab3de88e77461f1ff673cd6c857f23f87860e61ab6d80880933bb2b4f353dd89894c84b3497e3e02d791eaaab3750f4beabbba763c7a"
+					"sha512": "51cd60db1c77e80b7d4553bff37eba869eb7ff9e3eb2726f6e550f4209ca8bde123a74f04cd42b0ac3a34af92f32e313ff8d5b64488f3b46c4c4fc4b836fc003"
 				},
 				"length": 2407,
 				"version": 2

client/testdata/go-tuf/consistent-snapshot-true/1/repository/timestamp.json

@@ -2,7 +2,7 @@
 	"signatures": [
 		{
 			"keyid": "aa3255b4e8e17e566d2bdbea0e5842978f9fa1d2fa9ec76ae76b146164acbfc8",
-			"sig": "d2674f7afdfd013c9a8bdaf716db22521ce3929cb35917893ebae679a789d30ce875a4657c39906883766b5b80efe3ff96835fcbe704e08fda0e8887c936650f"
+			"sig": "ace63f55631ff695d7ad960b4f40cf6013732f76edcb945e89798c21304df43c03c0f0e6e322dba35d488c517de5681318600813ccedb3f166f6ceb4f462930f"
 		}
 	],
 	"signed": {
@@ -11,7 +11,7 @@
 		"meta": {
 			"snapshot.json": {
 				"hashes": {
-					"sha512": "e03715cdd4fc212599d320b3392052e1c452b3426d5658c9faee72b31df65e80823f42cb8649fae4d950f73736492a501e5f4569741d102386a4cb5194512b91"
+					"sha512": "08a4fe01b9da9d07909d8ec02d2ea870b67fcc816751e56c61a8aebc587f051003f4b8a77c268f2a97b79997f7cbf836855e8638fc1df6c5df54ab7b49b2c671"
 				},
 				"length": 847,
 				"version": 2

internal/signer/sort.go

@@ -0,0 +1,48 @@
+package signer
+
+import (
+	"sort"
+
+	"github.com/theupdateframework/go-tuf/sign"
+)
+
+// ByIDs implements sort.Interface for []sign.Signer based on the sorted
+// IDs() for each Signer. This facilitates deterministic order of signatures,
+// which prevents tests that use fixtures from being flaky.
+type ByIDs []sign.Signer
+
+func (b ByIDs) Len() int {
+	return len(b)
+}
+
+func (b ByIDs) Swap(i, j int) {
+	b[i], b[j] = b[j], b[i]
+}
+
+func (b ByIDs) Less(i, j int) bool {
+	ids := b[i].IDs()
+	iIDs := make([]string, len(ids))
+	copy(iIDs, ids)
+	sort.Strings(iIDs)
+
+	ids = b[j].IDs()
+	jIDs := make([]string, len(ids))
+	copy(jIDs, ids)
+	sort.Strings(jIDs)
+
+	minLen := len(iIDs)
+	if len(jIDs) < minLen {
+		minLen = len(jIDs)
+	}
+
+	// Compare iIDs[:minLen] to jIDs[:minLen] element-wise.
+	for c := 0; c < minLen; c++ {
+		if iIDs[c] == jIDs[c] {
+			continue
+		}
+		return iIDs[c] < jIDs[c]
+	}
+
+	// iIDs[:minLen] is equal to jIDs[:minLen], so sort based on length.
+	return len(iIDs) < len(jIDs)
+}

repo.go

@@ -6,11 +6,13 @@ import (
 	"fmt"
 	"io"
 	"path"
+	"sort"
 	"strings"
 	"time"
 
 	cjson "github.com/tent/canonical-json-go"
 	"github.com/theupdateframework/go-tuf/data"
+	"github.com/theupdateframework/go-tuf/internal/signer"
 	"github.com/theupdateframework/go-tuf/sign"
 	"github.com/theupdateframework/go-tuf/util"
 	"github.com/theupdateframework/go-tuf/verify"
@@ -605,7 +607,10 @@ func (r *Repo) getSigningKeys(name string) ([]sign.Signer, error) {
 		return nil, err
 	}
 	if name == "root" {
-		return signingKeys, nil
+		sorted := make([]sign.Signer, len(signingKeys))
+		copy(sorted, signingKeys)
+		sort.Sort(signer.ByIDs(sorted))
+		return sorted, nil
 	}
 	db, err := r.db()
 	if err != nil {
@@ -626,6 +631,9 @@ func (r *Repo) getSigningKeys(name string) ([]sign.Signer, error) {
 			}
 		}
 	}
+
+	sort.Sort(signer.ByIDs(keys))
+
 	return keys, nil
 }
 

repo_test.go

@@ -9,6 +9,8 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"reflect"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -19,9 +21,16 @@ import (
 	"github.com/theupdateframework/go-tuf/util"
 	"github.com/theupdateframework/go-tuf/verify"
 	"golang.org/x/crypto/ed25519"
+	"gopkg.in/check.v1"
 	. "gopkg.in/check.v1"
 )
 
+func sortStrings(in []string) []string {
+	out := append([]string{}, in...)
+	sort.Strings(out)
+	return out
+}
+
 // Hook up gocheck into the "go test" runner.
 func Test(t *testing.T) { TestingT(t) }
 
@@ -543,9 +552,17 @@ func (rs *RepoSuite) TestSign(c *C) {
 		s := &data.Signed{}
 		c.Assert(json.Unmarshal(rootJSON, s), IsNil)
 		c.Assert(s.Signatures, HasLen, len(keyIDs))
-		for i, id := range keyIDs {
-			c.Assert(s.Signatures[i].KeyID, Equals, id)
+
+		// Signatures may be in any order, so must sort key IDs before comparison.
+		wantKeyIDs := append([]string{}, keyIDs...)
+		sort.Strings(wantKeyIDs)
+
+		gotKeyIDs := []string{}
+		for _, sig := range s.Signatures {
+			gotKeyIDs = append(gotKeyIDs, sig.KeyID)
 		}
+
+		c.Assert(sortStrings(keyIDs), DeepEquals, sortStrings(gotKeyIDs))
 	}
 
 	// signing with an available key generates a signature
@@ -1170,17 +1187,49 @@ func (rs *RepoSuite) TestKeyPersistence(c *C) {
 			c.Assert(pk.Encrypted, Equals, false)
 			c.Assert(json.Unmarshal(pk.Data, &actual), IsNil)
 		}
+
+		// Compare slices of unique elements disregarding order.
 		c.Assert(actual, HasLen, len(expected))
-		for i, key := range expected {
-			c.Assert(expected[i], DeepEquals, key)
+		for _, gotKey := range actual {
+			expectedNumMatches := 0
+			for _, x := range actual {
+				if reflect.DeepEqual(gotKey, x) {
+					expectedNumMatches++
+				}
+			}
+
+			numMatches := 0
+			for _, wantKey := range expected {
+				if reflect.DeepEqual(gotKey, wantKey) {
+					numMatches++
+				}
+			}
+
+			c.Assert(numMatches, Equals, expectedNumMatches, check.Commentf("actual: %+v, expected: %+v", actual, expected))
 		}
 
 		// check GetKeys is correct
 		signers, err := store.GetSigningKeys(role)
 		c.Assert(err, IsNil)
+
+		// Compare slices of unique elements disregarding order.
 		c.Assert(signers, HasLen, len(expected))
-		for i, s := range signers {
-			c.Assert(s.IDs(), DeepEquals, expected[i].PublicData().IDs())
+		for _, s := range signers {
+			expectedNumMatches := 0
+			for _, x := range signers {
+				if reflect.DeepEqual(s, x) {
+					expectedNumMatches++
+				}
+			}
+
+			numMatches := 0
+			for _, e := range expected {
+				if reflect.DeepEqual(s.IDs(), e.PublicData().IDs()) {
+					numMatches++
+				}
+			}
+
+			c.Assert(numMatches, Equals, expectedNumMatches, check.Commentf("signers: %+v, expected: %+v", signers, expected))
 		}
 	}