A user could create a DOS by uploading files that are too large

[slifty]

(This is related to #144)

We currently store in-transit data locally AND have no limits on how large a given uploaded file can be. This means anybody could deny service by simply uploading a file that is larger than the locally configured storage space.

I believe the only solution here is to stop using local storage for temporary space; this would mean using S3 as the temporary storage location. We might, separately, also want to create limits to how much data can be uploaded by a given user (this could be rooted in knowing how much storage space they have available on their account)

[kfogel]

In our meeting today we decided on the S3-as-tmp-storage solution, along with enabling the SDK to query for account storage limits, so that we can do some basic "no you can't upload a 100TB file" protections by tracking the amount of in-flight data and making sure that doesn't exceed the user's quote (or 5x the user's quota if we have 5 separate SFTP servers, but whatever).

[slifty]

Some thoughts about implementation here:

1. We could integrate S3 interaction directly into the sftp service
2. We could instead find a way for ths sftp service to utilize the upload microservice (e.g. generating a presigned post from it and using that presigned post)

The idea of the microservice was to prevent the need for direct integration with S3 in our systems, and so this would be a useful approach unless the intention is to eliminate the microservice. I'll raise this with the team during our rclone meeting today.

[slifty]

On another note: S3 has a limit of 5GB per POST, which means we either will have a 5GB limit via SFTP or we should do multipart uploads.

The upload service as written does not currently support multipart uploads, though this would not be a bad idea to improve. This article talks about how to do multipart uploads + presigned urls.

This is a bit moot though because the permanent back end itself also does not support multipart uploads, which means it cannot support files larger than 5GB without a similar multipart enhancement.

[slifty]

We did decide to move to the S3 microservice!

[slifty]

My naive implementation plan needs improvement. I'm documenting it here to just record the evolution:

Where I started

1. Communicate with upload-service to generate a presigned post (max size 5GB)
2. Write to the temporary storage location over a series of posts
3. On completion, register the record with permanent, using the createArchiveRecord sdk method
   3b. Inside the SDK method, we are actually re-uploading the data to S3.

This approach is not ideal because data is being sent around a full blown 2x more than necessary (we write to S3 to tmp storage, then read from that and write AGAIN to "unprocessed" storage!)

Where I'm going

The registerRecord API endpoint already takes an S3 URL.

I think it would be good to update the SDK to optionally accept an S3 url instead of file data, and if that happens it should skip the "upload" step and instead just jump to registering the record.

This should also pair with the SDK to providing direct access to the createS3Upload endpoint (right now it abstracts that flow from the SDK user). This would allow an SDK client to either:

1. Register the record + upload without having to think about presigned posts.
2. Having more granular control over the upload by accessing the presigned post and register record separately.

This would also mean we don't need to have the SFTP service directly interact with the upload service, since it's just getting the presigned post via the API. Another benefit of this is that the API can handle size allocation / validation and prevent that form of potential abuse.

[slifty]

I began to explore this path with a bit more depth -- I'm fairly sure that it's not actually possible to re-use a presigned posat and specify a byte range to post, and instead this really is the purpose of multipart upload.

I spoke with @cecilia-donnelly and we agreed that although multipart upload is on the road map for backend / API support, it makes sense to just implement it on this service for now in the interests of resolving this issue that is affecting users today. Eventually that should be replaced with an SDK-driven multipart upload.

[cecilia-donnelly]

We didn't end up implementing multipart upload but we made the SFTP service have more temporary storage. I think that was the end of this, right?

[slifty]

This is still a vulnerability (though less extreme than it was before) -- I would leave the issue open until permanent is ready to prioritize resolving it.