From c043de5381fa5ae6190a8bd27e47a83c2e8fa0d6 Mon Sep 17 00:00:00 2001 From: Aestek Date: Thu, 10 Jan 2019 15:27:26 +0100 Subject: [PATCH] [Security] Allow blocking Write endpoints on Agent using Network Addresses (#4719) * Add -write-allowed-nets option * Add documentation for the new write_allowed_nets option --- agent/config/builder.go | 17 +++++ agent/config/config.go | 5 +- agent/config/flags.go | 1 + agent/config/runtime.go | 8 ++ agent/config/runtime_test.go | 11 ++- agent/http.go | 56 ++++++++++++-- agent/http_oss_test.go | 51 +++++++++++++ agent/http_test.go | 89 ++++++++++++++++++++++- website/source/docs/agent/options.html.md | 8 ++ 9 files changed, 237 insertions(+), 9 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index 0a26352006..6f5a8c9a1d 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -721,6 +721,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { HTTPSAddrs: httpsAddrs, HTTPBlockEndpoints: c.HTTPConfig.BlockEndpoints, HTTPResponseHeaders: c.HTTPConfig.ResponseHeaders, + AllowWriteHTTPFrom: b.cidrsVal("allow_write_http_from", c.HTTPConfig.AllowWriteHTTPFrom), // Telemetry Telemetry: lib.TelemetryConfig{ @@ -1355,6 +1356,22 @@ func (b *Builder) float64Val(v *float64) float64 { return *v } +func (b *Builder) cidrsVal(name string, v []string) (nets []*net.IPNet) { + if v == nil { + return + } + + for _, p := range v { + _, net, err := net.ParseCIDR(strings.TrimSpace(p)) + if err != nil { + b.err = multierror.Append(b.err, fmt.Errorf("%s: invalid cidr: %s", name, p)) + } + nets = append(nets, net) + } + + return +} + func (b *Builder) tlsCipherSuites(name string, v *string) []uint16 { if v == nil { return nil diff --git a/agent/config/config.go b/agent/config/config.go index e71aaa1d0b..f70cdb1e50 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -558,8 +558,9 @@ type DNS struct { } type HTTPConfig struct { - BlockEndpoints []string `json:"block_endpoints,omitempty" hcl:"block_endpoints" mapstructure:"block_endpoints"` - ResponseHeaders map[string]string `json:"response_headers,omitempty" hcl:"response_headers" mapstructure:"response_headers"` + 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"` } type Performance struct { diff --git a/agent/config/flags.go b/agent/config/flags.go index 53b255b859..1ee7165cc6 100644 --- a/agent/config/flags.go +++ b/agent/config/flags.go @@ -72,6 +72,7 @@ func AddFlags(fs *flag.FlagSet, f *Flags) { add(&f.Config.DNSDomain, "domain", "Domain to use for DNS interface.") add(&f.Config.EnableScriptChecks, "enable-script-checks", "Enables health check scripts.") add(&f.Config.EnableLocalScriptChecks, "enable-local-script-checks", "Enables health check scripts from configuration file.") + add(&f.Config.HTTPConfig.AllowWriteHTTPFrom, "allow-write-http-from", "Only allow write endpoint calls from given network. CIDR format, can be specified multiple times.") add(&f.Config.EncryptKey, "encrypt", "Provides the gossip encryption key.") add(&f.Config.Ports.GRPC, "grpc-port", "Sets the gRPC API port to listen on (currently needed for Envoy xDS only).") add(&f.Config.Ports.HTTP, "http-port", "Sets the HTTP API port to listen on.") diff --git a/agent/config/runtime.go b/agent/config/runtime.go index b9f732e945..c22ed7c95f 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -320,6 +320,14 @@ type RuntimeConfig struct { // hcl: http_config { block_endpoints = []string } HTTPBlockEndpoints []string + // AllowWriteHTTPFrom restricts the agent write endpoints to the given + // networks. Any request to a protected endpoint that is not mactched + // by one of these networks will get a 403 response. + // An empty slice means no restriction. + // + // hcl: http_config { allow_write_http_from = []string } + AllowWriteHTTPFrom []*net.IPNet + // HTTPResponseHeaders are used to add HTTP header response fields to the HTTP API responses. // // hcl: http_config { response_headers = map[string]string } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index d28ffd6b69..2694884991 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2875,6 +2875,11 @@ func TestFullConfig(t *testing.T) { dataDir := testutil.TempDir(t, "consul") defer os.RemoveAll(dataDir) + cidr := func(s string) *net.IPNet { + _, n, _ := net.ParseCIDR(s) + return n + } + flagSrc := []string{`-dev`} src := map[string]string{ "json": `{ @@ -3072,6 +3077,7 @@ func TestFullConfig(t *testing.T) { "encrypt_verify_outgoing": true, "http_config": { "block_endpoints": [ "RBvAFcGD", "fWOWFznh" ], + "allow_write_http_from": [ "127.0.0.1/8", "22.33.44.55/32", "0.0.0.0/0" ], "response_headers": { "M6TKa9NP": "xjuxjOzQ", "JRCrHZed": "rl0mTx81" @@ -3620,6 +3626,7 @@ func TestFullConfig(t *testing.T) { encrypt_verify_outgoing = true http_config { block_endpoints = [ "RBvAFcGD", "fWOWFznh" ] + allow_write_http_from = [ "127.0.0.1/8", "22.33.44.55/32", "0.0.0.0/0" ] response_headers = { "M6TKa9NP" = "xjuxjOzQ" "JRCrHZed" = "rl0mTx81" @@ -4259,6 +4266,7 @@ func TestFullConfig(t *testing.T) { GRPCAddrs: []net.Addr{tcpAddr("32.31.61.91:4881")}, HTTPAddrs: []net.Addr{tcpAddr("83.39.91.39:7999")}, HTTPBlockEndpoints: []string{"RBvAFcGD", "fWOWFznh"}, + AllowWriteHTTPFrom: []*net.IPNet{cidr("127.0.0.0/8"), cidr("22.33.44.55/32"), cidr("0.0.0.0/0")}, HTTPPort: 7999, HTTPResponseHeaders: map[string]string{"M6TKa9NP": "xjuxjOzQ", "JRCrHZed": "rl0mTx81"}, HTTPSAddrs: []net.Addr{tcpAddr("95.17.17.19:15127")}, @@ -5197,7 +5205,8 @@ func TestSanitize(t *testing.T) { "VerifyServerHostname": false, "Version": "", "VersionPrerelease": "", - "Watches": [] + "Watches": [], + "AllowWriteHTTPFrom": [] }` b, err := json.MarshalIndent(rt.Sanitized(), "", " ") if err != nil { diff --git a/agent/http.go b/agent/http.go index 49aa13c1cb..919d3ec47b 100644 --- a/agent/http.go +++ b/agent/http.go @@ -15,12 +15,13 @@ import ( "time" "github.com/NYTimes/gziphandler" - "github.com/armon/go-metrics" + metrics "github.com/armon/go-metrics" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/cache" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/go-cleanhttp" + cleanhttp "github.com/hashicorp/go-cleanhttp" "github.com/mitchellh/mapstructure" + "github.com/pkg/errors" ) // MethodNotAllowedError should be returned by a handler when the HTTP method is not allowed. @@ -54,6 +55,13 @@ func (e CodeWithPayloadError) Error() string { return e.Reason } +type ForbiddenError struct { +} + +func (e ForbiddenError) Error() string { + return "Access is restricted" +} + // HTTPServer provides an HTTP api for an agent. type HTTPServer struct { *http.Server @@ -309,6 +317,14 @@ func (s *HTTPServer) wrap(handler endpoint, methods []string) http.HandlerFunc { return } + isForbidden := func(err error) bool { + if acl.IsErrPermissionDenied(err) || acl.IsErrNotFound(err) { + return true + } + _, ok := err.(ForbiddenError) + return ok + } + isMethodNotAllowed := func(err error) bool { _, ok := err.(MethodNotAllowedError) return ok @@ -326,7 +342,7 @@ func (s *HTTPServer) wrap(handler endpoint, methods []string) http.HandlerFunc { handleErr := func(err error) { s.agent.logger.Printf("[ERR] http: Request %s %v, error: %v from=%s", req.Method, logURL, err, req.RemoteAddr) switch { - case acl.IsErrPermissionDenied(err) || acl.IsErrNotFound(err): + case isForbidden(err): resp.WriteHeader(http.StatusForbidden) fmt.Fprint(resp, err.Error()) case structs.IsErrRPCRateExceeded(err): @@ -373,8 +389,12 @@ func (s *HTTPServer) wrap(handler endpoint, methods []string) http.HandlerFunc { if !methodFound { err = MethodNotAllowedError{req.Method, append([]string{"OPTIONS"}, methods...)} } else { - // Invoke the handler - obj, err = handler(resp, req) + err = s.checkWriteAccess(req) + + if err == nil { + // Invoke the handler + obj, err = handler(resp, req) + } } contentType := "application/json" httpCode := http.StatusOK @@ -840,3 +860,29 @@ func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, dc *stri func (s *HTTPServer) parseWithoutResolvingProxyToken(resp http.ResponseWriter, req *http.Request, dc *string, b *structs.QueryOptions) bool { return s.parseInternal(resp, req, dc, b, false) } + +func (s *HTTPServer) checkWriteAccess(req *http.Request) error { + if req.Method == http.MethodGet || req.Method == http.MethodHead || req.Method == http.MethodOptions { + return nil + } + + allowed := s.agent.config.AllowWriteHTTPFrom + if len(allowed) == 0 { + return nil + } + + ipStr, _, err := net.SplitHostPort(req.RemoteAddr) + if err != nil { + return errors.Wrap(err, "unable to parse remote addr") + } + + ip := net.ParseIP(ipStr) + + for _, n := range allowed { + if n.Contains(ip) { + return nil + } + } + + return ForbiddenError{} +} diff --git a/agent/http_oss_test.go b/agent/http_oss_test.go index 502273cb84..8810f06fc7 100644 --- a/agent/http_oss_test.go +++ b/agent/http_oss_test.go @@ -9,6 +9,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/logger" @@ -158,3 +160,52 @@ func TestHTTPAPI_OptionMethod_OSS(t *testing.T) { } } } + +func TestHTTPAPI_AllowedNets_OSS(t *testing.T) { + a := NewTestAgent(t.Name(), ` + acl_datacenter = "dc1" + http_config { + allow_write_http_from = ["127.0.0.1/8"] + } + `) + a.Agent.LogWriter = logger.NewLogWriter(512) + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + + testOptionMethod := func(path string, method string) { + t.Run(method+" "+path, func(t *testing.T) { + uri := fmt.Sprintf("http://%s%s", a.HTTPAddr(), path) + req, _ := http.NewRequest(method, uri, nil) + req.RemoteAddr = "192.168.1.2:5555" + resp := httptest.NewRecorder() + a.srv.Handler.ServeHTTP(resp, req) + + require.Equal(t, http.StatusForbidden, resp.Code, "%s %s", method, path) + }) + } + + for path, methods := range extraTestEndpoints { + if !includePathInTest(path) { + continue + } + for _, method := range methods { + if method == http.MethodGet { + continue + } + + testOptionMethod(path, method) + } + } + for path, methods := range allowedMethods { + if !includePathInTest(path) { + continue + } + for _, method := range methods { + if method == http.MethodGet { + continue + } + + testOptionMethod(path, method) + } + } +} diff --git a/agent/http_test.go b/agent/http_test.go index 6bb269630b..b3b5d6b08e 100644 --- a/agent/http_test.go +++ b/agent/http_test.go @@ -22,7 +22,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/testutil" - "github.com/hashicorp/go-cleanhttp" + cleanhttp "github.com/hashicorp/go-cleanhttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/http2" @@ -1204,6 +1204,93 @@ func TestParseToken_ProxyTokenResolve(t *testing.T) { } } +func TestAllowedNets(t *testing.T) { + type testVal struct { + nets []string + ip string + expected bool + } + + for _, v := range []testVal{ + { + ip: "156.124.222.351", + expected: true, + }, + { + ip: "[::2]", + expected: true, + }, + { + nets: []string{"0.0.0.0/0"}, + ip: "115.124.32.64", + expected: true, + }, + { + nets: []string{"::0/0"}, + ip: "[::3]", + expected: true, + }, + { + nets: []string{"127.0.0.1/8"}, + ip: "127.0.0.1", + expected: true, + }, + { + nets: []string{"127.0.0.1/8"}, + ip: "128.0.0.1", + expected: false, + }, + { + nets: []string{"::1/8"}, + ip: "[::1]", + expected: true, + }, + { + nets: []string{"255.255.255.255/32"}, + ip: "127.0.0.1", + expected: false, + }, + { + nets: []string{"255.255.255.255/32", "127.0.0.1/8"}, + ip: "127.0.0.1", + expected: true, + }, + } { + var nets []*net.IPNet + for _, n := range v.nets { + _, in, err := net.ParseCIDR(n) + if err != nil { + t.Fatal(err) + } + nets = append(nets, in) + } + + a := &TestAgent{ + Name: t.Name(), + } + a.Start() + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + + a.config.AllowWriteHTTPFrom = nets + + err := a.srv.checkWriteAccess(&http.Request{ + Method: http.MethodPost, + RemoteAddr: fmt.Sprintf("%s:16544", v.ip), + }) + actual := err == nil + + if actual != v.expected { + t.Fatalf("bad checkWriteAccess for values %+v, got %v", v, err) + } + + _, isForbiddenErr := err.(ForbiddenError) + if err != nil && !isForbiddenErr { + t.Fatalf("expected ForbiddenError but got: %s", err) + } + } +} + // assertIndex tests that X-Consul-Index is set and non-zero func assertIndex(t *testing.T, resp *httptest.ResponseRecorder) { header := resp.Header().Get("X-Consul-Index") diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index e70d9d401c..965bb17e7a 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -1179,6 +1179,14 @@ default will automatically work with some tooling. } } ``` + * `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" ]` * `leave_on_terminate` If enabled, when the agent receives a TERM signal, it will send a `Leave` message to the rest