From 15137d032704c31a087d38b904b87aa76f2f72a2 Mon Sep 17 00:00:00 2001 From: Sveinn Date: Wed, 8 Nov 2023 17:47:05 +0000 Subject: [PATCH] refactor SFTP to use the new minio/pkg implementation (#18406) --- cmd/ftp-server.go | 196 --------------------------------------------- cmd/sftp-server.go | 172 +++++++++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 4 +- 4 files changed, 175 insertions(+), 199 deletions(-) create mode 100644 cmd/sftp-server.go diff --git a/cmd/ftp-server.go b/cmd/ftp-server.go index 245ee6dd6..66b23064e 100644 --- a/cmd/ftp-server.go +++ b/cmd/ftp-server.go @@ -18,20 +18,14 @@ package cmd import ( - "context" - "crypto/subtle" "fmt" "net" - "os" "strconv" "strings" - "time" "github.com/minio/cli" "github.com/minio/minio/internal/logger" - "github.com/pkg/sftp" ftp "goftp.io/server/v2" - "golang.org/x/crypto/ssh" ) var globalRemoteFTPClientTransport = NewRemoteTargetHTTPTransport(true)() @@ -75,196 +69,6 @@ func (log *minioLogger) PrintResponse(sessionID string, code int, message string } } -func acceptSFTPConnection(conn net.Conn, config *ssh.ServerConfig) { - // For SSH handshake keep a 2 minute deadline, as OpensSSH default. - conn.SetDeadline(time.Now().Add(2 * time.Minute)) - - // Before use, a handshake must be performed on the incoming net.Conn. - sconn, chans, reqs, err := ssh.NewServerConn(conn, config) - if err != nil { - return - } - - // Once we are done with SSH handshake, remove deadline. - conn.SetDeadline(time.Time{}) - - // The incoming Request channel must be serviced. - go ssh.DiscardRequests(reqs) - - // Service the incoming Channel channel. - for newChannel := range chans { - // Channels have a type, depending on the application level - // protocol intended. In the case of an SFTP session, this is "subsystem" - // with a payload string of "sftp" - if newChannel.ChannelType() != "session" { - newChannel.Reject(ssh.UnknownChannelType, "unknown channel type") - continue - } - - channel, requests, err := newChannel.Accept() - if err != nil { - logger.LogOnceIf(context.Background(), fmt.Errorf("unable to accept the connection requests channel: %w", err), "accept-channel-sftp") - continue - } - - // Sessions have out-of-band requests such as "shell", - // "pty-req" and "env". Here we handle only the - // "subsystem" request. - go func(in <-chan *ssh.Request) { - for req := range in { - ok := false - if req.Type == "subsystem" { - if len(req.Payload) > 4 && string(req.Payload[4:]) == "sftp" { - ok = true - go handleSFTPConnection(channel, sconn) - } - } - - if req.WantReply { - // We only reply to SSH packets that have `sftp` payload, all other - // packets are rejected - req.Reply(ok, nil) - } - } - }(requests) - } -} - -func handleSFTPConnection(channel ssh.Channel, sconn *ssh.ServerConn) { - // Create the server instance for the channel using the handler we created above. - server := sftp.NewRequestServer(channel, NewSFTPDriver(sconn.Permissions), sftp.WithRSAllocator()) - defer server.Close() - - server.Serve() -} - -func startSFTPServer(c *cli.Context) { - args := c.StringSlice("sftp") - - var ( - port int - publicIP string - sshPrivateKey string - ) - - var err error - for _, arg := range args { - tokens := strings.SplitN(arg, "=", 2) - if len(tokens) != 2 { - logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s", arg), "unable to start SFTP server") - } - switch tokens[0] { - case "address": - host, portStr, err := net.SplitHostPort(tokens[1]) - if err != nil { - logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s (%v)", arg, err), "unable to start SFTP server") - } - port, err = strconv.Atoi(portStr) - if err != nil { - logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s (%v)", arg, err), "unable to start SFTP server") - } - if port < 1 || port > 65535 { - logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s, (port number must be between 1 to 65535)", arg), "unable to start SFTP server") - } - publicIP = host - case "ssh-private-key": - sshPrivateKey = tokens[1] - } - } - - if port == 0 { - port = 8022 // Default SFTP port, since no port was given. - } - - if sshPrivateKey == "" { - logger.Fatal(fmt.Errorf("invalid arguments passed, private key file is mandatory for --sftp='ssh-private-key=path/to/id_ecdsa'"), "unable to start SFTP server") - } - - privateBytes, err := os.ReadFile(sshPrivateKey) - if err != nil { - logger.Fatal(fmt.Errorf("invalid arguments passed, private key file is not accessible: %v", err), "unable to start SFTP server") - } - - private, err := ssh.ParsePrivateKey(privateBytes) - if err != nil { - logger.Fatal(fmt.Errorf("invalid arguments passed, private key file is not parseable: %v", err), "unable to start SFTP server") - } - - // An SSH server is represented by a ServerConfig, which holds - // certificate details and handles authentication of ServerConns. - config := &ssh.ServerConfig{ - PasswordCallback: func(c ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { - if globalIAMSys.LDAPConfig.Enabled() { - targetUser, targetGroups, err := globalIAMSys.LDAPConfig.Bind(c.User(), string(pass)) - if err != nil { - return nil, err - } - ldapPolicies, _ := globalIAMSys.PolicyDBGet(targetUser, false, targetGroups...) - if len(ldapPolicies) == 0 { - return nil, errAuthentication - } - return &ssh.Permissions{ - CriticalOptions: map[string]string{ - ldapUser: targetUser, - ldapUserN: c.User(), - }, - Extensions: make(map[string]string), - }, nil - } - - ui, ok := globalIAMSys.GetUser(context.Background(), c.User()) - if !ok { - return nil, errNoSuchUser - } - - if subtle.ConstantTimeCompare([]byte(ui.Credentials.SecretKey), pass) == 1 { - return &ssh.Permissions{ - CriticalOptions: map[string]string{ - "accessKey": c.User(), - }, - Extensions: make(map[string]string), - }, nil - } - return nil, errAuthentication - }, - } - - config.AddHostKey(private) - - // Once a ServerConfig has been configured, connections can be accepted. - listener, err := net.Listen("tcp", net.JoinHostPort(publicIP, strconv.Itoa(port))) - if err != nil { - logger.Fatal(err, "unable to start listening on --sftp='port=%d'", port) - } - - logger.Info(fmt.Sprintf("MinIO SFTP Server listening on %s", net.JoinHostPort(publicIP, strconv.Itoa(port)))) - - var tempDelay time.Duration // how long to sleep on accept failure - - for { - conn, err := listener.Accept() - if err != nil { - // From https://github.com/golang/go/blob/4aa1efed4853ea067d665a952eee77c52faac774/src/net/http/server.go#L3046 - if ne, ok := err.(net.Error); ok && ne.Temporary() { - if tempDelay == 0 { - tempDelay = 5 * time.Millisecond - } else { - tempDelay *= 2 - } - if max := 1 * time.Second; tempDelay > max { - tempDelay = max - } - logger.LogOnceIf(context.Background(), fmt.Errorf("error while accepting connections: %w, retrying in %s", err, tempDelay), "accept-limit-sftp") - time.Sleep(tempDelay) - continue - } - logger.Fatal(err, "unrecoverable error while accepting new connections") - } - - go acceptSFTPConnection(conn, config) - } -} - func startFTPServer(c *cli.Context) { args := c.StringSlice("ftp") diff --git a/cmd/sftp-server.go b/cmd/sftp-server.go new file mode 100644 index 000000000..56bd78dd3 --- /dev/null +++ b/cmd/sftp-server.go @@ -0,0 +1,172 @@ +// Copyright (c) 2015-2023 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "context" + "crypto/subtle" + "fmt" + "net" + "os" + "strconv" + "strings" + "time" + + "github.com/minio/cli" + "github.com/minio/minio/internal/logger" + xsftp "github.com/minio/pkg/v2/sftp" + "github.com/pkg/sftp" + "golang.org/x/crypto/ssh" +) + +type sftpLogger struct{} + +func (s *sftpLogger) Info(tag xsftp.LogType, msg string) { + logger.Info(msg) +} + +func (s *sftpLogger) Error(tag xsftp.LogType, err error) { + switch tag { + case xsftp.AcceptNetworkError: + logger.LogOnceIf(context.Background(), err, "accept-limit-sftp") + case xsftp.AcceptChannelError: + logger.LogOnceIf(context.Background(), err, "accept-channel-sftp") + case xsftp.SSHKeyExchangeError: + logger.LogOnceIf(context.Background(), err, "key-exchange-sftp") + default: + logger.LogOnceIf(context.Background(), err, "unknown-error-sftp") + } +} + +func startSFTPServer(c *cli.Context) { + args := c.StringSlice("sftp") + + var ( + port int + publicIP string + sshPrivateKey string + ) + + var err error + for _, arg := range args { + tokens := strings.SplitN(arg, "=", 2) + if len(tokens) != 2 { + logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s", arg), "unable to start SFTP server") + } + switch tokens[0] { + case "address": + host, portStr, err := net.SplitHostPort(tokens[1]) + if err != nil { + logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s (%v)", arg, err), "unable to start SFTP server") + } + port, err = strconv.Atoi(portStr) + if err != nil { + logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s (%v)", arg, err), "unable to start SFTP server") + } + if port < 1 || port > 65535 { + logger.Fatal(fmt.Errorf("invalid arguments passed to --sftp=%s, (port number must be between 1 to 65535)", arg), "unable to start SFTP server") + } + publicIP = host + case "ssh-private-key": + sshPrivateKey = tokens[1] + } + } + + if port == 0 { + port = 8022 // Default SFTP port, since no port was given. + } + + if sshPrivateKey == "" { + logger.Fatal(fmt.Errorf("invalid arguments passed, private key file is mandatory for --sftp='ssh-private-key=path/to/id_ecdsa'"), "unable to start SFTP server") + } + + privateBytes, err := os.ReadFile(sshPrivateKey) + if err != nil { + logger.Fatal(fmt.Errorf("invalid arguments passed, private key file is not accessible: %v", err), "unable to start SFTP server") + } + + private, err := ssh.ParsePrivateKey(privateBytes) + if err != nil { + logger.Fatal(fmt.Errorf("invalid arguments passed, private key file is not parseable: %v", err), "unable to start SFTP server") + } + + // An SSH server is represented by a ServerConfig, which holds + // certificate details and handles authentication of ServerConns. + sshConfig := &ssh.ServerConfig{ + PasswordCallback: func(c ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { + if globalIAMSys.LDAPConfig.Enabled() { + targetUser, targetGroups, err := globalIAMSys.LDAPConfig.Bind(c.User(), string(pass)) + if err != nil { + return nil, err + } + ldapPolicies, _ := globalIAMSys.PolicyDBGet(targetUser, false, targetGroups...) + if len(ldapPolicies) == 0 { + return nil, errAuthentication + } + return &ssh.Permissions{ + CriticalOptions: map[string]string{ + ldapUser: targetUser, + ldapUserN: c.User(), + }, + Extensions: make(map[string]string), + }, nil + } + + ui, ok := globalIAMSys.GetUser(context.Background(), c.User()) + if !ok { + return nil, errNoSuchUser + } + + if subtle.ConstantTimeCompare([]byte(ui.Credentials.SecretKey), pass) == 1 { + return &ssh.Permissions{ + CriticalOptions: map[string]string{ + "accessKey": c.User(), + }, + Extensions: make(map[string]string), + }, nil + } + return nil, errAuthentication + }, + } + + sshConfig.AddHostKey(private) + + handleSFTPSession := func(channel ssh.Channel, sconn *ssh.ServerConn) { + server := sftp.NewRequestServer(channel, NewSFTPDriver(sconn.Permissions), sftp.WithRSAllocator()) + defer server.Close() + server.Serve() + } + + sftpServer, err := xsftp.NewServer(&xsftp.Options{ + PublicIP: publicIP, + Port: port, + // OpensSSH default handshake timeout is 2 minutes. + SSHHandshakeDeadline: 2 * time.Minute, + Logger: new(sftpLogger), + SSHConfig: sshConfig, + HandleSFTPSession: handleSFTPSession, + }) + if err != nil { + logger.Fatal(err, "Unable to start SFTP Server") + } + + err = sftpServer.Listen() + if err != nil { + logger.Fatal(err, "SFTP Server had an unrecoverable error while accepting connections") + } +} diff --git a/go.mod b/go.mod index 4473478de..66280a181 100644 --- a/go.mod +++ b/go.mod @@ -51,7 +51,7 @@ require ( github.com/minio/madmin-go/v3 v3.0.29 github.com/minio/minio-go/v7 v7.0.64-0.20230920204636-e783c9ba11b3 github.com/minio/mux v1.9.0 - github.com/minio/pkg/v2 v2.0.2 + github.com/minio/pkg/v2 v2.0.3-0.20231107172951-8a60b89ec9b4 github.com/minio/selfupdate v0.6.0 github.com/minio/sha256-simd v1.0.1 github.com/minio/simdjson-go v0.4.5 diff --git a/go.sum b/go.sum index bbe5364d0..4ef465340 100644 --- a/go.sum +++ b/go.sum @@ -491,8 +491,8 @@ github.com/minio/mux v1.9.0 h1:dWafQFyEfGhJvK6AwLOt83bIG5bxKxKJnKMCi0XAaoA= github.com/minio/mux v1.9.0/go.mod h1:1pAare17ZRL5GpmNL+9YmqHoWnLmMZF9C/ioUCfy0BQ= github.com/minio/pkg v1.7.5 h1:UOUJjewE5zoaDPlCMJtNx/swc1jT1ZR+IajT7hrLd44= github.com/minio/pkg v1.7.5/go.mod h1:mEfGMTm5Z0b5EGxKNuPwyb5A2d+CC/VlUyRj6RJtIwo= -github.com/minio/pkg/v2 v2.0.2 h1:cytXmC21fBNS+0NVKEE5FuYmQfY+HFTqis6Kkj3U9ac= -github.com/minio/pkg/v2 v2.0.2/go.mod h1:6xTAr5M9yobpUroXAAaTrGJ9fhOZIqKYOT0I87u2yZ4= +github.com/minio/pkg/v2 v2.0.3-0.20231107172951-8a60b89ec9b4 h1:5eHjHtFZrrCQ3eO0sesXomdAUTtcGh0Fpp7Qa6dtjrY= +github.com/minio/pkg/v2 v2.0.3-0.20231107172951-8a60b89ec9b4/go.mod h1:6xTAr5M9yobpUroXAAaTrGJ9fhOZIqKYOT0I87u2yZ4= github.com/minio/selfupdate v0.6.0 h1:i76PgT0K5xO9+hjzKcacQtO7+MjJ4JKA8Ak8XQ9DDwU= github.com/minio/selfupdate v0.6.0/go.mod h1:bO02GTIPCMQFTEvE5h4DjYB58bCoZ35XLeBf0buTDdM= github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=