Rotate roots per spec (#143)

* update roots

* removing some debugging comments

* removing duplicate code for getLocalRootMeta by calling it from getLocalMeta

* fix based on the reviews.

* enable an arbitrary root verify another root (use case: n verify n+1) without the need for store them permanently.

* check non root metadata, refactor test, address comments

* updated according to the comments

* remove persistent metadata is the keys have changed.

* removing the unused ErrWrongRootVersion

* add DeleteMeta to the LocalStore interface and implemenet in MemoryLocalStore and FileLocalStore  subtypes.

* delete (instead of setting to an empty raw message) the top-level metadata when their key has changed.

* add test fixtures for fast forward attack recovery.

* test for fast forward attack recovery

* addressed several comments.

* addressed more comments. Set the rootVersion in loadAndVerifyLocalRootMeta. Fixed a buggy test.

* Fixed a buggy test.

* fix comment typos

* fix race condition related to the expired check.

* fix race condition related to the expired check.

* kill unmarshalIgnoreExpired.

* add test for root update for client version above 1.

* add test for root update for client version greater than 1.

* update the VerifyIgnoreExpiredCheck method signature and add test for it.

* Avoid mocking IsExpired in the tests. Instead update test fixtured to have concerete timestamps (either expired or long exiring one)

* remove commented code

* update fixtures and clarify test comments.

* updating the comments based on the feedbacks.

* update roots

* removing some debugging comments

* removing duplicate code for getLocalRootMeta by calling it from getLocalMeta

* fix based on the reviews.

* enable an arbitrary root verify another root (use case: n verify n+1) without the need for store them permanently.

* check non root metadata, refactor test, address comments

* updated according to the comments

* remove persistent metadata is the keys have changed.

* removing the unused ErrWrongRootVersion

* delete (instead of setting to an empty raw message) the top-level metadata when their key has changed.

* add test fixtures for fast forward attack recovery.

* test for fast forward attack recovery

* addressed several comments.

* addressed more comments. Set the rootVersion in loadAndVerifyLocalRootMeta. Fixed a buggy test.

* Fixed a buggy test.

* fix comment typos

* Update client/client_test.go

Co-authored-by: Trishank Karthik Kuppusamy <[email protected]>

* Update client/client_test.go

Co-authored-by: Trishank Karthik Kuppusamy <[email protected]>

* fix race condition related to the expired check.

* fix race condition related to the expired check.

* kill unmarshalIgnoreExpired.

* add test for root update for client version above 1.

* add test for root update for client version greater than 1.

* update the VerifyIgnoreExpiredCheck method signature and add test for it.

* Avoid mocking IsExpired in the tests. Instead update test fixtured to have concerete timestamps (either expired or long exiring one)

* remove commented code

* update fixtures and clarify test comments.

* updating the comments based on the feedbacks.

* rebase and update test cases to long expiration (10 years from now), by default.

* add test cases for (1) when there is no local root, (2) there is a local root but no other top-level metadata

* remove the 'previous' of test folders

Co-authored-by: Trishank Karthik Kuppusamy <[email protected]>

Author: hosseinsia

client/client.go

@@ -3,14 +3,18 @@ package client
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net"
+	"net/http"
 	"os"
 	"path/filepath"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	cjson "github.com/tent/canonical-json-go"
 	tuf "github.com/theupdateframework/go-tuf"
 	"github.com/theupdateframework/go-tuf/data"
@@ -298,7 +302,7 @@ func (s *ClientSuite) TestNoChangeUpdate(c *C) {
 	_, err := client.Update()
 	c.Assert(err, IsNil)
 	_, err = client.Update()
-	c.Assert(IsLatestSnapshot(err), Equals, true)
+	c.Assert(err, IsNil)
 }
 
 func (s *ClientSuite) TestNewTimestamp(c *C) {
@@ -308,7 +312,7 @@ func (s *ClientSuite) TestNewTimestamp(c *C) {
 	c.Assert(s.repo.Timestamp(), IsNil)
 	s.syncRemote(c)
 	_, err := client.Update()
-	c.Assert(IsLatestSnapshot(err), Equals, true)
+	c.Assert(err, IsNil)
 	c.Assert(client.timestampVer > version, Equals, true)
 }
 
@@ -360,6 +364,187 @@ func (s *ClientSuite) TestNewRoot(c *C) {
 	}
 }
 
+// startTUFRepoServer starts a HTTP server to serve a TUF Repo.
+func startTUFRepoServer(baseDir string, relPath string) (net.Listener, error) {
+	serverDir := filepath.Join(baseDir, relPath)
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	go http.Serve(l, http.FileServer(http.Dir(serverDir)))
+	return l, err
+}
+
+// newClientWithMeta creates new client and sets the root metadata for it.
+func newClientWithMeta(baseDir string, relPath string, serverAddr string) (*Client, error) {
+	initialStateDir := filepath.Join(baseDir, relPath)
+	opts := &HTTPRemoteOptions{
+		MetadataPath: "metadata",
+		TargetsPath:  "targets",
+	}
+
+	remote, err := HTTPRemoteStore(fmt.Sprintf("http://%s/", serverAddr), opts, nil)
+	if err != nil {
+		return nil, err
+	}
+	c := NewClient(MemoryLocalStore(), remote)
+	for _, m := range []string{"root.json", "snapshot.json", "timestamp.json", "targets.json"} {
+		if _, err := os.Stat(initialStateDir + "/" + m); err == nil {
+			metadataJSON, err := ioutil.ReadFile(initialStateDir + "/" + m)
+			if err != nil {
+				return nil, err
+			}
+			c.local.SetMeta(m, metadataJSON)
+		}
+	}
+	return c, nil
+}
+
+func initRootTest(c *C, baseDir string) (*Client, func() error) {
+	l, err := startTUFRepoServer(baseDir, "server")
+	c.Assert(err, IsNil)
+	tufClient, err := newClientWithMeta(baseDir, "client/metadata/current", l.Addr().String())
+	c.Assert(err, IsNil)
+	return tufClient, l.Close
+}
+
+func (s *ClientSuite) TestUpdateRoots(c *C) {
+	var tests = []struct {
+		fixturePath      string
+		expectedError    error
+		expectedVersions map[string]int
+	}{
+		// Succeeds when there is no root update.
+		{"testdata/Published1Time", nil, map[string]int{"root": 1, "timestamp": 1, "snapshot": 1, "targets": 1}},
+		// Succeeds when client only has root.json
+		{"testdata/Published1Time_client_root_only", nil, map[string]int{"root": 1, "timestamp": 1, "snapshot": 1, "targets": 1}},
+		// Succeeds updating root from version 1 to version 2.
+		{"testdata/Published2Times_keyrotated", nil, map[string]int{"root": 2, "timestamp": 1, "snapshot": 1, "targets": 1}},
+		// Succeeds updating root from version 1 to version 2 when the client's initial root version is expired.
+		{"testdata/Published2Times_keyrotated_initialrootexpired", nil, map[string]int{"root": 2, "timestamp": 1, "snapshot": 1, "targets": 1}},
+		// Succeeds updating root from version 1 to version 3 when versions 1 and 2 are expired.
+		{"testdata/Published3Times_keyrotated_initialrootsexpired", nil, map[string]int{"root": 3, "timestamp": 1, "snapshot": 1, "targets": 1}},
+		// Succeeds updating root from version 2 to version 3.
+		{"testdata/Published3Times_keyrotated_initialrootsexpired_clientversionis2", nil, map[string]int{"root": 3, "timestamp": 1, "snapshot": 1, "targets": 1}},
+		// Fails updating root from version 1 to version 3 when versions 1 and 3 are expired but version 2 is not expired.
+		{"testdata/Published3Times_keyrotated_latestrootexpired", ErrDecodeFailed{File: "root.json", Err: verify.ErrExpired{}}, map[string]int{"root": 2, "timestamp": 1, "snapshot": 1, "targets": 1}},
+		// Fails updating root from version 1 to version 2 when old root 1 did not sign off on it (nth root didn't sign off n+1).
+		{"testdata/Published2Times_keyrotated_invalidOldRootSignature", errors.New("tuf: signature verification failed"), map[string]int{}},
+		// Fails updating root from version 1 to version 2 when the new root 2 did not sign itself (n+1th root didn't sign off n+1)
+		{"testdata/Published2Times_keyrotated_invalidNewRootSignature", errors.New("tuf: signature verification failed"), map[string]int{}},
+		// Fails updating root to 2.root.json when the value of the version field inside it is 1 (rollback attack prevention).
+		{"testdata/Published1Time_backwardRootVersion", verify.ErrWrongVersion(verify.ErrWrongVersion{Given: 1, Expected: 2}), map[string]int{}},
+		// Fails updating root to 2.root.json when the value of the version field inside it is 3 (rollforward attack prevention).
+		{"testdata/Published3Times_keyrotated_forwardRootVersion", verify.ErrWrongVersion(verify.ErrWrongVersion{Given: 3, Expected: 2}), map[string]int{}},
+		// Fails updating when there is no local trusted root.
+		{"testdata/Published1Time_client_no_root", errors.New("tuf: no root keys found in local meta store"), map[string]int{}},
+
+		// snapshot role key rotation increase the snapshot and timestamp.
+		{"testdata/Published2Times_snapshot_keyrotated", nil, map[string]int{"root": 2, "timestamp": 2, "snapshot": 2, "targets": 1}},
+		// targets role key rotation increase the snapshot, timestamp, and targets.
+		{"testdata/Published2Times_targets_keyrotated", nil, map[string]int{"root": 2, "timestamp": 2, "snapshot": 2, "targets": 2}},
+		// timestamp role key rotation increase the timestamp.
+		{"testdata/Published2Times_timestamp_keyrotated", nil, map[string]int{"root": 2, "timestamp": 2, "snapshot": 1, "targets": 1}},
+	}
+
+	for _, test := range tests {
+		tufClient, closer := initRootTest(c, test.fixturePath)
+		_, err := tufClient.Update()
+		if test.expectedError == nil {
+			c.Assert(err, IsNil)
+			// Check if the root.json is being saved in non-volatile storage.
+			tufClient.getLocalMeta()
+			versionMethods := map[string]int{"root": tufClient.rootVer,
+				"timestamp": tufClient.timestampVer,
+				"snapshot":  tufClient.snapshotVer,
+				"targets":   tufClient.targetsVer}
+			for m, v := range test.expectedVersions {
+				assert.Equal(c, v, versionMethods[m])
+			}
+		} else {
+			// For backward compatibility, the update root returns
+			// ErrDecodeFailed that wraps the verify.ErrExpired.
+			if _, ok := test.expectedError.(ErrDecodeFailed); ok {
+				decodeErr, ok := err.(ErrDecodeFailed)
+				c.Assert(ok, Equals, true)
+				c.Assert(decodeErr.File, Equals, "root.json")
+				_, ok = decodeErr.Err.(verify.ErrExpired)
+				c.Assert(ok, Equals, true)
+			} else {
+				assert.Equal(c, test.expectedError, err)
+			}
+		}
+		closer()
+	}
+}
+
+func (s *ClientSuite) TestFastForwardAttackRecovery(c *C) {
+	var tests = []struct {
+		fixturePath       string
+		expectMetaDeleted map[string]bool
+	}{
+		// Each of the following test cases each has a two sets of TUF metadata:
+		// (1) client's initial, and (2) server's current.
+		// The naming format is PublishedTwiceMultiKeysadd_X_revoke_Y_threshold_Z_ROLE
+		// The client includes TUF metadata before key rotation for TUF ROLE with X keys.
+		// The server includes updated TUF metadata after key rotation. The
+		// rotation involves revoking Y keys from the initial keys.
+		// For each test, the TUF client's will be initialized to the client files.
+		// The test checks whether the client is  able to update itself properly.
+
+		// Fast-forward recovery is not needed if less than threshold keys are revoked.
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_2_threshold_4_root",
+			map[string]bool{"root.json": false, "timestamp.json": false, "snapshot.json": false, "targets.json": false}},
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_2_threshold_4_snapshot",
+			map[string]bool{"root.json": false, "timestamp.json": false, "snapshot.json": false, "targets.json": false}},
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_2_threshold_4_targets",
+			map[string]bool{"root.json": false, "timestamp.json": false, "snapshot.json": false, "targets.json": false}},
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_2_threshold_4_timestamp",
+			map[string]bool{"root.json": false, "timestamp.json": false, "snapshot.json": false, "targets.json": false}},
+
+		// Fast-forward recovery not needed if root keys are revoked, even when the threshold number of root keys are revoked.
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_4_threshold_4_root",
+			map[string]bool{"root.json": false, "timestamp.json": false, "snapshot.json": false, "targets.json": false}},
+
+		// Delete snapshot and timestamp metadata if a threshold number of snapshot keys are revoked.
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_4_threshold_4_snapshot",
+			map[string]bool{"root.json": false, "timestamp.json": true, "snapshot.json": true, "targets.json": false}},
+		// Delete targets and snapshot metadata if a threshold number of targets keys are revoked.
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_4_threshold_4_targets",
+			map[string]bool{"root.json": false, "timestamp.json": false, "snapshot.json": true, "targets.json": true}},
+		// Delete timestamp metadata if a threshold number of timestamp keys are revoked.
+		{"testdata/PublishedTwiceMultiKeysadd_9_revoke_4_threshold_4_timestamp",
+			map[string]bool{"root.json": false, "timestamp.json": true, "snapshot.json": false, "targets.json": false}},
+	}
+	for _, test := range tests {
+		tufClient, closer := initRootTest(c, test.fixturePath)
+		c.Assert(tufClient.updateRoots(), IsNil)
+		m, err := tufClient.local.GetMeta()
+		c.Assert(err, IsNil)
+		for md, deleted := range test.expectMetaDeleted {
+			if deleted {
+				if _, ok := m[md]; ok {
+					c.Fatalf("Metadata %s is not deleted!", md)
+				}
+			} else {
+				if _, ok := m[md]; !ok {
+					c.Fatalf("Metadata %s deleted!", md)
+				}
+			}
+		}
+		closer()
+	}
+
+}
+
+func (s *ClientSuite) TestUpdateRace(c *C) {
+	// Tests race condition for the client update. You need to run the test with -race flag:
+	// go test -race
+	for i := 0; i < 2; i++ {
+		go func() {
+			c := NewClient(MemoryLocalStore(), newFakeRemoteStore())
+			c.Update()
+		}()
+	}
+}
+
 func (s *ClientSuite) TestNewTargets(c *C) {
 	client := s.newClient(c)
 	files, err := client.Update()
@@ -552,25 +737,31 @@ func (s *ClientSuite) TestUpdateLocalRootExpired(c *C) {
 
 	// add soon to expire root.json to local storage
 	s.genKeyExpired(c, "timestamp")
+	c.Assert(s.repo.Snapshot(), IsNil)
 	c.Assert(s.repo.Timestamp(), IsNil)
+	c.Assert(s.repo.Commit(), IsNil)
 	s.syncLocal(c)
 
 	// add far expiring root.json to remote storage
 	s.genKey(c, "timestamp")
 	s.addRemoteTarget(c, "bar.txt")
+	c.Assert(s.repo.Snapshot(), IsNil)
+	c.Assert(s.repo.Timestamp(), IsNil)
+	c.Assert(s.repo.Commit(), IsNil)
 	s.syncRemote(c)
 
+	const expectedRootVersion = 3
+
 	// check the update downloads the non expired remote root.json and
 	// restarts itself, thus successfully updating
 	s.withMetaExpired(func() {
 		err := client.getLocalMeta()
 		if _, ok := err.(verify.ErrExpired); !ok {
 			c.Fatalf("expected err to have type signed.ErrExpired, got %T", err)
 		}
-
-		client := NewClient(s.local, s.remote)
 		_, err = client.Update()
 		c.Assert(err, IsNil)
+		c.Assert(client.rootVer, Equals, expectedRootVersion)
 	})
 }
 
@@ -587,6 +778,7 @@ func (s *ClientSuite) TestUpdateRemoteExpired(c *C) {
 
 	c.Assert(s.repo.SnapshotWithExpires(s.expiredTime), IsNil)
 	c.Assert(s.repo.Timestamp(), IsNil)
+	c.Assert(s.repo.Commit(), IsNil)
 	s.syncRemote(c)
 	s.withMetaExpired(func() {
 		_, err := client.Update()
@@ -619,14 +811,18 @@ func (s *ClientSuite) TestUpdateLocalRootExpiredKeyChange(c *C) {
 
 	// add soon to expire root.json to local storage
 	s.genKeyExpired(c, "timestamp")
+	c.Assert(s.repo.Snapshot(), IsNil)
 	c.Assert(s.repo.Timestamp(), IsNil)
+	c.Assert(s.repo.Commit(), IsNil)
 	s.syncLocal(c)
 
 	// replace all keys
 	newKeyIDs := make(map[string][]string)
 	for role, ids := range s.keyIDs {
-		c.Assert(s.repo.RevokeKey(role, ids[0]), IsNil)
-		newKeyIDs[role] = s.genKey(c, role)
+		if role != "snapshot" && role != "timestamp" && role != "targets" {
+			c.Assert(s.repo.RevokeKey(role, ids[0]), IsNil)
+			newKeyIDs[role] = s.genKey(c, role)
+		}
 	}
 
 	// update metadata
@@ -704,7 +900,6 @@ func (s *ClientSuite) TestUpdateReplayAttack(c *C) {
 	c.Assert(s.repo.Timestamp(), IsNil)
 	s.syncRemote(c)
 	_, err := client.Update()
-	c.Assert(IsLatestSnapshot(err), Equals, true)
 	c.Assert(client.timestampVer > version, Equals, true)
 
 	// replace remote timestamp.json with the old one

client/client_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strconv"
 
 	"github.com/theupdateframework/go-tuf/data"
 	"github.com/theupdateframework/go-tuf/util"
@@ -130,14 +131,12 @@ func newTestCase(c *C, name string, consistentSnapshot bool, options *HTTPRemote
 func (t *testCase) run(c *C) {
 	c.Logf("test case: %s consistent-snapshot: %t", t.name, t.consistentSnapshot)
 
-	init := true
 	for _, stepName := range t.testSteps {
-		t.runStep(c, stepName, init)
-		init = false
+		t.runStep(c, stepName)
 	}
 }
 
-func (t *testCase) runStep(c *C, stepName string, init bool) {
+func (t *testCase) runStep(c *C, stepName string) {
 	c.Logf("step: %s", stepName)
 
 	addr, cleanup := startFileServer(c, t.testDir)
@@ -147,17 +146,15 @@ func (t *testCase) runStep(c *C, stepName string, init bool) {
 	c.Assert(err, IsNil)
 
 	client := NewClient(t.local, remote)
-
 	// initiate a client with the root keys
-	if init {
-		keys := getKeys(c, remote)
-		c.Assert(client.Init(keys, 1), IsNil)
-	}
+	keys := getKeys(c, remote)
+	c.Assert(client.Init(keys, 1), IsNil)
 
 	// check update returns the correct updated targets
 	files, err := client.Update()
 	c.Assert(err, IsNil)
-	c.Assert(files, HasLen, 1)
+	l, _ := strconv.Atoi(stepName)
+	c.Assert(files, HasLen, l+1)
 
 	targetName := stepName
 	t.targets[targetName] = []byte(targetName)

client/interop_test.go

@@ -40,6 +40,10 @@ func (f *fileLocalStore) SetMeta(name string, meta json.RawMessage) error {
 	return f.db.Put([]byte(name), []byte(meta), nil)
 }
 
+func (f *fileLocalStore) DeleteMeta(name string) error {
+	return f.db.Delete([]byte(name), nil)
+}
+
 func (f *fileLocalStore) Close() error {
 	if err := f.db.Close(); err != nil {
 		return err

client/leveldbstore/leveldbstore.go

@@ -48,3 +48,32 @@ func (LocalStoreSuite) TestFileLocalStore(c *C) {
 	c.Assert(err, IsNil)
 	assertGet(meta{"root.json": rootJSON, "targets.json": targetsJSON})
 }
+
+func (LocalStoreSuite) TestDeleteMeta(c *C) {
+	tmp := c.MkDir()
+	path := filepath.Join(tmp, "tuf.db")
+	store, err := FileLocalStore(path)
+	c.Assert(err, IsNil)
+
+	type meta map[string]json.RawMessage
+
+	assertGet := func(expected meta) {
+		actual, err := store.GetMeta()
+		c.Assert(err, IsNil)
+		c.Assert(meta(actual), DeepEquals, expected)
+	}
+
+	// initial GetMeta should return empty meta
+	assertGet(meta{})
+
+	// SetMeta should persist
+	rootJSON := []byte(`{"_type":"Root"}`)
+	c.Assert(store.SetMeta("root.json", rootJSON), IsNil)
+	assertGet(meta{"root.json": rootJSON})
+
+	store.DeleteMeta("root.json")
+	m, _ := store.GetMeta()
+	if _, ok := m["root.json"]; ok {
+		c.Fatalf("Metadata is not deleted!")
+	}
+}

client/leveldbstore/leveldbstore_test.go

@@ -18,3 +18,8 @@ func (m memoryLocalStore) SetMeta(name string, meta json.RawMessage) error {
 	m[name] = meta
 	return nil
 }
+
+func (m memoryLocalStore) DeleteMeta(name string) error {
+	delete(m, name)
+	return nil
+}