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

Add content hash to uploaded content #72

Open
Randgalt opened this issue Jun 17, 2024 · 4 comments
Open

Add content hash to uploaded content #72

Randgalt opened this issue Jun 17, 2024 · 4 comments

Comments

@Randgalt
Copy link
Member

We currently don't add a hash to content uploaded to the remote server. We should do some kind of hashing.

@Randgalt
Copy link
Member Author

Actually, I'm not sure this is needed. I'm looking in the AWS source code and I see this:

    protected String calculateContentHash(SdkHttpFullRequest.Builder mutableRequest, Aws4SignerParams signerParams,
                                          SdkChecksum contentFlexibleChecksum) {
        if ("https".equals(mutableRequest.protocol())) {
            return UNSIGNED_PAYLOAD;
        }
        return super.calculateContentHash(mutableRequest, signerParams, contentFlexibleChecksum);
    }

So, for https they don't do signed payloads. We will always be https to the remote server (we can validate this in our code) so we should be safe with unsigned payloads to the remote server.

@Randgalt
Copy link
Member Author

However...

if we don't sign the payload to the remote server, we have an edge case security hole. If someone is familiar with our proxy, they'd know that we stream content to the remote server. Thus, we only validate the signing content from the source as it streams. If the final bytes or final chunk doesn't validate, we will have already sent the entire payload to the remote server.

We can overcome this by adding a small buffer in the proxy. This way, if the final bytes or final chunk don't validate, we will still be able to cancel the upload to the remote server.

@vagaerg
Copy link
Member

vagaerg commented Jun 20, 2024

Would this be an issue? I've only had a quick look, but the chunked stream handler should fail before returning an EOF, and thus the multipart upload would never be finished (this is my understanding at least).

The new hash-validating handler added for non-chunked uploads in #71 should fail before returning an EOF, but I agree for those cases we would potentially have processed the content before we send the final bytes. I don't know how the framework would deal with this, but if the connection is not gracefully closed by the client is it possible the upload gets aborted too?

@Randgalt
Copy link
Member Author

I need to do some testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants