From 7163fde72453ca7d90cc64a7fe879752d3b1c92e Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 29 Feb 2020 00:02:06 +0100 Subject: [PATCH] proxy protocol: added an option to make the proxy header required now we can configure SFTPGo to accept or reject requests without the proxy header when the proxy protocol is enabled --- README.md | 7 ++++-- config/config.go | 9 +++++++- config/config_test.go | 21 ++++++++++++++++++ fail2ban/filters/sftpgo.conf | 2 +- sftpd/internal_test.go | 15 +++++++++++++ sftpd/server.go | 41 +++++++++++++++++++++++++----------- sftpd/sftpd_test.go | 15 +++++++++++++ 7 files changed, 94 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 64481784..ef0e12f4 100644 --- a/README.md +++ b/README.md @@ -160,7 +160,10 @@ The `sftpgo` configuration file contains the following sections: - `git-receive-pack`, `git-upload-pack`, `git-upload-archive`. These commands enable support for Git repositories over SSH, they need to be installed and in your system's `PATH`. Git commands are not allowed inside virtual folders. - `rsync`. The `rsync` command need to be installed and in your system's `PATH`. We cannot avoid that rsync create symlinks so if the user has the permission to create symlinks we add the option `--safe-links` to the received rsync command if it is not already set. This should prevent to create symlinks that point outside the home dir. If the user cannot create symlinks we add the option `--munge-links`, if it is not already set. This should make symlinks unusable (but manually recoverable). The `rsync` command interacts with the filesystem directly and it is not aware about virtual folders, so it will be automatically disabled for users with virtual folders. - `keyboard_interactive_auth_program`, string. Absolute path to an external program to use for keyboard interactive authentication. See the "Keyboard Interactive Authentication" paragraph for more details. - - `proxy_protocol`, integer. Support for [HAProxy PROXY protocol](https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt). If you are running SFTPGo behind a proxy server such as HAProxy, AWS ELB or NGNIX, you can enable the proxy protocol. It provides a convenient way to safely transport connection information such as a client's address across multiple layers of NAT or TCP proxies to get the real client IP address instead of the proxy IP. Set to 1 to enable proxy protocol, default 0. You have to enable the protocol in your proxy configuration too, for example for HAProxy add `send-proxy` or `send-proxy-v2` to each server configuration line + - `proxy_protocol`, integer. Support for [HAProxy PROXY protocol](https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt). If you are running SFTPGo behind a proxy server such as HAProxy, AWS ELB or NGNIX, you can enable the proxy protocol. It provides a convenient way to safely transport connection information such as a client's address across multiple layers of NAT or TCP proxies to get the real client IP address instead of the proxy IP. Both protocol v1 and v2 are supported. If the proxy protocol is enabled in SFTPGo then you have to enable the protocol in your proxy configuration too, for example for HAProxy add `send-proxy` or `send-proxy-v2` to each server configuration line. The following modes are supported: + - 0, disabled + - 1, enabled. Proxy header will be used and requests without proxy header will be accepted + - 2, required. Proxy header will be used and requests without proxy header will be rejected - **"data_provider"**, the configuration for the data provider - `driver`, string. Supported drivers are `sqlite`, `mysql`, `postgresql`, `bolt`, `memory` - `name`, string. Database name. For driver `sqlite` this can be the database name relative to the config dir or the absolute path to the SQLite database. For driver `memory` this is the (optional) path relative to the config dir or the absolute path to the users dump to load. @@ -888,7 +891,7 @@ The **connection failed logs** can be used for integration in tools such as [Fai SFTPGo can easily saturate a Gigabit connection, on low end hardware, with no special configurations and this is generally enough for most use cases. -The main bootlenecks are the encryption and the messages authentication, so if you can use a fast cipher with implicit message authentication, for example `aes128-gcm@openssh.com`, you will get a big performace boost. +The main bootlenecks are the encryption and the messages authentication, so if you can use a fast cipher with implicit message authentication, for example `aes128-gcm@openssh.com`, you will get a big performance boost. There is an open [issue](https://github.com/drakkan/sftpgo/issues/69) with some other suggestions to improve performance and some comparisons against OpenSSH. diff --git a/config/config.go b/config/config.go index aaaf6678..0a5196c1 100644 --- a/config/config.go +++ b/config/config.go @@ -172,12 +172,19 @@ func LoadConfig(configDir, configName string) error { globalConf.SFTPD.Banner = defaultBanner } if globalConf.SFTPD.UploadMode < 0 || globalConf.SFTPD.UploadMode > 2 { - err = fmt.Errorf("invalid upload_mode 0 and 1 are supported, configured: %v reset upload_mode to 0", + err = fmt.Errorf("invalid upload_mode 0, 1 and 2 are supported, configured: %v reset upload_mode to 0", globalConf.SFTPD.UploadMode) globalConf.SFTPD.UploadMode = 0 logger.Warn(logSender, "", "Configuration error: %v", err) logger.WarnToConsole("Configuration error: %v", err) } + if globalConf.SFTPD.ProxyProtocol < 0 || globalConf.SFTPD.ProxyProtocol > 2 { + err = fmt.Errorf("invalid proxy_protocol 0, 1 and 2 are supported, configured: %v reset proxy_protocol to 0", + globalConf.SFTPD.ProxyProtocol) + globalConf.SFTPD.ProxyProtocol = 0 + logger.Warn(logSender, "", "Configuration error: %v", err) + logger.WarnToConsole("Configuration error: %v", err) + } if globalConf.ProviderConf.ExternalAuthScope < 0 || globalConf.ProviderConf.ExternalAuthScope > 7 { err = fmt.Errorf("invalid external_auth_scope: %v reset to 0", globalConf.ProviderConf.ExternalAuthScope) globalConf.ProviderConf.ExternalAuthScope = 0 diff --git a/config/config_test.go b/config/config_test.go index e60c69ad..ff154a10 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -140,6 +140,27 @@ func TestInvalidCredentialsPath(t *testing.T) { os.Remove(configFilePath) } +func TestInvalidProxyProtocol(t *testing.T) { + configDir := ".." + confName := tempConfigName + ".json" + configFilePath := filepath.Join(configDir, confName) + config.LoadConfig(configDir, "") + sftpdConf := config.GetSFTPDConfig() + sftpdConf.ProxyProtocol = 10 + c := make(map[string]sftpd.Configuration) + c["sftpd"] = sftpdConf + jsonConf, _ := json.Marshal(c) + err := ioutil.WriteFile(configFilePath, jsonConf, 0666) + if err != nil { + t.Errorf("error saving temporary configuration") + } + err = config.LoadConfig(configDir, tempConfigName) + if err == nil { + t.Errorf("Loading configuration with invalid proxy_protocol must fail") + } + os.Remove(configFilePath) +} + func TestSetGetConfig(t *testing.T) { sftpdConf := config.GetSFTPDConfig() sftpdConf.IdleTimeout = 3 diff --git a/fail2ban/filters/sftpgo.conf b/fail2ban/filters/sftpgo.conf index ddf7a270..a8521b16 100644 --- a/fail2ban/filters/sftpgo.conf +++ b/fail2ban/filters/sftpgo.conf @@ -6,7 +6,7 @@ _daemon = sftpgo [Definition] -# By default, first authenticate method is public_key and must be excluded from the filter to avoid false positives failed attemps +# By default, first authenticate method is public_key and must be excluded from the filter to avoid false positives failed attempts failregex = ^.*"sender":"connection_failed","client_ip":"","username":".*","login_type":"password".*"}$ ignoreregex = diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index f6f7a63f..323c597e 100644 --- a/sftpd/internal_test.go +++ b/sftpd/internal_test.go @@ -1709,3 +1709,18 @@ func TestSFTPExtensions(t *testing.T) { } sftpExtensions = initialSFTPExtensions } + +func TestProxyProtocolVersion(t *testing.T) { + c := Configuration{ + ProxyProtocol: 1, + } + proxyListener := c.getProxyListener(nil) + if proxyListener.Policy != nil { + t.Error("proxy listener policy must be nil") + } + c.ProxyProtocol = 2 + proxyListener = c.getProxyListener(nil) + if proxyListener.Policy == nil { + t.Error("proxy listener policy must be not nil") + } +} diff --git a/sftpd/server.go b/sftpd/server.go index 3e37c519..ad9a22d3 100644 --- a/sftpd/server.go +++ b/sftpd/server.go @@ -3,6 +3,7 @@ package sftpd import ( "encoding/hex" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -26,7 +27,10 @@ const ( defaultPrivateECDSAKeyName = "id_ecdsa" ) -var sftpExtensions = []string{"posix-rename@openssh.com"} +var ( + sftpExtensions = []string{"posix-rename@openssh.com"} + errWrongProxyProtoVersion = errors.New("unacceptable proxy protocol version") +) // Configuration for the SFTP server type Configuration struct { @@ -104,10 +108,12 @@ type Configuration struct { // If you are running SFTPGo behind a proxy server such as HAProxy, AWS ELB or NGNIX, you can enable // the proxy protocol. It provides a convenient way to safely transport connection information // such as a client's address across multiple layers of NAT or TCP proxies to get the real - // client IP address instead of the proxy IP. - // Set to 1 to enable proxy protocol. - // You have to enable the protocol in your proxy configuration too, for example for HAProxy - // add "send-proxy" or "send-proxy-v2" to each server configuration line + // client IP address instead of the proxy IP. Both protocol v1 and v2 are supported. + // - 0 means disabled + // - 1 means proxy protocol enabled. Proxy header will be used and requests without proxy header will be accepted. + // - 2 means proxy protocol required. Proxy header will be used and requests without proxy header will be rejected. + // If the proxy protocol is enabled in SFTPGo then you have to enable the protocol in your proxy configuration too, + // for example for HAProxy add "send-proxy" or "send-proxy-v2" to each server configuration line. ProxyProtocol int `json:"proxy_protocol" mapstructure:"proxy_protocol"` } @@ -193,13 +199,7 @@ func (c Configuration) Initialize(configDir string) error { logger.Warn(logSender, "", "error starting listener on address %s:%d: %v", c.BindAddress, c.BindPort, err) return err } - var proxyListener *proxyproto.Listener - if c.ProxyProtocol == 1 { - proxyListener = &proxyproto.Listener{ - Listener: listener, - } - } - + proxyListener := c.getProxyListener(listener) actions = c.Actions uploadMode = c.UploadMode setstatMode = c.SetstatMode @@ -219,6 +219,23 @@ func (c Configuration) Initialize(configDir string) error { } } +func (c *Configuration) getProxyListener(listener net.Listener) *proxyproto.Listener { + var proxyListener *proxyproto.Listener + if c.ProxyProtocol > 0 { + var policyFunc func(upstream net.Addr) (proxyproto.Policy, error) + if c.ProxyProtocol == 2 { + policyFunc = func(upstream net.Addr) (proxyproto.Policy, error) { + return proxyproto.REQUIRE, nil + } + } + proxyListener = &proxyproto.Listener{ + Listener: listener, + Policy: policyFunc, + } + } + return proxyListener +} + func (c Configuration) checkIdleTimer() { if c.IdleTimeout > 0 { startIdleTimer(time.Duration(c.IdleTimeout) * time.Minute) diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index db71ab33..c2f8feb3 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -216,6 +216,17 @@ func TestMain(m *testing.M) { waitTCPListening(fmt.Sprintf("%s:%d", sftpdConf.BindAddress, sftpdConf.BindPort)) + sftpdConf.BindPort = 2224 + sftpdConf.ProxyProtocol = 2 + go func() { + logger.Debug(logSender, "", "initializing SFTP server with config %+v", sftpdConf) + if err := sftpdConf.Initialize(configDir); err != nil { + logger.Error(logSender, "", "could not start SFTP server: %v", err) + } + }() + + waitTCPListening(fmt.Sprintf("%s:%d", sftpdConf.BindAddress, sftpdConf.BindPort)) + exitCode := m.Run() os.Remove(logFilePath) os.Remove(loginBannerFile) @@ -341,6 +352,10 @@ func TestProxyProtocol(t *testing.T) { t.Errorf("error mkdir: %v", err) } } + client, err = getSftpClientWithAddr(user, usePubKey, "127.0.0.1:2224") + if err == nil { + t.Error("request without a proxy header must be rejected") + } httpd.RemoveUser(user, http.StatusOK) os.RemoveAll(user.GetHomeDir()) }