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

Detector-Competition-New - Dockerhub old format detector #1863

Closed
wants to merge 2 commits into from

Conversation

ankushgoel27
Copy link
Contributor

@ankushgoel27 ankushgoel27 commented Oct 5, 2023

Description:

This PR adds the older format of dockerhub token which is still valid and i have seen it in the wild.

Wanted to add 'detectors.PrefixRegex([]string{"docker"}) +' to the regex but was getting very weird and unexpected matches from string docker to the end of the token. see, if you can fix it.

Reference :
https://www.docker.com/blog/docker-hub-new-personal-access-tokens/

Checklist:

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

@ankushgoel27 ankushgoel27 requested a review from a team as a code owner October 5, 2023 08:48
@ankushgoel27 ankushgoel27 changed the title Dockerhub old format detector Detector-Competition-New - Dockerhub old format detector Oct 5, 2023
@ankushgoel27
Copy link
Contributor Author

dockerhub

emailPat = regexp.MustCompile(common.EmailPattern)

// Can use password or personal access token (PAT) for login, but this scanner will only check for PATs.
accessTokenPat = regexp.MustCompile(`\bdckr_pat_([a-zA-Z0-9_-]){27}\b`)
accessTokenPat = regexp.MustCompile(`\b([0-9Aa-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\b`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ankushgoel27 Since this regular expression matches a common key pattern, could we update this to use the prefixregex helper which adds keyword locality to the resulting regex?

accessTokenPat = regexp.MustCompile(detectors.PrefixRegex([]string{"docker"}) + `\b([0-9Aa-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\b`)

Copy link
Contributor Author

@ankushgoel27 ankushgoel27 Oct 5, 2023

Choose a reason for hiding this comment

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

i tried the above code you mentioned but the regex detector is behaving very unusually then.
if i use - accessTokenPat = regexp.MustCompile(detectors.PrefixRegex([]string{"docker"}) + \b([0-9Aa-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\b)

then the regex match on "docker_token = {token}" and not just the {token}. Very weird. it should just match on the token but it matches on everything where it sees the first occurrence of docker word

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wouldnt chunk filtering help here also. When i tried running trufflehog on a test file. only dockerhub detector ran and no other detector was triggered even being a general token regex

@zricethezav zricethezav added the Hacktoberfest-Detector-Competition-New Apply this label if you are adding a new detector for the detector competition label Oct 5, 2023
@zricethezav
Copy link
Collaborator

@ankushgoel27 To be considered for this competition, this PR needs the following missing items:

  1. A screenshot of the tests passing (you provided a screenshot of the secret being found in a test file).
  2. Cite official documentation or recognized community resources to justify the regex patterns used.

@ankushgoel27
Copy link
Contributor Author

ankushgoel27 commented Oct 5, 2023

I copied the same test detector for the original dockerhub detector that was there.
Running the test fail -

go test ./pkg/detectors/dockerhub -tags=detectors
--- FAIL: TestDockerhub_FromChunk (1.58s)
--- FAIL: TestDockerhub_FromChunk/found,verified(email) (0.25s)
dockerhub_test.go:119: Dockerhub.FromData() found, verified (email) diff: (-got +want)
[
{
DetectorType: 921,
DetectorName: "",
DecoderType: 0,
- Verified: false,
+ Verified: true,
Raw: [
],
RawV2: [
],
Redacted: "",
ExtraData: {
},
StructuredData: nil,
VerificationError: ,
},
]
FAIL
FAIL github.com/trufflesecurity/trufflehog/v3/pkg/detectors/dockerhub 2.030s
FAIL

@zricethezav
Copy link
Collaborator

@ankushgoel27
Copy link
Contributor Author

@ankushgoel27 did you follow the testing instructions? https://github.com/trufflesecurity/trufflehog/blob/main/hack/docs/Adding_Detectors_external.md#testing-the-detector

yes, these tests don't seem to pass for me

go test ./pkg/detectors/dockerhub -tags=detectors

github.com/trufflesecurity/trufflehog/v3/pkg/detectors/dockerhub [github.com/trufflesecurity/trufflehog/v3/pkg/detectors/dockerhub.test]

pkg/detectors/dockerhub/dockerhub_test.go:94:18: unknown field client in struct literal of type Scanner
pkg/detectors/dockerhub/dockerhub_test.go:112:18: unknown field client in struct literal of type Scanner
FAIL github.com/trufflesecurity/trufflehog/v3/pkg/detectors/dockerhub [build failed]
FAIL

@zricethezav
Copy link
Collaborator

@ankushgoel27

pkg/detectors/dockerhub/dockerhub_test.go:94:18: unknown field client in struct literal of type Scanner

The errors you commented don't match up with what is present in this PR. I'm getting failed tests but not those errors. The errors in the comments are likely coming from the fact that you aren't mocking a http client in the Scanner struct. You're tests likely had something that looked like this

s: Scanner{client: common.ConstantResponseHttpClient(404, "")},

s: Scanner{client: common.ConstantResponseHttpClient(404, "")},,
as it's provided to you when you generate new detectors and detector tests.

We can provide a client since the Scanner type should be defined as:

type Scanner struct {
	client *http.Client
}

What I would do is make sure you aren't getting any go warnings or errors and following the testing instructions again.

i tried the above code you mentioned but the regex detector is behaving very unusually then.
if i use - accessTokenPat = regexp.MustCompile(detectors.PrefixRegex([]string{"docker"}) + \b([0-9Aa-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\b) then the regex match on "docker_token = {token}" and not just the {token}. Very weird. it should just match on the token but it matches on everything where it sees the first occurrence of docker word

This is because you need to extract the capture group from the match. accessTokenMatches := accessTokenPat.FindAllString(dataStr, -1) will just match the regex without giving you any information on the subgroups. You'll need to update this to use FindAllStringSubmatch so you can extract the capture group.

	accessTokenMatches := accessTokenPat.FindAllStringSubmatch(dataStr, -1)
	accessTokenMatchesAll := accessTokenPat.FindAllString(dataStr, -1)
	fmt.Println("accessTokenMatches: ", accessTokenMatches[0][1])
	fmt.Println("accessTokenMatchesAll: ", accessTokenMatchesAll)
	
	# prints out this
	accessTokenMatches:  11111111-1111-1111-1111-111111111111
        accessTokenMatchesAll:  [docker login -u  -p 11111111-1111-1111-1111-111111111111]

@zricethezav
Copy link
Collaborator

Closing this PR as it's been open for 2 weeks without any activity. Feel free to reopen once comments have been addressed.

@ankushgoel27
Copy link
Contributor Author

can you please reopen the PR? i have made most the changes recomended by you but still having issue with the tests. need help

@ankushgoel27
Copy link
Contributor Author

i am seeing this errors with the new changes -

--- FAIL: TestDockerhub_FromChunk (2.38s)
--- FAIL: TestDockerhub_FromChunk/found,_would_be_verified_if_not_for_timeout (0.41s)
dockerhub_test.go:137: wantVerificationError = true, verification error =
--- FAIL: TestDockerhub_FromChunk/found,_verified_but_unexpected_api_surface (0.39s)
dockerhub_test.go:137: wantVerificationError = true, verification error =
FAIL
FAIL github.com/trufflesecurity/trufflehog/v3/pkg/detectors/dockerhub 2.408s
FAIL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest-Detector-Competition-New Apply this label if you are adding a new detector for the detector competition
Development

Successfully merging this pull request may close these issues.

2 participants