Merge pull request #6955 from ipfs/feat/AuHau-encoding-key-names

Introducing EncodedFSKeystore with base32 encoding (#5947)

.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:

keystore/keystore.go

@@ -7,12 +7,16 @@ import (
 	"path/filepath"
 	"strings"
 
+	base32 "encoding/base32"
+
 	logging "github.com/ipfs/go-log"
 	ci "github.com/libp2p/go-libp2p-core/crypto"
 )
 
 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
@@ -28,30 +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
-}
-
+// NewFSKeystore returns a new filesystem-backed keystore.
 func NewFSKeystore(dir string) (*FSKeystore, error) {
 	_, err := os.Stat(dir)
 	if err != nil {
@@ -68,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
 	}
 
@@ -121,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
 	}
 
@@ -140,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
 	}
 
@@ -164,13 +157,40 @@ 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
 }
+
+func encode(name string) (string, error) {
+	if name == "" {
+		return "", fmt.Errorf("key name must be at least one character")
+	}
+
+	encodedName := codec.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):])
+	decodedName, err := codec.DecodeString(nameWithoutPrefix)
+	if err != nil {
+		return "", err
+	}
+
+	log.Debugf("Decoded key name: %s to: %s", name, decodedName)
+
+	return string(decodedName), nil
+}

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, ".invalid"), bytes, 0644)
+	err = ioutil.WriteFile(filepath.Join(ks.dir, encodedName), bytes, 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	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)

keystore/memkeystore.go

@@ -1,13 +1,18 @@
 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.
 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
 }

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)
 	}
 }

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 = 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.`