Redact all secrets from config viewing APIs (#17380)

This change adds a `Secret` property to `HelpKV` to identify secrets
like passwords and auth tokens that should not be revealed by the server
in its configuration fetching APIs. Configuration reporting APIs now do
not return secrets.

cmd/admin-handlers-config-kv.go

@@ -241,6 +241,8 @@ func setConfigKV(ctx context.Context, objectAPI ObjectLayer, kvBytes []byte) (re
 // 1. `subsys:target` -> request for config of a single subsystem and target pair.
 // 2. `subsys:` -> request for config of a single subsystem and the default target.
 // 3. `subsys` -> request for config of all targets for the given subsystem.
+//
+// This is a reporting API and config secrets are redacted in the response.
 func (a adminAPIHandlers) GetConfigKVHandler(w http.ResponseWriter, r *http.Request) {
 	ctx := newContext(r, w, "GetConfigKV")
 
@@ -268,7 +270,7 @@ func (a adminAPIHandlers) GetConfigKVHandler(w http.ResponseWriter, r *http.Requ
 		}
 	}
 
-	subSysConfigs, err := cfg.GetSubsysInfo(subSys, target)
+	subSysConfigs, err := cfg.GetSubsysInfo(subSys, target, true)
 	if err != nil {
 		writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
 		return
@@ -490,7 +492,9 @@ func (a adminAPIHandlers) SetConfigHandler(w http.ResponseWriter, r *http.Reques
 }
 
 // GetConfigHandler - GET /minio/admin/v3/config
-// Get config.json of this minio setup.
+//
+// This endpoint is mainly for exporting and backing up the configuration.
+// Secrets are not redacted.
 func (a adminAPIHandlers) GetConfigHandler(w http.ResponseWriter, r *http.Request) {
 	ctx := newContext(r, w, "GetConfig")
 
@@ -507,7 +511,7 @@ func (a adminAPIHandlers) GetConfigHandler(w http.ResponseWriter, r *http.Reques
 	hkvs := config.HelpSubSysMap[""]
 	for _, hkv := range hkvs {
 		// We ignore the error below, as we cannot get one.
-		cfgSubsysItems, _ := cfg.GetSubsysInfo(hkv.Key, "")
+		cfgSubsysItems, _ := cfg.GetSubsysInfo(hkv.Key, "", false)
 
 		for _, item := range cfgSubsysItems {
 			off := item.Config.Get(config.Enable) == config.EnableOff

internal/config/config.go

@@ -221,13 +221,12 @@ const (
 )
 
 // DefaultKVS - default kvs for all sub-systems
-var DefaultKVS map[string]KVS
+var DefaultKVS = map[string]KVS{}
 
 // RegisterDefaultKVS - this function saves input kvsMap
 // globally, this should be called only once preferably
 // during `init()`.
 func RegisterDefaultKVS(kvsMap map[string]KVS) {
-	DefaultKVS = map[string]KVS{}
 	for subSys, kvs := range kvsMap {
 		DefaultKVS[subSys] = kvs
 	}
@@ -236,14 +235,13 @@ func RegisterDefaultKVS(kvsMap map[string]KVS) {
 // HelpSubSysMap - help for all individual KVS for each sub-systems
 // also carries a special empty sub-system which dumps
 // help for each sub-system key.
-var HelpSubSysMap map[string]HelpKVS
+var HelpSubSysMap = map[string]HelpKVS{}
 
 // RegisterHelpSubSys - this function saves
 // input help KVS for each sub-system globally,
 // this function should be called only once
 // preferably in during `init()`.
 func RegisterHelpSubSys(helpKVSMap map[string]HelpKVS) {
-	HelpSubSysMap = map[string]HelpKVS{}
 	for subSys, hkvs := range helpKVSMap {
 		HelpSubSysMap[subSys] = hkvs
 	}
@@ -251,12 +249,11 @@ func RegisterHelpSubSys(helpKVSMap map[string]HelpKVS) {
 
 // HelpDeprecatedSubSysMap - help for all deprecated sub-systems, that may be
 // removed in the future.
-var HelpDeprecatedSubSysMap map[string]HelpKV
+var HelpDeprecatedSubSysMap = map[string]HelpKV{}
 
 // RegisterHelpDeprecatedSubSys - saves input help KVS for deprecated
 // sub-systems globally. Should be called only once at init.
 func RegisterHelpDeprecatedSubSys(helpDeprecatedKVMap map[string]HelpKV) {
-	HelpDeprecatedSubSysMap = map[string]HelpKV{}
 	for k, v := range helpDeprecatedKVMap {
 		HelpDeprecatedSubSysMap[k] = v
 	}
@@ -1147,7 +1144,11 @@ const (
 // This function only works for a subset of sub-systems, others return
 // `ValueSourceAbsent`. FIXME: some parameters have custom environment
 // variables for which support needs to be added.
-func (c Config) ResolveConfigParam(subSys, target, cfgParam string) (value string, cs ValueSource) {
+//
+// When redactSecrets is true, the returned value is empty if the configuration
+// parameter is a secret, and the returned isRedacted flag is set.
+func (c Config) ResolveConfigParam(subSys, target, cfgParam string, redactSecrets bool,
+) (value string, cs ValueSource, isRedacted bool) {
 	// cs = ValueSourceAbsent initially as it is iota by default.
 
 	// Initially only support OpenID
@@ -1174,6 +1175,18 @@ func (c Config) ResolveConfigParam(subSys, target, cfgParam string) (value strin
 		target = Default
 	}
 
+	if redactSecrets {
+		// If the configuration parameter is a secret, make sure to redact it when
+		// we return.
+		helpKV, _ := HelpSubSysMap[subSys].Lookup(cfgParam)
+		if helpKV.Secret {
+			defer func() {
+				value = ""
+				isRedacted = true
+			}()
+		}
+	}
+
 	envVar := getEnvVarName(subSys, target, cfgParam)
 
 	// Lookup Env var.
@@ -1211,8 +1224,7 @@ type KVSrc struct {
 
 // GetResolvedConfigParams returns all applicable config parameters with their
 // value sources.
-func (c Config) GetResolvedConfigParams(subSys, target string) ([]KVSrc, error) {
+func (c Config) GetResolvedConfigParams(subSys, target string, redactSecrets bool) ([]KVSrc, error) {
-	// Initially only support OpenID
 	if !resolvableSubsystems.Contains(subSys) {
 		return nil, Errorf("unsupported subsystem: %s", subSys)
 	}
@@ -1225,13 +1237,18 @@ func (c Config) GetResolvedConfigParams(subSys, target string) ([]KVSrc, error)
 
 	r := make([]KVSrc, 0, len(defKVS)+1)
 	for _, kv := range defKVS {
-		v, vs := c.ResolveConfigParam(subSys, target, kv.Key)
+		v, vs, isRedacted := c.ResolveConfigParam(subSys, target, kv.Key, redactSecrets)
 
 		// Fix `vs` when default.
 		if v == kv.Value {
 			vs = ValueSourceDef
 		}
 
+		if redactSecrets && isRedacted {
+			// Skip adding redacted secrets to the output.
+			continue
+		}
+
 		r = append(r, KVSrc{
 			Key:   kv.Key,
 			Value: v,
@@ -1239,8 +1256,9 @@ func (c Config) GetResolvedConfigParams(subSys, target string) ([]KVSrc, error)
 		})
 	}
 
-	// Add the comment key as well if non-empty.
+	// Add the comment key as well if non-empty (and comments are never
-	v, vs := c.ResolveConfigParam(subSys, target, Comment)
+	// redacted).
+	v, vs, _ := c.ResolveConfigParam(subSys, target, Comment, redactSecrets)
 	if vs != ValueSourceDef {
 		r = append(r, KVSrc{
 			Key:   Comment,
@@ -1252,12 +1270,59 @@ func (c Config) GetResolvedConfigParams(subSys, target string) ([]KVSrc, error)
 	return r, nil
 }
 
-func (c Config) getTargetKVS(subSys, target string) KVS {
+// getTargetKVS returns configuration KVs for the given subsystem and target. It
+// does not return any secrets in the configuration values when `redactSecrets`
+// is set.
+func (c Config) getTargetKVS(subSys, target string, redactSecrets bool) KVS {
 	store, ok := c[subSys]
 	if !ok {
 		return nil
 	}
-	return store[target]
+
+	// Lookup will succeed, because this function only works with valid subSys
+	// values.
+	resultKVS := make([]KV, 0, len(store[target]))
+	hkvs := HelpSubSysMap[subSys]
+	for _, kv := range store[target] {
+		hkv, _ := hkvs.Lookup(kv.Key)
+		if hkv.Secret && redactSecrets && kv.Value != "" {
+			// Skip returning secrets.
+			continue
+			// clonedKV := kv
+			// clonedKV.Value = redactedSecret
+			// resultKVS = append(resultKVS, clonedKV)
+		} else {
+			resultKVS = append(resultKVS, kv)
+		}
+	}
+
+	return resultKVS
+}
+
+// getTargetEnvs returns configured environment variable settings for the given
+// subsystem and target.
+func (c Config) getTargetEnvs(subSys, target string, defKVS KVS, redactSecrets bool) map[string]EnvPair {
+	hkvs := HelpSubSysMap[subSys]
+	envMap := make(map[string]EnvPair)
+
+	// Add all env vars that are set.
+	for _, kv := range defKVS {
+		envName := getEnvVarName(subSys, target, kv.Key)
+		envPair := EnvPair{
+			Name:  envName,
+			Value: env.Get(envName, ""),
+		}
+		if envPair.Value != "" {
+			hkv, _ := hkvs.Lookup(kv.Key)
+			if hkv.Secret && redactSecrets {
+				// Skip adding any secret to the returned value.
+				continue
+				// envPair.Value = redactedSecret
+			}
+			envMap[kv.Key] = envPair
+		}
+	}
+	return envMap
 }
 
 // EnvPair represents an environment variable and its value.
@@ -1278,7 +1343,7 @@ type SubsysInfo struct {
 // GetSubsysInfo returns `SubsysInfo`s for all targets for the subsystem, when
 // target is empty. Otherwise returns `SubsysInfo` for the desired target only.
 // To request the default target only, target must be set to `Default`.
-func (c Config) GetSubsysInfo(subSys, target string) ([]SubsysInfo, error) {
+func (c Config) GetSubsysInfo(subSys, target string, redactSecrets bool) ([]SubsysInfo, error) {
 	// Check if config param requested is valid.
 	defKVS1, ok := DefaultKVS[subSys]
 	if !ok {
@@ -1314,28 +1379,13 @@ func (c Config) GetSubsysInfo(subSys, target string) ([]SubsysInfo, error) {
 
 	r := make([]SubsysInfo, 0, len(targets))
 	for _, target := range targets {
-		kvs := c.getTargetKVS(subSys, target)
+		r = append(r, SubsysInfo{
-		cs := SubsysInfo{
 			SubSys:   subSys,
 			Target:   target,
 			Defaults: defKVS,
-			Config:   kvs,
+			Config:   c.getTargetKVS(subSys, target, redactSecrets),
-			EnvMap:   make(map[string]EnvPair),
+			EnvMap:   c.getTargetEnvs(subSys, target, defKVS, redactSecrets),
-		}
+		})
-
-		// Add all env vars that are set.
-		for _, kv := range defKVS {
-			envName := getEnvVarName(subSys, target, kv.Key)
-			envPair := EnvPair{
-				Name:  envName,
-				Value: env.Get(envName, ""),
-			}
-			if envPair.Value != "" {
-				cs.EnvMap[kv.Key] = envPair
-			}
-		}
-
-		r = append(r, cs)
 	}
 
 	return r, nil

internal/config/help.go

@@ -25,10 +25,14 @@ type HelpKV struct {
 	Description string `json:"description"`
 	Optional    bool   `json:"optional"`
 
-	// Indicates if the value contains sensitive info
+	// Indicates if the value contains sensitive info that shouldn't be exposed
-	// that shouldn't be exposed in certain apis
+	// in certain apis (such as Health Diagnostics/Callhome)
 	Sensitive bool `json:"-"`
 
+	// Indicates if the value is a secret such as a password that shouldn't be
+	// exposed by the server
+	Secret bool `json:"-"`
+
 	// Indicates if sub-sys supports multiple targets.
 	MultipleTargets bool `json:"multipleTargets"`
 }

internal/config/identity/ldap/config.go

@@ -174,7 +174,7 @@ func Lookup(s config.Config, rootCAs *x509.CertPool) (l Config, err error) {
 	getCfgVal := func(cfgParam string) string {
 		// As parameters are already validated, we skip checking
 		// if the config param was found.
-		val, _ := s.ResolveConfigParam(config.IdentityLDAPSubSys, config.Default, cfgParam)
+		val, _, _ := s.ResolveConfigParam(config.IdentityLDAPSubSys, config.Default, cfgParam, false)
 		return val
 	}
 
@@ -272,7 +272,7 @@ func (l *Config) GetConfigInfo(s config.Config, cfgName string) ([]madmin.IDPCfg
 	if cfgName != madmin.Default {
 		return nil, ErrProviderConfigNotFound
 	}
-	kvsrcs, err := s.GetResolvedConfigParams(config.IdentityLDAPSubSys, cfgName)
+	kvsrcs, err := s.GetResolvedConfigParams(config.IdentityLDAPSubSys, cfgName, true)
 	if err != nil {
 		return nil, err
 	}

internal/config/identity/ldap/help.go

@@ -52,6 +52,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         UserDNSearchBaseDN,

internal/config/identity/openid/help.go

@@ -47,6 +47,7 @@ var (
 			Description: `secret for the unique public identifier for apps` + defaultHelpPostfix(ClientSecret),
 			Sensitive:   true,
 			Type:        "string",
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         RolePolicy,

internal/config/identity/openid/openid.go

@@ -230,7 +230,7 @@ func LookupConfig(s config.Config, transport http.RoundTripper, closeRespFn func
 		getCfgVal := func(cfgParam string) string {
 			// As parameters are already validated, we skip checking
 			// if the config param was found.
-			val, _ := s.ResolveConfigParam(config.IdentityOpenIDSubSys, cfgName, cfgParam)
+			val, _, _ := s.ResolveConfigParam(config.IdentityOpenIDSubSys, cfgName, cfgParam, false)
 			return val
 		}
 
@@ -416,7 +416,7 @@ func (r *Config) GetConfigInfo(s config.Config, cfgName string) ([]madmin.IDPCfg
 		return nil, ErrProviderConfigNotFound
 	}
 
-	kvsrcs, err := s.GetResolvedConfigParams(config.IdentityOpenIDSubSys, cfgName)
+	kvsrcs, err := s.GetResolvedConfigParams(config.IdentityOpenIDSubSys, cfgName, true)
 	if err != nil {
 		return nil, err
 	}

internal/config/identity/plugin/config.go

@@ -89,6 +89,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         RolePolicy,

internal/config/lambda/help.go

@@ -37,6 +37,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         config.Comment,

internal/config/notify/help.go

@@ -43,6 +43,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         target.WebhookQueueDir,
@@ -191,6 +192,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         target.KafkaSASLMechanism,
@@ -287,6 +289,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         target.MqttQoS,
@@ -438,6 +441,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         target.NATSToken,
@@ -445,6 +449,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         target.NATSTLS,
@@ -621,6 +626,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         config.Comment,
@@ -654,6 +660,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         target.RedisQueueDir,

internal/config/policy/opa/help.go

@@ -38,6 +38,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         config.Comment,

internal/config/policy/plugin/config.go

@@ -131,7 +131,7 @@ func LookupConfig(s config.Config, httpSettings xhttp.ConnSettings, closeRespFn
 	getCfg := func(cfgParam string) string {
 		// As parameters are already validated, we skip checking
 		// if the config param was found.
-		val, _ := s.ResolveConfigParam(config.PolicyPluginSubSys, config.Default, cfgParam)
+		val, _, _ := s.ResolveConfigParam(config.PolicyPluginSubSys, config.Default, cfgParam, false)
 		return val
 	}
 

internal/config/policy/plugin/help.go

@@ -38,6 +38,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         EnableHTTP2,

internal/config/subnet/help.go

@@ -39,6 +39,7 @@ var (
 			Description: "Subnet api key for the cluster" + defaultHelpPostfix(config.APIKey),
 			Optional:    true,
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         config.Proxy,

internal/logger/help.go

@@ -36,6 +36,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         ClientCert,
@@ -84,6 +85,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         ClientCert,
@@ -138,6 +140,7 @@ var (
 			Optional:    true,
 			Type:        "string",
 			Sensitive:   true,
+			Secret:      true,
 		},
 		config.HelpKV{
 			Key:         KafkaSASLMechanism,