From f0ddda1602a578fec810d51d5828db4156357d74 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 15 Feb 2016 16:03:48 -0800 Subject: [PATCH 1/8] Add methods for mutating per-metadata data to swizzler Signed-off-by: Ying Li --- tuf/testutils/swizzler.go | 132 +++++++++++++++++++++++++++++++++ tuf/testutils/swizzler_test.go | 93 +++++++++++++++++++++++ 2 files changed, 225 insertions(+) diff --git a/tuf/testutils/swizzler.go b/tuf/testutils/swizzler.go index 6150bda751..f62ef4d947 100644 --- a/tuf/testutils/swizzler.go +++ b/tuf/testutils/swizzler.go @@ -532,3 +532,135 @@ func (m *MetadataSwizzler) UpdateTimestampHash() error { } return m.MetadataCache.SetMeta(data.CanonicalTimestampRole, metaBytes) } + +// MutateRoot takes a function that mutates the root metadata - once done, it +// serializes the root again +func (m *MetadataSwizzler) MutateRoot(mutate func(*data.Root)) error { + signedThing, err := signedFromStore(m.MetadataCache, data.CanonicalRootRole) + if err != nil { + return err + } + + var root data.Root + if err := json.Unmarshal(signedThing.Signed, &root); err != nil { + return err + } + + mutate(&root) + + rootBytes, err := json.MarshalCanonical(root) + if err != nil { + return err + } + signedThing.Signed = json.RawMessage(rootBytes) + + pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalRootRole) + if err != nil { + return err + } + + metaBytes, err := serializeMetadata(m.CryptoService, signedThing, data.CanonicalRootRole, pubKeys...) + if err != nil { + return err + } + return m.MetadataCache.SetMeta(data.CanonicalRootRole, metaBytes) +} + +// MutateTimestamp takes a function that mutates the timestamp metadata - once done, it +// serializes the timestamp again +func (m *MetadataSwizzler) MutateTimestamp(mutate func(*data.Timestamp)) error { + signedThing, err := signedFromStore(m.MetadataCache, data.CanonicalTimestampRole) + if err != nil { + return err + } + + var timestamp data.Timestamp + if err := json.Unmarshal(signedThing.Signed, ×tamp); err != nil { + return err + } + + mutate(×tamp) + + timestampBytes, err := json.MarshalCanonical(timestamp) + if err != nil { + return err + } + signedThing.Signed = json.RawMessage(timestampBytes) + + pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalTimestampRole) + if err != nil { + return err + } + + metaBytes, err := serializeMetadata(m.CryptoService, signedThing, data.CanonicalTimestampRole, pubKeys...) + if err != nil { + return err + } + return m.MetadataCache.SetMeta(data.CanonicalTimestampRole, metaBytes) +} + +// MutateSnapshot takes a function that mutates the snapshot metadata - once done, it +// serializes the snapshot again +func (m *MetadataSwizzler) MutateSnapshot(mutate func(*data.Snapshot)) error { + signedThing, err := signedFromStore(m.MetadataCache, data.CanonicalSnapshotRole) + if err != nil { + return err + } + + var snapshot data.Snapshot + if err := json.Unmarshal(signedThing.Signed, &snapshot); err != nil { + return err + } + + mutate(&snapshot) + + snapshotBytes, err := json.MarshalCanonical(snapshot) + if err != nil { + return err + } + signedThing.Signed = json.RawMessage(snapshotBytes) + + pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalSnapshotRole) + if err != nil { + return err + } + + metaBytes, err := serializeMetadata(m.CryptoService, signedThing, data.CanonicalSnapshotRole, pubKeys...) + if err != nil { + return err + } + return m.MetadataCache.SetMeta(data.CanonicalSnapshotRole, metaBytes) +} + +// MutateTargets takes a function that mutates the targets metadata - once done, it +// serializes the targets again +func (m *MetadataSwizzler) MutateTargets(mutate func(*data.Targets)) error { + signedThing, err := signedFromStore(m.MetadataCache, data.CanonicalTargetsRole) + if err != nil { + return err + } + + var targets data.Targets + if err := json.Unmarshal(signedThing.Signed, &targets); err != nil { + return err + } + + mutate(&targets) + + targetsBytes, err := json.MarshalCanonical(targets) + if err != nil { + return err + } + signedThing.Signed = json.RawMessage(targetsBytes) + + pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalTargetsRole) + if err != nil { + return err + } + + metaBytes, err := serializeMetadata(m.CryptoService, signedThing, data.CanonicalTargetsRole, pubKeys...) + if err != nil { + return err + } + return m.MetadataCache.SetMeta(data.CanonicalTargetsRole, metaBytes) +} diff --git a/tuf/testutils/swizzler_test.go b/tuf/testutils/swizzler_test.go index 0e3e22e410..95b94e2765 100644 --- a/tuf/testutils/swizzler_test.go +++ b/tuf/testutils/swizzler_test.go @@ -15,6 +15,7 @@ import ( "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/store" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -577,3 +578,95 @@ func TestMissingSigningKey(t *testing.T) { require.IsType(t, ErrNoKeyForRole{}, f.UpdateSnapshotHashes()) require.IsType(t, ErrNoKeyForRole{}, f.UpdateTimestampHash()) } + +// This mutates the root +func TestSwizzlerMutateRoot(t *testing.T) { + f, origMeta := createNewSwizzler(t) + + assert.NoError(t, f.MutateRoot(func(r *data.Root) { r.Roles["hello"] = nil })) + + for role, metaBytes := range origMeta { + newMeta, err := f.MetadataCache.GetMeta(role, -1) + require.NoError(t, err) + + if role != data.CanonicalRootRole { + require.True(t, bytes.Equal(metaBytes, newMeta), "bytes have changed for role %s", role) + } else { + require.False(t, bytes.Equal(metaBytes, newMeta)) + origSigned, newSigned := &data.SignedRoot{}, &data.SignedRoot{} + require.NoError(t, json.Unmarshal(metaBytes, origSigned)) + require.NoError(t, json.Unmarshal(newMeta, newSigned)) + require.Len(t, origSigned.Signed.Roles, 4) + require.Len(t, newSigned.Signed.Roles, 5) + } + } +} + +// This mutates the timestamp +func TestSwizzlerMutateTimestamp(t *testing.T) { + f, origMeta := createNewSwizzler(t) + + assert.NoError(t, f.MutateTimestamp(func(t *data.Timestamp) { t.Meta["hello"] = data.FileMeta{} })) + + for role, metaBytes := range origMeta { + newMeta, err := f.MetadataCache.GetMeta(role, -1) + require.NoError(t, err) + + if role != data.CanonicalTimestampRole { + require.True(t, bytes.Equal(metaBytes, newMeta), "bytes have changed for role %s", role) + } else { + require.False(t, bytes.Equal(metaBytes, newMeta)) + origSigned, newSigned := &data.SignedTimestamp{}, &data.SignedTimestamp{} + require.NoError(t, json.Unmarshal(metaBytes, origSigned)) + require.NoError(t, json.Unmarshal(newMeta, newSigned)) + require.Len(t, origSigned.Signed.Meta, 1) + require.Len(t, newSigned.Signed.Meta, 2) + } + } +} + +// This mutates the snapshot +func TestSwizzlerMutateSnapshot(t *testing.T) { + f, origMeta := createNewSwizzler(t) + + assert.NoError(t, f.MutateSnapshot(func(s *data.Snapshot) { s.Meta["hello"] = data.FileMeta{} })) + + for role, metaBytes := range origMeta { + newMeta, err := f.MetadataCache.GetMeta(role, -1) + require.NoError(t, err) + + if role != data.CanonicalSnapshotRole { + require.True(t, bytes.Equal(metaBytes, newMeta), "bytes have changed for role %s", role) + } else { + require.False(t, bytes.Equal(metaBytes, newMeta)) + origSigned, newSigned := &data.SignedSnapshot{}, &data.SignedSnapshot{} + require.NoError(t, json.Unmarshal(metaBytes, origSigned)) + require.NoError(t, json.Unmarshal(newMeta, newSigned)) + require.Len(t, origSigned.Signed.Meta, 4) + require.Len(t, newSigned.Signed.Meta, 5) + } + } +} + +// This mutates the targets +func TestSwizzlerMutateTargets(t *testing.T) { + f, origMeta := createNewSwizzler(t) + + assert.NoError(t, f.MutateTargets(func(t *data.Targets) { t.Targets["hello"] = data.FileMeta{} })) + + for role, metaBytes := range origMeta { + newMeta, err := f.MetadataCache.GetMeta(role, -1) + require.NoError(t, err) + + if role != data.CanonicalTargetsRole { + require.True(t, bytes.Equal(metaBytes, newMeta), "bytes have changed for role %s", role) + } else { + require.False(t, bytes.Equal(metaBytes, newMeta)) + origSigned, newSigned := &data.SignedTargets{}, &data.SignedTargets{} + require.NoError(t, json.Unmarshal(metaBytes, origSigned)) + require.NoError(t, json.Unmarshal(newMeta, newSigned)) + require.Len(t, origSigned.Signed.Targets, 0) + require.Len(t, newSigned.Signed.Targets, 1) + } + } +} From 95325cd19b23a4197ad492157aabb43b1da457ff Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 15 Feb 2016 16:04:15 -0800 Subject: [PATCH 2/8] Include client update tests to test updating roots that are missing roles Signed-off-by: Ying Li --- client/client_update_test.go | 82 +++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/client/client_update_test.go b/client/client_update_test.go index 82972d3d4c..83d92ff29f 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -15,6 +15,7 @@ import ( "github.com/docker/notary/certs" "github.com/docker/notary/passphrase" + "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/client" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" @@ -818,7 +819,29 @@ func TestUpdateRootRemoteCorruptedNoLocalCache(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode") } - for _, testData := range waysToMessUpServer { + + waysToMessUpServerRoot := waysToMessUpServer + for _, roleName := range data.BaseRoles { + waysToMessUpServerRoot = append(waysToMessUpServer, + swizzleExpectations{ + desc: fmt.Sprintf("no %s keys", roleName), + expectErrs: []interface{}{ + &certs.ErrValidationFail{}, signed.ErrRoleThreshold{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { + r.Roles[roleName].KeyIDs = []string{} + }) + }}, + swizzleExpectations{ + desc: fmt.Sprintf("no %s role", roleName), + expectErrs: []interface{}{tuf.ErrNotLoaded{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) + }}, + ) + } + + for _, testData := range waysToMessUpServerRoot { if testData.desc == "insufficient signatures" { // Currently if we download the root during the bootstrap phase, // we don't check for enough signatures to meet the threshold. We @@ -842,7 +865,32 @@ func TestUpdateRootRemoteCorruptedNoLocalCache(t *testing.T) { // because the fact that the timestamp hasn't changed should mean that we don't // have to re-download the root. func TestUpdateRootRemoteCorruptedCanUseLocalCache(t *testing.T) { - for _, testData := range waysToMessUpServer { + if testing.Short() { + t.Skip("skipping test in short mode") + } + + waysToMessUpServerRoot := waysToMessUpServer + for _, roleName := range data.BaseRoles { + waysToMessUpServerRoot = append(waysToMessUpServer, + swizzleExpectations{ + desc: fmt.Sprintf("no %s keys", roleName), + expectErrs: []interface{}{ + &certs.ErrValidationFail{}, signed.ErrRoleThreshold{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { + r.Roles[roleName].KeyIDs = []string{} + }) + }}, + swizzleExpectations{ + desc: fmt.Sprintf("no %s role", roleName), + expectErrs: []interface{}{tuf.ErrNotLoaded{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) + }}, + ) + } + + for _, testData := range waysToMessUpServerRoot { testUpdateRemoteCorruptValidChecksum(t, updateOpts{ localCache: true, forWrite: false, @@ -862,19 +910,43 @@ func TestUpdateRootRemoteCorruptedCannotUseLocalCache(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode") } - for _, testData := range waysToMessUpServer { + + waysToMessUpServerRoot := waysToMessUpServer + for _, roleName := range data.BaseRoles { + waysToMessUpServerRoot = append(waysToMessUpServer, + swizzleExpectations{ + desc: fmt.Sprintf("no %s keys", roleName), + expectErrs: []interface{}{ + &certs.ErrValidationFail{}, signed.ErrRoleThreshold{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { + r.Roles[roleName].KeyIDs = []string{} + }) + }}, + swizzleExpectations{ + desc: fmt.Sprintf("no %s role", roleName), + expectErrs: []interface{}{tuf.ErrNotLoaded{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) + }}, + ) + } + + // if the timestamp role is missing, then the timestamp is broken, + // so it will revert to the prevoius timestamp + for _, testData := range waysToMessUpServerRoot { testUpdateRemoteCorruptValidChecksum(t, updateOpts{ serverHasNewData: true, localCache: true, forWrite: false, role: data.CanonicalRootRole, - }, testData, true) + }, testData, !strings.HasPrefix(testData.desc, "no timestamp role")) testUpdateRemoteCorruptValidChecksum(t, updateOpts{ serverHasNewData: true, localCache: true, forWrite: true, role: data.CanonicalRootRole, - }, testData, true) + }, testData, !strings.HasPrefix(testData.desc, "no timestamp role")) } } From 401690d621c82111b3ae9f75eefc24acf943d515 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 15 Feb 2016 17:03:42 -0800 Subject: [PATCH 3/8] Include client update tests to test updating non-root metadata that are missing pieces Signed-off-by: Ying Li --- client/client_update_test.go | 157 ++++++++++++++++++++++++++++++++++- 1 file changed, 155 insertions(+), 2 deletions(-) diff --git a/client/client_update_test.go b/client/client_update_test.go index 83d92ff29f..18ece0f197 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -965,6 +965,53 @@ func TestUpdateNonRootRemoteCorruptedNoLocalCache(t *testing.T) { role: role, }, testData, true) } + + switch role { + case data.CanonicalSnapshotRole: + for _, missing := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole} { + testData := swizzleExpectations{ + desc: fmt.Sprintf("snapshot missing root meta checksum"), + // TODO: this should probably be always client.ErrMissingMeta + expectErrs: []interface{}{&json.SyntaxError{}, client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateSnapshot(func(sn *data.Snapshot) { + delete(sn.Meta, missing) + }) + }, + } + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + role: role, + }, testData, true) + } + case data.CanonicalTargetsRole: + testData := swizzleExpectations{ + desc: fmt.Sprintf("target missing delegations data"), + expectErrs: []interface{}{client.ErrChecksumMismatch{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTargets(func(tg *data.Targets) { + tg.Delegations.Roles = tg.Delegations.Roles[1:] + }) + }, + } + // if there are no delegation target roles, we're fine, we just don't + // download them + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + role: role, + }, testData, false) + case data.CanonicalTimestampRole: + testData := swizzleExpectations{ + desc: fmt.Sprintf("timestamp missing snapshot meta checksum"), + expectErrs: []interface{}{client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTimestamp(func(ts *data.Timestamp) { + delete(ts.Meta, data.CanonicalSnapshotRole) + }) + }, + } + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + role: role, + }, testData, true) + } } } @@ -987,6 +1034,53 @@ func TestUpdateNonRootRemoteCorruptedCanUseLocalCache(t *testing.T) { role: role, }, testData, false) } + + switch role { + case data.CanonicalSnapshotRole: + for _, missing := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole} { + testData := swizzleExpectations{ + desc: fmt.Sprintf("snapshot missing root meta checksum"), + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateSnapshot(func(sn *data.Snapshot) { + delete(sn.Meta, missing) + }) + }, + } + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + localCache: true, + role: role, + }, testData, false) + } + case data.CanonicalTargetsRole: + testData := swizzleExpectations{ + desc: fmt.Sprintf("target missing delegations data"), + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTargets(func(tg *data.Targets) { + tg.Delegations.Roles = tg.Delegations.Roles[1:] + }) + }, + } + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + localCache: true, + role: role, + }, testData, false) + case data.CanonicalTimestampRole: + testData := swizzleExpectations{ + desc: fmt.Sprintf("timestamp missing snapshot meta checksum"), + expectErrs: []interface{}{client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTimestamp(func(ts *data.Timestamp) { + delete(ts.Meta, data.CanonicalSnapshotRole) + }) + }, + } + // TODO: this should succeed, because it's an invalid timestamp metadata + // and hence should fall back on the the local cached timestamp + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + localCache: true, + role: role, + }, testData, true) + } } } @@ -1015,6 +1109,59 @@ func TestUpdateNonRootRemoteCorruptedCannotUseLocalCache(t *testing.T) { role: role, }, testData, shouldFail) } + + switch role { + case data.CanonicalSnapshotRole: + for _, missing := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole} { + testData := swizzleExpectations{ + desc: fmt.Sprintf("snapshot missing root meta checksum"), + // TODO: this should probably be always client.ErrMissingMeta + expectErrs: []interface{}{&json.SyntaxError{}, client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateSnapshot(func(sn *data.Snapshot) { + delete(sn.Meta, missing) + }) + }, + } + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + serverHasNewData: true, + localCache: true, + role: role, + }, testData, true) + } + case data.CanonicalTargetsRole: + testData := swizzleExpectations{ + desc: fmt.Sprintf("target missing delegations data"), + expectErrs: []interface{}{client.ErrChecksumMismatch{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTargets(func(tg *data.Targets) { + tg.Delegations.Roles = tg.Delegations.Roles[1:] + }) + }, + } + // if there are no delegation target roles, we're fine, we just don't + // download them + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + serverHasNewData: true, + localCache: true, + role: role, + }, testData, false) + case data.CanonicalTimestampRole: + testData := swizzleExpectations{ + desc: fmt.Sprintf("timestamp missing snapshot meta checksum"), + expectErrs: []interface{}{client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTimestamp(func(ts *data.Timestamp) { + delete(ts.Meta, data.CanonicalSnapshotRole) + }) + }, + } + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + serverHasNewData: true, + localCache: true, + role: role, + }, testData, true) + } } } @@ -1049,10 +1196,16 @@ func testUpdateRemoteCorruptValidChecksum(t *testing.T, opts updateOpts, expt sw isSignatureSwizzle := expt.desc == "invalid signatures" || expt.desc == "meta signed by wrong key" // just try to do these - if they fail (probably because they've been swizzled), that's fine if opts.role != data.CanonicalSnapshotRole || !isSignatureSwizzle { - serverSwizzler.UpdateSnapshotHashes() + // if we are purposely editing out some snapshot metadata, don't re-generate + if !strings.HasPrefix(expt.desc, "snapshot missing") { + serverSwizzler.UpdateSnapshotHashes() + } } if opts.role != data.CanonicalTimestampRole || !isSignatureSwizzle { - serverSwizzler.UpdateTimestampHash() + // if we are purposely editing out some timestamp metadata, don't re-generate + if !strings.HasPrefix(expt.desc, "timestamp missing") { + serverSwizzler.UpdateTimestampHash() + } } } _, err := repo.Update(opts.forWrite) From edb70b5474ff357683ff467109a4b8cf8ef3713a Mon Sep 17 00:00:00 2001 From: Ying Li Date: Tue, 16 Feb 2016 14:34:39 -0800 Subject: [PATCH 4/8] Fix test bug where root role mutations weren't being tested Signed-off-by: Ying Li --- client/client_update_test.go | 115 ++++++++++++++--------------------- tuf/testutils/swizzler.go | 17 +++++- 2 files changed, 59 insertions(+), 73 deletions(-) diff --git a/client/client_update_test.go b/client/client_update_test.go index 18ece0f197..fd37a69197 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "os" "reflect" + "regexp" "strings" "testing" "time" @@ -812,6 +813,36 @@ var waysToMessUpServer = []swizzleExpectations{ }}, } +var _waysToMessUpServerRoot []swizzleExpectations + +// A getter that also sets once through - we can just add these, but it's very easier to do it +// programmatically +func waysToMessUpServerRoot() []swizzleExpectations { + if _waysToMessUpServerRoot == nil { + _waysToMessUpServerRoot = waysToMessUpServer + for _, roleName := range data.BaseRoles { + _waysToMessUpServerRoot = append(_waysToMessUpServerRoot, + swizzleExpectations{ + desc: fmt.Sprintf("no %s keys", roleName), + expectErrs: []interface{}{ + &certs.ErrValidationFail{}, signed.ErrRoleThreshold{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { + r.Roles[roleName].KeyIDs = []string{} + }) + }}, + swizzleExpectations{ + desc: fmt.Sprintf("no %s role", roleName), + expectErrs: []interface{}{tuf.ErrNotLoaded{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) + }}, + ) + } + } + return _waysToMessUpServerRoot +} + // If there's no local cache, we go immediately to check the remote server for // root, and if it invalid (corrupted), we cannot update. This happens // with and without a force check (update for write). @@ -820,28 +851,7 @@ func TestUpdateRootRemoteCorruptedNoLocalCache(t *testing.T) { t.Skip("skipping test in short mode") } - waysToMessUpServerRoot := waysToMessUpServer - for _, roleName := range data.BaseRoles { - waysToMessUpServerRoot = append(waysToMessUpServer, - swizzleExpectations{ - desc: fmt.Sprintf("no %s keys", roleName), - expectErrs: []interface{}{ - &certs.ErrValidationFail{}, signed.ErrRoleThreshold{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateRoot(func(r *data.Root) { - r.Roles[roleName].KeyIDs = []string{} - }) - }}, - swizzleExpectations{ - desc: fmt.Sprintf("no %s role", roleName), - expectErrs: []interface{}{tuf.ErrNotLoaded{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) - }}, - ) - } - - for _, testData := range waysToMessUpServerRoot { + for _, testData := range waysToMessUpServerRoot() { if testData.desc == "insufficient signatures" { // Currently if we download the root during the bootstrap phase, // we don't check for enough signatures to meet the threshold. We @@ -869,28 +879,7 @@ func TestUpdateRootRemoteCorruptedCanUseLocalCache(t *testing.T) { t.Skip("skipping test in short mode") } - waysToMessUpServerRoot := waysToMessUpServer - for _, roleName := range data.BaseRoles { - waysToMessUpServerRoot = append(waysToMessUpServer, - swizzleExpectations{ - desc: fmt.Sprintf("no %s keys", roleName), - expectErrs: []interface{}{ - &certs.ErrValidationFail{}, signed.ErrRoleThreshold{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateRoot(func(r *data.Root) { - r.Roles[roleName].KeyIDs = []string{} - }) - }}, - swizzleExpectations{ - desc: fmt.Sprintf("no %s role", roleName), - expectErrs: []interface{}{tuf.ErrNotLoaded{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) - }}, - ) - } - - for _, testData := range waysToMessUpServerRoot { + for _, testData := range waysToMessUpServerRoot() { testUpdateRemoteCorruptValidChecksum(t, updateOpts{ localCache: true, forWrite: false, @@ -910,43 +899,29 @@ func TestUpdateRootRemoteCorruptedCannotUseLocalCache(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode") } + for _, testData := range waysToMessUpServerRoot() { + // TODO: Bug: currently, if any role is missing from the root, the + // update succeeds because the cached root, with good roles, was + // bootstrapped and hence the roles are loaded into the KeyDB by the + // time the update happens. So when the new root is loaded, although + // it is missing roles, the cached role from the KeyDB is used instead. + // But really, if the root is missing a role, it should be invalid and + // the update should fail instead. + isbuggy, err := regexp.MatchString("no [a-z]+ role", testData.desc) + require.NoError(t, err) - waysToMessUpServerRoot := waysToMessUpServer - for _, roleName := range data.BaseRoles { - waysToMessUpServerRoot = append(waysToMessUpServer, - swizzleExpectations{ - desc: fmt.Sprintf("no %s keys", roleName), - expectErrs: []interface{}{ - &certs.ErrValidationFail{}, signed.ErrRoleThreshold{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateRoot(func(r *data.Root) { - r.Roles[roleName].KeyIDs = []string{} - }) - }}, - swizzleExpectations{ - desc: fmt.Sprintf("no %s role", roleName), - expectErrs: []interface{}{tuf.ErrNotLoaded{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) - }}, - ) - } - - // if the timestamp role is missing, then the timestamp is broken, - // so it will revert to the prevoius timestamp - for _, testData := range waysToMessUpServerRoot { testUpdateRemoteCorruptValidChecksum(t, updateOpts{ serverHasNewData: true, localCache: true, forWrite: false, role: data.CanonicalRootRole, - }, testData, !strings.HasPrefix(testData.desc, "no timestamp role")) + }, testData, !isbuggy) testUpdateRemoteCorruptValidChecksum(t, updateOpts{ serverHasNewData: true, localCache: true, forWrite: true, role: data.CanonicalRootRole, - }, testData, !strings.HasPrefix(testData.desc, "no timestamp role")) + }, testData, !isbuggy) } } diff --git a/tuf/testutils/swizzler.go b/tuf/testutils/swizzler.go index f62ef4d947..7f501b0d8c 100644 --- a/tuf/testutils/swizzler.go +++ b/tuf/testutils/swizzler.go @@ -9,6 +9,7 @@ import ( "github.com/docker/notary/cryptoservice" "github.com/docker/notary/passphrase" "github.com/docker/notary/trustmanager" + "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/store" @@ -41,7 +42,11 @@ func getPubKeys(cs signed.CryptoService, s *data.Signed, role string) ([]data.Pu if err := json.Unmarshal(s.Signed, root); err != nil { return nil, err } - for _, pubKeyID := range root.Roles[data.CanonicalRootRole].KeyIDs { + rootRole, ok := root.Roles[data.CanonicalRootRole] + if !ok || rootRole == nil { + return nil, tuf.ErrNotLoaded{} + } + for _, pubKeyID := range rootRole.KeyIDs { pubKeys = append(pubKeys, root.Keys[pubKeyID]) } } else { @@ -546,6 +551,12 @@ func (m *MetadataSwizzler) MutateRoot(mutate func(*data.Root)) error { return err } + // get the original keys, in case the mutation messes with the signing keys + oldPubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalRootRole) + if err != nil { + return err + } + mutate(&root) rootBytes, err := json.MarshalCanonical(root) @@ -555,8 +566,8 @@ func (m *MetadataSwizzler) MutateRoot(mutate func(*data.Root)) error { signedThing.Signed = json.RawMessage(rootBytes) pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalRootRole) - if err != nil { - return err + if err != nil || len(pubKeys) == 0 { // we have to sign it somehow - might as well use the old keys + pubKeys = oldPubKeys } metaBytes, err := serializeMetadata(m.CryptoService, signedThing, data.CanonicalRootRole, pubKeys...) From 9dfaee1add667f36bf53a1f3426c3c0898355ba8 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Tue, 16 Feb 2016 15:53:32 -0800 Subject: [PATCH 5/8] Refactor and add to per-metadata-type mutation tests Signed-off-by: Ying Li --- client/client_update_test.go | 284 +++++++++++++++++------------------ 1 file changed, 142 insertions(+), 142 deletions(-) diff --git a/client/client_update_test.go b/client/client_update_test.go index fd37a69197..17c7c154cb 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -9,14 +9,12 @@ import ( "net/http/httptest" "os" "reflect" - "regexp" "strings" "testing" "time" "github.com/docker/notary/certs" "github.com/docker/notary/passphrase" - "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/client" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" @@ -833,7 +831,7 @@ func waysToMessUpServerRoot() []swizzleExpectations { }}, swizzleExpectations{ desc: fmt.Sprintf("no %s role", roleName), - expectErrs: []interface{}{tuf.ErrNotLoaded{}}, + expectErrs: []interface{}{data.ErrInvalidRole{}}, swizzle: func(s *testutils.MetadataSwizzler, role string) error { return s.MutateRoot(func(r *data.Root) { delete(r.Roles, roleName) }) }}, @@ -900,31 +898,88 @@ func TestUpdateRootRemoteCorruptedCannotUseLocalCache(t *testing.T) { t.Skip("skipping test in short mode") } for _, testData := range waysToMessUpServerRoot() { - // TODO: Bug: currently, if any role is missing from the root, the - // update succeeds because the cached root, with good roles, was - // bootstrapped and hence the roles are loaded into the KeyDB by the - // time the update happens. So when the new root is loaded, although - // it is missing roles, the cached role from the KeyDB is used instead. - // But really, if the root is missing a role, it should be invalid and - // the update should fail instead. - isbuggy, err := regexp.MatchString("no [a-z]+ role", testData.desc) - require.NoError(t, err) - testUpdateRemoteCorruptValidChecksum(t, updateOpts{ serverHasNewData: true, localCache: true, forWrite: false, role: data.CanonicalRootRole, - }, testData, !isbuggy) + }, testData, true) testUpdateRemoteCorruptValidChecksum(t, updateOpts{ serverHasNewData: true, localCache: true, forWrite: true, role: data.CanonicalRootRole, - }, testData, !isbuggy) + }, testData, true) } } +func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectations { + perRoleSwizzling := make(map[string][]swizzleExpectations) + for _, missing := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole} { + perRoleSwizzling[data.CanonicalSnapshotRole] = append( + perRoleSwizzling[data.CanonicalSnapshotRole], + swizzleExpectations{ + desc: fmt.Sprintf("snapshot missing root meta checksum"), + // TODO: this should probably be always client.ErrMissingMeta + // or some kind of snapshot validation failure - please see + // https://github.com/docker/notary/pull/572#discussion_r53093988 for an explanation + // of the JSON syntax error + expectErrs: []interface{}{&json.SyntaxError{}, client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateSnapshot(func(sn *data.Snapshot) { + delete(sn.Meta, missing) + }) + }, + }) + } + perRoleSwizzling[data.CanonicalTargetsRole] = []swizzleExpectations{{ + desc: fmt.Sprintf("target missing delegations data"), + expectErrs: []interface{}{client.ErrChecksumMismatch{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTargets(func(tg *data.Targets) { + tg.Delegations.Roles = tg.Delegations.Roles[1:] + }) + }, + }} + perRoleSwizzling[data.CanonicalTimestampRole] = []swizzleExpectations{{ + desc: fmt.Sprintf("timestamp missing snapshot meta checksum"), + expectErrs: []interface{}{client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTimestamp(func(ts *data.Timestamp) { + delete(ts.Meta, data.CanonicalSnapshotRole) + }) + }, + }} + perRoleSwizzling["targets/a"] = []swizzleExpectations{{ + desc: fmt.Sprintf("delegation has invalid role"), + expectErrs: []interface{}{client.ErrMissingMeta{}}, + swizzle: func(s *testutils.MetadataSwizzler, role string) error { + return s.MutateTargets(func(tg *data.Targets) { + var keyIDs []string + for k := range tg.Delegations.Keys { + keyIDs = append(keyIDs, k) + } + // add the keys from root too + rootMeta, err := s.MetadataCache.GetMeta(data.CanonicalRootRole, -1) + require.NoError(t, err) + + signedRoot := &data.SignedRoot{} + require.NoError(t, json.Unmarshal(rootMeta, signedRoot)) + + for k := range signedRoot.Signed.Keys { + keyIDs = append(keyIDs, k) + } + + // add an invalid role (root) to delegation + tg.Delegations.Roles = append(tg.Delegations.Roles, + &data.Role{RootRole: data.RootRole{KeyIDs: keyIDs, Threshold: 1}, + Name: data.CanonicalRootRole}) + }) + }, + }} + return perRoleSwizzling +} + // If there's no local cache, we just download from the server and if anything // is corrupt, we cannot update. func TestUpdateNonRootRemoteCorruptedNoLocalCache(t *testing.T) { @@ -940,52 +995,31 @@ func TestUpdateNonRootRemoteCorruptedNoLocalCache(t *testing.T) { role: role, }, testData, true) } - - switch role { - case data.CanonicalSnapshotRole: - for _, missing := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole} { - testData := swizzleExpectations{ - desc: fmt.Sprintf("snapshot missing root meta checksum"), - // TODO: this should probably be always client.ErrMissingMeta - expectErrs: []interface{}{&json.SyntaxError{}, client.ErrMissingMeta{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateSnapshot(func(sn *data.Snapshot) { - delete(sn.Meta, missing) - }) - }, - } + } + for role, expectations := range waysToMessUpServerNonRootPerRole(t) { + for _, testData := range expectations { + switch role { + case data.CanonicalSnapshotRole: testUpdateRemoteCorruptValidChecksum(t, updateOpts{ role: role, }, testData, true) + case data.CanonicalTargetsRole: + // if there are no delegation target roles, we're fine, we just don't + // download them + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + role: role, + }, testData, false) + case data.CanonicalTimestampRole: + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + role: role, + }, testData, true) + case "targets/a": + // TODO: Bug where invalid (non-delegation) roles in delegations + // are accepted + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + role: role, + }, testData, false) } - case data.CanonicalTargetsRole: - testData := swizzleExpectations{ - desc: fmt.Sprintf("target missing delegations data"), - expectErrs: []interface{}{client.ErrChecksumMismatch{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateTargets(func(tg *data.Targets) { - tg.Delegations.Roles = tg.Delegations.Roles[1:] - }) - }, - } - // if there are no delegation target roles, we're fine, we just don't - // download them - testUpdateRemoteCorruptValidChecksum(t, updateOpts{ - role: role, - }, testData, false) - case data.CanonicalTimestampRole: - testData := swizzleExpectations{ - desc: fmt.Sprintf("timestamp missing snapshot meta checksum"), - expectErrs: []interface{}{client.ErrMissingMeta{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateTimestamp(func(ts *data.Timestamp) { - delete(ts.Meta, data.CanonicalSnapshotRole) - }) - }, - } - testUpdateRemoteCorruptValidChecksum(t, updateOpts{ - role: role, - }, testData, true) } } } @@ -1009,52 +1043,36 @@ func TestUpdateNonRootRemoteCorruptedCanUseLocalCache(t *testing.T) { role: role, }, testData, false) } + } + for role, expectations := range waysToMessUpServerNonRootPerRole(t) { + for _, testData := range expectations { - switch role { - case data.CanonicalSnapshotRole: - for _, missing := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole} { - testData := swizzleExpectations{ - desc: fmt.Sprintf("snapshot missing root meta checksum"), - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateSnapshot(func(sn *data.Snapshot) { - delete(sn.Meta, missing) - }) - }, - } + switch role { + case data.CanonicalSnapshotRole: + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + localCache: true, + role: role, + }, testData, false) + case data.CanonicalTargetsRole: + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + localCache: true, + role: role, + }, testData, false) + case data.CanonicalTimestampRole: + // TODO: Bug: this should succeed, because it's an invalid timestamp metadata + // and hence should fall back on the the local cached timestamp. It's not + // succeeding now because the failure occurs on downloading the snapshot, + // not during the validation of the timestamp. + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + localCache: true, + role: role, + }, testData, true) + case "targets/a": testUpdateRemoteCorruptValidChecksum(t, updateOpts{ localCache: true, role: role, }, testData, false) } - case data.CanonicalTargetsRole: - testData := swizzleExpectations{ - desc: fmt.Sprintf("target missing delegations data"), - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateTargets(func(tg *data.Targets) { - tg.Delegations.Roles = tg.Delegations.Roles[1:] - }) - }, - } - testUpdateRemoteCorruptValidChecksum(t, updateOpts{ - localCache: true, - role: role, - }, testData, false) - case data.CanonicalTimestampRole: - testData := swizzleExpectations{ - desc: fmt.Sprintf("timestamp missing snapshot meta checksum"), - expectErrs: []interface{}{client.ErrMissingMeta{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateTimestamp(func(ts *data.Timestamp) { - delete(ts.Meta, data.CanonicalSnapshotRole) - }) - }, - } - // TODO: this should succeed, because it's an invalid timestamp metadata - // and hence should fall back on the the local cached timestamp - testUpdateRemoteCorruptValidChecksum(t, updateOpts{ - localCache: true, - role: role, - }, testData, true) } } } @@ -1084,58 +1102,40 @@ func TestUpdateNonRootRemoteCorruptedCannotUseLocalCache(t *testing.T) { role: role, }, testData, shouldFail) } + } - switch role { - case data.CanonicalSnapshotRole: - for _, missing := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole} { - testData := swizzleExpectations{ - desc: fmt.Sprintf("snapshot missing root meta checksum"), - // TODO: this should probably be always client.ErrMissingMeta - expectErrs: []interface{}{&json.SyntaxError{}, client.ErrMissingMeta{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateSnapshot(func(sn *data.Snapshot) { - delete(sn.Meta, missing) - }) - }, - } + for role, expectations := range waysToMessUpServerNonRootPerRole(t) { + for _, testData := range expectations { + switch role { + case data.CanonicalSnapshotRole: testUpdateRemoteCorruptValidChecksum(t, updateOpts{ serverHasNewData: true, localCache: true, role: role, }, testData, true) + case data.CanonicalTargetsRole: + // if there are no delegation target roles, we're fine, we just don't + // download them + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + serverHasNewData: true, + localCache: true, + role: role, + }, testData, false) + case data.CanonicalTimestampRole: + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + serverHasNewData: true, + localCache: true, + role: role, + }, testData, true) + case "targets/a": + // TODO: Bug where invalid (non-delegation) roles in delegations + // are accepted + testUpdateRemoteCorruptValidChecksum(t, updateOpts{ + serverHasNewData: true, + localCache: true, + role: role, + }, testData, false) } - case data.CanonicalTargetsRole: - testData := swizzleExpectations{ - desc: fmt.Sprintf("target missing delegations data"), - expectErrs: []interface{}{client.ErrChecksumMismatch{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateTargets(func(tg *data.Targets) { - tg.Delegations.Roles = tg.Delegations.Roles[1:] - }) - }, - } - // if there are no delegation target roles, we're fine, we just don't - // download them - testUpdateRemoteCorruptValidChecksum(t, updateOpts{ - serverHasNewData: true, - localCache: true, - role: role, - }, testData, false) - case data.CanonicalTimestampRole: - testData := swizzleExpectations{ - desc: fmt.Sprintf("timestamp missing snapshot meta checksum"), - expectErrs: []interface{}{client.ErrMissingMeta{}}, - swizzle: func(s *testutils.MetadataSwizzler, role string) error { - return s.MutateTimestamp(func(ts *data.Timestamp) { - delete(ts.Meta, data.CanonicalSnapshotRole) - }) - }, - } - testUpdateRemoteCorruptValidChecksum(t, updateOpts{ - serverHasNewData: true, - localCache: true, - role: role, - }, testData, true) } } } From 14edbe33e11533cc8a93727f6c32c4328d7ae3de Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Wed, 17 Feb 2016 10:44:09 -0800 Subject: [PATCH 6/8] add snapshot meta check in download root, update tests Signed-off-by: Riyaz Faizullabhoy --- client/client_update_test.go | 8 ++------ tuf/client/client.go | 6 ++++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/client/client_update_test.go b/client/client_update_test.go index 17c7c154cb..a3e3a223cf 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -919,12 +919,8 @@ func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectat perRoleSwizzling[data.CanonicalSnapshotRole] = append( perRoleSwizzling[data.CanonicalSnapshotRole], swizzleExpectations{ - desc: fmt.Sprintf("snapshot missing root meta checksum"), - // TODO: this should probably be always client.ErrMissingMeta - // or some kind of snapshot validation failure - please see - // https://github.com/docker/notary/pull/572#discussion_r53093988 for an explanation - // of the JSON syntax error - expectErrs: []interface{}{&json.SyntaxError{}, client.ErrMissingMeta{}}, + desc: fmt.Sprintf("snapshot missing root meta checksum"), + expectErrs: []interface{}{client.ErrMissingMeta{}}, swizzle: func(s *testutils.MetadataSwizzler, role string) error { return s.MutateSnapshot(func(sn *data.Snapshot) { delete(sn.Meta, missing) diff --git a/tuf/client/client.go b/tuf/client/client.go index 3732b7361a..549a6babd0 100644 --- a/tuf/client/client.go +++ b/tuf/client/client.go @@ -129,8 +129,10 @@ func (c *Client) downloadRoot() error { var size int64 = -1 var expectedSha256 []byte if c.local.Snapshot != nil { - size = c.local.Snapshot.Signed.Meta[role].Length - expectedSha256 = c.local.Snapshot.Signed.Meta[role].Hashes["sha256"] + if prevRootMeta, ok := c.local.Snapshot.Signed.Meta[role]; ok { + size = prevRootMeta.Length + expectedSha256 = prevRootMeta.Hashes["sha256"] + } } // if we're bootstrapping we may not have a cached root, an From b8866877b0ed9705d40996f71451d9066e6f8094 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Thu, 18 Feb 2016 18:59:14 -0800 Subject: [PATCH 7/8] Clarify test comment Signed-off-by: Ying Li --- client/client_update_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/client_update_test.go b/client/client_update_test.go index a3e3a223cf..f4196a5cc6 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -813,8 +813,8 @@ var waysToMessUpServer = []swizzleExpectations{ var _waysToMessUpServerRoot []swizzleExpectations -// A getter that also sets once through - we can just add these, but it's very easier to do it -// programmatically +// We also want to remove a every role from root once, or remove the role's keys. +// This function generates once and caches the result for later re-use. func waysToMessUpServerRoot() []swizzleExpectations { if _waysToMessUpServerRoot == nil { _waysToMessUpServerRoot = waysToMessUpServer From 1e48fd35ffbcfedd8e4bec1263f9bace963d3b3c Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 22 Feb 2016 15:27:53 -0800 Subject: [PATCH 8/8] Use ToSigned in the swizzler Signed-off-by: Ying Li --- tuf/testutils/swizzler.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tuf/testutils/swizzler.go b/tuf/testutils/swizzler.go index 7f501b0d8c..b790b809a4 100644 --- a/tuf/testutils/swizzler.go +++ b/tuf/testutils/swizzler.go @@ -559,11 +559,10 @@ func (m *MetadataSwizzler) MutateRoot(mutate func(*data.Root)) error { mutate(&root) - rootBytes, err := json.MarshalCanonical(root) + signedThing, err = data.SignedRoot{Signed: root, Signatures: signedThing.Signatures}.ToSigned() if err != nil { return err } - signedThing.Signed = json.RawMessage(rootBytes) pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalRootRole) if err != nil || len(pubKeys) == 0 { // we have to sign it somehow - might as well use the old keys @@ -592,11 +591,10 @@ func (m *MetadataSwizzler) MutateTimestamp(mutate func(*data.Timestamp)) error { mutate(×tamp) - timestampBytes, err := json.MarshalCanonical(timestamp) + signedThing, err = data.SignedTimestamp{Signed: timestamp, Signatures: signedThing.Signatures}.ToSigned() if err != nil { return err } - signedThing.Signed = json.RawMessage(timestampBytes) pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalTimestampRole) if err != nil { @@ -625,11 +623,10 @@ func (m *MetadataSwizzler) MutateSnapshot(mutate func(*data.Snapshot)) error { mutate(&snapshot) - snapshotBytes, err := json.MarshalCanonical(snapshot) + signedThing, err = data.SignedSnapshot{Signed: snapshot, Signatures: signedThing.Signatures}.ToSigned() if err != nil { return err } - signedThing.Signed = json.RawMessage(snapshotBytes) pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalSnapshotRole) if err != nil { @@ -658,11 +655,10 @@ func (m *MetadataSwizzler) MutateTargets(mutate func(*data.Targets)) error { mutate(&targets) - targetsBytes, err := json.MarshalCanonical(targets) + signedThing, err = data.SignedTargets{Signed: targets, Signatures: signedThing.Signatures}.ToSigned() if err != nil { return err } - signedThing.Signed = json.RawMessage(targetsBytes) pubKeys, err := getPubKeys(m.CryptoService, signedThing, data.CanonicalTargetsRole) if err != nil {