From 30a195faff296d5dfbc03041222a61f45c0df3fd Mon Sep 17 00:00:00 2001 From: Owen Ou <169064+owenthereal@users.noreply.github.com> Date: Wed, 10 Jan 2024 21:48:15 -0800 Subject: [PATCH] Restrict resource usage when shelling out `jq` (#175) * Limit resource usage when shelling out `jq` * Merge Dockerfile for go test * Remove unused make task * Return syscall error * Tweak limit params --- Dockerfile | 12 +++++ Makefile | 15 +++--- go.mod | 8 ++- go.sum | 11 +++-- jq/jq.go | 101 +++++++++++++++++++++++++++----------- jq/jq_darwin.go | 9 ++++ jq/jq_linux.go | 41 ++++++++++++++++ jq/jq_test.go | 121 ++++++++++++++++++++++++---------------------- server/handler.go | 16 ++---- server/server.go | 9 +++- 10 files changed, 229 insertions(+), 114 deletions(-) create mode 100644 jq/jq_darwin.go create mode 100644 jq/jq_linux.go diff --git a/Dockerfile b/Dockerfile index 71fef02..d9ab773 100644 --- a/Dockerfile +++ b/Dockerfile @@ -27,6 +27,18 @@ RUN git clone --recurse-submodules https://github.com/jqlang/jq.git && \ --prefix=/usr/local && \ make install +FROM golang:latest as gotest + +COPY --from=jqbuilder /usr/local/bin/jq /usr/local/bin/jq + +WORKDIR $GOPATH/src/github.com/owenthereal/jqplay + +ARG TIMESTAMP +RUN --mount=target=. \ + --mount=type=cache,target=/root/.cache/go-build \ + --mount=type=cache,target=/go/pkg \ + go test ./... -count=1 -race -v + FROM golang:latest as gobuilder ARG TARGETOS TARGETARCH diff --git a/Makefile b/Makefile index c34a963..6b10ef5 100644 --- a/Makefile +++ b/Makefile @@ -6,8 +6,13 @@ build: .PHONY: test test: - go test ./... -coverprofile=jqplay.c.out -covermode=atomic -count=1 -race -v - + docker \ + buildx \ + build \ + --rm \ + --build-arg TIMESTAMP=$$(date +%s) \ + --target gotest \ + . .PHONY: vet vet: @@ -29,12 +34,6 @@ docker_build: docker_push: docker_build docker buildx build --rm -t $(REPO):$(TAG) --push . -.PHONY: setup -setup: - dropdb --if-exists jqplay - createdb jqplay - psql -d jqplay -f server/db.sql - .PHONY: start start: docker compose up --build --force-recreate diff --git a/go.mod b/go.mod index 06b5f35..dae7be8 100644 --- a/go.mod +++ b/go.mod @@ -10,12 +10,14 @@ require ( github.com/joeshaw/envdecode v0.0.0-20200121155833-099f1fc765bd github.com/oklog/run v1.1.1-0.20200508094559-c7096881717e github.com/sirupsen/logrus v1.9.3 + github.com/stretchr/testify v1.8.3 github.com/unrolled/secure v1.14.0 ) require ( github.com/bytedance/sonic v1.9.1 // indirect github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/gabriel-vasile/mimetype v1.4.2 // indirect github.com/gin-contrib/sse v0.1.0 // indirect github.com/go-playground/locales v0.14.1 // indirect @@ -37,12 +39,14 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/pelletier/go-toml/v2 v2.0.8 // indirect + github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/twitchyliquid64/golang-asm v0.15.1 // indirect github.com/ugorji/go/codec v1.2.11 // indirect golang.org/x/arch v0.3.0 // indirect golang.org/x/crypto v0.17.0 // indirect - golang.org/x/net v0.17.0 // indirect - golang.org/x/sys v0.15.0 // indirect + golang.org/x/net v0.19.0 // indirect + golang.org/x/sys v0.16.0 // indirect golang.org/x/text v0.14.0 // indirect google.golang.org/protobuf v1.30.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index b1f84ba..502aa1c 100644 --- a/go.sum +++ b/go.sum @@ -137,8 +137,9 @@ github.com/oklog/run v1.1.1-0.20200508094559-c7096881717e h1:bxQ+jj+8fdl9112bovU github.com/oklog/run v1.1.1-0.20200508094559-c7096881717e/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU= github.com/pelletier/go-toml/v2 v2.0.8 h1:0ctb6s9mE31h0/lhu+J6OPmVeDxJn+kYnJc2jZR9tGQ= github.com/pelletier/go-toml/v2 v2.0.8/go.mod h1:vuYfssBdrU2XDZ9bYydBu6t+6a6PYNcZljzZR9VXg+4= -github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= @@ -215,8 +216,8 @@ golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= -golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= +golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -237,8 +238,8 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= +golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= diff --git a/jq/jq.go b/jq/jq.go index b662a12..38a69cb 100644 --- a/jq/jq.go +++ b/jq/jq.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "os" "os/exec" "strings" ) @@ -21,6 +22,7 @@ func (e *ValidationError) Error() string { var ( ErrExecTimeout = errors.New("jq execution was timeout") ErrExecCancelled = errors.New("jq execution was cancelled") + ErrExecAborted = errors.New("jq execution was aborted") allowedOpts = map[string]struct{}{ "slurp": {}, "null-input": {}, @@ -66,33 +68,6 @@ func (j *JQ) Opts() []string { return opts } -func (j *JQ) Eval(ctx context.Context, w io.Writer) error { - if err := j.Validate(); err != nil { - return err - } - - opts := j.Opts() - opts = append(opts, j.Q) - cmd := exec.CommandContext(ctx, Path, opts...) - cmd.Stdin = bytes.NewBufferString(j.J) - cmd.Env = make([]string, 0) - cmd.Stdout = w - cmd.Stderr = w - - err := cmd.Run() - if err != nil { - ctxErr := ctx.Err() - if ctxErr == context.DeadlineExceeded { - return ErrExecTimeout - } - if ctxErr == context.Canceled { - return ErrExecCancelled - } - } - - return err -} - func (j *JQ) Validate() error { errMsgs := []string{} @@ -120,3 +95,75 @@ func (j *JQ) Validate() error { func (j JQ) String() string { return fmt.Sprintf("j=%s, q=%s, o=%v", j.J, j.Q, j.Opts()) } + +func NewJQExec() *JQExec { + return &JQExec{ + ResourceLimiter: &resourceLimiter{ + MemoryLimit: limitMemory, + CPUTimeLimit: limitCPUTime, + }, + } +} + +const ( + limitMemory uint64 = 10 * 1024 * 1024 // 10 MiB + limitCPUTime uint64 = 10 // 10 percentage +) + +type ResourceLimiter interface { + LimitResources(proc *os.Process) error +} + +type resourceLimiter struct { + MemoryLimit uint64 + CPUTimeLimit uint64 +} + +func (r *resourceLimiter) LimitResources(proc *os.Process) error { + return limitResources(proc, r.MemoryLimit, r.CPUTimeLimit) +} + +type JQExec struct { + ResourceLimiter ResourceLimiter +} + +func (e *JQExec) Eval(ctx context.Context, jq JQ, w io.Writer) error { + if err := jq.Validate(); err != nil { + return err + } + + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + cmd := exec.CommandContext(ctx, Path, append(jq.Opts(), jq.Q)...) + cmd.Stdin = bytes.NewBufferString(jq.J) + cmd.Env = make([]string, 0) + cmd.Stdout = w + cmd.Stderr = w + + if err := cmd.Start(); err != nil { + return err + } + + if err := e.ResourceLimiter.LimitResources(cmd.Process); err != nil { + return err + } + + err := cmd.Wait() + if err != nil { + ctxErr := ctx.Err() + if ctxErr == context.DeadlineExceeded { + return ErrExecTimeout + } + if ctxErr == context.Canceled { + return ErrExecCancelled + } + + if strings.Contains(err.Error(), "signal: segmentation fault") || + strings.Contains(err.Error(), "signal: aborted") { + return ErrExecAborted + } + } + + return err +} diff --git a/jq/jq_darwin.go b/jq/jq_darwin.go new file mode 100644 index 0000000..4395b9f --- /dev/null +++ b/jq/jq_darwin.go @@ -0,0 +1,9 @@ +//go:build darwin + +package jq + +import "os" + +func limitResources(proc *os.Process, memoryLimit, cpuLimit uint64) error { + return nil +} diff --git a/jq/jq_linux.go b/jq/jq_linux.go new file mode 100644 index 0000000..5e31633 --- /dev/null +++ b/jq/jq_linux.go @@ -0,0 +1,41 @@ +//go:build linux + +package jq + +import ( + "os" + "syscall" + "unsafe" +) + +func limitResources(proc *os.Process, memoryLimit, cpuLimit uint64) error { + if proc == nil { + return nil + } + + pid := proc.Pid + var err error + + // limit address space + lim := syscall.Rlimit{Cur: memoryLimit, Max: memoryLimit} + _, _, errno := syscall.Syscall6(syscall.SYS_PRLIMIT64, uintptr(pid), syscall.RLIMIT_AS, uintptr(unsafe.Pointer(&lim)), 0, 0, 0) + err = errnoToErr(errno) + if err != nil { + return err + } + + // limit cpu time + lim = syscall.Rlimit{Cur: cpuLimit, Max: cpuLimit} + _, _, errno = syscall.Syscall6(syscall.SYS_PRLIMIT64, uintptr(pid), syscall.RLIMIT_CPU, uintptr(unsafe.Pointer(&lim)), 0, 0, 0) + err = errnoToErr(errno) + + return err +} + +func errnoToErr(errno syscall.Errno) error { + if errno != 0 { + return errno + } + + return nil +} diff --git a/jq/jq_test.go b/jq/jq_test.go index 7730e51..38ea056 100644 --- a/jq/jq_test.go +++ b/jq/jq_test.go @@ -6,10 +6,11 @@ import ( "io" "log" "os" - "strings" "sync" "testing" "time" + + "github.com/stretchr/testify/assert" ) func TestMain(m *testing.M) { @@ -20,22 +21,26 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -func TestJQEvalInvalidInput(t *testing.T) { - jq := &JQ{} - err := jq.Eval(context.Background(), io.Discard) - - if err == nil { - t.Errorf("err should not be nil since it's invalid input") - } -} - -func TestJQNullInputOption(t *testing.T) { +func TestJQValidate(t *testing.T) { cases := []struct { J string Q string O []JQOpt ErrStr string }{ + { + ErrStr: "missing filter, missing JSON", + }, + { + J: "{}", + Q: ".", + O: []JQOpt{ + { + Name: "from-file", + }, + }, + ErrStr: `disallow option "from-file"`, + }, { Q: ".", O: []JQOpt{ @@ -69,66 +74,40 @@ func TestJQNullInputOption(t *testing.T) { for _, c := range cases { c := c t.Run(fmt.Sprintf("j=%q q=%q o=%v", c.J, c.Q, c.O), func(t *testing.T) { + t.Parallel() + + assert := assert.New(t) jq := &JQ{ J: c.J, Q: c.Q, O: c.O, } err := jq.Validate() - if err == nil && c.ErrStr != "" { - t.Errorf("err should not be nil: %s", c.ErrStr) + + if c.ErrStr != "" { + assert.ErrorContains(err, c.ErrStr) } - if err != nil && c.ErrStr == "" { - t.Errorf("err should be nil: %s", err) - } - - if err != nil && c.ErrStr != "" { - if want, got := c.ErrStr, err.Error(); !strings.Contains(got, want) { - t.Errorf(`err not equal: want=%v got=%v`, want, got) - } + if c.ErrStr == "" { + assert.NoError(err) } }) } } -func TestJQValidateDisallowOpts(t *testing.T) { - jq := &JQ{ - J: "{}", - Q: ".", - O: []JQOpt{ - { - Name: "from-file", - }, - }, - } - - err := jq.Validate() - if err == nil || !strings.Contains(err.Error(), `disallow option "from-file"`) { - t.Errorf(`err should include disallow option "from-file"`) - } -} - func TestJQEvalTimeout(t *testing.T) { - t.Parallel() - - jq := &JQ{ + jq := JQ{ J: `{"dependencies":{"capnp":{"version":"0.1.4","dependencies":{"es6-promise":{"version":"1.0.0","dependencies":{"es6-promise":{"version":"1.0.0"}}}}}}}`, Q: `.dependencies | recurse(to_entries | map(.values.dependencies))`, } - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) - err := jq.Eval(ctx, io.Discard) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + err := newNoLimitJQExec().Eval(ctx, jq, io.Discard) cancel() - - if err != ErrExecTimeout { - t.Errorf("err message should be jq execution timeout, but it's %s", err) - } + assert.ErrorIs(t, err, ErrExecTimeout) } func TestJQEvalCancelled(t *testing.T) { - t.Parallel() - - jq := &JQ{ + jq := JQ{ J: `{"dependencies":{"capnp":{"version":"0.1.4","dependencies":{"es6-promise":{"version":"1.0.0","dependencies":{"es6-promise":{"version":"1.0.0"}}}}}}}`, Q: `.dependencies | recurse(to_entries | map(.values.dependencies))`, } @@ -137,11 +116,17 @@ func TestJQEvalCancelled(t *testing.T) { time.Sleep(3 * time.Second) cancel() }() - err := jq.Eval(ctx, io.Discard) + err := newNoLimitJQExec().Eval(ctx, jq, io.Discard) + assert.ErrorIs(t, err, ErrExecCancelled) +} - if err != ErrExecCancelled { - t.Errorf("err message should be jq execution timeout, but it's %s", err) +func TestJQEvalAborted(t *testing.T) { + jq := JQ{ + J: `{"dependencies":{"capnp":{"version":"0.1.4","dependencies":{"es6-promise":{"version":"1.0.0","dependencies":{"es6-promise":{"version":"1.0.0"}}}}}}}`, + Q: `.dependencies | recurse(to_entries | map(.values.dependencies))`, } + err := newLimitJQExec().Eval(context.Background(), jq, io.Discard) + assert.ErrorIs(t, err, ErrExecAborted) } func TestJQEvalRaceCondition(t *testing.T) { @@ -154,16 +139,36 @@ func TestJQEvalRaceCondition(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - jq := &JQ{ + jq := JQ{ J: `{ "foo": { "bar": { "baz": 123 } } }`, Q: ".", } - err := jq.Eval(ctx, io.Discard) - if err != nil { - t.Errorf("err should be nil: %s", err) - } + err := newNoLimitJQExec().Eval(ctx, jq, io.Discard) + assert.NoError(t, err) }() } wg.Wait() } + +func newNoLimitJQExec() *JQExec { + return &JQExec{ + ResourceLimiter: &noResourceLimiter{}, + } +} + +func newLimitJQExec() *JQExec { + return &JQExec{ + ResourceLimiter: &resourceLimiter{ + MemoryLimit: 1 * 1024 * 1024, // 1 MiB + CPUTimeLimit: 1, // 1 percentage + }, + } +} + +type noResourceLimiter struct { +} + +func (r *noResourceLimiter) LimitResources(proc *os.Process) error { + return nil +} diff --git a/server/handler.go b/server/handler.go index 12c93d7..cb9e583 100644 --- a/server/handler.go +++ b/server/handler.go @@ -1,11 +1,9 @@ package server import ( - "context" "encoding/json" "fmt" "net/http" - "time" "github.com/gin-gonic/gin" "github.com/owenthereal/jqplay/config" @@ -13,10 +11,6 @@ import ( "github.com/sirupsen/logrus" ) -const ( - jqExecTimeout = 15 * time.Second -) - type JQHandlerContext struct { *config.Config JQ string @@ -31,6 +25,7 @@ func (c *JQHandlerContext) ShouldInitJQ() bool { } type JQHandler struct { + JQExec *jq.JQExec DB *DB Config *config.Config } @@ -40,22 +35,19 @@ func (h *JQHandler) handleIndex(c *gin.Context) { } func (h *JQHandler) handleJqPost(c *gin.Context) { - var j *jq.JQ - if err := c.BindJSON(&j); err != nil { + var jq jq.JQ + if err := c.BindJSON(&jq); err != nil { err = fmt.Errorf("error parsing JSON: %s", err) h.logger(c).WithError(err).Info("error parsing JSON") c.String(http.StatusUnprocessableEntity, err.Error()) return } - ctx, cancel := context.WithTimeout(c.Request.Context(), jqExecTimeout) - defer cancel() - c.Header("Content-Type", "text/plain; charset=utf-8") // Evaling into ResponseWriter sets the status code to 200 // appending error message in the end if there's any - if err := j.Eval(ctx, c.Writer); err != nil { + if err := h.JQExec.Eval(c.Request.Context(), jq, c.Writer); err != nil { fmt.Fprint(c.Writer, err.Error()) h.logger(c).WithError(err).Info("jq error") } diff --git a/server/server.go b/server/server.go index ed69659..55a06d5 100644 --- a/server/server.go +++ b/server/server.go @@ -11,10 +11,15 @@ import ( "github.com/oklog/run" "github.com/owenthereal/jqplay" "github.com/owenthereal/jqplay/config" + "github.com/owenthereal/jqplay/jq" "github.com/owenthereal/jqplay/server/middleware" log "github.com/sirupsen/logrus" ) +const ( + requestTimeout = 5 * time.Second +) + func New(c *config.Config) *Server { return &Server{c} } @@ -66,7 +71,7 @@ func newHTTPServer(cfg *config.Config, db *DB) (*http.Server, error) { router := gin.New() router.Use( - middleware.Timeout(25*time.Second), + middleware.Timeout(requestTimeout), middleware.LimitContentLength(10), middleware.Secure(cfg.IsProd()), middleware.RequestID(), @@ -75,7 +80,7 @@ func newHTTPServer(cfg *config.Config, db *DB) (*http.Server, error) { ) router.SetHTMLTemplate(tmpl) - h := &JQHandler{Config: cfg, DB: db} + h := &JQHandler{JQExec: jq.NewJQExec(), Config: cfg, DB: db} router.StaticFS("/assets", http.FS(jqplay.PublicFS)) router.GET("/", h.handleIndex)