Merge pull request #3969 from MichaelEischer/key-by-id

Port restic.Find to return IDs and identify keys by restic.ID

cmd/restic/cmd_cat.go

@@ -93,7 +93,7 @@ func runCat(ctx context.Context, gopts GlobalOptions, args []string) error {
 		Println(string(buf))
 		return nil
 	case "key":
-		key, err := repository.LoadKey(ctx, repo, id.String())
+		key, err := repository.LoadKey(ctx, repo, id)
 		if err != nil {
 			return err
 		}

cmd/restic/cmd_debug.go

@@ -436,10 +436,7 @@ func runDebugExamine(ctx context.Context, gopts GlobalOptions, args []string) er
 	for _, name := range args {
 		id, err := restic.ParseID(name)
 		if err != nil {
-			name, err = restic.Find(ctx, repo.Backend(), restic.PackFile, name)
+			id, err = restic.Find(ctx, repo.Backend(), restic.PackFile, name)
-			if err == nil {
-				id, err = restic.ParseID(name)
-			}
 			if err != nil {
 				Warnf("error: %v\n", err)
 				continue

cmd/restic/cmd_key.go

@@ -59,14 +59,14 @@ func listKeys(ctx context.Context, s *repository.Repository, gopts GlobalOptions
 	var keys []keyInfo
 
 	err := s.List(ctx, restic.KeyFile, func(id restic.ID, size int64) error {
-		k, err := repository.LoadKey(ctx, s, id.String())
+		k, err := repository.LoadKey(ctx, s, id)
 		if err != nil {
 			Warnf("LoadKey() failed: %v\n", err)
 			return nil
 		}
 
 		key := keyInfo{
-			Current:  id.String() == s.KeyName(),
+			Current:  id == s.KeyID(),
 			ID:       id.Str(),
 			UserName: k.Username,
 			HostName: k.Hostname,
@@ -141,18 +141,18 @@ func addKey(ctx context.Context, repo *repository.Repository, gopts GlobalOption
 	return nil
 }
 
-func deleteKey(ctx context.Context, repo *repository.Repository, name string) error {
+func deleteKey(ctx context.Context, repo *repository.Repository, id restic.ID) error {
-	if name == repo.KeyName() {
+	if id == repo.KeyID() {
 		return errors.Fatal("refusing to remove key currently used to access repository")
 	}
 
-	h := restic.Handle{Type: restic.KeyFile, Name: name}
+	h := restic.Handle{Type: restic.KeyFile, Name: id.String()}
 	err := repo.Backend().Remove(ctx, h)
 	if err != nil {
 		return err
 	}
 
-	Verbosef("removed key %v\n", name)
+	Verbosef("removed key %v\n", id)
 	return nil
 }
 
@@ -166,14 +166,14 @@ func changePassword(ctx context.Context, repo *repository.Repository, gopts Glob
 	if err != nil {
 		return errors.Fatalf("creating new key failed: %v\n", err)
 	}
-	oldID := repo.KeyName()
+	oldID := repo.KeyID()
 
 	err = switchToNewKeyAndRemoveIfBroken(ctx, repo, id, pw)
 	if err != nil {
 		return err
 	}
 
-	h := restic.Handle{Type: restic.KeyFile, Name: oldID}
+	h := restic.Handle{Type: restic.KeyFile, Name: oldID.String()}
 	err = repo.Backend().Remove(ctx, h)
 	if err != nil {
 		return err
@@ -187,10 +187,10 @@ func changePassword(ctx context.Context, repo *repository.Repository, gopts Glob
 func switchToNewKeyAndRemoveIfBroken(ctx context.Context, repo *repository.Repository, key *repository.Key, pw string) error {
 	// Verify new key to make sure it really works. A broken key can render the
 	// whole repository inaccessible
-	err := repo.SearchKey(ctx, pw, 0, key.Name())
+	err := repo.SearchKey(ctx, pw, 0, key.ID().String())
 	if err != nil {
 		// the key is invalid, try to remove it
-		h := restic.Handle{Type: restic.KeyFile, Name: key.Name()}
+		h := restic.Handle{Type: restic.KeyFile, Name: key.ID().String()}
 		_ = repo.Backend().Remove(ctx, h)
 		return errors.Fatalf("failed to access repository with new key: %v", err)
 	}

internal/repository/key.go

@@ -40,7 +40,7 @@ type Key struct {
 	user   *crypto.Key
 	master *crypto.Key
 
-	name string
+	id restic.ID
 }
 
 // Params tracks the parameters used for the KDF. If not set, it will be
@@ -62,10 +62,10 @@ func createMasterKey(ctx context.Context, s *Repository, password string) (*Key,
 }
 
 // OpenKey tries do decrypt the key specified by name with the given password.
-func OpenKey(ctx context.Context, s *Repository, name string, password string) (*Key, error) {
+func OpenKey(ctx context.Context, s *Repository, id restic.ID, password string) (*Key, error) {
-	k, err := LoadKey(ctx, s, name)
+	k, err := LoadKey(ctx, s, id)
 	if err != nil {
-		debug.Log("LoadKey(%v) returned error %v", name, err)
+		debug.Log("LoadKey(%v) returned error %v", id.String(), err)
 		return nil, err
 	}
 
@@ -99,7 +99,7 @@ func OpenKey(ctx context.Context, s *Repository, name string, password string) (
 		debug.Log("Unmarshal() returned error %v", err)
 		return nil, errors.Wrap(err, "Unmarshal")
 	}
-	k.name = name
+	k.id = id
 
 	if !k.Valid() {
 		return nil, errors.New("Invalid key for repository")
@@ -136,22 +136,16 @@ func SearchKey(ctx context.Context, s *Repository, password string, maxKeys int,
 	defer cancel()
 
 	// try at most maxKeys keys in repo
-	err = s.Backend().List(listCtx, restic.KeyFile, func(fi restic.FileInfo) error {
+	err = s.List(listCtx, restic.KeyFile, func(id restic.ID, size int64) error {
 		checked++
 		if maxKeys > 0 && checked > maxKeys {
 			return ErrMaxKeysReached
 		}
 
-		_, err := restic.ParseID(fi.Name)
+		debug.Log("trying key %q", id.String())
+		key, err := OpenKey(ctx, s, id, password)
 		if err != nil {
-			debug.Log("rejecting key with invalid name: %v", fi.Name)
+			debug.Log("key %v returned error %v", id.String(), err)
-			return nil
-		}
-
-		debug.Log("trying key %q", fi.Name)
-		key, err := OpenKey(ctx, s, fi.Name, password)
-		if err != nil {
-			debug.Log("key %v returned error %v", fi.Name, err)
 
 			// ErrUnauthenticated means the password is wrong, try the next key
 			if errors.Is(err, crypto.ErrUnauthenticated) {
@@ -161,7 +155,7 @@ func SearchKey(ctx context.Context, s *Repository, password string, maxKeys int,
 			return err
 		}
 
-		debug.Log("successfully opened key %v", fi.Name)
+		debug.Log("successfully opened key %v", id.String())
 		k = key
 		cancel()
 		return nil
@@ -183,8 +177,8 @@ func SearchKey(ctx context.Context, s *Repository, password string, maxKeys int,
 }
 
 // LoadKey loads a key from the backend.
-func LoadKey(ctx context.Context, s *Repository, name string) (k *Key, err error) {
+func LoadKey(ctx context.Context, s *Repository, id restic.ID) (k *Key, err error) {
-	h := restic.Handle{Type: restic.KeyFile, Name: name}
+	h := restic.Handle{Type: restic.KeyFile, Name: id.String()}
 	data, err := backend.LoadAll(ctx, nil, s.be, h)
 	if err != nil {
 		return nil, err
@@ -274,10 +268,11 @@ func AddKey(ctx context.Context, s *Repository, password, username, hostname str
 		return nil, errors.Wrap(err, "Marshal")
 	}
 
+	id := restic.Hash(buf)
 	// store in repository and return
 	h := restic.Handle{
 		Type: restic.KeyFile,
-		Name: restic.Hash(buf).String(),
+		Name: id.String(),
 	}
 
 	err = s.be.Save(ctx, h, restic.NewByteReader(buf, s.be.Hasher()))
@@ -285,7 +280,7 @@ func AddKey(ctx context.Context, s *Repository, password, username, hostname str
 		return nil, err
 	}
 
-	newkey.name = h.Name
+	newkey.id = id
 
 	return newkey, nil
 }
@@ -297,9 +292,9 @@ func (k *Key) String() string {
 	return fmt.Sprintf("<Key of %s@%s, created on %s>", k.Username, k.Hostname, k.Created)
 }
 
-// Name returns an identifier for the key.
+// ID returns an identifier for the key.
-func (k Key) Name() string {
+func (k Key) ID() restic.ID {
-	return k.name
+	return k.id
 }
 
 // Valid tests whether the mac and encryption keys are valid (i.e. not zero)

internal/repository/repository.go

@@ -38,7 +38,7 @@ type Repository struct {
 	be    restic.Backend
 	cfg   restic.Config
 	key   *crypto.Key
-	keyName string
+	keyID restic.ID
 	idx   *index.MasterIndex
 	Cache *cache.Cache
 
@@ -709,10 +709,10 @@ func (r *Repository) SearchKey(ctx context.Context, password string, maxKeys int
 	}
 
 	r.key = key.master
-	r.keyName = key.Name()
+	r.keyID = key.ID()
 	cfg, err := restic.LoadConfig(ctx, r)
 	if err == crypto.ErrUnauthenticated {
-		return errors.Fatalf("config or key %v is damaged: %v", key.Name(), err)
+		return errors.Fatalf("config or key %v is damaged: %v", key.ID(), err)
 	} else if err != nil {
 		return errors.Fatalf("config cannot be loaded: %v", err)
 	}
@@ -760,7 +760,7 @@ func (r *Repository) init(ctx context.Context, password string, cfg restic.Confi
 	}
 
 	r.key = key.master
-	r.keyName = key.Name()
+	r.keyID = key.ID()
 	r.setConfig(cfg)
 	return restic.SaveConfig(ctx, r, cfg)
 }
@@ -770,9 +770,9 @@ func (r *Repository) Key() *crypto.Key {
 	return r.key
 }
 
-// KeyName returns the name of the current key in the backend.
+// KeyID returns the id of the current key in the backend.
-func (r *Repository) KeyName() string {
+func (r *Repository) KeyID() restic.ID {
-	return r.keyName
+	return r.keyID
 }
 
 // List runs fn for all files of type t in the repo.

internal/restic/backend_find.go

@@ -26,23 +26,23 @@ func (e *NoIDByPrefixError) Error() string {
 // Find loads the list of all files of type t and searches for names which
 // start with prefix. If none is found, nil and ErrNoIDPrefixFound is returned.
 // If more than one is found, nil and ErrMultipleIDMatches is returned.
-func Find(ctx context.Context, be Lister, t FileType, prefix string) (string, error) {
+func Find(ctx context.Context, be Lister, t FileType, prefix string) (ID, error) {
-	match := ""
+	match := ID{}
 
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	err := be.List(ctx, t, func(fi FileInfo) error {
 		// ignore filename which are not an id
-		_, err := ParseID(fi.Name)
+		id, err := ParseID(fi.Name)
 		if err != nil {
 			debug.Log("unable to parse %v as an ID", fi.Name)
 			return nil
 		}
 
 		if len(fi.Name) >= len(prefix) && prefix == fi.Name[:len(prefix)] {
-			if match == "" {
+			if match.IsNull() {
-				match = fi.Name
+				match = id
 			} else {
 				return &MultipleIDMatchesError{prefix}
 			}
@@ -52,12 +52,12 @@ func Find(ctx context.Context, be Lister, t FileType, prefix string) (string, er
 	})
 
 	if err != nil {
-		return "", err
+		return ID{}, err
 	}
 
-	if match != "" {
+	if !match.IsNull() {
 		return match, nil
 	}
 
-	return "", &NoIDByPrefixError{prefix}
+	return ID{}, &NoIDByPrefixError{prefix}
 }

internal/restic/backend_find_test.go

@@ -43,7 +43,7 @@ func TestFind(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	expectedMatch := "20bdc1402a6fc9b633aaffffffffffffffffffffffffffffffffffffffffffff"
+	expectedMatch := TestParseID("20bdc1402a6fc9b633aaffffffffffffffffffffffffffffffffffffffffffff")
 	if f != expectedMatch {
 		t.Errorf("Wrong match returned want %s, got %s", expectedMatch, f)
 	}
@@ -52,7 +52,7 @@ func TestFind(t *testing.T) {
 	if _, ok := err.(*NoIDByPrefixError); !ok || !strings.Contains(err.Error(), "NotAPrefix") {
 		t.Error("Expected no snapshots to be found.")
 	}
-	if f != "" {
+	if !f.IsNull() {
 		t.Errorf("Find should not return a match on error.")
 	}
 
@@ -62,7 +62,7 @@ func TestFind(t *testing.T) {
 	if _, ok := err.(*NoIDByPrefixError); !ok || !strings.Contains(err.Error(), extraLengthID) {
 		t.Errorf("Wrong error %v for no snapshots matched", err)
 	}
-	if f != "" {
+	if !f.IsNull() {
 		t.Errorf("Find should not return a match on error.")
 	}
 
@@ -71,7 +71,7 @@ func TestFind(t *testing.T) {
 	if _, ok := err.(*MultipleIDMatchesError); !ok {
 		t.Errorf("Wrong error %v for multiple snapshots", err)
 	}
-	if f != "" {
+	if !f.IsNull() {
 		t.Errorf("Find should not return a match on error.")
 	}
 }

internal/restic/snapshot_find.go

@@ -77,12 +77,7 @@ func FindSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, s strin
 	id, err := ParseID(s)
 	if err != nil {
 		// find snapshot id with prefix
-		name, err := Find(ctx, be, SnapshotFile, s)
+		id, err = Find(ctx, be, SnapshotFile, s)
-		if err != nil {
-			return nil, err
-		}
-
-		id, err = ParseID(name)
 		if err != nil {
 			return nil, err
 		}