Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed sentry auth token detector #3827

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
140 changes: 78 additions & 62 deletions pkg/detectors/sentrytoken/sentrytoken.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package sentrytoken
import (
"context"
"encoding/json"
"errors"
"fmt"
regexp "github.com/wasilibs/go-re2"
"io"
"net/http"
"strings"

regexp "github.com/wasilibs/go-re2"

"github.com/trufflesecurity/trufflehog/v3/pkg/common"
"github.com/trufflesecurity/trufflehog/v3/pkg/detectors"
"github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb"
Expand All @@ -19,54 +19,65 @@ type Scanner struct {
client *http.Client
}

type Response []Organization

type Organization struct {
ID string `json:"id"`
Name string `json:"name"`
}

// Ensure the Scanner satisfies the interface at compile time.
var _ detectors.Detector = (*Scanner)(nil)

var (
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{"sentry"}) + `\b([a-f0-9]{64})\b`)
keyPat = regexp.MustCompile(`\b(sntryu_[a-f0-9]{64})\b`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still possible to find tokens with the old format. You should create a new detector version, even if it calls the same code from v1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you referring to legacy API keys? According to the documentation, the legacy authentication method using API keys is still supported. However, it states that these must be passed as Basic Auth. Does this mean the functionality was broken? 😕

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Either way, we should still detect old keys without the sntryu_ prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Either way, we should still detect old keys without the sntryu_ prefix.

Done Added new changes to version 2 ✅


errUnauthorized = fmt.Errorf("token unauthorized")
forbiddenError = "You do not have permission to perform this action."
)

func (s Scanner) getClient() *http.Client {
if s.client != nil {
return s.client
}

return defaultClient
}

// Keywords are used for efficiently pre-filtering chunks.
// Use identifiers in the secret preferably, or the provider name.
func (s Scanner) Keywords() []string {
return []string{"sentry"}
return []string{"sentry", "sntryu"}
}

