From a6159a45d16b5cc453c5effe4f7ce84296bcffc8 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Fri, 22 Jan 2016 14:54:16 -0800 Subject: [PATCH] ensure filestore GetMeta only returns up to size bytes. Standardize constant for max size Signed-off-by: Riyaz Faizullabhoy --- client/client.go | 14 +++++-------- client/client_update_test.go | 13 ++++++------ const.go | 2 ++ tuf/client/client.go | 9 ++++----- tuf/store/filestore.go | 8 ++++++-- tuf/store/filestore_test.go | 6 ++++++ tuf/testutils/swizzler.go | 21 +++++++++---------- tuf/testutils/swizzler_test.go | 37 +++++++++++++++++----------------- 8 files changed, 58 insertions(+), 52 deletions(-) diff --git a/client/client.go b/client/client.go index b383c94dca..01eb007375 100644 --- a/client/client.go +++ b/client/client.go @@ -25,10 +25,6 @@ import ( "github.com/docker/notary/tuf/store" ) -const ( - maxSize = 5 << 20 -) - func init() { data.SetDefaultExpiryTimes( map[string]int{ @@ -747,7 +743,7 @@ func (r *NotaryRepository) bootstrapRepo() error { tufRepo := tuf.NewRepo(kdb, r.CryptoService) logrus.Debugf("Loading trusted collection.") - rootJSON, err := r.fileStore.GetMeta("root", 0) + rootJSON, err := r.fileStore.GetMeta("root", notary.MaxMetaSize) if err != nil { return err } @@ -760,7 +756,7 @@ func (r *NotaryRepository) bootstrapRepo() error { if err != nil { return err } - targetsJSON, err := r.fileStore.GetMeta("targets", 0) + targetsJSON, err := r.fileStore.GetMeta("targets", notary.MaxMetaSize) if err != nil { return err } @@ -771,7 +767,7 @@ func (r *NotaryRepository) bootstrapRepo() error { } tufRepo.SetTargets("targets", targets) - snapshotJSON, err := r.fileStore.GetMeta("snapshot", 0) + snapshotJSON, err := r.fileStore.GetMeta("snapshot", notary.MaxMetaSize) if err == nil { snapshot := &data.SignedSnapshot{} err = json.Unmarshal(snapshotJSON, snapshot) @@ -876,7 +872,7 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Cl // try to read root from cache first. We will trust this root // until we detect a problem during update which will cause // us to download a new root and perform a rotation. - rootJSON, cachedRootErr := r.fileStore.GetMeta("root", maxSize) + rootJSON, cachedRootErr := r.fileStore.GetMeta("root", notary.MaxMetaSize) if cachedRootErr == nil { signedRoot, cachedRootErr = r.validateRoot(rootJSON) @@ -890,7 +886,7 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Cl // checking for initialization of the repo). // if remote store successfully set up, try and get root from remote - tmpJSON, err := remote.GetMeta("root", maxSize) + tmpJSON, err := remote.GetMeta("root", notary.MaxMetaSize) if err != nil { // we didn't have a root in cache and were unable to load one from // the server. Nothing we can do but error. diff --git a/client/client_update_test.go b/client/client_update_test.go index 1bfb86873e..b6394b4030 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -9,6 +9,7 @@ import ( "os" "testing" + "github.com/docker/notary" "github.com/docker/notary/passphrase" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/store" @@ -58,7 +59,7 @@ func readOnlyServer(t *testing.T, cache store.MetadataStore, notFoundStatus int) 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) + metaBytes, err := cache.GetMeta(vars["role"], notary.MaxMetaSize) if _, ok := err.(store.ErrMetaNotFound); ok { w.WriteHeader(notFoundStatus) } else { @@ -107,7 +108,7 @@ func TestUpdateSucceedsEvenIfCannotWriteNewRepo(t *testing.T) { } for r, expected := range serverMeta { - actual, err := repo.fileStore.GetMeta(r, maxSize) + actual, err := repo.fileStore.GetMeta(r, notary.MaxMetaSize) if r == role { require.Error(t, err) require.IsType(t, store.ErrMetaNotFound{}, err, @@ -158,7 +159,7 @@ func TestUpdateSucceedsEvenIfCannotWriteExistingRepo(t *testing.T) { require.NoError(t, err) for r, expected := range serverMeta { - actual, err := repo.fileStore.GetMeta(r, maxSize) + actual, err := repo.fileStore.GetMeta(r, notary.MaxMetaSize) require.NoError(t, err, "problem getting repo metadata for %s", r) if role == r { require.False(t, bytes.Equal(expected, actual), @@ -223,7 +224,7 @@ func TestUpdateReplacesCorruptOrMissingMetadata(t *testing.T) { _, err := repo.Update(forWrite) require.NoError(t, err) for r, expected := range serverMeta { - actual, err := repo.fileStore.GetMeta(r, maxSize) + actual, err := repo.fileStore.GetMeta(r, notary.MaxMetaSize) require.NoError(t, err, "problem getting repo metadata for %s", role) require.True(t, bytes.Equal(expected, actual), "%s for %s: expected to recover after update", text, role) @@ -270,7 +271,7 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) { for text, messItUp := range waysToMessUpLocalMetadata(repoSwizzler) { for _, forWrite := range []bool{true, false} { require.NoError(t, messItUp(data.CanonicalRootRole), "could not fuzz root (%s)", text) - messedUpMeta, err := repo.fileStore.GetMeta(data.CanonicalRootRole, maxSize) + messedUpMeta, err := repo.fileStore.GetMeta(data.CanonicalRootRole, notary.MaxMetaSize) if _, ok := err.(store.ErrMetaNotFound); ok { // one of the ways to mess up is to delete metadata @@ -289,7 +290,7 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) { // same because it has failed to update. for role, expected := range origMeta { if role != data.CanonicalTimestampRole && role != data.CanonicalSnapshotRole { - actual, err := repo.fileStore.GetMeta(role, maxSize) + actual, err := repo.fileStore.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err, "problem getting repo metadata for %s", role) if role == data.CanonicalRootRole { diff --git a/const.go b/const.go index a1140c0dc0..855d0b7a55 100644 --- a/const.go +++ b/const.go @@ -2,6 +2,8 @@ package notary // application wide constants const ( + // MaxMetaSize is the maximum size of metadata - 5MiB + MaxMetaSize int64 = 5 << 20 // MinRSABitSize is the minimum bit size for RSA keys allowed in notary MinRSABitSize = 2048 // MinThreshold requires a minimum of one threshold for roles; currently we do not support a higher threshold diff --git a/tuf/client/client.go b/tuf/client/client.go index 0eaa8c87e7..4c562e0ee8 100644 --- a/tuf/client/client.go +++ b/tuf/client/client.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/Sirupsen/logrus" + "github.com/docker/notary" tuf "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/keys" @@ -19,8 +20,6 @@ import ( "github.com/docker/notary/tuf/utils" ) -const maxSize int64 = 5 << 20 - // Client is a usability wrapper around a raw TUF repo type Client struct { local *tuf.Repo @@ -131,7 +130,7 @@ func (c Client) checkRoot() error { func (c *Client) downloadRoot() error { logrus.Debug("Downloading Root...") role := data.CanonicalRootRole - size := maxSize + size := notary.MaxMetaSize var expectedSha256 []byte if c.local.Snapshot != nil { size = c.local.Snapshot.Signed.Meta[role].Length @@ -252,7 +251,7 @@ func (c *Client) downloadTimestamp() error { old *data.Signed version = 0 ) - cachedTS, err := c.cache.GetMeta(role, maxSize) + cachedTS, err := c.cache.GetMeta(role, notary.MaxMetaSize) if err == nil { cached := &data.Signed{} err := json.Unmarshal(cachedTS, cached) @@ -266,7 +265,7 @@ func (c *Client) downloadTimestamp() error { } // 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) + raw, s, err := c.downloadSigned(role, notary.MaxMetaSize, nil) if err != nil || len(raw) == 0 { if old == nil { if err == nil { diff --git a/tuf/store/filestore.go b/tuf/store/filestore.go index 52e7c8f289..9a61a91578 100644 --- a/tuf/store/filestore.go +++ b/tuf/store/filestore.go @@ -44,7 +44,7 @@ func (f *FilesystemStore) getPath(name string) string { return filepath.Join(f.metaDir, fileName) } -// GetMeta returns the meta for the given name (a role) +// GetMeta returns the meta for the given name (a role) up to size bytes func (f *FilesystemStore) GetMeta(name string, size int64) ([]byte, error) { meta, err := ioutil.ReadFile(f.getPath(name)) if err != nil { @@ -53,7 +53,11 @@ func (f *FilesystemStore) GetMeta(name string, size int64) ([]byte, error) { } return nil, err } - return meta, nil + // Only return up to size bytes + if int64(len(meta)) < size { + return meta, nil + } + return meta[:size], nil } // SetMultiMeta sets the metadata for multiple roles in one operation diff --git a/tuf/store/filestore_test.go b/tuf/store/filestore_test.go index 6c6d273569..34bfc1234a 100644 --- a/tuf/store/filestore_test.go +++ b/tuf/store/filestore_test.go @@ -89,6 +89,12 @@ func TestGetMeta(t *testing.T) { assert.Nil(t, err, "GetMeta returned unexpected error: %v", err) assert.Equal(t, testContent, content, "Content read from file was corrupted.") + + // Check that we return only up to size bytes + content, err = s.GetMeta("testMeta", 4) + assert.Nil(t, err, "GetMeta returned unexpected error: %v", err) + + assert.Equal(t, []byte("test"), content, "Content read from file was corrupted.") } func TestGetSetMetadata(t *testing.T) { diff --git a/tuf/testutils/swizzler.go b/tuf/testutils/swizzler.go index 91a0c780e1..e7e5c5e349 100644 --- a/tuf/testutils/swizzler.go +++ b/tuf/testutils/swizzler.go @@ -6,6 +6,7 @@ import ( "time" "github.com/docker/go/canonical/json" + "github.com/docker/notary" "github.com/docker/notary/cryptoservice" "github.com/docker/notary/passphrase" "github.com/docker/notary/trustmanager" @@ -14,10 +15,6 @@ import ( "github.com/docker/notary/tuf/store" ) -const ( - maxSize = 5 << 20 -) - // ErrNoKeyForRole returns an error when the cryptoservice provided to // MetadataSwizzler has no key for a particular role type ErrNoKeyForRole struct { @@ -87,7 +84,7 @@ func serializeMetadata(cs signed.CryptoService, s *data.Signed, role string, // gets a Signed from the metadata store func signedFromStore(cache store.MetadataStore, role string) (*data.Signed, error) { - b, err := cache.GetMeta(role, maxSize) + b, err := cache.GetMeta(role, notary.MaxMetaSize) if err != nil { return nil, err } @@ -120,7 +117,7 @@ func NewMetadataSwizzler(gun string, initialMetadata map[string][]byte, // SetInvalidJSON corrupts metadata into something that is no longer valid JSON func (m *MetadataSwizzler) SetInvalidJSON(role string) error { - metaBytes, err := m.MetadataCache.GetMeta(role, maxSize) + metaBytes, err := m.MetadataCache.GetMeta(role, notary.MaxMetaSize) if err != nil { return err } @@ -327,7 +324,7 @@ func (m *MetadataSwizzler) SetThreshold(role string, newThreshold int) error { roleSpecifier = path.Dir(role) } - b, err := m.MetadataCache.GetMeta(roleSpecifier, maxSize) + b, err := m.MetadataCache.GetMeta(roleSpecifier, notary.MaxMetaSize) if err != nil { return err } @@ -377,7 +374,7 @@ func (m *MetadataSwizzler) ChangeRootKey() error { return err } - b, err := m.MetadataCache.GetMeta(data.CanonicalRootRole, maxSize) + b, err := m.MetadataCache.GetMeta(data.CanonicalRootRole, notary.MaxMetaSize) if err != nil { return err } @@ -410,7 +407,7 @@ func (m *MetadataSwizzler) UpdateSnapshotHashes(roles ...string) error { snapshotSigned *data.Signed err error ) - if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, maxSize); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, notary.MaxMetaSize); err != nil { return err } @@ -426,7 +423,7 @@ func (m *MetadataSwizzler) UpdateSnapshotHashes(roles ...string) error { for _, role := range roles { if role != data.CanonicalSnapshotRole && role != data.CanonicalTimestampRole { - if metaBytes, err = m.MetadataCache.GetMeta(role, maxSize); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(role, notary.MaxMetaSize); err != nil { return err } @@ -458,7 +455,7 @@ func (m *MetadataSwizzler) UpdateTimestampHash() error { timestampSigned *data.Signed err error ) - if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalTimestampRole, maxSize); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalTimestampRole, notary.MaxMetaSize); err != nil { return err } // we can't just create a new timestamp, because then the expiry would be @@ -467,7 +464,7 @@ func (m *MetadataSwizzler) UpdateTimestampHash() error { return err } - if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, maxSize); err != nil { + if metaBytes, err = m.MetadataCache.GetMeta(data.CanonicalSnapshotRole, notary.MaxMetaSize); err != nil { return err } diff --git a/tuf/testutils/swizzler_test.go b/tuf/testutils/swizzler_test.go index 6ee26f42d7..6d967e1ba9 100644 --- a/tuf/testutils/swizzler_test.go +++ b/tuf/testutils/swizzler_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/docker/notary" "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/keys" @@ -81,7 +82,7 @@ func TestSwizzlerSetInvalidJSON(t *testing.T) { f.SetInvalidJSON(data.CanonicalSnapshotRole) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != data.CanonicalSnapshotRole { @@ -103,7 +104,7 @@ func TestSwizzlerSetInvalidSigned(t *testing.T) { f.SetInvalidSigned(data.CanonicalTargetsRole) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != data.CanonicalTargetsRole { @@ -127,7 +128,7 @@ func TestSwizzlerSetInvalidSignedMeta(t *testing.T) { f.SetInvalidSignedMeta(data.CanonicalTargetsRole) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != data.CanonicalTargetsRole { @@ -151,7 +152,7 @@ func TestSwizzlerSetInvalidMetadataType(t *testing.T) { f.SetInvalidMetadataType(data.CanonicalTargetsRole) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != data.CanonicalTargetsRole { @@ -174,7 +175,7 @@ func TestSwizzlerInvalidateMetadataSignatures(t *testing.T) { f.InvalidateMetadataSignatures(data.CanonicalRootRole) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != data.CanonicalRootRole { @@ -205,7 +206,7 @@ func TestSwizzlerRemoveMetadata(t *testing.T) { f.RemoveMetadata("targets/a") for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) if role != "targets/a" { require.NoError(t, err) require.True(t, bytes.Equal(metaBytes, newMeta), "bytes have changed for role %s", role) @@ -223,7 +224,7 @@ func TestSwizzlerSignMetadataWithInvalidKey(t *testing.T) { f.SignMetadataWithInvalidKey(data.CanonicalTimestampRole) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != data.CanonicalTimestampRole { @@ -250,7 +251,7 @@ func TestSwizzlerOffsetMetadataVersion(t *testing.T) { f.OffsetMetadataVersion("targets/a", -2) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != "targets/a" { @@ -273,7 +274,7 @@ func TestSwizzlerExpireMetadata(t *testing.T) { f.ExpireMetadata(data.CanonicalRootRole) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) if role != data.CanonicalRootRole { @@ -297,7 +298,7 @@ func TestSwizzlerSetThresholdBaseRole(t *testing.T) { f.SetThreshold(data.CanonicalTargetsRole, 3) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) // the threshold for base roles is set in root @@ -325,7 +326,7 @@ func TestSwizzlerSetThresholdDelegatedRole(t *testing.T) { f.SetThreshold("targets/a/b", 3) for role, metaBytes := range origMeta { - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) // the threshold for "targets/a/b" is in "targets/a" @@ -357,7 +358,7 @@ func TestSwizzlerChangeRootKey(t *testing.T) { for _, role := range roles { origMeta := origMeta[role] - newMeta, err := f.MetadataCache.GetMeta(role, maxSize) + newMeta, err := f.MetadataCache.GetMeta(role, notary.MaxMetaSize) require.NoError(t, err) // the threshold for base roles is set in root @@ -400,7 +401,7 @@ func TestSwizzlerUpdateSnapshotHashesSpecifiedRoles(t *testing.T) { // nothing has changed, signed data should be the same (signatures might // change because signatures may have random elements f.UpdateSnapshotHashes(data.CanonicalTargetsRole) - newMeta, err := f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, maxSize) + newMeta, err := f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, notary.MaxMetaSize) origSigned, newSigned := &data.Signed{}, &data.Signed{} require.NoError(t, json.Unmarshal(origMeta[data.CanonicalSnapshotRole], origSigned)) @@ -414,7 +415,7 @@ func TestSwizzlerUpdateSnapshotHashesSpecifiedRoles(t *testing.T) { // update the snapshot with just 1 role f.UpdateSnapshotHashes(data.CanonicalTargetsRole) - newMeta, err = f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, maxSize) + newMeta, err = f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, notary.MaxMetaSize) require.NoError(t, err) require.False(t, bytes.Equal(origMeta[data.CanonicalSnapshotRole], newMeta)) @@ -444,7 +445,7 @@ func TestSwizzlerUpdateSnapshotHashesNoSpecifiedRoles(t *testing.T) { // nothing has changed, signed data should be the same (signatures might // change because signatures may have random elements f.UpdateSnapshotHashes() - newMeta, err := f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, maxSize) + newMeta, err := f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, notary.MaxMetaSize) require.NoError(t, err) origSigned, newSigned := &data.Signed{}, &data.Signed{} @@ -459,7 +460,7 @@ func TestSwizzlerUpdateSnapshotHashesNoSpecifiedRoles(t *testing.T) { // update the snapshot with just no specified roles f.UpdateSnapshotHashes() - newMeta, err = f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, maxSize) + newMeta, err = f.MetadataCache.GetMeta(data.CanonicalSnapshotRole, notary.MaxMetaSize) require.NoError(t, err) require.False(t, bytes.Equal(origMeta[data.CanonicalSnapshotRole], newMeta)) @@ -490,7 +491,7 @@ func TestSwizzlerUpdateTimestamp(t *testing.T) { // nothing has changed, signed data should be the same (signatures might // change because signatures may have random elements f.UpdateTimestampHash() - newMeta, err := f.MetadataCache.GetMeta(data.CanonicalTimestampRole, maxSize) + newMeta, err := f.MetadataCache.GetMeta(data.CanonicalTimestampRole, notary.MaxMetaSize) require.NoError(t, err) origSigned, newSigned := &data.Signed{}, &data.Signed{} @@ -503,7 +504,7 @@ func TestSwizzlerUpdateTimestamp(t *testing.T) { // update the timestamp f.UpdateTimestampHash() - newMeta, err = f.MetadataCache.GetMeta(data.CanonicalTimestampRole, maxSize) + newMeta, err = f.MetadataCache.GetMeta(data.CanonicalTimestampRole, notary.MaxMetaSize) require.NoError(t, err) require.False(t, bytes.Equal(origMeta[data.CanonicalTimestampRole], newMeta))