diff --git a/client/client_update_test.go b/client/client_update_test.go index 05a79cf03d..1bfb86873e 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -28,109 +28,47 @@ func newBlankRepo(t *testing.T, url string) *NotaryRepository { return repo } -// If there's no local cache, we go immediately to check the remote server for -// root, and if it doesn't exist, we return ErrRepositoryNotExist. This happens -// with or without a force check (update for write). -func TestUpdateNotExistNoLocalCache(t *testing.T) { - testUpdateNotExistNoLocalCache(t, false) - testUpdateNotExistNoLocalCache(t, true) -} +var metadataDelegations = []string{"targets/a", "targets/a/b", "targets/b", "targets/a/b/c", "targets/b/c"} +var delegationsWithNonEmptyMetadata = []string{"targets/a", "targets/a/b", "targets/b"} -func testUpdateNotExistNoLocalCache(t *testing.T, forWrite bool) { - ts, _, _ := simpleTestServer(t) - defer ts.Close() - - repo := newBlankRepo(t, ts.URL) - defer os.RemoveAll(repo.baseDir) - - // there is no metadata at all - this is a fresh repo, and the server isn't - // aware of the root. - _, err := repo.Update(forWrite) - require.Error(t, err) - require.IsType(t, ErrRepositoryNotExist{}, err) -} - -// If there is a local cache, we use the local root as the trust anchor and we -// then an update. If the server has no root.json, we return an ErrRepositoryNotExist. -// If we force check (update for write), then it hits the server first, and -// still returns an ErrRepositoryNotExist. -func TestUpdateNotExistWithLocalCache(t *testing.T) { - testUpdateNotExistWithLocalCache(t, false) - testUpdateNotExistWithLocalCache(t, true) -} - -func testUpdateNotExistWithLocalCache(t *testing.T, forWrite bool) { - ts, _, _ := simpleTestServer(t) - defer ts.Close() - - repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, false) - defer os.RemoveAll(repo.baseDir) - - // the repo has metadata, but the server is unaware of any metadata - // whatsoever. - _, err := repo.Update(forWrite) - require.Error(t, err) - require.IsType(t, ErrRepositoryNotExist{}, err) -} - -// If there is a local cache, we use the local root as the trust anchor and we -// then an update. If the server has a root.json, but is missing other data, -// then we propagate the ErrMetaNotFound. Same if we force check -// (update for write); the root exists, but other metadata doesn't. -func TestUpdateWithLocalCacheRemoteMissingMetadata(t *testing.T) { - testUpdateWithLocalCacheRemoteMissingMetadata(t, false) - testUpdateWithLocalCacheRemoteMissingMetadata(t, true) -} - -func testUpdateWithLocalCacheRemoteMissingMetadata(t *testing.T, forWrite bool) { - ts, m, _ := simpleTestServer(t) - defer ts.Close() - - repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, false) - defer os.RemoveAll(repo.baseDir) - - rootJSON, err := repo.fileStore.GetMeta(data.CanonicalRootRole, maxSize) +func newServerSwizzler(t *testing.T) (map[string][]byte, *testutils.MetadataSwizzler) { + serverMeta, cs, err := testutils.NewRepoMetadata("docker.com/notary", metadataDelegations...) require.NoError(t, err) - // the server should know about the root.json, and nothing else - m.HandleFunc( - fmt.Sprintf("/v2/docker.com/notary/_trust/tuf/root.json"), - func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, string(rootJSON)) - }) + serverSwizzler := testutils.NewMetadataSwizzler("docker.com/notary", serverMeta, cs) + require.NoError(t, err) - // the first thing the client tries to get is the timestamp - so that - // will be the failed metadata update. - _, err = repo.Update(forWrite) - require.Error(t, err) - require.IsType(t, store.ErrMetaNotFound{}, err) - metaNotFound, ok := err.(store.ErrMetaNotFound) - require.True(t, ok) - require.Equal(t, data.CanonicalTimestampRole, metaNotFound.Resource) + return serverMeta, serverSwizzler +} + +// bumps the versions of everything in the metadata cache, to force local cache +// to update +func bumpVersions(t *testing.T, s *testutils.MetadataSwizzler, offset int) { + // bump versions of everything on the server, to force everything to update + for _, r := range s.Roles { + require.NoError(t, s.OffsetMetadataVersion(r, offset)) + } + require.NoError(t, s.UpdateSnapshotHashes()) + require.NoError(t, s.UpdateTimestampHash()) } // create a server that just serves static metadata files from a metaStore -func readOnlyServer(t *testing.T, cache store.MetadataStore) *httptest.Server { +func readOnlyServer(t *testing.T, cache store.MetadataStore, notFoundStatus int) *httptest.Server { m := mux.NewRouter() m.HandleFunc("/v2/docker.com/notary/_trust/tuf/{role:.*}.json", func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) metaBytes, err := cache.GetMeta(vars["role"], maxSize) - require.NoError(t, err) - w.Write(metaBytes) + if _, ok := err.(store.ErrMetaNotFound); ok { + w.WriteHeader(notFoundStatus) + } else { + require.NoError(t, err) + w.Write(metaBytes) + } }) - return httptest.NewServer(m) } -func bumpVersions(t *testing.T, s *testutils.MetadataSwizzler) { - for _, r := range s.Roles { - require.NoError(t, s.OffsetMetadataVersion(r, 1)) - } - require.NoError(t, s.UpdateSnapshotHashes()) - require.NoError(t, s.UpdateTimestampHash()) -} - type unwritableStore struct { store.MetadataStore roleToNotWrite string @@ -150,10 +88,10 @@ func TestUpdateSucceedsEvenIfCannotWriteNewRepo(t *testing.T) { t.Skip("skipping test in short mode") } - serverMeta, _, err := testutils.NewRepoMetadata("docker.com/notary", "targets/a", "targets/a/b", "targets/a/b/c") + serverMeta, _, err := testutils.NewRepoMetadata("docker.com/notary", metadataDelegations...) require.NoError(t, err) - ts := readOnlyServer(t, store.NewMemoryStore(serverMeta, nil)) + ts := readOnlyServer(t, store.NewMemoryStore(serverMeta, nil), http.StatusNotFound) defer ts.Close() for role := range serverMeta { @@ -191,20 +129,15 @@ func TestUpdateSucceedsEvenIfCannotWriteExistingRepo(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode") } - serverMeta, cs, err := testutils.NewRepoMetadata("docker.com/notary", "targets/a", "targets/a/b", "targets/a/b/c") - require.NoError(t, err) - - serverSwizzler := testutils.NewMetadataSwizzler("docker.com/notary", serverMeta, cs) - require.NoError(t, err) - - ts := readOnlyServer(t, serverSwizzler.MetadataCache) + serverMeta, serverSwizzler := newServerSwizzler(t) + ts := readOnlyServer(t, serverSwizzler.MetadataCache, http.StatusNotFound) defer ts.Close() // download existing metadata repo := newBlankRepo(t, ts.URL) defer os.RemoveAll(repo.baseDir) - _, err = repo.Update(false) + _, err := repo.Update(false) require.NoError(t, err) origFileStore := repo.fileStore @@ -212,7 +145,7 @@ func TestUpdateSucceedsEvenIfCannotWriteExistingRepo(t *testing.T) { for role := range serverMeta { for _, forWrite := range []bool{true, false} { // bump versions of everything on the server, to force everything to update - bumpVersions(t, serverSwizzler) + bumpVersions(t, serverSwizzler, 1) // update fileStore repo.fileStore = &unwritableStore{MetadataStore: origFileStore, roleToNotWrite: role} @@ -267,10 +200,10 @@ func TestUpdateReplacesCorruptOrMissingMetadata(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode") } - serverMeta, cs, err := testutils.NewRepoMetadata("docker.com/notary", "targets/a", "targets/a/b", "targets/a/b/c") + serverMeta, cs, err := testutils.NewRepoMetadata("docker.com/notary", metadataDelegations...) require.NoError(t, err) - ts := readOnlyServer(t, store.NewMemoryStore(serverMeta, nil)) + ts := readOnlyServer(t, store.NewMemoryStore(serverMeta, nil), http.StatusNotFound) defer ts.Close() repo := newBlankRepo(t, ts.URL) @@ -310,28 +243,22 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) { t.Skip("skipping test in short mode") } - serverMeta, cs, err := testutils.NewRepoMetadata("docker.com/notary", "targets/a", "targets/a/b", "targets/a/b/c") - require.NoError(t, err) - - serverSwizzler := testutils.NewMetadataSwizzler("docker.com/notary", serverMeta, cs) - require.NoError(t, err) - + serverMeta, serverSwizzler := newServerSwizzler(t) origMeta := testutils.CopyRepoMetadata(serverMeta) - ts := readOnlyServer(t, serverSwizzler.MetadataCache) + ts := readOnlyServer(t, serverSwizzler.MetadataCache, http.StatusNotFound) defer ts.Close() repo := newBlankRepo(t, ts.URL) defer os.RemoveAll(repo.baseDir) - _, err = repo.Update(false) // ensure we have all metadata to start with + _, err := repo.Update(false) // ensure we have all metadata to start with require.NoError(t, err) - ts.Close() // rotate the server's root.json root key so that they no longer match trust anchors require.NoError(t, serverSwizzler.ChangeRootKey()) // bump versions, update snapshot and timestamp too so it will not fail on a hash - bumpVersions(t, serverSwizzler) + bumpVersions(t, serverSwizzler, 1) // we want to swizzle the local cache, not the server, so create a new one repoSwizzler := &testutils.MetadataSwizzler{ @@ -381,3 +308,301 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) { } } } + +type updateOpts struct { + notFoundCode int // what code to return when the cache doesn't have the metadata + serverHasNewData bool // whether the server should have the same or new version than the local cache + localCache bool // whether the repo should have a local cache before updating + forWrite bool // whether the update is for writing or not (force check remote root.json) + role string // the role to mess up on the server +} + +// If there's no local cache, we go immediately to check the remote server for +// root, and if it doesn't exist, we return ErrRepositoryNotExist. This happens +// with or without a force check (update for write). +func TestUpdateRemoteRootNotExistNoLocalCache(t *testing.T) { + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + forWrite: false, + role: data.CanonicalRootRole, + }, ErrRepositoryNotExist{}) + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + forWrite: true, + role: data.CanonicalRootRole, + }, ErrRepositoryNotExist{}) +} + +// If there is a local cache, we use the local root as the trust anchor and we +// then an update. If the server has no root.json, and we don't need to force +// check (update for write), we can used the cached root because the timestamp +// has not changed. +// If we force check (update for write), then it hits the server first, and +// still returns an ErrRepositoryNotExist. This is the +// case where the server has the same data as the client, in which case we might +// be able to just used the cached data and not have to download. +func TestUpdateRemoteRootNotExistCanUseLocalCache(t *testing.T) { + // if for-write is false, then we don't need to check the root.json on bootstrap, + // and hence we can just use the cached version on update + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + localCache: true, + forWrite: false, + role: data.CanonicalRootRole, + }, nil) + // fails because bootstrap requires a check to remote root.json and fails if + // the check fails + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + localCache: true, + forWrite: true, + role: data.CanonicalRootRole, + }, ErrRepositoryNotExist{}) +} + +// If there is a local cache, we use the local root as the trust anchor and we +// then an update. If the server has no root.json, we return an ErrRepositoryNotExist. +// If we force check (update for write), then it hits the server first, and +// still returns an ErrRepositoryNotExist. This is the case where the server +// has new updated data, so we cannot default to cached data. +func TestUpdateRemoteRootNotExistCannotUseLocalCache(t *testing.T) { + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + serverHasNewData: true, + localCache: true, + forWrite: false, + role: data.CanonicalRootRole, + }, ErrRepositoryNotExist{}) + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + serverHasNewData: true, + localCache: true, + forWrite: true, + role: data.CanonicalRootRole, + }, ErrRepositoryNotExist{}) +} + +// If there's no local cache, we go immediately to check the remote server for +// root, and if it 50X's, we return ErrServerUnavailable. This happens +// with or without a force check (update for write). +func TestUpdateRemoteRoot50XNoLocalCache(t *testing.T) { + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + forWrite: false, + role: data.CanonicalRootRole, + }, store.ErrServerUnavailable{}) + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + forWrite: true, + role: data.CanonicalRootRole, + }, store.ErrServerUnavailable{}) +} + +// If there is a local cache, we use the local root as the trust anchor and we +// then an update. If the server 50X's on root.json, and we don't force check, +// then because the timestamp is the same we can just use our cached root.json +// and don't have to download another. +// If we force check (update for write), we return an ErrServerUnavailable. +// This is the case where the server has the same data as the client +func TestUpdateRemoteRoot50XCanUseLocalCache(t *testing.T) { + // if for-write is false, then we don't need to check the root.json on bootstrap, + // and hence we can just use the cached version on update. + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + localCache: true, + forWrite: false, + role: data.CanonicalRootRole, + }, nil) + // fails because bootstrap requires a check to remote root.json and fails if + // the check fails + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + localCache: true, + forWrite: true, + role: data.CanonicalRootRole, + }, store.ErrServerUnavailable{}) +} + +// If there is a local cache, we use the local root as the trust anchor and we +// then an update. If the server 50X's on root.json, we return an ErrServerUnavailable. +// This happens with or without a force check (update for write) +func TestUpdateRemoteRoot50XCannotUseLocalCache(t *testing.T) { + // if for-write is false, then we don't need to check the root.json on bootstrap, + // and hence we can just use the cached version on update + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + serverHasNewData: true, + localCache: true, + forWrite: false, + role: data.CanonicalRootRole, + }, store.ErrServerUnavailable{}) + // fails because of bootstrap + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + serverHasNewData: true, + localCache: true, + forWrite: true, + role: data.CanonicalRootRole, + }, store.ErrServerUnavailable{}) +} + +// If there is no local cache, we just update. If the server has a root.json, +// but is missing other data, then we propagate the ErrMetaNotFound. Skipping +// force check, because that only matters for root. +func TestUpdateNonRootRemoteMissingMetadataNoLocalCache(t *testing.T) { + for _, role := range append(data.BaseRoles, delegationsWithNonEmptyMetadata...) { + if role == data.CanonicalRootRole { + continue + } + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + role: role, + }, store.ErrMetaNotFound{}) + } +} + +// If there is a local cache, we update anyway and see if anything's different +// (assuming remote has a root.json). If the timestamp is missing, we use the +// local timestamp and already have all data, so nothing needs to be downloaded. +// If the timestamp is present, but the same, we already have all the data, so +// nothing needs to be downloaded. +// Skipping force check, because that only matters for root. +func TestUpdateNonRootRemoteMissingMetadataCanUseLocalCache(t *testing.T) { + // really we can delete everything at once except for the timestamp, but + // it's better to check one by one in case we change the download code + // somewhat. + for _, role := range append(data.BaseRoles, delegationsWithNonEmptyMetadata...) { + if role == data.CanonicalRootRole { + continue + } + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + localCache: true, + role: role, + }, nil) + } +} + +// If there is a local cache, we update anyway and see if anything's different +// (assuming remote has a root.json). If the server has new data, we cannot +// use the local cache so if the server is missing any metadata we cannot update. +// Skipping force check, because that only matters for root. +func TestUpdateNonRootRemoteMissingMetadataCannotUseLocalCache(t *testing.T) { + for _, role := range append(data.BaseRoles, delegationsWithNonEmptyMetadata...) { + if role == data.CanonicalRootRole { + continue + } + var errExpected interface{} = store.ErrMetaNotFound{} + if role == data.CanonicalTimestampRole { + // if we can't download the timestamp, we use the cached timestamp. + // it says that we have all the local data already, so we download + // nothing. So the update won't error, it will just fail to update + // to the latest version. We log a warning in this case. + errExpected = nil + } + + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusNotFound, + serverHasNewData: true, + localCache: true, + role: role, + }, errExpected) + } +} + +// If there is no local cache, we just update. If the server 50X's when getting +// metadata, we propagate ErrServerUnavailable. +func TestUpdateNonRootRemote50XNoLocalCache(t *testing.T) { + for _, role := range append(data.BaseRoles, delegationsWithNonEmptyMetadata...) { + if role == data.CanonicalRootRole { + continue + } + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + role: role, + }, store.ErrServerUnavailable{}) + } +} + +// If there is a local cache, we update anyway and see if anything's different +// (assuming remote has a root.json). If the timestamp is 50X's, we use the +// local timestamp and already have all data, so nothing needs to be downloaded. +// If the timestamp is present, but the same, we already have all the data, so +// nothing needs to be downloaded. +func TestUpdateNonRootRemote50XCanUseLocalCache(t *testing.T) { + // actually everything can error at once, but it's better to check one by + // one in case we change the download code somewhat. + for _, role := range append(data.BaseRoles, delegationsWithNonEmptyMetadata...) { + if role == data.CanonicalRootRole { + continue + } + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + localCache: true, + role: role, + }, nil) + } +} + +// If there is a local cache, we update anyway and see if anything's different +// (assuming remote has a root.json). If the server has new data, we cannot +// use the local cache so if the server 50X's on any metadata we cannot update. +// This happens whether or not we force a remote check (because that's on the +// root) +func TestUpdateNonRootRemote50XCannotUseLocalCache(t *testing.T) { + for _, role := range append(data.BaseRoles, delegationsWithNonEmptyMetadata...) { + if role == data.CanonicalRootRole { + continue + } + + var errExpected interface{} = store.ErrServerUnavailable{} + if role == data.CanonicalTimestampRole { + // if we can't download the timestamp, we use the cached timestamp. + // it says that we have all the local data already, so we download + // nothing. So the update won't error, it will just fail to update + // to the latest version. We log a warning in this case. + errExpected = nil + } + + testUpdateRemoteNon200Error(t, updateOpts{ + notFoundCode: http.StatusServiceUnavailable, + serverHasNewData: true, + localCache: true, + role: role, + }, errExpected) + } +} + +func testUpdateRemoteNon200Error(t *testing.T, opts updateOpts, errExpected interface{}) { + _, serverSwizzler := newServerSwizzler(t) + ts := readOnlyServer(t, serverSwizzler.MetadataCache, opts.notFoundCode) + defer ts.Close() + + repo := newBlankRepo(t, ts.URL) + defer os.RemoveAll(repo.baseDir) + + if opts.localCache { + _, err := repo.Update(false) // acquire local cache + require.NoError(t, err) + } + + if opts.serverHasNewData { + bumpVersions(t, serverSwizzler, 1) + } + + require.NoError(t, serverSwizzler.RemoveMetadata(opts.role), "failed to remove %s", opts.role) + + _, err := repo.Update(opts.forWrite) + if errExpected == nil { + require.NoError(t, err, "expected no failure updating when %s is %v (forWrite: %v)", + opts.role, opts.notFoundCode, opts.forWrite) + } else { + require.Error(t, err, "expected failure updating when %s is %v (forWrite: %v)", + opts.role, opts.notFoundCode, opts.forWrite) + require.IsType(t, errExpected, err, "wrong update error when %s is %v (forWrite: %v)", + opts.role, opts.notFoundCode, opts.forWrite) + if notFound, ok := err.(store.ErrMetaNotFound); ok { + require.Equal(t, opts.role, notFound.Resource, "wrong resource missing (forWrite: %v)", opts.forWrite) + } + } +} diff --git a/tuf/client/client.go b/tuf/client/client.go index b35757851e..33ffa20936 100644 --- a/tuf/client/client.go +++ b/tuf/client/client.go @@ -54,7 +54,7 @@ func (c *Client) Update() error { if err != nil { logrus.Debug("Error occurred. Root will be downloaded and another update attempted") if err := c.downloadRoot(); err != nil { - logrus.Error("client Update (Root):", err) + logrus.Error("Client Update (Root):", err) return err } // If we error again, we now have the latest root and just want to fail @@ -247,28 +247,27 @@ func (c *Client) downloadTimestamp() error { // We may not have a cached timestamp if this is the first time // we're interacting with the repo. This will result in the // version being 0 - var download bool - old := &data.Signed{} - version := 0 + var ( + saveToCache bool + old *data.Signed + version = 0 + ) cachedTS, err := c.cache.GetMeta(role, maxSize) if err == nil { - err := json.Unmarshal(cachedTS, old) + cached := &data.Signed{} + err := json.Unmarshal(cachedTS, cached) if err == nil { - ts, err := data.TimestampFromSigned(old) + ts, err := data.TimestampFromSigned(cached) if err == nil { version = ts.Signed.Version } - } else { - old = nil + old = cached } } // unlike root, targets and snapshot, always try and download timestamps // from remote, only using the cache one if we couldn't reach remote. raw, s, err := c.downloadSigned(role, maxSize, nil) if err != nil || len(raw) == 0 { - if err, ok := err.(store.ErrMetaNotFound); ok { - return err - } if old == nil { if err == nil { // couldn't retrieve data from server and don't have valid @@ -277,17 +276,18 @@ func (c *Client) downloadTimestamp() error { } return err } - logrus.Debug("using cached timestamp") + logrus.Debug(err.Error()) + logrus.Warn("Error while downloading remote metadata, using cached timestamp - this might not be the latest version available remotely") s = old } else { - download = true + saveToCache = true } err = signed.Verify(s, role, version, c.keysDB) if err != nil { return err } logrus.Debug("successfully verified timestamp") - if download { + if saveToCache { c.cache.SetMeta(role, raw) } ts, err := data.TimestampFromSigned(s)