[Fix] Upgraded AWS go-sdk to V2 (only for AWS Canary Token Verification) #3907

abmussani · 2025-02-12T14:12:33Z

Description:

The AWS SDK for Go (aws-sdk-go) has entered maintenance mode, as noted in its README. This PR upgrades the SDK to v2, specifically for verifying AWS Canary tokens.

This upgrade addresses an issue where the previous version intermittently returned signature errors, incorrectly marking valid credentials as unverified.

Note: This upgrade is limited to AWS Canary token verification. The rest of the codebase will continue using the older AWS SDK version.

Checklist:

Tests passing (make test-community)?
Lint passing (make lint this requires golangci-lint)?

kashifkhan0771

🚀

rgmz · 2025-02-12T14:39:35Z

This upgrade addresses an issue where the previous version intermittently returned signature errors, incorrectly marking valid credentials as unverified.

What part of the change fixes this issue?

abmussani · 2025-02-12T15:22:54Z

This upgrade addresses an issue where the previous version intermittently returned signature errors, incorrectly marking valid credentials as unverified.

What part of the change fixes this issue?

Just migration of aws sdk to v2 and some code in canary.go file

rgmz · 2025-02-12T15:27:52Z

Just migration of aws sdk to v2 and some code in canary.go file

That was never an issue with the canary flow AFAIk. It was in the normal Access Key / Session Key detector flows.

trufflehog/pkg/detectors/aws/access_keys/accesskey.go

