From 1715e7ad23388a354f05828131df4d7dc7685356 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: Mon, 25 Sep 2023 15:34:13 -0400 Subject: [PATCH] updating browserstack detector to use tri-state verification (#1785) * updating browserstack detector to use tri-state verification * cleaning up nested conditions --- pkg/detectors/browserstack/browserstack.go | 65 +++++++++++++------ .../browserstack/browserstack_test.go | 64 ++++++++++++++---- 2 files changed, 96 insertions(+), 33 deletions(-) diff --git a/pkg/detectors/browserstack/browserstack.go b/pkg/detectors/browserstack/browserstack.go index 23835b91f489..54cdf1aa880d 100644 --- a/pkg/detectors/browserstack/browserstack.go +++ b/pkg/detectors/browserstack/browserstack.go @@ -2,6 +2,7 @@ package browserstack import ( "context" + "fmt" "net/http" "regexp" "strings" @@ -11,17 +12,21 @@ import ( "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" ) -type Scanner struct{} +type Scanner struct { + client *http.Client +} // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +const browserStackAPIURL = "https://www.browserstack.com/automate/plan.json" + var ( - client = common.SaneHttpClient() + defaultClient = common.SaneHttpClient() // Make sure that your group is surrounded in boundary characters such as below to reduce false positives. - keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"https://", "http://", "hub-cloud.browserstack.com", "accessKey", "\"access_Key\":", "ACCESS_KEY", "key", "browserstackKey", "BS_AUTHKEY", "BROWSERSTACK_ACCESS_KEY"}) + `\b([0-9a-zA-Z]{20})\b`) - userPat = regexp.MustCompile(detectors.PrefixRegex([]string{"https://", "http://", "hub-cloud.browserstack.com", "userName", "\"username\":", "USER_NAME", "user", "browserstackUser", "BS_USERNAME", "BROWSERSTACK_USERNAME"}) + `\b([a-zA-Z\d]{3,18}[._-]+[a-zA-Z\d]{6})\b`) + keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"hub-cloud.browserstack.com", "accessKey", "\"access_Key\":", "ACCESS_KEY", "key", "browserstackKey", "BS_AUTHKEY", "BROWSERSTACK_ACCESS_KEY"}) + `\b([0-9a-zA-Z]{20})\b`) + userPat = regexp.MustCompile(detectors.PrefixRegex([]string{"hub-cloud.browserstack.com", "userName", "\"username\":", "USER_NAME", "user", "browserstackUser", "BS_USERNAME", "BROWSERSTACK_USERNAME"}) + `\b([a-zA-Z\d]{3,18}[._-]?[a-zA-Z\d]{6,11})\b`) ) // Keywords are used for efficiently pre-filtering chunks. @@ -56,32 +61,50 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } if verify { - req, err := http.NewRequestWithContext(ctx, "GET", "https://www.browserstack.com/automate/plan.json", nil) - if err != nil { - continue - } - req.Header.Add("Content-Type", "application/json") - req.SetBasicAuth(resUserMatch, resMatch) - res, err := client.Do(req) - if err == nil { - defer res.Body.Close() - if res.StatusCode >= 200 && res.StatusCode < 300 { - s1.Verified = true - } else { - // This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key. - if detectors.IsKnownFalsePositive(resMatch, detectors.DefaultFalsePositives, true) { - continue - } - } + client := s.client + if client == nil { + client = defaultClient } + + isVerified, verificationErr := verifyBrowserStackCredentials(ctx, client, resUserMatch, resMatch) + s1.Verified = isVerified + s1.VerificationError = verificationErr } + // This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key. + if !s1.Verified && detectors.IsKnownFalsePositive(resMatch, detectors.DefaultFalsePositives, true) { + continue + } results = append(results, s1) } } + return results, nil } +func verifyBrowserStackCredentials(ctx context.Context, client *http.Client, username, accessKey string) (bool, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, browserStackAPIURL, nil) + if err != nil { + return false, err + } + req.Header.Add("Content-Type", "application/json") + req.SetBasicAuth(username, accessKey) + + res, err := client.Do(req) + if err != nil { + return false, err + } + defer res.Body.Close() + + if res.StatusCode >= 200 && res.StatusCode < 300 { + return true, nil + } else if res.StatusCode != 401 { + return false, fmt.Errorf("unexpected HTTP response status %d", res.StatusCode) + } + + return false, nil +} + func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_BrowserStack } diff --git a/pkg/detectors/browserstack/browserstack_test.go b/pkg/detectors/browserstack/browserstack_test.go index 670b8f8eeeda..e32b377f8727 100644 --- a/pkg/detectors/browserstack/browserstack_test.go +++ b/pkg/detectors/browserstack/browserstack_test.go @@ -9,7 +9,8 @@ import ( "testing" "time" - "github.com/kylelemons/godebug/pretty" + "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" @@ -33,18 +34,19 @@ func TestBrowserStack_FromChunk(t *testing.T) { verify bool } tests := []struct { - name string - s Scanner - args args - want []detectors.Result - wantErr bool + name string + s Scanner + args args + want []detectors.Result + wantErr bool + wantVerificationErr bool }{ { name: "found, verified", s: Scanner{}, args: args{ ctx: context.Background(), - data: []byte(fmt.Sprintf("You can find a browserstack secret %s within browserstack %s", secret, secretUser)), + data: []byte(fmt.Sprintf("You can find a BROWSERSTACK_ACCESS_KEY %s within BROWSERSTACK_USERNAME %s", secret, secretUser)), verify: true, }, want: []detectors.Result{ @@ -61,7 +63,7 @@ func TestBrowserStack_FromChunk(t *testing.T) { s: Scanner{}, args: args{ ctx: context.Background(), - data: []byte(fmt.Sprintf("You can find a browserstack secret %s within browserstack %s but not valid", inactiveSecret, secretUser)), // the secret would satisfy the regex but not pass validation + data: []byte(fmt.Sprintf("You can find a BROWSERSTACK_ACCESS_KEY %s within BROWSERSTACK_USERNAME %s but not valid", inactiveSecret, secretUser)), // the secret would satisfy the regex but not pass validation verify: true, }, want: []detectors.Result{ @@ -73,6 +75,42 @@ func TestBrowserStack_FromChunk(t *testing.T) { }, wantErr: false, }, + { + name: "found, would be verified if not for timeout", + s: Scanner{client: common.SaneHttpClientTimeOut(1 * time.Microsecond)}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf("You can find a BROWSERSTACK_ACCESS_KEY %s within BROWSERSTACK_USERNAME %s", secret, secretUser)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_BrowserStack, + RawV2: []byte(fmt.Sprintf("%s%s", secret, secretUser)), + Verified: false, + }, + }, + wantErr: false, + wantVerificationErr: true, + }, + { + name: "found, verified but unexpected api surface", + s: Scanner{client: common.ConstantResponseHttpClient(404, "")}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf("You can find a BROWSERSTACK_ACCESS_KEY %s within BROWSERSTACK_USERNAME %s", secret, secretUser)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_BrowserStack, + Verified: false, + RawV2: []byte(fmt.Sprintf("%s%s", secret, secretUser)), + }, + }, + wantErr: false, + wantVerificationErr: true, + }, { name: "not found", s: Scanner{}, @@ -87,8 +125,7 @@ func TestBrowserStack_FromChunk(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := Scanner{} - got, err := s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) + got, err := tt.s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) if (err != nil) != tt.wantErr { t.Errorf("BrowserStack.FromData() error = %v, wantErr %v", err, tt.wantErr) return @@ -97,9 +134,12 @@ func TestBrowserStack_FromChunk(t *testing.T) { if len(got[i].Raw) == 0 { t.Fatalf("no raw secret present: \n %+v", got[i]) } - got[i].Raw = nil + if (got[i].VerificationError != nil) != tt.wantVerificationErr { + t.Fatalf("wantVerificationError = %v, verification error = %v", tt.wantVerificationErr, got[i].VerificationError) + } } - if diff := pretty.Compare(got, tt.want); diff != "" { + ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "Raw", "VerificationError") + if diff := cmp.Diff(got, tt.want, ignoreOpts); diff != "" { t.Errorf("BrowserStack.FromData() %s diff: (-got +want)\n%s", tt.name, diff) } })