[S3] Binaries uploaded without permission check before session.save()?

[Alexander Klimetschek <ak...@adobe.com.INVALID>]

Hi everyone,

one thing I noticed in the S3 datastore while working on the direct binary access: it seems as if binaries added via InputStream might get uploaded before a session.save(). That might be because the upload happens (by default) asynchronously in the S3 DS, and that pool is filled right when the binary (Blob) is added, but before it gets persisted.

That might be ok, but the problem is that the AC check only happens during session.save(). IIUC, that means an anonymous session could upload binaries into the S3.

I saw the behavior while testing on the 1.8 branch. The code was very complicated to follow because of the asynchronous nature, so maybe I am missing something, such as a "cancel upload" upon failure of the session.save().

On a related note, it also seemed impossible to disable the async upload. Apparently setting stagingSplitPercentage = 0 should disable it (which is a bit of an unintuitive name if you ask me) but it did not work for me.

Please excuse if this has been fixed in trunk, I didn't have the chance to test this there.

Cheers,
Alex

Re: [S3] Binaries uploaded without permission check before session.save()?

[Amit Jain <am...@ieee.org>]

>> one thing I noticed in the S3 datastore while working on the direct
binary access: it seems as if binaries added via InputStream might get
uploaded before a session.save(). That might be because the upload happens
(by default) asynchronously in the S3 DS, and that pool is filled right
when the binary (Blob) is added, but before it gets persisted.
The behavior is not dependent on the type of the DataStore or if the
uploads are synchronous or asynchronous. This is the behavior to be
expected while using any BlobStore as the InputStream is uploaded as part
of creating the Binary object which is then set as the property value to be
created in the Node [1].

>> On a related note, it also seemed impossible to disable the async
upload. Apparently setting stagingSplitPercentage = 0 should disable it
(which is a bit of an unintuitive name if you ask me) but it did not work
for me.
How did you conclude that the async upload was not disabled, through the
logs or by glancing at the cache directory configured? Setting the property
to 0 should disable async uploads [2], [3], [4]. Maybe the config change
(to set stagingSplitPercentage = 0)  did not get reflected.

Thanks
Amit

[1]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-spi/src/main/java/org/apache/jackrabbit/oak/plugins/value/jcr/ValueFactoryImpl.java#L286
[2]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/CompositeDataStoreCache.java#L76
[3]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/UploadStagingCache.java#L208
[4]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/AbstractSharedCachingDataStore.java#L244


On Thu, Jun 21, 2018 at 11:27 PM Alexander Klimetschek
<ak...@adobe.com.invalid> wrote:

> Hi everyone,
>
> one thing I noticed in the S3 datastore while working on the direct binary
> access: it seems as if binaries added via InputStream might get uploaded
> before a session.save(). That might be because the upload happens (by
> default) asynchronously in the S3 DS, and that pool is filled right when
> the binary (Blob) is added, but before it gets persisted.
>
> That might be ok, but the problem is that the AC check only happens during
> session.save(). IIUC, that means an anonymous session could upload binaries
> into the S3.
>
> I saw the behavior while testing on the 1.8 branch. The code was very
> complicated to follow because of the asynchronous nature, so maybe I am
> missing something, such as a "cancel upload" upon failure of the
> session.save().
>
> On a related note, it also seemed impossible to disable the async upload.
> Apparently setting stagingSplitPercentage = 0 should disable it (which is a
> bit of an unintuitive name if you ask me) but it did not work for me.
>
> Please excuse if this has been fixed in trunk, I didn't have the chance to
> test this there.
>
> Cheers,
> Alex