Skip to content

Commit 0709b68

Browse files
committed
MASSIVE HACK: use sigv4 for sending data to s3
Currently, this is only supported for one tenant with a feature flag. Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
1 parent d58fc65 commit 0709b68

File tree

9 files changed

+62
-32
lines changed

9 files changed

+62
-32
lines changed

internal/cyberark/client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func LoadClientConfigFromEnvironment() (ClientConfig, error) {
4949
// NewDatauploadClient initializes and returns a new CyberArk Data Upload client.
5050
// It performs service discovery to find the necessary API endpoints and authenticates
5151
// using the provided client configuration.
52-
func NewDatauploadClient(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, cfg ClientConfig) (*dataupload.CyberArkClient, error) {
52+
func NewDatauploadClient(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, tenantUUID string, cfg ClientConfig) (*dataupload.CyberArkClient, error) {
5353
identityAPI := serviceMap.Identity.API
5454
if identityAPI == "" {
5555
return nil, errors.New("service discovery returned an empty identity API")
@@ -67,5 +67,5 @@ func NewDatauploadClient(ctx context.Context, httpClient *http.Client, serviceMa
6767
return nil, err
6868
}
6969

70-
return dataupload.New(httpClient, discoveryAPI, identityClient.AuthenticateRequest), nil
70+
return dataupload.New(httpClient, discoveryAPI, tenantUUID, cfg.Username, identityClient.AuthenticateRequest), nil
7171
}

internal/cyberark/client_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) {
3434

3535
discoveryClient := servicediscovery.New(httpClient)
3636

37-
serviceMap, err := discoveryClient.DiscoverServices(t.Context(), cfg.Subdomain)
37+
serviceMap, tenantUUID, err := discoveryClient.DiscoverServices(t.Context(), cfg.Subdomain)
3838
if err != nil {
3939
t.Fatalf("failed to discover mock services: %v", err)
4040
}
4141

42-
cl, err := cyberark.NewDatauploadClient(ctx, httpClient, serviceMap, cfg)
42+
cl, err := cyberark.NewDatauploadClient(ctx, httpClient, serviceMap, tenantUUID, cfg)
4343
require.NoError(t, err)
4444

4545
err = cl.PutSnapshot(ctx, dataupload.Snapshot{
@@ -78,12 +78,12 @@ func TestCyberArkClient_PutSnapshot_RealAPI(t *testing.T) {
7878

7979
discoveryClient := servicediscovery.New(httpClient)
8080

81-
serviceMap, err := discoveryClient.DiscoverServices(t.Context(), cfg.Subdomain)
81+
serviceMap, tenantUUID, err := discoveryClient.DiscoverServices(t.Context(), cfg.Subdomain)
8282
if err != nil {
8383
t.Fatalf("failed to discover services: %v", err)
8484
}
8585

86-
cl, err := cyberark.NewDatauploadClient(ctx, httpClient, serviceMap, cfg)
86+
cl, err := cyberark.NewDatauploadClient(ctx, httpClient, serviceMap, tenantUUID, cfg)
8787
require.NoError(t, err)
8888

8989
err = cl.PutSnapshot(ctx, dataupload.Snapshot{

internal/cyberark/dataupload/dataupload.go

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"io"
1212
"net/http"
1313
"net/url"
14+
"strings"
1415

1516
"k8s.io/apimachinery/pkg/runtime"
1617

@@ -33,13 +34,21 @@ type CyberArkClient struct {
3334
baseURL string
3435
httpClient *http.Client
3536

37+
tenantUUID string
38+
username string
39+
3640
authenticateRequest func(req *http.Request) error
3741
}
3842

39-
func New(httpClient *http.Client, baseURL string, authenticateRequest func(req *http.Request) error) *CyberArkClient {
43+
// TODO: should probably take a cyberark Identity client directly and query subdomain + username from that
44+
func New(httpClient *http.Client, baseURL string, tenantUUID string, username string, authenticateRequest func(req *http.Request) error) *CyberArkClient {
4045
return &CyberArkClient{
41-
baseURL: baseURL,
42-
httpClient: httpClient,
46+
baseURL: baseURL,
47+
httpClient: httpClient,
48+
49+
tenantUUID: tenantUUID,
50+
username: username,
51+
4352
authenticateRequest: authenticateRequest,
4453
}
4554
}
@@ -102,13 +111,6 @@ type Snapshot struct {
102111
// has been received intact.
103112
// Read [Checking object integrity for data uploads in Amazon S3](https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity-upload.html),
104113
// to learn more.
105-
//
106-
// TODO(wallrj): There is a bug in the AWS backend:
107-
// [S3 Presigned PutObjectCommand URLs ignore Sha256 Hash when uploading](https://github.com/aws/aws-sdk/issues/480)
108-
// ...which means that the `x-amz-checksum-sha256` request header is optional.
109-
// If you omit that header, it is possible to PUT any data.
110-
// There is a work around listed in that issue which we have shared with the
111-
// CyberArk API team.
112114
func (c *CyberArkClient) PutSnapshot(ctx context.Context, snapshot Snapshot) error {
113115
if snapshot.ClusterID == "" {
114116
return fmt.Errorf("programmer mistake: the snapshot cluster ID cannot be left empty")
@@ -133,6 +135,30 @@ func (c *CyberArkClient) PutSnapshot(ctx context.Context, snapshot Snapshot) err
133135
return err
134136
}
135137
req.Header.Set("X-Amz-Checksum-Sha256", checksumBase64)
138+
139+
// TODO: this is temporary logic to only enable the extra sigv4 headers on the specific tenant with
140+
// the feature flag enabled
141+
// We'll remove this later.
142+
if c.tenantUUID == "8f08a102-58ca-49cd-960e-debc5e0d3cd4" {
143+
req.Header.Set("X-Amz-Server-Side-Encryption", "AES256")
144+
145+
q := url.Values{}
146+
147+
q.Add("agent_version", snapshot.AgentVersion)
148+
q.Add("tenant_id", c.tenantUUID)
149+
q.Add("upload_type", "k8s_snapshot")
150+
q.Add("uploader_id", snapshot.ClusterID)
151+
q.Add("username", c.username)
152+
q.Add("vendor", "k8s")
153+
154+
// TODO: Remove this hack when urlencoding is fixed on the backend
155+
// MASSIVE HACK: backend is not url-encoding the username, so we need to "decode" it here to match what the backend expects
156+
// Backend has committed to change this soon, but to unbreak tests in the meantime we need to do this hack.
157+
encodedTags := strings.ReplaceAll(q.Encode(), "%40", "@")
158+
159+
req.Header.Set("X-Amz-Tagging", encodedTags)
160+
}
161+
136162
version.SetUserAgent(req)
137163

138164
res, err := c.httpClient.Do(req)

internal/cyberark/dataupload/dataupload_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) {
9696

9797
datauploadAPIBaseURL, httpClient := dataupload.MockDataUploadServer(t)
9898

99-
cyberArkClient := dataupload.New(httpClient, datauploadAPIBaseURL, tc.authenticate)
99+
cyberArkClient := dataupload.New(httpClient, datauploadAPIBaseURL, "test-tenant-uuid", "test-user@example.com", tc.authenticate)
100100

101101
err := cyberArkClient.PutSnapshot(ctx, tc.snapshot)
102102
tc.requireFn(t, err)

internal/cyberark/identity/cmd/testidentity/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func run(ctx context.Context) error {
5151
httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs)
5252

5353
sdClient := servicediscovery.New(httpClient)
54-
services, err := sdClient.DiscoverServices(ctx, subdomain)
54+
services, _, err := sdClient.DiscoverServices(ctx, subdomain)
5555
if err != nil {
5656
return fmt.Errorf("while performing service discovery: %s", err)
5757
}

internal/cyberark/identity/identity_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestLoginUsernamePassword_RealAPI(t *testing.T) {
5353
arktesting.SkipIfNoEnv(t)
5454
subdomain := os.Getenv("ARK_SUBDOMAIN")
5555
httpClient := http.DefaultClient
56-
services, err := servicediscovery.New(httpClient).DiscoverServices(t.Context(), subdomain)
56+
services, _, err := servicediscovery.New(httpClient).DiscoverServices(t.Context(), subdomain)
5757
require.NoError(t, err)
5858

5959
loginUsernamePasswordTests(t, func(t testing.TB) inputs {

internal/cyberark/servicediscovery/discovery.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,17 +95,21 @@ type Services struct {
9595

9696
// DiscoverServices fetches from the service discovery service for a given subdomain
9797
// and parses the CyberArk Identity API URL and Inventory API URL.
98-
func (c *Client) DiscoverServices(ctx context.Context, subdomain string) (*Services, error) {
98+
// It also returns the Tenant ID UUID corresponding to the subdomain.
99+
func (c *Client) DiscoverServices(ctx context.Context, subdomain string) (*Services, string, error) {
99100
u, err := url.Parse(c.baseURL)
100101
if err != nil {
101-
return nil, fmt.Errorf("invalid base URL for service discovery: %w", err)
102+
return nil, "", fmt.Errorf("invalid base URL for service discovery: %w", err)
102103
}
104+
103105
u.Path = path.Join(u.Path, "api/public/tenant-discovery")
104106
u.RawQuery = url.Values{"bySubdomain": []string{subdomain}}.Encode()
107+
105108
endpoint := u.String()
109+
106110
request, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
107111
if err != nil {
108-
return nil, fmt.Errorf("failed to initialise request to %s: %s", endpoint, err)
112+
return nil, "", fmt.Errorf("failed to initialise request to %s: %s", endpoint, err)
109113
}
110114

111115
request.Header.Set("Accept", "application/json")
@@ -114,7 +118,7 @@ func (c *Client) DiscoverServices(ctx context.Context, subdomain string) (*Servi
114118
arkapi.SetTelemetryRequestHeader(request)
115119
resp, err := c.client.Do(request)
116120
if err != nil {
117-
return nil, fmt.Errorf("failed to perform HTTP request: %s", err)
121+
return nil, "", fmt.Errorf("failed to perform HTTP request: %s", err)
118122
}
119123

120124
defer resp.Body.Close()
@@ -123,19 +127,19 @@ func (c *Client) DiscoverServices(ctx context.Context, subdomain string) (*Servi
123127
// a 404 error is returned with an empty JSON body "{}" if the subdomain is unknown; at the time of writing, we haven't observed
124128
// any other errors and so we can't special case them
125129
if resp.StatusCode == http.StatusNotFound {
126-
return nil, fmt.Errorf("got an HTTP 404 response from service discovery; maybe the subdomain %q is incorrect or does not exist?", subdomain)
130+
return nil, "", fmt.Errorf("got an HTTP 404 response from service discovery; maybe the subdomain %q is incorrect or does not exist?", subdomain)
127131
}
128132

129-
return nil, fmt.Errorf("got unexpected status code %s from request to service discovery API", resp.Status)
133+
return nil, "", fmt.Errorf("got unexpected status code %s from request to service discovery API", resp.Status)
130134
}
131135

132136
var discoveryResp DiscoveryResponse
133137
err = json.NewDecoder(io.LimitReader(resp.Body, maxDiscoverBodySize)).Decode(&discoveryResp)
134138
if err != nil {
135139
if err == io.ErrUnexpectedEOF {
136-
return nil, fmt.Errorf("rejecting JSON response from server as it was too large or was truncated")
140+
return nil, "", fmt.Errorf("rejecting JSON response from server as it was too large or was truncated")
137141
}
138-
return nil, fmt.Errorf("failed to parse JSON from otherwise successful request to service discovery endpoint: %s", err)
142+
return nil, "", fmt.Errorf("failed to parse JSON from otherwise successful request to service discovery endpoint: %s", err)
139143
}
140144
var identityAPI, discoveryContextAPI string
141145
for _, svc := range discoveryResp.Services {
@@ -158,13 +162,13 @@ func (c *Client) DiscoverServices(ctx context.Context, subdomain string) (*Servi
158162
}
159163

160164
if identityAPI == "" {
161-
return nil, fmt.Errorf("didn't find %s in service discovery response, "+
165+
return nil, "", fmt.Errorf("didn't find %s in service discovery response, "+
162166
"which may indicate a suspended tenant; unable to detect CyberArk Identity API URL", IdentityServiceName)
163167
}
164168
//TODO: Should add a check for discoveryContextAPI too?
165169

166170
return &Services{
167171
Identity: ServiceEndpoint{API: identityAPI},
168172
DiscoveryContext: ServiceEndpoint{API: discoveryContextAPI},
169-
}, nil
173+
}, discoveryResp.TenantID, nil
170174
}

internal/cyberark/servicediscovery/discovery_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) {
6666

6767
client := New(httpClient)
6868

69-
services, err := client.DiscoverServices(ctx, testSpec.subdomain)
69+
services, _, err := client.DiscoverServices(ctx, testSpec.subdomain)
7070
if testSpec.expectedError != nil {
7171
assert.EqualError(t, err, testSpec.expectedError.Error())
7272
assert.Nil(t, services)

pkg/client/client_cyberark.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (o *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readin
6767

6868
discoveryClient := servicediscovery.New(o.httpClient)
6969

70-
serviceMap, err := discoveryClient.DiscoverServices(ctx, cfg.Subdomain)
70+
serviceMap, tenantUUID, err := discoveryClient.DiscoverServices(ctx, cfg.Subdomain)
7171
if err != nil {
7272
return err
7373
}
@@ -81,7 +81,7 @@ func (o *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readin
8181
// Minimize the snapshot to reduce size and improve privacy
8282
minimizeSnapshot(log.V(logs.Debug), &snapshot)
8383

84-
datauploadClient, err := cyberark.NewDatauploadClient(ctx, o.httpClient, serviceMap, cfg)
84+
datauploadClient, err := cyberark.NewDatauploadClient(ctx, o.httpClient, serviceMap, tenantUUID, cfg)
8585
if err != nil {
8686
return fmt.Errorf("while initializing data upload client: %s", err)
8787
}

0 commit comments

Comments
 (0)