From 201a67686026be8159dcb237d29d2fcb319a1975 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Sun, 8 Sep 2019 14:39:40 -0700 Subject: [PATCH 1/8] sys/pprof: add pprof routes to the system backend --- http/logical.go | 6 +- vault/logical_system.go | 1 + vault/logical_system_pprof.go | 184 ++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 vault/logical_system_pprof.go diff --git a/http/logical.go b/http/logical.go index d179a2873b94..53a3c4899936 100644 --- a/http/logical.go +++ b/http/logical.go @@ -59,7 +59,11 @@ func buildLogicalRequest(core *vault.Core, w http.ResponseWriter, r *http.Reques data = parseQuery(queryVals) } - if path == "sys/storage/raft/snapshot" { + switch { + case strings.HasPrefix(path, "sys/pprof/"): + passHTTPReq = true + responseWriter = w + case path == "sys/storage/raft/snapshot": responseWriter = w } diff --git a/vault/logical_system.go b/vault/logical_system.go index d331c253c4ae..77a47fbced4e 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -164,6 +164,7 @@ func NewSystemBackend(core *Core, logger log.Logger) *SystemBackend { b.Backend.Paths = append(b.Backend.Paths, b.toolsPaths()...) b.Backend.Paths = append(b.Backend.Paths, b.capabilitiesPaths()...) b.Backend.Paths = append(b.Backend.Paths, b.internalPaths()...) + b.Backend.Paths = append(b.Backend.Paths, b.pprofPaths()...) b.Backend.Paths = append(b.Backend.Paths, b.remountPath()) b.Backend.Paths = append(b.Backend.Paths, b.metricsPath()) diff --git a/vault/logical_system_pprof.go b/vault/logical_system_pprof.go new file mode 100644 index 000000000000..634e98b8df9e --- /dev/null +++ b/vault/logical_system_pprof.go @@ -0,0 +1,184 @@ +package vault + +import ( + "context" + "errors" + "net/http/pprof" + + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +func (b *SystemBackend) pprofPaths() []*framework.Path { + return []*framework.Path{ + { + Pattern: "pprof/?$", + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handlePprofIndex, + Summary: "Returns the running program's command line.", + Description: "Returns the running program's command line, with arguments separated by NUL bytes.", + }, + }, + }, + { + Pattern: "pprof/cmdline", + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handlePprofCmdline, + Summary: "Returns the running program's command line.", + Description: "Returns the running program's command line, with arguments separated by NUL bytes.", + }, + }, + }, + { + Pattern: "pprof/goroutine", + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handlePprofGoroutine, + Summary: "Returns stack traces of all current goroutines.", + Description: "Returns stack traces of all current goroutines.", + }, + }, + }, + { + Pattern: "pprof/heap", + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handlePprofHeap, + Summary: "Returns a sampling of memory allocations of live object.", + Description: "Returns a sampling of memory allocations of live object.", + }, + }, + }, + { + Pattern: "pprof/profile", + + Fields: map[string]*framework.FieldSchema{ + "seconds": { + Type: framework.TypeInt, + Description: "If provided, specifies the duration to run the profiling command.", + }, + }, + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handlePprofProfile, + Summary: "Returns a pprof-formatted cpu profile payload.", + Description: "Returns a pprof-formatted cpu profile payload. Profiling lasts for duration specified in seconds GET parameter, or for 30 seconds if not specified.", + }, + }, + }, + { + Pattern: "pprof/symbol", + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handlePprofSymbol, + Summary: "Returns the program counters listed in the request.", + Description: "Returns the program counters listed in the request.", + }, + }, + }, + + { + Pattern: "pprof/trace", + + Fields: map[string]*framework.FieldSchema{ + "seconds": { + Type: framework.TypeInt, + Description: "If provided, specifies the duration to run the tracing command.", + }, + }, + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handlePprofTrace, + Summary: "Returns the execution trace in binary form.", + Description: "Returns the execution trace in binary form. Tracing lasts for duration specified in seconds GET parameter, or for 1 second if not specified.", + }, + }, + }, + } +} + +func (b *SystemBackend) handlePprofIndex(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + if err := checkRequestHandlerParams(req); err != nil { + return nil, err + } + + pprof.Index(req.ResponseWriter, req.HTTPRequest) + return nil, nil +} + +func (b *SystemBackend) handlePprofCmdline(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + if err := checkRequestHandlerParams(req); err != nil { + return nil, err + } + + pprof.Cmdline(req.ResponseWriter, req.HTTPRequest) + return nil, nil +} + +func (b *SystemBackend) handlePprofGoroutine(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + if err := checkRequestHandlerParams(req); err != nil { + return nil, err + } + + pprof.Handler("goroutine").ServeHTTP(req.ResponseWriter, req.HTTPRequest) + return nil, nil +} + +func (b *SystemBackend) handlePprofHeap(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + if err := checkRequestHandlerParams(req); err != nil { + return nil, err + } + + pprof.Handler("heap").ServeHTTP(req.ResponseWriter, req.HTTPRequest) + return nil, nil +} + +func (b *SystemBackend) handlePprofProfile(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + if err := checkRequestHandlerParams(req); err != nil { + return nil, err + } + + pprof.Profile(req.ResponseWriter, req.HTTPRequest) + return nil, nil +} + +func (b *SystemBackend) handlePprofSymbol(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + if err := checkRequestHandlerParams(req); err != nil { + return nil, err + } + + pprof.Symbol(req.ResponseWriter, req.HTTPRequest) + return nil, nil +} + +func (b *SystemBackend) handlePprofTrace(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + if err := checkRequestHandlerParams(req); err != nil { + return nil, err + } + + pprof.Trace(req.ResponseWriter, req.HTTPRequest) + return nil, nil +} + +// checkRequestHandlerParams is a helper that checks for the existence of the +// HTTP request and response writer in a logical.Request. +func checkRequestHandlerParams(req *logical.Request) error { + if req.ResponseWriter == nil { + return errors.New("no writer for request") + } + + if req.HTTPRequest == nil || req.HTTPRequest.Body == nil { + return errors.New("no reader for request") + } + + return nil +} From 912dc41f6347f92643b7fd1b3219454480cc311c Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 13 Sep 2019 09:50:24 -0700 Subject: [PATCH 2/8] sys/pprof: add pprof paths to handler with local-only check --- http/handler.go | 4 ++++ http/logical.go | 10 +++++++++- vault/cluster.go | 3 ++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/http/handler.go b/http/handler.go index 2562c6a052c2..b87d921a25bb 100644 --- a/http/handler.go +++ b/http/handler.go @@ -110,6 +110,10 @@ func Handler(props *vault.HandlerProperties) http.Handler { // Create the muxer to handle the actual endpoints mux := http.NewServeMux() + + // Handle pprof paths + mux.Handle("/v1/sys/pprof/", handleLogical(core)) + mux.Handle("/v1/sys/init", handleSysInit(core)) mux.Handle("/v1/sys/seal-status", handleSysSealStatus(core)) mux.Handle("/v1/sys/seal", handleSysSeal(core)) diff --git a/http/logical.go b/http/logical.go index 53a3c4899936..30fab1465bfd 100644 --- a/http/logical.go +++ b/http/logical.go @@ -60,7 +60,7 @@ func buildLogicalRequest(core *vault.Core, w http.ResponseWriter, r *http.Reques } switch { - case strings.HasPrefix(path, "sys/pprof/"): + case strings.HasPrefix(path, "sys/pprof"): passHTTPReq = true responseWriter = w case path == "sys/storage/raft/snapshot": @@ -281,6 +281,14 @@ func handleLogicalInternal(core *vault.Core, injectDataIntoTopLevel bool) http.H // success. resp, ok, needsForward := request(core, w, r, req) if needsForward { + // Do not forward these specific requests since they are only + // applicable to the local node. + switch { + case strings.HasPrefix(req.Path, "sys/pprof"): + respondError(w, http.StatusBadRequest, vault.ErrCannotForwardLocalOnly) + return + } + if origBody != nil { r.Body = origBody } diff --git a/vault/cluster.go b/vault/cluster.go index 983afdbb8b1c..364cd56d3c7e 100644 --- a/vault/cluster.go +++ b/vault/cluster.go @@ -37,7 +37,8 @@ const ( ) var ( - ErrCannotForward = errors.New("cannot forward request; no connection or address not known") + ErrCannotForward = errors.New("cannot forward request; no connection or address not known") + ErrCannotForwardLocalOnly = errors.New("cannot forward local-only request") ) type ClusterLeaderParams struct { From ca35630e464eeea294c8990ca44cc7486bd60630 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 16 Sep 2019 12:19:00 -0700 Subject: [PATCH 3/8] fix trailing slash on pprof index endpoint --- http/logical.go | 4 ++-- vault/logical_system_pprof.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/http/logical.go b/http/logical.go index 30fab1465bfd..39321d64740f 100644 --- a/http/logical.go +++ b/http/logical.go @@ -60,7 +60,7 @@ func buildLogicalRequest(core *vault.Core, w http.ResponseWriter, r *http.Reques } switch { - case strings.HasPrefix(path, "sys/pprof"): + case strings.HasPrefix(path, "sys/pprof/"): passHTTPReq = true responseWriter = w case path == "sys/storage/raft/snapshot": @@ -284,7 +284,7 @@ func handleLogicalInternal(core *vault.Core, injectDataIntoTopLevel bool) http.H // Do not forward these specific requests since they are only // applicable to the local node. switch { - case strings.HasPrefix(req.Path, "sys/pprof"): + case strings.HasPrefix(req.Path, "sys/pprof/"): respondError(w, http.StatusBadRequest, vault.ErrCannotForwardLocalOnly) return } diff --git a/vault/logical_system_pprof.go b/vault/logical_system_pprof.go index 634e98b8df9e..eff5bb8b5632 100644 --- a/vault/logical_system_pprof.go +++ b/vault/logical_system_pprof.go @@ -12,7 +12,7 @@ import ( func (b *SystemBackend) pprofPaths() []*framework.Path { return []*framework.Path{ { - Pattern: "pprof/?$", + Pattern: "pprof/$", Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ From 1c66107ec95ce1d591d5200e3221ee2821c70686 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 17 Sep 2019 13:51:51 -0700 Subject: [PATCH 4/8] use new no-forward handler on pprof --- http/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http/handler.go b/http/handler.go index b87d921a25bb..ef568065bb44 100644 --- a/http/handler.go +++ b/http/handler.go @@ -112,7 +112,7 @@ func Handler(props *vault.HandlerProperties) http.Handler { mux := http.NewServeMux() // Handle pprof paths - mux.Handle("/v1/sys/pprof/", handleLogical(core)) + mux.Handle("/v1/sys/pprof/", handleLogicalNoForward(core)) mux.Handle("/v1/sys/init", handleSysInit(core)) mux.Handle("/v1/sys/seal-status", handleSysSealStatus(core)) From 2c3a1f99f0fe00c63a28e8b3f4ebb69523ac7b20 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 17 Sep 2019 13:53:12 -0700 Subject: [PATCH 5/8] go mod tidy --- go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/go.mod b/go.mod index 0bc600fa103f..5a6dad0d75d9 100644 --- a/go.mod +++ b/go.mod @@ -91,7 +91,6 @@ require ( github.com/joyent/triton-go v0.0.0-20190112182421-51ffac552869 github.com/keybase/go-crypto v0.0.0-20190403132359-d65b6b94177f github.com/kr/pretty v0.1.0 - github.com/kr/pty v1.1.3 // indirect github.com/kr/text v0.1.0 github.com/lib/pq v1.2.0 github.com/mattn/go-colorable v0.1.2 From cff6809cb821b6663ac1a5544a930233f45b5369 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Thu, 19 Sep 2019 09:52:25 -0700 Subject: [PATCH 6/8] add pprof external tests --- vault/external_tests/pprof/pprof_test.go | 118 +++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 vault/external_tests/pprof/pprof_test.go diff --git a/vault/external_tests/pprof/pprof_test.go b/vault/external_tests/pprof/pprof_test.go new file mode 100644 index 000000000000..dcea5c515a58 --- /dev/null +++ b/vault/external_tests/pprof/pprof_test.go @@ -0,0 +1,118 @@ +package pprof + +import ( + "encoding/json" + "io/ioutil" + "net/http" + "testing" + + "github.com/hashicorp/go-cleanhttp" + vaulthttp "github.com/hashicorp/vault/http" + "github.com/hashicorp/vault/vault" + "golang.org/x/net/http2" +) + +func TestSysPprof(t *testing.T) { + cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + core := cluster.Cores[0].Core + vault.TestWaitActive(t, core) + client := cluster.Cores[0].Client + + transport := cleanhttp.DefaultPooledTransport() + transport.TLSClientConfig = cluster.Cores[0].TLSConfig.Clone() + if err := http2.ConfigureTransport(transport); err != nil { + t.Fatal(err) + } + httpClient := &http.Client{ + Transport: transport, + } + + cases := []struct { + name string + path string + seconds string + }{ + { + "index", + "/v1/sys/pprof/", + "", + }, + { + "cmdline", + "/v1/sys/pprof/cmdline", + "", + }, + { + "goroutine", + "/v1/sys/pprof/goroutine", + "", + }, + { + "heap", + "/v1/sys/pprof/heap", + "", + }, + { + "profile", + "/v1/sys/pprof/profile", + "1", + }, + { + "symbol", + "/v1/sys/pprof/symbol", + "", + }, + { + "trace", + "/v1/sys/pprof/trace", + "1", + }, + } + + pprofRequest := func(path string, seconds string) { + req := client.NewRequest("GET", path) + if seconds != "" { + req.Params.Set("seconds", seconds) + } + httpReq, err := req.ToHTTP() + if err != nil { + t.Fatal(err) + } + resp, err := httpClient.Do(httpReq) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + httpRespBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + // Make sure that we don't get a error response + httpResp := make(map[string]interface{}) + + // Skip this error check since some endpoints return binary blobs, we + // only care about the ok check right after as an existence check. + _ = json.Unmarshal(httpRespBody, &httpResp) + + if _, ok := httpResp["errors"]; ok { + t.Fatalf("unexpected error response: %v", httpResp["errors"]) + } + + if len(httpRespBody) == 0 { + t.Fatal("no pprof index returned") + } + t.Log(string(httpRespBody)) + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + pprofRequest(tc.path, tc.seconds) + }) + } +} From 3b27e9135756271fdc0e58d32c754bcdc7865417 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Thu, 19 Sep 2019 11:28:19 -0700 Subject: [PATCH 7/8] disallow streaming requests to exceed DefaultMaxRequestDuration --- vault/logical_system_pprof.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/vault/logical_system_pprof.go b/vault/logical_system_pprof.go index eff5bb8b5632..87c892dc2f41 100644 --- a/vault/logical_system_pprof.go +++ b/vault/logical_system_pprof.go @@ -3,7 +3,9 @@ package vault import ( "context" "errors" + "fmt" "net/http/pprof" + "strconv" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" @@ -147,6 +149,18 @@ func (b *SystemBackend) handlePprofProfile(ctx context.Context, req *logical.Req return nil, err } + // Return an error if seconds exceeds max request duration. This follows a + // similar behavior to how pprof treats seconds > WriteTimeout (i.e. it + // error with a 400), and avoids drift between what gets audited vs what + // ends up happening. + if secQueryVal := req.HTTPRequest.FormValue("seconds"); secQueryVal != "" { + maxDur := int64(DefaultMaxRequestDuration.Seconds()) + sec, _ := strconv.ParseInt(secQueryVal, 10, 64) + if sec > maxDur { + return logical.ErrorResponse(fmt.Sprintf("seconds %d exceeds max request duration of %d", sec, maxDur)), nil + } + } + pprof.Profile(req.ResponseWriter, req.HTTPRequest) return nil, nil } @@ -165,6 +179,18 @@ func (b *SystemBackend) handlePprofTrace(ctx context.Context, req *logical.Reque return nil, err } + // Return an error if seconds exceeds max request duration. This follows a + // similar behavior to how pprof treats seconds > WriteTimeout (i.e. it + // error with a 400), and avoids drift between what gets audited vs what + // ends up happening. + if secQueryVal := req.HTTPRequest.FormValue("seconds"); secQueryVal != "" { + maxDur := int64(DefaultMaxRequestDuration.Seconds()) + sec, _ := strconv.ParseInt(secQueryVal, 10, 64) + if sec > maxDur { + return logical.ErrorResponse(fmt.Sprintf("seconds %d exceeds max request duration of %d", sec, maxDur)), nil + } + } + pprof.Trace(req.ResponseWriter, req.HTTPRequest) return nil, nil } From 1d40cb01e37cdf0b42567c908910c4f08bad0476 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Thu, 19 Sep 2019 12:59:47 -0700 Subject: [PATCH 8/8] add max request duration test --- vault/external_tests/pprof/pprof_test.go | 59 +++++++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/vault/external_tests/pprof/pprof_test.go b/vault/external_tests/pprof/pprof_test.go index dcea5c515a58..5da8c2bb3aca 100644 --- a/vault/external_tests/pprof/pprof_test.go +++ b/vault/external_tests/pprof/pprof_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "io/ioutil" "net/http" + "strconv" + "strings" "testing" "github.com/hashicorp/go-cleanhttp" @@ -93,13 +95,14 @@ func TestSysPprof(t *testing.T) { if err != nil { t.Fatal(err) } - // Make sure that we don't get a error response + httpResp := make(map[string]interface{}) // Skip this error check since some endpoints return binary blobs, we // only care about the ok check right after as an existence check. _ = json.Unmarshal(httpRespBody, &httpResp) + // Make sure that we don't get a error response if _, ok := httpResp["errors"]; ok { t.Fatalf("unexpected error response: %v", httpResp["errors"]) } @@ -107,7 +110,6 @@ func TestSysPprof(t *testing.T) { if len(httpRespBody) == 0 { t.Fatal("no pprof index returned") } - t.Log(string(httpRespBody)) } for _, tc := range cases { @@ -116,3 +118,56 @@ func TestSysPprof(t *testing.T) { }) } } + +func TestSysPprof_MaxRequestDuration(t *testing.T) { + cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + client := cluster.Cores[0].Client + + transport := cleanhttp.DefaultPooledTransport() + transport.TLSClientConfig = cluster.Cores[0].TLSConfig.Clone() + if err := http2.ConfigureTransport(transport); err != nil { + t.Fatal(err) + } + httpClient := &http.Client{ + Transport: transport, + } + + sec := strconv.Itoa(int(vault.DefaultMaxRequestDuration.Seconds()) + 1) + + req := client.NewRequest("GET", "/v1/sys/pprof/profile") + req.Params.Set("seconds", sec) + httpReq, err := req.ToHTTP() + if err != nil { + t.Fatal(err) + } + resp, err := httpClient.Do(httpReq) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + httpRespBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + + httpResp := make(map[string]interface{}) + + // If we error here, it means that profiling likely happened, which is not + // what we're checking for in this case. + if err := json.Unmarshal(httpRespBody, &httpResp); err != nil { + t.Fatalf("expected valid error response, got: %v", err) + } + + errs, ok := httpResp["errors"].([]interface{}) + if !ok { + t.Fatalf("expected error response, got: %v", httpResp) + } + if len(errs) == 0 || !strings.Contains(errs[0].(string), "exceeds max request duration") { + t.Fatalf("unexptected error returned: %v", errs) + } +}