diff --git a/Makefile b/Makefile index df1891af1..07133ae50 100644 --- a/Makefile +++ b/Makefile @@ -118,7 +118,6 @@ GORELEASER_OPTIONS+=--skip-publish endif goreleaser: $(goreleaser) - # print current git diff, pipe through cat to avoid blocking the build on pager -git diff | cat $(goreleaser) release $(GORELEASER_OPTIONS) diff --git a/cli/storage_sftp.go b/cli/storage_sftp.go index 12e9ec849..e0cfd68b9 100644 --- a/cli/storage_sftp.go +++ b/cli/storage_sftp.go @@ -2,7 +2,9 @@ import ( "context" + "io/ioutil" + "github.com/pkg/errors" "gopkg.in/alecthomas/kingpin.v2" "github.com/kopia/kopia/repo/blob" @@ -11,8 +13,9 @@ func init() { var ( - options sftp.Options - connectFlat bool + options sftp.Options + connectFlat bool + embedCredentials bool ) RegisterStorageConnectFlags( @@ -23,12 +26,28 @@ func(cmd *kingpin.CmdClause) { cmd.Flag("host", "SFTP/SSH server hostname").Required().StringVar(&options.Host) cmd.Flag("port", "SFTP/SSH server port").Default("22").IntVar(&options.Port) cmd.Flag("username", "SFTP/SSH server username").Required().StringVar(&options.Username) - cmd.Flag("keyfile", "path to private key file for SFTP/SSH server").Required().StringVar(&options.Keyfile) + cmd.Flag("keyfile", "path to private key file for SFTP/SSH server").StringVar(&options.Keyfile) + cmd.Flag("key-data", "private key data").StringVar(&options.KeyData) + cmd.Flag("embed-credentials", "Embed key and known_hosts in Kopia configuration").BoolVar(&embedCredentials) cmd.Flag("flat", "Use flat directory structure").BoolVar(&connectFlat) }, func(ctx context.Context, isNew bool) (blob.Storage, error) { sftpo := options + if embedCredentials { + d, err := ioutil.ReadFile(sftpo.Keyfile) + if err != nil { + return nil, err + } + + sftpo.KeyData = string(d) + sftpo.Keyfile = "" + } + + if sftpo.KeyData == "" && sftpo.Keyfile == "" { + return nil, errors.Errorf("must provide either key file or key data") + } + if connectFlat { sftpo.DirectoryShards = []int{} } diff --git a/htmlui/src/SetupRepository.js b/htmlui/src/SetupRepository.js index af9103c6b..3c309c03e 100644 --- a/htmlui/src/SetupRepository.js +++ b/htmlui/src/SetupRepository.js @@ -30,8 +30,6 @@ export class SetupRepository extends Component { this.state = { confirmCreate: false, isLoading: false, - password: "", - confirmPassword: "", }; this.handleChange = handleChange.bind(this); diff --git a/htmlui/src/SetupSFTP.js b/htmlui/src/SetupSFTP.js index efb27c6e7..a2068a730 100644 --- a/htmlui/src/SetupSFTP.js +++ b/htmlui/src/SetupSFTP.js @@ -1,6 +1,6 @@ import React, { Component } from 'react'; import Form from 'react-bootstrap/Form'; -import { handleChange, RequiredField, validateRequiredFields, OptionalNumberField } from './forms'; +import { handleChange, OptionalField, OptionalNumberField, RequiredField, validateRequiredFields, hasExactlyOneOf } from './forms'; export class SetupSFTP extends Component { constructor() { @@ -13,7 +13,19 @@ export class SetupSFTP extends Component { } validate() { - return validateRequiredFields(this, ["host", "port", "path", "keyfile"]) + if (!validateRequiredFields(this, ["host", "port", "username", "path"])) { + return false; + } + + if (!hasExactlyOneOf(this, ["keyfile", "keyData"])) { + return false; + } + + if (!hasExactlyOneOf(this, ["knownHostsFile", "knownHostsData"])) { + return false; + } + + return true; } render() { @@ -21,10 +33,28 @@ export class SetupSFTP extends Component { {RequiredField(this, "Host", "host", { placeholder: "host name" })} {OptionalNumberField(this, "Port", "port", { placeholder: "port number (e.g. 22)" })} + {RequiredField(this, "User", "username", { placeholder: "user name" })} + + {RequiredField(this, "Path", "path", { placeholder: "enter remote path" })} - {RequiredField(this, "Key File", "keyfile", { placeholder: "enter path to the key file" })} + {OptionalField(this, "Path to key file", "keyfile", { placeholder: "enter path to the key file" })} + {OptionalField(this, "Path to known_hosts File", "knownHostsFile", { placeholder: "enter path to known_hosts file" })} + + + {OptionalField(this, "Key Data", "keyData", { + placeholder: "paste contents of the key file", + as: "textarea", + rows: 5, + isInvalid: !hasExactlyOneOf(this, ["keyfile", "keyData"]), + }, null, <>Either Key File or Key Data is required, but not both.)} + {OptionalField(this, "Known Hosts Data", "knownHostsData", { + placeholder: "paste contents of the known_hosts file", + as: "textarea", + rows: 5, + isInvalid: !hasExactlyOneOf(this, ["knownHostsFile", "knownHostsData"]), + }, null, <>Either Known Hosts File or Known Hosts Data is required, but not both.)})} ; } diff --git a/htmlui/src/forms.js b/htmlui/src/forms.js index f7defeb53..0819029da 100644 --- a/htmlui/src/forms.js +++ b/htmlui/src/forms.js @@ -57,10 +57,14 @@ export function stateProperty(component, name, defaultValue = "") { return undefined; } - st = st[part]; + if (part in st) { + st = st[part]; + } else { + return defaultValue; + } } - return st || defaultValue; + return st; } export function RequiredField(component, label, name, props = {}, helpText = null) { @@ -78,7 +82,7 @@ export function RequiredField(component, label, name, props = {}, helpText = nul } -export function OptionalField(component, label, name, props = {}, helpText = null) { +export function OptionalField(component, label, name, props = {}, helpText = null, invalidFeedback = null) { return {label} {helpText && {helpText}} + {invalidFeedback && {invalidFeedback}} } @@ -130,6 +135,18 @@ export function OptionalNumberField(component, label, name, props = {}) { } +export function hasExactlyOneOf(component, names) { + let count = 0; + + for (let i = 0; i < names.length; i++) { + if (stateProperty(component, names[i])) { + count++ + } + } + + return count === 1; +} + function checkedToBool(t) { if (t.checked) { return true; diff --git a/htmlui/src/tests/SetupSFTP.test.js b/htmlui/src/tests/SetupSFTP.test.js index 74219dd5e..dbd918b50 100644 --- a/htmlui/src/tests/SetupSFTP.test.js +++ b/htmlui/src/tests/SetupSFTP.test.js @@ -12,12 +12,36 @@ it('can set fields', async () => { changeControlValue(getByTestId("control-host"), "some-host"); changeControlValue(getByTestId("control-port"), "22"); changeControlValue(getByTestId("control-path"), "some-path"); + changeControlValue(getByTestId("control-username"), "some-username"); changeControlValue(getByTestId("control-keyfile"), "some-keyfile"); + changeControlValue(getByTestId("control-knownHostsFile"), "some-knownHostsFile"); + expect(ref.current.validate()).toBe(true); + expect(ref.current.state).toStrictEqual({ + "host": "some-host", + "username": "some-username", + "keyfile": "some-keyfile", + "knownHostsFile": "some-knownHostsFile", + "path": "some-path", + "port": 22, + }); + + // now enter key data instead of key file, make sure validation triggers along the way + changeControlValue(getByTestId("control-keyData"), "some-keyData"); + expect(ref.current.validate()).toBe(false); + changeControlValue(getByTestId("control-keyfile"), ""); + expect(ref.current.validate()).toBe(true); + changeControlValue(getByTestId("control-knownHostsData"), "some-knownHostsData"); + expect(ref.current.validate()).toBe(false); + changeControlValue(getByTestId("control-knownHostsFile"), ""); expect(ref.current.validate()).toBe(true); expect(ref.current.state).toStrictEqual({ "host": "some-host", - "keyfile": "some-keyfile", + "username": "some-username", + "keyfile": "", + "keyData": "some-keyData", + "knownHostsFile": "", + "knownHostsData": "some-knownHostsData", "path": "some-path", "port": 22, }); diff --git a/repo/blob/sftp/sftp_options.go b/repo/blob/sftp/sftp_options.go index 5048de354..ce04da1c6 100644 --- a/repo/blob/sftp/sftp_options.go +++ b/repo/blob/sftp/sftp_options.go @@ -1,14 +1,21 @@ package sftp +import ( + "os" + "path/filepath" +) + // Options defines options for sftp-backed storage. type Options struct { Path string `json:"path"` - Host string `json:"host"` - Port int `json:"port"` - Username string `json:"username"` - Keyfile string `json:"keyfile,omitempty"` - KnownHosts string `json:"-"` + Host string `json:"host"` + Port int `json:"port"` + Username string `json:"username"` + Keyfile string `json:"keyfile,omitempty"` + KeyData string `json:"keyData,omitempty" kopia:"sensitive"` + KnownHostsFile string `json:"knownHostsFile,omitempty"` + KnownHostsData string `json:"knownHostsData,omitempty"` DirectoryShards []int `json:"dirShards"` } @@ -21,10 +28,12 @@ func (sftpo *Options) shards() []int { return sftpo.DirectoryShards } -func (sftpo *Options) knownHosts() string { - if sftpo.KnownHosts == "" { - return sftpDefaultKnownHosts +func (sftpo *Options) knownHostsFile() string { + if sftpo.KnownHostsFile == "" { + d, _ := os.UserHomeDir() + + return filepath.Join(d, ".ssh", "known_hosts") } - return sftpo.KnownHosts + return sftpo.KnownHostsFile } diff --git a/repo/blob/sftp/sftp_storage.go b/repo/blob/sftp/sftp_storage.go index b4818cf1e..0325d7714 100644 --- a/repo/blob/sftp/sftp_storage.go +++ b/repo/blob/sftp/sftp_storage.go @@ -7,9 +7,10 @@ "context" "crypto/rand" "fmt" + "io" "io/ioutil" "os" - "path/filepath" + "path" "strings" "github.com/pkg/errors" @@ -28,8 +29,7 @@ ) var ( - sftpDefaultShards = []int{3, 3} - sftpDefaultKnownHosts = filepath.Join(os.Getenv("HOME"), ".ssh", "known_hosts") + sftpDefaultShards = []int{3, 3} ) // sftpStorage implements blob.Storage on top of sftp. @@ -44,9 +44,9 @@ type sftpImpl struct { cli *psftp.Client } -func (s *sftpImpl) GetBlobFromPath(ctx context.Context, dirPath, path string, offset, length int64) ([]byte, error) { - r, err := s.cli.Open(path) - if os.IsNotExist(err) { +func (s *sftpImpl) GetBlobFromPath(ctx context.Context, dirPath, fullPath string, offset, length int64) ([]byte, error) { + r, err := s.cli.Open(fullPath) + if isNotExist(err) { return nil, blob.ErrBlobNotFound } @@ -80,13 +80,20 @@ func (s *sftpImpl) GetBlobFromPath(ctx context.Context, dirPath, path string, of return data[0:length], nil } -func (s *sftpImpl) PutBlobInPath(ctx context.Context, dirPath, path string, data []byte) error { +func (s *sftpImpl) PutBlobInPath(ctx context.Context, dirPath, fullPath string, data []byte) error { randSuffix := make([]byte, 8) if _, err := rand.Read(randSuffix); err != nil { return errors.Wrap(err, "can't get random bytes") } - tempFile := fmt.Sprintf("%s.tmp.%x", path, randSuffix) + progressCallback := blob.ProgressCallback(ctx) + + if progressCallback != nil { + progressCallback(fullPath, 0, int64(len(data))) + defer progressCallback(fullPath, int64(len(data)), int64(len(data))) + } + + tempFile := fmt.Sprintf("%s.tmp.%x", fullPath, randSuffix) f, err := s.createTempFileAndDir(tempFile) if err != nil { @@ -101,7 +108,7 @@ func (s *sftpImpl) PutBlobInPath(ctx context.Context, dirPath, path string, data return errors.Wrap(err, "can't close temporary file") } - err = s.cli.PosixRename(tempFile, path) + err = s.cli.PosixRename(tempFile, fullPath) if err != nil { if removeErr := s.cli.Remove(tempFile); removeErr != nil { fmt.Printf("warning: can't remove temp file: %v", removeErr) @@ -117,8 +124,9 @@ func (s *sftpImpl) createTempFileAndDir(tempFile string) (*psftp.File, error) { flags := os.O_CREATE | os.O_WRONLY | os.O_EXCL f, err := s.cli.OpenFile(tempFile, flags) - if os.IsNotExist(err) { - if err = s.cli.MkdirAll(filepath.Dir(tempFile)); err != nil { + if isNotExist(err) { + parentDir := path.Dir(tempFile) + if err = s.cli.MkdirAll(parentDir); err != nil { return nil, errors.Wrap(err, "cannot create directory") } @@ -128,9 +136,17 @@ func (s *sftpImpl) createTempFileAndDir(tempFile string) (*psftp.File, error) { return f, err } -func (s *sftpImpl) DeleteBlobInPath(ctx context.Context, dirPath, path string) error { - err := s.cli.Remove(path) - if err == nil || os.IsNotExist(err) { +func isNotExist(err error) bool { + if err == nil { + return false + } + + return strings.Contains(err.Error(), "does not exist") +} + +func (s *sftpImpl) DeleteBlobInPath(ctx context.Context, dirPath, fullPath string) error { + err := s.cli.Remove(fullPath) + if err == nil || isNotExist(err) { return nil } @@ -185,40 +201,56 @@ func hostExists(host string, hosts []string) bool { // getHostKey parses OpenSSH known_hosts file for a public key that matches the host // The known_hosts file format is documented in the sshd(8) manual page -func getHostKey(host, knownHosts string) (ssh.PublicKey, error) { - file, err := os.Open(knownHosts) //nolint:gosec - if err != nil { - return nil, err +func getHostKey(opt *Options) (ssh.PublicKey, error) { + var reader io.Reader + + if opt.KnownHostsData != "" { + reader = strings.NewReader(opt.KnownHostsData) + } else { + file, err := os.Open(opt.knownHostsFile()) //nolint:gosec + if err != nil { + return nil, err + } + defer file.Close() //nolint:errcheck + + reader = file } - defer file.Close() //nolint:errcheck - var hostKey ssh.PublicKey - - var hosts []string - - scanner := bufio.NewScanner(file) + scanner := bufio.NewScanner(reader) for scanner.Scan() { - _, hosts, hostKey, _, _, err = ssh.ParseKnownHosts(scanner.Bytes()) + _, hosts, hostKey, _, _, err := ssh.ParseKnownHosts(scanner.Bytes()) if err != nil { return nil, errors.Wrapf(err, "error parsing %s", scanner.Text()) } - if hostExists(host, hosts) { + if hostExists(opt.Host, hosts) { return hostKey, nil } } - return nil, errors.Wrapf(err, "no hostkey found for %s", host) + return nil, errors.Errorf("no hostkey found for %s", opt.Host) } // getSigner parses and returns a signer for the user-entered private key -func getSigner(path string) (ssh.Signer, error) { - buffer, err := ioutil.ReadFile(path) //nolint:gosec - if err != nil { - return nil, err +func getSigner(opts *Options) (ssh.Signer, error) { + if opts.Keyfile == "" && opts.KeyData == "" { + return nil, errors.New("must specify the location of the ssh server private key or the key data") } - key, err := ssh.ParsePrivateKey(buffer) + var privateKeyData []byte + + if opts.KeyData != "" { + privateKeyData = []byte(opts.KeyData) + } else { + var err error + + privateKeyData, err = ioutil.ReadFile(opts.Keyfile) //nolint:gosec + if err != nil { + return nil, err + } + } + + key, err := ssh.ParsePrivateKey(privateKeyData) if err != nil { return nil, err } @@ -227,18 +259,14 @@ func getSigner(path string) (ssh.Signer, error) { } func createSSHConfig(opts *Options) (*ssh.ClientConfig, error) { - if opts.Keyfile == "" { - return nil, errors.New("must specify the location of the ssh server private key") - } - - hostKey, err := getHostKey(opts.Host, opts.knownHosts()) + hostKey, err := getHostKey(opts) if err != nil { return nil, errors.Wrapf(err, "unable to getHostKey: %s", opts.Host) } - signer, err := getSigner(opts.Keyfile) + signer, err := getSigner(opts) if err != nil { - return nil, errors.Wrapf(err, "unable to getSigner: %s", opts.Keyfile) + return nil, errors.Wrapf(err, "unable to getSigner") } config := &ssh.ClientConfig{ @@ -272,7 +300,7 @@ func New(ctx context.Context, opts *Options) (blob.Storage, error) { } if _, err = c.Stat(opts.Path); err != nil { - if os.IsNotExist(err) { + if isNotExist(err) { if err = c.MkdirAll(opts.Path); err != nil { return nil, errors.Wrap(err, "cannot create path") } diff --git a/repo/blob/sftp/sftp_storage_test.go b/repo/blob/sftp/sftp_storage_test.go index 6f51f2e46..c69b2544b 100644 --- a/repo/blob/sftp/sftp_storage_test.go +++ b/repo/blob/sftp/sftp_storage_test.go @@ -2,11 +2,11 @@ import ( "context" + "fmt" + "io/ioutil" "os" - "runtime" "strconv" "testing" - "time" "github.com/kopia/kopia/internal/blobtesting" "github.com/kopia/kopia/internal/testlogging" @@ -14,49 +14,29 @@ "github.com/kopia/kopia/repo/blob/sftp" ) -const ( - t1 = "392ee1bc299db9f235e046a62625afb84902" - t2 = "2a7ff4f29eddbcd4c18fa9e73fec20bbb71f" - t3 = "0dae5918f83e6a24c8b3e274ca1026e43f24" -) - func TestSFTPStorageValid(t *testing.T) { - ctx := testlogging.Context(t) + for _, embedCreds := range []bool{false, true} { + embedCreds := embedCreds + t.Run(fmt.Sprintf("Embed=%v", embedCreds), func(t *testing.T) { + ctx := testlogging.Context(t) - if runtime.GOOS == "windows" { - t.Skip("temporarily disabled - https://github.com/kopia/kopia/issues/216") - } + st, err := createSFTPStorage(ctx, t, embedCreds) + if err != nil { + t.Fatalf("unable to connect to SSH: %v", err) + } - st, err := createSFTPStorage(ctx, t) + deleteBlobs(ctx, t, st) - if err != nil { - t.Fatalf("unable to connect to SSH: %v", err) - } + blobtesting.VerifyStorage(ctx, t, st) + blobtesting.AssertConnectionInfoRoundTrips(ctx, t, st) - assertNoError(t, st.PutBlob(ctx, t1, []byte{1})) - time.Sleep(1 * time.Second) // sleep a bit to accommodate Apple filesystems with low timestamp resolution - assertNoError(t, st.PutBlob(ctx, t2, []byte{1})) - time.Sleep(1 * time.Second) - assertNoError(t, st.PutBlob(ctx, t3, []byte{1})) + // delete everything again + deleteBlobs(ctx, t, st) - deleteBlobs(ctx, t, st) - - blobtesting.VerifyStorage(ctx, t, st) - blobtesting.AssertConnectionInfoRoundTrips(ctx, t, st) - - // delete everything again - deleteBlobs(ctx, t, st) - - if err := st.Close(ctx); err != nil { - t.Fatalf("err: %v", err) - } -} - -func assertNoError(t *testing.T, err error) { - t.Helper() - - if err != nil { - t.Errorf("err: %v", err) + if err := st.Close(ctx); err != nil { + t.Fatalf("err: %v", err) + } + }) } } @@ -68,7 +48,7 @@ func deleteBlobs(ctx context.Context, t *testing.T, st blob.Storage) { } } -func createSFTPStorage(ctx context.Context, t *testing.T) (blob.Storage, error) { +func createSFTPStorage(ctx context.Context, t *testing.T, embed bool) (blob.Storage, error) { host := os.Getenv("KOPIA_SFTP_TEST_HOST") if host == "" { t.Skip("KOPIA_SFTP_TEST_HOST not provided") @@ -99,17 +79,36 @@ func createSFTPStorage(ctx context.Context, t *testing.T) (blob.Storage, error) t.Skip("KOPIA_SFTP_TEST_USER not provided") } - knownHosts := os.Getenv("KOPIA_SFTP_KNOWN_HOSTS_FILE") - if _, err = os.Stat(knownHosts); err != nil { + knownHostsFile := os.Getenv("KOPIA_SFTP_KNOWN_HOSTS_FILE") + if _, err = os.Stat(knownHostsFile); err != nil { t.Skip("skipping test because SFTP known hosts file can't be opened") } - return sftp.New(ctx, &sftp.Options{ - Path: path, - Host: host, - Username: usr, - Port: int(port), - Keyfile: keyfile, - KnownHosts: knownHosts, - }) + opt := &sftp.Options{ + Path: path, + Host: host, + Username: usr, + Port: int(port), + Keyfile: keyfile, + KnownHostsFile: knownHostsFile, + } + + if embed { + opt.KeyData = mustReadFileToString(t, opt.Keyfile) + opt.Keyfile = "" + + opt.KnownHostsData = mustReadFileToString(t, opt.KnownHostsFile) + opt.KnownHostsFile = "" + } + + return sftp.New(ctx, opt) +} + +func mustReadFileToString(t *testing.T, fname string) string { + data, err := ioutil.ReadFile(fname) + if err != nil { + t.Fatal(err) + } + + return string(data) }