From 5e3ea1a8f2194f2c1f5448d69e7cf43bb560fc6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=81h=CC=95=CC=B3m=CD=AD=CD=AD=CD=A8=CD=A9=CC=90e=CD=AC?= =?UTF-8?q?=CC=81=CD=8B=CD=AC=CC=8A=CC=93=CD=82=CC=98d?= <13666360+0x1@users.noreply.github.com> Date: Tue, 12 Dec 2023 16:07:09 -0500 Subject: [PATCH] Fix azurestorage detector (#2207) * bugfix + cleanup - update azurestorage detector raw string to use key instead of id --- pkg/detectors/azurestorage/azurestorage.go | 92 +++++++++++-------- .../azurestorage/azurestorage_test.go | 92 ++++++++++++++----- 2 files changed, 121 insertions(+), 63 deletions(-) diff --git a/pkg/detectors/azurestorage/azurestorage.go b/pkg/detectors/azurestorage/azurestorage.go index 4954208b4ae2..b43be3d18c5a 100644 --- a/pkg/detectors/azurestorage/azurestorage.go +++ b/pkg/detectors/azurestorage/azurestorage.go @@ -30,6 +30,13 @@ func (s Scanner) Keywords() []string { return []string{"DefaultEndpointsProtocol=https;AccountName="} } +func (s Scanner) getClient() *http.Client { + if s.client != nil { + return s.client + } + return defaultClient +} + func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (results []detectors.Result, err error) { dataStr := string(data) @@ -44,47 +51,18 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result s1 := detectors.Result{ DetectorType: detectorspb.DetectorType_AzureStorage, - Raw: []byte(accountName), + Raw: []byte(accountKey), + ExtraData: map[string]string{ + "account_name": accountName, + }, } if verify { - client := s.client - if client == nil { - client = defaultClient - } - - now := time.Now().UTC().Format(http.TimeFormat) - stringToSign := "GET\n\n\n\n\n\n\n\n\n\n\n\nx-ms-date:" + now + "\nx-ms-version:2019-12-12\n/" + accountName + "/\ncomp:list" - accountKeyBytes, _ := base64.StdEncoding.DecodeString(accountKey) - h := hmac.New(sha256.New, accountKeyBytes) - h.Write([]byte(stringToSign)) - signature := base64.StdEncoding.EncodeToString(h.Sum(nil)) - - url := "https://" + accountName + ".blob.core.windows.net/?comp=list" - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) - if err != nil { - continue - } - req.Header.Set("x-ms-date", now) - req.Header.Set("x-ms-version", "2019-12-12") - req.Header.Set("Authorization", "SharedKey "+accountName+":"+signature) - - if err != nil { - continue - } - res, err := client.Do(req) - if err == nil { - defer res.Body.Close() - if res.StatusCode >= 200 && res.StatusCode < 300 { - s1.Verified = true - } else if res.StatusCode == 403 { - } else { - err = fmt.Errorf("unexpected HTTP response status %d", res.StatusCode) - s1.SetVerificationError(err, accountKey) - } - } else { - s1.SetVerificationError(err, accountKey) - } + client := s.getClient() + + isVerified, verificationErr := verifyAzureStorageKey(ctx, client, accountName, accountKey) + s1.Verified = isVerified + s1.SetVerificationError(verificationErr, accountKey) } results = append(results, s1) @@ -93,6 +71,44 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result return results, nil } +func verifyAzureStorageKey(ctx context.Context, client *http.Client, accountName, accountKey string) (bool, error) { + now := time.Now().UTC().Format(http.TimeFormat) + stringToSign := "GET\n\n\n\n\n\n\n\n\n\n\n\nx-ms-date:" + now + "\nx-ms-version:2019-12-12\n/" + accountName + "/\ncomp:list" + accountKeyBytes, _ := base64.StdEncoding.DecodeString(accountKey) + h := hmac.New(sha256.New, accountKeyBytes) + h.Write([]byte(stringToSign)) + signature := base64.StdEncoding.EncodeToString(h.Sum(nil)) + + url := "https://" + accountName + ".blob.core.windows.net/?comp=list" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return false, err + } + req.Header.Set("x-ms-date", now) + req.Header.Set("x-ms-version", "2019-12-12") + req.Header.Set("Authorization", "SharedKey "+accountName+":"+signature) + + res, err := client.Do(req) + if err != nil { + // If the host is not found, we can assume that the accountName is not valid + if strings.Contains(err.Error(), "no such host") { + return false, nil + } + return false, err + } + defer res.Body.Close() + + switch res.StatusCode { + case http.StatusOK: + return true, nil + case http.StatusForbidden: + // 403 if account id or key is invalid, or if the account is disabled + return false, nil + default: + return false, fmt.Errorf("unexpected HTTP response status %d", res.StatusCode) + } +} + func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_AzureStorage } diff --git a/pkg/detectors/azurestorage/azurestorage_test.go b/pkg/detectors/azurestorage/azurestorage_test.go index 0b3c537b4424..b48844266cc4 100644 --- a/pkg/detectors/azurestorage/azurestorage_test.go +++ b/pkg/detectors/azurestorage/azurestorage_test.go @@ -6,15 +6,16 @@ package azurestorage import ( "context" "fmt" + "regexp" + "strings" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" - "github.com/trufflesecurity/trufflehog/v3/pkg/common" + "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" ) @@ -28,18 +29,21 @@ func TestAzurestorage_FromChunk(t *testing.T) { secret := testSecrets.MustGetField("AZURE_STORAGE") inactiveSecret := testSecrets.MustGetField("AZURE_STORAGE_INACTIVE") + accountNamePat := regexp.MustCompile(`AccountName=(?P[^;]+);AccountKey`) + accountName := accountNamePat.FindStringSubmatch(secret)[1] + validKeyInvalidAccountName := strings.Replace(secret, accountName, "invalid", 1) + type args struct { ctx context.Context data []byte verify bool } tests := []struct { - name string - s Scanner - args args - want []detectors.Result - wantErr bool - wantVerificationErr bool + name string + s Scanner + args args + want []detectors.Result + wantErr bool }{ { name: "found, verified", @@ -53,10 +57,12 @@ func TestAzurestorage_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_AzureStorage, Verified: true, + ExtraData: map[string]string{ + "account_name": "teststoragebytruffle", + }, }, }, - wantErr: false, - wantVerificationErr: false, + wantErr: false, }, { name: "found, unverified", @@ -70,10 +76,12 @@ func TestAzurestorage_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_AzureStorage, Verified: false, + ExtraData: map[string]string{ + "account_name": "teststoragebytruffle", + }, }, }, - wantErr: false, - wantVerificationErr: false, + wantErr: false, }, { name: "not found", @@ -83,9 +91,8 @@ func TestAzurestorage_FromChunk(t *testing.T) { data: []byte("You cannot find the secret within"), verify: true, }, - want: nil, - wantErr: false, - wantVerificationErr: false, + want: nil, + wantErr: false, }, { name: "found, would be verified if not for timeout", @@ -95,14 +102,18 @@ func TestAzurestorage_FromChunk(t *testing.T) { data: []byte(fmt.Sprintf("You can find a azurestorage secret %s within", secret)), verify: true, }, - want: []detectors.Result{ - { + want: func() []detectors.Result { + r := detectors.Result{ DetectorType: detectorspb.DetectorType_AzureStorage, Verified: false, - }, - }, - wantErr: false, - wantVerificationErr: true, + ExtraData: map[string]string{ + "account_name": "teststoragebytruffle", + }, + } + r.SetVerificationError(fmt.Errorf("context deadline exceeded"), secret) + return []detectors.Result{r} + }(), + wantErr: false, }, { name: "found, verified but unexpected api surface", @@ -112,14 +123,37 @@ func TestAzurestorage_FromChunk(t *testing.T) { data: []byte(fmt.Sprintf("You can find a azurestorage secret %s within", secret)), verify: true, }, + want: func() []detectors.Result { + r := detectors.Result{ + DetectorType: detectorspb.DetectorType_AzureStorage, + Verified: false, + ExtraData: map[string]string{ + "account_name": "teststoragebytruffle", + }, + } + r.SetVerificationError(fmt.Errorf("unexpected HTTP response status 404"), secret) + return []detectors.Result{r} + }(), + wantErr: false, + }, + { + name: "found secret with invalid account name", + s: Scanner{}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf("You can find a azurestorage secret %s within", validKeyInvalidAccountName)), + verify: true, + }, want: []detectors.Result{ { DetectorType: detectorspb.DetectorType_AzureStorage, Verified: false, + ExtraData: map[string]string{ + "account_name": "invalid", + }, }, }, - wantErr: false, - wantVerificationErr: true, + wantErr: false, }, } for _, tt := range tests { @@ -133,8 +167,16 @@ func TestAzurestorage_FromChunk(t *testing.T) { if len(got[i].Raw) == 0 { t.Fatalf("no raw secret present: \n %+v", got[i]) } - if (got[i].VerificationError() != nil) != tt.wantVerificationErr { - t.Fatalf("wantVerificationError = %v, verification error = %v", tt.wantVerificationErr, got[i].VerificationError()) + gotErr := "" + if got[i].VerificationError() != nil { + gotErr = got[i].VerificationError().Error() + } + wantErr := "" + if tt.want[i].VerificationError() != nil { + wantErr = tt.want[i].VerificationError().Error() + } + if gotErr != wantErr { + t.Fatalf("wantVerificationError = %v, verification error = %v", tt.want[i].VerificationError(), got[i].VerificationError()) } } ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "Raw", "verificationError")