Merge pull request #130 from MichaelEischer/verify-upload

Verify uploaded files

changelog/unreleased/issue-122

@@ -0,0 +1,9 @@
+Enhancement: Verify uploaded files
+
+The rest-server now by default verifies that the hash of content of uploaded
+files matches their filename. This ensures that transmission errors are
+detected and forces restic to retry the upload. On low-power devices it can
+make sense to disable this check by passing the `--no-verify-upload` flag.
+
+https://github.com/restic/rest-server/issues/122
+https://github.com/restic/rest-server/pull/130

cmd/rest-server/main.go

@@ -45,6 +45,8 @@ func init() {
 	flags.StringVar(&server.TLSCert, "tls-cert", server.TLSCert, "TLS certificate path")
 	flags.StringVar(&server.TLSKey, "tls-key", server.TLSKey, "TLS key path")
 	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.PrivateRepos, "private-repos", server.PrivateRepos, "users can only access their private repo")
 	flags.BoolVar(&server.Prometheus, "prometheus", server.Prometheus, "enable Prometheus metrics")

handlers.go

@@ -29,6 +29,7 @@ type Server struct {
 	Debug            bool
 	MaxRepoSize      int64
 	PanicOnError     bool
+	NoVerifyUpload   bool
 
 	htpasswdFile *HtpasswdFile
 	quotaManager *quota.Manager
@@ -84,10 +85,11 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	// Pass the request to the repo.Handler
 	opt := repo.Options{
-		AppendOnly:   s.AppendOnly,
+		AppendOnly:     s.AppendOnly,
-		Debug:        s.Debug,
+		Debug:          s.Debug,
-		QuotaManager: s.quotaManager, // may be nil
+		QuotaManager:   s.quotaManager, // may be nil
-		PanicOnError: s.PanicOnError,
+		PanicOnError:   s.PanicOnError,
+		NoVerifyUpload: s.NoVerifyUpload,
 	}
 	if s.Prometheus {
 		opt.BlobMetricFunc = makeBlobMetricFunc(username, folderPath)

handlers_test.go

@@ -3,6 +3,7 @@ package restserver
 import (
 	"bytes"
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -103,47 +104,58 @@ type TestRequest struct {
 
 // 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) []TestRequest {
+func createOverwriteDeleteSeq(t testing.TB, path string, data string) []TestRequest {
 	// add a file, try to overwrite and delete it
 	req := []TestRequest{
 		{
 			req:  newRequest(t, "GET", path, nil),
 			want: []wantFunc{wantCode(http.StatusNotFound)},
 		},
-		{
+	}
-			req:  newRequest(t, "POST", path, strings.NewReader("foobar test config")),
+
+	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)},
 		},
-		{
+		TestRequest{
 			req: newRequest(t, "GET", path, nil),
 			want: []wantFunc{
 				wantCode(http.StatusOK),
-				wantBody("foobar test config"),
+				wantBody(data),
 			},
 		},
-		{
+		TestRequest{
-			req:  newRequest(t, "POST", path, strings.NewReader("other config")),
+			req:  newRequest(t, "POST", path, strings.NewReader(data+"other stuff")),
 			want: []wantFunc{wantCode(http.StatusForbidden)},
 		},
-		{
+		TestRequest{
 			req: newRequest(t, "GET", path, nil),
 			want: []wantFunc{
 				wantCode(http.StatusOK),
-				wantBody("foobar test config"),
+				wantBody(data),
 			},
 		},
-		{
+		TestRequest{
 			req:  newRequest(t, "DELETE", path, nil),
 			want: []wantFunc{wantCode(http.StatusForbidden)},
 		},
-		{
+		TestRequest{
 			req: newRequest(t, "GET", path, nil),
 			want: []wantFunc{
 				wantCode(http.StatusOK),
-				wantBody("foobar test config"),
+				wantBody(data),
 			},
 		},
-	}
+	)
 	return req
 }
 
@@ -154,53 +166,59 @@ func TestResticHandler(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	randomID := hex.EncodeToString(buf)
+	data := "random data file " + hex.EncodeToString(buf)
+	dataHash := sha256.Sum256([]byte(data))
+	fileID := hex.EncodeToString(dataHash[:])
 
 	var tests = []struct {
 		seq []TestRequest
 	}{
-		{createOverwriteDeleteSeq(t, "/config")},
+		{createOverwriteDeleteSeq(t, "/config", data)},
-		{createOverwriteDeleteSeq(t, "/data/"+randomID)},
+		{createOverwriteDeleteSeq(t, "/data/"+fileID, data)},
 		{
 			// ensure we can add and remove lock files
 			[]TestRequest{
 				{
-					req:  newRequest(t, "GET", "/locks/"+randomID, nil),
+					req:  newRequest(t, "GET", "/locks/"+fileID, nil),
 					want: []wantFunc{wantCode(http.StatusNotFound)},
 				},
 				{
-					req:  newRequest(t, "POST", "/locks/"+randomID, strings.NewReader("lock file")),
+					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/"+randomID, nil),
+					req: newRequest(t, "GET", "/locks/"+fileID, nil),
 					want: []wantFunc{
 						wantCode(http.StatusOK),
-						wantBody("lock file"),
+						wantBody(data),
 					},
 				},
 				{
-					req:  newRequest(t, "POST", "/locks/"+randomID, strings.NewReader("other lock file")),
+					req:  newRequest(t, "POST", "/locks/"+fileID, strings.NewReader(data+"other data")),
 					want: []wantFunc{wantCode(http.StatusForbidden)},
 				},
 				{
-					req:  newRequest(t, "DELETE", "/locks/"+randomID, nil),
+					req:  newRequest(t, "DELETE", "/locks/"+fileID, nil),
 					want: []wantFunc{wantCode(http.StatusOK)},
 				},
 				{
-					req:  newRequest(t, "GET", "/locks/"+randomID, nil),
+					req:  newRequest(t, "GET", "/locks/"+fileID, nil),
 					want: []wantFunc{wantCode(http.StatusNotFound)},
 				},
 			},
 		},
 
 		// Test subrepos
-		{createOverwriteDeleteSeq(t, "/parent1/sub1/config")},
+		{createOverwriteDeleteSeq(t, "/parent1/sub1/config", "foobar")},
-		{createOverwriteDeleteSeq(t, "/parent1/sub1/data/"+randomID)},
+		{createOverwriteDeleteSeq(t, "/parent1/sub1/data/"+fileID, data)},
-		{createOverwriteDeleteSeq(t, "/parent1/config")},
+		{createOverwriteDeleteSeq(t, "/parent1/config", "foobar")},
-		{createOverwriteDeleteSeq(t, "/parent1/data/"+randomID)},
+		{createOverwriteDeleteSeq(t, "/parent1/data/"+fileID, data)},
-		{createOverwriteDeleteSeq(t, "/parent2/config")},
+		{createOverwriteDeleteSeq(t, "/parent2/config", "foobar")},
-		{createOverwriteDeleteSeq(t, "/parent2/data/"+randomID)},
+		{createOverwriteDeleteSeq(t, "/parent2/data/"+fileID, data)},
 	}
 
 	// setup rclone with a local backend in a temporary directory

repo/repo.go

@@ -1,6 +1,8 @@
 package repo
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -19,10 +21,11 @@ import (
 
 // Options are options for the Handler accepted by New
 type Options struct {
-	AppendOnly bool // if set, delete actions are not allowed
+	AppendOnly     bool // if set, delete actions are not allowed
-	Debug      bool
+	Debug          bool
-	DirMode    os.FileMode
+	DirMode        os.FileMode
-	FileMode   os.FileMode
+	FileMode       os.FileMode
+	NoVerifyUpload bool
 
 	// If set, we will panic when an internal server error happens. This
 	// makes it easier to debug such errors.
@@ -569,7 +572,22 @@ func (h *Handler) saveBlob(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	written, err := io.Copy(outFile, r.Body)
+	var written int64
+
+	if h.opt.NoVerifyUpload {
+		// just write the file without checking the contents
+		written, err = io.Copy(outFile, r.Body)
+	} else {
+		// calculate hash for current request
+		hasher := sha256.New()
+		written, err = io.Copy(outFile, io.TeeReader(r.Body, hasher))
+
+		// reject if file content doesn't match file name
+		if err == nil && hex.EncodeToString(hasher.Sum(nil)) != objectID {
+			err = fmt.Errorf("file content does not match hash")
+		}
+	}
+
 	if err != nil {
 		_ = tf.Close()
 		_ = os.Remove(path)