diff --git a/sftp/common/e2e_test.go b/sftp/common/e2e_test.go new file mode 100644 index 00000000..ba01e053 --- /dev/null +++ b/sftp/common/e2e_test.go @@ -0,0 +1,366 @@ +package common + +import ( + "crypto/ed25519" + "crypto/rand" + "encoding/pem" + "fmt" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/pkg/sftp" + "golang.org/x/crypto/ssh" +) + +func TestConnectSuccess(t *testing.T) { + endpoint := &url.URL{ + Scheme: "sftp", + Host: fixture.host + ":" + fixture.port, + Path: "/", + User: url.User("testuser"), + } + + // Each test is done on both openssh and native mode + tests := []struct { + name string + params map[string]string + }{ + { + name: "with identity", + params: map[string]string{ + "identity": fixture.clientKeyPath, + "insecure_ignore_host_key": "true", + }, + }, + { + name: "with ssh_private_key", + params: map[string]string{ + "ssh_private_key": fixture.clientKeyPEM, + "insecure_ignore_host_key": "true", + }, + }, + } + modes := []string{"openssh", "native"} + + for _, tt := range tests { + for _, mode := range modes { + t.Run(mode+"-"+tt.name, func(t *testing.T) { + params := cloneParams(tt.params) + params["ssh_mode"] = mode + if mode == "openssh" { + t.Cleanup(func() { + stopOpenSSHMaster(t, endpoint, params) + }) + } + + client, err := Connect(endpoint, params) + if err != nil { + t.Fatalf("failed to connect: %v", err) + } + defer client.Close() + + if _, err := client.Getwd(); err != nil { + t.Fatalf("sftp session unusable: %v", err) + } + }) + } + } +} + +func TestConnectFailure(t *testing.T) { + + endpoint := &url.URL{ + Scheme: "sftp", + Host: fixture.host + ":" + fixture.port, + Path: "/", + User: url.User("testuser"), + } + + tests := []struct { + name string + params map[string]string + errorContains []string + }{ + { + name: "native with missing identity", + params: map[string]string{ + "ssh_mode": "native", + "identity": filepath.Join(fixture.dir, "missing_identity"), + "insecure_ignore_host_key": "true", + }, + errorContains: []string{"failed to read identity file"}, + }, + { + name: "native with invalid private key", + params: map[string]string{ + "ssh_mode": "native", + "ssh_private_key": "invalid", + "insecure_ignore_host_key": "true", + }, + errorContains: []string{"failed to parse ssh_private_key", "no key found"}, + }, + { + name: "openssh with invalid private key", + params: map[string]string{ + "ssh_mode": "openssh", + "ssh_private_key": "invalid", + "insecure_ignore_host_key": "true", + }, + errorContains: []string{"Error loading key", "invalid format"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, err := Connect(endpoint, tt.params) + if err == nil { + if client != nil { + client.Close() + } + t.Fatalf("expected error, got nil") + } + for _, errorContains := range tt.errorContains { + if !strings.Contains(err.Error(), errorContains) { + t.Fatalf("got error %q, want it to contain %q", err, errorContains) + } + } + if client != nil { + t.Fatalf("got %v, want nil", client) + } + }) + } +} + +func cloneParams(params map[string]string) map[string]string { + cloned := make(map[string]string, len(params)+1) + for k, v := range params { + cloned[k] = v + } + return cloned +} + +func stopOpenSSHMaster(t *testing.T, endpoint *url.URL, params map[string]string) { + t.Helper() + + sock, err := controlSock(endpoint, params) + if err != nil { + t.Logf("failed to resolve ssh control socket: %v", err) + return + } + + args := []string{"-S", sock, "-O", "exit"} + if p := endpoint.Port(); p != "" { + args = append(args, "-p", p) + } + if endpoint.User != nil { + args = append(args, "-l", endpoint.User.Username()) + } + args = append(args, endpoint.Hostname()) + + _ = exec.Command("ssh", args...).Run() +} + +// sftpFixture describes the shared, package-wide sftp server container. +type sftpFixture struct { + containerID string + host string // always 127.0.0.1 + port string // mapped host port for container port 22 + clientKeyPEM string // OpenSSH private key accepted by the server + clientKeyPath string // temp identity file accepted by OpenSSH + dir string // temp dir containing fixture files +} + +var fixture *sftpFixture + +// TestMain sets up the sftp fixture and runs the tests, reusing the same fixture over multiple tests. +func TestMain(m *testing.M) { + if _, err := exec.LookPath("docker"); err != nil { + fmt.Fprintln(os.Stderr, "docker not found in PATH; skipping docker-tagged tests") + os.Exit(0) + } + + f, err := startSFTPServer() + if err != nil { + fmt.Fprintf(os.Stderr, "failed to start sftp fixture: %v\n", err) + os.Exit(1) + } + fixture = f + + code := m.Run() + + f.stop() + os.Exit(code) +} + +// startSFTPServer generates an ephemeral client key, boots one atmoz/sftp +// container, and waits until the SFTP subsystem is reachable. +func startSFTPServer() (*sftpFixture, error) { + dir, err := os.MkdirTemp("", "sftp-fixture-") + if err != nil { + return nil, err + } + success := false + defer func() { + if !success { + _ = os.RemoveAll(dir) + } + }() + + // Client key pair: private key is fed to connectNativeSSH, public key is + // authorized on the server. + clientPEM, clientAuthorized, err := genKeyPair() + if err != nil { + return nil, err + } + clientKeyPath := filepath.Join(dir, "id_ed25519") + if err := os.WriteFile(clientKeyPath, []byte(clientPEM), 0600); err != nil { + return nil, err + } + + // atmoz/sftp copies every *.pub under /home//.ssh/keys into the + // user's authorized_keys at startup. + keysDir := filepath.Join(dir, "keys") + if err := os.MkdirAll(keysDir, 0755); err != nil { + return nil, err + } + if err := os.WriteFile(filepath.Join(keysDir, "client.pub"), []byte(clientAuthorized), 0644); err != nil { + return nil, err + } + + // Bind container port 22 to a random loopback port so parallel runs (and + // whatever is already using :22) never collide. + runOut, err := exec.Command("docker", "run", "-d", + "-p", "127.0.0.1::22", + "-v", keysDir+":/home/testuser/.ssh/keys:ro", + "atmoz/sftp", "testuser::1001", + ).Output() + if err != nil { + return nil, fmt.Errorf("docker run: %w", asExecErr(err)) + } + containerID := strings.TrimSpace(string(runOut)) + + f := &sftpFixture{ + containerID: containerID, + host: "127.0.0.1", + clientKeyPEM: clientPEM, + clientKeyPath: clientKeyPath, + dir: dir, + } + + port, err := dockerMappedPort(containerID, "22") + if err != nil { + f.stop() + return nil, err + } + f.port = port + + if err := waitForSSH(f, 60*time.Second); err != nil { + f.stop() + return nil, err + } + + success = true + return f, nil +} + +func (f *sftpFixture) stop() { + if f == nil { + return + } + if f.containerID != "" { + _ = exec.Command("docker", "rm", "-f", f.containerID).Run() + } + if f.dir != "" { + _ = os.RemoveAll(f.dir) + } +} + +// genKeyPair returns an OpenSSH-format private key (PEM) and its matching +// authorized_keys line, using ed25519. No external tooling required. +func genKeyPair() (privPEM string, authorizedKey string, err error) { + _, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return "", "", err + } + block, err := ssh.MarshalPrivateKey(priv, "") + if err != nil { + return "", "", err + } + signer, err := ssh.NewSignerFromKey(priv) + if err != nil { + return "", "", err + } + return string(pem.EncodeToMemory(block)), string(ssh.MarshalAuthorizedKey(signer.PublicKey())), nil +} + +// dockerMappedPort resolves the host port bound to the given container port. +func dockerMappedPort(containerID, containerPort string) (string, error) { + out, err := exec.Command("docker", "port", containerID, containerPort).Output() + if err != nil { + return "", fmt.Errorf("docker port: %w", asExecErr(err)) + } + // Output looks like "127.0.0.1:49153" (possibly multiple lines). + line := strings.TrimSpace(strings.SplitN(string(out), "\n", 2)[0]) + i := strings.LastIndex(line, ":") + if i < 0 { + return "", fmt.Errorf("unexpected docker port output: %q", string(out)) + } + return line[i+1:], nil +} + +// waitForSSH blocks until the server can serve a full SFTP session, not merely +// accept a TCP connection or complete an SSH handshake. +func waitForSSH(f *sftpFixture, timeout time.Duration) error { + signer, err := ssh.ParsePrivateKey([]byte(f.clientKeyPEM)) + if err != nil { + return err + } + cfg := &ssh.ClientConfig{ + User: "testuser", + Auth: []ssh.AuthMethod{ssh.PublicKeys(signer)}, + HostKeyCallback: ssh.InsecureIgnoreHostKey(), + Timeout: 3 * time.Second, + } + addr := f.host + ":" + f.port + deadline := time.Now().Add(timeout) + var lastErr error + for time.Now().Before(deadline) { + if err := trySFTP(addr, cfg); err == nil { + return nil + } else { + lastErr = err + } + time.Sleep(500 * time.Millisecond) + } + return fmt.Errorf("timed out waiting for sftp at %s: %w", addr, lastErr) +} + +// trySFTP performs one full SSH+SFTP handshake and tears it down. +func trySFTP(addr string, cfg *ssh.ClientConfig) error { + conn, err := ssh.Dial("tcp", addr, cfg) + if err != nil { + return err + } + defer conn.Close() + + client, err := sftp.NewClient(conn) + if err != nil { + return err + } + defer client.Close() + + _, err = client.Getwd() + return err +} + +func asExecErr(err error) error { + if ee, ok := err.(*exec.ExitError); ok { + return fmt.Errorf("%w: %s", err, strings.TrimSpace(string(ee.Stderr))) + } + return err +} diff --git a/sftp/common/native_ssh.go b/sftp/common/native_ssh.go new file mode 100644 index 00000000..1e4b7178 --- /dev/null +++ b/sftp/common/native_ssh.go @@ -0,0 +1,112 @@ +package common + +import ( + "fmt" + "net" + "net/url" + "os" + + "github.com/pkg/sftp" + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/agent" +) + +// nativeSFTPClient is a wrapper around *sftp.Client that ensures the ssh connection +// is closed when the sftp client is closed. +type nativeSFTPClient struct { + *sftp.Client + conn *ssh.Client +} + +func (c *nativeSFTPClient) Close() error { + err := c.Client.Close() + if cerr := c.conn.Close(); err == nil { + err = cerr + } + return err +} + +func buildAuthMethods(params map[string]string) ([]ssh.AuthMethod, error) { + var methods []ssh.AuthMethod + + if key := params["ssh_private_key"]; key != "" { + signer, err := ssh.ParsePrivateKey([]byte(key)) + if err != nil { + return nil, fmt.Errorf("failed to parse ssh_private_key: %w", err) + } + methods = append(methods, ssh.PublicKeys(signer)) + } + + if id := params["identity"]; id != "" { + data, err := os.ReadFile(id) + if err != nil { + return nil, fmt.Errorf("failed to read identity file: %w", err) + } + signer, err := ssh.ParsePrivateKey(data) + if err != nil { + return nil, fmt.Errorf("failed to parse identity file: %w", err) + } + methods = append(methods, ssh.PublicKeys(signer)) + } + + if sock := params["ssh_auth_sock"]; sock != "" { + if conn, err := net.Dial("unix", sock); err == nil { + ag := agent.NewClient(conn) + methods = append(methods, ssh.PublicKeysCallback(ag.Signers)) + } + } + + if len(methods) == 0 { + return nil, fmt.Errorf("no authentication method available (set identity, ssh_private_key, or ssh_auth_sock)") + } + + return methods, nil +} + +func hostKeyCallback(params map[string]string) (ssh.HostKeyCallback, error) { + return ssh.InsecureIgnoreHostKey(), nil +} + +// connectNativeSSH connects to the remote server using the native SSH client +func connectNativeSSH(endpoint *url.URL, params map[string]string) (*sftp.Client, error) { + user, err := resolveUsername(endpoint, params) + if err != nil { + return nil, err + } + + auth, err := buildAuthMethods(params) + if err != nil { + return nil, err + } + + hkcb, err := hostKeyCallback(params) + if err != nil { + return nil, err + } + + cfg := &ssh.ClientConfig{ + User: user, + Auth: auth, + HostKeyCallback: hkcb, + } + + addr := endpoint.Hostname() + if p := endpoint.Port(); p != "" { + addr += ":" + p + } else { + addr += ":22" + } + + conn, err := ssh.Dial("tcp", addr, cfg) + if err != nil { + return nil, fmt.Errorf("ssh dial: %w", err) + } + + client, err := sftp.NewClient(conn) + if err != nil { + conn.Close() + return nil, err + } + + return client, nil +} diff --git a/sftp/common/native_ssh_test.go b/sftp/common/native_ssh_test.go new file mode 100644 index 00000000..1a9e3987 --- /dev/null +++ b/sftp/common/native_ssh_test.go @@ -0,0 +1,59 @@ +package common + +import ( + "net/url" + "testing" +) + +var testSSHCert = ` +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACCeF/EJkMysMrz2pWZfs95OtIqbDkz5jJHdXEI2aQ+ZQAAAALBS9q/GUvav +xgAAAAtzc2gtZWQyNTUxOQAAACCeF/EJkMysMrz2pWZfs95OtIqbDkz5jJHdXEI2aQ+ZQA +AAAEChwRRqrpme6kwm/PVrr7AmODBU2ZpcMy0eLmOJn6EdpJ4X8QmQzKwyvPalZl+z3k60 +ipsOTPmMkd1cQjZpD5lAAAAAKXBhdWxvb3N0ZW5yaWprQE1hY0Jvb2stUHJvLXZhbi1QYX +VsLmxvY2FsAQIDBA== +-----END OPENSSH PRIVATE KEY----- +` + +func TestBuildAuthMethodsSuccess(t *testing.T) { + got, err := buildAuthMethods(map[string]string{ + "ssh_private_key": testSSHCert, + }) + if err != nil { + t.Fatalf("failed to build auth methods: %v", err) + } + if len(got) != 1 { + t.Fatalf("got %d auth methods, want 1", len(got)) + } +} + +func TestBuildAuthMethodsFailure(t *testing.T) { + got, err := buildAuthMethods(map[string]string{ + "ssh_private_key": "invalid", + }) + if err == nil { + t.Fatalf("expected error, got nil") + } + if got != nil { + t.Fatalf("got %v, want nil", got) + } +} + +func TestConnectNativeSSHWithoutUsernameFailure(t *testing.T) { + endpoint := &url.URL{ + Scheme: "sftp", + Host: "localhost", + Path: "/", + User: nil, + } + client, err := connectNativeSSH(endpoint, map[string]string{ + "ssh_mode": "native", + }) + if err == nil { + t.Fatalf("expected error, got nil") + } + if client != nil { + t.Fatalf("got %v, want nil", client) + } +} diff --git a/sftp/common/sftp.go b/sftp/common/sftp.go index dd1222c1..229094cc 100644 --- a/sftp/common/sftp.go +++ b/sftp/common/sftp.go @@ -9,12 +9,44 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "sync" "github.com/pkg/sftp" ) +func Connect(endpoint *url.URL, params map[string]string) (*sftp.Client, error) { + if endpoint == nil { + return nil, fmt.Errorf("nil endpoint") + } + if endpoint.Hostname() == "" { + return nil, fmt.Errorf("missing hostname in endpoint: %q", endpoint.String()) + } + + switch mode := resolveSSHMode(params); mode { + case "openssh": + if runtime.GOOS == "windows" { + return nil, fmt.Errorf("ssh_mode=openssh is not supported on windows") + } + return connectOpenSSH(endpoint, params) + case "native": + return connectNativeSSH(endpoint, params) + default: + return nil, fmt.Errorf("unknown ssh_mode %q (want %q or %q)", mode, "openssh", "native") + } +} + +func resolveSSHMode(params map[string]string) string { + if m := params["ssh_mode"]; m != "" { + return m + } + if runtime.GOOS == "windows" { + return "native" + } + return "openssh" +} + func controlSock(endpoint *url.URL, params map[string]string) (string, error) { if endpoint == nil { return "", fmt.Errorf("nil endpoint") @@ -27,6 +59,7 @@ func controlSock(endpoint *url.URL, params map[string]string) (string, error) { // guard master creation per ControlPath var masterMu sync.Map // map[string]*sync.Mutex + func lockFor(sock string) *sync.Mutex { m, _ := masterMu.LoadOrStore(sock, &sync.Mutex{}) return m.(*sync.Mutex) @@ -176,7 +209,7 @@ func ensureMaster(endpoint *url.URL, params map[string]string) (string, error) { return sock, nil } -func Connect(endpoint *url.URL, params map[string]string) (*sftp.Client, error) { +func connectOpenSSH(endpoint *url.URL, params map[string]string) (*sftp.Client, error) { if endpoint == nil { return nil, fmt.Errorf("nil endpoint") } @@ -204,15 +237,13 @@ func Connect(endpoint *url.URL, params map[string]string) (*sftp.Client, error) args = append(args, "-i", id) } - // username resolution: forbid both user@host AND username param - if endpoint.User != nil && params["username"] != "" { - return nil, fmt.Errorf("can not use user@host foo syntax and username parameter") - } else if endpoint.User != nil { - args = append(args, "-l", endpoint.User.Username()) - } else if params["username"] != "" { - args = append(args, "-l", params["username"]) + username, err := resolveUsername(endpoint, params) + if err != nil { + return nil, err } + args = append(args, "-l", username) + if p := endpoint.Port(); p != "" { args = append(args, "-p", p) } @@ -267,3 +298,13 @@ func Connect(endpoint *url.URL, params map[string]string) (*sftp.Client, error) return client, nil } + +func resolveUsername(endpoint *url.URL, params map[string]string) (string, error) { + if endpoint.User != nil && params["username"] != "" { + return "", fmt.Errorf("can not use user@host syntax and username parameter") + } + if endpoint.User != nil { + return endpoint.User.Username(), nil + } + return params["username"], nil +} diff --git a/sftp/go.mod b/sftp/go.mod index df7dbcb1..19302dfb 100644 --- a/sftp/go.mod +++ b/sftp/go.mod @@ -6,6 +6,7 @@ require ( github.com/PlakarKorp/go-kloset-sdk v1.1.0 github.com/PlakarKorp/kloset v1.1.0 github.com/pkg/sftp v1.13.9 + golang.org/x/crypto v0.52.0 golang.org/x/sync v0.20.0 ) @@ -29,7 +30,6 @@ require ( github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect github.com/zeebo/blake3 v0.2.4 // indirect - golang.org/x/crypto v0.52.0 // indirect golang.org/x/mod v0.36.0 // indirect golang.org/x/net v0.54.0 // indirect golang.org/x/sys v0.45.0 // indirect