From 02634dce7a532e63a959282d09a521b98607b9ce Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 15 Oct 2022 16:00:05 +0200 Subject: [PATCH 1/2] restic: change Find to return ids That way consumers no longer have to manually convert the returned name to an id. --- cmd/restic/cmd_debug.go | 5 +---- cmd/restic/cmd_key.go | 2 +- internal/repository/key.go | 2 +- internal/restic/backend_find.go | 16 ++++++++-------- internal/restic/backend_find_test.go | 8 ++++---- internal/restic/snapshot_find.go | 7 +------ 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/cmd/restic/cmd_debug.go b/cmd/restic/cmd_debug.go index 9fcf1b6c7..4c1ffc383 100644 --- a/cmd/restic/cmd_debug.go +++ b/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) - if err == nil { - id, err = restic.ParseID(name) - } + id, err = restic.Find(ctx, repo.Backend(), restic.PackFile, name) if err != nil { Warnf("error: %v\n", err) continue diff --git a/cmd/restic/cmd_key.go b/cmd/restic/cmd_key.go index 52143bbc1..cab5a9d4d 100644 --- a/cmd/restic/cmd_key.go +++ b/cmd/restic/cmd_key.go @@ -236,7 +236,7 @@ func runKey(ctx context.Context, gopts GlobalOptions, args []string) error { return err } - return deleteKey(ctx, repo, id) + return deleteKey(ctx, repo, id.String()) case "passwd": lock, ctx, err := lockRepoExclusive(ctx, repo) defer unlockRepo(lock) diff --git a/internal/repository/key.go b/internal/repository/key.go index 4ce59a1f5..f0e091239 100644 --- a/internal/repository/key.go +++ b/internal/repository/key.go @@ -119,7 +119,7 @@ func SearchKey(ctx context.Context, s *Repository, password string, maxKeys int, id, err := restic.Find(ctx, s.Backend(), restic.KeyFile, keyHint) if err == nil { - key, err := OpenKey(ctx, s, id, password) + key, err := OpenKey(ctx, s, id.String(), password) if err == nil { debug.Log("successfully opened hinted key %v", id) diff --git a/internal/restic/backend_find.go b/internal/restic/backend_find.go index 8ce57dd1d..7c78b3355 100644 --- a/internal/restic/backend_find.go +++ b/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) { - match := "" +func Find(ctx context.Context, be Lister, t FileType, prefix string) (ID, error) { + 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 == "" { - match = fi.Name + if match.IsNull() { + 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} } diff --git a/internal/restic/backend_find_test.go b/internal/restic/backend_find_test.go index bd1e6f7e6..cbd5e7f48 100644 --- a/internal/restic/backend_find_test.go +++ b/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.") } } diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index eadb01f4b..4f8231a7f 100644 --- a/internal/restic/snapshot_find.go +++ b/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) - if err != nil { - return nil, err - } - - id, err = ParseID(name) + id, err = Find(ctx, be, SnapshotFile, s) if err != nil { return nil, err } From 8d62a7adb4feafe150e28c4c6361e485313fdc39 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 15 Oct 2022 16:01:38 +0200 Subject: [PATCH 2/2] identify keys by ID and not name --- cmd/restic/cmd_cat.go | 2 +- cmd/restic/cmd_key.go | 22 ++++++++-------- internal/repository/key.go | 43 ++++++++++++++----------------- internal/repository/repository.go | 24 ++++++++--------- 4 files changed, 43 insertions(+), 48 deletions(-) diff --git a/cmd/restic/cmd_cat.go b/cmd/restic/cmd_cat.go index ec2da564a..f46502d5a 100644 --- a/cmd/restic/cmd_cat.go +++ b/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 } diff --git a/cmd/restic/cmd_key.go b/cmd/restic/cmd_key.go index cab5a9d4d..a673e5756 100644 --- a/cmd/restic/cmd_key.go +++ b/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 { - if name == repo.KeyName() { +func deleteKey(ctx context.Context, repo *repository.Repository, id restic.ID) error { + 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) } @@ -236,7 +236,7 @@ func runKey(ctx context.Context, gopts GlobalOptions, args []string) error { return err } - return deleteKey(ctx, repo, id.String()) + return deleteKey(ctx, repo, id) case "passwd": lock, ctx, err := lockRepoExclusive(ctx, repo) defer unlockRepo(lock) diff --git a/internal/repository/key.go b/internal/repository/key.go index f0e091239..b3e13ade4 100644 --- a/internal/repository/key.go +++ b/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) { - k, err := LoadKey(ctx, s, name) +func OpenKey(ctx context.Context, s *Repository, id restic.ID, password string) (*Key, error) { + 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") @@ -119,7 +119,7 @@ func SearchKey(ctx context.Context, s *Repository, password string, maxKeys int, id, err := restic.Find(ctx, s.Backend(), restic.KeyFile, keyHint) if err == nil { - key, err := OpenKey(ctx, s, id.String(), password) + key, err := OpenKey(ctx, s, id, password) if err == nil { debug.Log("successfully opened hinted key %v", id) @@ -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) - 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) + debug.Log("key %v returned error %v", id.String(), 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) { - h := restic.Handle{Type: restic.KeyFile, Name: name} +func LoadKey(ctx context.Context, s *Repository, id restic.ID) (k *Key, err error) { + 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("", k.Username, k.Hostname, k.Created) } -// Name returns an identifier for the key. -func (k Key) Name() string { - return k.name +// ID returns an identifier for the key. +func (k Key) ID() restic.ID { + return k.id } // Valid tests whether the mac and encryption keys are valid (i.e. not zero) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index cee0d762b..3cae7c597 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -35,12 +35,12 @@ const MaxPackSize = 128 * 1024 * 1024 // Repository is used to access a repository in a backend. type Repository struct { - be restic.Backend - cfg restic.Config - key *crypto.Key - keyName string - idx *index.MasterIndex - Cache *cache.Cache + be restic.Backend + cfg restic.Config + key *crypto.Key + keyID restic.ID + idx *index.MasterIndex + Cache *cache.Cache opts Options @@ -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. -func (r *Repository) KeyName() string { - return r.keyName +// KeyID returns the id of the current key in the backend. +func (r *Repository) KeyID() restic.ID { + return r.keyID } // List runs fn for all files of type t in the repo.