From f3248a4b370ea3f80d256b74baa3c51b3f70fb74 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Fri, 23 Jun 2023 07:45:27 -0700 Subject: [PATCH] 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 | 10 +- internal/config/config.go | 118 +++++++++++++++------- internal/config/help.go | 8 +- internal/config/identity/ldap/config.go | 4 +- internal/config/identity/ldap/help.go | 1 + internal/config/identity/openid/help.go | 1 + internal/config/identity/openid/openid.go | 4 +- internal/config/identity/plugin/config.go | 1 + internal/config/lambda/help.go | 1 + internal/config/notify/help.go | 7 ++ internal/config/policy/opa/help.go | 1 + internal/config/policy/plugin/config.go | 2 +- internal/config/policy/plugin/help.go | 1 + internal/config/subnet/help.go | 1 + internal/logger/help.go | 3 + 15 files changed, 119 insertions(+), 44 deletions(-) diff --git a/cmd/admin-handlers-config-kv.go b/cmd/admin-handlers-config-kv.go index ad71a1225..8dc86bbc7 100644 --- a/cmd/admin-handlers-config-kv.go +++ b/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 diff --git a/internal/config/config.go b/internal/config/config.go index 570f18314..40efc93e0 100644 --- a/internal/config/config.go +++ b/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) { - // Initially only support OpenID +func (c Config) GetResolvedConfigParams(subSys, target string, redactSecrets bool) ([]KVSrc, error) { 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. - v, vs := c.ResolveConfigParam(subSys, target, Comment) + // Add the comment key as well if non-empty (and comments are never + // 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) - cs := SubsysInfo{ + r = append(r, SubsysInfo{ SubSys: subSys, Target: target, Defaults: defKVS, - Config: kvs, - 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 != "" { - cs.EnvMap[kv.Key] = envPair - } - } - - r = append(r, cs) + Config: c.getTargetKVS(subSys, target, redactSecrets), + EnvMap: c.getTargetEnvs(subSys, target, defKVS, redactSecrets), + }) } return r, nil diff --git a/internal/config/help.go b/internal/config/help.go index ce6ec926a..9f0d265c0 100644 --- a/internal/config/help.go +++ b/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 - // that shouldn't be exposed in certain apis + // Indicates if the value contains sensitive info that shouldn't be exposed + // 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"` } diff --git a/internal/config/identity/ldap/config.go b/internal/config/identity/ldap/config.go index 0e4ea4d0d..4bb4d8249 100644 --- a/internal/config/identity/ldap/config.go +++ b/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 } diff --git a/internal/config/identity/ldap/help.go b/internal/config/identity/ldap/help.go index 2d6227369..035a9d80f 100644 --- a/internal/config/identity/ldap/help.go +++ b/internal/config/identity/ldap/help.go @@ -52,6 +52,7 @@ var ( Optional: true, Type: "string", Sensitive: true, + Secret: true, }, config.HelpKV{ Key: UserDNSearchBaseDN, diff --git a/internal/config/identity/openid/help.go b/internal/config/identity/openid/help.go index f8301ef4f..1469034c5 100644 --- a/internal/config/identity/openid/help.go +++ b/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, diff --git a/internal/config/identity/openid/openid.go b/internal/config/identity/openid/openid.go index 9f16a1b37..f5e0c3b67 100644 --- a/internal/config/identity/openid/openid.go +++ b/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 } diff --git a/internal/config/identity/plugin/config.go b/internal/config/identity/plugin/config.go index b991940ee..812cea5aa 100644 --- a/internal/config/identity/plugin/config.go +++ b/internal/config/identity/plugin/config.go @@ -89,6 +89,7 @@ var ( Optional: true, Type: "string", Sensitive: true, + Secret: true, }, config.HelpKV{ Key: RolePolicy, diff --git a/internal/config/lambda/help.go b/internal/config/lambda/help.go index 62fa3ccf8..2f728ad38 100644 --- a/internal/config/lambda/help.go +++ b/internal/config/lambda/help.go @@ -37,6 +37,7 @@ var ( Optional: true, Type: "string", Sensitive: true, + Secret: true, }, config.HelpKV{ Key: config.Comment, diff --git a/internal/config/notify/help.go b/internal/config/notify/help.go index 8f00cc4e5..d327d6cb1 100644 --- a/internal/config/notify/help.go +++ b/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, diff --git a/internal/config/policy/opa/help.go b/internal/config/policy/opa/help.go index 4865406e3..c0825b657 100644 --- a/internal/config/policy/opa/help.go +++ b/internal/config/policy/opa/help.go @@ -38,6 +38,7 @@ var ( Optional: true, Type: "string", Sensitive: true, + Secret: true, }, config.HelpKV{ Key: config.Comment, diff --git a/internal/config/policy/plugin/config.go b/internal/config/policy/plugin/config.go index 078248f07..636527a96 100644 --- a/internal/config/policy/plugin/config.go +++ b/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 } diff --git a/internal/config/policy/plugin/help.go b/internal/config/policy/plugin/help.go index d71872c0c..182de61ea 100644 --- a/internal/config/policy/plugin/help.go +++ b/internal/config/policy/plugin/help.go @@ -38,6 +38,7 @@ var ( Optional: true, Type: "string", Sensitive: true, + Secret: true, }, config.HelpKV{ Key: EnableHTTP2, diff --git a/internal/config/subnet/help.go b/internal/config/subnet/help.go index f16713f33..33014528b 100644 --- a/internal/config/subnet/help.go +++ b/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, diff --git a/internal/logger/help.go b/internal/logger/help.go index 3bc1419ff..db9379a16 100644 --- a/internal/logger/help.go +++ b/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,