From 4bbaa2aa27b57dff2919cda69819baf49b1c2854 Mon Sep 17 00:00:00 2001 From: matrushka Date: Wed, 14 Feb 2018 12:31:57 +0100 Subject: [PATCH 1/4] Modified keystore to ignore invalid key files inside the keystore directory. * Has calls the validateName function before checking if we have the file * List filters the returned list of file names by validateName. License: MIT Signed-off-by: matrushka --- keystore/keystore.go | 20 +++++++++++++- keystore/keystore_test.go | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/keystore/keystore.go b/keystore/keystore.go index 56dfd1b0137..2db9a9b920c 100644 --- a/keystore/keystore.go +++ b/keystore/keystore.go @@ -77,6 +77,10 @@ func (ks *FSKeystore) Has(name string) (bool, error) { return false, err } + if err := validateName(name); err != nil { + return false, err + } + return true, nil } @@ -149,5 +153,19 @@ func (ks *FSKeystore) List() ([]string, error) { return nil, err } - return dir.Readdirnames(0) + dirs, err := dir.Readdirnames(0) + if err != nil { + return nil, err + } + + var list []string + + for _, name := range dirs { + err := validateName(name) + if err == nil { + list = append(list, name) + } + } + + return list, err } diff --git a/keystore/keystore_test.go b/keystore/keystore_test.go index cf6281be20f..f0c1b310544 100644 --- a/keystore/keystore_test.go +++ b/keystore/keystore_test.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "math/rand" + "path/filepath" "sort" "testing" @@ -143,6 +144,62 @@ func TestKeystoreBasics(t *testing.T) { } } +func TestInvalidKeyFiles(t *testing.T) { + tdir, err := ioutil.TempDir("", "keystore-test") + + if err != nil { + t.Fatal(err) + } + + ks, err := NewFSKeystore(tdir) + if err != nil { + t.Fatal(err) + } + + key := privKeyOrFatal(t) + + bytes, err := key.Bytes() + if err != nil { + t.Fatal(err) + } + + err = ioutil.WriteFile(filepath.Join(ks.dir, "valid"), bytes, 0644) + if err != nil { + t.Fatal(err) + } + + err = ioutil.WriteFile(filepath.Join(ks.dir, ".invalid"), bytes, 0644) + if err != nil { + t.Fatal(err) + } + + l, err := ks.List() + if err != nil { + t.Fatal(err) + } + + sort.Strings(l) + if len(l) != 1 { + t.Fatal("wrong entry count") + } + + if l[0] != "valid" { + t.Fatal("wrong entries listed") + } + + exist, err := ks.Has("valid") + if !exist { + t.Fatal("should know it has a key named valid") + } + if err != nil { + t.Fatal(err) + } + + if exist, err = ks.Has(".invalid"); err == nil { + t.Fatal("shouldnt be able to put a key with a 'hidden' name") + } +} + func TestNonExistingKey(t *testing.T) { tdir, err := ioutil.TempDir("", "keystore-test") if err != nil { From 23e1179d34feb0153ac2f375022d6517379c1a64 Mon Sep 17 00:00:00 2001 From: matrushka Date: Wed, 14 Feb 2018 15:15:25 +0100 Subject: [PATCH 2/4] Removing the tmp directory after the TestInvalidKeyFiles test. License: MIT Signed-off-by: matrushka --- keystore/keystore_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/keystore/keystore_test.go b/keystore/keystore_test.go index f0c1b310544..0731f252bb5 100644 --- a/keystore/keystore_test.go +++ b/keystore/keystore_test.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "math/rand" + "os" "path/filepath" "sort" "testing" @@ -151,6 +152,8 @@ func TestInvalidKeyFiles(t *testing.T) { t.Fatal(err) } + defer os.RemoveAll(tdir) + ks, err := NewFSKeystore(tdir) if err != nil { t.Fatal(err) From 40aeea6abd54ddf9a58238d62e70162fb73164f8 Mon Sep 17 00:00:00 2001 From: matrushka Date: Wed, 14 Feb 2018 18:16:29 +0100 Subject: [PATCH 3/4] Added logging for ignored keyfiles in keystore.List and minor improvements. License: MIT Signed-off-by: matrushka --- keystore/keystore.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/keystore/keystore.go b/keystore/keystore.go index 2db9a9b920c..1f95d2e4f5d 100644 --- a/keystore/keystore.go +++ b/keystore/keystore.go @@ -7,9 +7,12 @@ import ( "path/filepath" "strings" + logging "gx/ipfs/QmRb5jh8z2E8hMGN2tkvs1yHynUanqnZ3UeKwgN1i9P1F8/go-log" ci "gx/ipfs/QmaPbCnUMBohSGo3KnxEa2bHqyJVVeEEcwtqJAYxerieBo/go-libp2p-crypto" ) +var log = logging.Logger("keystore") + // Keystore provides a key management interface type Keystore interface { // Has returns whether or not a key exist in the Keystore @@ -158,14 +161,16 @@ func (ks *FSKeystore) List() ([]string, error) { return nil, err } - var list []string + list := make([]string, 0) for _, name := range dirs { err := validateName(name) if err == nil { list = append(list, name) + } else { + log.Warningf("Ignoring the invalid keyfile: %s", name) } } - return list, err + return list, nil } From e338cdf5beb18de24ceb51804fdcd4335d571462 Mon Sep 17 00:00:00 2001 From: matrushka Date: Wed, 14 Feb 2018 19:40:05 +0100 Subject: [PATCH 4/4] Handling requested changes. License: MIT Signed-off-by: matrushka --- keystore/keystore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keystore/keystore.go b/keystore/keystore.go index 1f95d2e4f5d..5dd43385280 100644 --- a/keystore/keystore.go +++ b/keystore/keystore.go @@ -161,7 +161,7 @@ func (ks *FSKeystore) List() ([]string, error) { return nil, err } - list := make([]string, 0) + list := make([]string, 0, len(dirs)) for _, name := range dirs { err := validateName(name)