Skip to content

Commit e61ac5f

Browse files
bradlarsenbradlarsen
committed
Fix JWT signature verification; fix tests; clean up detector code
1 parent 1335ce0 commit e61ac5f

File tree

2 files changed

+60
-52
lines changed

2 files changed

+60
-52
lines changed

‎pkg/detectors/jwt/jwt.go‎

Lines changed: 52 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net"
1010
"net/http"
1111
"net/url"
12+
"strings"
1213
"time"
1314

1415
"github.com/golang-jwt/jwt/v5"
@@ -52,38 +53,39 @@ func (s Scanner) Keywords() []string {
5253
}
5354
}
5455

56+
var jwtOptions = []jwt.ParserOption{
57+
jwt.WithValidMethods([]string{
58+
// HMAC-based algorithms
59+
// jwt.SigningMethodHS256.Alg(),
60+
// jwt.SigningMethodHS384.Alg(),
61+
// jwt.SigningMethodHS512.Alg(),
62+
63+
// Public key-based algorithms
64+
jwt.SigningMethodRS256.Alg(),
65+
jwt.SigningMethodRS384.Alg(),
66+
jwt.SigningMethodRS512.Alg(),
67+
jwt.SigningMethodEdDSA.Alg(),
68+
jwt.SigningMethodES256.Alg(),
69+
jwt.SigningMethodES384.Alg(),
70+
jwt.SigningMethodES512.Alg(),
71+
jwt.SigningMethodPS256.Alg(),
72+
jwt.SigningMethodPS384.Alg(),
73+
jwt.SigningMethodPS512.Alg(),
74+
}),
75+
jwt.WithIssuedAt(),
76+
jwt.WithPaddingAllowed(),
77+
jwt.WithLeeway(time.Minute),
78+
}
79+
80+
var jwtParser = jwt.NewParser(jwtOptions...)
81+
82+
var jwtValidator = jwt.NewValidator(jwtOptions...)
83+
5584
// FromData will find and optionally verify JWT secrets in a given set of bytes.
5685
func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (results []detectors.Result, err error) {
5786
client := cmp.Or(s.client, common.SaneHttpClient())
5887
seenMatches := make(map[string]struct{})
5988

60-
var validator *jwt.Validator = nil
61-
if verify {
62-
validator = jwt.NewValidator(
63-
jwt.WithValidMethods([]string{
64-
// HMAC-based algorithms
65-
jwt.SigningMethodHS256.Alg(),
66-
jwt.SigningMethodHS384.Alg(),
67-
jwt.SigningMethodHS512.Alg(),
68-
69-
// Public key-based algorithms
70-
jwt.SigningMethodRS256.Alg(),
71-
jwt.SigningMethodRS384.Alg(),
72-
jwt.SigningMethodRS512.Alg(),
73-
jwt.SigningMethodEdDSA.Alg(),
74-
jwt.SigningMethodES256.Alg(),
75-
jwt.SigningMethodES384.Alg(),
76-
jwt.SigningMethodES512.Alg(),
77-
jwt.SigningMethodPS256.Alg(),
78-
jwt.SigningMethodPS384.Alg(),
79-
jwt.SigningMethodPS512.Alg(),
80-
}),
81-
jwt.WithIssuedAt(),
82-
jwt.WithPaddingAllowed(),
83-
jwt.WithLeeway(time.Minute),
84-
)
85-
}
86-
8789
for _, matchGroups := range keyPat.FindAllStringSubmatch(string(data), -1) {
8890
match := matchGroups[1]
8991

@@ -93,12 +95,28 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result
9395
seenMatches[match] = struct{}{}
9496

9597
claims := jwt.MapClaims{}
96-
parsedToken, _, err := jwt.NewParser(jwt.WithPaddingAllowed()).ParseUnverified(match, claims)
97-
if err != nil {
98+
parsedToken, tokenParts, err := jwtParser.ParseUnverified(match, claims)
99+
if err != nil || len(tokenParts) != 3 {
98100
// skip malformed tokens; no need to do claims validation or signature verification
99101
continue
100102
}
101103

104+
switch parsedToken.Method.Alg() {
105+
case "HS256", "HS384", "HS512":
106+
// The JWT *might* be valid, but we can't in general do signature verification on HMAC-based algorithms.
107+
// We don't have a suitable status to represent this situation in trufflehog.
108+
// (The `unknown` status is intended to indicate that an error occurred to to external environment conditions, like trannsient network errors.)
109+
// So instead, to avoid possible false positives, totally skip HMAC-based JWTs; don't even create results for them.
110+
continue
111+
}
112+
113+
// Decode signature
114+
parsedToken.Signature, err = jwtParser.DecodeSegment(tokenParts[2])
115+
if err != nil {
116+
// skip JWTs with malformed signatures
117+
continue
118+
}
119+
102120
issString, _ := claims.GetIssuer()
103121

104122
iatString := ""
@@ -127,7 +145,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result
127145
}
128146

129147
if verify {
130-
isVerified, verificationErr := verifyJWT(ctx, client, validator, parsedToken)
148+
isVerified, verificationErr := verifyJWT(ctx, client, tokenParts, parsedToken)
131149
s1.Verified = isVerified
132150
s1.SetVerificationError(verificationErr, match)
133151
}
@@ -194,21 +212,12 @@ func limitReader(reader io.Reader) io.Reader {
194212
//
195213
// - If the JWT uses public key cryptography and the OIDC Discovery protocol, we can fetch the public key and perform signature verification
196214
// - In all cases, we can perform claims validation (e.g., checking expiration time) and sometimes get a definite answer that a JWT is *not* live
197-
func verifyJWT(ctx context.Context, client *http.Client, validator *jwt.Validator, parsedToken *jwt.Token) (bool, error) {
198-
if err := validator.Validate(parsedToken.Claims); err != nil {
215+
func verifyJWT(ctx context.Context, client *http.Client, tokenParts []string, parsedToken *jwt.Token) (bool, error) {
216+
if err := jwtValidator.Validate(parsedToken.Claims); err != nil {
199217
// though we have not checked the signature, the token is definitely invalid
200218
return false, nil
201219
}
202220

203-
switch parsedToken.Method.Alg() {
204-
case "HS256", "HS384", "HS512":
205-
// The JWT *might* be valid, but we can't in general do signature verification on HMAC-based algorithms.
206-
// We don't have a suitable status to represent this situation in trufflehog.
207-
// (The `unknown` status is intended to indicate that an error occurred to to external environment conditions, like trannsient network errors.)
208-
// So instead, to avoid possible false positives, we choose to give this JWT `unverified` status, even though that's not technically correct.
209-
return false, nil
210-
}
211-
212221
// Use the OIDC Discovery protocol to fetch the public signing key,
213222
// being careful to avoid possible DoS from a potentially malicious JWKS server.
214223
issuer, err := parsedToken.Claims.GetIssuer()
@@ -285,14 +294,13 @@ func verifyJWT(ctx context.Context, client *http.Client, validator *jwt.Validato
285294
return false, fmt.Errorf("failed to export matching key: %w", err)
286295
}
287296

288-
err = parsedToken.Method.Verify(parsedToken.Raw, parsedToken.Signature, rawMatchingKey)
289-
297+
err = parsedToken.Method.Verify(strings.Join(tokenParts[0:2], "."), parsedToken.Signature, rawMatchingKey)
290298
if err != nil {
291299
// signature invalid
292300
return false, nil
293301
}
294302

295-
// signature valid and claims check out
303+
// signature valid and claims check out
296304
return true, nil
297305
}
298306

‎pkg/detectors/jwt/jwt_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ func TestJwt_Pattern(t *testing.T) {
2626
// secret is "a-string-secret-at-least-256-bits-long"
2727
eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30
2828
`,
29-
want: []string{"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30"},
29+
want: []string{},
3030
},
3131
{
3232
name: "HS256/valid-verbose-header",
3333
input: `
3434
// secret is "a-string-secret-at-least-256-bits-long"
3535
ewogICJhbGciOiJIUzI1NiIsCiIgIHR5cCI6IkpXVCIKfQo.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30
3636
`,
37-
want: []string{"ewogICJhbGciOiJIUzI1NiIsCiIgIHR5cCI6IkpXVCIKfQo.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30"},
37+
want: []string{},
3838
},
3939

4040
{
@@ -43,7 +43,7 @@ func TestJwt_Pattern(t *testing.T) {
4343
// secret is "a-string-secret-at-least-256-bits-long"
4444
eyJhbGciOiJIUzM4NCJ9.eyJtc2ciOiJoZWxsbyBoYWNrZXIsIHRoZXJlJ3Mgbm90aGluZyBmb3IgeW91IGhlcmUg8J-YhiJ9.NArdGjJ9DjXwGCLdNXDVjlwlvI_tUa2B3H44dvrZfKliBNTUL0YyKi8q4Al0Wl8u
4545
`,
46-
want: []string{"eyJhbGciOiJIUzM4NCJ9.eyJtc2ciOiJoZWxsbyBoYWNrZXIsIHRoZXJlJ3Mgbm90aGluZyBmb3IgeW91IGhlcmUg8J-YhiJ9.NArdGjJ9DjXwGCLdNXDVjlwlvI_tUa2B3H44dvrZfKliBNTUL0YyKi8q4Al0Wl8u"},
46+
want: []string{},
4747
},
4848

4949
{
@@ -52,7 +52,7 @@ func TestJwt_Pattern(t *testing.T) {
5252
// secret is "a-string-secret-at-least-256-bits-long"
5353
eyJhbGciOiJIUzUxMiJ9.eyJtc2ciOiJoZWxsbyBoYWNrZXIsIHRoZXJlJ3Mgbm90aGluZyBmb3IgeW91IGhlcmUg8J-YhiJ9.SiKgg2-kq7kVXhe5uLMakzlygHsJ70aTyXGhdbqG2SfkUC_fwk8MZ3JAWXrCIEJAUi_QMmQm-7qMU0SCMFRQug
5454
`,
55-
want: []string{"eyJhbGciOiJIUzUxMiJ9.eyJtc2ciOiJoZWxsbyBoYWNrZXIsIHRoZXJlJ3Mgbm90aGluZyBmb3IgeW91IGhlcmUg8J-YhiJ9.SiKgg2-kq7kVXhe5uLMakzlygHsJ70aTyXGhdbqG2SfkUC_fwk8MZ3JAWXrCIEJAUi_QMmQm-7qMU0SCMFRQug"},
55+
want: []string{},
5656
},
5757

5858
{
@@ -61,15 +61,15 @@ func TestJwt_Pattern(t *testing.T) {
6161
// secret is "a-string-secret-at-least-256-bits-long"
6262
ewogICJhbGciOiJIUzI1NiIsCiIgIHR5cCI6IkpXVCIKfQ==.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30
6363
`,
64-
want: []string{"ewogICJhbGciOiJIUzI1NiIsCiIgIHR5cCI6IkpXVCIKfQ==.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30"},
64+
want: []string{},
6565
},
6666
{
6767
name: "HS256/padding-in-claims/invalid-sig",
6868
input: `
6969
// secret is "a-string-secret-at-least-256-bits-long"
7070
ewogICJhbGciOiJIUzI1NiIsCiAgInR5cCI6IkpXVCIKfQ.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyfQo=.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30
7171
`,
72-
want: []string{"ewogICJhbGciOiJIUzI1NiIsCiAgInR5cCI6IkpXVCIKfQ.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyfQo=.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30"},
72+
want: []string{},
7373
},
7474

7575
{
@@ -78,7 +78,7 @@ func TestJwt_Pattern(t *testing.T) {
7878
// secret is "a-string-secret-at-least-256-bits-long"
7979
eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTQxNjIzOTAyMiwiZXhwIjoxNDE2MjM5MTIyfQ.EwRkAg9uOr6kVajMdMvB6KWGvIdDlGNRAH3lsZ2qQHI
8080
`,
81-
want: []string{"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTQxNjIzOTAyMiwiZXhwIjoxNDE2MjM5MTIyfQ.EwRkAg9uOr6kVajMdMvB6KWGvIdDlGNRAH3lsZ2qQHI"},
81+
want: []string{},
8282
},
8383

8484
{
@@ -87,7 +87,7 @@ func TestJwt_Pattern(t *testing.T) {
8787
// secret is "a-string-secret-at-least-256-bits-long"
8888
eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MjQxNjIzOTAyMiwibmJmIjozNDE2MjM5MDIyLCJleHAiOjQ0MTYyMzkwMjJ9.rVQaCey3ETfhn8AeiC_EmFjp6_X2Dq8QY_AzBAF2ZzQ
8989
`,
90-
want: []string{"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MjQxNjIzOTAyMiwibmJmIjozNDE2MjM5MDIyLCJleHAiOjQ0MTYyMzkwMjJ9.rVQaCey3ETfhn8AeiC_EmFjp6_X2Dq8QY_AzBAF2ZzQ"},
90+
want: []string{},
9191
},
9292

9393
{

0 commit comments

Comments
 (0)