From 9d2590ffb5fe7c9a6a701c998ab228b588b87617 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Thu, 10 Dec 2015 17:52:20 -0800 Subject: [PATCH] Only allow publishing if there is no snapshot.json, not if it's corrupt or unreadable. This also modifies tuf/store/filestore to return ErrMetaNotFound if the metadata file does not exist. Signed-off-by: Ying Li --- client/client.go | 10 ++++---- client/client_test.go | 47 ++++++++++++++++++++++++++++++------- tuf/store/errors.go | 8 +++++-- tuf/store/filestore.go | 3 +++ tuf/store/filestore_test.go | 19 +++++++++++++++ 5 files changed, 72 insertions(+), 15 deletions(-) diff --git a/client/client.go b/client/client.go index 6504b46bad..af5e5985aa 100644 --- a/client/client.go +++ b/client/client.go @@ -53,13 +53,13 @@ type ErrExpired struct { signed.ErrExpired } -// ErrInvalidRemoteKeyType is returned when the server is requested to manage +// ErrInvalidRemoteRole is returned when the server is requested to manage // an unsupported key type -type ErrInvalidRemoteKeyType struct { +type ErrInvalidRemoteRole struct { Role string } -func (e ErrInvalidRemoteKeyType) Error() string { +func (e ErrInvalidRemoteRole) Error() string { return fmt.Sprintf( "notary does not support the server managing the %s key", e.Role) } @@ -177,7 +177,7 @@ func (r *NotaryRepository) Initialize(rootKeyID string, serverManagedRoles ...st remotelyManagedKeys, data.CanonicalSnapshotRole) serverManagesSnapshot = true default: - return ErrInvalidRemoteKeyType{Role: role} + return ErrInvalidRemoteRole{Role: role} } } @@ -516,6 +516,8 @@ func (r *NotaryRepository) bootstrapRepo() error { return err } tufRepo.SetSnapshot(snapshot) + } else if _, ok := err.(store.ErrMetaNotFound); !ok { + return err } r.tufRepo = tufRepo diff --git a/client/client_test.go b/client/client_test.go index efce960f10..1ccc114971 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -3,6 +3,7 @@ package client import ( "bytes" "crypto/rand" + regJson "encoding/json" "fmt" "io/ioutil" "net/http" @@ -130,7 +131,7 @@ func TestInitRepositoryManagedRolesIncludingRoot(t *testing.T) { t, data.ECDSAKey, tempBaseDir, "docker.com/notary", "http://localhost") err = repo.Initialize(rootPubKeyID, data.CanonicalRootRole) assert.Error(t, err) - assert.IsType(t, ErrInvalidRemoteKeyType{}, err) + assert.IsType(t, ErrInvalidRemoteRole{}, err) // Just testing the error message here in this one case assert.Equal(t, err.Error(), "notary does not support the server managing the root key") @@ -148,7 +149,7 @@ func TestInitRepositoryManagedRolesInvalidRole(t *testing.T) { t, data.ECDSAKey, tempBaseDir, "docker.com/notary", "http://localhost") err = repo.Initialize(rootPubKeyID, "randomrole") assert.Error(t, err) - assert.IsType(t, ErrInvalidRemoteKeyType{}, err) + assert.IsType(t, ErrInvalidRemoteRole{}, err) } // Initializing a new repo while specifying that the server should manage the @@ -163,7 +164,7 @@ func TestInitRepositoryManagedRolesIncludingTargets(t *testing.T) { t, data.ECDSAKey, tempBaseDir, "docker.com/notary", "http://localhost") err = repo.Initialize(rootPubKeyID, data.CanonicalTargetsRole) assert.Error(t, err) - assert.IsType(t, ErrInvalidRemoteKeyType{}, err) + assert.IsType(t, ErrInvalidRemoteRole{}, err) } // Initializing a new repo while specifying that the server should manage the @@ -927,11 +928,21 @@ func testPublishNoOneHasSnapshotKey(t *testing.T, rootType string) { // snapshot key, we can't publish. // We test this with both an RSA and ECDSA root key func TestPublishSnapshotCorrupt(t *testing.T) { - testPublishSnapshotCorrupt(t, data.ECDSAKey, true) - testPublishSnapshotCorrupt(t, data.ECDSAKey, false) + testPublishBadExistingSnapshot(t, data.ECDSAKey, true, true) + testPublishBadExistingSnapshot(t, data.ECDSAKey, false, true) } -func testPublishSnapshotCorrupt(t *testing.T, rootType string, serverManagesSnapshot bool) { +// If the snapshot metadata is unreadable, whether the client or server has the +// snapshot key, we can't publish. +// We test this with both an RSA and ECDSA root key +func TestPublishSnapshotUnreadable(t *testing.T) { + testPublishBadExistingSnapshot(t, data.ECDSAKey, true, false) + testPublishBadExistingSnapshot(t, data.ECDSAKey, false, false) +} + +func testPublishBadExistingSnapshot(t *testing.T, rootType string, + serverManagesSnapshot bool, readable bool) { + // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-") defer os.RemoveAll(tempBaseDir) @@ -941,13 +952,31 @@ func testPublishSnapshotCorrupt(t *testing.T, rootType string, serverManagesSnap ts := fullTestServer(t) defer ts.Close() - repo, _ := initializeRepo(t, rootType, tempBaseDir, gun, ts.URL, serverManagesSnapshot) - // write a corrupt snapshots file - repo.fileStore.SetMeta(data.CanonicalSnapshotRole, []byte("this isn't JSON")) + repo, _ := initializeRepo( + t, rootType, tempBaseDir, gun, ts.URL, serverManagesSnapshot) addTarget(t, repo, "v1", "../fixtures/intermediate-ca.crt") + + var expectedErrType interface{} + if readable { + // write a corrupt snapshots file + repo.fileStore.SetMeta(data.CanonicalSnapshotRole, []byte("this isn't JSON")) + expectedErrType = ®Json.SyntaxError{} + } else { + // create a directory instead of a file + path := fmt.Sprintf("%s.%s", + filepath.Join(tempBaseDir, tufDir, filepath.FromSlash(gun), + "metadata", data.CanonicalSnapshotRole), "json") + os.RemoveAll(path) + err := os.Mkdir(path, 0755) + defer os.RemoveAll(path) + assert.NoError(t, err) + + expectedErrType = &os.PathError{} + } err = repo.Publish() assert.Error(t, err) + assert.IsType(t, expectedErrType, err) } type cannotCreateKeys struct { diff --git a/tuf/store/errors.go b/tuf/store/errors.go index b944c2d7a8..2e0eb0a3ce 100644 --- a/tuf/store/errors.go +++ b/tuf/store/errors.go @@ -1,9 +1,13 @@ package store +import "fmt" + // ErrMetaNotFound indicates we did not find a particular piece // of metadata in the store -type ErrMetaNotFound struct{} +type ErrMetaNotFound struct { + Role string +} func (err ErrMetaNotFound) Error() string { - return "no trust data available" + return fmt.Sprintf("no %s trust data available", err.Role) } diff --git a/tuf/store/filestore.go b/tuf/store/filestore.go index ff3f785fa6..de9d7a52cd 100644 --- a/tuf/store/filestore.go +++ b/tuf/store/filestore.go @@ -45,6 +45,9 @@ func (f *FilesystemStore) GetMeta(name string, size int64) ([]byte, error) { path := filepath.Join(f.metaDir, fileName) meta, err := ioutil.ReadFile(path) if err != nil { + if os.IsNotExist(err) { + err = ErrMetaNotFound{Role: name} + } return nil, err } return meta, nil diff --git a/tuf/store/filestore_test.go b/tuf/store/filestore_test.go index 9f41597b41..f6d35163d8 100644 --- a/tuf/store/filestore_test.go +++ b/tuf/store/filestore_test.go @@ -56,3 +56,22 @@ func TestGetMeta(t *testing.T) { assert.Equal(t, testContent, content, "Content read from file was corrupted.") } + +func TestGetMetaNoSuchMetadata(t *testing.T) { + testDir, err := ioutil.TempDir("/tmp", "testFileSystemStore") + assert.NoError(t, err) + // ensure that the random directory doesn't exist + os.RemoveAll(testDir) + + // don't use the constructor, which creates the directories - just + s := FilesystemStore{ + baseDir: testDir, + metaDir: "metadata", + metaExtension: "json", + targetsDir: "targets", + } + + _, err = s.GetMeta("testMeta", int64(5)) + assert.Error(t, err) + assert.IsType(t, ErrMetaNotFound{}, err) +}