Lines 280 to 284 in f2dc96e

    
           } else if res.StatusCode == 403 { 
        
           	// Experimentation has indicated that if you make two GetCallerIdentity requests within five seconds that 
        
           	// share a key ID but are signed with different secrets the second one will be rejected with a 403 that 
        
           	// carries a SignatureDoesNotMatch code in its body. This happens even if the second ID-secret pair is 
        
           	// valid. Since this is exactly our access pattern, we need to work around it.

abmussani · 2025-02-12T15:47:35Z

Just migration of aws sdk to v2 and some code in canary.go file

That was never an issue with the canary flow AFAIk. It was in the normal Access Key / Session Key detector flows.

trufflehog/pkg/detectors/aws/access_keys/accesskey.go

Lines 280 to 284 in f2dc96e

} else if res.StatusCode == 403 {

// Experimentation has indicated that if you make two GetCallerIdentity requests within five seconds that

// share a key ID but are signed with different secrets the second one will be rejected with a 403 that

// carries a SignatureDoesNotMatch code in its body. This happens even if the second ID-secret pair is

// valid. Since this is exactly our access pattern, we need to work around it.

Some of the customer reported that Live AWS Canary tokens are not being verified. On Debugging, I noted that the old version of SDK was randomly causing signature verification error which got fixed when migrated to V2.

Sorry I misread your comment. Verification of Canary id/secret is the part of access key / session key detector. Few of the customer reported that Live AWS Canary tokens are not being verified which got fixed once I migrated to V2.

abmussani · 2025-02-12T16:01:26Z

thank you @rgmz for pointing out the usage in access_key.go I should also migrate the code to V2.

abmussani · 2025-02-14T10:55:46Z

HI @rgmz , Thank you for pointing out that comment. I have replaced that custom API Request code using SDK methods. After that I am not able to reproduce signature related error even using the same Id and multiple secrets with it. Can you do me a favor to review the code and if I have missed any case. TIA 🙇
The new changes includes:

Use the SDK getCallerIdentity method to verify access key and secret (non-canary).
Use of custom HTTP Client for better logging.
Handling of error if Id is invalid. Reason: In SDK, they have change the error text for this case.

Note: AWS SessionKey detector is still using the custom API request method. If these changes are good enough, I can work on that part too.

abmussani · 2025-02-14T11:12:07Z

@zricethezav I also request you to re-review the updated changes. TIA.

rgmz · 2025-02-14T15:17:12Z

pkg/detectors/aws/access_keys/accesskey.go

-		return false, nil, fmt.Errorf("request returned status %d with an unexpected reason (%s: %s)", res.StatusCode, body.Error.Code, body.Error.Message)
-	} else {
-		return false, nil, fmt.Errorf("request to %v returned unexpected status %d", res.Request.URL, res.StatusCode)
+		return false, nil, fmt.Errorf("request returned unexpected error: %s", err.Error())


Suggested change

return false, nil, fmt.Errorf("request returned unexpected error: %s", err.Error())

return false, nil, fmt.Errorf("request returned unexpected error: %w", err)

rgmz · 2025-02-14T15:27:44Z

pkg/detectors/aws/access_keys/accesskey.go

+	cfg, err := config.LoadDefaultConfig(ctx,
+		config.WithRegion(region),
+		config.WithHTTPClient(s.getClient()),
+		config.WithCredentialsProvider(
+			credentials.NewStaticCredentialsProvider(resIDMatch, resSecretMatch, ""),
+		),
+	)
 	if err != nil {
 		return false, nil, err
 	}
-	req.Header.Set("Accept", "application/json")
-
-	// TASK 1: CREATE A CANONICAL REQUEST.
-	// http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
-	canonicalURI := "/"
-	canonicalHeaders := "host:" + host + "\n"
-	signedHeaders := "host"
-	algorithm := "AWS4-HMAC-SHA256"
-	credentialScope := fmt.Sprintf("%s/%s/%s/aws4_request", datestamp, region, service)
-
-	params := req.URL.Query()
-	params.Add("Action", "GetCallerIdentity")
-	params.Add("Version", "2011-06-15")
-	params.Add("X-Amz-Algorithm", algorithm)
-	params.Add("X-Amz-Credential", resIDMatch+"/"+credentialScope)
-	params.Add("X-Amz-Date", amzDate)
-	params.Add("X-Amz-Expires", "30")
-	params.Add("X-Amz-SignedHeaders", signedHeaders)
-
-	canonicalQuerystring := params.Encode()
-	payloadHash := aws.GetHash("") // empty payload
-	canonicalRequest := method + "\n" + canonicalURI + "\n" + canonicalQuerystring + "\n" + canonicalHeaders + "\n" + signedHeaders + "\n" + payloadHash
-
-	// TASK 2: CREATE THE STRING TO SIGN.
-	stringToSign := algorithm + "\n" + amzDate + "\n" + credentialScope + "\n" + aws.GetHash(canonicalRequest)
-
-	// TASK 3: CALCULATE THE SIGNATURE.
-	// https://docs.aws.amazon.com/general/latest/gr/sigv4-calculate-signature.html
-	hash := aws.GetHMAC([]byte(fmt.Sprintf("AWS4%s", resSecretMatch)), []byte(datestamp))
-	hash = aws.GetHMAC(hash, []byte(region))
-	hash = aws.GetHMAC(hash, []byte(service))
-	hash = aws.GetHMAC(hash, []byte("aws4_request"))
-
-	signature2 := aws.GetHMAC(hash, []byte(stringToSign)) // Get Signature HMAC SHA256
-	signature := hex.EncodeToString(signature2)
-
-	// TASK 4: ADD SIGNING INFORMATION TO THE REQUEST.
-	params.Add("X-Amz-Signature", signature)
-	req.Header.Add("Content-type", "application/x-www-form-urlencoded; charset=utf-8")
-	req.URL.RawQuery = params.Encode()
-
-	client := s.verificationClient
-	if client == nil {
-		client = defaultVerificationClient
-	}
+	// Create STS client
+	stsClient := sts.NewFromConfig(cfg)


This upgrade addresses an issue where the previous version intermittently returned signature errors, incorrectly marking valid credentials as unverified.

This does not resolve the mentioned issue. I can easily replicate the intermittent behaviour by taking a valid id+secret pair and adding a secondary invalid secret.

STORAGE_ACCESS_KEY_ID=AKIA... #STORAGE_ACCESS_KEY_SECRET=... STORAGE_ACCESS_KEY_SECRET=...

Output:

$ ./trufflehog filesystem /tmp/aws.txt 🐷🔑🐷 TruffleHog. Unearth your secrets. 🐷🔑🐷 ✅ Found verified result 🐷🔑 Detector Type: AWS Decoder Type: PLAIN ... $ ./trufflehog filesystem /tmp/aws.txt 🐷🔑🐷 TruffleHog. Unearth your secrets. 🐷🔑🐷 Found unverified result 🐷🔑❓ Detector Type: AWS Decoder Type: PLAIN ...

@rgmz after some experiment, I noted that if SaneHttpClient is used to make calls, AWS returns Invalid Signature issue. But things work as expected if default AWS SDK client is being used. I tried to use the most simple go http.Client but invalid signature is appearing with that as well.

But things work as expected if default AWS SDK client is being used.

By this do you mean omitting config.WithHTTPClient(s.getClient()),?

Correct.

Yesterday, I was going through the AWS GO SDK documentation. They have provided a recommended way to customize the AWS HTTP client. Link. Few of the properties are allowed to be modified. I am trying that out.

Moreover, I noted that SaneHttpClient is modifying two parts via http.Transport layer.

Modify connection related properties like KeepAlive or maxIdleConnection etc. These can be override in AWS http client.

Customized User-Agent: SDK does not provide any interface to do that directly view Http Client. The recommended way to override the agent is using the AWS Middleware. Which is not part of the HTTP client.

From documentation perspective, Both of the modification is doable via AWS SDK. I need to try out to see the results.

… returning errors randomly when verify canary tokens. ran go mod tidy.

SDK has changed the error if the provided Id is invalid, incorporated the handling for that error as well.

passing the user-agent via middleware in aws sdk. remove the hash logic since it was making the AWS Access key test as flaky by randomly generating the incorrect values.

abmussani · 2025-02-20T15:10:51Z

Hey @rgmz, I have updated the PR and checked the issue you mentioned. The issue is no more reproducible with the changes.

What's changed:
Since, SaneHttpClient is not compatible with AWS go SDK. I used AWS BuildableClient to send the requests. Along with that Middleware is added to inject the TruffleHog + Suffix in User-Agent Header.

Input:

STORAGE_ACCESS_KEY_ID=
#STORAGE_ACCESS_KEY_SECRET=...
STORAGE_ACCESS_KEY_SECRET=...

Output - First attempt:

$ go run . filesystem ~/Desktop/Trufflehog/aws/canary/accesskey.txt 
🐷🔑🐷  TruffleHog. Unearth your secrets. 🐷🔑🐷

2025-02-20T20:02:39+05:00	info-0	trufflehog	running source	{"source_manager_worker_id": "8FFgK", "with_units": true}
✅ Found verified result 🐷🔑
Detector Type: AWS
Decoder Type: PLAIN
...

Output - Second attempt:

$ go run . filesystem ~/Desktop/Trufflehog/aws/canary/accesskey.txt 
🐷🔑🐷  TruffleHog. Unearth your secrets. 🐷🔑🐷

2025-02-20T20:05:51+05:00	info-0	trufflehog	running source	{"source_manager_worker_id": "xZmZ7", "with_units": true}
✅ Found verified result 🐷🔑
Detector Type: AWS
Decoder Type: PLAIN
...

Also, I modified the test as generating the hash was causing flaky output. Sometime, it was getting passed and sometime not.

$ go test -tags=detectors ./pkg/detectors/aws/access_keys/
ok  	github.com/trufflesecurity/trufflehog/v3/pkg/detectors/aws/access_keys	(cached)

abmussani requested review from a team as code owners February 12, 2025 14:12

kashifkhan0771 approved these changes Feb 12, 2025

View reviewed changes

zricethezav approved these changes Feb 12, 2025

View reviewed changes

abmussani requested a review from zricethezav February 14, 2025 11:11

rgmz suggested changes Feb 14, 2025

View reviewed changes

abmussani added 7 commits February 19, 2025 11:08

upgrade go-aws-sdk to v2 as the v1 is about to be deprecated and also…

f88b16b

… returning errors randomly when verify canary tokens. ran go mod tidy.

accidently removed the old version. readding old version

aa35e5d

use aws go sdk v2 for access key verification

1cd8495

remove unused param

883a591

updated comments.

c4cdcb2

use verification client for better logging.

239f965

SDK has changed the error if the provided Id is invalid, incorporated the handling for that error as well.

resolved the http client issue using with aws sdk.

b906a35

passing the user-agent via middleware in aws sdk. remove the hash logic since it was making the AWS Access key test as flaky by randomly generating the incorrect values.

abmussani force-pushed the upgrade-aws-go-sdk-v2 branch from 00558c5 to b906a35 Compare February 20, 2025 14:52

abmussani requested a review from a team as a code owner February 20, 2025 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] Upgraded AWS go-sdk to V2 (only for AWS Canary Token Verification) #3907

