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 <ying.li@docker.com>

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

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 = &regJson.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 {

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)
 }

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

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)
+}