oidc: verify signature before parsing token#465
Conversation
This change updates the verification logic of this library to always verify the signature of the token before validating the payload. See associated issue. coreos#464
ericchiang
force-pushed
the
validate-before-parse
branch
from
4236a71 to
a4308ca
Compare
| return nil, fmt.Errorf("oidc: id token signed with unsupported algorithm, expected %q got %q", supportedSigAlgs, sig.Header.Algorithm) | ||
| } | ||
|
|
||
| payload, err := v.keySet.VerifySignature(ctx, rawIDToken) |
There was a problem hiding this comment.
how did moving this first help the DOS? this still calls:
VerifySignature → jose.ParseSigned → parseSignedCompact
which still calls parts := strings.Split(input, ".")
If this was trying to catch spurious tokens with lots of . characters, something like the count check in https://go.dev/cl/652155 would seem more effective
There was a problem hiding this comment.
Hey! The v3 version of this package depends on a go-jose version that doesn't do that.
Line 8 in 35b8e03
https://github.com/go-jose/go-jose/blob/v4.1.3/jws.go#L367
For v2, maybe we should do our own check before this to throw out invalid ID tokens early
There was a problem hiding this comment.
For v2, maybe we should do our own check before this to throw out invalid ID tokens early
Yeah, that's exactly what I'd recommend
There was a problem hiding this comment.
(also, sorry for the random 6-month-later drive-by ... I just got here while reviewing kubernetes/kubernetes#136162 (comment))
There was a problem hiding this comment.
Lol, no worries. Appreciate you actually checking my work.
Sent the change and tagged a release:
2 participants
This change updates the verification logic of this library to always verify the signature of the token before validating the payload. See associated issue.
#464