Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys/config: config state endpoint #7424

Merged
merged 22 commits into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7a45f56
sys/config: initial work on adding config state endpoint
calvn Sep 4, 2019
66afa3c
server/config: add tests, fix Sanitized method
calvn Sep 4, 2019
e612b78
thread config through NewTestCluster's config to avoid panic on dev m…
calvn Sep 4, 2019
15f41ee
properly guard endpoint against request forwarding
calvn Sep 4, 2019
c4a9edb
add http tests, guard against panics on nil RawConfig
calvn Sep 5, 2019
9cfbdb3
ensure non-nil rawConfig on NewTestCluster cores
calvn Sep 5, 2019
d1d2919
Merge remote-tracking branch 'origin/master' into sys-config-state
calvn Sep 5, 2019
048961c
update non-forwarding logic
calvn Sep 16, 2019
0813829
Merge remote-tracking branch 'origin/master' into sys-config-state
calvn Sep 17, 2019
31c2454
fix imports; use no-forward handler
calvn Sep 17, 2019
6567fac
add missing config test fixture; update gitignore
calvn Sep 17, 2019
543f765
Merge remote-tracking branch 'origin/master' into sys-config-state
calvn Sep 20, 2019
26fdcb5
return sanitized config as a map
calvn Sep 20, 2019
7fef0b8
fix test, use deep.Equal to check for equality
calvn Sep 21, 2019
66eb350
fix http test
calvn Sep 21, 2019
507f708
Merge remote-tracking branch 'origin/master' into sys-config-state
calvn Oct 3, 2019
95ab4e0
minor comment fix
calvn Oct 3, 2019
da85f38
config: change Sanitized to return snake-cased keys, update tests
calvn Oct 7, 2019
219d6ee
Merge remote-tracking branch 'origin/master' into sys-config-state
calvn Oct 7, 2019
d5b5a9f
core: hold rlock when reading config; add docstring
calvn Oct 7, 2019
d62aa0f
update docstring
calvn Oct 8, 2019
86a808c
Merge branch 'master' into sys-config-state
calvn Oct 8, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Vagrantfile
# Configs
*.hcl
!command/agent/config/test-fixtures/*.hcl
!command/server/test-fixtures/*.hcl


.DS_Store
Expand Down
5 changes: 4 additions & 1 deletion command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ func (c *ServerCommand) Run(args []string) int {
}

coreConfig := &vault.CoreConfig{
RawConfig: config,
Physical: backend,
RedirectAddr: config.Storage.RedirectAddr,
StorageType: config.Storage.Type,
Expand Down Expand Up @@ -973,7 +974,7 @@ CLUSTER_SYNTHESIS_COMPLETE:
}
props["max_request_size"] = fmt.Sprintf("%d", maxRequestSize)

var maxRequestDuration time.Duration = vault.DefaultMaxRequestDuration
maxRequestDuration := vault.DefaultMaxRequestDuration
if valRaw, ok := lnConfig.Config["max_request_duration"]; ok {
val, err := parseutil.ParseDurationSecond(valRaw)
if err != nil {
Expand Down Expand Up @@ -1415,6 +1416,8 @@ CLUSTER_SYNTHESIS_COMPLETE:
goto RUNRELOADFUNCS
}

core.SetConfig(config)

if config.LogLevel != "" {
configLogLevel := strings.ToLower(strings.TrimSpace(config.LogLevel))
switch configLogLevel {
Expand Down
125 changes: 125 additions & 0 deletions command/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -905,3 +905,128 @@ func parseTelemetry(result *Config, list *ast.ObjectList) error {

return nil
}

// Sanitized returns a copy of the config with all values that are considered
// sensitive stripped. It also strips all `*Raw` values that are mainly
// used for parsing.
//
// Specifically, the fields that this method strips are:
// - Storage.Config
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like stripping out Storage.Config is extreme. I understand that it's impractical to be more fine-grained in general, but our enterprise customers are mostly going to be using Consul and Raft. Can we special-case those and only strip out the truly sensitive config, e.g. consul token?

Copy link
Contributor Author

@calvn calvn Sep 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we were going to show certain values from Config, I think the safer approach would be to have an allow-list rather than a set of values to strip, as they could be easily overlooked/forgotten since it's better to forget adding a non-sensitive value than leaking a sensitive one. I started down this road initially, but stopped after realizing the numerous types of storage backends that we support and the number of fields that we'd need to include for each of those.

I think the cleaner approach would be to have a separate endpoint that displayed the complete configuration file params, including the Config map (and that could be included in the debug bundle in a future release), but didn't want to add this prematurely.

// - HAStorage.Config
// - Seals.Config
// - Telemetry.CirconusAPIToken
func (c *Config) Sanitized() map[string]interface{} {
result := map[string]interface{}{
"cache_size": c.CacheSize,
"disable_cache": c.DisableCache,
"disable_mlock": c.DisableMlock,
"disable_printable_check": c.DisablePrintableCheck,

"enable_ui": c.EnableUI,

"max_lease_ttl": c.MaxLeaseTTL,
"default_lease_ttl": c.DefaultLeaseTTL,

"default_max_request_duration": c.DefaultMaxRequestDuration,

"cluster_name": c.ClusterName,
"cluster_cipher_suites": c.ClusterCipherSuites,

"plugin_directory": c.PluginDirectory,

"log_level": c.LogLevel,
"log_format": c.LogFormat,

"pid_file": c.PidFile,
"raw_storage_endpoint": c.EnableRawEndpoint,

"api_addr": c.APIAddr,
"cluster_addr": c.ClusterAddr,
"disable_clustering": c.DisableClustering,

"disable_performance_standby": c.DisablePerformanceStandby,

"disable_sealwrap": c.DisableSealWrap,

"disable_indexing": c.DisableIndexing,
}

// Sanitize listeners
if len(c.Listeners) != 0 {
var sanitizedListeners []interface{}
for _, ln := range c.Listeners {
cleanLn := map[string]interface{}{
"type": ln.Type,
"config": ln.Config,
}
sanitizedListeners = append(sanitizedListeners, cleanLn)
}
result["listeners"] = sanitizedListeners
}

// Sanitize storage stanza
if c.Storage != nil {
sanitizedStorage := map[string]interface{}{
"type": c.Storage.Type,
"redirect_addr": c.Storage.RedirectAddr,
"cluster_addr": c.Storage.ClusterAddr,
"disable_clustering": c.Storage.DisableClustering,
}
result["storage"] = sanitizedStorage
}

// Sanitize HA storage stanza
if c.HAStorage != nil {
sanitizedHAStorage := map[string]interface{}{
"type": c.HAStorage.Type,
"redirect_addr": c.HAStorage.RedirectAddr,
"cluster_addr": c.HAStorage.ClusterAddr,
"disable_clustering": c.HAStorage.DisableClustering,
}
result["ha_storage"] = sanitizedHAStorage
}

// Sanitize seals stanza
if len(c.Seals) != 0 {
var sanitizedSeals []interface{}
for _, s := range c.Seals {
cleanSeal := map[string]interface{}{
"type": s.Type,
"disabled": s.Disabled,
}
sanitizedSeals = append(sanitizedSeals, cleanSeal)
}
result["seals"] = sanitizedSeals
}

// Sanitize telemetry stanza
if c.Telemetry != nil {
sanitizedTelemetry := map[string]interface{}{
"statsite_address": c.Telemetry.StatsiteAddr,
"statsd_address": c.Telemetry.StatsdAddr,
"disable_hostname": c.Telemetry.DisableHostname,
"circonus_api_token": "",
"circonus_api_app": c.Telemetry.CirconusAPIApp,
"circonus_api_url": c.Telemetry.CirconusAPIURL,
"circonus_submission_interval": c.Telemetry.CirconusSubmissionInterval,
"circonus_submission_url": c.Telemetry.CirconusCheckSubmissionURL,
"circonus_check_id": c.Telemetry.CirconusCheckID,
"circonus_check_force_metric_activation": c.Telemetry.CirconusCheckForceMetricActivation,
"circonus_check_instance_id": c.Telemetry.CirconusCheckInstanceID,
"circonus_check_search_tag": c.Telemetry.CirconusCheckSearchTag,
"circonus_check_tags": c.Telemetry.CirconusCheckTags,
"circonus_check_display_name": c.Telemetry.CirconusCheckDisplayName,
"circonus_broker_id": c.Telemetry.CirconusBrokerID,
"circonus_broker_select_tag": c.Telemetry.CirconusBrokerSelectTag,
"dogstatsd_addr": c.Telemetry.DogStatsDAddr,
"dogstatsd_tags": c.Telemetry.DogStatsDTags,
"prometheus_retention_time": c.Telemetry.PrometheusRetentionTime,
"stackdriver_project_id": c.Telemetry.StackdriverProjectID,
"stackdriver_location": c.Telemetry.StackdriverLocation,
"stackdriver_namespace": c.Telemetry.StackdriverNamespace,
}
result["telemetry"] = sanitizedTelemetry
}

return result
}
85 changes: 85 additions & 0 deletions command/server/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"
"time"

"github.com/go-test/deep"
"github.com/hashicorp/hcl"
"github.com/hashicorp/hcl/hcl/ast"
)
Expand Down Expand Up @@ -349,6 +350,90 @@ func TestLoadConfigDir(t *testing.T) {
}
}

func TestConfig_Sanitized(t *testing.T) {
config, err := LoadConfigFile("./test-fixtures/config3.hcl")
if err != nil {
t.Fatalf("err: %s", err)
}
sanitizedConfig := config.Sanitized()

expected := map[string]interface{}{
"api_addr": "top_level_api_addr",
"cache_size": 0,
"cluster_addr": "top_level_cluster_addr",
"cluster_cipher_suites": "",
"cluster_name": "testcluster",
"default_lease_ttl": 10 * time.Hour,
"default_max_request_duration": 0 * time.Second,
"disable_cache": true,
"disable_clustering": false,
"disable_indexing": false,
"disable_mlock": true,
"disable_performance_standby": false,
"disable_printable_check": false,
"disable_sealwrap": true,
"raw_storage_endpoint": true,
"enable_ui": true,
"ha_storage": map[string]interface{}{
"cluster_addr": "top_level_cluster_addr",
"disable_clustering": true,
"redirect_addr": "top_level_api_addr",
"type": "consul"},
"listeners": []interface{}{
map[string]interface{}{
"config": map[string]interface{}{
"address": "127.0.0.1:443",
},
"type": "tcp",
},
},
"log_format": "",
"log_level": "",
"max_lease_ttl": 10 * time.Hour,
"pid_file": "./pidfile",
"plugin_directory": "",
"seals": []interface{}{
map[string]interface{}{
"disabled": false,
"type": "awskms",
},
},
"storage": map[string]interface{}{
"cluster_addr": "top_level_cluster_addr",
"disable_clustering": false,
"redirect_addr": "top_level_api_addr",
"type": "consul",
},
"telemetry": map[string]interface{}{
"circonus_api_app": "",
"circonus_api_token": "",
"circonus_api_url": "",
"circonus_broker_id": "",
"circonus_broker_select_tag": "",
"circonus_check_display_name": "",
"circonus_check_force_metric_activation": "",
"circonus_check_id": "",
"circonus_check_instance_id": "",
"circonus_check_search_tag": "",
"circonus_submission_url": "",
"circonus_check_tags": "",
"circonus_submission_interval": "",
"disable_hostname": false,
"dogstatsd_addr": "",
"dogstatsd_tags": []string(nil),
"prometheus_retention_time": 24 * time.Hour,
"stackdriver_location": "",
"stackdriver_namespace": "",
"stackdriver_project_id": "",
"statsd_address": "bar",
"statsite_address": ""},
}

if diff := deep.Equal(sanitizedConfig, expected); len(diff) > 0 {
t.Fatalf("bad, diff: %#v", diff)
}
}

func TestParseListeners(t *testing.T) {
obj, _ := hcl.Parse(strings.TrimSpace(`
listener "tcp" {
Expand Down
41 changes: 41 additions & 0 deletions command/server/test-fixtures/config3.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
disable_cache = true
disable_mlock = true

ui = true

api_addr = "top_level_api_addr"
cluster_addr = "top_level_cluster_addr"

listener "tcp" {
address = "127.0.0.1:443"
}

backend "consul" {
advertise_addr = "foo"
token = "foo"
}

ha_backend "consul" {
bar = "baz"
advertise_addr = "snafu"
disable_clustering = "true"
token = "foo"
}

telemetry {
statsd_address = "bar"
circonus_api_token = "baz"
}

seal "awskms" {
region = "us-east-1"
access_key = "AKIAIOSFODNN7EXAMPLE"
secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
}

max_lease_ttl = "10h"
default_lease_ttl = "10h"
cluster_name = "testcluster"
pid_file = "./pidfile"
raw_storage_endpoint = true
disable_sealwrap = true
21 changes: 21 additions & 0 deletions http/forwarding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,3 +582,24 @@ func TestHTTP_Forwarding_HelpOperation(t *testing.T) {
testHelp(cores[0].Client)
testHelp(cores[1].Client)
}

func TestHTTP_Forwarding_LocalOnly(t *testing.T) {
cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{
HandlerFunc: Handler,
})
cluster.Start()
defer cluster.Cleanup()
cores := cluster.Cores

vault.TestWaitActive(t, cores[0].Core)

testLocalOnly := func(client *api.Client) {
_, err := client.Logical().Read("sys/config/state/sanitized")
if err == nil {
t.Fatal("expected error")
}
}

testLocalOnly(cores[1].Client)
testLocalOnly(cores[2].Client)
}
3 changes: 2 additions & 1 deletion http/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ func Handler(props *vault.HandlerProperties) http.Handler {
mux := http.NewServeMux()

// Handle non-forwarded paths
mux.Handle("/v1/sys/pprof/", handleLogicalNoForward(core))
mux.Handle("/v1/sys/config/state/", handleLogicalNoForward(core))
mux.Handle("/v1/sys/host-info", handleLogicalNoForward(core))
mux.Handle("/v1/sys/pprof/", handleLogicalNoForward(core))

mux.Handle("/v1/sys/init", handleSysInit(core))
mux.Handle("/v1/sys/seal-status", handleSysSealStatus(core))
Expand Down
Loading