Use sigv4 for sending data to s3 #772

SgtCoDFish · 2026-02-05T16:28:05Z

So far, I believe only our e2e test tenant is feature gated to have this enabled, but the implication of that is that our tests fail. The long term goal is to have this enabled everywhere.

Just changing to sigv4 shouldn't require changes to the agent but the backend added a couple of extra requirements alongside changing to sigv4 - i.e. requiring headers for tagging and encryption.

This PR also adds tests for those new headers.

Backwards compat: the backend will handle backwards compatibility (i.e. they'll allow older version of the agent to not send the required headers for some period of time, to give people time to update).

Just using sigv4 shouldn't require changes, but the backend made a couple of extra requirements alongside changing to sigv4 - requiring headers for tagging and encryption. Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish

Minor self review

SgtCoDFish · 2026-02-10T11:17:05Z

internal/cyberark/dataupload/dataupload.go

-// TODO(wallrj): There is a bug in the AWS backend:
-// [S3 Presigned PutObjectCommand URLs ignore Sha256 Hash when uploading](https://github.com/aws/aws-sdk/issues/480)
-// ...which means that the `x-amz-checksum-sha256` request header is optional.
-// If you omit that header, it is possible to PUT any data.
-// There is a work around listed in that issue which we have shared with the
-// CyberArk API team.


nb: switching to sigv4 should fix this, so I removed the comment.

I verified that S3 does now reject a payload that differs from the checksum , by modifying the code to send a different body

modified internal/cyberark/dataupload/dataupload.go @@ -130,7 +130,7 @@ func (c *CyberArkClient) PutSnapshot(ctx context.Context, snapshot Snapshot) err } // The snapshot-links endpoint returns an AWS presigned URL which only supports the PUT verb. - req, err := http.NewRequestWithContext(ctx, http.MethodPut, presignedUploadURL, encodedBody) + req, err := http.NewRequestWithContext(ctx, http.MethodPut, presignedUploadURL, bytes.NewBufferString("foo")) if err != nil { return err }

