initial checksum support for ocis (#1400)

changelog/unreleased/checksums.md

@@ -0,0 +1,8 @@
+Enhancement: Checksum support
+
+We now support checksums on file uploads and PROPFIND results. On uploads, the ocdav service now forwards the `OC-Checksum` (and the similar TUS `Upload-Checksum`)  header to the storage provider. We added an internal http status code that allows storage drivers to return checksum errors. On PROPFINDs, ocdav now renders the `<oc:checksum>` header in a bug compatible way for oc10 backward compatability with existing clients. Finally, GET and HEAD requests now return the `OC-Checksum` header.
+
+https://github.com/cs3org/reva/pull/1400
+https://github.com/owncloud/core/pull/38304
+https://github.com/owncloud/ocis/issues/1291
+https://github.com/owncloud/ocis/issues/1316

internal/grpc/services/storageprovider/storageprovider.go

@@ -315,6 +315,11 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate
 				}, nil
 			}
 		}
+		// TUS forward Upload-Checksum header as checksum, uses '[type] [hash]' format
+		if req.Opaque.Map["Upload-Checksum"] != nil {
+			metadata["checksum"] = string(req.Opaque.Map["Upload-Checksum"].Value)
+		}
+		// ownCloud mtime to set for the uploaded file
 		if req.Opaque.Map["X-OC-Mtime"] != nil {
 			metadata["mtime"] = string(req.Opaque.Map["X-OC-Mtime"].Value)
 		}
@@ -325,6 +330,17 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate
 		switch err.(type) {
 		case errtypes.IsNotFound:
 			st = status.NewNotFound(ctx, "path not found when initiating upload")
+		case errtypes.IsBadRequest, errtypes.IsChecksumMismatch:
+			st = status.NewInvalidArg(ctx, err.Error())
+			// TODO TUS uses a custom ChecksumMismatch 460 http status which is in an unnasigned range in
+			// https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml
+			// maybe 409 conflict is good enough
+			// someone is proposing `419 Checksum Error`, see https://stackoverflow.com/a/35665694
+			// - it is also unassigned
+			// - ends in 9 as the 409 conflict
+			// - is near the 4xx errors about conditions: 415 Unsupported Media Type, 416 Range Not Satisfiable or 417 Expectation Failed
+			// owncloud only expects a 400 Bad request so InvalidArg is good enough for now
+			// seealso errtypes.StatusChecksumMismatch
 		case errtypes.PermissionDenied:
 			st = status.NewPermissionDenied(ctx, err, "permission denied")
 		default:

internal/http/services/owncloud/ocdav/error.go

@@ -23,18 +23,23 @@ import (
 	"net/http"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
+	"github.com/pkg/errors"
 	"github.com/rs/zerolog"
 )
 
 type code int
 
 const (
+
+	// SabredavMethodBadRequest maps to HTTP 400
+	SabredavMethodBadRequest code = iota
 	// SabredavMethodNotAllowed maps to HTTP 405
-	SabredavMethodNotAllowed code = iota
+	SabredavMethodNotAllowed
 )
 
 var (
 	codesEnum = []string{
+		"Sabre\\DAV\\Exception\\BadRequest",
 		"Sabre\\DAV\\Exception\\MethodNotAllowed",
 	}
 )
@@ -54,6 +59,18 @@ func Marshal(e exception) ([]byte, error) {
 	})
 }
 
