Merge pull request #570 from ericchiang/update-go-oidc-for-hmac-changes

Update go-oidc for hmac changes

glide.lock

-hash: 9e02d162b5001e9ba3028f5cac8b8a85f73be2e7546a4add488b446821e13da6
-updated: 2016-08-16T12:24:59.701803152-07:00
+hash: a453b9008bef3edc06f6df648bc40048ab387bf03a3e9127cdccb817569d518e
+updated: 2016-08-27T08:50:06.025458672-07:00
 imports:
 - name: github.com/andybalholm/cascadia
   version: 6122e68c2642b7b75c538a63b15168c6c80fb757
 - name: github.com/coreos/go-oidc
-  version: 1efe0e1303a62da553fcb6beb8bd2aa9250c0ca8
+  version: 9fae754a41cbdc3be9cb97a180eb323b625db614
   subpackages:
   - http
   - jose

glide.yaml

@@ -5,7 +5,7 @@ import:
 - package: github.com/andybalholm/cascadia
   version: 6122e68c2642b7b75c538a63b15168c6c80fb757
 - package: github.com/coreos/go-oidc
-  version: 1efe0e1303a62da553fcb6beb8bd2aa9250c0ca8
+  version: 9fae754a41cbdc3be9cb97a180eb323b625db614
   subpackages:
   - http
   - jose

vendor/github.com/coreos/go-oidc/.travis.yml

@@ -4,6 +4,7 @@ go:
   - 1.4.3
   - 1.5.4
   - 1.6.1
+  - 1.7
 
 install:
  - go get -v -t ./...

vendor/github.com/coreos/go-oidc/jose/sig_hmac.go

 package jose
 
 import (
-	"bytes"
 	"crypto"
 	"crypto/hmac"
 	_ "crypto/sha256"
@@ -44,7 +43,9 @@ func (v *VerifierHMAC) Alg() string {
 func (v *VerifierHMAC) Verify(sig []byte, data []byte) error {
 	h := hmac.New(v.Hash.New, v.Secret)
 	h.Write(data)
-	if !bytes.Equal(sig, h.Sum(nil)) {
+	// hmac.Equal compares two hmacs but does it in constant time to mitigating time
+	// based attacks. See #98
+	if !hmac.Equal(sig, h.Sum(nil)) {
 		return errors.New("invalid hmac signature")
 	}
 	return nil

vendor/github.com/coreos/go-oidc/oidc/verification.go

@@ -161,11 +161,18 @@ func NewJWTVerifier(issuer, clientID string, syncFunc func() error, keysFunc fun
 }
 
 func (v *JWTVerifier) Verify(jwt jose.JWT) error {
+	// Verify claims before verifying the signature. This is an optimization to throw out
+	// tokens we know are invalid without undergoing an expensive signature check and
+	// possibly a re-sync event.
+	if err := VerifyClaims(jwt, v.issuer, v.clientID); err != nil {
+		return fmt.Errorf("oidc: JWT claims invalid: %v", err)
+	}
+
 	ok, err := VerifySignature(jwt, v.keysFunc())
-	if ok {
-		goto SignatureVerified
-	} else if err != nil {
+	if err != nil {
 		return fmt.Errorf("oidc: JWT signature verification failed: %v", err)
+	} else if ok {
+		return nil
 	}
 
 	if err = v.syncFunc(); err != nil {
@@ -179,10 +186,5 @@ func (v *JWTVerifier) Verify(jwt jose.JWT) error {
 		return errors.New("oidc: unable to verify JWT signature: no matching keys")
 	}
 
-SignatureVerified:
-	if err := VerifyClaims(jwt, v.issuer, v.clientID); err != nil {
-		return fmt.Errorf("oidc: JWT claims invalid: %v", err)
-	}
-
 	return nil
 }

vendor/github.com/coreos/go-oidc/oidc/verification_test.go

@@ -192,43 +192,22 @@ func TestJWTVerifier(t *testing.T) {
 	}
 	pk2 := *key.NewPublicKey(priv2.JWK())
 
-	jwtPK1, err := jose.NewSignedJWT(NewClaims(iss, "XXX", "XXX", past12, future12), priv1.Signer())
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	jwtPK1BadClaims, err := jose.NewSignedJWT(NewClaims(iss, "XXX", "YYY", past12, future12), priv1.Signer())
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	jwtPK1BadClaims2, err := jose.NewSignedJWT(NewClaims(iss, "XXX", []string{"YYY", "ZZZ"}, past12, future12), priv1.Signer())
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	jwtExpired, err := jose.NewSignedJWT(NewClaims(iss, "XXX", "XXX", past36, past12), priv1.Signer())
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	jwtPK2, err := jose.NewSignedJWT(NewClaims(iss, "XXX", "XXX", past12, future12), priv2.Signer())
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	jwtPK3, err := jose.NewSignedJWT(NewClaims(iss, "XXX", []string{"ZZZ", "XXX"}, past12, future12), priv1.Signer())
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
+	newJWT := func(issuer, subject string, aud interface{}, issuedAt, exp time.Time, signer jose.Signer) jose.JWT {
+		jwt, err := jose.NewSignedJWT(NewClaims(issuer, subject, aud, issuedAt, exp), signer)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return *jwt
 	}
 
 	tests := []struct {
+		name     string
 		verifier JWTVerifier
 		jwt      jose.JWT
 		wantErr  bool
 	}{
-		// JWT signed with available key
 		{
+			name: "JWT signed with available key",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -237,12 +216,11 @@ func TestJWTVerifier(t *testing.T) {
 					return []key.PublicKey{pk1}
 				},
 			},
-			jwt:     *jwtPK1,
+			jwt:     newJWT(iss, "XXX", "XXX", past12, future12, priv1.Signer()),
 			wantErr: false,
 		},
-
-		// JWT signed with available key, with bad claims
 		{
+			name: "JWT signed with available key, with bad claims",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -251,12 +229,12 @@ func TestJWTVerifier(t *testing.T) {
 					return []key.PublicKey{pk1}
 				},
 			},
-			jwt:     *jwtPK1BadClaims,
+			jwt:     newJWT(iss, "XXX", "YYY", past12, future12, priv1.Signer()),
 			wantErr: true,
 		},
 
-		// JWT signed with available key,
 		{
+			name: "JWT signed with available key",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -265,12 +243,12 @@ func TestJWTVerifier(t *testing.T) {
 					return []key.PublicKey{pk1}
 				},
 			},
-			jwt:     *jwtPK1BadClaims2,
+			jwt:     newJWT(iss, "XXX", []string{"YYY", "ZZZ"}, past12, future12, priv1.Signer()),
 			wantErr: true,
 		},
 
-		// expired JWT signed with available key
 		{
+			name: "expired JWT signed with available key",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -279,12 +257,12 @@ func TestJWTVerifier(t *testing.T) {
 					return []key.PublicKey{pk1}
 				},
 			},
-			jwt:     *jwtExpired,
+			jwt:     newJWT(iss, "XXX", "XXX", past36, past12, priv1.Signer()),
 			wantErr: true,
 		},
 
-		// JWT signed with unrecognized key, verifiable after sync
 		{
+			name: "JWT signed with unrecognized key, verifiable after sync",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -300,12 +278,12 @@ func TestJWTVerifier(t *testing.T) {
 					}
 				}(),
 			},
-			jwt:     *jwtPK2,
+			jwt:     newJWT(iss, "XXX", "XXX", past36, future12, priv2.Signer()),
 			wantErr: false,
 		},
 
-		// JWT signed with unrecognized key, not verifiable after sync
 		{
+			name: "JWT signed with unrecognized key, not verifiable after sync",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -314,12 +292,12 @@ func TestJWTVerifier(t *testing.T) {
 					return []key.PublicKey{pk1}
 				},
 			},
-			jwt:     *jwtPK2,
+			jwt:     newJWT(iss, "XXX", "XXX", past12, future12, priv2.Signer()),
 			wantErr: true,
 		},
 