$ HTTPS_PROXY=localhost:8080 go test ./internal/cyberark -run TestCyberArkClient_PutSnapshot_RealAPI -v -count 1 -args -testing.v 6 === RUN TestCyberArkClient_PutSnapshot_RealAPI client_test.go:68: This test runs against a live service and has been known to flake. If you see timeout issues it's possible that the test is flaking and it could be unrelated to your changes. roundtripper_log.go:40: I0212 09:38:17.751277] outgoing http request succeeded method="POST" uri="https://anb5751.id.integration-cyberark.cloud/Security/StartAuthentication" status="200 OK" duration="864.907164ms" identity.go:296: I0212 09:38:17.753307] made successful request to StartAuthentication source="Identity.doStartAuthentication" summary="NewPackage" roundtripper_log.go:40: I0212 09:38:18.290674] outgoing http request succeeded method="POST" uri="https://anb5751.id.integration-cyberark.cloud/Security/AdvanceAuthentication" status="200 OK" duration="536.940608ms" identity.go:406: I0212 09:38:18.293336] successfully completed AdvanceAuthentication request to CyberArk Identity; login complete username="github-jetstack-secure-tests@cyberark.cloud.420375" roundtripper_log.go:40: I0212 09:38:20.618176] outgoing http request succeeded method="POST" uri="https://tlskp-test.inventory.integration-cyberark.cloud/api/ingestions/kubernetes/snapshot-links" status="200 OK" duration="2.323008336s" roundtripper_log.go:40: I0212 09:38:21.599913] outgoing http request succeeded method="PUT" uri="https://snapshots-marshmallow-a1b2c3d4-integration.s3.amazonaws.com/k8s_snapshot/8f08a102-58ca-49cd-960e-debc5e0d3cd4/ffffffff-ffff-ffff-ffff-ffffffffffff/20260212_093819_024.snapshot?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAR53UX6ITGLFJ3X7O%2F20260212%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20260212T093820Z&X-Amz-SignedHeaders=host%3Bx-amz-checksum-sha256%3Bx-amz-server-side-encryption%3Bx-amz-tagging&X-Amz-Expires=300&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEAoaCXVzLWVhc3QtMSJGMEQCIFYTedh%2FtPURHsWllUDXtmFzYLBl2ACZJZZscIxApSPuAiBZ9%2Big%2BRIpBjxkcbsRB%2F9u9zNjRLQCDhdCfK%2B%2BLku3DiqmBAjT%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F8BEAAaDDEzMjg1MTk1NDIxNCIM8ShUS54FDYcNlFzSKvoDfSXbVrv1TG33VxVBECBPhw1eUx8OUIcrquWueS%2F8oKFkp9NWzOtl8CyLA9UXldxup%2BtG4MEevYkwxYJR2YUGnowCSKYwQ9MSM7HecXOoEYiv2R3Limw7FJ32TqoswlnVseDrMvvcBRIT0h5Iriw1MqEQgTpeCm8L3jWoPtOuA7Cl0faL1VMiVM%2FmTfB4SmIeUEuHj8dC1Vh96x6%2B9PWQPKbAxKrJJhTxGJgaRp7r2S3v5kqsw94wvU6T56km597okOsAWZS%2FVswQNwZkjTDRgr1qamefBRsUR6S54kY7oUxHmNvffL%2BshAUrWwEePR0eb%2Fi%2BZZ4bPJEX1lqHZlpHVh5E9SexGbNHkmPqrU4Uz4NM3%2BZ9QBHBOYrjE0Wl3eZbUzn%2FPXfF%2BXAp0pzrDyF2eTF00zDn9%2FuSB1SCpRBh6mLP490E%2BilXFFc25x0Km8jpVMEruDjH77UhVIoLdhKz%2B8NYpZwvfDkHW%2F82e5JuWwUv8ON7Ax%2FC55%2B13m7IogN2WgiOAjHun2gHqXq%2FyQDxi%2BMNmOxUEstaixlMcc4uyl4GMZId%2FpcD7avWeUrUjmiEiIGUjyJCeOnsWTWnEAo2hVGDee4q2cBUz515gpZNbp6NHJ6cqkDOMl20SYWwKuoFWPQkDZD0Lm8bctuhv%2BeqrOOSNOdo7lcnm3gwhr%2B2zAY6ngEtm1mWVekgk1uDzsiSUH4iC50qabbQkgHJ87n5uvW944G5wCahkqtK8s0KFM02c5ZgrPZDsPcNXCh%2FDuiVkJxSybpKFkrG2MGjnCG%2FyLfkXGNYmiNNZCj6eVJ7cbEIWRv4GwVStA2BZbPNNqFNlNYtuXNEOQHAi4g5ado5T2eh0uC1VCvjwJqLWRmSvzVXfagptFILYuquy2JolEUFyQ%3D%3D&X-Amz-Signature=38dc054859815b32ce4ca81e41e16dd5dc87e0e5fa2459c9fbbaf5dc7717a9d6" status="400 Bad Request" duration="949.808567ms" client_test.go:94: Error Trace: /home/richard/projects/jetstack/jetstack-secure/internal/cyberark/client_test.go:94 Error: Received unexpected error: received response with status code 400: <?xml version="1.0" encoding="UTF-8"?> <Error><Code>BadDigest</Code><Message>The SHA256 you specified did not match the calculated checksum.</Message><RequestId>4MPWDH55H9VP8EYD</RequestId><HostId>+tbF2v0ebSHEQuipi9nY0qWgv5ZIYmY8gWT5nOnBdvGb44o82qDBxWOw4S60xg/V1ku1ojVROA4=</HostId></Error> Test: TestCyberArkClient_PutSnapshot_RealAPI --- FAIL: TestCyberArkClient_PutSnapshot_RealAPI (6.03s) FAIL FAIL github.com/jetstack/preflight/internal/cyberark 6.416s FAIL

SgtCoDFish · 2026-02-10T13:12:01Z

internal/cyberark/dataupload/dataupload.go

+	username, err := c.authenticateRequest(req)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to authenticate request: %s", err)
 	}


note: since the username is inherently tied to the token, I wanted to ensure that the two were only able to be pulled simultaneously while the lock was held.

In the current codebase I can't really see how the username could desync from the token if we passed it around some other way but I think it's best to make the change now.

I assume it's possible to get the username from the token itself if we parse it but I'd rather keep it as a black-box and not depend on any implementation details of it.

Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish · 2026-02-11T15:06:03Z

internal/cyberark/dataupload/mock.go

-	switch r.URL.Path {
-	case apiPathSnapshotLinks:
-		mds.handleSnapshotLinks(w, r)
-		return
-	case "/presigned-upload":
-		mds.handlePresignedUpload(w, r)
-		return
-	default:
-		w.WriteHeader(http.StatusNotFound)
-	}


note: I wanted to use ServeMux for this to track variables across calls, so I reworked this a bit

