coredump: add checksum for uploads #542
Conversation
It seems the OTel S3 endpoint did get more restrictive and now requires a checksum for uploaded items. Without this checksum, the following error is returned: failed to upload file: operation error S3: PutObject, https response error StatusCode: 400, RequestID: sjc-1:<some-random-ID>, HostID: , api error InvalidArgument: x-amz-content-sha256 must be UNSIGNED-PAYLOAD or a valid sha256 value. Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
tools/coredump/modulestore/store.go
Outdated
| @@ -197,16 +200,26 @@ func (store *Store) UploadModule(id ID) error { | |||
| return fmt.Errorf("failed to open local file: %w", err) | |||
| } | |||
|
|
|||
| fileData, err := io.ReadAll(file) | |||
There was a problem hiding this comment.
We don't know how large a coredump file in the future can be, how much RAM a user has available or if the coredump already is in a RAM disk.
I'd suggest to hash from disk and also let PutObject stream from disk.
The potential (but extremely unlikely) TOCTOU (data changes between reads) is caught on the server side with the checksum provided.
There was a problem hiding this comment.
The Oracle backed S3 storage did not accept streams. That's why I made this change alongside the newly required checksum.
There was a problem hiding this comment.
Ah, you should have mentioned this in the PR description.
It means that just adding the checksum does not allow to upload?
There was a problem hiding this comment.
sorry 🙏 I just try to get it working and did forget about this. but I think its not worth a dedicated PR.
There was a problem hiding this comment.
This sounds weird. How did you test this? The Body seems to be working on a reader interface so this really should not matter.
Wild guess: did you use same reader for first hashing and not seeking back to beginning so that the http send starts from right position?
There was a problem hiding this comment.
While helping to get the coredump tests pass for #289 I did run into this issue. And as coredump tests pass in #289, I was able to upload the new coredumps with this change successfully.
Based on your feedback, I dropped the io.ReadAll and bytes.NewReader. Hope this works for you.
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
It seems the OTel S3 endpoint did get more restrictive and now requires a checksum for uploaded items.
Without this checksum, the following error is returned:
failed to upload file: operation error S3: PutObject, https response error StatusCode: 400, RequestID: sjc-1:, HostID: , api error InvalidArgument: x-amz-content-sha256 must be UNSIGNED-PAYLOAD or a valid sha256 value.