Skip to content

Commit

Permalink
Merge pull request #4808 from MichaelEischer/insecure-no-password
Browse files Browse the repository at this point in the history
Implement `--insecure-no-password` option.
  • Loading branch information
MichaelEischer committed May 24, 2024
2 parents 55cb8d1 + 1305062 commit 80132e7
Show file tree
Hide file tree
Showing 11 changed files with 211 additions and 51 deletions.
19 changes: 19 additions & 0 deletions changelog/unreleased/issue-1786
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Enhancement: Support repositories with empty password

Restic refused to create or operate on repositories with an emtpy password.
Using the new option `--insecure-no-password` it is now possible to disable
this check. Restic will not prompt for a password when using this option.
For security reasons, the option must always be specified when operating on
repositories with an empty password.

Specifying `--insecure-no-password` while also passing a password to restic
via a CLI option or via environment variable results in an error.

The `init` and `copy` command also support the option `--from-insecure-no-password`
which applies to the source repository. The `key add` and `key passwd` comands
include the `--new-insecure-no-password` option to add or set an emtpy password.

https://github.com/restic/restic/issues/1786
https://github.com/restic/restic/issues/4326
https://github.com/restic/restic/pull/4698
https://github.com/restic/restic/pull/4808
2 changes: 1 addition & 1 deletion cmd/restic/cmd_backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func readFilenamesRaw(r io.Reader) (names []string, err error) {

// Check returns an error when an invalid combination of options was set.
func (opts BackupOptions) Check(gopts GlobalOptions, args []string) error {
if gopts.password == "" {
if gopts.password == "" && !gopts.InsecureNoPassword {
if opts.Stdin {
return errors.Fatal("cannot read both password and data from stdin")
}
Expand Down
14 changes: 14 additions & 0 deletions cmd/restic/cmd_backup_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,3 +627,17 @@ func TestStdinFromCommandFailNoOutputAndExitCode(t *testing.T) {

testRunCheck(t, env.gopts)
}

func TestBackupEmptyPassword(t *testing.T) {
// basic sanity test that empty passwords work
env, cleanup := withTestEnvironment(t)
defer cleanup()

env.gopts.password = ""
env.gopts.InsecureNoPassword = true

testSetupBackupData(t, env)
testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, BackupOptions{}, env.gopts)
testListSnapshots(t, env.gopts, 1)
testRunCheck(t, env.gopts)
}
25 changes: 23 additions & 2 deletions cmd/restic/cmd_copy_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ func testRunCopy(t testing.TB, srcGopts GlobalOptions, dstGopts GlobalOptions) {
gopts := srcGopts
gopts.Repo = dstGopts.Repo
gopts.password = dstGopts.password
gopts.InsecureNoPassword = dstGopts.InsecureNoPassword
copyOpts := CopyOptions{
secondaryRepoOptions: secondaryRepoOptions{
Repo: srcGopts.Repo,
password: srcGopts.password,
Repo: srcGopts.Repo,
password: srcGopts.password,
InsecureNoPassword: srcGopts.InsecureNoPassword,
},
}

Expand Down Expand Up @@ -134,3 +136,22 @@ func TestCopyUnstableJSON(t *testing.T) {
testRunCheck(t, env2.gopts)
testListSnapshots(t, env2.gopts, 1)
}

func TestCopyToEmptyPassword(t *testing.T) {
env, cleanup := withTestEnvironment(t)
defer cleanup()
env2, cleanup2 := withTestEnvironment(t)
defer cleanup2()
env2.gopts.password = ""
env2.gopts.InsecureNoPassword = true

testSetupBackupData(t, env)
testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9")}, BackupOptions{}, env.gopts)

testRunInit(t, env2.gopts)
testRunCopy(t, env.gopts, env2.gopts)

testListSnapshots(t, env.gopts, 1)
testListSnapshots(t, env2.gopts, 1)
testRunCheck(t, env2.gopts)
}
49 changes: 35 additions & 14 deletions cmd/restic/cmd_key_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/repository"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

var cmdKeyAdd = &cobra.Command{
Expand All @@ -23,26 +24,30 @@ EXIT STATUS
Exit status is 0 if the command is successful, and non-zero if there was any error.
`,
DisableAutoGenTag: true,
RunE: func(cmd *cobra.Command, args []string) error {
return runKeyAdd(cmd.Context(), globalOptions, keyAddOpts, args)
},
}

type KeyAddOptions struct {
NewPasswordFile string
Username string
Hostname string
NewPasswordFile string
InsecureNoPassword bool
Username string
Hostname string
}

var keyAddOpts KeyAddOptions
func (opts *KeyAddOptions) Add(flags *pflag.FlagSet) {
flags.StringVarP(&opts.NewPasswordFile, "new-password-file", "", "", "`file` from which to read the new password")
flags.BoolVar(&opts.InsecureNoPassword, "new-insecure-no-password", false, "add an empty password for the repository (insecure)")
flags.StringVarP(&opts.Username, "user", "", "", "the username for new key")
flags.StringVarP(&opts.Hostname, "host", "", "", "the hostname for new key")
}

func init() {
cmdKey.AddCommand(cmdKeyAdd)

flags := cmdKeyAdd.Flags()
flags.StringVarP(&keyAddOpts.NewPasswordFile, "new-password-file", "", "", "`file` from which to read the new password")
flags.StringVarP(&keyAddOpts.Username, "user", "", "", "the username for new key")
flags.StringVarP(&keyAddOpts.Hostname, "host", "", "", "the hostname for new key")
var keyAddOpts KeyAddOptions
keyAddOpts.Add(cmdKeyAdd.Flags())
cmdKeyAdd.RunE = func(cmd *cobra.Command, args []string) error {
return runKeyAdd(cmd.Context(), globalOptions, keyAddOpts, args)
}
}

func runKeyAdd(ctx context.Context, gopts GlobalOptions, opts KeyAddOptions, args []string) error {
Expand All @@ -60,7 +65,7 @@ func runKeyAdd(ctx context.Context, gopts GlobalOptions, opts KeyAddOptions, arg
}

func addKey(ctx context.Context, repo *repository.Repository, gopts GlobalOptions, opts KeyAddOptions) error {
pw, err := getNewPassword(ctx, gopts, opts.NewPasswordFile)
pw, err := getNewPassword(ctx, gopts, opts.NewPasswordFile, opts.InsecureNoPassword)
if err != nil {
return err
}
Expand All @@ -83,19 +88,35 @@ func addKey(ctx context.Context, repo *repository.Repository, gopts GlobalOption
// testKeyNewPassword is used to set a new password during integration testing.
var testKeyNewPassword string

func getNewPassword(ctx context.Context, gopts GlobalOptions, newPasswordFile string) (string, error) {
func getNewPassword(ctx context.Context, gopts GlobalOptions, newPasswordFile string, insecureNoPassword bool) (string, error) {
if testKeyNewPassword != "" {
return testKeyNewPassword, nil
}

if insecureNoPassword {
if newPasswordFile != "" {
return "", fmt.Errorf("only either --new-password-file or --new-insecure-no-password may be specified")
}
return "", nil
}

if newPasswordFile != "" {
return loadPasswordFromFile(newPasswordFile)
password, err := loadPasswordFromFile(newPasswordFile)
if err != nil {
return "", err
}
if password == "" {
return "", fmt.Errorf("an empty password is not allowed by default. Pass the flag `--new-insecure-no-password` to restic to disable this check")
}
return password, nil
}

// Since we already have an open repository, temporary remove the password
// to prompt the user for the passwd.
newopts := gopts
newopts.password = ""
// empty passwords are already handled above
newopts.InsecureNoPassword = false

return ReadPasswordTwice(ctx, newopts,
"enter new password: ",
Expand Down
39 changes: 39 additions & 0 deletions cmd/restic/cmd_key_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package main
import (
"bufio"
"context"
"os"
"path/filepath"
"regexp"
"strings"
"testing"
Expand Down Expand Up @@ -109,6 +111,43 @@ func TestKeyAddRemove(t *testing.T) {
testRunKeyAddNewKeyUserHost(t, env.gopts)
}

func TestKeyAddInvalid(t *testing.T) {
env, cleanup := withTestEnvironment(t)
defer cleanup()
testRunInit(t, env.gopts)

err := runKeyAdd(context.TODO(), env.gopts, KeyAddOptions{
NewPasswordFile: "some-file",
InsecureNoPassword: true,
}, []string{})
rtest.Assert(t, strings.Contains(err.Error(), "only either"), "unexpected error message, got %q", err)

pwfile := filepath.Join(t.TempDir(), "pwfile")
rtest.OK(t, os.WriteFile(pwfile, []byte{}, 0o666))

err = runKeyAdd(context.TODO(), env.gopts, KeyAddOptions{
NewPasswordFile: pwfile,
}, []string{})
rtest.Assert(t, strings.Contains(err.Error(), "an empty password is not allowed by default"), "unexpected error message, got %q", err)
}

func TestKeyAddEmpty(t *testing.T) {
env, cleanup := withTestEnvironment(t)
// must list keys more than once
env.gopts.backendTestHook = nil
defer cleanup()
testRunInit(t, env.gopts)

rtest.OK(t, runKeyAdd(context.TODO(), env.gopts, KeyAddOptions{
InsecureNoPassword: true,
}, []string{}))

env.gopts.password = ""
env.gopts.InsecureNoPassword = true

testRunCheck(t, env.gopts)
}

type emptySaveBackend struct {
backend.Backend
}
Expand Down
16 changes: 6 additions & 10 deletions cmd/restic/cmd_key_passwd.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,20 @@ EXIT STATUS
Exit status is 0 if the command is successful, and non-zero if there was any error.
`,
DisableAutoGenTag: true,
RunE: func(cmd *cobra.Command, args []string) error {
return runKeyPasswd(cmd.Context(), globalOptions, keyPasswdOpts, args)
},
}

type KeyPasswdOptions struct {
KeyAddOptions
}

var keyPasswdOpts KeyPasswdOptions

func init() {
cmdKey.AddCommand(cmdKeyPasswd)

flags := cmdKeyPasswd.Flags()
flags.StringVarP(&keyPasswdOpts.NewPasswordFile, "new-password-file", "", "", "`file` from which to read the new password")
flags.StringVarP(&keyPasswdOpts.Username, "user", "", "", "the username for new key")
flags.StringVarP(&keyPasswdOpts.Hostname, "host", "", "", "the hostname for new key")
var keyPasswdOpts KeyPasswdOptions
keyPasswdOpts.KeyAddOptions.Add(cmdKeyPasswd.Flags())
cmdKeyPasswd.RunE = func(cmd *cobra.Command, args []string) error {
return runKeyPasswd(cmd.Context(), globalOptions, keyPasswdOpts, args)
}
}

func runKeyPasswd(ctx context.Context, gopts GlobalOptions, opts KeyPasswdOptions, args []string) error {
Expand All @@ -57,7 +53,7 @@ func runKeyPasswd(ctx context.Context, gopts GlobalOptions, opts KeyPasswdOption
}

func changePassword(ctx context.Context, repo *repository.Repository, gopts GlobalOptions, opts KeyPasswdOptions) error {
pw, err := getNewPassword(ctx, gopts, opts.NewPasswordFile)
pw, err := getNewPassword(ctx, gopts, opts.NewPasswordFile, opts.InsecureNoPassword)
if err != nil {
return err
}
Expand Down
45 changes: 27 additions & 18 deletions cmd/restic/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,23 @@ type backendWrapper func(r backend.Backend) (backend.Backend, error)

// GlobalOptions hold all global options for restic.
type GlobalOptions struct {
Repo string
RepositoryFile string
PasswordFile string
PasswordCommand string
KeyHint string
Quiet bool
Verbose int
NoLock bool
RetryLock time.Duration
JSON bool
CacheDir string
NoCache bool
CleanupCache bool
Compression repository.CompressionMode
PackSize uint
NoExtraVerify bool
Repo string
RepositoryFile string
PasswordFile string
PasswordCommand string
KeyHint string
Quiet bool
Verbose int
NoLock bool
RetryLock time.Duration
JSON bool
CacheDir string
NoCache bool
CleanupCache bool
Compression repository.CompressionMode
PackSize uint
NoExtraVerify bool
InsecureNoPassword bool

backend.TransportOptions
limiter.Limits
Expand Down Expand Up @@ -125,6 +126,7 @@ func init() {
f.BoolVar(&globalOptions.NoCache, "no-cache", false, "do not use a local cache")
f.StringSliceVar(&globalOptions.RootCertFilenames, "cacert", nil, "`file` to load root certificates from (default: use system certificates or $RESTIC_CACERT)")
f.StringVar(&globalOptions.TLSClientCertKeyFilename, "tls-client-cert", "", "path to a `file` containing PEM encoded TLS client certificate and private key (default: $RESTIC_TLS_CLIENT_CERT)")
f.BoolVar(&globalOptions.InsecureNoPassword, "insecure-no-password", false, "use an empty password for the repository, must be passed to every restic command (insecure)")
f.BoolVar(&globalOptions.InsecureTLS, "insecure-tls", false, "skip TLS certificate verification when connecting to the repository (insecure)")
f.BoolVar(&globalOptions.CleanupCache, "cleanup-cache", false, "auto remove old cache directories")
f.Var(&globalOptions.Compression, "compression", "compression mode (only available for repository format version 2), one of (auto|off|max) (default: $RESTIC_COMPRESSION)")
Expand Down Expand Up @@ -327,6 +329,13 @@ func readPasswordTerminal(ctx context.Context, in *os.File, out *os.File, prompt
// variable RESTIC_PASSWORD or prompts the user. If the context is canceled,
// the function leaks the password reading goroutine.
func ReadPassword(ctx context.Context, opts GlobalOptions, prompt string) (string, error) {
if opts.InsecureNoPassword {
if opts.password != "" {
return "", errors.Fatal("--insecure-no-password must not be specified together with providing a password via a cli option or environment variable")
}
return "", nil
}

if opts.password != "" {
return opts.password, nil
}
Expand All @@ -348,7 +357,7 @@ func ReadPassword(ctx context.Context, opts GlobalOptions, prompt string) (strin
}

if len(password) == 0 {
return "", errors.Fatal("an empty password is not a password")
return "", errors.Fatal("an empty password is not allowed by default. Pass the flag `--insecure-no-password` to restic to disable this check")
}

return password, nil
Expand Down Expand Up @@ -445,7 +454,7 @@ func OpenRepository(ctx context.Context, opts GlobalOptions) (*repository.Reposi
}

passwordTriesLeft := 1
if stdinIsTerminal() && opts.password == "" {
if stdinIsTerminal() && opts.password == "" && !opts.InsecureNoPassword {
passwordTriesLeft = 3
}

Expand Down
13 changes: 13 additions & 0 deletions cmd/restic/global_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package main

import (
"context"
"os"
"path/filepath"
"strings"
"testing"

rtest "github.com/restic/restic/internal/test"
Expand Down Expand Up @@ -50,3 +52,14 @@ func TestReadRepo(t *testing.T) {
t.Fatal("must not read repository path from invalid file path")
}
}

func TestReadEmptyPassword(t *testing.T) {
opts := GlobalOptions{InsecureNoPassword: true}
password, err := ReadPassword(context.TODO(), opts, "test")
rtest.OK(t, err)
rtest.Equals(t, "", password, "got unexpected password")

opts.password = "invalid"
_, err = ReadPassword(context.TODO(), opts, "test")
rtest.Assert(t, strings.Contains(err.Error(), "must not be specified together with providing a password via a cli option or environment variable"), "unexpected error message, got %v", err)
}
Loading

0 comments on commit 80132e7

Please sign in to comment.