Skip to content

Commit

Permalink
Sort signatures by keys IDs for deterministic signature order (#155)
Browse files Browse the repository at this point in the history
* Sort signatures by key ID

* Improve slice comparison

* Fix assert comment

* Remove sortStrings test helper

* Add test for Signer sort

* Sort keys before usage

* Revert "Sort keys before usage"

This reverts commit 88d9340.

* Rename to getSortedSigningKeys

* Check sort in a different way
  • Loading branch information
ethan-lowman-dd authored Oct 18, 2021
1 parent 7ba0400 commit 5908a16
Show file tree
Hide file tree
Showing 14 changed files with 225 additions and 39 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
.DS_Store
cmd/tuf/tuf
cmd/tuf-client/tuf-client
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"signatures": [
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
},
{
"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
"sig": "20e91b55c995989b270091b714347b15169285cb636ef05d68d49ed7b7a96ebfda13898e0d9ef928c382873b9dba90dca492dbf705d56a4b293adaaed574340f"
},
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
}
],
"signed": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"signatures": [
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
},
{
"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
"sig": "20e91b55c995989b270091b714347b15169285cb636ef05d68d49ed7b7a96ebfda13898e0d9ef928c382873b9dba90dca492dbf705d56a4b293adaaed574340f"
},
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "65f63745aa8bd39132f827c427ea6d87f620ec805d9f376b6a8400dc3db7eb964e5423d31ed08867916d039661a70d6bf255bca552248021a4e78b5d357c2b0f"
}
],
"signed": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"signatures": [
{
"keyid": "289e5a9e71afd7909326aa4caea92f7557ee0e2283d8c31f0c3401ce67248a45",
"sig": "9a90d837dd4f3f4020777108de0ea9a06ea1d5e4693247a21feecf8ab5855e356dc0ef909086b664b9535cde8c74d9130cff8faa488d97f0cd2922f61d42f309"
"sig": "f832b8fd3e3a2ea65e35d8fe11861d9e252bef6c1a1775e39b4453d93314e0fc6c1b81b819af93ecd1433f707d06e026794e1dd3eccb1f5df5ed99dc3f13d30f"
}
],
"signed": {
Expand All @@ -11,7 +11,7 @@
"meta": {
"root.json": {
"hashes": {
"sha512": "db01435dcd98422dc54771acf9a0091ac2f6e223ad953dc665261b155bf84fdf4a3f306d197185f0a39fbc7f14ef2165c1aa609b2103dff49c473253f449ee02"
"sha512": "6b79dd15ae5aed0b96b5eb6226b27ba9b77f33bdac90a8da9b749120d312cd0240bd254cb8569daa777a7668edfee80681ee988f57416586bf1f7b04eefa4dc0"
},
"length": 2408,
"version": 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"signatures": [
{
"keyid": "aa3255b4e8e17e566d2bdbea0e5842978f9fa1d2fa9ec76ae76b146164acbfc8",
"sig": "bee62f48a1383aadad3ff47ef1eeed4f28158c903c5eee2cfc55b2d4d30f6b665f943e2d2b8f5aa81a67a3ad9edb3b4a4b625cda2f7b7c2806efc7e343758b02"
"sig": "5543ad32130a134921cf82d44575ad4b72a276297d6f8eafca116476d7e62b397e75e596f485517afb6c0a74e92f8ff96e1d24a1a341f8c68ceff06363386d07"
}
],
"signed": {
Expand All @@ -11,7 +11,7 @@
"meta": {
"snapshot.json": {
"hashes": {
"sha512": "f8fd7a63c597491397c7b155c39286c5be15127e0570ecefbb4cf3b6a166e0c35e5ef103e48650d44aedf93ab5d26f91465c970b1c0d0aa75aaefb4d38ec38d7"
"sha512": "d5ca4e4060b044075d38132891de10ba7ed9024f0a709674bd76b8c6270afabc16da2a449f5fd8740aebcca7c9828eb7fb28c917aa290a0cf1fc9c0daed879e7"
},
"length": 847,
"version": 2
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"signatures": [
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
},
{
"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
"sig": "7ac71619b21fe3a076fbbf3e17f92177c5374f005c32f1818e7c92eee107e3c726ccf906e800878035a9caf7679610147d72cb515cd76164b08b5c8bf93c0f0e"
},
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
}
],
"signed": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"signatures": [
{
"keyid": "289e5a9e71afd7909326aa4caea92f7557ee0e2283d8c31f0c3401ce67248a45",
"sig": "25c7977a662d4cd511eb348284a0420eb62e654f90e10338e0653bc2f7428af8033818ba2901734956112bdcf95ebfd9f86070c6ed75ae4c3a21ebf7098b1f04"
"sig": "a9aa74e87f6ee1719d0c2cb3a37c2ff63bee3648bff572377b9169833012ed50b6d5a4604e668e200600ed6ef7c622e998e44580d4358aa638ab767e5de90509"
}
],
"signed": {
Expand All @@ -11,7 +11,7 @@
"meta": {
"root.json": {
"hashes": {
"sha512": "645840e3191d531617ebab3de88e77461f1ff673cd6c857f23f87860e61ab6d80880933bb2b4f353dd89894c84b3497e3e02d791eaaab3750f4beabbba763c7a"
"sha512": "51cd60db1c77e80b7d4553bff37eba869eb7ff9e3eb2726f6e550f4209ca8bde123a74f04cd42b0ac3a34af92f32e313ff8d5b64488f3b46c4c4fc4b836fc003"
},
"length": 2407,
"version": 2
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"signatures": [
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
},
{
"keyid": "b2403f96ae9b1089d8cbc15bbc35e9acbacd7984571f951b43aab56aedcfa84f",
"sig": "7ac71619b21fe3a076fbbf3e17f92177c5374f005c32f1818e7c92eee107e3c726ccf906e800878035a9caf7679610147d72cb515cd76164b08b5c8bf93c0f0e"
},
{
"keyid": "ce72db3f938914205461a415c9b7b91267a2079df991fd6283aa8461988c1add",
"sig": "e560f386c0c270bb44cedda965ad21526e65162b41a2c30d84fb3a80f58913432e8dc74e5e7b0635ccdf51a2b951dae3b0ba28b4aac4088641a2b2cd2933dd04"
}
],
"signed": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"signatures": [
{
"keyid": "289e5a9e71afd7909326aa4caea92f7557ee0e2283d8c31f0c3401ce67248a45",
"sig": "25c7977a662d4cd511eb348284a0420eb62e654f90e10338e0653bc2f7428af8033818ba2901734956112bdcf95ebfd9f86070c6ed75ae4c3a21ebf7098b1f04"
"sig": "a9aa74e87f6ee1719d0c2cb3a37c2ff63bee3648bff572377b9169833012ed50b6d5a4604e668e200600ed6ef7c622e998e44580d4358aa638ab767e5de90509"
}
],
"signed": {
Expand All @@ -11,7 +11,7 @@
"meta": {
"root.json": {
"hashes": {
"sha512": "645840e3191d531617ebab3de88e77461f1ff673cd6c857f23f87860e61ab6d80880933bb2b4f353dd89894c84b3497e3e02d791eaaab3750f4beabbba763c7a"
"sha512": "51cd60db1c77e80b7d4553bff37eba869eb7ff9e3eb2726f6e550f4209ca8bde123a74f04cd42b0ac3a34af92f32e313ff8d5b64488f3b46c4c4fc4b836fc003"
},
"length": 2407,
"version": 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"signatures": [
{
"keyid": "aa3255b4e8e17e566d2bdbea0e5842978f9fa1d2fa9ec76ae76b146164acbfc8",
"sig": "d2674f7afdfd013c9a8bdaf716db22521ce3929cb35917893ebae679a789d30ce875a4657c39906883766b5b80efe3ff96835fcbe704e08fda0e8887c936650f"
"sig": "ace63f55631ff695d7ad960b4f40cf6013732f76edcb945e89798c21304df43c03c0f0e6e322dba35d488c517de5681318600813ccedb3f166f6ceb4f462930f"
}
],
"signed": {
Expand All @@ -11,7 +11,7 @@
"meta": {
"snapshot.json": {
"hashes": {
"sha512": "e03715cdd4fc212599d320b3392052e1c452b3426d5658c9faee72b31df65e80823f42cb8649fae4d950f73736492a501e5f4569741d102386a4cb5194512b91"
"sha512": "08a4fe01b9da9d07909d8ec02d2ea870b67fcc816751e56c61a8aebc587f051003f4b8a77c268f2a97b79997f7cbf836855e8638fc1df6c5df54ab7b49b2c671"
},
"length": 847,
"version": 2
Expand Down
49 changes: 49 additions & 0 deletions internal/signer/sort.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package signer

import (
"sort"

"github.com/theupdateframework/go-tuf/pkg/keys"
)

// ByIDs implements sort.Interface for []keys.Signer based on
// the sorted public IDs() for each Signer. This facilitates
// deterministic order of signatures, which prevents tests
// that use fixtures from being flaky.
type ByIDs []keys.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].PublicData().IDs()
iIDs := make([]string, len(ids))
copy(iIDs, ids)
sort.Strings(iIDs)

ids = b[j].PublicData().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)
}
76 changes: 76 additions & 0 deletions internal/signer/sort_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package signer_test

import (
"encoding/json"
"sort"
"testing"

"github.com/theupdateframework/go-tuf/data"
"github.com/theupdateframework/go-tuf/internal/signer"
"github.com/theupdateframework/go-tuf/pkg/keys"
)

type mockSigner struct {
value json.RawMessage
}

func (s *mockSigner) MarshalPrivateKey() (*data.PrivateKey, error) {
panic("not implemented")
return nil, nil
}

func (s *mockSigner) UnmarshalPrivateKey(key *data.PrivateKey) error {
panic("not implemented")
return nil
}

func (s *mockSigner) PublicData() *data.PublicKey {
return &data.PublicKey{
Type: "mock",
Scheme: "mock",
Algorithms: []string{"mock"},
Value: s.value,
}
}
func (s *mockSigner) SignMessage(message []byte) ([]byte, error) {
panic("not implemented")
return nil, nil
}

func TestSignerSortByIDs(t *testing.T) {
s1 := &mockSigner{
value: json.RawMessage(`{"mock": 1}`),
}
s2 := &mockSigner{
value: json.RawMessage(`{"mock": 2}`),
}
s3 := &mockSigner{
value: json.RawMessage(`{"mock": 3}`),
}
s4 := &mockSigner{
value: json.RawMessage(`{"mock": 4}`),
}
s5 := &mockSigner{
value: json.RawMessage(`{"mock": 5}`),
}

s := []keys.Signer{
s1, s2, s3, s4, s5,
}

sort.Sort(signer.ByIDs(s))

signerIDs := []string{}

for i, signer := range s {
ids := signer.PublicData().IDs()
if len(ids) != 1 {
t.Errorf("Signer %v IDs %v should have length 1", i, ids)
}
signerIDs = append(signerIDs, ids[0])
}

if !sort.StringsAreSorted(signerIDs) {
t.Errorf("Signers incorrectly sorted: %+v", signerIDs)
}
}
18 changes: 13 additions & 5 deletions repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/pkg/keys"
"github.com/theupdateframework/go-tuf/sign"
"github.com/theupdateframework/go-tuf/util"
Expand Down Expand Up @@ -498,7 +500,7 @@ func (r *Repo) jsonMarshal(v interface{}) ([]byte, error) {
}

func (r *Repo) setMeta(roleFilename string, meta interface{}) error {
keys, err := r.getSigningKeys(strings.TrimSuffix(roleFilename, ".json"))
keys, err := r.getSortedSigningKeys(strings.TrimSuffix(roleFilename, ".json"))
if err != nil {
return err
}
Expand All @@ -525,7 +527,7 @@ func (r *Repo) Sign(roleFilename string) error {
return err
}

keys, err := r.getSigningKeys(role)
keys, err := r.getSortedSigningKeys(role)
if err != nil {
return err
}
Expand Down Expand Up @@ -597,19 +599,22 @@ func (r *Repo) AddOrUpdateSignature(roleFilename string, signature data.Signatur
return r.local.SetMeta(roleFilename, b)
}

// getSigningKeys returns available signing keys.
// getSortedSigningKeys returns available signing keys, sorted by key ID.
//
// Only keys contained in the keys db are returned (i.e. local keys which have
// been revoked are omitted), except for the root role in which case all local
// keys are returned (revoked root keys still need to sign new root metadata so
// clients can verify the new root.json and update their keys db accordingly).
func (r *Repo) getSigningKeys(name string) ([]keys.Signer, error) {
func (r *Repo) getSortedSigningKeys(name string) ([]keys.Signer, error) {
signingKeys, err := r.local.GetSigners(name)
if err != nil {
return nil, err
}
if name == "root" {
return signingKeys, nil
sorted := make([]keys.Signer, len(signingKeys))
copy(sorted, signingKeys)
sort.Sort(signer.ByIDs(sorted))
return sorted, nil
}
db, err := r.db()
if err != nil {
Expand All @@ -630,6 +635,9 @@ func (r *Repo) getSigningKeys(name string) ([]keys.Signer, error) {
}
}
}

sort.Sort(signer.ByIDs(keys))

return keys, nil
}

Expand Down
Loading

0 comments on commit 5908a16

Please sign in to comment.