NotaryRepository.Publish supports server managing snapshot keys.

When publishing, do not sign and send the snapshot metadata if the
client does not have the snapshot key.  If the server sends back
an error, then it also does not have a snapshot key and the
client should propogate the no signing key error.

Signed-off-by: Ying Li <ying.li@docker.com>

client/client.go

@@ -345,15 +345,12 @@ func (r *NotaryRepository) Publish() error {
 	if err != nil {
 		if _, ok := err.(store.ErrMetaNotFound); ok {
 			// if the remote store return a 404 (translated into ErrMetaNotFound),
-			// the repo hasn't been initialized yet. Attempt to load it from disk.
+			// there is no trust data for yet. Attempt to load it from disk.
 			err := r.bootstrapRepo()
 			if err != nil {
-				// Repo hasn't been initialized, It must be initialized before
+				// There are lots of reasons there might be an error, such as
-				// it can be published. Return an error and let caller determine
+				// corrupt metadata.  We need better errors from bootstrapRepo.
-				// what it wants to do.
+				return err
-				logrus.Debug(err.Error())
-				logrus.Debug("Repository not initialized during Publish")
-				return &ErrRepoNotInitialized{}
 			}
 			// We had local data but the server doesn't know about the repo yet,
 			// ensure we will push the initial root file
@@ -370,8 +367,8 @@ func (r *NotaryRepository) Publish() error {
 		}
 	} else {
 		// If we were successfully able to bootstrap the client (which only pulls
-		// root.json), update it the rest of the tuf metadata in preparation for
+		// root.json), update it with the rest of the tuf metadata in
-		// applying the changelist.
+		// preparation for applying the changelist.
 		err = c.Update()
 		if err != nil {
 			if err, ok := err.(signed.ErrExpired); ok {
@@ -391,25 +388,50 @@ func (r *NotaryRepository) Publish() error {
 		return err
 	}
 
+	// these are the tuf files we will need to update, serialized as JSON before
+	// we send anything to remote
+	update := make(map[string][]byte)
+
 	// check if our root file is nearing expiry. Resign if it is.
 	if nearExpiry(r.tufRepo.Root) || r.tufRepo.Root.Dirty {
-		if err != nil {
-			return err
-		}
 		root, err = r.tufRepo.SignRoot(data.DefaultExpires("root"))
-		if err != nil {
-			return err
-		}
 		updateRoot = true
 	}
-	// we will always resign targets and snapshots
+
-	targets, err := r.tufRepo.SignTargets("targets", data.DefaultExpires("targets"))
+	if updateRoot {
+		rootJSON, err := json.Marshal(root)
+		if err != nil {
+			return err
+		}
+		update[data.CanonicalRootRole] = rootJSON
+	}
+
+	// we will always resign targets, and snapshots if we have the snapshots key
+	targetsJSON, err := serializeCanonicalRole(r.tufRepo, data.CanonicalTargetsRole)
 	if err != nil {
 		return err
 	}
-	snapshot, err := r.tufRepo.SignSnapshot(data.DefaultExpires("snapshot"))
+	update[data.CanonicalTargetsRole] = targetsJSON
-	if err != nil {
+
-		return err
+	// do not update the snapshot role if we do not have the snapshot key or
+	// any snapshot data.  There might not be any snapshot data the repo was
+	// initialized with the snapshot signing role delegated to the server.
+	// The repo might have snapshot data, because it was requested from
+	// the server by listing, but not have the snapshot key, so signing will
+	// fail.
+	clientCantSignSnapshot := true
+	if r.tufRepo.Snapshot != nil {
+		snapshotJSON, err := serializeCanonicalRole(
+			r.tufRepo, data.CanonicalSnapshotRole)
+		if err == nil { // we have the key - snapshot signed, let's update it
+			update[data.CanonicalSnapshotRole] = snapshotJSON
+			clientCantSignSnapshot = false
+		} else if _, ok := err.(signed.ErrNoKeys); ok {
+			logrus.Debugf("Client does not have the key to sign snapshot. " +
+				"Assuming that server should sign the snapshot.")
+		} else {
+			return err
+		}
 	}
 
 	remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip)
@@ -417,28 +439,16 @@ func (r *NotaryRepository) Publish() error {
 		return err
 	}
 
-	// ensure we can marshal all the json before sending anything to remote
-	targetsJSON, err := json.Marshal(targets)
-	if err != nil {
-		return err
-	}
-	snapshotJSON, err := json.Marshal(snapshot)
-	if err != nil {
-		return err
-	}
-	update := make(map[string][]byte)
-	// if we need to update the root, marshal it and push the update to remote
-	if updateRoot {
-		rootJSON, err := json.Marshal(root)
-		if err != nil {
-			return err
-		}
-		update["root"] = rootJSON
-	}
-	update["targets"] = targetsJSON
-	update["snapshot"] = snapshotJSON
 	err = remote.SetMultiMeta(update)
 	if err != nil {
+		// TODO: this isn't exactly right, since there could be lots of
+		// reasons a request 400'ed.  Need better error translation from HTTP
+		// status codes maybe back to the server errors?
+		if _, ok := err.(store.ErrInvalidOperation); ok && clientCantSignSnapshot {
+			return signed.ErrNoKeys{
+				KeyIDs: r.tufRepo.Root.Signed.Roles[data.CanonicalSnapshotRole].KeyIDs,
+			}
+		}
 		return err
 	}
 	err = cl.Clear("")
@@ -451,6 +461,11 @@ func (r *NotaryRepository) Publish() error {
 	return nil
 }
 
+// bootstrapRepo loads the repository from the local file system.  This attempts
+// to load metadata for all roles.  Since server snapshots are supported,
+// if the snapshot metadata fails to load, that's ok.
+// This can also be unified with some cache reading tools from tuf/client.
+// This assumes that bootstrapRepo is only used by Publish()
 func (r *NotaryRepository) bootstrapRepo() error {
 	kdb := keys.NewDB()
 	tufRepo := tuf.NewRepo(kdb, r.CryptoService)
@@ -479,16 +494,16 @@ func (r *NotaryRepository) bootstrapRepo() error {
 		return err
 	}
 	tufRepo.SetTargets("targets", targets)
+
 	snapshotJSON, err := r.fileStore.GetMeta("snapshot", 0)
-	if err != nil {
+	if err == nil {
-		return err
+		snapshot := &data.SignedSnapshot{}
+		err = json.Unmarshal(snapshotJSON, snapshot)
+		if err != nil {
+			return err
+		}
+		tufRepo.SetSnapshot(snapshot)
 	}
-	snapshot := &data.SignedSnapshot{}
-	err = json.Unmarshal(snapshotJSON, snapshot)
-	if err != nil {
-		return err
-	}
-	tufRepo.SetSnapshot(snapshot)
 
 	r.tufRepo = tufRepo
 
@@ -502,7 +517,7 @@ func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error {
 	if err != nil {
 		return err
 	}
-	err = r.fileStore.SetMeta("root", rootJSON)
+	err = r.fileStore.SetMeta(data.CanonicalRootRole, rootJSON)
 	if err != nil {
 		return err
 	}
@@ -535,7 +550,7 @@ func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error {
 		return err
 	}
 
-	return r.fileStore.SetMeta("snapshot", snapshotJSON)
+	return r.fileStore.SetMeta(data.CanonicalSnapshotRole, snapshotJSON)
 }
 
 func (r *NotaryRepository) bootstrapClient() (*tufclient.Client, error) {

client/client_test.go

@@ -10,7 +10,6 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
-	"strings"
 	"testing"
 
 	"github.com/Sirupsen/logrus"
@@ -23,6 +22,7 @@ import (
 	"github.com/docker/notary/server/storage"
 	"github.com/docker/notary/trustmanager"
 	"github.com/docker/notary/tuf/data"
+	"github.com/docker/notary/tuf/signed"
 	"github.com/docker/notary/tuf/store"
 	"github.com/jfrazelle/go/canonical/json"
 	"github.com/stretchr/testify/assert"
@@ -212,7 +212,7 @@ func TestInitRepoServerManagesTimestampAndSnapshotKeys(t *testing.T) {
 // This creates a new KeyFileStore in the repo's base directory and makes sure
 // the repo has the right number of keys
 func assertRepoHasExpectedKeys(t *testing.T, repo *NotaryRepository,
-	rootKeyID string, serverManagesSnapshot bool) {
+	rootKeyID string, expectedSnapshotKey bool) {
 
 	// The repo should have a keyFileStore and have created keys using it,
 	// so create a new KeyFileStore, and check that the keys do exist and are
@@ -241,11 +241,11 @@ func assertRepoHasExpectedKeys(t *testing.T, repo *NotaryRepository,
 	// there may be a snapshots key, depending on whether the server is managing
 	// the snapshots key
 	_, ok := roles[data.CanonicalSnapshotRole]
-	if serverManagesSnapshot {
+	if expectedSnapshotKey {
+		assert.True(t, ok, "missing snapshot key")
+	} else {
 		assert.False(t, ok,
 			"there should be no snapshot key because the server manages it")
-	} else {
-		assert.True(t, ok, "missing snapshot key")
 	}
 
 	// The server manages the timestamp key - there should not be a timestamp
@@ -271,68 +271,62 @@ func assertRepoHasExpectedCerts(t *testing.T, repo *NotaryRepository) {
 	assert.NotEqual(t, certID, "")
 }
 
-// Sanity check the TUF metadata files. Verify that they exist, the JSON is
+// Sanity check the TUF metadata files. Verify that it exists for a particular
-// well-formed, and the signatures exist. For the root.json file, also check
+// role, the JSON is well-formed, and the signatures exist.
-// that the root, snapshot, and targets key IDs are present.
+// For the root.json file, also check that the root, snapshot, and
+// targets key IDs are present.
 func assertRepoHasExpectedMetadata(t *testing.T, repo *NotaryRepository,
-	serverManagesSnapshot bool) {
+	role string, expected bool) {
 
-	expectedTUFMetadataFiles := []string{
+	filename := filepath.Join(tufDir, filepath.FromSlash(repo.gun),
-		filepath.Join(tufDir, filepath.FromSlash(repo.gun), "metadata", "root.json"),
+		"metadata", role+".json")
-		filepath.Join(tufDir, filepath.FromSlash(repo.gun), "metadata", "snapshot.json"),
+	fullPath := filepath.Join(repo.baseDir, filename)
-		filepath.Join(tufDir, filepath.FromSlash(repo.gun), "metadata", "targets.json"),
+	_, err := os.Stat(fullPath)
+
+	if expected {
+		assert.NoError(t, err, "missing TUF metadata file: %s", filename)
+	} else {
+		assert.Error(t, err,
+			"%s metadata should not exist, but does: %s", role, filename)
+		return
 	}
-	for _, filename := range expectedTUFMetadataFiles {
-		fullPath := filepath.Join(repo.baseDir, filename)
-		_, err := os.Stat(fullPath)
 
-		// special case snapshot metadata - if the server manages the snapshot key
+	jsonBytes, err := ioutil.ReadFile(fullPath)
-		// there should be no snapshot metadata file.
+	assert.NoError(t, err, "error reading TUF metadata file %s: %s", filename, err)
-		if strings.HasSuffix(filename, "snapshot.json") && serverManagesSnapshot {
-			assert.Error(t, err,
-				"snapshot metadata should not exist, but does: %s", filename)
-			continue
-		} else {
-			assert.NoError(t, err, "missing TUF metadata file: %s", filename)
-		}
 
-		jsonBytes, err := ioutil.ReadFile(fullPath)
+	var decoded data.Signed
-		assert.NoError(t, err, "error reading TUF metadata file %s: %s", filename, err)
+	err = json.Unmarshal(jsonBytes, &decoded)
+	assert.NoError(t, err, "error parsing TUF metadata file %s: %s", filename, err)
 
-		var decoded data.Signed
+	assert.Len(t, decoded.Signatures, 1,
-		err = json.Unmarshal(jsonBytes, &decoded)
+		"incorrect number of signatures in TUF metadata file %s", filename)
-		assert.NoError(t, err, "error parsing TUF metadata file %s: %s", filename, err)
 
-		assert.Len(t, decoded.Signatures, 1,
+	assert.NotEmpty(t, decoded.Signatures[0].KeyID,
-			"incorrect number of signatures in TUF metadata file %s", filename)
+		"empty key ID field in TUF metadata file %s", filename)
+	assert.NotEmpty(t, decoded.Signatures[0].Method,
+		"empty method field in TUF metadata file %s", filename)
+	assert.NotEmpty(t, decoded.Signatures[0].Signature,
+		"empty signature in TUF metadata file %s", filename)
 
-		assert.NotEmpty(t, decoded.Signatures[0].KeyID,
+	// Special case for root.json: also check that the signed
-			"empty key ID field in TUF metadata file %s", filename)
+	// content for keys and roles
-		assert.NotEmpty(t, decoded.Signatures[0].Method,
+	if role == data.CanonicalRootRole {
-			"empty method field in TUF metadata file %s", filename)
+		var decodedRoot data.Root
-		assert.NotEmpty(t, decoded.Signatures[0].Signature,
+		err := json.Unmarshal(decoded.Signed, &decodedRoot)
-			"empty signature in TUF metadata file %s", filename)
+		assert.NoError(t, err, "error parsing root.json signed section: %s", err)
 
-		// Special case for root.json: also check that the signed
+		assert.Equal(t, "Root", decodedRoot.Type, "_type mismatch in root.json")
-		// content for keys and roles
-		if strings.HasSuffix(filename, "root.json") {
-			var decodedRoot data.Root
-			err := json.Unmarshal(decoded.Signed, &decodedRoot)
-			assert.NoError(t, err, "error parsing root.json signed section: %s", err)
 
-			assert.Equal(t, "Root", decodedRoot.Type, "_type mismatch in root.json")
+		// Expect 1 key for each valid role in the Keys map - one for
+		// each of root, targets, snapshot, timestamp
+		assert.Len(t, decodedRoot.Keys, len(data.ValidRoles),
+			"wrong number of keys in root.json")
+		assert.Len(t, decodedRoot.Roles, len(data.ValidRoles),
+			"wrong number of roles in root.json")
 
-			// Expect 1 key for each valid role in the Keys map - one for
+		for role := range data.ValidRoles {
-			// each of root, targets, snapshot, timestamp
+			_, ok := decodedRoot.Roles[role]
-			assert.Len(t, decodedRoot.Keys, len(data.ValidRoles),
+			assert.True(t, ok, "Missing role %s in root.json", role)
-				"wrong number of keys in root.json")
-			assert.Len(t, decodedRoot.Roles, len(data.ValidRoles),
-				"wrong number of roles in root.json")
-
-			for role := range data.ValidRoles {
-				_, ok := decodedRoot.Roles[role]
-				assert.True(t, ok, "Missing role %s in root.json", role)
-			}
 		}
 	}
 }
@@ -351,9 +345,12 @@ func testInitRepo(t *testing.T, rootType string, serverManagesSnapshot bool) {
 	repo, rootKeyID := initializeRepo(t, rootType, tempBaseDir, gun, ts.URL,
 		serverManagesSnapshot)
 
-	assertRepoHasExpectedKeys(t, repo, rootKeyID, serverManagesSnapshot)
+	assertRepoHasExpectedKeys(t, repo, rootKeyID, !serverManagesSnapshot)
 	assertRepoHasExpectedCerts(t, repo)
-	assertRepoHasExpectedMetadata(t, repo, serverManagesSnapshot)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole,
+		!serverManagesSnapshot)
 }
 
 // TestInitRepoAttemptsExceeded tests error handling when passphrase.Retriever
@@ -533,10 +530,6 @@ func testListEmptyTargets(t *testing.T, rootType string) {
 
 	repo, _ := initializeRepo(t, rootType, tempBaseDir, gun, ts.URL, false)
 
-	// tests need to manually boostrap timestamp as client doesn't generate it
-	err = repo.tufRepo.InitTimestamp()
-	assert.NoError(t, err, "error creating repository: %s", err)
-
 	_, err = repo.ListTargets()
 	assert.Error(t, err) // no trust data
 }
@@ -766,19 +759,30 @@ func testGetChangelist(t *testing.T, rootType string) {
 	assert.Equal(t, "latest", latestChange.Path())
 }
 
-// TestPublish creates a repo, instantiates a notary server, and publishes
+// Create a repo, instantiate a notary server, and publish the repo to the
-// the repo to the server.
+// server, signing all the non-timestamp metadata.
 // We test this with both an RSA and ECDSA root key
-func TestPublish(t *testing.T) {
+func TestPublishClientHasSnapshotKey(t *testing.T) {
-	testPublish(t, data.ECDSAKey)
+	testPublish(t, data.ECDSAKey, false)
 	if !testing.Short() {
-		testPublish(t, data.RSAKey)
+		testPublish(t, data.RSAKey, false)
 	}
 }
 
-func testPublish(t *testing.T, rootType string) {
+// Create a repo, instantiate a notary server (designating the server as the
+// snapshot signer) , and publish the repo to the server, signing the root and
+// targets metadata only.  The server should sign just fine.
+// We test this with both an RSA and ECDSA root key
+func TestPublishAfterInitServerHasSnapshotKey(t *testing.T) {
+	testPublish(t, data.ECDSAKey, true)
+	if !testing.Short() {
+		testPublish(t, data.RSAKey, true)
+	}
+}
+
+func testPublish(t *testing.T, rootType string, serverManagesSnapshot bool) {
 	// Temporary directory where test files will be created
-	tempBaseDir, err := ioutil.TempDir("", "notary-test-")
+	tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-")
 	defer os.RemoveAll(tempBaseDir)
 
 	assert.NoError(t, err, "failed to create a temporary directory: %s", err)
@@ -786,46 +790,172 @@ func testPublish(t *testing.T, rootType string) {
 	gun := "docker.com/notary"
 	ts := fullTestServer(t)
 	defer ts.Close()
-	repo, _ := initializeRepo(t, rootType, tempBaseDir, gun, ts.URL, false)
 
+	repo, _ := initializeRepo(t, rootType, tempBaseDir, gun, ts.URL,
+		serverManagesSnapshot)
+	assertPublishSucceeds(t, repo)
+}
+
+func assertPublishSucceeds(t *testing.T, repo1 *NotaryRepository) {
 	// Create 2 targets
-	latestTarget := addTarget(t, repo, "latest", "../fixtures/intermediate-ca.crt")
+	latestTarget := addTarget(t, repo1, "latest", "../fixtures/intermediate-ca.crt")
-	currentTarget := addTarget(t, repo, "current", "../fixtures/intermediate-ca.crt")
+	currentTarget := addTarget(t, repo1, "current", "../fixtures/intermediate-ca.crt")
-	assert.Len(t, getChanges(t, repo), 2, "wrong number of changelist files found")
+	assert.Len(t, getChanges(t, repo1), 2, "wrong number of changelist files found")
 
 	// Now test Publish
-	err = repo.Publish()
+	err := repo1.Publish()
 	assert.NoError(t, err)
-	assert.Len(t, getChanges(t, repo), 0, "wrong number of changelist files found")
+	assert.Len(t, getChanges(t, repo1), 0, "wrong number of changelist files found")
 
 	// Create a new repo and pull from the server
-	tempBaseDir2, err := ioutil.TempDir("", "notary-test-")
+	tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-")
-	defer os.RemoveAll(tempBaseDir2)
+	defer os.RemoveAll(tempBaseDir)
 
 	assert.NoError(t, err, "failed to create a temporary directory: %s", err)
 
-	repo2, err := NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, passphraseRetriever)
+	repo2, err := NewNotaryRepository(tempBaseDir, repo1.gun, repo1.baseURL,
+		http.DefaultTransport, passphraseRetriever)
 	assert.NoError(t, err, "error creating repository: %s", err)
 
-	targets, err := repo2.ListTargets()
-	assert.NoError(t, err)
-
 	// Should be two targets
-	assert.Len(t, targets, 2, "unexpected number of targets returned by ListTargets")
+	for _, repo := range []*NotaryRepository{repo1, repo2} {
+		targets, err := repo.ListTargets()
+		assert.NoError(t, err)
 
-	sort.Stable(targetSorter(targets))
+		assert.Len(t, targets, 2, "unexpected number of targets returned by ListTargets")
 
-	assert.Equal(t, currentTarget, targets[0], "current target does not match")
+		sort.Stable(targetSorter(targets))
-	assert.Equal(t, latestTarget, targets[1], "latest target does not match")
 
-	// Also test GetTargetByName
+		assert.Equal(t, currentTarget, targets[0], "current target does not match")
-	newLatestTarget, err := repo2.GetTargetByName("latest")
+		assert.Equal(t, latestTarget, targets[1], "latest target does not match")
+
+		// Also test GetTargetByName
+		newLatestTarget, err := repo.GetTargetByName("latest")
+		assert.NoError(t, err)
+		assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match")
+
+		newCurrentTarget, err := repo.GetTargetByName("current")
+		assert.NoError(t, err)
+		assert.Equal(t, currentTarget, newCurrentTarget, "current target does not match")
+	}
+}
+
+// After pulling a repo from the server, so there is a snapshots metadata file,
+// push a different target to the server (the server is still the snapshot
+// signer).  The server should sign just fine.
+// We test this with both an RSA and ECDSA root key
+func TestPublishAfterPullServerHasSnapshotKey(t *testing.T) {
+	testPublishAfterPullServerHasSnapshotKey(t, data.ECDSAKey)
+	if !testing.Short() {
+		testPublishAfterPullServerHasSnapshotKey(t, data.RSAKey)
+	}
+}
+
+func testPublishAfterPullServerHasSnapshotKey(t *testing.T, rootType string) {
+	// Temporary directory where test files will be created
+	tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-")
+	defer os.RemoveAll(tempBaseDir)
+
+	assert.NoError(t, err, "failed to create a temporary directory: %s", err)
+
+	gun := "docker.com/notary"
+	ts := fullTestServer(t)
+	defer ts.Close()
+
+	repo, _ := initializeRepo(t, rootType, tempBaseDir, gun, ts.URL, true)
+	// no timestamp metadata because that comes from the server
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, false)
+	// no snapshot metadata because that comes from the server
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, false)
+
+	// Publish something
+	published := addTarget(t, repo, "v1", "../fixtures/intermediate-ca.crt")
+	err = repo.Publish()
 	assert.NoError(t, err)
-	assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match")
+	// still no timestamp or snapshot metadata info
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, false)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, false)
 
-	newCurrentTarget, err := repo2.GetTargetByName("current")
+	// list, so that the snapshot metadata is pulled from server
+	targets, err := repo.ListTargets()
 	assert.NoError(t, err)
-	assert.Equal(t, currentTarget, newCurrentTarget, "current target does not match")
+	assert.Equal(t, []*Target{published}, targets)
+	// listing downloaded the timestamp and snapshot metadata info
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, true)
+	assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true)
+
+	// Publish again should succeed
+	addTarget(t, repo, "v2", "../fixtures/intermediate-ca.crt")
+	err = repo.Publish()
+	assert.NoError(t, err)
+}
+
+// If neither the client nor the server has the snapshot key, signing will fail
+// with an ErrNoKeys error.
+// We test this with both an RSA and ECDSA root key
+func TestPublishNoOneHasSnapshotKey(t *testing.T) {
+	testPublishNoOneHasSnapshotKey(t, data.ECDSAKey)
+	if !testing.Short() {
+		testPublishNoOneHasSnapshotKey(t, data.RSAKey)
+	}
+}
+
+func testPublishNoOneHasSnapshotKey(t *testing.T, rootType string) {
+	// Temporary directory where test files will be created
+	tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-")
+	defer os.RemoveAll(tempBaseDir)
+
+	assert.NoError(t, err, "failed to create a temporary directory: %s", err)
+
+	gun := "docker.com/notary"
+	ts := fullTestServer(t)
+	defer ts.Close()
+
+	// create repo and delete the snapshot key and metadata
+	repo, _ := initializeRepo(t, rootType, tempBaseDir, gun, ts.URL, false)
+	snapshotRole, ok := repo.tufRepo.Root.Signed.Roles[data.CanonicalSnapshotRole]
+	assert.True(t, ok)
+	for _, keyID := range snapshotRole.KeyIDs {
+		repo.CryptoService.RemoveKey(keyID)
+	}
+
+	// Publish something
+	addTarget(t, repo, "v1", "../fixtures/intermediate-ca.crt")
+	err = repo.Publish()
+	assert.Error(t, err)
+	assert.IsType(t, signed.ErrNoKeys{}, err)
+}
+
+// If the snapshot metadata is corrupt, 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 TestPublishSnapshotCorrupt(t *testing.T) {
+	testPublishSnapshotCorrupt(t, data.ECDSAKey, true)
+	testPublishSnapshotCorrupt(t, data.ECDSAKey, false)
+	if !testing.Short() {
+		testPublishSnapshotCorrupt(t, data.RSAKey, true)
+		testPublishSnapshotCorrupt(t, data.RSAKey, false)
+	}
+}
+
+func testPublishSnapshotCorrupt(t *testing.T, rootType string, serverManagesSnapshot bool) {
+	// Temporary directory where test files will be created
+	tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-")
+	defer os.RemoveAll(tempBaseDir)
+
+	assert.NoError(t, err, "failed to create a temporary directory: %s", err)
+
+	gun := "docker.com/notary"
+	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"))
+
+	addTarget(t, repo, "v1", "../fixtures/intermediate-ca.crt")
+	err = repo.Publish()
+	assert.Error(t, err)
 }
 
 func TestRotate(t *testing.T) {

tuf/signed/errors.go

@@ -63,10 +63,10 @@ func (e ErrInvalidKeyLength) Error() string {
 
 // ErrNoKeys indicates no signing keys were found when trying to sign
 type ErrNoKeys struct {
-	keyIDs []string
+	KeyIDs []string
 }
 
 func (e ErrNoKeys) Error() string {
 	return fmt.Sprintf("could not find necessary signing keys, at least one of these keys must be available: %s",
-		strings.Join(e.keyIDs, ", "))
+		strings.Join(e.KeyIDs, ", "))
 }

tuf/signed/sign.go

@@ -46,7 +46,7 @@ func Sign(service CryptoService, s *data.Signed, keys ...data.PublicKey) error {
 
 	// Check to ensure we have at least one signing key
 	if len(privKeys) == 0 {
-		return ErrNoKeys{keyIDs: ids}
+		return ErrNoKeys{KeyIDs: ids}
 	}
 
 	// Do signing and generate list of signatures