Add generic JWT detection and verification

[bradlarsen]

Description:

This adds a generic detector and verifier for generic JWTs.

Detection is simple using regular expressions. However, this will produce many false positives. JWT verification is performed using the github.com/golang-jwt/jwt/v5 package, performing the usual checks (proper decoding, checking timestamp-related claims, checking for an allow-list of supported algorithms, etc). Only public key cryptography algorithms are supported. Additionally, OIDC Discovery is attempted against the issuer to fetch the public key for signature verification.

Testing:

A few unit tests demonstrate the detector working. Further testing of verification is needed.

TODO:

• thoroughly test verification
• add integration tests
• think about and write up the security implications of using OIDC Discovery with an attacker-controlled URL
• estimate the impact on increased finding volume when verification is disabled

[bradlarsen]

Note: the custom detector test failed in the test-community job above:

--- FAIL: TestDetectorValidations (0.00s)
    --- FAIL: TestDetectorValidations/custom_validation_-_multiple_regex_validations (0.00s)
        custom_detectors_test.go:556: CustomDetector.FromData() custom validation - multiple regex validations diff: (-got +want)
              []detectors.Result{
              	{
              		... // 2 identical fields
              		Verified:              false,
              		VerificationFromCache: false,
              		Raw: bytes.Join({
            + 			"MyStrongP@ssword",
              			"c392c9837d69b44c764cbf260b-e6184",
            - 			"MyStrongP@ssword",
              		}, ""),
              		RawV2:    nil,
              		Redacted: "",
              		... // 3 ignored and 2 identical fields
              	},
              }
FAIL
FAIL	github.com/trufflesecurity/trufflehog/v3/pkg/custom_detectors	0.043s

I have seen this sporadic test failure a few times now. I'm pretty sure it's caused by iterating over a map (the order of which is unspecified in Go) in the custom detector code:

https://github.com/bradlarsen/trufflehog/blob/1e8671c6de6d721636e672b3066c6773e4ab6d0b/pkg/custom_detectors/custom_detectors.go#L118

[bradlarsen]

I have seen this sporadic test failure a few times now. I'm pretty sure it's caused by iterating over a map (the order of which is unspecified in Go) in the custom detector code

Fixed and merged in #4446.

[bradlarsen]

A TODO item from the description:

estimate the impact on increased finding volume when verification is disabled

I ran with this new JWT detection over 250k recent commits from GitHub. From there, about 4k distinct JWTs were found, and several dozen of them verified completely.

The newly detected JWTs will be notable, but less volume than, say, Github V1.

[joeleonjr]

@bradlarsen given clarification on the unknown status in #4477, I think usage of that status in the JWT detector should be removed. I think it's worth just commenting out those code paths (instead of fully deleting), since they would be useful in the future if we introduce any additional statuses. Your call. And if you have any other ideas here, I'm sure the team would be open to discussing.

[shahzadhaider1]

Good work.
Looks good, left one minor comment.

[rosecodym]

I have a question about some verification errors this returns.

[rosecodym]

I'm generally opposed to functions that signify they can return things they can't actually return (an error in this case), because I think it usually adds unnecessary cognitive overhead during future maintenance. (I'm not requesting a change - I'm just flagging this because it looks like the elimination of any actually returned error might have happened in the course of writing this function rather than as your initial plan.)

[bradlarsen]

@rosecodym thanks for pointing this out! I agree with you. And yeah, you're right, this code changed during implementation in response to comments that clarified indeterminate verification status.

In a previous commit, HMAC-signed JWTs whose other claims (timestamps, etc) checked out would be reported as indeterminate. But I later learned that that type is reserved today for essentially transient / network-related errors. So I changed the verifyHMAC function.

It's essentially dead code today, and perhaps should be ripped out entirely, to be added back in one day in the glorious future when we can more comfortably handle secrets that are probably live but not feasibly tested.

[rosecodym]

Every single returned error here is going to cause an indeterminate result. Is that correct? The "verification error" state is designed to indicate that we couldn't verify a candidate finding because of uncontrollable external conditions, and some of these errors don't look like that.

[bradlarsen]

@rosecodym Yeah, you're right to remark on this. Thanks for the extra set of eyes.

JWTs are a tricky thing to do liveness checking on — different from just about all our other detectors. They are "stateless", and verification is done locally. (Once you get past the OIDC Discovery process at least.)

The oidcDiscoveryKeyFunc does return many error values for non-transient things, like when an issuer URL is missing or malformed. In these cases, indeed, we probably don't want to propagate the errors all the way to be indeterminate results at the finding level. I'll rework that.

I will point out for posterity that the current set of statuses (verified, unverified, and indeterminate due to external conditions) doesn't have a way of representing secrets that may be live but don't have a definitive way of doing verification.

For example: the current code will produce an indeterminate result for a JWT finding whose issuer comes from a non-routing host (localhost, internal IP, etc). These could in theory be checked for liveness, but the verification procedure would have to be performed from the appropriate location in the network. There is not a suitable status to use for this today.