From 1bf3dd08db83cd79862545803ed35e68cf5bf20f Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 1 Feb 2016 13:03:55 -0800 Subject: [PATCH] Addressing comments from review Signed-off-by: David Lawrence (github: endophage) --- tuf/client/client.go | 12 ++++---- tuf/client/client_test.go | 17 ++++------- tuf/store/memorystore.go | 6 ++-- tuf/testutils/corrupt_memorystore.go | 43 ++++++++++++++++++++++++++++ tuf/utils/utils.go | 6 ++-- 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/tuf/client/client.go b/tuf/client/client.go index 88734bf9e0..fb3e5136ce 100644 --- a/tuf/client/client.go +++ b/tuf/client/client.go @@ -177,7 +177,7 @@ func (c *Client) downloadRoot() error { var raw []byte if download { // use consistent download if we have the checksum. - raw, s, err = c.downloadSigned(role, size, expectedSha256, len(expectedSha256) > 0) + raw, s, err = c.downloadSigned(role, size, expectedSha256) if err != nil { return err } @@ -265,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, notary.MaxTimestampSize, nil, false) + raw, s, err := c.downloadSigned(role, notary.MaxTimestampSize, nil) if err == nil { ts, err = c.verifyTimestamp(s, version, c.keysDB) if err == nil { @@ -342,7 +342,7 @@ func (c *Client) downloadSnapshot() error { } var s *data.Signed if download { - raw, s, err = c.downloadSigned(role, size, expectedSha256, true) + raw, s, err = c.downloadSigned(role, size, expectedSha256) if err != nil { return err } @@ -419,8 +419,8 @@ func (c *Client) downloadTargets(role string) error { return nil } -func (c *Client) downloadSigned(role string, size int64, expectedSha256 []byte, consistent bool) ([]byte, *data.Signed, error) { - rolePath := utils.URLFilePath(role, expectedSha256, consistent) +func (c *Client) downloadSigned(role string, size int64, expectedSha256 []byte) ([]byte, *data.Signed, error) { + rolePath := utils.ConsistentName(role, expectedSha256) raw, err := c.remote.GetMeta(rolePath, size) if err != nil { return nil, nil, err @@ -480,7 +480,7 @@ func (c Client) getTargetsFile(role string, keyIDs []string, snapshotMeta data.F size := snapshotMeta[role].Length var s *data.Signed if download { - raw, s, err = c.downloadSigned(role, size, expectedSha256, true) + raw, s, err = c.downloadSigned(role, size, expectedSha256) if err != nil { return nil, err } diff --git a/tuf/client/client_test.go b/tuf/client/client_test.go index 6fb4c2bf88..e76d1bf5ff 100644 --- a/tuf/client/client_test.go +++ b/tuf/client/client_test.go @@ -245,7 +245,7 @@ func TestChecksumMismatch(t *testing.T) { remoteStorage.SetMeta("targets", orig) - _, _, err = client.downloadSigned("targets", int64(len(orig)), origSha256[:], false) + _, _, err = client.downloadSigned("targets", int64(len(orig)), origSha256[:]) assert.IsType(t, ErrChecksumMismatch{}, err) } @@ -262,14 +262,14 @@ func TestChecksumMatch(t *testing.T) { remoteStorage.SetMeta("targets", orig) - _, _, err = client.downloadSigned("targets", int64(len(orig)), origSha256[:], false) + _, _, err = client.downloadSigned("targets", int64(len(orig)), origSha256[:]) assert.NoError(t, err) } func TestSizeMismatchLong(t *testing.T) { repo := tuf.NewRepo(nil, nil) localStorage := store.NewMemoryStore(nil, nil) - remoteStorage := store.NewMemoryStore(nil, nil) + remoteStorage := testutils.NewLongMemoryStore(nil, nil) client := NewClient(repo, remoteStorage, nil, localStorage) sampleTargets := data.NewTargets() @@ -278,12 +278,9 @@ func TestSizeMismatchLong(t *testing.T) { assert.NoError(t, err) l := int64(len(orig)) - orig = append([]byte(" "), orig...) - assert.Equal(t, l+1, int64(len(orig))) - remoteStorage.SetMeta("targets", orig) - _, _, err = client.downloadSigned("targets", l, origSha256[:], false) + _, _, err = client.downloadSigned("targets", l, origSha256[:]) // size just limits the data received, the error is caught // either during checksum verification or during json deserialization assert.IsType(t, ErrChecksumMismatch{}, err) @@ -292,7 +289,7 @@ func TestSizeMismatchLong(t *testing.T) { func TestSizeMismatchShort(t *testing.T) { repo := tuf.NewRepo(nil, nil) localStorage := store.NewMemoryStore(nil, nil) - remoteStorage := store.NewMemoryStore(nil, nil) + remoteStorage := testutils.NewShortMemoryStore(nil, nil) client := NewClient(repo, remoteStorage, nil, localStorage) sampleTargets := data.NewTargets() @@ -301,11 +298,9 @@ func TestSizeMismatchShort(t *testing.T) { assert.NoError(t, err) l := int64(len(orig)) - orig = orig[1:] - remoteStorage.SetMeta("targets", orig) - _, _, err = client.downloadSigned("targets", l, origSha256[:], false) + _, _, err = client.downloadSigned("targets", l, origSha256[:]) // size just limits the data received, the error is caught // either during checksum verification or during json deserialization assert.IsType(t, ErrChecksumMismatch{}, err) diff --git a/tuf/store/memorystore.go b/tuf/store/memorystore.go index b0a630ad4f..44fd13784e 100644 --- a/tuf/store/memorystore.go +++ b/tuf/store/memorystore.go @@ -19,7 +19,7 @@ func NewMemoryStore(meta map[string][]byte, files map[string][]byte) *MemoryStor // add all seed meta to consistent for name, data := range meta { checksum := sha256.Sum256(data) - path := utils.URLFilePath(name, checksum[:], true) + path := utils.ConsistentName(name, checksum[:]) consistent[path] = data } } @@ -73,7 +73,7 @@ func (m *MemoryStore) SetMeta(name string, meta []byte) error { m.meta[name] = meta checksum := sha256.Sum256(meta) - path := utils.URLFilePath(name, checksum[:], true) + path := utils.ConsistentName(name, checksum[:]) m.consistent[path] = meta return nil } @@ -92,7 +92,7 @@ func (m *MemoryStore) SetMultiMeta(metas map[string][]byte) error { func (m *MemoryStore) RemoveMeta(name string) error { if meta, ok := m.meta[name]; ok { checksum := sha256.Sum256(meta) - path := utils.URLFilePath(name, checksum[:], true) + path := utils.ConsistentName(name, checksum[:]) delete(m.meta, name) delete(m.consistent, path) } diff --git a/tuf/testutils/corrupt_memorystore.go b/tuf/testutils/corrupt_memorystore.go index 52b2b70680..58a0f4c10c 100644 --- a/tuf/testutils/corrupt_memorystore.go +++ b/tuf/testutils/corrupt_memorystore.go @@ -26,3 +26,46 @@ func (cm CorruptingMemoryStore) GetMeta(name string, size int64) ([]byte, error) d[0] = '}' // all our content is JSON so must start with { return d, err } + +// LongMemoryStore corrupts all data returned by GetMeta +type LongMemoryStore struct { + store.MemoryStore +} + +// NewLongMemoryStore returns a new instance of memory store that +// returns one byte too much data on any request to GetMeta +func NewLongMemoryStore(meta map[string][]byte, files map[string][]byte) *LongMemoryStore { + s := store.NewMemoryStore(meta, files) + return &LongMemoryStore{MemoryStore: *s} +} + +// GetMeta returns one byte too much +func (lm LongMemoryStore) GetMeta(name string, size int64) ([]byte, error) { + d, err := lm.MemoryStore.GetMeta(name, size) + if err != nil { + return nil, err + } + d = append(d, ' ') + return d, err +} + +// ShortMemoryStore corrupts all data returned by GetMeta +type ShortMemoryStore struct { + store.MemoryStore +} + +// NewShortMemoryStore returns a new instance of memory store that +// returns one byte too little data on any request to GetMeta +func NewShortMemoryStore(meta map[string][]byte, files map[string][]byte) *ShortMemoryStore { + s := store.NewMemoryStore(meta, files) + return &ShortMemoryStore{MemoryStore: *s} +} + +// GetMeta returns one byte too few +func (sm ShortMemoryStore) GetMeta(name string, size int64) ([]byte, error) { + d, err := sm.MemoryStore.GetMeta(name, size) + if err != nil { + return nil, err + } + return d[1:], err +} diff --git a/tuf/utils/utils.go b/tuf/utils/utils.go index 48c68ec1bd..d676b8782f 100644 --- a/tuf/utils/utils.go +++ b/tuf/utils/utils.go @@ -148,11 +148,11 @@ func FindRoleIndex(rs []*data.Role, name string) int { return -1 } -// URLFilePath generates the appropriate HTTP URL path for the role, +// ConsistentName generates the appropriate HTTP URL path for the role, // based on whether the repo is marked as consistent. The RemoteStore // is responsible for adding file extensions. -func URLFilePath(role string, hashSha256 []byte, consistent bool) string { - if consistent && len(hashSha256) > 0 { +func ConsistentName(role string, hashSha256 []byte) string { + if len(hashSha256) > 0 { hash := hex.EncodeToString(hashSha256) return fmt.Sprintf("%s.%s", role, hash) }