From d67a4e06cf65405489567701ef5fbda1fbbb48f2 Mon Sep 17 00:00:00 2001 From: Adam Uhlir Date: Thu, 21 Feb 2019 11:53:44 -0800 Subject: [PATCH 1/3] Introducing EncodedFSKeystore with base32 encoding (#5947) Encoding the key's filename with base32 introduces coherent behaviour across different platforms and their case-sensitive/case-insensitive file-systems. Moreover it allows wider character set to be used for the name of the keys as the original restriction for special FS's characters (e.g. '/', '.') will not apply. License: MIT Signed-off-by: Adam Uhlir --- keystore/keystore.go | 120 ++++++++++++++++++++++++++++++++++++++ keystore/keystore_test.go | 89 ++++++++++++++++++++++++++++ repo/fsrepo/fsrepo.go | 4 +- 3 files changed, 211 insertions(+), 2 deletions(-) diff --git a/keystore/keystore.go b/keystore/keystore.go index 991de5dd1d2..a40ab5cb060 100644 --- a/keystore/keystore.go +++ b/keystore/keystore.go @@ -9,6 +9,7 @@ import ( logging "github.com/ipfs/go-log" ci "github.com/libp2p/go-libp2p-core/crypto" + base32 "github.com/whyrusleeping/base32" ) var log = logging.Logger("keystore") @@ -52,6 +53,22 @@ func validateName(name string) error { return nil } +// NewKeystore is a factory for getting instance of Keystore interface implementation +func NewKeystore(dir string) (Keystore, error) { + return NewEncodedFSKeystore(dir) +} + +// NewEncodedFSKeystore is a factory for getting instance of EncodedFSKeystore +func NewEncodedFSKeystore(dir string) (*EncodedFSKeystore, error) { + keystore, err := NewFSKeystore(dir) + + if err != nil { + return nil, err + } + + return &EncodedFSKeystore{keystore}, nil +} + func NewFSKeystore(dir string) (*FSKeystore, error) { _, err := os.Stat(dir) if err != nil { @@ -174,3 +191,106 @@ func (ks *FSKeystore) List() ([]string, error) { return list, nil } + +const keyFilenamePrefix = "key_" + +func encode(name string) (string, error) { + if name == "" { + return "", fmt.Errorf("key name must be at least one character") + } + + encodedName := base32.RawStdEncoding.EncodeToString([]byte(name)) + log.Debugf("Encoded key name: %s to: %s", name, encodedName) + + return keyFilenamePrefix + strings.ToLower(encodedName), nil +} + +func decode(name string) (string, error) { + if !strings.HasPrefix(name, keyFilenamePrefix) { + return "", fmt.Errorf("key's filename has unexpected format") + } + + nameWithoutPrefix := strings.ToUpper(name[len(keyFilenamePrefix):]) + data, err := base32.RawStdEncoding.DecodeString(nameWithoutPrefix) + + if err != nil { + return "", err + } + + decodedName := string(data[:]) + + log.Debugf("Decoded key name: %s to: %s", name, decodedName) + + return decodedName, nil +} + +// EncodedFSKeystore is extension of FSKeystore that encodes the key filenames in base32 +type EncodedFSKeystore struct { + *FSKeystore +} + +// Has indicates if key is in keystore +func (ks *EncodedFSKeystore) Has(name string) (bool, error) { + encodedName, err := encode(name) + + if err != nil { + return false, err + } + + return ks.FSKeystore.Has(encodedName) +} + +// Put places key into the keystore +func (ks *EncodedFSKeystore) Put(name string, k ci.PrivKey) error { + encodedName, err := encode(name) + + if err != nil { + return err + } + + return ks.FSKeystore.Put(encodedName, k) +} + +// Get retrieves key by its name from the keystore +func (ks *EncodedFSKeystore) Get(name string) (ci.PrivKey, error) { + encodedName, err := encode(name) + + if err != nil { + return nil, err + } + + return ks.FSKeystore.Get(encodedName) +} + +// Delete removes key from the keystore +func (ks *EncodedFSKeystore) Delete(name string) error { + encodedName, err := encode(name) + + if err != nil { + return err + } + + return ks.FSKeystore.Delete(encodedName) +} + +// List returns list of all keys in keystore +func (ks *EncodedFSKeystore) List() ([]string, error) { + dirs, err := ks.FSKeystore.List() + + if err != nil { + return nil, err + } + + list := make([]string, 0, len(dirs)) + + for _, name := range dirs { + decodedName, err := decode(name) + if err == nil { + list = append(list, decodedName) + } else { + log.Warningf("Ignoring keyfile with invalid encoded filename: %s", name) + } + } + + return list, nil +} diff --git a/keystore/keystore_test.go b/keystore/keystore_test.go index 37f59ebffff..685e5d942ef 100644 --- a/keystore/keystore_test.go +++ b/keystore/keystore_test.go @@ -271,3 +271,92 @@ func assertDirContents(dir string, exp []string) error { } return nil } + +func TestEncodedKeystoreBasics(t *testing.T) { + tdir, err := ioutil.TempDir("", "encoded-keystore-test") + if err != nil { + t.Fatal(err) + } + + ks, err := NewEncodedFSKeystore(tdir) + if err != nil { + t.Fatal(err) + } + + l, err := ks.List() + if err != nil { + t.Fatal(err) + } + + if len(l) != 0 { + t.Fatal("expected no keys") + } + + k1 := privKeyOrFatal(t) + k1Name, err := encode("foo") + if err != nil { + t.Fatal(err) + } + + k2 := privKeyOrFatal(t) + k2Name, err := encode("bar") + if err != nil { + t.Fatal(err) + } + + err = ks.Put("foo", k1) + if err != nil { + t.Fatal(err) + } + + err = ks.Put("bar", k2) + if err != nil { + t.Fatal(err) + } + + l, err = ks.List() + if err != nil { + t.Fatal(err) + } + + sort.Strings(l) + if l[0] != "bar" || l[1] != "foo" { + t.Fatal("wrong entries listed") + } + + if err := assertDirContents(tdir, []string{k1Name, k2Name}); err != nil { + t.Fatal(err) + } + + exist, err := ks.Has("foo") + if !exist { + t.Fatal("should know it has a key named foo") + } + if err != nil { + t.Fatal(err) + } + + if err := ks.Delete("bar"); err != nil { + t.Fatal(err) + } + + if err := assertDirContents(tdir, []string{k1Name}); err != nil { + t.Fatal(err) + } + + if err := assertGetKey(ks, "foo", k1); err != nil { + t.Fatal(err) + } + + if err := ks.Put("..///foo/", k1); err != nil { + t.Fatal(err) + } + + if err := ks.Put("", k1); err == nil { + t.Fatal("shouldnt be able to put a key with no name") + } + + if err := ks.Put(".foo", k1); err != nil { + t.Fatal(err) + } +} diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index 6ee436605cc..59728dc466d 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -36,7 +36,7 @@ const LockFile = "repo.lock" var log = logging.Logger("fsrepo") // version number that we are currently expecting to see -var RepoVersion = 7 +var RepoVersion = 8 var migrationInstructions = `See https://github.com/ipfs/fs-repo-migrations/blob/master/run.md Sorry for the inconvenience. In the future, these will run automatically.` @@ -385,7 +385,7 @@ func (r *FSRepo) openConfig() error { func (r *FSRepo) openKeystore() error { ksp := filepath.Join(r.path, "keystore") - ks, err := keystore.NewFSKeystore(ksp) + ks, err := keystore.NewKeystore(ksp) if err != nil { return err } From 059f396baca17d96b69441d3788e913cf334a36d Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Thu, 5 Mar 2020 14:28:02 +0100 Subject: [PATCH 2/3] keystore: finish addressing encodedFSKeystore * Use Go's base32 library * Set repo to version 9 * Resolve linting problems and docs. * Merge EncodedFSKeystore into FSKeystore * Remove name limitations and adjust tests --- keystore/keystore.go | 158 +++++++---------------------------- keystore/keystore_test.go | 120 ++++---------------------- keystore/memkeystore.go | 19 ++--- keystore/memkeystore_test.go | 8 +- repo/fsrepo/fsrepo.go | 4 +- 5 files changed, 61 insertions(+), 248 deletions(-) diff --git a/keystore/keystore.go b/keystore/keystore.go index a40ab5cb060..463f90e007e 100644 --- a/keystore/keystore.go +++ b/keystore/keystore.go @@ -7,13 +7,16 @@ import ( "path/filepath" "strings" + base32 "encoding/base32" + logging "github.com/ipfs/go-log" ci "github.com/libp2p/go-libp2p-core/crypto" - base32 "github.com/whyrusleeping/base32" ) var log = logging.Logger("keystore") +var codec = base32.StdEncoding.WithPadding(base32.NoPadding) + // Keystore provides a key management interface type Keystore interface { // Has returns whether or not a key exist in the Keystore @@ -29,46 +32,20 @@ type Keystore interface { List() ([]string, error) } +// ErrNoSuchKey is an error message returned when no key of a given name was found. var ErrNoSuchKey = fmt.Errorf("no key by the given name was found") + +// ErrKeyExists is an error message returned when a key already exists var ErrKeyExists = fmt.Errorf("key by that name already exists, refusing to overwrite") +const keyFilenamePrefix = "key_" + // FSKeystore is a keystore backed by files in a given directory stored on disk. type FSKeystore struct { dir string } -func validateName(name string) error { - if name == "" { - return fmt.Errorf("key names must be at least one character") - } - - if strings.Contains(name, "/") { - return fmt.Errorf("key names may not contain slashes") - } - - if strings.HasPrefix(name, ".") { - return fmt.Errorf("key names may not begin with a period") - } - - return nil -} - -// NewKeystore is a factory for getting instance of Keystore interface implementation -func NewKeystore(dir string) (Keystore, error) { - return NewEncodedFSKeystore(dir) -} - -// NewEncodedFSKeystore is a factory for getting instance of EncodedFSKeystore -func NewEncodedFSKeystore(dir string) (*EncodedFSKeystore, error) { - keystore, err := NewFSKeystore(dir) - - if err != nil { - return nil, err - } - - return &EncodedFSKeystore{keystore}, nil -} - +// NewFSKeystore returns a new filesystem-backed keystore. func NewFSKeystore(dir string) (*FSKeystore, error) { _, err := os.Stat(dir) if err != nil { @@ -85,28 +62,25 @@ func NewFSKeystore(dir string) (*FSKeystore, error) { // Has returns whether or not a key exist in the Keystore func (ks *FSKeystore) Has(name string) (bool, error) { + name, err := encode(name) + if err != nil { + return false, err + } + kp := filepath.Join(ks.dir, name) - _, err := os.Stat(kp) + _, err = os.Stat(kp) if os.IsNotExist(err) { return false, nil } - - if err != nil { - return false, err - } - - if err := validateName(name); err != nil { - return false, err - } - - return true, nil + return err == nil, err } // Put stores a key in the Keystore, if a key with the same name already exists, returns ErrKeyExists func (ks *FSKeystore) Put(name string, k ci.PrivKey) error { - if err := validateName(name); err != nil { + name, err := encode(name) + if err != nil { return err } @@ -138,7 +112,8 @@ func (ks *FSKeystore) Put(name string, k ci.PrivKey) error { // Get retrieves a key from the Keystore if it exists, and returns ErrNoSuchKey // otherwise. func (ks *FSKeystore) Get(name string) (ci.PrivKey, error) { - if err := validateName(name); err != nil { + name, err := encode(name) + if err != nil { return nil, err } @@ -157,7 +132,8 @@ func (ks *FSKeystore) Get(name string) (ci.PrivKey, error) { // Delete removes a key from the Keystore func (ks *FSKeystore) Delete(name string) error { - if err := validateName(name); err != nil { + name, err := encode(name) + if err != nil { return err } @@ -181,25 +157,23 @@ func (ks *FSKeystore) List() ([]string, error) { list := make([]string, 0, len(dirs)) for _, name := range dirs { - err := validateName(name) + decodedName, err := decode(name) if err == nil { - list = append(list, name) + list = append(list, decodedName) } else { - log.Warnf("Ignoring the invalid keyfile: %s", name) + log.Errorf("Ignoring keyfile with invalid encoded filename: %s", name) } } return list, nil } -const keyFilenamePrefix = "key_" - func encode(name string) (string, error) { if name == "" { return "", fmt.Errorf("key name must be at least one character") } - encodedName := base32.RawStdEncoding.EncodeToString([]byte(name)) + encodedName := codec.EncodeToString([]byte(name)) log.Debugf("Encoded key name: %s to: %s", name, encodedName) return keyFilenamePrefix + strings.ToLower(encodedName), nil @@ -211,86 +185,12 @@ func decode(name string) (string, error) { } nameWithoutPrefix := strings.ToUpper(name[len(keyFilenamePrefix):]) - data, err := base32.RawStdEncoding.DecodeString(nameWithoutPrefix) - + decodedName, err := codec.DecodeString(nameWithoutPrefix) if err != nil { return "", err } - decodedName := string(data[:]) - log.Debugf("Decoded key name: %s to: %s", name, decodedName) - return decodedName, nil -} - -// EncodedFSKeystore is extension of FSKeystore that encodes the key filenames in base32 -type EncodedFSKeystore struct { - *FSKeystore -} - -// Has indicates if key is in keystore -func (ks *EncodedFSKeystore) Has(name string) (bool, error) { - encodedName, err := encode(name) - - if err != nil { - return false, err - } - - return ks.FSKeystore.Has(encodedName) -} - -// Put places key into the keystore -func (ks *EncodedFSKeystore) Put(name string, k ci.PrivKey) error { - encodedName, err := encode(name) - - if err != nil { - return err - } - - return ks.FSKeystore.Put(encodedName, k) -} - -// Get retrieves key by its name from the keystore -func (ks *EncodedFSKeystore) Get(name string) (ci.PrivKey, error) { - encodedName, err := encode(name) - - if err != nil { - return nil, err - } - - return ks.FSKeystore.Get(encodedName) -} - -// Delete removes key from the keystore -func (ks *EncodedFSKeystore) Delete(name string) error { - encodedName, err := encode(name) - - if err != nil { - return err - } - - return ks.FSKeystore.Delete(encodedName) -} - -// List returns list of all keys in keystore -func (ks *EncodedFSKeystore) List() ([]string, error) { - dirs, err := ks.FSKeystore.List() - - if err != nil { - return nil, err - } - - list := make([]string, 0, len(dirs)) - - for _, name := range dirs { - decodedName, err := decode(name) - if err == nil { - list = append(list, decodedName) - } else { - log.Warningf("Ignoring keyfile with invalid encoded filename: %s", name) - } - } - - return list, nil + return string(decodedName), nil } diff --git a/keystore/keystore_test.go b/keystore/keystore_test.go index 685e5d942ef..2a48b43e569 100644 --- a/keystore/keystore_test.go +++ b/keystore/keystore_test.go @@ -132,16 +132,16 @@ func TestKeystoreBasics(t *testing.T) { t.Fatal(err) } - if err := ks.Put("..///foo/", k1); err == nil { - t.Fatal("shouldnt be able to put a poorly named key") + if err := ks.Put("..///foo/", k1); err != nil { + t.Fatal(err) } if err := ks.Put("", k1); err == nil { t.Fatal("shouldnt be able to put a key with no name") } - if err := ks.Put(".foo", k1); err == nil { - t.Fatal("shouldnt be able to put a key with a 'hidden' name") + if err := ks.Put(".foo", k1); err != nil { + t.Fatal(err) } } @@ -166,12 +166,17 @@ func TestInvalidKeyFiles(t *testing.T) { t.Fatal(err) } - err = ioutil.WriteFile(filepath.Join(ks.dir, "valid"), bytes, 0644) + encodedName, err := encode("valid") + if err != nil { + t.Fatal(err) + } + + err = ioutil.WriteFile(filepath.Join(ks.dir, encodedName), bytes, 0644) if err != nil { t.Fatal(err) } - err = ioutil.WriteFile(filepath.Join(ks.dir, ".invalid"), bytes, 0644) + err = ioutil.WriteFile(filepath.Join(ks.dir, "z.invalid"), bytes, 0644) if err != nil { t.Fatal(err) } @@ -197,10 +202,6 @@ func TestInvalidKeyFiles(t *testing.T) { if err != nil { t.Fatal(err) } - - if _, err = ks.Has(".invalid"); err == nil { - t.Fatal("shouldnt be able to put a key with a 'hidden' name") - } } func TestNonExistingKey(t *testing.T) { @@ -231,12 +232,12 @@ func TestMakeKeystoreNoDir(t *testing.T) { } func assertGetKey(ks Keystore, name string, exp ci.PrivKey) error { - out_k, err := ks.Get(name) + outK, err := ks.Get(name) if err != nil { return err } - if !out_k.Equals(exp) { + if !outK.Equals(exp) { return fmt.Errorf("key we got out didnt match expectation") } @@ -255,7 +256,11 @@ func assertDirContents(dir string, exp []string) error { var names []string for _, fi := range finfos { - names = append(names, fi.Name()) + decodedName, err := decode(fi.Name()) + if err != nil { + return err + } + names = append(names, decodedName) } sort.Strings(names) @@ -271,92 +276,3 @@ func assertDirContents(dir string, exp []string) error { } return nil } - -func TestEncodedKeystoreBasics(t *testing.T) { - tdir, err := ioutil.TempDir("", "encoded-keystore-test") - if err != nil { - t.Fatal(err) - } - - ks, err := NewEncodedFSKeystore(tdir) - if err != nil { - t.Fatal(err) - } - - l, err := ks.List() - if err != nil { - t.Fatal(err) - } - - if len(l) != 0 { - t.Fatal("expected no keys") - } - - k1 := privKeyOrFatal(t) - k1Name, err := encode("foo") - if err != nil { - t.Fatal(err) - } - - k2 := privKeyOrFatal(t) - k2Name, err := encode("bar") - if err != nil { - t.Fatal(err) - } - - err = ks.Put("foo", k1) - if err != nil { - t.Fatal(err) - } - - err = ks.Put("bar", k2) - if err != nil { - t.Fatal(err) - } - - l, err = ks.List() - if err != nil { - t.Fatal(err) - } - - sort.Strings(l) - if l[0] != "bar" || l[1] != "foo" { - t.Fatal("wrong entries listed") - } - - if err := assertDirContents(tdir, []string{k1Name, k2Name}); err != nil { - t.Fatal(err) - } - - exist, err := ks.Has("foo") - if !exist { - t.Fatal("should know it has a key named foo") - } - if err != nil { - t.Fatal(err) - } - - if err := ks.Delete("bar"); err != nil { - t.Fatal(err) - } - - if err := assertDirContents(tdir, []string{k1Name}); err != nil { - t.Fatal(err) - } - - if err := assertGetKey(ks, "foo", k1); err != nil { - t.Fatal(err) - } - - if err := ks.Put("..///foo/", k1); err != nil { - t.Fatal(err) - } - - if err := ks.Put("", k1); err == nil { - t.Fatal("shouldnt be able to put a key with no name") - } - - if err := ks.Put(".foo", k1); err != nil { - t.Fatal(err) - } -} diff --git a/keystore/memkeystore.go b/keystore/memkeystore.go index 4067bbce29a..c96985252ac 100644 --- a/keystore/memkeystore.go +++ b/keystore/memkeystore.go @@ -1,6 +1,10 @@ package keystore -import ci "github.com/libp2p/go-libp2p-core/crypto" +import ( + "errors" + + ci "github.com/libp2p/go-libp2p-core/crypto" +) // MemKeystore is an in memory keystore implementation that is not persisted to // any backing storage. @@ -8,6 +12,7 @@ type MemKeystore struct { keys map[string]ci.PrivKey } +// NewMemKeystore creates a MemKeystore. func NewMemKeystore() *MemKeystore { return &MemKeystore{make(map[string]ci.PrivKey)} } @@ -20,8 +25,8 @@ func (mk *MemKeystore) Has(name string) (bool, error) { // Put store a key in the Keystore func (mk *MemKeystore) Put(name string, k ci.PrivKey) error { - if err := validateName(name); err != nil { - return err + if name == "" { + return errors.New("key name must be at least one character") } _, ok := mk.keys[name] @@ -35,10 +40,6 @@ func (mk *MemKeystore) Put(name string, k ci.PrivKey) error { // Get retrieve a key from the Keystore func (mk *MemKeystore) Get(name string) (ci.PrivKey, error) { - if err := validateName(name); err != nil { - return nil, err - } - k, ok := mk.keys[name] if !ok { return nil, ErrNoSuchKey @@ -49,10 +50,6 @@ func (mk *MemKeystore) Get(name string) (ci.PrivKey, error) { // Delete remove a key from the Keystore func (mk *MemKeystore) Delete(name string) error { - if err := validateName(name); err != nil { - return err - } - delete(mk.keys, name) return nil } diff --git a/keystore/memkeystore_test.go b/keystore/memkeystore_test.go index 62533d54b92..a7214893a3f 100644 --- a/keystore/memkeystore_test.go +++ b/keystore/memkeystore_test.go @@ -85,15 +85,15 @@ func TestMemKeyStoreBasics(t *testing.T) { t.Fatal(err) } - if err := ks.Put("..///foo/", k1); err == nil { - t.Fatal("shouldnt be able to put a poorly named key") + if err := ks.Put("..///foo/", k1); err != nil { + t.Fatal(err) } if err := ks.Put("", k1); err == nil { t.Fatal("shouldnt be able to put a key with no name") } - if err := ks.Put(".foo", k1); err == nil { - t.Fatal("shouldnt be able to put a key with a 'hidden' name") + if err := ks.Put(".foo", k1); err != nil { + t.Fatal(err) } } diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index 59728dc466d..a14e96ff25c 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -36,7 +36,7 @@ const LockFile = "repo.lock" var log = logging.Logger("fsrepo") // version number that we are currently expecting to see -var RepoVersion = 8 +var RepoVersion = 9 var migrationInstructions = `See https://github.com/ipfs/fs-repo-migrations/blob/master/run.md Sorry for the inconvenience. In the future, these will run automatically.` @@ -385,7 +385,7 @@ func (r *FSRepo) openConfig() error { func (r *FSRepo) openKeystore() error { ksp := filepath.Join(r.path, "keystore") - ks, err := keystore.NewKeystore(ksp) + ks, err := keystore.NewFSKeystore(ksp) if err != nil { return err } From 0d9d6e94952f3393f2a5f08e9431413e70e87d85 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 23 Mar 2020 17:40:28 -0700 Subject: [PATCH 3/3] ci: checkout working interop commit --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index bc30f97f8c2..368359faac8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -174,6 +174,7 @@ jobs: name: Cloning command: | git clone https://github.com/ipfs/interop.git + git -C interop checkout "fix/disable-repo-interop-test" git -C interop log -1 - restore_cache: keys: