Skip to content

Commit

Permalink
oidc: add Config.InsecureSkipSignatureCheck
Browse files Browse the repository at this point in the history
Includes a big warning for why this is usually a bad idea.

Fixes #350
  • Loading branch information
ericchiang committed Sep 9, 2022
1 parent 366ac4a commit fb9e009
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 9 deletions.
25 changes: 20 additions & 5 deletions oidc/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,17 @@ type Config struct {

// Time function to check Token expiry. Defaults to time.Now
Now func() time.Time

// InsecureSkipSignatureCheck causes this package to skip JWT signature validation.
// It's intended for special cases where providers (such as Azure), use the "none"
// algorithm.
//
// This option can only be enabled safely when the ID Token is received directly
// from the provider after the token exchange.
//
// This option MUST NOT be used when receiving an ID Token from sources other
// than the token endpoint.
InsecureSkipSignatureCheck bool
}

// Verifier returns an IDTokenVerifier that uses the provider's key set to verify JWTs.
Expand Down Expand Up @@ -196,11 +207,6 @@ func resolveDistributedClaim(ctx context.Context, verifier *IDTokenVerifier, src
// token, err := verifier.Verify(ctx, rawIDToken)
//
func (v *IDTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*IDToken, error) {
jws, err := jose.ParseSigned(rawIDToken)
if err != nil {
return nil, fmt.Errorf("oidc: malformed jwt: %v", err)
}

// Throw out tokens with invalid claims before trying to verify the token. This lets
// us do cheap checks before possibly re-syncing keys.
payload, err := parseJWT(rawIDToken)
Expand Down Expand Up @@ -288,6 +294,15 @@ func (v *IDTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*IDTok
}
}

if v.config.InsecureSkipSignatureCheck {
return t, nil
}

jws, err := jose.ParseSigned(rawIDToken)
if err != nil {
return nil, fmt.Errorf("oidc: malformed jwt: %v", err)
}

switch len(jws.Signatures) {
case 0:
return nil, fmt.Errorf("oidc: id token not signed")
Expand Down
34 changes: 30 additions & 4 deletions oidc/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package oidc
import (
"context"
"crypto"
"encoding/base64"
"errors"
"io"
"net/http"
Expand Down Expand Up @@ -122,6 +123,24 @@ func TestVerify(t *testing.T) {
},
signKey: newRSAKey(t),
},
{
name: "unsigned token",
idToken: `{"iss":"https://foo"}`,
config: Config{
SkipClientIDCheck: true,
SkipExpiryCheck: true,
},
wantErr: true,
},
{
name: "unsigned token InsecureSkipSignatureCheck",
idToken: `{"iss":"https://foo"}`,
config: Config{
SkipClientIDCheck: true,
SkipExpiryCheck: true,
InsecureSkipSignatureCheck: true,
},
},
}
for _, test := range tests {
t.Run(test.name, test.run)
Expand Down Expand Up @@ -537,7 +556,14 @@ type verificationTest struct {
}

func (v verificationTest) runGetToken(t *testing.T) (*IDToken, error) {
token := v.signKey.sign(t, []byte(v.idToken))
var token string
if v.signKey != nil {
token = v.signKey.sign(t, []byte(v.idToken))
} else {
token = base64.RawURLEncoding.EncodeToString([]byte(`{alg: "none"}`))
token += "."
token += base64.RawURLEncoding.EncodeToString([]byte(v.idToken))
}

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand All @@ -547,10 +573,10 @@ func (v verificationTest) runGetToken(t *testing.T) (*IDToken, error) {
issuer = v.issuer
}
var ks KeySet
if v.verificationKey == nil {
ks = &StaticKeySet{PublicKeys: []crypto.PublicKey{v.signKey.pub}}
} else {
if v.verificationKey != nil {
ks = &StaticKeySet{PublicKeys: []crypto.PublicKey{v.verificationKey.pub}}
} else if v.signKey != nil {
ks = &StaticKeySet{PublicKeys: []crypto.PublicKey{v.signKey.pub}}
}
verifier := NewVerifier(issuer, ks, &v.config)

Expand Down

0 comments on commit fb9e009

Please sign in to comment.