From 40e759c983add3253daf8fb2ece5de6b9c94ed8b Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Tue, 29 Dec 2020 09:20:09 +0100 Subject: [PATCH] FTP: add support for client certificate authentication --- README.md | 1 + config/config.go | 11 +++++++++++ config/config_test.go | 13 +++++++++---- docs/full-configuration.md | 4 +++- docs/webdav.md | 2 +- ftpd/ftpd.go | 8 ++++++++ ftpd/ftpd_test.go | 8 ++++++++ ftpd/internal_test.go | 10 ++++++++++ ftpd/server.go | 9 +++++++-- sftpgo.json | 6 ++++-- 10 files changed, 62 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 81dcf5ea..c19afa03 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,7 @@ Several storage backends are supported: local filesystem, encrypted local filesy - SCP and rsync are supported. - FTP/S is supported. You can configure the FTP service to require TLS for both control and data connections. - [WebDAV](./docs/webdav.md) is supported. +- Two-Way TLS authentication, aka TLS with client certificate authentication, is supported for FTPS and WebDAV over HTTPS. - Support for serving local filesystem, encrypted local filesystem, S3 Compatible Object Storage, Google Cloud Storage, Azure Blob Storage or other SFTP accounts over SFTP/SCP/FTP/WebDAV. - Per user protocols restrictions. You can configure the allowed protocols (SSH/FTP/WebDAV) for each user. - [Prometheus metrics](./docs/metrics.md) are exposed. diff --git a/config/config.go b/config/config.go index 22509c56..013e2aef 100644 --- a/config/config.go +++ b/config/config.go @@ -47,6 +47,9 @@ var ( Address: "", Port: 0, ApplyProxyConfig: true, + TLSMode: 0, + ForcePassiveIP: "", + ClientAuthType: 0, } defaultWebDAVDBinding = webdavd.Binding{ Address: "", @@ -120,6 +123,7 @@ func Init() { CombineSupport: 0, CertificateFile: "", CertificateKeyFile: "", + CACertificates: []string{}, }, WebDAVD: webdavd.Configuration{ Bindings: []webdavd.Binding{defaultWebDAVDBinding}, @@ -596,6 +600,12 @@ func getFTPDBindingFromEnv(idx int) { isSet = true } + clientAuthType, ok := lookupIntFromEnv(fmt.Sprintf("SFTPGO_FTPD__BINDINGS__%v__CLIENT_AUTH_TYPE", idx)) + if ok { + binding.ClientAuthType = clientAuthType + isSet = true + } + if isSet { if len(globalConf.FTPD.Bindings) > idx { globalConf.FTPD.Bindings[idx] = binding @@ -678,6 +688,7 @@ func setViperDefaults() { viper.SetDefault("ftpd.combine_support", globalConf.FTPD.CombineSupport) viper.SetDefault("ftpd.certificate_file", globalConf.FTPD.CertificateFile) viper.SetDefault("ftpd.certificate_key_file", globalConf.FTPD.CertificateKeyFile) + viper.SetDefault("ftpd.ca_certificates", globalConf.FTPD.CACertificates) viper.SetDefault("webdavd.certificate_file", globalConf.WebDAVD.CertificateFile) viper.SetDefault("webdavd.certificate_key_file", globalConf.WebDAVD.CertificateKeyFile) viper.SetDefault("webdavd.ca_certificates", globalConf.WebDAVD.CACertificates) diff --git a/config/config_test.go b/config/config_test.go index 6bb4f003..29821a45 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -549,6 +549,7 @@ func TestFTPDBindingsFromEnv(t *testing.T) { os.Setenv("SFTPGO_FTPD__BINDINGS__9__APPLY_PROXY_CONFIG", "t") os.Setenv("SFTPGO_FTPD__BINDINGS__9__TLS_MODE", "1") os.Setenv("SFTPGO_FTPD__BINDINGS__9__FORCE_PASSIVE_IP", "127.0.1.1") + os.Setenv("SFTPGO_FTPD__BINDINGS__9__CLIENT_AUTH_TYPE", "1") t.Cleanup(func() { os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__ADDRESS") @@ -561,6 +562,7 @@ func TestFTPDBindingsFromEnv(t *testing.T) { os.Unsetenv("SFTPGO_FTPD__BINDINGS__9__APPLY_PROXY_CONFIG") os.Unsetenv("SFTPGO_FTPD__BINDINGS__9__TLS_MODE") os.Unsetenv("SFTPGO_FTPD__BINDINGS__9__FORCE_PASSIVE_IP") + os.Unsetenv("SFTPGO_FTPD__BINDINGS__9__CLIENT_AUTH_TYPE") }) configDir := ".." @@ -571,13 +573,15 @@ func TestFTPDBindingsFromEnv(t *testing.T) { require.Equal(t, 2200, bindings[0].Port) require.Equal(t, "127.0.0.1", bindings[0].Address) require.False(t, bindings[0].ApplyProxyConfig) - require.Equal(t, bindings[0].TLSMode, 2) - require.Equal(t, bindings[0].ForcePassiveIP, "127.0.1.2") + require.Equal(t, 2, bindings[0].TLSMode) + require.Equal(t, "127.0.1.2", bindings[0].ForcePassiveIP) + require.Equal(t, 0, bindings[0].ClientAuthType) require.Equal(t, 2203, bindings[1].Port) require.Equal(t, "127.0.1.1", bindings[1].Address) require.True(t, bindings[1].ApplyProxyConfig) - require.Equal(t, bindings[1].TLSMode, 1) - require.Equal(t, bindings[1].ForcePassiveIP, "127.0.1.1") + require.Equal(t, 1, bindings[1].TLSMode) + require.Equal(t, "127.0.1.1", bindings[1].ForcePassiveIP) + require.Equal(t, 1, bindings[1].ClientAuthType) } func TestWebDAVBindingsFromEnv(t *testing.T) { @@ -611,6 +615,7 @@ func TestWebDAVBindingsFromEnv(t *testing.T) { require.Equal(t, 8000, bindings[1].Port) require.Equal(t, "127.0.0.1", bindings[1].Address) require.False(t, bindings[1].EnableHTTPS) + require.Equal(t, 0, bindings[1].ClientAuthType) require.Equal(t, 9000, bindings[2].Port) require.Equal(t, "127.0.1.1", bindings[2].Address) require.True(t, bindings[2].EnableHTTPS) diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 5602753c..881eedd8 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -97,6 +97,7 @@ The configuration file contains the following sections: - `apply_proxy_config`, boolean. If enabled the common proxy configuration, if any, will be applied. Default `true` - `tls_mode`, integer. 0 means accept both cleartext and encrypted sessions. 1 means TLS is required for both control and data connection. 2 means implicit TLS. Do not enable this blindly, please check that a proper TLS config is in place if you set `tls_mode` is different from 0. - `force_passive_ip`, ip address. External IP address to expose for passive connections. Leavy empty to autodetect. Defaut: "". + - `client_auth_type`, integer. Set to `1` to require client certificate authentication in addition to FTP authentication. You need to define at least a certificate authority for this to work. Default: 0. - `bind_port`, integer. Deprecated, please use `bindings` - `bind_address`, string. Deprecated, please use `bindings` - `banner`, string. Greeting banner displayed when a connection first comes in. Leave empty to use the default banner. Default `SFTPGo ready`, for example `SFTPGo 1.0.0-dev ready`. @@ -110,13 +111,14 @@ The configuration file contains the following sections: - `combine_support`, integer. Set to 1 to enable support for the non standard `COMB` FTP command. Combine is only supported for local filesystem, for cloud backends it has no advantage as it will download the partial files and will upload the combined one. Cloud backends natively support multipart uploads. Default `0`. - `certificate_file`, string. Certificate for FTPS. This can be an absolute path or a path relative to the config dir. - `certificate_key_file`, string. Private key matching the above certificate. This can be an absolute path or a path relative to the config dir. A certificate and the private key are required to enable explicit and implicit TLS. Certificate and key files can be reloaded on demand sending a `SIGHUP` signal on Unix based systems and a `paramchange` request to the running service on Windows. + - `ca_certificates`, list of strings. Set of root certificate authorities to use to verify client certificates. - `tls_mode`, integer. Deprecated, please use `bindings` - **webdavd**, the configuration for the WebDAV server, more info [here](./webdav.md) - `bindings`, list of structs. Each struct has the following fields: - `port`, integer. The port used for serving WebDAV requests. 0 means disabled. Default: 0. - `address`, string. Leave blank to listen on all available network interfaces. Default: "". - `enable_https`, boolean. Set to `true` and provide both a certificate and a key file to enable HTTPS connection for this binding. Default `false` - - `client_auth_type`, integer. Set to `1` to require client certificate authentication in addition to basic auth. You need to define at least a certificate authority for this to work. Default: 0. + - `client_auth_type`, integer. Set to `1` to require client certificate authentication in addition to basic authentication. You need to define at least a certificate authority for this to work. Default: 0. - `bind_port`, integer. Deprecated, please use `bindings` - `bind_address`, string. Deprecated, please use `bindings` - `certificate_file`, string. Certificate for WebDAV over HTTPS. This can be an absolute path or a path relative to the config dir. diff --git a/docs/webdav.md b/docs/webdav.md index 6f55db90..1c78bc2d 100644 --- a/docs/webdav.md +++ b/docs/webdav.md @@ -1,6 +1,6 @@ # WebDAV -The experimental `WebDAV` support can be enabled by setting a `bind_port` inside the `webdavd` configuration section. +The experimental `WebDAV` support can be enabled by configuring one or more `bindings` inside the `webdavd` configuration section. Each user has his own path like `http/s://:/` and it must authenticate using password credentials. diff --git a/ftpd/ftpd.go b/ftpd/ftpd.go index 26a429e3..de44e036 100644 --- a/ftpd/ftpd.go +++ b/ftpd/ftpd.go @@ -34,6 +34,9 @@ type Binding struct { TLSMode int `json:"tls_mode" mapstructure:"tls_mode"` // External IP address to expose for passive connections. ForcePassiveIP string `json:"force_passive_ip" mapstructure:"force_passive_ip"` + // set to 1 to require client certificate authentication in addition to FTP auth. + // You need to define at least a certificate authority for this to work + ClientAuthType int `json:"client_auth_type" mapstructure:"client_auth_type"` } // GetAddress returns the binding address @@ -102,6 +105,8 @@ type Configuration struct { // "paramchange" request to the running service on Windows. CertificateFile string `json:"certificate_file" mapstructure:"certificate_file"` CertificateKeyFile string `json:"certificate_key_file" mapstructure:"certificate_key_file"` + // CACertificates defines the set of root certificate authorities to use to verify client certificates. + CACertificates []string `json:"ca_certificates" mapstructure:"ca_certificates"` // Do not impose the port 20 for active data transfer. Enabling this option allows to run SFTPGo with less privilege ActiveTransfersPortNon20 bool `json:"active_transfers_port_non_20" mapstructure:"active_transfers_port_non_20"` // Set to true to disable active FTP @@ -151,6 +156,9 @@ func (c *Configuration) Initialize(configDir string) error { if err != nil { return err } + if err := mgr.LoadRootCAs(c.CACertificates, configDir); err != nil { + return err + } certMgr = mgr } serviceStatus = ServiceStatus{ diff --git a/ftpd/ftpd_test.go b/ftpd/ftpd_test.go index 45637b03..2838bbd9 100644 --- a/ftpd/ftpd_test.go +++ b/ftpd/ftpd_test.go @@ -282,6 +282,14 @@ func TestInitializationFailure(t *testing.T) { ftpdConf.Bindings[1].TLSMode = 1 err = ftpdConf.Initialize(configDir) require.Error(t, err) + + certPath := filepath.Join(os.TempDir(), "test_ftpd.crt") + keyPath := filepath.Join(os.TempDir(), "test_ftpd.key") + ftpdConf.CertificateFile = certPath + ftpdConf.CertificateKeyFile = keyPath + ftpdConf.CACertificates = []string{"invalid ca cert"} + err = ftpdConf.Initialize(configDir) + require.Error(t, err) } func TestBasicFTPHandling(t *testing.T) { diff --git a/ftpd/internal_test.go b/ftpd/internal_test.go index 81e6d8ad..3e718c91 100644 --- a/ftpd/internal_test.go +++ b/ftpd/internal_test.go @@ -1,6 +1,7 @@ package ftpd import ( + "crypto/tls" "fmt" "io/ioutil" "net" @@ -164,6 +165,15 @@ func TestInitialization(t *testing.T) { assert.NoError(t, err) certMgr = oldMgr + + binding = Binding{ + Port: 2121, + ClientAuthType: 1, + } + server = NewServer(c, configDir, binding, 0) + cfg, err := server.GetTLSConfig() + assert.NoError(t, err) + assert.Equal(t, tls.RequireAndVerifyClientCert, cfg.ClientAuth) } func TestServerGetSettings(t *testing.T) { diff --git a/ftpd/server.go b/ftpd/server.go index 0de20fcd..9d58e9f5 100644 --- a/ftpd/server.go +++ b/ftpd/server.go @@ -152,10 +152,15 @@ func (s *Server) AuthUser(cc ftpserver.ClientContext, username, password string) // GetTLSConfig returns a TLS Certificate to use func (s *Server) GetTLSConfig() (*tls.Config, error) { if certMgr != nil { - return &tls.Config{ + tlsConfig := &tls.Config{ GetCertificate: certMgr.GetCertificateFunc(), MinVersion: tls.VersionTLS12, - }, nil + } + if s.binding.ClientAuthType == 1 { + tlsConfig.ClientCAs = certMgr.GetRootCAs() + tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert + } + return tlsConfig, nil } return nil, errors.New("no TLS certificate configured") } diff --git a/sftpgo.json b/sftpgo.json index 02cf8351..c3c5e0ab 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -45,7 +45,8 @@ "port": 0, "apply_proxy_config": true, "tls_mode": 0, - "force_passive_ip": "" + "force_passive_ip": "", + "client_auth_type": 0 } ], "banner": "", @@ -60,7 +61,8 @@ "hash_support": 0, "combine_support": 0, "certificate_file": "", - "certificate_key_file": "" + "certificate_key_file": "", + "ca_certificates": [] }, "webdavd": { "bindings": [