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

CVE-2020-13250: Cache DoS / OOM #8023

Merged
merged 2 commits into from
Jun 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions agent/catalog_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (s *HTTPServer) CatalogDatacenters(resp http.ResponseWriter, req *http.Requ
parseCacheControl(resp, req, &args.QueryOptions)
var out []string

if args.QueryOptions.UseCache {
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(cachetype.CatalogDatacentersName, &args)
if err != nil {
metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_datacenters"}, 1,
Expand Down Expand Up @@ -166,7 +166,7 @@ func (s *HTTPServer) CatalogServices(resp http.ResponseWriter, req *http.Request
var out structs.IndexedServices
defer setMeta(resp, &out.QueryMeta)

if args.QueryOptions.UseCache {
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(cachetype.CatalogListServicesName, &args)
if err != nil {
metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_services"}, 1,
Expand Down Expand Up @@ -255,7 +255,7 @@ func (s *HTTPServer) catalogServiceNodes(resp http.ResponseWriter, req *http.Req
var out structs.IndexedServiceNodes
defer setMeta(resp, &out.QueryMeta)

if args.QueryOptions.UseCache {
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(cachetype.CatalogServicesName, &args)
if err != nil {
metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_service_nodes"}, 1,
Expand Down
1 change: 1 addition & 0 deletions agent/config/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
HTTPBlockEndpoints: c.HTTPConfig.BlockEndpoints,
HTTPResponseHeaders: c.HTTPConfig.ResponseHeaders,
AllowWriteHTTPFrom: b.cidrsVal("allow_write_http_from", c.HTTPConfig.AllowWriteHTTPFrom),
HTTPUseCache: b.boolValWithDefault(c.HTTPConfig.UseCache, true),

// Telemetry
Telemetry: lib.TelemetryConfig{
Expand Down
1 change: 1 addition & 0 deletions agent/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,7 @@ type HTTPConfig struct {
BlockEndpoints []string `json:"block_endpoints,omitempty" hcl:"block_endpoints" mapstructure:"block_endpoints"`
AllowWriteHTTPFrom []string `json:"allow_write_http_from,omitempty" hcl:"allow_write_http_from" mapstructure:"allow_write_http_from"`
ResponseHeaders map[string]string `json:"response_headers,omitempty" hcl:"response_headers" mapstructure:"response_headers"`
UseCache *bool `json:"use_cache,omitempty" hcl:"use_cache" mapstructure:"use_cache"`
}

type Performance struct {
Expand Down
6 changes: 6 additions & 0 deletions agent/config/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,12 @@ type RuntimeConfig struct {
// hcl: dns_config { cache_max_age = "duration" }
DNSCacheMaxAge time.Duration

// HTTPUseCache whether or not to use cache for http queries. Defaults
// to true.
//
// hcl: http_config { use_cache = (true|false) }
HTTPUseCache bool

// HTTPBlockEndpoints is a list of endpoint prefixes to block in the
// HTTP API. Any requests to these will get a 403 response.
//
Expand Down
54 changes: 53 additions & 1 deletion agent/config/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2118,6 +2118,54 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
`},
err: "Serf Advertise WAN address 10.0.0.1:1000 already configured for RPC Advertise",
},
{
desc: "http use_cache defaults to true",
args: []string{
`-data-dir=` + dataDir,
},
json: []string{`{
"http_config": {}
}`},
hcl: []string{`
http_config = {}
`},
patch: func(rt *RuntimeConfig) {
rt.DataDir = dataDir
rt.HTTPUseCache = true
},
},
{
desc: "http use_cache is enabled when true",
args: []string{
`-data-dir=` + dataDir,
},
json: []string{`{
"http_config": { "use_cache": true }
}`},
hcl: []string{`
http_config = { use_cache = true }
`},
patch: func(rt *RuntimeConfig) {
rt.DataDir = dataDir
rt.HTTPUseCache = true
},
},
{
desc: "http use_cache is disabled when false",
args: []string{
`-data-dir=` + dataDir,
},
json: []string{`{
"http_config": { "use_cache": false }
}`},
hcl: []string{`
http_config = { use_cache = false }
`},
patch: func(rt *RuntimeConfig) {
rt.DataDir = dataDir
rt.HTTPUseCache = false
},
},
{
desc: "sidecar_service can't have ID",
args: []string{
Expand Down Expand Up @@ -4167,7 +4215,8 @@ func TestFullConfig(t *testing.T) {
"response_headers": {
"M6TKa9NP": "xjuxjOzQ",
"JRCrHZed": "rl0mTx81"
}
},
"use_cache": false
},
"key_file": "IEkkwgIA",
"leave_on_terminate": true,
Expand Down Expand Up @@ -4804,6 +4853,7 @@ func TestFullConfig(t *testing.T) {
"M6TKa9NP" = "xjuxjOzQ"
"JRCrHZed" = "rl0mTx81"
}
use_cache = false
}
key_file = "IEkkwgIA"
leave_on_terminate = true
Expand Down Expand Up @@ -5517,6 +5567,7 @@ func TestFullConfig(t *testing.T) {
HTTPMaxConnsPerClient: 100,
HTTPSHandshakeTimeout: 2391 * time.Millisecond,
HTTPSPort: 15127,
HTTPUseCache: false,
KeyFile: "IEkkwgIA",
KVMaxValueSize: 1234567800000000,
LeaveDrainTime: 8265 * time.Second,
Expand Down Expand Up @@ -6402,6 +6453,7 @@ func TestSanitize(t *testing.T) {
"HTTPMaxConnsPerClient": 0,
"HTTPPort": 0,
"HTTPResponseHeaders": {},
"HTTPUseCache": false,
"HTTPSAddrs": [],
"HTTPSHandshakeTimeout": "0s",
"HTTPSPort": 0,
Expand Down
2 changes: 1 addition & 1 deletion agent/discovery_chain_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (s *HTTPServer) DiscoveryChainRead(resp http.ResponseWriter, req *http.Requ
var out structs.DiscoveryChainResponse
defer setMeta(resp, &out.QueryMeta)

if args.QueryOptions.UseCache {
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(cachetype.CompiledDiscoveryChainName, &args)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion agent/health_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Requ
var out structs.IndexedCheckServiceNodes
defer setMeta(resp, &out.QueryMeta)

if args.QueryOptions.UseCache {
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(cachetype.HealthServicesName, &args)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion agent/prepared_query_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (s *HTTPServer) preparedQueryExecute(id string, resp http.ResponseWriter, r
var reply structs.PreparedQueryExecuteResponse
defer setMeta(resp, &reply.QueryMeta)

if args.QueryOptions.UseCache {
if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache {
raw, m, err := s.agent.cache.Get(cachetype.PreparedQueryName, &args)
if err != nil {
// Don't return error if StaleIfError is set and we are within it and had
Expand Down
2 changes: 2 additions & 0 deletions website/pages/docs/agent/options.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,8 @@ Valid time units are 'ns', 'us' (or 'µs'), 'ms', 's', 'm', 'h'."

- `allow_write_http_from` This object is a list of networks in CIDR notation (eg "127.0.0.0/8") that are allowed to call the agent write endpoints. It defaults to an empty list, which means all networks are allowed. This is used to make the agent read-only, except for select ip ranges. - To block write calls from anywhere, use `[ "255.255.255.255/32" ]`. - To only allow write calls from localhost, use `[ "127.0.0.0/8" ]` - To only allow specific IPs, use `[ "10.0.0.1/32", "10.0.0.2/32" ]`

- `use_cache` Defaults to true. If disabled, the agent won't be using [agent caching](/api/features/caching) to answer the request. Even when the url parameter is provided.

- `leave_on_terminate` If enabled, when the agent receives a TERM signal, it will send a `Leave` message to the rest of the cluster and gracefully leave. The default behavior for this feature varies based on whether or not the agent is running as a client or a server (prior to Consul 0.7 the default value was unconditionally set to `false`). On agents in client-mode, this defaults to `true` and for agents in server-mode, this defaults to `false`.

- `limits` Available in Consul 0.9.3 and later, this is a nested
Expand Down