Merge pull request #3429 from MichaelEischer/safe-key-change

key: Check that new key works before deleting the old one

cmd/restic/cmd_key.go

@@ -131,6 +131,11 @@ func addKey(gopts GlobalOptions, repo *repository.Repository) error {
 		return errors.Fatalf("creating new key failed: %v\n", err)
 	}
 
+	err = switchToNewKeyAndRemoveIfBroken(gopts.ctx, repo, id, pw)
+	if err != nil {
+		return err
+	}
+
 	Verbosef("saved new key as %s\n", id)
 
 	return nil
@@ -161,8 +166,14 @@ func changePassword(gopts GlobalOptions, repo *repository.Repository) error {
 	if err != nil {
 		return errors.Fatalf("creating new key failed: %v\n", err)
 	}
+	oldID := repo.KeyName()
 
-	h := restic.Handle{Type: restic.KeyFile, Name: repo.KeyName()}
+	err = switchToNewKeyAndRemoveIfBroken(gopts.ctx, repo, id, pw)
+	if err != nil {
+		return err
+	}
+
+	h := restic.Handle{Type: restic.KeyFile, Name: oldID}
 	err = repo.Backend().Remove(gopts.ctx, h)
 	if err != nil {
 		return err
@@ -173,6 +184,19 @@ func changePassword(gopts GlobalOptions, repo *repository.Repository) error {
 	return nil
 }
 
+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())
+	if err != nil {
+		// the key is invalid, try to remove it
+		h := restic.Handle{Type: restic.KeyFile, Name: key.Name()}
+		_ = repo.Backend().Remove(ctx, h)
+		return errors.Fatalf("failed to access repository with new key: %v", err)
+	}
+	return nil
+}
+
 func runKey(gopts GlobalOptions, args []string) error {
 	if len(args) < 1 || (args[0] == "remove" && len(args) != 2) || (args[0] != "remove" && len(args) != 1) {
 		return errors.Fatal("wrong number of arguments")

cmd/restic/integration_test.go

@@ -1084,6 +1084,41 @@ func TestKeyAddRemove(t *testing.T) {
 	testRunKeyAddNewKeyUserHost(t, env.gopts)
 }
 
+type emptySaveBackend struct {
+	restic.Backend
+}
+
+func (b *emptySaveBackend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
+	return b.Backend.Save(ctx, h, restic.NewByteReader(make([]byte, 0)))
+}
+
+func TestKeyProblems(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testRunInit(t, env.gopts)
+	env.gopts.backendTestHook = func(r restic.Backend) (restic.Backend, error) {
+		return &emptySaveBackend{r}, nil
+	}
+
+	testKeyNewPassword = "geheim2"
+	defer func() {
+		testKeyNewPassword = ""
+	}()
+
+	err := runKey(env.gopts, []string{"passwd"})
+	t.Log(err)
+	rtest.Assert(t, err != nil, "expected passwd change to fail")
+
+	err = runKey(env.gopts, []string{"add"})
+	t.Log(err)
+	rtest.Assert(t, err != nil, "expected key adding to fail")
+
+	t.Logf("testing access with initial password %q\n", env.gopts.password)
+	rtest.OK(t, runKey(env.gopts, []string{"list"}))
+	testRunCheck(t, env.gopts)
+}
+
 func testFileSize(filename string, size int64) error {
 	fi, err := os.Stat(filename)
 	if err != nil {