+// http://www.webdav.org/specs/rfc4918.html#ELEMENT_error
+type errorXML struct {
+	XMLName   xml.Name `xml:"d:error"`
+	Xmlnsd    string   `xml:"xmlns:d,attr"`
+	Xmlnss    string   `xml:"xmlns:s,attr"`
+	Exception string   `xml:"s:exception"`
+	Message   string   `xml:"s:message"`
+	InnerXML  []byte   `xml:",innerxml"`
+}
+
+var errInvalidPropfind = errors.New("webdav: invalid propfind")
+
 // HandleErrorStatus checks the status code, logs a Debug or Error level message
 // and writes an appropriate http status
 func HandleErrorStatus(log *zerolog.Logger, w http.ResponseWriter, s *rpc.Status) {

internal/http/services/owncloud/ocdav/get.go

@@ -19,12 +19,15 @@
 package ocdav
 
 import (
+	"fmt"
 	"io"
 	"net/http"
 	"path"
 	"strconv"
+	"strings"
 	"time"
 
+	"github.com/cs3org/reva/internal/grpc/services/storageprovider"
 	"github.com/cs3org/reva/internal/http/services/datagateway"
 	"go.opencensus.io/trace"
 
@@ -144,11 +147,9 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) {
 	} else {
 		w.Header().Set("Content-Length", strconv.FormatUint(info.Size, 10))
 	}
-	/*
-		if md.Checksum != "" {
-			w.Header().Set("OC-Checksum", md.Checksum)
-		}
-	*/
+	if info.Checksum != nil {
+		w.Header().Set("OC-Checksum", fmt.Sprintf("%s:%s", strings.ToUpper(string(storageprovider.GRPC2PKGXS(info.Checksum.Type))), info.Checksum.Sum))
+	}
 	var c int64
 	if c, err = io.Copy(w, httpRes.Body); err != nil {
 		sublog.Error().Err(err).Msg("error finishing copying data to response")

internal/http/services/owncloud/ocdav/head.go

@@ -19,13 +19,16 @@
 package ocdav
 
 import (
+	"fmt"
 	"net/http"
 	"path"
 	"strconv"
+	"strings"
 	"time"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/cs3org/reva/internal/grpc/services/storageprovider"
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/utils"
 	"go.opencensus.io/trace"
@@ -68,6 +71,9 @@ func (s *svc) handleHead(w http.ResponseWriter, r *http.Request, ns string) {
 	w.Header().Set("ETag", info.Etag)
 	w.Header().Set("OC-FileId", wrapResourceID(info.Id))
 	w.Header().Set("OC-ETag", info.Etag)
+	if info.Checksum != nil {
+		w.Header().Set("OC-Checksum", fmt.Sprintf("%s:%s", strings.ToUpper(string(storageprovider.GRPC2PKGXS(info.Checksum.Type))), info.Checksum.Sum))
+	}
 	t := utils.TSToTime(info.Mtime).UTC()
 	lastModifiedString := t.Format(time.RFC1123Z)
 	w.Header().Set("Last-Modified", lastModifiedString)

internal/http/services/owncloud/ocdav/propfind.go

@@ -38,11 +38,11 @@ import (
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/cs3org/reva/internal/grpc/services/storageprovider"
 	"github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions"
 	"github.com/cs3org/reva/pkg/appctx"
 	ctxuser "github.com/cs3org/reva/pkg/user"
 	"github.com/cs3org/reva/pkg/utils"
-	"github.com/pkg/errors"
 )
 
 const (
@@ -106,6 +106,11 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string)
 
 	metadataKeys := []string{}
 	if pf.Allprop != nil {
+		// TODO this changes the behavior and returns all properties if allprops has been set,
+		// but allprops should only return some default properties
+		// see https://tools.ietf.org/html/rfc4918#section-9.1
+		// the description of arbitrary_metadata_keys in https://cs3org.github.io/cs3apis/#cs3.storage.provider.v1beta1.ListContainerRequest an others may need clarification
+		// tracked in https://github.com/cs3org/cs3apis/issues/104
 		metadataKeys = append(metadataKeys, "*")
 	} else {
 		for i := range pf.Prop {
@@ -214,7 +219,7 @@ func requiresExplicitFetching(n *xml.Name) bool {
 		return false
 	case _nsOwncloud:
 		switch n.Local {
-		case "favorite", "share-types":
+		case "favorite", "share-types", "checksums":
 			return true
 		default:
 			return false
@@ -391,14 +396,35 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide
 		lastModifiedString := t.Format(time.RFC1123Z)
 		response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("d:getlastmodified", lastModifiedString))
 
+		// stay bug compatible with oc10, see https://github.com/owncloud/core/pull/38304#issuecomment-762185241
+		var checksums strings.Builder
 		if md.Checksum != nil {
-			// TODO(jfd): the actual value is an abomination like this:
-			// <oc:checksums>
-			//   <oc:checksum>SHA1:9bd253a09d58be107bcb4169ebf338c8df34d086 MD5:d90bcc6bf847403d22a4abba64e79994 ADLER32:fca23ff5</oc:checksum>
-			// </oc:checksums>
-			// yep, correct, space delimited key value pairs inside an oc:checksum tag inside an oc:checksums tag
-			value := fmt.Sprintf("<oc:checksum>%s:%s</oc:checksum>", md.Checksum.Type, md.Checksum.Sum)
-			response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:checksums", value))
+			checksums.WriteString("<oc:checksum>")
+			checksums.WriteString(strings.ToUpper(string(storageprovider.GRPC2PKGXS(md.Checksum.Type))))
+			checksums.WriteString(":")
+			checksums.WriteString(md.Checksum.Sum)
+		}
+		if md.Opaque != nil {
+			if e, ok := md.Opaque.Map["md5"]; ok {
+				if checksums.Len() == 0 {
+					checksums.WriteString("<oc:checksum>MD5:")
+				} else {
+					checksums.WriteString(" MD5:")
+				}
+				checksums.WriteString(string(e.Value))
+			}
+			if e, ok := md.Opaque.Map["adler32"]; ok {
+				if checksums.Len() == 0 {
+					checksums.WriteString("<oc:checksum>ADLER32:")
+				} else {
+					checksums.WriteString(" ADLER32:")
+				}
+				checksums.WriteString(string(e.Value))
+			}
+		}
+		if checksums.Len() > 0 {
+			checksums.WriteString("</oc:checksum>")
+			response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:checksums", checksums.String()))
 		}
 
 		// favorites from arbitrary metadata
@@ -534,15 +560,37 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide
 					} else {
 						propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:favorite", "0"))
 					}
-				case "checksums": // desktop
+				case "checksums": // desktop ... not really ... the desktop sends the OC-Checksum header
+
+					// stay bug compatible with oc10, see https://github.com/owncloud/core/pull/38304#issuecomment-762185241
+					var checksums strings.Builder
 					if md.Checksum != nil {
-						// TODO(jfd): the actual value is an abomination like this:
-						// <oc:checksums>
-						//   <oc:checksum>SHA1:9bd253a09d58be107bcb4169ebf338c8df34d086 MD5:d90bcc6bf847403d22a4abba64e79994 ADLER32:fca23ff5</oc:checksum>
-						// </oc:checksums>
-						// yep, correct, space delimited key value pairs inside an oc:checksum tag inside an oc:checksums tag
-						value := fmt.Sprintf("<oc:checksum>%s:%s</oc:checksum>", md.Checksum.Type, md.Checksum.Sum)
-						propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:checksums", value))
+						checksums.WriteString("<oc:checksum>")
+						checksums.WriteString(strings.ToUpper(string(storageprovider.GRPC2PKGXS(md.Checksum.Type))))
+						checksums.WriteString(":")
+						checksums.WriteString(md.Checksum.Sum)
+					}
+					if md.Opaque != nil {
+						if e, ok := md.Opaque.Map["md5"]; ok {
+							if checksums.Len() == 0 {
+								checksums.WriteString("<oc:checksum>MD5:")
+							} else {
+								checksums.WriteString(" MD5:")
+							}
+							checksums.WriteString(string(e.Value))
+						}
+						if e, ok := md.Opaque.Map["adler32"]; ok {
+							if checksums.Len() == 0 {
+								checksums.WriteString("<oc:checksum>ADLER32:")
+							} else {
+								checksums.WriteString(" ADLER32:")
+							}
+							checksums.WriteString(string(e.Value))
+						}
+					}
+					if checksums.Len() > 13 {
+						checksums.WriteString("</oc:checksum>")
+						propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:checksums", checksums.String()))
 					} else {
 						propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:checksums", ""))
 					}
@@ -781,15 +829,3 @@ type propertyXML struct {
 	// even including the DAV: namespace.
 	InnerXML []byte `xml:",innerxml"`
 }
-
-// http://www.webdav.org/specs/rfc4918.html#ELEMENT_error
-type errorXML struct {
-	XMLName   xml.Name `xml:"d:error"`
-	Xmlnsd    string   `xml:"xmlns:d,attr"`
-	Xmlnss    string   `xml:"xmlns:s,attr"`
-	Exception string   `xml:"s:exception"`
-	Message   string   `xml:"s:message"`
-	InnerXML  []byte   `xml:",innerxml"`
-}
-
-var errInvalidPropfind = errors.New("webdav: invalid propfind")

internal/http/services/owncloud/ocdav/put.go

@@ -23,13 +23,15 @@ import (
 	"net/http"
 	"path"
 	"strconv"
+	"strings"
 	"time"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/cs3org/reva/internal/http/services/datagateway"
 	"github.com/cs3org/reva/pkg/appctx"
+	"github.com/cs3org/reva/pkg/errtypes"
 	"github.com/cs3org/reva/pkg/rhttp"
 	"github.com/cs3org/reva/pkg/storage/utils/chunking"
 	"github.com/cs3org/reva/pkg/utils"
@@ -204,6 +206,36 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io
 		w.Header().Set("X-OC-Mtime", "accepted")
 	}
 
+	// curl -X PUT https://demo.owncloud.com/remote.php/webdav/testcs.bin -u demo:demo -d '123' -v -H 'OC-Checksum: SHA1:40bd001563085fc35165329ea1ff5c5ecbdbbeef'
+
+	var cparts []string
+	// TUS Upload-Checksum header takes precedence
+	if checksum := r.Header.Get("Upload-Checksum"); checksum != "" {
+		cparts = strings.SplitN(checksum, " ", 2)
+		if len(cparts) != 2 {
+			sublog.Debug().Str("upload-checksum", checksum).Msg("invalid Upload-Checksum format, expected '[algorithm] [checksum]'")
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		// Then try owncloud header
+	} else if checksum := r.Header.Get("OC-Checksum"); checksum != "" {
+		cparts = strings.SplitN(checksum, ":", 2)
+		if len(cparts) != 2 {
+			sublog.Debug().Str("oc-checksum", checksum).Msg("invalid OC-Checksum format, expected '[algorithm]:[checksum]'")
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+	}
+	// we do not check the algorithm here, because it might depend on the storage
+	if len(cparts) == 2 {
+		// Translate into TUS style Upload-Checksum header
+		opaqueMap["Upload-Checksum"] = &typespb.OpaqueEntry{
+			Decoder: "plain",
+			// algorithm is always lowercase, checksum is separated by space
+			Value: []byte(strings.ToLower(cparts[0]) + " " + cparts[1]),
+		}
+	}
+
 	uReq := &provider.InitiateFileUploadRequest{
 		Ref:    ref,
 		Opaque: &typespb.Opaque{Map: opaqueMap},
@@ -249,8 +281,25 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io
 				w.WriteHeader(http.StatusPartialContent)
 				return
 			}
+			if httpRes.StatusCode == errtypes.StatusChecksumMismatch {
+				w.WriteHeader(http.StatusBadRequest)
+				b, err := Marshal(exception{
+					code:    SabredavMethodBadRequest,
+					message: "The computed checksum does not match the one received from the client.",
+				})
+				if err != nil {
+					sublog.Error().Msgf("error marshaling xml response: %s", b)
+					w.WriteHeader(http.StatusInternalServerError)
+					return
+				}
+				_, err = w.Write(b)
+				if err != nil {
+					sublog.Err(err).Msg("error writing response")
+				}
+				return
+			}
 			sublog.Error().Err(err).Msg("PUT request to data server failed")
-			w.WriteHeader(http.StatusInternalServerError)
+			w.WriteHeader(httpRes.StatusCode)
 			return
 		}
 	}

internal/http/services/owncloud/ocdav/tus.go

@@ -56,6 +56,10 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) {
 		w.WriteHeader(http.StatusPreconditionFailed)
 		return
 	}
+	// r.Header.Get("OC-Checksum")
+	// TODO must be SHA1, ADLER32 or MD5 ... in capital letters????
+	// curl -X PUT https://demo.owncloud.com/remote.php/webdav/testcs.bin -u demo:demo -d '123' -v -H 'OC-Checksum: SHA1:40bd001563085fc35165329ea1ff5c5ecbdbbeef'
+
 	//TODO check Expect: 100-continue
 
 	// read filename from metadata