diff --git a/changelog/unreleased/enforce-postprocessing-quarantine-on-download.md b/changelog/unreleased/enforce-postprocessing-quarantine-on-download.md new file mode 100644 index 00000000000..7da3b4edde7 --- /dev/null +++ b/changelog/unreleased/enforce-postprocessing-quarantine-on-download.md @@ -0,0 +1,11 @@ +Security: Enforce postprocessing quarantine on the storage download path + +The "still being processed" quarantine (used e.g. for antivirus scanning) was +only enforced by the ocdav GET/HEAD/PROPFIND handlers via a separate Stat +call. `Decomposedfs.Download` and `Decomposedfs.DownloadRevision` did not +check the node's processing state, so the datagateway `/data` endpoint and +revision downloads could serve unscanned/in-process content, and the ocdav +path was racy. The processing state is now enforced at the storage layer and +the data transfer handler returns `425 Too Early` accordingly. + +https://github.com/owncloud/reva/pull/596 diff --git a/pkg/rhttp/datatx/utils/download/download.go b/pkg/rhttp/datatx/utils/download/download.go index d4b5fbd31e9..47b3382798f 100644 --- a/pkg/rhttp/datatx/utils/download/download.go +++ b/pkg/rhttp/datatx/utils/download/download.go @@ -275,6 +275,9 @@ func handleError(w http.ResponseWriter, log *zerolog.Logger, err error, action s case errtypes.Aborted: log.Debug().Err(err).Str("action", action).Msg("etags do not match") w.WriteHeader(http.StatusPreconditionFailed) + case errtypes.IsTooEarly: + log.Debug().Err(err).Str("action", action).Msg("resource still being processed") + w.WriteHeader(http.StatusTooEarly) default: log.Error().Err(err).Str("action", action).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index c4c4fd1e08f..05d519d99f4 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -1133,6 +1133,15 @@ func (fs *Decomposedfs) Download(ctx context.Context, ref *provider.Reference, o return nil, nil, errtypes.NotFound(f) } + // Do not serve the blob while the node is still being post-processed + // (e.g. virus scan in progress). This must be enforced here, at the + // storage layer, so every caller (ocdav, the datagateway /data endpoint, + // the archiver, ...) is covered and the check cannot be raced against a + // preceding Stat. + if n.IsProcessing(ctx) { + return nil, nil, errtypes.TooEarly(n.ID) + } + ri, err := n.AsResourceInfo(ctx, rp, nil, []string{"size", "mimetype", "etag"}, true) if err != nil { return nil, nil, err diff --git a/pkg/storage/utils/decomposedfs/revisions.go b/pkg/storage/utils/decomposedfs/revisions.go index 0948529f5bf..c4fc075e4f6 100644 --- a/pkg/storage/utils/decomposedfs/revisions.go +++ b/pkg/storage/utils/decomposedfs/revisions.go @@ -152,6 +152,14 @@ func (fs *Decomposedfs) DownloadRevision(ctx context.Context, ref *provider.Refe return nil, nil, errtypes.NotFound(f) } + // Do not serve a revision blob while the node is still being + // post-processed (e.g. virus scan in progress). Enforced at the storage + // layer so the revision download path cannot be used to bypass the + // quarantine. + if n.IsProcessing(ctx) { + return nil, nil, errtypes.TooEarly(n.ID) + } + contentPath := fs.lu.InternalPath(spaceID, revisionKey) blobid, blobsize, err := fs.lu.ReadBlobIDAndSizeAttr(ctx, contentPath, nil)