Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial checksum support for ocis #1400

Merged
merged 12 commits into from
Jan 25, 2021
8 changes: 8 additions & 0 deletions changelog/unreleased/checksums.md
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions internal/grpc/services/storageprovider/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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:
Expand Down
19 changes: 18 additions & 1 deletion internal/http/services/owncloud/ocdav/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
)
Expand All @@ -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) {
Expand Down
11 changes: 6 additions & 5 deletions internal/http/services/owncloud/ocdav/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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")
Expand Down
4 changes: 4 additions & 0 deletions internal/http/services/owncloud/ocdav/head.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -68,6 +71,7 @@ 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)
w.Header().Set("OC-Checksum", fmt.Sprintf("%s:%s", strings.ToUpper(string(storageprovider.GRPC2PKGXS(info.Checksum.Type))), info.Checksum.Sum))
ishank011 marked this conversation as resolved.
Show resolved Hide resolved
t := utils.TSToTime(info.Mtime).UTC()
lastModifiedString := t.Format(time.RFC1123Z)
w.Header().Set("Last-Modified", lastModifiedString)
Expand Down
89 changes: 60 additions & 29 deletions internal/http/services/owncloud/ocdav/propfind.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -207,7 +207,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
Expand Down Expand Up @@ -384,14 +384,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
butonic marked this conversation as resolved.
Show resolved Hide resolved
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() > 13 {
ishank011 marked this conversation as resolved.
Show resolved Hide resolved
checksums.WriteString("</oc:checksum>")
response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:checksums", checksums.String()))
}

// favorites from arbitrary metadata
Expand Down Expand Up @@ -527,15 +548,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", ""))
}
Expand Down Expand Up @@ -774,15 +817,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")
51 changes: 50 additions & 1 deletion internal/http/services/owncloud/ocdav/put.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 != "" {
butonic marked this conversation as resolved.
Show resolved Hide resolved
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},
Expand Down Expand Up @@ -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
}
}
Expand Down
4 changes: 4 additions & 0 deletions internal/http/services/owncloud/ocdav/tus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions pkg/errtypes/errtypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,22 @@ func (e BadRequest) Error() string { return "error: bad request: " + string(e) }
// IsBadRequest implements the IsBadRequest interface.
func (e BadRequest) IsBadRequest() {}

// ChecksumMismatch is the error to use when the sent hash does not match the calculated hash.
type ChecksumMismatch string

func (e ChecksumMismatch) Error() string { return "error: checksum mismatch: " + string(e) }

// IsChecksumMismatch implements the IsChecksumMismatch interface.
func (e ChecksumMismatch) IsChecksumMismatch() {}

// StatusChecksumMismatch 419 is an unofficial http status code in an unassigned range that is used for checksum mismatches
// Proposed by https://stackoverflow.com/a/35665694
// Official HTTP status code registry: https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml
// Note: TUS uses unassigned 460 Checksum-Mismatch
// RFC proposal for checksum digest uses a `Want-Digest` header: https://tools.ietf.org/html/rfc3230
// oc clienst issue: https://github.com/owncloud/core/issues/22711
const StatusChecksumMismatch = 419

// IsNotFound is the interface to implement
// to specify that an a resource is not found.
type IsNotFound interface {
Expand Down Expand Up @@ -147,3 +163,9 @@ type IsPartialContent interface {
type IsBadRequest interface {
IsBadRequest()
}

// IsChecksumMismatch is the interface to implement
// to specify that a checksum does not match.
type IsChecksumMismatch interface {
IsChecksumMismatch()
}
Loading