From cc0f7cea2c6efb7b01d8b32e96d3fa033f4f8e40 Mon Sep 17 00:00:00 2001 From: Dmitry Nezhevenko Date: Fri, 27 May 2022 18:19:20 +0300 Subject: [PATCH 1/2] Add 'write-only' mode Write-only mode allows only backup. So there is no way to read data from repo (except metadata) --- cmd/rest-server/main.go | 3 +- handlers.go | 2 + handlers_test.go | 283 ++++++++++++++++++++++++++-------------- repo/repo.go | 15 ++- 4 files changed, 204 insertions(+), 99 deletions(-) diff --git a/cmd/rest-server/main.go b/cmd/rest-server/main.go index 4287b97a..ddc8e52c 100644 --- a/cmd/rest-server/main.go +++ b/cmd/rest-server/main.go @@ -49,7 +49,8 @@ func init() { flags.BoolVar(&server.NoAuth, "no-auth", server.NoAuth, "disable .htpasswd authentication") flags.BoolVar(&server.NoVerifyUpload, "no-verify-upload", server.NoVerifyUpload, "do not verify the integrity of uploaded data. DO NOT enable unless the rest-server runs on a very low-power device") - flags.BoolVar(&server.AppendOnly, "append-only", server.AppendOnly, "enable append only mode") + flags.BoolVar(&server.AppendOnly, "append-only", server.AppendOnly, "enable append only mode (disables delete)") + flags.BoolVar(&server.WriteOnly, "write-only", server.WriteOnly, "enable write only mode (disables delete and restore)") flags.BoolVar(&server.PrivateRepos, "private-repos", server.PrivateRepos, "users can only access their private repo") flags.BoolVar(&server.Prometheus, "prometheus", server.Prometheus, "enable Prometheus metrics") flags.BoolVar(&server.PrometheusNoAuth, "prometheus-no-auth", server.PrometheusNoAuth, "disable auth for Prometheus /metrics endpoint") diff --git a/handlers.go b/handlers.go index 9df6adf8..a1e194f1 100644 --- a/handlers.go +++ b/handlers.go @@ -23,6 +23,7 @@ type Server struct { TLS bool NoAuth bool AppendOnly bool + WriteOnly bool PrivateRepos bool Prometheus bool PrometheusNoAuth bool @@ -86,6 +87,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Pass the request to the repo.Handler opt := repo.Options{ AppendOnly: s.AppendOnly, + WriteOnly: s.WriteOnly, Debug: s.Debug, QuotaManager: s.quotaManager, // may be nil PanicOnError: s.PanicOnError, diff --git a/handlers_test.go b/handlers_test.go index e5931d7d..c7b561cd 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -107,65 +107,105 @@ type TestRequest struct { want []wantFunc } +// TestSuite is a group of TestRequest that covers some functionality +type TestSuite []struct { + seq []TestRequest +} + +const ( + GetForbidden = 1 << iota + PostForbidden + PostBrokenForbidden + DeleteForbidden +) + // createOverwriteDeleteSeq returns a sequence which will create a new file at -// path, and then try to overwrite and delete it. -func createOverwriteDeleteSeq(t testing.TB, path string, data string) []TestRequest { +// path, and then try to overwrite and delete it if allowed by flags +func createOverwriteDeleteSeq(t testing.TB, path string, data string, forbiddenFlags int) []TestRequest { + // path, read it and then try to overwrite and delete (if not forbidden by flags) + checkFlag := func(flag int, flagged []wantFunc, arg []wantFunc) []wantFunc { + if flag&forbiddenFlags == 0 { + return arg + } + return flagged + } + + checkForbidden := func(flag int, arg []wantFunc) []wantFunc { + return checkFlag(flag, []wantFunc{ + wantCode(http.StatusForbidden), + }, arg) + } + + ifNotDeleted := func(arg []wantFunc) []wantFunc { + if forbiddenFlags&DeleteForbidden != 0 { + return arg + } + return []wantFunc{ + wantCode(http.StatusNotFound), + } + } + + brokenData := data + "_broken" + expectedData := data + if forbiddenFlags&PostBrokenForbidden == 0 { + expectedData = brokenData + } + // add a file, try to overwrite and delete it req := []TestRequest{ { req: newRequest(t, "GET", path, nil), - want: []wantFunc{wantCode(http.StatusNotFound)}, + want: checkForbidden(GetForbidden, []wantFunc{wantCode(http.StatusNotFound)}), }, - } - - if !strings.HasSuffix(path, "/config") { - req = append(req, TestRequest{ - // broken upload must fail - req: newRequest(t, "POST", path, strings.NewReader(data+"broken")), - want: []wantFunc{wantCode(http.StatusBadRequest)}, - }) - } - - req = append(req, - TestRequest{ - req: newRequest(t, "POST", path, strings.NewReader(data)), - want: []wantFunc{wantCode(http.StatusOK)}, + { + // broken upload must fail if repo is configured to verify blobs + req: newRequest(t, "POST", path, strings.NewReader(brokenData)), + want: checkForbidden(PostForbidden, + checkFlag(PostBrokenForbidden, + []wantFunc{wantCode(http.StatusBadRequest)}, + []wantFunc{wantCode(http.StatusOK)})), + }, + { // if blob verification is not enabled, we'll get Forbidden here because broken data was uploaded before + req: newRequest(t, "POST", path, strings.NewReader(data)), + want: checkForbidden(PostForbidden, + checkFlag(PostBrokenForbidden, + []wantFunc{wantCode(http.StatusOK)}, + []wantFunc{wantCode(http.StatusForbidden)})), }, - TestRequest{ + { req: newRequest(t, "GET", path, nil), - want: []wantFunc{ + want: checkForbidden(GetForbidden, []wantFunc{ wantCode(http.StatusOK), - wantBody(data), - }, + wantBody(expectedData), + }), }, - TestRequest{ + { // always Forbidden because it's overwrite of existing data req: newRequest(t, "POST", path, strings.NewReader(data+"other stuff")), want: []wantFunc{wantCode(http.StatusForbidden)}, }, - TestRequest{ + { req: newRequest(t, "GET", path, nil), - want: []wantFunc{ + want: checkForbidden(GetForbidden, []wantFunc{ wantCode(http.StatusOK), - wantBody(data), - }, + wantBody(expectedData), + }), }, - TestRequest{ + { req: newRequest(t, "DELETE", path, nil), - want: []wantFunc{wantCode(http.StatusForbidden)}, + want: checkForbidden(DeleteForbidden, []wantFunc{wantCode(http.StatusOK)}), }, - TestRequest{ + { req: newRequest(t, "GET", path, nil), - want: []wantFunc{ + want: checkForbidden(GetForbidden, ifNotDeleted([]wantFunc{ wantCode(http.StatusOK), - wantBody(data), - }, + wantBody(expectedData), + })), }, - ) + } return req } -// TestResticHandler runs tests on the restic handler code, especially in append-only mode. -func TestResticHandler(t *testing.T) { +func randomDataAndId(t *testing.T) (string, string) { buf := make([]byte, 32) _, err := io.ReadFull(rand.Reader, buf) if err != nil { @@ -174,58 +214,11 @@ func TestResticHandler(t *testing.T) { data := "random data file " + hex.EncodeToString(buf) dataHash := sha256.Sum256([]byte(data)) fileID := hex.EncodeToString(dataHash[:]) + return data, fileID +} - var tests = []struct { - seq []TestRequest - }{ - {createOverwriteDeleteSeq(t, "/config", data)}, - {createOverwriteDeleteSeq(t, "/data/"+fileID, data)}, - { - // ensure we can add and remove lock files - []TestRequest{ - { - req: newRequest(t, "GET", "/locks/"+fileID, nil), - want: []wantFunc{wantCode(http.StatusNotFound)}, - }, - { - req: newRequest(t, "POST", "/locks/"+fileID, strings.NewReader(data+"broken")), - want: []wantFunc{wantCode(http.StatusBadRequest)}, - }, - { - req: newRequest(t, "POST", "/locks/"+fileID, strings.NewReader(data)), - want: []wantFunc{wantCode(http.StatusOK)}, - }, - { - req: newRequest(t, "GET", "/locks/"+fileID, nil), - want: []wantFunc{ - wantCode(http.StatusOK), - wantBody(data), - }, - }, - { - req: newRequest(t, "POST", "/locks/"+fileID, strings.NewReader(data+"other data")), - want: []wantFunc{wantCode(http.StatusForbidden)}, - }, - { - req: newRequest(t, "DELETE", "/locks/"+fileID, nil), - want: []wantFunc{wantCode(http.StatusOK)}, - }, - { - req: newRequest(t, "GET", "/locks/"+fileID, nil), - want: []wantFunc{wantCode(http.StatusNotFound)}, - }, - }, - }, - - // Test subrepos - {createOverwriteDeleteSeq(t, "/parent1/sub1/config", "foobar")}, - {createOverwriteDeleteSeq(t, "/parent1/sub1/data/"+fileID, data)}, - {createOverwriteDeleteSeq(t, "/parent1/config", "foobar")}, - {createOverwriteDeleteSeq(t, "/parent1/data/"+fileID, data)}, - {createOverwriteDeleteSeq(t, "/parent2/config", "foobar")}, - {createOverwriteDeleteSeq(t, "/parent2/data/"+fileID, data)}, - } - +// testResticHandler creates repo in temporary dir and runs tests on the restic handler code +func testResticHandler(t *testing.T, tests *TestSuite, server Server, pathsToCreate []string) { // setup the server with a local backend in a temporary directory tempdir, err := ioutil.TempDir("", "rest-server-test-") if err != nil { @@ -240,26 +233,20 @@ func TestResticHandler(t *testing.T) { } }() - // set append-only mode and configure path - mux, err := NewHandler(&Server{ - AppendOnly: true, - Path: tempdir, - NoAuth: true, - Debug: true, - PanicOnError: true, - }) + server.Path = tempdir + mux, err := NewHandler(&server) if err != nil { t.Fatalf("error from NewHandler: %v", err) } // create the repos - for _, path := range []string{"/", "/parent1/sub1/", "/parent1/", "/parent2/"} { + for _, path := range pathsToCreate { checkRequest(t, mux.ServeHTTP, newRequest(t, "POST", path+"?create=true", nil), []wantFunc{wantCode(http.StatusOK)}) } - for _, test := range tests { + for _, test := range *tests { t.Run("", func(t *testing.T) { for i, seq := range test.seq { t.Logf("request %v: %v %v", i, seq.req.Method, seq.req.URL.Path) @@ -269,6 +256,110 @@ func TestResticHandler(t *testing.T) { } } +// TestResticHandler runs tests on the restic handler code, default mode (everything allowed) +func TestResticDefaultHandler(t *testing.T) { + data, fileID := randomDataAndId(t) + + var tests = TestSuite{ + {createOverwriteDeleteSeq(t, "/config", data, 0)}, + {createOverwriteDeleteSeq(t, "/keys/"+fileID, data, PostBrokenForbidden)}, + {createOverwriteDeleteSeq(t, "/index/"+fileID, data, PostBrokenForbidden)}, + {createOverwriteDeleteSeq(t, "/data/"+fileID, data, PostBrokenForbidden)}, + {createOverwriteDeleteSeq(t, "/snapshots/"+fileID, data, PostBrokenForbidden)}, + {createOverwriteDeleteSeq(t, "/locks/"+fileID, data, PostBrokenForbidden)}, + } + // set append-only mode + testResticHandler(t, &tests, Server{ + NoAuth: true, + Debug: true, + PanicOnError: true, + }, []string{"/"}) +} + +// TestResticHandler runs tests on the restic handler code, disabled blob verification +func TestResticNoVerifyUploadHandler(t *testing.T) { + data, fileID := randomDataAndId(t) + + var tests = TestSuite{ + {createOverwriteDeleteSeq(t, "/config", data, 0)}, + {createOverwriteDeleteSeq(t, "/keys/"+fileID, data, 0)}, + {createOverwriteDeleteSeq(t, "/index/"+fileID, data, 0)}, + {createOverwriteDeleteSeq(t, "/data/"+fileID, data, 0)}, + {createOverwriteDeleteSeq(t, "/snapshots/"+fileID, data, 0)}, + {createOverwriteDeleteSeq(t, "/locks/"+fileID, data, 0)}, + } + // set append-only mode + testResticHandler(t, &tests, Server{ + NoAuth: true, + Debug: true, + PanicOnError: true, + NoVerifyUpload: true, + }, []string{"/"}) +} + +// TestResticHandler runs tests on the restic handler code, default mode (everything allowed) +func TestResticAppendOnlyUploadHandler(t *testing.T) { + data, fileID := randomDataAndId(t) + + var tests = TestSuite{ + {createOverwriteDeleteSeq(t, "/config", data, DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/keys/"+fileID, data, PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/index/"+fileID, data, PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/data/"+fileID, data, PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/snapshots/"+fileID, data, PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/locks/"+fileID, data, PostBrokenForbidden)}, + } + // set append-only mode + testResticHandler(t, &tests, Server{ + NoAuth: true, + Debug: true, + PanicOnError: true, + AppendOnly: true, + }, []string{"/"}) +} + +// TestResticHandler runs tests on the restic handler code, default mode (everything allowed) +func TestResticWriteOnlyUploadHandler(t *testing.T) { + data, fileID := randomDataAndId(t) + + var tests = TestSuite{ + {createOverwriteDeleteSeq(t, "/config", data, DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/keys/"+fileID, data, PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/index/"+fileID, data, PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/data/"+fileID, data, GetForbidden|PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/snapshots/"+fileID, data, PostBrokenForbidden|DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/locks/"+fileID, data, PostBrokenForbidden)}, + } + // set append-only mode + testResticHandler(t, &tests, Server{ + NoAuth: true, + Debug: true, + PanicOnError: true, + WriteOnly: true, + }, []string{"/"}) +} + +// TestResticHandler runs tests on the restic handler code, default mode (everything allowed) +func TestResticPrivateRepoUploadHandler(t *testing.T) { + data, fileID := randomDataAndId(t) + + var tests = TestSuite{ + {createOverwriteDeleteSeq(t, "/parent1/sub1/config", "foobar", DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/parent1/sub1/data/"+fileID, data, DeleteForbidden|PostBrokenForbidden)}, + {createOverwriteDeleteSeq(t, "/parent1/config", "foobar", DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/parent1/data/"+fileID, data, DeleteForbidden|PostBrokenForbidden)}, + {createOverwriteDeleteSeq(t, "/parent2/config", "foobar", DeleteForbidden)}, + {createOverwriteDeleteSeq(t, "/parent2/data/"+fileID, data, DeleteForbidden|PostBrokenForbidden)}, + } + // set append-only mode + testResticHandler(t, &tests, Server{ + AppendOnly: true, + NoAuth: true, + Debug: true, + PanicOnError: true, + }, []string{"/", "/parent1/sub1/", "/parent1/", "/parent2/"}) +} + func TestSplitURLPath(t *testing.T) { var tests = []struct { // Params diff --git a/repo/repo.go b/repo/repo.go index 812bee2a..2d1da231 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -27,6 +27,7 @@ import ( // Options are options for the Handler accepted by New type Options struct { AppendOnly bool // if set, delete actions are not allowed + WriteOnly bool // if set, delete and get blob actions are not allowed Debug bool DirMode os.FileMode FileMode os.FileMode @@ -40,6 +41,10 @@ type Options struct { QuotaManager *quota.Manager } +func (o *Options) deleteDataForbidden() bool { + return o.AppendOnly || o.WriteOnly +} + // DefaultDirMode is the file mode used for directory creation if not // overridden in the Options const DefaultDirMode os.FileMode = 0700 @@ -323,7 +328,7 @@ func (h *Handler) deleteConfig(w http.ResponseWriter, r *http.Request) { log.Println("deleteConfig()") } - if h.opt.AppendOnly { + if h.opt.deleteDataForbidden() { httpDefaultError(w, http.StatusForbidden) return } @@ -515,6 +520,12 @@ func (h *Handler) getBlob(w http.ResponseWriter, r *http.Request) { "cannot determine object type or id: %s", r.URL.Path)) return } + + if h.opt.WriteOnly && objectType == "data" { + httpDefaultError(w, http.StatusForbidden) + return + } + path := h.getObjectPath(objectType, objectID) file, err := os.Open(path) @@ -705,7 +716,7 @@ func (h *Handler) deleteBlob(w http.ResponseWriter, r *http.Request) { "cannot determine object type or id: %s", r.URL.Path)) return } - if h.opt.AppendOnly && objectType != "locks" { + if h.opt.deleteDataForbidden() && objectType != "locks" { httpDefaultError(w, http.StatusForbidden) return } From 626075a47c3472e3e213132485e15aa8272e6640 Mon Sep 17 00:00:00 2001 From: Dmitry Nezhevenko Date: Fri, 27 May 2022 18:33:01 +0300 Subject: [PATCH 2/2] Add changelog --- changelog/unreleased/issue-110 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/issue-110 diff --git a/changelog/unreleased/issue-110 b/changelog/unreleased/issue-110 new file mode 100644 index 00000000..2443d0b4 --- /dev/null +++ b/changelog/unreleased/issue-110 @@ -0,0 +1,8 @@ +Enhancement: Add write-only mode + +'write-only' mode is similar to 'append-only', but also disables 'restore'. If +enabled, attacker that got access to one of systems where restic is running +will be unable download data from shared restic repository + +https://github.com/restic/rest-server/issues/110 +https://github.com/restic/rest-server/pull/192