From 6175acb572bfb3fd31f3207181c9baa9fefd1368 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 24 Nov 2023 20:43:50 +0100 Subject: [PATCH] add support for reading more secrets from files Signed-off-by: Nicola Murino --- docs/full-configuration.md | 8 ++++-- internal/config/config.go | 33 ++++++++++++++--------- internal/config/config_test.go | 3 +++ internal/dataprovider/dataprovider.go | 16 +++++------ internal/httpd/httpd.go | 39 ++++++++++++++++++--------- internal/httpd/httpd_test.go | 10 +++++++ internal/httpd/oidc.go | 10 ++++++- internal/httpd/oidc_test.go | 13 ++++++++- internal/kms/kms.go | 13 +++++---- internal/util/util.go | 21 +++++++++++++++ sftpgo.json | 4 +++ 11 files changed, 126 insertions(+), 44 deletions(-) diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 6a5cbd81..a432538d 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -247,7 +247,9 @@ The configuration file contains the following sections: - `host`, string. Database host. For `postgresql` and `cockroachdb` drivers you can specify multiple hosts separated by commas. Leave empty for drivers `sqlite`, `bolt` and `memory` - `port`, integer. Database port. Leave empty for drivers `sqlite`, `bolt` and `memory` - `username`, string. Database user. Leave empty for drivers `sqlite`, `bolt` and `memory` + - `username_file`, string. Defines the path to a file containing the database user. This can be an absolute path or a path relative to the config dir. If not empty it takes precedence over `username`. Default: blank. - `password`, string. Database password. Leave empty for drivers `sqlite`, `bolt` and `memory` + - `password_file`, string. Defines the path to a file containing the database password. This can be an absolute path or a path relative to the config dir. If not empty it takes precedence over `password`. Default: blank. - `sslmode`, integer. Used for drivers `mysql` and `postgresql`. 0 disable TLS connections, 1 require TLS, 2 set TLS mode to `verify-ca` for driver `postgresql` and `skip-verify` for driver `mysql`, 3 set TLS mode to `verify-full` for driver `postgresql` and `preferred` for driver `mysql`, 4 set the TLS mode to `prefer` for driver `postgresql`, 5 set the TLS mode to `allow` for driver `postgresql` - `root_cert`, string. Path to the root certificate authority used to verify that the server certificate was signed by a trusted CA - `disable_sni`, boolean. Allows to opt out Server Name Indication (SNI) for TLS connections. Default: `false` @@ -326,6 +328,7 @@ The configuration file contains the following sections: - `config_url`, string. Identifier for the service. If defined, SFTPGo will add `/.well-known/openid-configuration` to this url and attempt to retrieve the provider configuration on startup. SFTPGo will refuse to start if it fails to connect to the specified URL. Default: blank. - `client_id`, string. Defines the application's ID. Default: blank. - `client_secret`, string. Defines the application's secret. Default: blank. + - `client_secret_file`, string. Defines the path to a file containing the application secret. This can be an absolute path or a path relative to the config dir. If not empty, it takes precedence over `client_secret`. Default: blank. - `redirect_base_url`, string. Defines the base URL to redirect to after OpenID authentication. The suffix `/web/oidc/redirect` will be added to this base URL, adding also the `web_root` if configured. Default: blank. - `username_field`, string. Defines the ID token claims field to map to the SFTPGo username. Default: blank. - `scopes`, list of strings. Request the OAuth provider to provide the scope information from an authenticated users. The `openid` scope is mandatory. Default: `"openid", "profile", "email"`. @@ -369,6 +372,7 @@ The configuration file contains the following sections: - `ca_certificates`, list of strings. Set of root certificate authorities to be used to verify client certificates. - `ca_revocation_lists`, list of strings. Set a revocation lists, one for each root CA, to be used to check if a client certificate has been revoked. The revocation lists can be reloaded on demand sending a `SIGHUP` signal on Unix based systems and a `paramchange` request to the running service on Windows. - `signing_passphrase`, string. Passphrase to use to derive the signing key for JWT and CSRF tokens. If empty a random signing key will be generated each time SFTPGo starts. If you set a signing passphrase you should consider rotating it periodically for added security. + - `signing_passphrase_file`, string. Defines the path to a file containing the signing passphrase. This can be an absolute path or a path relative to the config dir. If not empty, it takes precedence over `signing_passphrase`. Default: blank. - `token_validation`, integer. Define how to validate JWT tokens, cookies and CSRF tokens. By default all the available security checks are enabled. Set to 1 to disable the requirement that a token must be used by the same IP for which it was issued. Default: `0`. - `max_upload_file_size`, integer. Defines the maximum request body size, in bytes, for Web Client/API HTTP upload requests. `0` means no limit. Default: `0`. - `cors` struct containing CORS configuration. SFTPGo uses [Go CORS handler](https://github.com/rs/cors), please refer to upstream documentation for fields meaning and their default values. @@ -438,8 +442,8 @@ The configuration file contains the following sections: - **kms**, configuration for the Key Management Service, more details can be found [here](./kms.md) - `secrets` - `url`, string. Defines the URI to the KMS service. Default: blank. - - `master_key`, string. Defines the master encryption key as string. If not empty, it takes precedence over `master_key_path`. Default: blank. - - `master_key_path`, string. Defines the absolute path to a file containing the master encryption key. Default: blank. + - `master_key`, string. Defines the master encryption key as string. Default: blank. + - `master_key_path`, string. Defines the absolute path to a file containing the master encryption key. If not empty, it takes precedence over `master_key`. Default: blank.
MFA diff --git a/internal/config/config.go b/internal/config/config.go index 948e85cb..8e30142d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -121,6 +121,7 @@ var ( OIDC: httpd.OIDC{ ClientID: "", ClientSecret: "", + ClientSecretFile: "", ConfigURL: "", RedirectBaseURL: "", UsernameField: "", @@ -386,18 +387,19 @@ func Init() { BackupsPath: "backups", }, HTTPDConfig: httpd.Conf{ - Bindings: []httpd.Binding{defaultHTTPDBinding}, - TemplatesPath: "templates", - StaticFilesPath: "static", - OpenAPIPath: "openapi", - WebRoot: "", - CertificateFile: "", - CertificateKeyFile: "", - CACertificates: nil, - CARevocationLists: nil, - SigningPassphrase: "", - TokenValidation: 0, - MaxUploadFileSize: 0, + Bindings: []httpd.Binding{defaultHTTPDBinding}, + TemplatesPath: "templates", + StaticFilesPath: "static", + OpenAPIPath: "openapi", + WebRoot: "", + CertificateFile: "", + CertificateKeyFile: "", + CACertificates: nil, + CARevocationLists: nil, + SigningPassphrase: "", + SigningPassphraseFile: "", + TokenValidation: 0, + MaxUploadFileSize: 0, Cors: httpd.CorsConfig{ Enabled: false, AllowedOrigins: []string{}, @@ -1568,6 +1570,12 @@ func getHTTPDOIDCFromEnv(idx int) (httpd.OIDC, bool) { isSet = true } + clientSecretFile, ok := os.LookupEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__CLIENT_SECRET_FILE", idx)) + if ok { + result.ClientSecretFile = clientSecretFile + isSet = true + } + configURL, ok := os.LookupEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__CONFIG_URL", idx)) if ok { result.ConfigURL = configURL @@ -2119,6 +2127,7 @@ func setViperDefaults() { viper.SetDefault("httpd.ca_certificates", globalConf.HTTPDConfig.CACertificates) viper.SetDefault("httpd.ca_revocation_lists", globalConf.HTTPDConfig.CARevocationLists) viper.SetDefault("httpd.signing_passphrase", globalConf.HTTPDConfig.SigningPassphrase) + viper.SetDefault("httpd.signing_passphrase_file", globalConf.HTTPDConfig.SigningPassphraseFile) viper.SetDefault("httpd.token_validation", globalConf.HTTPDConfig.TokenValidation) viper.SetDefault("httpd.max_upload_file_size", globalConf.HTTPDConfig.MaxUploadFileSize) viper.SetDefault("httpd.cors.enabled", globalConf.HTTPDConfig.Cors.Enabled) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 789d1e10..e0007410 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -615,6 +615,7 @@ func TestHTTPDSubObjectsFromEnv(t *testing.T) { os.Setenv("SFTPGO_HTTPD__BINDINGS__0__SECURITY__HTTPS_PROXY_HEADERS__0__VALUE", "https") os.Setenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CLIENT_ID", "client_id") os.Setenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CLIENT_SECRET", "client_secret") + os.Setenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CLIENT_SECRET_FILE", "client_secret_file") os.Setenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CONFIG_URL", "config_url") os.Setenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__REDIRECT_BASE_URL", "redirect_base_url") os.Setenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__USERNAME_FIELD", "email") @@ -623,6 +624,7 @@ func TestHTTPDSubObjectsFromEnv(t *testing.T) { os.Unsetenv("SFTPGO_HTTPD__BINDINGS__0__SECURITY__HTTPS_PROXY_HEADERS__0__VALUE") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CLIENT_ID") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CLIENT_SECRET") + os.Unsetenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CLIENT_SECRET_FILE") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__CONFIG_URL") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__REDIRECT_BASE_URL") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__0__OIDC__USERNAME_FIELD") @@ -636,6 +638,7 @@ func TestHTTPDSubObjectsFromEnv(t *testing.T) { require.Len(t, httpdConf.Bindings[0].Security.HTTPSProxyHeaders, 1) require.Equal(t, "client_id", httpdConf.Bindings[0].OIDC.ClientID) require.Equal(t, "client_secret", httpdConf.Bindings[0].OIDC.ClientSecret) + require.Equal(t, "client_secret_file", httpdConf.Bindings[0].OIDC.ClientSecretFile) require.Equal(t, "config_url", httpdConf.Bindings[0].OIDC.ConfigURL) require.Equal(t, "redirect_base_url", httpdConf.Bindings[0].OIDC.RedirectBaseURL) require.Equal(t, "email", httpdConf.Bindings[0].OIDC.UsernameField) diff --git a/internal/dataprovider/dataprovider.go b/internal/dataprovider/dataprovider.go index 15ee6142..bf7874fb 100644 --- a/internal/dataprovider/dataprovider.go +++ b/internal/dataprovider/dataprovider.go @@ -354,10 +354,10 @@ type Config struct { // Database port Port int `json:"port" mapstructure:"port"` // Database username - Username string `json:"username" mapstructure:"username"` + Username string `json:"username" mapstructure:"username"` UsernameFile string `json:"username_file" mapstructure:"username_file"` // Database password - Password string `json:"password" mapstructure:"password"` + Password string `json:"password" mapstructure:"password"` PasswordFile string `json:"password_file" mapstructure:"password_file"` // Used for drivers mysql and postgresql. // 0 disable SSL/TLS connections. @@ -877,20 +877,20 @@ func Initialize(cnf Config, basePath string, checkAdmins bool) error { config.Actions.ExecuteOn = util.RemoveDuplicates(config.Actions.ExecuteOn, true) config.Actions.ExecuteFor = util.RemoveDuplicates(config.Actions.ExecuteFor, true) - if config.Username == "" && config.UsernameFile != "" { - user, err := os.ReadFile(config.UsernameFile) + if config.UsernameFile != "" { + user, err := util.ReadConfigFromFile(config.UsernameFile, basePath) if err != nil { return err } - config.Username = string(user) + config.Username = user } - if config.Password == "" && config.PasswordFile != "" { - password, err := os.ReadFile(config.PasswordFile) + if config.PasswordFile != "" { + password, err := util.ReadConfigFromFile(config.PasswordFile, basePath) if err != nil { return err } - config.Password = string(password) + config.Password = password } cnf.BackupsPath = getConfigPath(cnf.BackupsPath, basePath) diff --git a/internal/httpd/httpd.go b/internal/httpd/httpd.go index 67b1af4a..5957a746 100644 --- a/internal/httpd/httpd.go +++ b/internal/httpd/httpd.go @@ -760,7 +760,8 @@ type Conf struct { // SigningPassphrase defines the passphrase to use to derive the signing key for JWT and CSRF tokens. // If empty a random signing key will be generated each time SFTPGo starts. If you set a // signing passphrase you should consider rotating it periodically for added security - SigningPassphrase string `json:"signing_passphrase" mapstructure:"signing_passphrase"` + SigningPassphrase string `json:"signing_passphrase" mapstructure:"signing_passphrase"` + SigningPassphraseFile string `json:"signing_passphrase_file" mapstructure:"signing_passphrase_file"` // TokenValidation allows to define how to validate JWT tokens, cookies and CSRF tokens. // By default all the available security checks are enabled. Set to 1 to disable the requirement // that a token must be used by the same IP for which it was issued. @@ -912,6 +913,21 @@ func (c *Conf) loadFromProvider() error { return nil } +func (c *Conf) loadTemplates(templatesPath string) { + if c.isWebAdminEnabled() { + updateWebAdminURLs(c.WebRoot) + loadAdminTemplates(templatesPath) + } else { + logger.Info(logSender, "", "built-in web admin interface disabled") + } + if c.isWebClientEnabled() { + updateWebClientURLs(c.WebRoot) + loadClientTemplates(templatesPath) + } else { + logger.Info(logSender, "", "built-in web client interface disabled") + } +} + // Initialize configures and starts the HTTP server func (c *Conf) Initialize(configDir string, isShared int) error { if err := c.loadFromProvider(); err != nil { @@ -929,18 +945,7 @@ func (c *Conf) Initialize(configDir string, isShared int) error { if err := c.checkRequiredDirs(staticFilesPath, templatesPath); err != nil { return err } - if c.isWebAdminEnabled() { - updateWebAdminURLs(c.WebRoot) - loadAdminTemplates(templatesPath) - } else { - logger.Info(logSender, "", "built-in web admin interface disabled") - } - if c.isWebClientEnabled() { - updateWebClientURLs(c.WebRoot) - loadClientTemplates(templatesPath) - } else { - logger.Info(logSender, "", "built-in web client interface disabled") - } + c.loadTemplates(templatesPath) keyPairs := c.getKeyPairs(configDir) if len(keyPairs) > 0 { mgr, err := common.NewCertManager(keyPairs, configDir, logSender) @@ -958,6 +963,14 @@ func (c *Conf) Initialize(configDir string, isShared int) error { certMgr = mgr } + if c.SigningPassphraseFile != "" { + passphrase, err := util.ReadConfigFromFile(c.SigningPassphraseFile, configDir) + if err != nil { + return err + } + c.SigningPassphrase = passphrase + } + csrfTokenAuth = jwtauth.New(jwa.HS256.String(), getSigningKey(c.SigningPassphrase), nil) hideSupportLink = c.HideSupportLink diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index c937facd..40942368 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -474,7 +474,15 @@ func TestInitialization(t *testing.T) { err := config.LoadConfig(configDir, "") assert.NoError(t, err) invalidFile := "invalid file" + passphraseFile := filepath.Join(os.TempDir(), util.GenerateUniqueID()) + err = os.WriteFile(passphraseFile, []byte("my secret"), 0600) + assert.NoError(t, err) + defer os.Remove(passphraseFile) httpdConf := config.GetHTTPDConfig() + httpdConf.SigningPassphraseFile = invalidFile + err = httpdConf.Initialize(configDir, isShared) + assert.ErrorIs(t, err, fs.ErrNotExist) + httpdConf.SigningPassphraseFile = passphraseFile defaultTemplatesPath := httpdConf.TemplatesPath defaultStaticPath := httpdConf.StaticFilesPath httpdConf.CertificateFile = invalidFile @@ -506,11 +514,13 @@ func TestInitialization(t *testing.T) { err = httpdConf.Initialize(configDir, isShared) assert.Error(t, err) httpdConf.CARevocationLists = nil + httpdConf.SigningPassphraseFile = passphraseFile httpdConf.Bindings[0].ProxyAllowed = []string{"invalid ip/network"} err = httpdConf.Initialize(configDir, isShared) if assert.Error(t, err) { assert.Contains(t, err.Error(), "is not a valid IP range") } + assert.Equal(t, "my secret", httpdConf.SigningPassphrase) httpdConf.Bindings[0].ProxyAllowed = nil httpdConf.Bindings[0].EnableWebAdmin = false httpdConf.Bindings[0].EnableWebClient = false diff --git a/internal/httpd/oidc.go b/internal/httpd/oidc.go index fb31b973..5d4b55fa 100644 --- a/internal/httpd/oidc.go +++ b/internal/httpd/oidc.go @@ -64,7 +64,8 @@ type OIDC struct { // ClientID is the application's ID ClientID string `json:"client_id" mapstructure:"client_id"` // ClientSecret is the application's secret - ClientSecret string `json:"client_secret" mapstructure:"client_secret"` + ClientSecret string `json:"client_secret" mapstructure:"client_secret"` + ClientSecretFile string `json:"client_secret_file" mapstructure:"client_secret_file"` // ConfigURL is the identifier for the service. // SFTPGo will try to retrieve the provider configuration on startup and then // will refuse to start if it fails to connect to the specified URL @@ -144,6 +145,13 @@ func (o *OIDC) initialize() error { if !util.Contains(o.Scopes, oidc.ScopeOpenID) { return fmt.Errorf("oidc: required scope %q is not set", oidc.ScopeOpenID) } + if o.ClientSecretFile != "" { + secret, err := util.ReadConfigFromFile(o.ClientSecretFile, configurationDir) + if err != nil { + return err + } + o.ClientSecret = secret + } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() diff --git a/internal/httpd/oidc_test.go b/internal/httpd/oidc_test.go index 52a07429..adde9906 100644 --- a/internal/httpd/oidc_test.go +++ b/internal/httpd/oidc_test.go @@ -19,6 +19,7 @@ import ( "context" "encoding/json" "fmt" + "io/fs" "net/http" "net/http/httptest" "net/url" @@ -101,9 +102,10 @@ func TestOIDCInitialization(t *testing.T) { config := OIDC{} err := config.initialize() assert.NoError(t, err) + secret := "jRsmE0SWnuZjP7djBqNq0mrf8QN77j2c" config = OIDC{ ClientID: "sftpgo-client", - ClientSecret: "jRsmE0SWnuZjP7djBqNq0mrf8QN77j2c", + ClientSecret: util.GenerateUniqueID(), ConfigURL: fmt.Sprintf("http://%v/", oidcMockAddr), RedirectBaseURL: "http://127.0.0.1:8081/", UsernameField: "preferred_username", @@ -114,10 +116,19 @@ func TestOIDCInitialization(t *testing.T) { assert.Contains(t, err.Error(), "oidc: required scope \"openid\" is not set") } config.Scopes = []string{oidc.ScopeOpenID} + config.ClientSecretFile = "missing file" + err = config.initialize() + assert.ErrorIs(t, err, fs.ErrNotExist) + secretFile := filepath.Join(os.TempDir(), util.GenerateUniqueID()) + defer os.Remove(secretFile) + err = os.WriteFile(secretFile, []byte(secret), 0600) + assert.NoError(t, err) + config.ClientSecretFile = secretFile err = config.initialize() if assert.Error(t, err) { assert.Contains(t, err.Error(), "oidc: unable to initialize provider") } + assert.Equal(t, secret, config.ClientSecret) config.ConfigURL = fmt.Sprintf("http://%v/auth/realms/sftpgo", oidcMockAddr) err = config.initialize() assert.NoError(t, err) diff --git a/internal/kms/kms.go b/internal/kms/kms.go index 2e7e1604..57e29a7d 100644 --- a/internal/kms/kms.go +++ b/internal/kms/kms.go @@ -18,13 +18,13 @@ package kms import ( "encoding/json" "errors" - "os" "strings" "sync" sdkkms "github.com/sftpgo/sdk/kms" "github.com/drakkan/sftpgo/v2/internal/logger" + "github.com/drakkan/sftpgo/v2/internal/util" ) // SecretProvider defines the interface for a KMS secrets provider @@ -105,15 +105,14 @@ func NewPlainSecret(payload string) *Secret { // Initialize configures the KMS support func (c *Configuration) Initialize() error { - if c.Secrets.MasterKeyString != "" { - c.Secrets.masterKey = c.Secrets.MasterKeyString - } - if c.Secrets.masterKey == "" && c.Secrets.MasterKeyPath != "" { - mKey, err := os.ReadFile(c.Secrets.MasterKeyPath) + if c.Secrets.MasterKeyPath != "" { + mKey, err := util.ReadConfigFromFile(c.Secrets.MasterKeyPath, "") if err != nil { return err } - c.Secrets.masterKey = strings.TrimSpace(string(mKey)) + c.Secrets.masterKey = mKey + } else if c.Secrets.MasterKeyString != "" { + c.Secrets.masterKey = c.Secrets.MasterKeyString } config = *c if config.Secrets.URL == "" { diff --git a/internal/util/util.go b/internal/util/util.go index 09c661b9..153a519d 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -876,3 +876,24 @@ func JSONEscape(val string) string { } return string(b[1 : len(b)-1]) } + +// ReadConfigFromFile reads a configuration parameter from the specified file +func ReadConfigFromFile(name, configDir string) (string, error) { + if !IsFileInputValid(name) { + return "", fmt.Errorf("invalid file input: %q", name) + } + if configDir == "" { + if !filepath.IsAbs(name) { + return "", fmt.Errorf("%q must be an absolute file path", name) + } + } else { + if name != "" && !filepath.IsAbs(name) { + name = filepath.Join(configDir, name) + } + } + val, err := os.ReadFile(name) + if err != nil { + return "", err + } + return strings.TrimSpace(string(val)), nil +} diff --git a/sftpgo.json b/sftpgo.json index 21d59c37..ff973c68 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -198,7 +198,9 @@ "host": "", "port": 0, "username": "", + "username_file": "", "password": "", + "password_file": "", "sslmode": 0, "disable_sni": false, "target_session_attrs": "", @@ -278,6 +280,7 @@ "oidc": { "client_id": "", "client_secret": "", + "client_secret_file": "", "config_url": "", "redirect_base_url": "", "scopes": [ @@ -344,6 +347,7 @@ "ca_certificates": [], "ca_revocation_lists": [], "signing_passphrase": "", + "signing_passphrase_file": "", "token_validation": 0, "max_upload_file_size": 0, "cors": {