wallrj-cyberark

Looks good @SgtCoDFish

I ran one of the integration tests locally, via mitmproxy, and observed the new tagging headers:

$ HTTPS_PROXY=localhost:8080 go test ./internal/cyberark -v --testing.v 6

X-Amz-Tagging:                 agent_version=development&tenant_id=8f08a102-58ca-49cd-960e-debc5e0d3cd4&upload_type=k8s_snapshot&uploader_id=ffffffff-ffff-ffff-ffff-ffffffffffff&username=g
                               ithub-jetstack-secure-tests%40cyberark.cloud.420375&vendor=k8s

Here's the full copilot review FYI: https://gist.github.com/wallrj-cyberark/da6a2188c37d414a61ace5d345f0ebaa

wallrj-cyberark · 2026-02-11T17:14:26Z

internal/cyberark/dataupload/mock.go

+
 	// Write response body
 	w.WriteHeader(http.StatusOK)
 	w.Header().Set("Content-Type", "application/json")


Copilot spotted a problem in this existing code. It thinks that Header.Set MUST come before any WriteHeader otherwise it is discarded.

5. Mock server: w.WriteHeader before w.Header().Set

w.WriteHeader(http.StatusOK) w.Header().Set("Content-Type", "application/json")

In Go's net/http, headers must be set before calling WriteHeader. After WriteHeader is called, setting headers has no effect. This is a bug in the mock (though it may not affect test correctness since JSON decoding doesn't depend on Content-Type).

Good shout, I'll address that in #770 since that'll need a rebase once this merges!

SgtCoDFish force-pushed the sigv4 branch 4 times, most recently from b2bfea1 to 0709b68 Compare February 6, 2026 16:12

SgtCoDFish mentioned this pull request Feb 6, 2026

Update links to non-broken page #774

Open

SgtCoDFish force-pushed the sigv4 branch 2 times, most recently from ed5a68d to 90d4cd0 Compare February 10, 2026 10:34

SgtCoDFish changed the title ~~wip: use sigv4 for sending data to s3~~ Use sigv4 for sending data to s3 Feb 10, 2026

SgtCoDFish force-pushed the sigv4 branch from 90d4cd0 to a6a2a9c Compare February 10, 2026 12:16

use sigv4 for sending data to s3

f8ee4b1

Just using sigv4 shouldn't require changes, but the backend made a couple of extra requirements alongside changing to sigv4 - requiring headers for tagging and encryption. Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish force-pushed the sigv4 branch from a6a2a9c to f8ee4b1 Compare February 10, 2026 13:12

SgtCoDFish commented Feb 10, 2026

View reviewed changes

add file size sending and improve tests for sigv4

298d84f

Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish commented Feb 11, 2026

View reviewed changes

wallrj-cyberark self-requested a review February 11, 2026 16:05

wallrj-cyberark approved these changes Feb 11, 2026

View reviewed changes

SgtCoDFish merged commit d6d80f4 into master Feb 12, 2026
7 of 8 checks passed

SgtCoDFish deleted the sigv4 branch February 12, 2026 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use sigv4 for sending data to s3 #772

Use sigv4 for sending data to s3 #772

Uh oh!

SgtCoDFish commented Feb 5, 2026 •

edited

Loading

Uh oh!

SgtCoDFish left a comment

Uh oh!

SgtCoDFish Feb 10, 2026

Uh oh!

wallrj-cyberark Feb 12, 2026

Uh oh!

SgtCoDFish Feb 10, 2026

Uh oh!

SgtCoDFish Feb 11, 2026

Uh oh!

wallrj-cyberark left a comment

Uh oh!

wallrj-cyberark Feb 11, 2026

Uh oh!

SgtCoDFish Feb 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Use sigv4 for sending data to s3 #772

Use sigv4 for sending data to s3 #772

Uh oh!

Conversation

SgtCoDFish commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SgtCoDFish left a comment

Choose a reason for hiding this comment

Uh oh!

SgtCoDFish Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

wallrj-cyberark Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

SgtCoDFish Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

SgtCoDFish Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

wallrj-cyberark left a comment

Choose a reason for hiding this comment

Uh oh!

wallrj-cyberark Feb 11, 2026

Choose a reason for hiding this comment

5. Mock server: w.WriteHeader before w.Header().Set

Uh oh!

SgtCoDFish Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SgtCoDFish commented Feb 5, 2026 •

edited

Loading

5. Mock server: `w.WriteHeader` before `w.Header().Set`