// FromData will find and optionally verify SentryToken secrets in a given set of bytes.
func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (results []detectors.Result, err error) {
dataStr := string(data)

matches := keyPat.FindAllStringSubmatch(dataStr, -1)
// find all unique auth tokens
var uniqueAuthTokens = make(map[string]struct{})

for _, match := range matches {
if len(match) != 2 {
continue
}
resMatch := strings.TrimSpace(match[1])
for _, authToken := range keyPat.FindAllStringSubmatch(dataStr, -1) {
uniqueAuthTokens[authToken[1]] = struct{}{}
}

for authToken := range uniqueAuthTokens {
s1 := detectors.Result{
DetectorType: detectorspb.DetectorType_SentryToken,
Raw: []byte(resMatch),
Raw: []byte(authToken),
ExtraData: make(map[string]string),
kashifkhan0771 marked this conversation as resolved.
Show resolved Hide resolved
}

if verify {
client := s.client
if client == nil {
client = defaultClient
}
isVerified, verificationErr := verifyToken(ctx, client, resMatch)

switch {
case errors.Is(verificationErr, errUnauthorized):
s1.Verified = false
case isVerified:
s1.Verified = true
default:
s1.SetVerificationError(verificationErr, resMatch)
client := s.getClient()
kashifkhan0771 marked this conversation as resolved.
Show resolved Hide resolved
extraData, isVerified, verificationErr := verifyToken(ctx, client, authToken)
s1.Verified = isVerified
s1.SetVerificationError(verificationErr, authToken)

for key, value := range extraData {
s1.ExtraData[key] = value
}
}

Expand All @@ -76,54 +87,59 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result
return results, nil
}

type Response []Project

type Project struct {
Organization Organization `json:"organization"`
}

type Organization struct {
ID string `json:"id"`
Name string `json:"name"`
}

func verifyToken(ctx context.Context, client *http.Client, token string) (bool, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://sentry.io/api/0/projects/", nil)
func verifyToken(ctx context.Context, client *http.Client, token string) (map[string]string, bool, error) {
// api docs: https://docs.sentry.io/api/organizations/
// this api will return 200 for user auth tokens with scope of org:<>
req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://sentry.io/api/0/organizations/", nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/projects API require project:<> scope and /organizations API require org:<> scope. I updated to organization API because we capture the organizations from response for extraData.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How common is it for a token to have org: scope versus project: scope?

if err != nil {
return false, err
return nil, false, err
}
req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token))

res, err := client.Do(req)
resp, err := client.Do(req)
if err != nil {
return false, err
return nil, false, err
}
defer res.Body.Close()
defer func() {
_, _ = io.Copy(io.Discard, resp.Body)
_ = resp.Body.Close()
}()

var isVerified bool
switch res.StatusCode {
case http.StatusOK, http.StatusForbidden:
isVerified = true
case http.StatusUnauthorized:
return false, errUnauthorized
default:
return false, fmt.Errorf("unexpected HTTP response status %d", res.StatusCode)
bytes, err := io.ReadAll(resp.Body)
if err != nil {
return nil, false, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary, you can read the body directly with json.NewDecoder(res.Body).Decode(...) instead of json.Unmarshal(...).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How common is it for a token to have org: scope versus project: scope?

It depends on the task the user is creating the token for. When generating a token, users have the option to add any scope they need, with at least one scope being mandatory. The reason for using the organization API in this case is that we're trying to retrieve some organization-level data for extraData.

}

bytes, readErr := io.ReadAll(res.Body)
if readErr != nil {
return false, readErr
}
switch resp.StatusCode {
case http.StatusOK:
var resp Response
if err = json.Unmarshal(bytes, &resp); err != nil {
return nil, false, err
}

var resp Response
if err = json.Unmarshal(bytes, &resp); err != nil {
return false, err
}
if len(resp) == 0 {
return false, fmt.Errorf("unexpected response body: %s", string(bytes))
}
var extraData = make(map[string]string)
for _, org := range resp {
extraData[fmt.Sprintf("orginzation_%s", org.ID)] = org.Name
}

return extraData, true, nil
case http.StatusForbidden:
var responseBody interface{}
if err := json.Unmarshal(bytes, &responseBody); err != nil {
return nil, false, err
}

return isVerified, err
// if response contain the forbiddenError message it means the token is active but does not have the right scope for this API call
if strings.Contains(fmt.Sprintf("%v", responseBody), forbiddenError) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case the token is Active and does not have the org:<> scope the API returns 403 with a specific error message. In case token is removed the API return 401

return nil, true, nil
}

return nil, false, nil
case http.StatusUnauthorized:
return nil, false, nil
default:
return nil, false, fmt.Errorf("unexpected HTTP response status %d", resp.StatusCode)
}
}

func (s Scanner) Type() detectorspb.DetectorType {
Expand Down
56 changes: 5 additions & 51 deletions pkg/detectors/sentrytoken/sentrytoken_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
func TestSentryToken_FromChunk(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
defer cancel()
testSecrets, err := common.GetSecret(ctx, "trufflehog-testing", "detectors3")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detectors3 vault limit is full

testSecrets, err := common.GetSecret(ctx, "trufflehog-testing", "detectors5")
if err != nil {
t.Fatalf("could not get test secrets from GCP: %s", err)
}
Expand Down Expand Up @@ -52,6 +52,7 @@ func TestSentryToken_FromChunk(t *testing.T) {
{
DetectorType: detectorspb.DetectorType_SentryToken,
Verified: true,
ExtraData: map[string]string{"orginzation_4508567357947904": "Truffle Security"},
},
},
wantErr: false,
Expand All @@ -68,6 +69,7 @@ func TestSentryToken_FromChunk(t *testing.T) {
{
DetectorType: detectorspb.DetectorType_SentryToken,
Verified: false,
ExtraData: map[string]string{},
},
},
wantErr: false,
Expand All @@ -84,6 +86,7 @@ func TestSentryToken_FromChunk(t *testing.T) {
{
DetectorType: detectorspb.DetectorType_SentryToken,
Verified: false,
ExtraData: map[string]string{},
},
},
wantErr: false,
Expand All @@ -101,56 +104,7 @@ func TestSentryToken_FromChunk(t *testing.T) {
{
DetectorType: detectorspb.DetectorType_SentryToken,
Verified: false,
},
},
wantErr: false,
wantVerificationErr: true,
},
{
name: "found, good key but wrong scope",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed some tests which are not necessary.

s: Scanner{client: common.ConstantResponseHttpClient(403, responseBody403)},
args: args{
ctx: context.Background(),
data: []byte(fmt.Sprintf("You can find a sentry super secret %s within", secret)),
verify: true,
},
want: []detectors.Result{
{
DetectorType: detectorspb.DetectorType_SentryToken,
Verified: true,
},
},
wantErr: false,
},
{
name: "found, account deactivated",
s: Scanner{client: common.ConstantResponseHttpClient(200, responseAccountDeactivated)},
args: args{
ctx: context.Background(),
data: []byte(fmt.Sprintf("You can find a sentry super secret %s within", secret)),
verify: true,
},
want: []detectors.Result{
{
DetectorType: detectorspb.DetectorType_SentryToken,
Verified: false,
},
},
wantErr: false,
wantVerificationErr: true,
},
{
name: "found, account deactivated",
s: Scanner{client: common.ConstantResponseHttpClient(200, responseEmpty)},
args: args{
ctx: context.Background(),
data: []byte(fmt.Sprintf("You can find a sentry super secret %s within", secret)),
verify: true,
},
want: []detectors.Result{
{
DetectorType: detectorspb.DetectorType_SentryToken,
Verified: false,
ExtraData: map[string]string{},
},
},
wantErr: false,
Expand Down
21 changes: 10 additions & 11 deletions pkg/detectors/sentrytoken/sentrytoken_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ import (
)

var (
validPattern = "ad00eba0e2b5b057146e1b2b9373f86dbb0e712d106529111d97cb13f081de20"
invalidPattern = "ad00e?a0e2b5b057146e1b2b9373f86dbb0e712d106529111d97cb13f081de20"
keyword = "sentrytoken"
validPattern = `
sentry_token := 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
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", sentry_token))
`
invalidPattern = "sntryu_822255ea24285a3f8f863dee3g1720fff21628cf331fbde22a16f27bef9cd7a6"
keyword = "sentry"
token = "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"
)

func TestSentryToken_Pattern(t *testing.T) {
Expand All @@ -27,18 +31,13 @@ func TestSentryToken_Pattern(t *testing.T) {
}{
{
name: "valid pattern - with keyword sentrytoken",
input: fmt.Sprintf("%s token = '%s'", keyword, validPattern),
want: []string{validPattern},
input: validPattern,
want: []string{token},
},
{
name: "valid pattern - ignore duplicate",
input: fmt.Sprintf("%s token = '%s' | '%s'", keyword, validPattern, validPattern),
want: []string{validPattern},
},
{
name: "valid pattern - key out of prefix range",
input: fmt.Sprintf("%s keyword is not close to the real key in the data\n = '%s'", keyword, validPattern),
want: []string{},
want: []string{token},
},
{
name: "invalid pattern",
Expand Down
2 changes: 1 addition & 1 deletion pkg/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func TestEngine_DuplicateSecrets(t *testing.T) {

// Wait for all the chunks to be processed.
assert.Nil(t, e.Finish(ctx))
want := uint64(5)
want := uint64(1)
assert.Equal(t, want, e.GetMetrics().UnverifiedSecretsFound)
}

Expand Down
Loading