[Fix] Upgraded AWS go-sdk to V2 (only for AWS Canary Token Verification) #3907

abmussani commented Feb 12, 2025

kashifkhan0771 left a comment

rgmz commented Feb 12, 2025

abmussani commented Feb 12, 2025

rgmz commented Feb 12, 2025

abmussani commented Feb 12, 2025 •

edited

Loading

abmussani commented Feb 12, 2025

abmussani commented Feb 14, 2025

abmussani commented Feb 14, 2025

rgmz Feb 14, 2025

rgmz Feb 14, 2025

abmussani Feb 19, 2025

rgmz Feb 19, 2025

abmussani Feb 20, 2025

abmussani Feb 20, 2025

abmussani commented Feb 20, 2025

	return false, nil, fmt.Errorf("request returned unexpected error: %s", err.Error())
	return false, nil, fmt.Errorf("request returned unexpected error: %w", err)

[Fix] Upgraded AWS go-sdk to V2 (only for AWS Canary Token Verification) #3907

Are you sure you want to change the base?

[Fix] Upgraded AWS go-sdk to V2 (only for AWS Canary Token Verification) #3907

Conversation

abmussani commented Feb 12, 2025

Description:

Checklist:

kashifkhan0771 left a comment

Choose a reason for hiding this comment

rgmz commented Feb 12, 2025

abmussani commented Feb 12, 2025

rgmz commented Feb 12, 2025

abmussani commented Feb 12, 2025 • edited Loading

abmussani commented Feb 12, 2025

abmussani commented Feb 14, 2025

abmussani commented Feb 14, 2025

rgmz Feb 14, 2025

Choose a reason for hiding this comment

rgmz Feb 14, 2025

Choose a reason for hiding this comment

abmussani Feb 19, 2025

Choose a reason for hiding this comment

rgmz Feb 19, 2025

Choose a reason for hiding this comment

abmussani Feb 20, 2025

Choose a reason for hiding this comment

abmussani Feb 20, 2025

Choose a reason for hiding this comment

abmussani commented Feb 20, 2025

abmussani commented Feb 12, 2025 •

edited

Loading