From 4ca877d471b8b4e9319ac5131de6fdf0d4dd97b4 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 10 Feb 2020 19:16:33 -0800 Subject: [PATCH] fix: be explicit about key limitations Only allow keys of the form `/[0-9A-Z+-_=]`. That is, upper-case alphanumeric keys in the root namespace (plus some special characters). Why? We don't encode keys before writing them to the filesystem. This change ensures that: 1. Case sensitivity doesn't matter because we only allow upper-case keys. 2. Path separators and special characters doesn't matter. For context, go-ipfs only uses flatfs for storing blocks. Every block CID is encoded as uppercase alphanumeric text (specifically, uppercase base32). We could be less restrictive, but this is safer and easier to understand. Unfortunately, we _can't_ allow mixed case (Windows) and can't allow lowercase because we're already using uppercase keys. fixes #23 --- README.md | 15 +++++++- convert_test.go | 4 +- flatfs.go | 31 +++++++++++++++- flatfs_test.go | 99 +++++++++++++++++++++++++++++++++---------------- go.mod | 4 +- go.sum | 4 +- key.go | 28 ++++++++++++++ key_test.go | 36 ++++++++++++++++++ 8 files changed, 182 insertions(+), 39 deletions(-) create mode 100644 key.go create mode 100644 key_test.go diff --git a/README.md b/README.md index a75b150..89df8a0 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,9 @@ `go-ds-flatfs` is used by `go-ipfs` to store raw block contents on disk. It supports several sharding functions (prefix, suffix, next-to-last/*). +It is _not_ a general-purpose datastore and has several important restrictions. +See the restrictions section for details. + ## Lead Maintainer [Jakub Sztandera](https://github.com/kubuxu) @@ -33,13 +36,21 @@ import "github.com/ipfs/go-ds-flatfs" ``` -`go-ds-flatfs` uses [`Gx`](https://github.com/whyrusleeping/gx) and [`Gx-go`](https://github.com/whyrusleeping/gx-go) to handle dependendencies. Run `make deps` to download and rewrite the imports to their fixed dependencies. - ## Usage Check the [GoDoc module documentation](https://godoc.org/github.com/ipfs/go-ds-flatfs) for an overview of this module's functionality. +### Restrictions + +FlatFS keys are severely restricted. Only keys that match `/[0-9A-Z+-_=]\+` are +allowed. That is, keys may only contain upper-case alpha-numeric characters, +'-', '+', '_', and '='. This is because values are written directly to the +filesystem without encoding. + +Importantly, this means namespaced keys (e.g., /FOO/BAR), are _not_ allowed. +Attempts to write to such keys will result in an error. + ### DiskUsage and Accuracy This datastore implements the [`PersistentDatastore`](https://godoc.org/github.com/ipfs/go-datastore#PersistentDatastore) interface. It offers a `DiskUsage()` method which strives to find a balance between accuracy and performance. This implies: diff --git a/convert_test.go b/convert_test.go index 2b173cc..9e819ad 100644 --- a/convert_test.go +++ b/convert_test.go @@ -2,7 +2,7 @@ package flatfs_test import ( "bytes" - "encoding/hex" + "encoding/base32" "io/ioutil" "math/rand" "os" @@ -205,7 +205,7 @@ func populateDatastore(t *testing.T, dir string) ([]datastore.Key, [][]byte) { r.Read(blk) blocks = append(blocks, blk) - key := "x" + hex.EncodeToString(blk[:8]) + key := "X" + base32.StdEncoding.EncodeToString(blk[:8]) keys = append(keys, datastore.NewKey(key)) err := ds.Put(keys[i], blocks[i]) if err != nil { diff --git a/flatfs.go b/flatfs.go index 24741ac..2f8a3cf 100644 --- a/flatfs.go +++ b/flatfs.go @@ -361,6 +361,10 @@ var putMaxRetries = 6 // concurrent Put and a Delete operation, we cannot guarantee which one // will win. func (fs *Datastore) Put(key datastore.Key, value []byte) error { + if !keyIsValid(key) { + return fmt.Errorf("key not supported by flatfs: '%q'", key) + } + fs.shutdownLock.RLock() defer fs.shutdownLock.RUnlock() if fs.shutdown { @@ -580,6 +584,11 @@ func (fs *Datastore) putMany(data map[datastore.Key][]byte) error { } func (fs *Datastore) Get(key datastore.Key) (value []byte, err error) { + // Can't exist in datastore. + if !keyIsValid(key) { + return nil, datastore.ErrNotFound + } + _, path := fs.encode(key) data, err := ioutil.ReadFile(path) if err != nil { @@ -593,6 +602,11 @@ func (fs *Datastore) Get(key datastore.Key) (value []byte, err error) { } func (fs *Datastore) Has(key datastore.Key) (exists bool, err error) { + // Can't exist in datastore. + if !keyIsValid(key) { + return false, nil + } + _, path := fs.encode(key) switch _, err := os.Stat(path); { case err == nil: @@ -605,6 +619,11 @@ func (fs *Datastore) Has(key datastore.Key) (exists bool, err error) { } func (fs *Datastore) GetSize(key datastore.Key) (size int, err error) { + // Can't exist in datastore. + if !keyIsValid(key) { + return -1, datastore.ErrNotFound + } + _, path := fs.encode(key) switch s, err := os.Stat(path); { case err == nil: @@ -620,6 +639,11 @@ func (fs *Datastore) GetSize(key datastore.Key) (size int, err error) { // the Put() explanation about the handling of concurrent write // operations to the same key. func (fs *Datastore) Delete(key datastore.Key) error { + // Can't exist in datastore. + if !keyIsValid(key) { + return nil + } + fs.shutdownLock.RLock() defer fs.shutdownLock.RUnlock() if fs.shutdown { @@ -1113,12 +1137,17 @@ func (fs *Datastore) Batch() (datastore.Batch, error) { } func (bt *flatfsBatch) Put(key datastore.Key, val []byte) error { + if !keyIsValid(key) { + return fmt.Errorf("key not supported by flatfs: '%q'", key) + } bt.puts[key] = val return nil } func (bt *flatfsBatch) Delete(key datastore.Key) error { - bt.deletes[key] = struct{}{} + if keyIsValid(key) { + bt.deletes[key] = struct{}{} + } // otherwise, delete is a no-op anyways. return nil } diff --git a/flatfs_test.go b/flatfs_test.go index a55b92f..8cf2743 100644 --- a/flatfs_test.go +++ b/flatfs_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/ipfs/go-datastore" + "github.com/ipfs/go-datastore/mount" "github.com/ipfs/go-datastore/query" dstest "github.com/ipfs/go-datastore/test" @@ -54,10 +55,15 @@ func testPut(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - err = fs.Put(datastore.NewKey("quux"), []byte("foobar")) + err = fs.Put(datastore.NewKey("QUUX"), []byte("foobar")) if err != nil { t.Fatalf("Put fail: %v\n", err) } + + err = fs.Put(datastore.NewKey("foo"), []byte("nonono")) + if err == nil { + t.Fatalf("did not expect to put a lowercase key") + } } func TestPut(t *testing.T) { tryAllShardFuncs(t, testPut) } @@ -73,18 +79,23 @@ func testGet(dirFunc mkShardFunc, t *testing.T) { defer fs.Close() const input = "foobar" - err = fs.Put(datastore.NewKey("quux"), []byte(input)) + err = fs.Put(datastore.NewKey("QUUX"), []byte(input)) if err != nil { t.Fatalf("Put fail: %v\n", err) } - buf, err := fs.Get(datastore.NewKey("quux")) + buf, err := fs.Get(datastore.NewKey("QUUX")) if err != nil { t.Fatalf("Get failed: %v", err) } if g, e := string(buf), input; g != e { t.Fatalf("Get gave wrong content: %q != %q", g, e) } + + _, err = fs.Get(datastore.NewKey("/FOO/BAR")) + if err != datastore.ErrNotFound { + t.Fatalf("expected ErrNotFound, got %s", err) + } } func TestGet(t *testing.T) { tryAllShardFuncs(t, testGet) } @@ -103,17 +114,17 @@ func testPutOverwrite(dirFunc mkShardFunc, t *testing.T) { loser = "foobar" winner = "xyzzy" ) - err = fs.Put(datastore.NewKey("quux"), []byte(loser)) + err = fs.Put(datastore.NewKey("QUUX"), []byte(loser)) if err != nil { t.Fatalf("Put fail: %v\n", err) } - err = fs.Put(datastore.NewKey("quux"), []byte(winner)) + err = fs.Put(datastore.NewKey("QUUX"), []byte(winner)) if err != nil { t.Fatalf("Put fail: %v\n", err) } - data, err := fs.Get(datastore.NewKey("quux")) + data, err := fs.Get(datastore.NewKey("QUUX")) if err != nil { t.Fatalf("Get failed: %v", err) } @@ -134,7 +145,7 @@ func testGetNotFoundError(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - _, err = fs.Get(datastore.NewKey("quux")) + _, err = fs.Get(datastore.NewKey("QUUX")) if g, e := err, datastore.ErrNotFound; g != e { t.Fatalf("expected ErrNotFound, got: %v\n", g) } @@ -222,22 +233,22 @@ func TestStorage(t *testing.T) { t.Run("prefix", func(t *testing.T) { testStorage(¶ms{ shard: flatfs.Prefix(2), - dir: "qu", - key: "quux", + dir: "QU", + key: "QUUX", }, t) }) t.Run("suffix", func(t *testing.T) { testStorage(¶ms{ shard: flatfs.Suffix(2), - dir: "ux", - key: "quux", + dir: "UX", + key: "QUUX", }, t) }) t.Run("next-to-last", func(t *testing.T) { testStorage(¶ms{ shard: flatfs.NextToLast(2), - dir: "uu", - key: "quux", + dir: "UU", + key: "QUUX", }, t) }) } @@ -252,7 +263,7 @@ func testHasNotFound(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - found, err := fs.Has(datastore.NewKey("quux")) + found, err := fs.Has(datastore.NewKey("QUUX")) if err != nil { t.Fatalf("Has fail: %v\n", err) } @@ -273,12 +284,12 @@ func testHasFound(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - err = fs.Put(datastore.NewKey("quux"), []byte("foobar")) + err = fs.Put(datastore.NewKey("QUUX"), []byte("foobar")) if err != nil { t.Fatalf("Put fail: %v\n", err) } - found, err := fs.Has(datastore.NewKey("quux")) + found, err := fs.Has(datastore.NewKey("QUUX")) if err != nil { t.Fatalf("Has fail: %v\n", err) } @@ -299,7 +310,7 @@ func testGetSizeFound(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - _, err = fs.GetSize(datastore.NewKey("quux")) + _, err = fs.GetSize(datastore.NewKey("QUUX")) if err != datastore.ErrNotFound { t.Fatalf("GetSize should have returned ErrNotFound, got: %v\n", err) } @@ -317,12 +328,12 @@ func testGetSizeNotFound(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - err = fs.Put(datastore.NewKey("quux"), []byte("foobar")) + err = fs.Put(datastore.NewKey("QUUX"), []byte("foobar")) if err != nil { t.Fatalf("Put fail: %v\n", err) } - size, err := fs.GetSize(datastore.NewKey("quux")) + size, err := fs.GetSize(datastore.NewKey("QUUX")) if err != nil { t.Fatalf("GetSize failed with: %v\n", err) } @@ -343,7 +354,7 @@ func testDeleteNotFound(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - err = fs.Delete(datastore.NewKey("quux")) + err = fs.Delete(datastore.NewKey("QUUX")) if err != nil { t.Fatalf("expected nil, got: %v\n", err) } @@ -361,18 +372,18 @@ func testDeleteFound(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - err = fs.Put(datastore.NewKey("quux"), []byte("foobar")) + err = fs.Put(datastore.NewKey("QUUX"), []byte("foobar")) if err != nil { t.Fatalf("Put fail: %v\n", err) } - err = fs.Delete(datastore.NewKey("quux")) + err = fs.Delete(datastore.NewKey("QUUX")) if err != nil { t.Fatalf("Delete fail: %v\n", err) } // check that it's gone - _, err = fs.Get(datastore.NewKey("quux")) + _, err = fs.Get(datastore.NewKey("QUUX")) if g, e := err, datastore.ErrNotFound; g != e { t.Fatalf("expected Get after Delete to give ErrNotFound, got: %v\n", g) } @@ -390,7 +401,7 @@ func testQuerySimple(dirFunc mkShardFunc, t *testing.T) { } defer fs.Close() - const myKey = "quux" + const myKey = "QUUX" err = fs.Put(datastore.NewKey(myKey), []byte("foobar")) if err != nil { t.Fatalf("Put fail: %v\n", err) @@ -439,7 +450,7 @@ func testDiskUsage(dirFunc mkShardFunc, t *testing.T) { count := 200 for i := 0; i < count; i++ { - k := datastore.NewKey(fmt.Sprintf("test-%d", i)) + k := datastore.NewKey(fmt.Sprintf("TEST-%d", i)) v := []byte("10bytes---") err = fs.Put(k, v) if err != nil { @@ -455,7 +466,7 @@ func testDiskUsage(dirFunc mkShardFunc, t *testing.T) { t.Log("duPostPut:", duElems) for i := 0; i < count; i++ { - k := datastore.NewKey(fmt.Sprintf("test-%d", i)) + k := datastore.NewKey(fmt.Sprintf("TEST-%d", i)) err = fs.Delete(k) if err != nil { t.Fatalf("Delete fail: %v\n", err) @@ -553,7 +564,7 @@ func testDiskUsageDoubleCount(dirFunc mkShardFunc, t *testing.T) { var count int var wg sync.WaitGroup - testKey := datastore.NewKey("test") + testKey := datastore.NewKey("TEST") put := func() { defer wg.Done() @@ -633,7 +644,7 @@ func testDiskUsageBatch(dirFunc mkShardFunc, t *testing.T) { var wg sync.WaitGroup testKeys := []datastore.Key{} for i := 0; i < count; i++ { - k := datastore.NewKey(fmt.Sprintf("test%d", i)) + k := datastore.NewKey(fmt.Sprintf("TEST%d", i)) testKeys = append(testKeys, k) } @@ -726,7 +737,7 @@ func testDiskUsageEstimation(dirFunc mkShardFunc, t *testing.T) { count := 50000 for i := 0; i < count; i++ { - k := datastore.NewKey(fmt.Sprintf("%d-test-%d", i, i)) + k := datastore.NewKey(fmt.Sprintf("%d-TEST-%d", i, i)) v := make([]byte, 1000) err = fs.Put(k, v) if err != nil { @@ -837,14 +848,14 @@ func testClose(dirFunc mkShardFunc, t *testing.T) { t.Fatalf("New fail: %v\n", err) } - err = fs.Put(datastore.NewKey("quux"), []byte("foobar")) + err = fs.Put(datastore.NewKey("QUUX"), []byte("foobar")) if err != nil { t.Fatalf("Put fail: %v\n", err) } fs.Close() - err = fs.Put(datastore.NewKey("qaax"), []byte("foobar")) + err = fs.Put(datastore.NewKey("QAAX"), []byte("foobar")) if err == nil { t.Fatal("expected put on closed datastore to fail") } @@ -1065,3 +1076,29 @@ func TestQueryLeak(t *testing.T) { t.Errorf("leaked %d goroutines", after-before) } } + +func TestSuite(t *testing.T) { + temp, cleanup := tempdir(t) + defer cleanup() + + fs, err := flatfs.CreateOrOpen(temp, flatfs.Prefix(2), false) + if err != nil { + t.Fatalf("New fail: %v\n", err) + } + + ds := mount.New([]mount.Mount{{ + Prefix: datastore.RawKey("/"), + Datastore: datastore.NewMapDatastore(), + }, { + Prefix: datastore.RawKey("/capital"), + Datastore: fs, + }}) + defer func() { + err := ds.Close() + if err != nil { + t.Error(err) + } + }() + + dstest.SubtestAll(t, ds) +} diff --git a/go.mod b/go.mod index 6270dad..8927351 100644 --- a/go.mod +++ b/go.mod @@ -1,9 +1,9 @@ module github.com/ipfs/go-ds-flatfs require ( - github.com/ipfs/go-datastore v0.4.0 + github.com/ipfs/go-datastore v0.4.1 github.com/ipfs/go-log v0.0.1 github.com/jbenet/goprocess v0.1.3 ) -go 1.12 +go 1.13 diff --git a/go.sum b/go.sum index e4ce0ca..2f0cdb1 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,9 @@ github.com/gogo/protobuf v1.2.1 h1:/s5zKNz0uPFCZ5hddgPdo2TK2TVrUNMn0OOX8/aZMTE= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/ipfs/go-datastore v0.4.0 h1:Kiep/Ll245udr3DEnWBG0c5oofT6Dsin7fubWtFdmsc= github.com/ipfs/go-datastore v0.4.0/go.mod h1:SX/xMIKoCszPqp+z9JhPYCmoOoXTvaa13XEbGtsFUhA= +github.com/ipfs/go-datastore v0.4.1 h1:W4ZfzyhNi3xmuU5dQhjfuRn/wFuqEE1KnOmmQiOevEY= +github.com/ipfs/go-datastore v0.4.1/go.mod h1:SX/xMIKoCszPqp+z9JhPYCmoOoXTvaa13XEbGtsFUhA= github.com/ipfs/go-ipfs-delay v0.0.0-20181109222059-70721b86a9a8/go.mod h1:8SP1YXK1M1kXuc4KJZINY3TQQ03J2rwBG9QfXmbRPrw= github.com/ipfs/go-log v0.0.1 h1:9XTUN/rW64BCG1YhPK9Hoy3q8nr4gOmHHBpgFdfw6Lc= github.com/ipfs/go-log v0.0.1/go.mod h1:kL1d2/hzSpI0thNYjiKfjanbVNU+IIGA/WnNESY9leM= @@ -38,6 +39,7 @@ golang.org/x/net v0.0.0-20190227160552-c95aed5357e7/go.mod h1:mL1N/T3taQHkDXs73r golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223 h1:DH4skfRX4EBpamg7iV4ZlCpblAHI6s6TDM39bFZumv8= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/key.go b/key.go new file mode 100644 index 0000000..df66adc --- /dev/null +++ b/key.go @@ -0,0 +1,28 @@ +package flatfs + +import ( + "github.com/ipfs/go-datastore" +) + +// keyIsValid returns true if the key is valid for flatfs. +// Allows keys that match [0-9A-Z+-_=]. +func keyIsValid(key datastore.Key) bool { + ks := key.String() + if len(ks) < 2 || ks[0] != '/' { + return false + } + for _, b := range ks[1:] { + if '0' <= b && b <= '9' { + continue + } + if 'A' <= b && b <= 'Z' { + continue + } + switch b { + case '+', '-', '_', '=': + continue + } + return false + } + return true +} diff --git a/key_test.go b/key_test.go new file mode 100644 index 0000000..4ca4563 --- /dev/null +++ b/key_test.go @@ -0,0 +1,36 @@ +package flatfs + +import ( + "testing" + + "github.com/ipfs/go-datastore" +) + +var ( + validKeys = []string{ + "/FOO", + "/1BAR1", + "/=EMACS-IS-KING=", + } + invalidKeys = []string{ + "/foo/bar", + `/foo\bar`, + "/foo\000bar", + "/=Vim-IS-KING=", + } +) + +func TestKeyIsValid(t *testing.T) { + for _, key := range validKeys { + k := datastore.NewKey(key) + if !keyIsValid(k) { + t.Errorf("expected key %s to be valid", k) + } + } + for _, key := range invalidKeys { + k := datastore.NewKey(key) + if keyIsValid(k) { + t.Errorf("expected key %s to be invalid", k) + } + } +}