-		// verifier gets no keys from keysFunc, still not verifiable after sync
 		{
+			name: "verifier gets no keys from keysFunc, still not verifiable after sync",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -328,12 +306,12 @@ func TestJWTVerifier(t *testing.T) {
 					return []key.PublicKey{}
 				},
 			},
-			jwt:     *jwtPK1,
+			jwt:     newJWT(iss, "XXX", "XXX", past12, future12, priv1.Signer()),
 			wantErr: true,
 		},
 
-		// verifier gets no keys from keysFunc, verifiable after sync
 		{
+			name: "verifier gets no keys from keysFunc, verifiable after sync",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -349,12 +327,12 @@ func TestJWTVerifier(t *testing.T) {
 					}
 				}(),
 			},
-			jwt:     *jwtPK2,
+			jwt:     newJWT(iss, "XXX", "XXX", past12, future12, priv2.Signer()),
 			wantErr: false,
 		},
 
-		// JWT signed with available key, 'aud' is a string array.
 		{
+			name: "JWT signed with available key, 'aud' is a string array",
 			verifier: JWTVerifier{
 				issuer:   "example.com",
 				clientID: "XXX",
@@ -363,17 +341,40 @@ func TestJWTVerifier(t *testing.T) {
 					return []key.PublicKey{pk1}
 				},
 			},
-			jwt:     *jwtPK3,
+			jwt:     newJWT(iss, "XXX", []string{"ZZZ", "XXX"}, past12, future12, priv1.Signer()),
 			wantErr: false,
 		},
+		{
+			name: "invalid issuer claim shouldn't trigger sync",
+			verifier: JWTVerifier{
+				issuer:   "example.com",
+				clientID: "XXX",
+				syncFunc: func() error {
+					t.Errorf("invalid issuer claim shouldn't trigger a sync")
+					return nil
+				},
+				keysFunc: func() func() []key.PublicKey {
+					var i int
+					return func() []key.PublicKey {
+						defer func() { i++ }()
+						return [][]key.PublicKey{
+							[]key.PublicKey{},
+							[]key.PublicKey{pk2},
+						}[i]
+					}
+				}(),
+			},
+			jwt:     newJWT("invalid-issuer", "XXX", []string{"ZZZ", "XXX"}, past12, future12, priv2.Signer()),
+			wantErr: true,
+		},
 	}
 
-	for i, tt := range tests {
+	for _, tt := range tests {
 		err := tt.verifier.Verify(tt.jwt)
 		if tt.wantErr && (err == nil) {
-			t.Errorf("case %d: wanted non-nil error", i)
+			t.Errorf("case %q: wanted non-nil error", tt.name)
 		} else if !tt.wantErr && (err != nil) {
-			t.Errorf("case %d: wanted nil error, got %v", i, err)
+			t.Errorf("case %q: wanted nil error, got %v", tt.name, err)
 		}
 	}
 }