From d377181b25ce5e206efd4dbfc426df438df2038a Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 26 Apr 2020 23:29:09 +0200 Subject: [PATCH] add a new configuration section for HTTP clients HTTP clients are used for executing hooks such as the ones used for custom actions, external authentication and pre-login user modifications. This allows, for example, to use self-signed certificate without defeating the purpose of using TLS --- config/config.go | 11 +++ config/config_test.go | 5 ++ dataprovider/dataprovider.go | 23 ++----- docs/custom-actions.md | 4 +- docs/dynamic-user-mod.md | 2 +- docs/external-auth.md | 2 +- docs/full-configuration.md | 5 +- examples/ldapauthserver/config/config.go | 2 +- httpclient/httpclient.go | 86 ++++++++++++++++++++++++ httpd/api_utils.go | 11 +-- httpd/httpd.go | 2 +- httpd/httpd_test.go | 4 ++ service/service.go | 3 + sftpd/sftpd.go | 6 +- sftpd/sftpd_test.go | 4 ++ sftpgo.json | 4 ++ 16 files changed, 138 insertions(+), 36 deletions(-) create mode 100644 httpclient/httpclient.go diff --git a/config/config.go b/config/config.go index a0922c657..7390da9b0 100644 --- a/config/config.go +++ b/config/config.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/drakkan/sftpgo/dataprovider" + "github.com/drakkan/sftpgo/httpclient" "github.com/drakkan/sftpgo/httpd" "github.com/drakkan/sftpgo/logger" "github.com/drakkan/sftpgo/sftpd" @@ -36,6 +37,7 @@ type globalConfig struct { SFTPD sftpd.Configuration `json:"sftpd" mapstructure:"sftpd"` ProviderConf dataprovider.Config `json:"data_provider" mapstructure:"data_provider"` HTTPDConfig httpd.Conf `json:"httpd" mapstructure:"httpd"` + HTTPConfig httpclient.Config `json:"http" mapstructure:"http"` } func init() { @@ -98,6 +100,10 @@ func init() { CertificateFile: "", CertificateKeyFile: "", }, + HTTPConfig: httpclient.Config{ + Timeout: 20, + CACertificates: nil, + }, } viper.SetEnvPrefix(configEnvPrefix) @@ -138,6 +144,11 @@ func SetProviderConf(config dataprovider.Config) { globalConf.ProviderConf = config } +// GetHTTPConfig returns the configuration for HTTP clients +func GetHTTPConfig() httpclient.Config { + return globalConf.HTTPConfig +} + func getRedactedGlobalConf() globalConfig { conf := globalConf conf.ProviderConf.Password = "[redacted]" diff --git a/config/config_test.go b/config/config_test.go index 11fce0cff..11c30ff7e 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -10,6 +10,7 @@ import ( "github.com/drakkan/sftpgo/config" "github.com/drakkan/sftpgo/dataprovider" + "github.com/drakkan/sftpgo/httpclient" "github.com/drakkan/sftpgo/httpd" "github.com/drakkan/sftpgo/sftpd" ) @@ -36,6 +37,10 @@ func TestLoadConfigTest(t *testing.T) { if config.GetSFTPDConfig().BindPort == emptySFTPDConf.BindPort { t.Errorf("error loading SFTPD conf") } + emptyHTTPConfig := httpclient.Config{} + if config.GetHTTPConfig().Timeout == emptyHTTPConfig.Timeout { + t.Errorf("error loading HTTP conf") + } confName := tempConfigName + ".json" configFilePath := filepath.Join(configDir, confName) err = config.LoadConfig(configDir, tempConfigName) diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index b8ed5a04a..c99c1744b 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -38,6 +38,7 @@ import ( "golang.org/x/crypto/pbkdf2" "golang.org/x/crypto/ssh" + "github.com/drakkan/sftpgo/httpclient" "github.com/drakkan/sftpgo/logger" "github.com/drakkan/sftpgo/metrics" "github.com/drakkan/sftpgo/utils" @@ -1146,9 +1147,7 @@ func validateKeyboardAuthResponse(response keyboardAuthHookResponse) error { func sendKeyboardAuthHTTPReq(url *url.URL, request keyboardAuthHookRequest) (keyboardAuthHookResponse, error) { var response keyboardAuthHookResponse - httpClient := &http.Client{ - Timeout: 30 * time.Second, - } + httpClient := httpclient.GetHTTPClient() reqAsJSON, err := json.Marshal(request) if err != nil { providerLog(logger.LevelWarn, "error serializing keyboard interactive auth request: %v", err) @@ -1326,7 +1325,6 @@ func doKeyboardInteractiveAuth(user User, authHook string, client ssh.KeyboardIn } func getPreLoginHookResponse(loginMethod string, userAsJSON []byte) ([]byte, error) { - timeout := 30 * time.Second if strings.HasPrefix(config.PreLoginHook, "http") { var url *url.URL var result []byte @@ -1338,9 +1336,7 @@ func getPreLoginHookResponse(loginMethod string, userAsJSON []byte) ([]byte, err q := url.Query() q.Add("login_method", loginMethod) url.RawQuery = q.Encode() - httpClient := &http.Client{ - Timeout: timeout, - } + httpClient := httpclient.GetHTTPClient() resp, err := httpClient.Post(url.String(), "application/json", bytes.NewBuffer(userAsJSON)) if err != nil { providerLog(logger.LevelWarn, "error getting pre-login hook response: %v", err) @@ -1355,7 +1351,7 @@ func getPreLoginHookResponse(loginMethod string, userAsJSON []byte) ([]byte, err } return ioutil.ReadAll(resp.Body) } - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() cmd := exec.CommandContext(ctx, config.PreLoginHook) cmd.Env = append(os.Environ(), @@ -1420,7 +1416,6 @@ func executePreLoginHook(username, loginMethod string) (User, error) { } func getExternalAuthResponse(username, password, pkey, keyboardInteractive string) ([]byte, error) { - timeout := 30 * time.Second if strings.HasPrefix(config.ExternalAuthHook, "http") { var url *url.URL var result []byte @@ -1429,9 +1424,7 @@ func getExternalAuthResponse(username, password, pkey, keyboardInteractive strin providerLog(logger.LevelWarn, "invalid url for external auth hook %#v, error: %v", config.ExternalAuthHook, err) return result, err } - httpClient := &http.Client{ - Timeout: timeout, - } + httpClient := httpclient.GetHTTPClient() authRequest := make(map[string]string) authRequest["username"] = username authRequest["password"] = password @@ -1453,7 +1446,7 @@ func getExternalAuthResponse(username, password, pkey, keyboardInteractive strin } return ioutil.ReadAll(resp.Body) } - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() cmd := exec.CommandContext(ctx, config.ExternalAuthHook) cmd.Env = append(os.Environ(), @@ -1564,9 +1557,7 @@ func executeAction(operation string, user User) { return } startTime := time.Now() - httpClient := &http.Client{ - Timeout: 15 * time.Second, - } + httpClient := httpclient.GetHTTPClient() resp, err := httpClient.Post(url.String(), "application/json", bytes.NewBuffer(userAsJSON)) respCode := 0 if err == nil { diff --git a/docs/custom-actions.md b/docs/custom-actions.md index 56a9e0f07..5f347f1c6 100644 --- a/docs/custom-actions.md +++ b/docs/custom-actions.md @@ -43,7 +43,7 @@ The `http_notification_url`, if defined, will be invoked as HTTP POST. The reque - `status`, integer. 0 means an error occurred. 1 means no error -The HTTP request is executed with a 15-second timeout. +The HTTP request will use the global configuration for HTTP clients. The `actions` struct inside the "data_provider" configuration section allows you to configure actions on user add, update, delete. @@ -83,4 +83,4 @@ The `command` must finish within 15 seconds. The `http_notification_url`, if defined, will be invoked as HTTP POST. The action is added to the query string, for example `?action=update`, and the user is sent serialized as JSON inside the POST body with sensitive fields removed. -The HTTP request is executed with a 15-second timeout. +The HTTP request will use the global configuration for HTTP clients. diff --git a/docs/dynamic-user-mod.md b/docs/dynamic-user-mod.md index 71a808552..d385fb041 100644 --- a/docs/dynamic-user-mod.md +++ b/docs/dynamic-user-mod.md @@ -26,7 +26,7 @@ The JSON response can include only the fields to update instead of the full user Please note that if you want to create a new user, the pre-login hook response must include all the mandatory user fields. -The hook must finish within 30 seconds. +The program hook must finish within 30 seconds, the HTTP hook will use the global configuration for HTTP clients. If an error happens while executing the hook then login will be denied. diff --git a/docs/external-auth.md b/docs/external-auth.md index 5171f3dc8..7baa0c228 100644 --- a/docs/external-auth.md +++ b/docs/external-auth.md @@ -23,7 +23,7 @@ If authentication succeed the HTTP response code must be 200 and the response bo If the authentication succeeds, the user will be automatically added/updated inside the defined data provider. Actions defined for users added/updated will not be executed in this case. The external hook should check authentication only. If there are login restrictions such as user disabled, expired, or login allowed only from specific IP addresses, it is enough to populate the matching user fields, and these conditions will be checked in the same way as for built-in users. -The hook must finish within 30 seconds. +The program hook must finish within 30 seconds, the HTTP hook timeout will use the global configuration for HTTP clients. This method is slower than built-in authentication, but it's very flexible as anyone can easily write his own authentication hooks. You can also restrict the authentication scope for the hook using the `external_auth_scope` configuration key: diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 8305072c8..c3827210c 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -99,7 +99,7 @@ The configuration file contains the following sections: - `credentials_path`, string. It defines the directory for storing user provided credential files such as Google Cloud Storage credentials. This can be an absolute path or a path relative to the config dir - `pre_login_program`, string. Deprecated, please use `pre_login_hook`. - `pre_login_hook`, string. Absolute path to an external program or an HTTP URL to invoke to modify user details just before the login. See the "Dynamic user modification" paragraph for more details. Leave empty to disable. -- **"httpd"**, the configuration for the HTTP server used to serve REST API +- **"httpd"**, the configuration for the HTTP server used to serve REST API and to expose the built-in web interface - `bind_port`, integer. The port used for serving HTTP requests. Set to 0 to disable HTTP server. Default: 8080 - `bind_address`, string. Leave blank to listen on all available network interfaces. Default: "127.0.0.1" - `templates_path`, string. Path to the HTML web templates. This can be an absolute path or a path relative to the config dir @@ -108,6 +108,9 @@ The configuration file contains the following sections: - `auth_user_file`, string. Path to a file used to store usernames and passwords for basic authentication. This can be an absolute path or a path relative to the config dir. We support HTTP basic authentication, and the file format must conform to the one generated using the Apache `htpasswd` tool. The supported password formats are bcrypt (`$2y$` prefix) and md5 crypt (`$apr1$` prefix). If empty, HTTP authentication is disabled. - `certificate_file`, string. Certificate for HTTPS. 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. If both the certificate and the private key are provided, the server will expect HTTPS connections. 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. +- **"http"**, the configuration for HTTP clients. HTTP clients are used for executing hooks such as the ones used for custom actions, external authentication and pre-login user modifications + - `timeout`, integer. Timeout specifies a time limit, in seconds, for requests. + - `ca_certificates`, list of strings. List of paths to extra CA certificates to trust. The paths can be absolute or relative to the config dir. Adding trusted CA certificates is a convenient way to use self-signed certificates without defeating the purpose of using TLS. A full example showing the default config (in JSON format) can be found [here](../sftpgo.json). diff --git a/examples/ldapauthserver/config/config.go b/examples/ldapauthserver/config/config.go index cf353630d..2a3c9a8d1 100644 --- a/examples/ldapauthserver/config/config.go +++ b/examples/ldapauthserver/config/config.go @@ -26,7 +26,7 @@ type HTTPDConfig struct { CertificateKeyFile string `mapstructure:"certificate_key_file"` } -// LDAPConfig defines the configuration parameters for LDAP connections and searchs +// LDAPConfig defines the configuration parameters for LDAP connections and searches type LDAPConfig struct { BaseDN string `mapstructure:"basedn"` BindURL string `mapstructure:"bind_url"` diff --git a/httpclient/httpclient.go b/httpclient/httpclient.go new file mode 100644 index 000000000..b7abcad6d --- /dev/null +++ b/httpclient/httpclient.go @@ -0,0 +1,86 @@ +package httpclient + +import ( + "crypto/tls" + "crypto/x509" + "io/ioutil" + "net/http" + "path/filepath" + "time" + + "github.com/drakkan/sftpgo/logger" + "github.com/drakkan/sftpgo/utils" +) + +// Config defines the configuration for HTTP clients. +// HTTP clients are used for executing hooks such as the ones used for +// custom actions, external authentication and pre-login user modifications +type Config struct { + // Timeout specifies a time limit, in seconds, for requests + Timeout int64 `json:"timeout" mapstructure:"timeout"` + // CACertificates defines extra CA certificates to trust. + // The paths can be absolute or relative to the config dir. + // Adding trusted CA certificates is a convenient way to use self-signed + // certificates without defeating the purpose of using TLS + CACertificates []string `json:"ca_certificates" mapstructure:"ca_certificates"` + customTransport *http.Transport +} + +const logSender = "httpclient" + +var httpConfig Config + +// Initialize configures HTTP clients +func (c Config) Initialize(configDir string) { + httpConfig = c + rootCAs := c.loadCACerts(configDir) + customTransport := http.DefaultTransport.(*http.Transport).Clone() + if customTransport.TLSClientConfig != nil { + customTransport.TLSClientConfig.RootCAs = rootCAs + } else { + customTransport.TLSClientConfig = &tls.Config{ + RootCAs: rootCAs, + } + } + httpConfig.customTransport = customTransport +} + +// loadCACerts returns system cert pools and try to add the configured +// CA certificates to it +func (c Config) loadCACerts(configDir string) *x509.CertPool { + rootCAs, err := x509.SystemCertPool() + if err != nil { + rootCAs = x509.NewCertPool() + } + + for _, ca := range c.CACertificates { + if !utils.IsFileInputValid(ca) { + logger.Warn(logSender, "", "unable to load invalid CA certificate: %#v", ca) + logger.WarnToConsole("unable to load invalid CA certificate: %#v", ca) + continue + } + if !filepath.IsAbs(ca) { + ca = filepath.Join(configDir, ca) + } + certs, err := ioutil.ReadFile(ca) + if err != nil { + logger.Warn(logSender, "", "unable to load CA certificate: %v", err) + logger.WarnToConsole("unable to load CA certificate: %#v", err) + } + if rootCAs.AppendCertsFromPEM(certs) { + logger.Debug(logSender, "", "CA certificate %#v added to the trusted certificates", ca) + } else { + logger.Warn(logSender, "", "unable to add CA certificate %#v to the trusted cetificates", ca) + logger.WarnToConsole("unable to add CA certificate %#v to the trusted cetificates", ca) + } + } + return rootCAs +} + +// GetHTTPClient returns an HTTP client with the configured parameters +func GetHTTPClient() *http.Client { + return &http.Client{ + Timeout: time.Duration(httpConfig.Timeout) * time.Second, + Transport: httpConfig.customTransport, + } +} diff --git a/httpd/api_utils.go b/httpd/api_utils.go index f65115607..357258327 100644 --- a/httpd/api_utils.go +++ b/httpd/api_utils.go @@ -15,9 +15,9 @@ import ( "path/filepath" "strconv" "strings" - "time" "github.com/drakkan/sftpgo/dataprovider" + "github.com/drakkan/sftpgo/httpclient" "github.com/drakkan/sftpgo/sftpd" "github.com/drakkan/sftpgo/utils" "github.com/go-chi/render" @@ -37,13 +37,6 @@ func SetBaseURLAndCredentials(url, username, password string) { authPassword = password } -// gets an HTTP Client with a timeout -func getHTTPClient() *http.Client { - return &http.Client{ - Timeout: 15 * time.Second, - } -} - func sendHTTPRequest(method, url string, body io.Reader, contentType string) (*http.Response, error) { req, err := http.NewRequest(method, url, body) if err != nil { @@ -55,7 +48,7 @@ func sendHTTPRequest(method, url string, body io.Reader, contentType string) (*h if len(authUsername) > 0 || len(authPassword) > 0 { req.SetBasicAuth(authUsername, authPassword) } - return getHTTPClient().Do(req) + return httpclient.GetHTTPClient().Do(req) } func buildURLRelativeToBase(paths ...string) string { diff --git a/httpd/httpd.go b/httpd/httpd.go index cf89ae060..5e6395b43 100644 --- a/httpd/httpd.go +++ b/httpd/httpd.go @@ -85,7 +85,7 @@ func SetDataProvider(provider dataprovider.Provider) { dataProvider = provider } -// Initialize the HTTP server +// Initialize configures and starts the HTTP server func (c Conf) Initialize(configDir string, profiler bool) error { var err error logger.Debug(logSender, "", "initializing HTTP server with config %+v", c) diff --git a/httpd/httpd_test.go b/httpd/httpd_test.go index 79f2cff58..ac715eb30 100644 --- a/httpd/httpd_test.go +++ b/httpd/httpd_test.go @@ -105,6 +105,10 @@ func TestMain(m *testing.M) { logger.Warn(logSender, "", "error initializing data provider: %v", err) os.Exit(1) } + + httpConfig := config.GetHTTPConfig() + httpConfig.Initialize(configDir) + dataProvider := dataprovider.GetProvider() httpdConf := config.GetHTTPDConfig() diff --git a/service/service.go b/service/service.go index f1dddde22..4f1f38f07 100644 --- a/service/service.go +++ b/service/service.go @@ -78,6 +78,9 @@ func (s *Service) Start() error { return err } + httpConfig := config.GetHTTPConfig() + httpConfig.Initialize(s.ConfigDir) + dataProvider := dataprovider.GetProvider() sftpdConf := config.GetSFTPDConfig() httpdConf := config.GetHTTPDConfig() diff --git a/sftpd/sftpd.go b/sftpd/sftpd.go index 50723b916..caf53c85e 100644 --- a/sftpd/sftpd.go +++ b/sftpd/sftpd.go @@ -8,7 +8,6 @@ import ( "context" "encoding/json" "fmt" - "net/http" "net/url" "os" "os/exec" @@ -17,6 +16,7 @@ import ( "time" "github.com/drakkan/sftpgo/dataprovider" + "github.com/drakkan/sftpgo/httpclient" "github.com/drakkan/sftpgo/logger" "github.com/drakkan/sftpgo/metrics" "github.com/drakkan/sftpgo/utils" @@ -509,9 +509,7 @@ func executeAction(a actionNotification) error { return err } startTime := time.Now() - httpClient := &http.Client{ - Timeout: 15 * time.Second, - } + httpClient := httpclient.GetHTTPClient() resp, err := httpClient.Post(url.String(), "application/json", bytes.NewBuffer(a.AsJSON())) respCode := 0 if err == nil { diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index f8efed87f..0c57e63fb 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -119,6 +119,10 @@ func TestMain(m *testing.M) { logger.Warn(logSender, "", "error initializing data provider: %v", err) os.Exit(1) } + + httpConfig := config.GetHTTPConfig() + httpConfig.Initialize(configDir) + dataProvider := dataprovider.GetProvider() sftpdConf := config.GetSFTPDConfig() httpdConf := config.GetHTTPDConfig() diff --git a/sftpgo.json b/sftpgo.json index 1d84e3bc0..45f9e2724 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -65,5 +65,9 @@ "auth_user_file": "", "certificate_file": "", "certificate_key_file": "" + }, + "http": { + "timeout": 20, + "ca_certificates": [] } }