[MM-63760] Only partially sanitize DB datasources for Support Packet (#30728)
Co-authored-by: Claude <noreply@anthropic.com>
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
160cb91ab9
Коммит
5b389c5224
@@ -226,11 +226,11 @@ func (a *App) SanitizedConfig(cfg *model.Config) {
|
||||
if err != nil {
|
||||
// GetPluginManifests might error, e.g. when plugins are disabled.
|
||||
// Sanitize all plugin settings in this case.
|
||||
cfg.Sanitize(nil)
|
||||
cfg.Sanitize(nil, nil)
|
||||
return
|
||||
}
|
||||
|
||||
cfg.Sanitize(manifests)
|
||||
cfg.Sanitize(manifests, nil)
|
||||
}
|
||||
|
||||
// GetEnvironmentConfig returns a map of configuration keys whose values have been overridden by an environment variable.
|
||||
|
||||
@@ -42,7 +42,7 @@ func (ps *PlatformService) Config() *model.Config {
|
||||
}
|
||||
|
||||
// getSanitizedConfig gets the configuration without any secrets.
|
||||
func (ps *PlatformService) getSanitizedConfig(rctx request.CTX) *model.Config {
|
||||
func (ps *PlatformService) getSanitizedConfig(rctx request.CTX, opts *model.SanitizeOptions) *model.Config {
|
||||
cfg := ps.Config().Clone()
|
||||
|
||||
manifests, err := ps.getPluginManifests()
|
||||
@@ -50,9 +50,9 @@ func (ps *PlatformService) getSanitizedConfig(rctx request.CTX) *model.Config {
|
||||
// getPluginManifests might error, e.g. when plugins are disabled.
|
||||
// Sanitize all plugin settings in this case.
|
||||
rctx.Logger().Warn("Failed to get plugin manifests for config sanitization. Will sanitize all plugin settings.", mlog.Err(err))
|
||||
cfg.Sanitize(nil)
|
||||
cfg.Sanitize(nil, opts)
|
||||
} else {
|
||||
cfg.Sanitize(manifests)
|
||||
cfg.Sanitize(manifests, opts)
|
||||
}
|
||||
|
||||
return cfg
|
||||
|
||||
@@ -196,7 +196,7 @@ func (ps *PlatformService) getSupportPacketDiagnostics(rctx request.CTX) (*model
|
||||
}
|
||||
|
||||
func (ps *PlatformService) getSanitizedConfigFile(rctx request.CTX) (*model.FileData, error) {
|
||||
config := ps.getSanitizedConfig(rctx)
|
||||
config := ps.getSanitizedConfig(rctx, &model.SanitizeOptions{PartiallyRedactDataSources: true})
|
||||
spConfig := model.SupportPacketConfig{
|
||||
Config: config,
|
||||
FeatureFlags: *config.FeatureFlags,
|
||||
|
||||
@@ -360,13 +360,18 @@ func TestGetSanitizedConfigFile(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
// Ensure sensitive fields are redacted
|
||||
assert.Equal(t, model.FakeSetting, *config.SqlSettings.DataSource)
|
||||
assert.Equal(t, model.FakeSetting, *config.FileSettings.PublicLinkSalt)
|
||||
|
||||
// Ensure non-sensitive fields are present
|
||||
assert.Equal(t, "example.com", *config.ServiceSettings.AllowedUntrustedInternalConnections)
|
||||
|
||||
// Ensure feature flags are present
|
||||
assert.Equal(t, "true", config.FeatureFlags.TestFeature)
|
||||
|
||||
// Ensure DataSource is partially sanitized (not completely replaced with FakeSetting)
|
||||
// The default test database connection string should have username/password redacted
|
||||
assert.Contains(t, *config.SqlSettings.DataSource, "****:****")
|
||||
assert.NotEqual(t, model.FakeSetting, *config.SqlSettings.DataSource)
|
||||
}
|
||||
|
||||
func TestGetCPUProfile(t *testing.T) {
|
||||
|
||||
@@ -409,7 +409,7 @@ func (ds *DatabaseStore) RemoveFile(name string) error {
|
||||
func (ds *DatabaseStore) String() string {
|
||||
// This is called during the running of MM, so we expect the parsing of DSN
|
||||
// to be successful.
|
||||
sanitized, _ := sqlUtils.SanitizeDataSource(ds.driverName, ds.originalDsn)
|
||||
sanitized, _ := model.SanitizeDataSource(ds.driverName, ds.originalDsn)
|
||||
return sanitized
|
||||
}
|
||||
|
||||
|
||||
@@ -65,19 +65,19 @@ func (cd ConfigDiffs) Sanitize() ConfigDiffs {
|
||||
|
||||
cfgPtr, ok := cd[0].BaseVal.(*model.Config)
|
||||
if ok {
|
||||
cfgPtr.Sanitize(pluginManifests)
|
||||
cfgPtr.Sanitize(pluginManifests, nil)
|
||||
}
|
||||
cfgPtr, ok = cd[0].ActualVal.(*model.Config)
|
||||
if ok {
|
||||
cfgPtr.Sanitize(pluginManifests)
|
||||
cfgPtr.Sanitize(pluginManifests, nil)
|
||||
}
|
||||
cfgVal, ok := cd[0].BaseVal.(model.Config)
|
||||
if ok {
|
||||
cfgVal.Sanitize(pluginManifests)
|
||||
cfgVal.Sanitize(pluginManifests, nil)
|
||||
}
|
||||
cfgVal, ok = cd[0].ActualVal.(model.Config)
|
||||
if ok {
|
||||
cfgVal.Sanitize(pluginManifests)
|
||||
cfgVal.Sanitize(pluginManifests, nil)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -113,7 +113,7 @@ func TestDiffSanitized(t *testing.T) {
|
||||
Path: "",
|
||||
BaseVal: func() model.Config {
|
||||
cfg := defaultConfigGen()
|
||||
cfg.Sanitize(nil)
|
||||
cfg.Sanitize(nil, nil)
|
||||
return *cfg
|
||||
}(),
|
||||
ActualVal: model.Config{},
|
||||
@@ -131,7 +131,7 @@ func TestDiffSanitized(t *testing.T) {
|
||||
BaseVal: model.Config{},
|
||||
ActualVal: func() model.Config {
|
||||
cfg := defaultConfigGen()
|
||||
cfg.Sanitize(nil)
|
||||
cfg.Sanitize(nil, nil)
|
||||
return *cfg
|
||||
}(),
|
||||
},
|
||||
|
||||
@@ -20,7 +20,9 @@ import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/go-sql-driver/mysql"
|
||||
"github.com/mattermost/ldap"
|
||||
"github.com/pkg/errors"
|
||||
|
||||
"github.com/mattermost/mattermost/server/public/shared/mlog"
|
||||
"github.com/mattermost/mattermost/server/public/utils"
|
||||
@@ -73,6 +75,9 @@ const (
|
||||
|
||||
FakeSetting = "********************************"
|
||||
|
||||
// SanitizedPassword is the placeholder used for redacting passwords in data sources
|
||||
SanitizedPassword = "****"
|
||||
|
||||
RestrictEmojiCreationAll = "all"
|
||||
RestrictEmojiCreationAdmin = "admin"
|
||||
RestrictEmojiCreationSystemAdmin = "system_admin"
|
||||
@@ -4681,6 +4686,14 @@ func (s *ImageProxySettings) isValid() *AppError {
|
||||
return nil
|
||||
}
|
||||
|
||||
// SanitizeOptions specifies options for the [Config.Sanitize] method.
|
||||
type SanitizeOptions struct {
|
||||
// PartiallyRedactDataSources, when true, only redacts usernames and passwords
|
||||
// from data sources, keeping other connection parameters visible.
|
||||
// When false, replaces the entire data source with FakeSetting.
|
||||
PartiallyRedactDataSources bool
|
||||
}
|
||||
|
||||
func (o *Config) GetSanitizeOptions() map[string]bool {
|
||||
options := map[string]bool{}
|
||||
options["fullname"] = *o.PrivacySettings.ShowFullName
|
||||
@@ -4689,7 +4702,32 @@ func (o *Config) GetSanitizeOptions() map[string]bool {
|
||||
return options
|
||||
}
|
||||
|
||||
func (o *Config) Sanitize(pluginManifests []*Manifest) {
|
||||
// Sanitize removes sensitive information from the configuration object.
|
||||
// It replaces sensitive fields with [FakeSetting] or sanitizes them.
|
||||
//
|
||||
// Parameters:
|
||||
// - pluginManifests: Plugin manifests for sanitizing plugin settings.
|
||||
// - opts: Options for controlling sanitization behavior. If nil, defaults are used. See [SanitizeOptions].
|
||||
func (o *Config) Sanitize(pluginManifests []*Manifest, opts *SanitizeOptions) {
|
||||
if opts == nil {
|
||||
opts = &SanitizeOptions{}
|
||||
}
|
||||
|
||||
var driverName string
|
||||
if o.SqlSettings.DriverName != nil {
|
||||
driverName = *o.SqlSettings.DriverName
|
||||
}
|
||||
sanitizeDataSourceField := func(dataSource string, fieldName string) string {
|
||||
if opts.PartiallyRedactDataSources && driverName != "" {
|
||||
sanitized, err := SanitizeDataSource(driverName, dataSource)
|
||||
if err != nil {
|
||||
mlog.Warn("Failed to sanitize "+fieldName+". Falling back to fully sanitizing the setting.", mlog.Err(err))
|
||||
return FakeSetting
|
||||
}
|
||||
return sanitized
|
||||
}
|
||||
return FakeSetting
|
||||
}
|
||||
if o.LdapSettings.BindPassword != nil && *o.LdapSettings.BindPassword != "" {
|
||||
*o.LdapSettings.BindPassword = FakeSetting
|
||||
}
|
||||
@@ -4723,7 +4761,7 @@ func (o *Config) Sanitize(pluginManifests []*Manifest) {
|
||||
}
|
||||
|
||||
if o.SqlSettings.DataSource != nil {
|
||||
*o.SqlSettings.DataSource = FakeSetting
|
||||
*o.SqlSettings.DataSource = sanitizeDataSourceField(*o.SqlSettings.DataSource, "SqlSettings.DataSource")
|
||||
}
|
||||
|
||||
if o.SqlSettings.AtRestEncryptKey != nil {
|
||||
@@ -4735,15 +4773,18 @@ func (o *Config) Sanitize(pluginManifests []*Manifest) {
|
||||
}
|
||||
|
||||
for i := range o.SqlSettings.DataSourceReplicas {
|
||||
o.SqlSettings.DataSourceReplicas[i] = FakeSetting
|
||||
o.SqlSettings.DataSourceReplicas[i] = sanitizeDataSourceField(o.SqlSettings.DataSourceReplicas[i], "SqlSettings.DataSourceReplicas")
|
||||
}
|
||||
|
||||
for i := range o.SqlSettings.DataSourceSearchReplicas {
|
||||
o.SqlSettings.DataSourceSearchReplicas[i] = FakeSetting
|
||||
o.SqlSettings.DataSourceSearchReplicas[i] = sanitizeDataSourceField(o.SqlSettings.DataSourceSearchReplicas[i], "SqlSettings.DataSourceSearchReplicas")
|
||||
}
|
||||
|
||||
for i := range o.SqlSettings.ReplicaLagSettings {
|
||||
o.SqlSettings.ReplicaLagSettings[i].DataSource = NewPointer(FakeSetting)
|
||||
if o.SqlSettings.ReplicaLagSettings[i].DataSource != nil {
|
||||
sanitized := sanitizeDataSourceField(*o.SqlSettings.ReplicaLagSettings[i].DataSource, "SqlSettings.ReplicaLagSettings")
|
||||
o.SqlSettings.ReplicaLagSettings[i].DataSource = NewPointer(sanitized)
|
||||
}
|
||||
}
|
||||
|
||||
if o.MessageExportSettings.GlobalRelaySettings != nil &&
|
||||
@@ -4763,6 +4804,59 @@ func (o *Config) Sanitize(pluginManifests []*Manifest) {
|
||||
o.PluginSettings.Sanitize(pluginManifests)
|
||||
}
|
||||
|
||||
// SanitizeDataSource redacts sensitive information (username and password) from a database
|
||||
// connection string while preserving other connection parameters.
|
||||
//
|
||||
// Parameters:
|
||||
// - driverName: The database driver name (postgres or mysql)
|
||||
// - dataSource: The database connection string to sanitize
|
||||
//
|
||||
// Returns:
|
||||
// - The sanitized connection string with username/password replaced by SanitizedPassword
|
||||
// - An error if the driverName is not supported or if parsing fails
|
||||
//
|
||||
// Examples:
|
||||
// - PostgreSQL: "postgres://user:pass@host:5432/db" -> "postgres://****:****@host:5432/db"
|
||||
// - MySQL: "user:pass@tcp(host:3306)/db" -> "****:****@tcp(host:3306)/db"
|
||||
func SanitizeDataSource(driverName, dataSource string) (string, error) {
|
||||
// Handle empty data source
|
||||
if dataSource == "" {
|
||||
return "", nil
|
||||
}
|
||||
|
||||
switch driverName {
|
||||
case DatabaseDriverPostgres:
|
||||
u, err := url.Parse(dataSource)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
u.User = url.UserPassword(SanitizedPassword, SanitizedPassword)
|
||||
|
||||
// Remove username and password from query string
|
||||
params := u.Query()
|
||||
params.Del("user")
|
||||
params.Del("password")
|
||||
u.RawQuery = params.Encode()
|
||||
|
||||
// Unescape the URL to make it human-readable
|
||||
out, err := url.QueryUnescape(u.String())
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return out, nil
|
||||
case DatabaseDriverMysql:
|
||||
cfg, err := mysql.ParseDSN(dataSource)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
cfg.User = SanitizedPassword
|
||||
cfg.Passwd = SanitizedPassword
|
||||
return cfg.FormatDSN(), nil
|
||||
default:
|
||||
return "", errors.New("invalid drivername. Not postgres or mysql.")
|
||||
}
|
||||
}
|
||||
|
||||
type FilterTag struct {
|
||||
TagType string
|
||||
TagName string
|
||||
|
||||
@@ -1521,7 +1521,7 @@ func TestConfigSanitize(t *testing.T) {
|
||||
QueryTimeLag: NewPointer("QueryTimeLag"),
|
||||
}}
|
||||
|
||||
c.Sanitize(nil)
|
||||
c.Sanitize(nil, nil)
|
||||
|
||||
assert.Equal(t, FakeSetting, *c.LdapSettings.BindPassword)
|
||||
assert.Equal(t, FakeSetting, *c.FileSettings.PublicLinkSalt)
|
||||
@@ -1543,10 +1543,20 @@ func TestConfigSanitize(t *testing.T) {
|
||||
t.Run("with default config", func(t *testing.T) {
|
||||
c := Config{}
|
||||
c.SetDefaults()
|
||||
c.Sanitize(nil)
|
||||
c.Sanitize(nil, nil)
|
||||
|
||||
assert.Len(t, c.SqlSettings.ReplicaLagSettings, 0)
|
||||
})
|
||||
|
||||
t.Run("partially sanitize DataSource", func(t *testing.T) {
|
||||
c := Config{}
|
||||
c.SetDefaults()
|
||||
*c.SqlSettings.DataSource = "postgres://mmuser:mostest@localhost:5432/mattermost_test?sslmode=disable"
|
||||
c.Sanitize(nil, &SanitizeOptions{PartiallyRedactDataSources: true})
|
||||
|
||||
expectedURL := "postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost:5432/mattermost_test?sslmode=disable"
|
||||
assert.Equal(t, expectedURL, *c.SqlSettings.DataSource)
|
||||
})
|
||||
}
|
||||
|
||||
func TestPluginSettingsSanitize(t *testing.T) {
|
||||
@@ -1689,6 +1699,56 @@ func TestPluginSettingsSanitize(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeDataSource(t *testing.T) {
|
||||
t.Run(DatabaseDriverPostgres, func(t *testing.T) {
|
||||
testCases := []struct {
|
||||
Original string
|
||||
Sanitized string
|
||||
}{
|
||||
{
|
||||
"",
|
||||
"",
|
||||
},
|
||||
{
|
||||
"postgres://mmuser:mostest@localhost",
|
||||
"postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost",
|
||||
},
|
||||
{
|
||||
"postgres://mmuser:mostest@localhost/dummy?sslmode=disable",
|
||||
"postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost/dummy?sslmode=disable",
|
||||
},
|
||||
{
|
||||
"postgres://localhost/dummy?sslmode=disable&user=mmuser&password=mostest",
|
||||
"postgres://" + SanitizedPassword + ":" + SanitizedPassword + "@localhost/dummy?sslmode=disable",
|
||||
},
|
||||
}
|
||||
driver := DatabaseDriverPostgres
|
||||
for _, tc := range testCases {
|
||||
out, err := SanitizeDataSource(driver, tc.Original)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tc.Sanitized, out)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(DatabaseDriverMysql, func(t *testing.T) {
|
||||
testCases := []struct {
|
||||
Original string
|
||||
Sanitized string
|
||||
}{
|
||||
{
|
||||
"mmuser:mostest@tcp(localhost:3306)/mattermost_test?charset=utf8mb4,utf8&readTimeout=30s&writeTimeout=30s",
|
||||
SanitizedPassword + ":" + SanitizedPassword + "@tcp(localhost:3306)/mattermost_test?charset=utf8mb4,utf8&readTimeout=30s&writeTimeout=30s",
|
||||
},
|
||||
}
|
||||
driver := DatabaseDriverMysql
|
||||
for _, tc := range testCases {
|
||||
out, err := SanitizeDataSource(driver, tc.Original)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tc.Sanitized, out)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestConfigFilteredByTag(t *testing.T) {
|
||||
c := Config{}
|
||||
c.SetDefaults()
|
||||
@@ -2146,7 +2206,7 @@ func TestFilterConfig(t *testing.T) {
|
||||
require.NotEmpty(t, m)
|
||||
require.Equal(t, dsn, m["SqlSettings"].(map[string]any)["DataSource"])
|
||||
|
||||
cfg.Sanitize(nil)
|
||||
cfg.Sanitize(nil, nil)
|
||||
m, err = FilterConfig(cfg, ConfigFilterOptions{
|
||||
GetConfigOptions: GetConfigOptions{
|
||||
RemoveDefaults: true,
|
||||
@@ -2156,7 +2216,7 @@ func TestFilterConfig(t *testing.T) {
|
||||
require.NotEmpty(t, m)
|
||||
require.Equal(t, FakeSetting, m["SqlSettings"].(map[string]any)["DataSource"])
|
||||
|
||||
cfg.Sanitize(nil)
|
||||
cfg.Sanitize(nil, nil)
|
||||
m, err = FilterConfig(cfg, ConfigFilterOptions{
|
||||
GetConfigOptions: GetConfigOptions{
|
||||
RemoveDefaults: true,
|
||||
|
||||
@@ -6,7 +6,6 @@ package sql
|
||||
import (
|
||||
"context"
|
||||
dbsql "database/sql"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -57,7 +56,7 @@ func SetupConnection(logger mlog.LoggerIFace, connType string, dataSource string
|
||||
}
|
||||
|
||||
// At this point, we have passed sql.Open, so we deliberately ignore any errors.
|
||||
sanitized, _ := SanitizeDataSource(*settings.DriverName, dataSource)
|
||||
sanitized, _ := model.SanitizeDataSource(*settings.DriverName, dataSource)
|
||||
|
||||
logger = logger.With(
|
||||
mlog.String("database", connType),
|
||||
@@ -101,37 +100,3 @@ func SetupConnection(logger mlog.LoggerIFace, connType string, dataSource string
|
||||
|
||||
return db, nil
|
||||
}
|
||||
|
||||
func SanitizeDataSource(driverName, dataSource string) (string, error) {
|
||||
switch driverName {
|
||||
case model.DatabaseDriverPostgres:
|
||||
u, err := url.Parse(dataSource)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
u.User = url.UserPassword("****", "****")
|
||||
|
||||
// Remove username and password from query string
|
||||
params := u.Query()
|
||||
params.Del("user")
|
||||
params.Del("password")
|
||||
u.RawQuery = params.Encode()
|
||||
|
||||
// Unescape the URL to make it human-readable
|
||||
out, err := url.QueryUnescape(u.String())
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return out, nil
|
||||
case model.DatabaseDriverMysql:
|
||||
cfg, err := mysql.ParseDSN(dataSource)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
cfg.User = "****"
|
||||
cfg.Passwd = "****"
|
||||
return cfg.FormatDSN(), nil
|
||||
default:
|
||||
return "", errors.New("invalid drivername. Not postgres or mysql.")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,8 +6,6 @@ package sql
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/mattermost/mattermost/server/public/model"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -65,53 +63,3 @@ func TestResetReadTimeout(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeDataSource(t *testing.T) {
|
||||
t.Run(model.DatabaseDriverPostgres, func(t *testing.T) {
|
||||
testCases := []struct {
|
||||
Original string
|
||||
Sanitized string
|
||||
}{
|
||||
{
|
||||
"",
|
||||
"//****:****@",
|
||||
},
|
||||
{
|
||||
"postgres://mmuser:mostest@localhost",
|
||||
"postgres://****:****@localhost",
|
||||
},
|
||||
{
|
||||
"postgres://mmuser:mostest@localhost/dummy?sslmode=disable",
|
||||
"postgres://****:****@localhost/dummy?sslmode=disable",
|
||||
},
|
||||
{
|
||||
"postgres://localhost/dummy?sslmode=disable&user=mmuser&password=mostest",
|
||||
"postgres://****:****@localhost/dummy?sslmode=disable",
|
||||
},
|
||||
}
|
||||
driver := model.DatabaseDriverPostgres
|
||||
for _, tc := range testCases {
|
||||
out, err := SanitizeDataSource(driver, tc.Original)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tc.Sanitized, out)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(model.DatabaseDriverMysql, func(t *testing.T) {
|
||||
testCases := []struct {
|
||||
Original string
|
||||
Sanitized string
|
||||
}{
|
||||
{
|
||||
"mmuser:mostest@tcp(localhost:3306)/mattermost_test?charset=utf8mb4,utf8&readTimeout=30s&writeTimeout=30s",
|
||||
"****:****@tcp(localhost:3306)/mattermost_test?charset=utf8mb4,utf8&readTimeout=30s&writeTimeout=30s",
|
||||
},
|
||||
}
|
||||
driver := model.DatabaseDriverMysql
|
||||
for _, tc := range testCases {
|
||||
out, err := SanitizeDataSource(driver, tc.Original)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tc.Sanitized, out)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Ссылка в новой задаче
Block a user