mirror/kopia
1
0
Fork 0
mirror of https://github.com/kopia/kopia.git synced 2026-04-28 09:59:10 -04:00
Code Issues Packages Projects Releases Wiki Activity

sftp: Fixed issues in SFTP provider, Fixes #216

Browse Source 
- did not work on windows due to use of filepath which uses backslash
  instead of slash
- added support for embedding SFTP key
- fixed UI controls
- misc fixes for KopiaUI
- added progress reporting
This commit is contained in:
Jarek Kowalski
2020-03-01 14:14:30 -08:00
parent faa7225c23
commit d95e6a3d09
9 changed files with 235 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Makefile
View File

@@ -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)

25
cli/storage_sftp.go
View File

@@ -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{}
}

2
htmlui/src/SetupRepository.js
View File

@@ -30,8 +30,6 @@ export class SetupRepository extends Component {
this.state = {
confirmCreate: false,
isLoading: false,
password: "",
confirmPassword: "",
};
this.handleChange = handleChange.bind(this);

36
htmlui/src/SetupSFTP.js
View File

@@ -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 {
<Form.Row>
{RequiredField(this, "Host", "host", { placeholder: "host name" })}
{OptionalNumberField(this, "Port", "port", { placeholder: "port number (e.g. 22)" })}
{RequiredField(this, "User", "username", { placeholder: "user name" })}
</Form.Row>
<Form.Row>
{RequiredField(this, "Path", "path", { placeholder: "enter remote path" })}
</Form.Row>
<Form.Row>
{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" })}
</Form.Row>
<Form.Row>
{OptionalField(this, "Key Data", "keyData", {
placeholder: "paste contents of the key file",
as: "textarea",
rows: 5,
isInvalid: !hasExactlyOneOf(this, ["keyfile", "keyData"]),
}, null, <>Either <b>Key File</b> or <b>Key Data</b> 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 <b>Known Hosts File</b> or <b>Known Hosts Data</b> is required, but not both.</>)})}
</Form.Row>
</>;
}

23
htmlui/src/forms.js
View File

@@ -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
</Form.Group>
}
export function OptionalField(component, label, name, props = {}, helpText = null) {
export function OptionalField(component, label, name, props = {}, helpText = null, invalidFeedback = null) {
return <Form.Group as={Col}>
<Form.Label>{label}</Form.Label>
<Form.Control
@@ -88,6 +92,7 @@ export function OptionalField(component, label, name, props = {}, helpText = nul
onChange={component.handleChange}
{...props} />
{helpText && <Form.Text className="text-muted">{helpText}</Form.Text>}
{invalidFeedback && <Form.Control.Feedback type="invalid">{invalidFeedback}</Form.Control.Feedback>}
</Form.Group>
}
@@ -130,6 +135,18 @@ export function OptionalNumberField(component, label, name, props = {}) {
</Form.Group>
}
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;

26
htmlui/src/tests/SetupSFTP.test.js
View File

@@ -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,
});

27
repo/blob/sftp/sftp_options.go
View File

@@ -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
}

108
repo/blob/sftp/sftp_storage.go
View File

@@ -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")
}

99
repo/blob/sftp/sftp_storage_test.go
View File

@@ -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)
}