From a8d059a237d6eb18bf9ac0577a1f486123228837 Mon Sep 17 00:00:00 2001 From: Maarten den Braber Date: Mon, 27 May 2019 09:17:39 +0200 Subject: [PATCH 01/11] Add userinfo endpoint Co-authored-by: Yuxing Li <360983+jackielii@users.noreply.github.com> Co-authored-by: Francisco Santiago <1737357+fjbsantiago@users.noreply.github.com> --- server/handlers.go | 104 ++++++++++++++++++++++++++++++++++++++++++++- server/oauth2.go | 5 +++ server/server.go | 1 + 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index ae6a21db..058ea3ab 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -3,6 +3,7 @@ package server import ( "context" "encoding/json" + "encoding/base64" "errors" "fmt" "net/http" @@ -22,6 +23,10 @@ import ( "github.com/dexidp/dex/storage" ) +var ( + errTokenExpired = errors.New("token has expired") +) + // newHealthChecker returns the healthz handler. The handler runs until the // provided context is canceled. func (s *Server) newHealthChecker(ctx context.Context) http.Handler { @@ -151,6 +156,7 @@ type discovery struct { Auth string `json:"authorization_endpoint"` Token string `json:"token_endpoint"` Keys string `json:"jwks_uri"` + UserInfo string `json:"userinfo_endpoint"` ResponseTypes []string `json:"response_types_supported"` Subjects []string `json:"subject_types_supported"` IDTokenAlgs []string `json:"id_token_signing_alg_values_supported"` @@ -165,6 +171,7 @@ func (s *Server) discoveryHandler() (http.HandlerFunc, error) { Auth: s.absURL("/auth"), Token: s.absURL("/token"), Keys: s.absURL("/keys"), + Keys: s.absURL("/userinfo"), Subjects: []string{"public"}, IDTokenAlgs: []string{string(jose.RS256)}, Scopes: []string{"openid", "email", "groups", "profile", "offline_access"}, @@ -559,7 +566,12 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe idToken string idTokenExpiry time.Time - accessToken = storage.NewID() +i accessToken, err := s.newAccessToken(client.ID, authCode.Claims, authCode.Scopes, authCode.Nonce, authCode.ConnectorID) + if err != nil { + s.logger.Errorf("failed to create new access token: %v", err) + s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) + return + } ) for _, responseType := range authReq.ResponseTypes { @@ -965,7 +977,13 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie Groups: ident.Groups, } - accessToken := storage.NewID() + accessToken, err := s.newAccessToken(client.ID, claims, scopes, refresh.Nonce, refresh.ConnectorID) + if err != nil { + s.logger.Errorf("failed to create new access token: %v", err) + s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) + return + } + idToken, expiry, err := s.newIDToken(client.ID, claims, scopes, refresh.Nonce, accessToken, refresh.ConnectorID) if err != nil { s.logger.Errorf("failed to create ID token: %v", err) @@ -1026,6 +1044,88 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie s.writeAccessToken(w, idToken, accessToken, rawNewToken, expiry) } +func (s *Server) handleUserInfo(w http.ResponseWriter, r *http.Request) { + authorization := r.Header.Get("Authorization") + parts := strings.Fields(authorization) + + if len(parts) != 2 || !strings.EqualFold(parts[0], "bearer") { + msg := "invalid authorization header" + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer realm="dex", error="%s", error_description="%s"`, errInvalidRequest, msg)) + s.tokenErrHelper(w, errInvalidRequest, msg, http.StatusBadRequest) + return + } + + token := parts[1] + + verified, err := s.verify(token) + if err != nil { + if err == errTokenExpired { + s.tokenErrHelper(w, errAccessDenied, err.Error(), http.StatusUnauthorized) + return + } + s.tokenErrHelper(w, errInvalidRequest, err.Error(), http.StatusBadRequest) + return + } + + w.Header().Set("Content-Type", "application/json") + w.Write(verified) + } + +func (s *Server) verify(token string) ([]byte, error) { + keys, err := s.storage.GetKeys() + if err != nil { + return nil, fmt.Errorf("failed to get keys: %v", err) + } + + if keys.SigningKey == nil { + return nil, fmt.Errorf("no private keys found") + } + + object, err := jose.ParseSigned(token) + if err != nil { + return nil, fmt.Errorf("unable to parse signed message") + } + + // Parse the message to check expiry, as it jose doesn't distinguish expiry error from others + parts := strings.Split(token, ".") + if len(parts) != 3 { + return nil, fmt.Errorf("square/go-jose: compact JWS format must have three parts") + } + + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return nil, err + } + + // TODO: check other claims + var tokenInfo struct { + Expiry int64 `json:"exp"` + } + + if err := json.Unmarshal(payload, &tokenInfo); err != nil { + return nil, err + } + + if tokenInfo.Expiry < s.now().Unix() { + return nil, errTokenExpired + } + + var allKeys []*jose.JSONWebKey + + allKeys = append(allKeys, keys.SigningKeyPub) + for _, key := range keys.VerificationKeys { + allKeys = append(allKeys, key.PublicKey) + } + + for _, pubKey := range allKeys { + verified, err := object.Verify(pubKey) + if err == nil { + return verified, nil + } + } + return nil, errors.New("unable to verify jwt") +} + func (s *Server) writeAccessToken(w http.ResponseWriter, idToken, accessToken, refreshToken string, expiry time.Time) { // TODO(ericchiang): figure out an access token story and support the user info // endpoint. For now use a random value so no one depends on the access_token diff --git a/server/oauth2.go b/server/oauth2.go index 8c9494f5..26d152f4 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -265,6 +265,11 @@ type federatedIDClaims struct { UserID string `json:"user_id,omitempty"` } +func (s *Server) newAccessToken(clientID string, claims storage.Claims, scopes []string, nonce, connID string) (accessToken string, err error) { + idToken, _, err := s.newIDToken(clientID, claims, scopes, nonce, storage.NewID(), connID) + return idToken, err +} + func (s *Server) newIDToken(clientID string, claims storage.Claims, scopes []string, nonce, accessToken, connID string) (idToken string, expiry time.Time, err error) { keys, err := s.storage.GetKeys() if err != nil { diff --git a/server/server.go b/server/server.go index 9dc259fb..69b4d0d7 100644 --- a/server/server.go +++ b/server/server.go @@ -270,6 +270,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy) // TODO(ericchiang): rate limit certain paths based on IP. handleWithCORS("/token", s.handleToken) handleWithCORS("/keys", s.handlePublicKeys) + handleWithCORS("/userinfo", s.handleUserInfo) handleFunc("/auth", s.handleAuthorization) handleFunc("/auth/{connector}", s.handleConnectorLogin) r.HandleFunc(path.Join(issuerURL.Path, "/callback"), func(w http.ResponseWriter, r *http.Request) { From d7750b1e264e81608e8b1ad1c0ff4a4e65f5bbd1 Mon Sep 17 00:00:00 2001 From: Maarten den Braber Date: Mon, 27 May 2019 09:41:00 +0200 Subject: [PATCH 02/11] Fix changes --- server/handlers.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 058ea3ab..854fe075 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -171,7 +171,7 @@ func (s *Server) discoveryHandler() (http.HandlerFunc, error) { Auth: s.absURL("/auth"), Token: s.absURL("/token"), Keys: s.absURL("/keys"), - Keys: s.absURL("/userinfo"), + UserInfo: s.absURL("/userinfo"), Subjects: []string{"public"}, IDTokenAlgs: []string{string(jose.RS256)}, Scopes: []string{"openid", "email", "groups", "profile", "offline_access"}, @@ -566,12 +566,8 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe idToken string idTokenExpiry time.Time -i accessToken, err := s.newAccessToken(client.ID, authCode.Claims, authCode.Scopes, authCode.Nonce, authCode.ConnectorID) - if err != nil { - s.logger.Errorf("failed to create new access token: %v", err) - s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) - return - } + // Access token + accessToken string ) for _, responseType := range authReq.ResponseTypes { @@ -607,6 +603,14 @@ i accessToken, err := s.newAccessToken(client.ID, authCode.Claims, authCode.Sco case responseTypeIDToken: implicitOrHybrid = true var err error + + accessToken, err := s.newAccessToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce, authReq.ConnectorID) + if err != nil { + s.logger.Errorf("failed to create new access token: %v", err) + s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) + return + } + idToken, idTokenExpiry, err = s.newIDToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce, accessToken, authReq.ConnectorID) if err != nil { s.logger.Errorf("failed to create ID token: %v", err) @@ -728,7 +732,13 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s return } - accessToken := storage.NewID() + accessToken, err := s.newAccessToken(client.ID, authCode.Claims, authCode.Scopes, authCode.Nonce, authCode.ConnectorID) + if err != nil { + s.logger.Errorf("failed to create new access token: %v", err) + s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) + return + } + idToken, expiry, err := s.newIDToken(client.ID, authCode.Claims, authCode.Scopes, authCode.Nonce, accessToken, authCode.ConnectorID) if err != nil { s.logger.Errorf("failed to create ID token: %v", err) From 74f4e749b998eb02e4fe35b6d9630c4046770030 Mon Sep 17 00:00:00 2001 From: Maarten den Braber Date: Mon, 27 May 2019 11:14:15 +0200 Subject: [PATCH 03/11] Formatting --- server/handlers.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 854fe075..ef835da6 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -2,8 +2,8 @@ package server import ( "context" - "encoding/json" "encoding/base64" + "encoding/json" "errors" "fmt" "net/http" @@ -567,7 +567,7 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe idTokenExpiry time.Time // Access token - accessToken string + accessToken string ) for _, responseType := range authReq.ResponseTypes { @@ -1079,7 +1079,7 @@ func (s *Server) handleUserInfo(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.Write(verified) - } +} func (s *Server) verify(token string) ([]byte, error) { keys, err := s.storage.GetKeys() From 3dd1bac821852bc01f8a250e64b7dff5b89c57f6 Mon Sep 17 00:00:00 2001 From: mdbraber Date: Wed, 5 Jun 2019 22:07:34 +0200 Subject: [PATCH 04/11] Fix comments --- server/handlers.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index ef835da6..573542c0 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -1096,10 +1096,9 @@ func (s *Server) verify(token string) ([]byte, error) { return nil, fmt.Errorf("unable to parse signed message") } - // Parse the message to check expiry, as it jose doesn't distinguish expiry error from others parts := strings.Split(token, ".") if len(parts) != 3 { - return nil, fmt.Errorf("square/go-jose: compact JWS format must have three parts") + return nil, fmt.Errorf("compact JWS format must have three parts") } payload, err := base64.RawURLEncoding.DecodeString(parts[1]) From 157c359f3e860c8529e34fecfc94c83f98f9e227 Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Thu, 20 Jun 2019 12:27:47 -0400 Subject: [PATCH 05/11] Bump go-oidc to latest v2 --- go.mod | 2 +- go.sum | 4 +- server/server_test.go | 33 +-- vendor/github.com/coreos/go-oidc/MAINTAINERS | 5 +- vendor/github.com/coreos/go-oidc/README.md | 2 +- .../coreos/go-oidc/code-of-conduct.md | 61 +++++ vendor/github.com/coreos/go-oidc/gen.go | 150 ------------ vendor/github.com/coreos/go-oidc/jwks.go | 225 ++++++++++-------- vendor/github.com/coreos/go-oidc/oidc.go | 85 +++++-- vendor/github.com/coreos/go-oidc/test | 3 +- vendor/github.com/coreos/go-oidc/verify.go | 149 ++++++------ vendor/modules.txt | 2 +- 12 files changed, 350 insertions(+), 371 deletions(-) create mode 100644 vendor/github.com/coreos/go-oidc/code-of-conduct.md delete mode 100644 vendor/github.com/coreos/go-oidc/gen.go diff --git a/go.mod b/go.mod index 50d393b2..d4231946 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/boltdb/bolt v1.3.1 // indirect github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292 // indirect github.com/coreos/etcd v3.2.9+incompatible - github.com/coreos/go-oidc v0.0.0-20170307191026-be73733bb8cc + github.com/coreos/go-oidc v2.0.0+incompatible github.com/coreos/go-semver v0.2.0 // indirect github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 // indirect github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f // indirect diff --git a/go.sum b/go.sum index 8d37bef6..44255005 100644 --- a/go.sum +++ b/go.sum @@ -8,8 +8,8 @@ github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292 h1:dzj1/xcivGjNPw github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292/go.mod h1:qRiX68mZX1lGBkTWyp3CLcenw9I94W2dLeRvMzcn9N4= github.com/coreos/etcd v3.2.9+incompatible h1:3TbjfK5+aSRLTU/KgBC1xlgA2dn2ddYQngRqX6HFwlQ= github.com/coreos/etcd v3.2.9+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= -github.com/coreos/go-oidc v0.0.0-20170307191026-be73733bb8cc h1:9yuvA19Q5WFkLwJcMDoYm8m89ilzqZ5zEHqdvU+Zbds= -github.com/coreos/go-oidc v0.0.0-20170307191026-be73733bb8cc/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc= +github.com/coreos/go-oidc v2.0.0+incompatible h1:+RStIopZ8wooMx+Vs5Bt8zMXxV1ABl5LbakNExNmZIg= +github.com/coreos/go-oidc v2.0.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc= github.com/coreos/go-semver v0.2.0 h1:3Jm3tLmsgAYcjC+4Up7hJrFBPr+n7rAqYeSw/SZazuY= github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 h1:3jFq2xL4ZajGK4aZY8jz+DAF0FHjI51BXjjSwCzS1Dk= diff --git a/server/server_test.go b/server/server_test.go index 536387c4..87c80fbb 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -16,7 +16,6 @@ import ( "reflect" "sort" "strings" - "sync" "testing" "time" @@ -541,23 +540,6 @@ func TestOAuth2CodeFlow(t *testing.T) { } } -type nonceSource struct { - nonce string - once sync.Once -} - -func (n *nonceSource) ClaimNonce(nonce string) error { - if n.nonce != nonce { - return errors.New("invalid nonce") - } - ok := false - n.once.Do(func() { ok = true }) - if !ok { - return errors.New("invalid nonce") - } - return nil -} - func TestOAuth2ImplicitFlow(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -623,11 +605,8 @@ func TestOAuth2ImplicitFlow(t *testing.T) { t.Fatalf("failed to create client: %v", err) } - src := &nonceSource{nonce: nonce} - idTokenVerifier := p.Verifier(&oidc.Config{ - ClientID: client.ID, - ClaimNonce: src.ClaimNonce, + ClientID: client.ID, }) oauth2Config = &oauth2.Config{ @@ -646,13 +625,17 @@ func TestOAuth2ImplicitFlow(t *testing.T) { if err != nil { return fmt.Errorf("failed to parse fragment: %v", err) } - idToken := v.Get("id_token") - if idToken == "" { + rawIDToken := v.Get("id_token") + if rawIDToken == "" { return errors.New("no id_token in fragment") } - if _, err := idTokenVerifier.Verify(ctx, idToken); err != nil { + idToken, err := idTokenVerifier.Verify(ctx, rawIDToken) + if err != nil { return fmt.Errorf("failed to verify id_token: %v", err) } + if idToken.Nonce != nonce { + return fmt.Errorf("failed to verify id_token: nonce was %v, but want %v", idToken.Nonce, nonce) + } return nil } diff --git a/vendor/github.com/coreos/go-oidc/MAINTAINERS b/vendor/github.com/coreos/go-oidc/MAINTAINERS index 68079f1e..0a1f2937 100644 --- a/vendor/github.com/coreos/go-oidc/MAINTAINERS +++ b/vendor/github.com/coreos/go-oidc/MAINTAINERS @@ -1,3 +1,2 @@ -Bobby Rullo (@bobbyrullo) -Ed Rooth (@sym3tri) -Eric Chiang (@ericchiang) +Eric Chiang (@ericchiang) +Rithu Leena John (@rithujohn191) diff --git a/vendor/github.com/coreos/go-oidc/README.md b/vendor/github.com/coreos/go-oidc/README.md index 2a5c13e5..520d7c87 100644 --- a/vendor/github.com/coreos/go-oidc/README.md +++ b/vendor/github.com/coreos/go-oidc/README.md @@ -38,7 +38,7 @@ func handleRedirect(w http.ResponseWriter, r *http.Request) { The on responses, the provider can be used to verify ID Tokens. ```go -var verifier = provider.Verifier() +var verifier = provider.Verifier(&oidc.Config{ClientID: clientID}) func handleOAuth2Callback(w http.ResponseWriter, r *http.Request) { // Verify state and errors. diff --git a/vendor/github.com/coreos/go-oidc/code-of-conduct.md b/vendor/github.com/coreos/go-oidc/code-of-conduct.md new file mode 100644 index 00000000..a234f360 --- /dev/null +++ b/vendor/github.com/coreos/go-oidc/code-of-conduct.md @@ -0,0 +1,61 @@ +## CoreOS Community Code of Conduct + +### Contributor Code of Conduct + +As contributors and maintainers of this project, and in the interest of +fostering an open and welcoming community, we pledge to respect all people who +contribute through reporting issues, posting feature requests, updating +documentation, submitting pull requests or patches, and other activities. + +We are committed to making participation in this project a harassment-free +experience for everyone, regardless of level of experience, gender, gender +identity and expression, sexual orientation, disability, personal appearance, +body size, race, ethnicity, age, religion, or nationality. + +Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery +* Personal attacks +* Trolling or insulting/derogatory comments +* Public or private harassment +* Publishing others' private information, such as physical or electronic addresses, without explicit permission +* Other unethical or unprofessional conduct. + +Project maintainers have the right and responsibility to remove, edit, or +reject comments, commits, code, wiki edits, issues, and other contributions +that are not aligned to this Code of Conduct. By adopting this Code of Conduct, +project maintainers commit themselves to fairly and consistently applying these +principles to every aspect of managing this project. Project maintainers who do +not follow or enforce the Code of Conduct may be permanently removed from the +project team. + +This code of conduct applies both within project spaces and in public spaces +when an individual is representing the project or its community. + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported by contacting a project maintainer, Brandon Philips +, and/or Rithu John . + +This Code of Conduct is adapted from the Contributor Covenant +(http://contributor-covenant.org), version 1.2.0, available at +http://contributor-covenant.org/version/1/2/0/ + +### CoreOS Events Code of Conduct + +CoreOS events are working conferences intended for professional networking and +collaboration in the CoreOS community. Attendees are expected to behave +according to professional standards and in accordance with their employer’s +policies on appropriate workplace behavior. + +While at CoreOS events or related social networking opportunities, attendees +should not engage in discriminatory or offensive speech or actions including +but not limited to gender, sexuality, race, age, disability, or religion. +Speakers should be especially aware of these concerns. + +CoreOS does not condone any statements by speakers contrary to these standards. +CoreOS reserves the right to deny entrance and/or eject from an event (without +refund) any individual found to be engaging in discriminatory or offensive +speech or actions. + +Please bring any concerns to the immediate attention of designated on-site +staff, Brandon Philips , and/or Rithu John . diff --git a/vendor/github.com/coreos/go-oidc/gen.go b/vendor/github.com/coreos/go-oidc/gen.go deleted file mode 100644 index 0c798f67..00000000 --- a/vendor/github.com/coreos/go-oidc/gen.go +++ /dev/null @@ -1,150 +0,0 @@ -// +build ignore - -// This file is used to generate keys for tests. - -package main - -import ( - "bytes" - "crypto" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "crypto/rsa" - "encoding/hex" - "encoding/json" - "fmt" - "io/ioutil" - "log" - "text/template" - - jose "gopkg.in/square/go-jose.v2" -) - -type key struct { - name string - new func() (crypto.Signer, error) -} - -var keys = []key{ - { - "ECDSA_256", func() (crypto.Signer, error) { - return ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - }, - }, - { - "ECDSA_384", func() (crypto.Signer, error) { - return ecdsa.GenerateKey(elliptic.P384(), rand.Reader) - }, - }, - { - "ECDSA_521", func() (crypto.Signer, error) { - return ecdsa.GenerateKey(elliptic.P521(), rand.Reader) - }, - }, - { - "RSA_1024", func() (crypto.Signer, error) { - return rsa.GenerateKey(rand.Reader, 1024) - }, - }, - { - "RSA_2048", func() (crypto.Signer, error) { - return rsa.GenerateKey(rand.Reader, 2048) - }, - }, - { - "RSA_4096", func() (crypto.Signer, error) { - return rsa.GenerateKey(rand.Reader, 4096) - }, - }, -} - -func newJWK(k key, prefix, ident string) (privBytes, pubBytes []byte, err error) { - priv, err := k.new() - if err != nil { - return nil, nil, fmt.Errorf("generate %s: %v", k.name, err) - } - pub := priv.Public() - - privKey := &jose.JSONWebKey{Key: priv} - thumbprint, err := privKey.Thumbprint(crypto.SHA256) - if err != nil { - return nil, nil, fmt.Errorf("computing thumbprint: %v", err) - } - - keyID := hex.EncodeToString(thumbprint) - privKey.KeyID = keyID - pubKey := &jose.JSONWebKey{Key: pub, KeyID: keyID} - - privBytes, err = json.MarshalIndent(privKey, prefix, ident) - if err != nil { - return - } - pubBytes, err = json.MarshalIndent(pubKey, prefix, ident) - return -} - -type keyData struct { - Name string - Priv string - Pub string -} - -var tmpl = template.Must(template.New("").Parse(`// +build !golint - -// This file contains statically created JWKs for tests created by gen.go - -package oidc - -import ( - "encoding/json" - - jose "gopkg.in/square/go-jose.v2" -) - -func mustLoadJWK(s string) jose.JSONWebKey { - var jwk jose.JSONWebKey - if err := json.Unmarshal([]byte(s), &jwk); err != nil { - panic(err) - } - return jwk -} - -var ( -{{- range $i, $key := .Keys }} - testKey{{ $key.Name }} = mustLoadJWK(` + "`" + `{{ $key.Pub }}` + "`" + `) - testKey{{ $key.Name }}_Priv = mustLoadJWK(` + "`" + `{{ $key.Priv }}` + "`" + `) -{{ end -}} -) -`)) - -func main() { - var tmplData struct { - Keys []keyData - } - for _, k := range keys { - for i := 0; i < 4; i++ { - log.Printf("generating %s", k.name) - priv, pub, err := newJWK(k, "\t", "\t") - if err != nil { - log.Fatal(err) - } - name := fmt.Sprintf("%s_%d", k.name, i) - - tmplData.Keys = append(tmplData.Keys, keyData{ - Name: name, - Priv: string(priv), - Pub: string(pub), - }) - } - } - - buff := new(bytes.Buffer) - if err := tmpl.Execute(buff, tmplData); err != nil { - log.Fatalf("excuting template: %v", err) - } - - if err := ioutil.WriteFile("jose_test.go", buff.Bytes(), 0644); err != nil { - log.Fatal(err) - } -} diff --git a/vendor/github.com/coreos/go-oidc/jwks.go b/vendor/github.com/coreos/go-oidc/jwks.go index 0f3cf0c9..e6a82c84 100644 --- a/vendor/github.com/coreos/go-oidc/jwks.go +++ b/vendor/github.com/coreos/go-oidc/jwks.go @@ -2,7 +2,7 @@ package oidc import ( "context" - "encoding/json" + "errors" "fmt" "io/ioutil" "net/http" @@ -23,6 +23,20 @@ import ( // updated. const keysExpiryDelta = 30 * time.Second +// NewRemoteKeySet returns a KeySet that can validate JSON web tokens by using HTTP +// GETs to fetch JSON web token sets hosted at a remote URL. This is automatically +// used by NewProvider using the URLs returned by OpenID Connect discovery, but is +// exposed for providers that don't support discovery or to prevent round trips to the +// discovery URL. +// +// The returned KeySet is a long lived verifier that caches keys based on cache-control +// headers. Reuse a common remote key set instead of creating new ones as needed. +// +// The behavior of the returned KeySet is undefined once the context is canceled. +func NewRemoteKeySet(ctx context.Context, jwksURL string) KeySet { + return newRemoteKeySet(ctx, jwksURL, time.Now) +} + func newRemoteKeySet(ctx context.Context, jwksURL string, now func() time.Time) *remoteKeySet { if now == nil { now = time.Now @@ -38,147 +52,168 @@ type remoteKeySet struct { // guard all other fields mu sync.Mutex - // inflightCtx suppresses parallel execution of updateKeys and allows + // inflight suppresses parallel execution of updateKeys and allows // multiple goroutines to wait for its result. - // Its Err() method returns any errors encountered during updateKeys. - // - // If nil, there is no inflight updateKeys request. - inflightCtx *inflight + inflight *inflight // A set of cached keys and their expiry. cachedKeys []jose.JSONWebKey expiry time.Time } -// inflight is used to wait on some in-flight request from multiple goroutines +// inflight is used to wait on some in-flight request from multiple goroutines. type inflight struct { - done chan struct{} + doneCh chan struct{} + + keys []jose.JSONWebKey err error } -// Done returns a channel that is closed when the inflight request finishes. -func (i *inflight) Done() <-chan struct{} { - return i.done +func newInflight() *inflight { + return &inflight{doneCh: make(chan struct{})} } -// Err returns any error encountered during request execution. May be nil. -func (i *inflight) Err() error { - return i.err +// wait returns a channel that multiple goroutines can receive on. Once it returns +// a value, the inflight request is done and result() can be inspected. +func (i *inflight) wait() <-chan struct{} { + return i.doneCh } -// Cancel signals completion of the inflight request with error err. -// Must be called only once for particular inflight instance. -func (i *inflight) Cancel(err error) { +// done can only be called by a single goroutine. It records the result of the +// inflight request and signals other goroutines that the result is safe to +// inspect. +func (i *inflight) done(keys []jose.JSONWebKey, err error) { + i.keys = keys i.err = err - close(i.done) + close(i.doneCh) } -func (r *remoteKeySet) keysWithIDFromCache(keyIDs []string) ([]jose.JSONWebKey, bool) { - r.mu.Lock() - keys, expiry := r.cachedKeys, r.expiry - r.mu.Unlock() +// result cannot be called until the wait() channel has returned a value. +func (i *inflight) result() ([]jose.JSONWebKey, error) { + return i.keys, i.err +} - // Have the keys expired? - if expiry.Add(keysExpiryDelta).Before(r.now()) { - return nil, false +func (r *remoteKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) { + jws, err := jose.ParseSigned(jwt) + if err != nil { + return nil, fmt.Errorf("oidc: malformed jwt: %v", err) + } + return r.verify(ctx, jws) +} + +func (r *remoteKeySet) verify(ctx context.Context, jws *jose.JSONWebSignature) ([]byte, error) { + // We don't support JWTs signed with multiple signatures. + keyID := "" + for _, sig := range jws.Signatures { + keyID = sig.Header.KeyID + break } - var signingKeys []jose.JSONWebKey + keys, expiry := r.keysFromCache() + + // Don't check expiry yet. This optimizes for when the provider is unavailable. for _, key := range keys { - if contains(keyIDs, key.KeyID) { - signingKeys = append(signingKeys, key) + if keyID == "" || key.KeyID == keyID { + if payload, err := jws.Verify(&key); err == nil { + return payload, nil + } } } - if len(signingKeys) == 0 { - // Are the keys about to expire? - if r.now().Add(keysExpiryDelta).After(expiry) { - return nil, false - } + if !r.now().Add(keysExpiryDelta).After(expiry) { + // Keys haven't expired, don't refresh. + return nil, errors.New("failed to verify id token signature") } - return signingKeys, true + keys, err := r.keysFromRemote(ctx) + if err != nil { + return nil, fmt.Errorf("fetching keys %v", err) + } + + for _, key := range keys { + if keyID == "" || key.KeyID == keyID { + if payload, err := jws.Verify(&key); err == nil { + return payload, nil + } + } + } + return nil, errors.New("failed to verify id token signature") } -func (r *remoteKeySet) keysWithID(ctx context.Context, keyIDs []string) ([]jose.JSONWebKey, error) { - keys, ok := r.keysWithIDFromCache(keyIDs) - if ok { - return keys, nil + +func (r *remoteKeySet) keysFromCache() (keys []jose.JSONWebKey, expiry time.Time) { + r.mu.Lock() + defer r.mu.Unlock() + return r.cachedKeys, r.expiry +} + +// keysFromRemote syncs the key set from the remote set, records the values in the +// cache, and returns the key set. +func (r *remoteKeySet) keysFromRemote(ctx context.Context) ([]jose.JSONWebKey, error) { + // Need to lock to inspect the inflight request field. + r.mu.Lock() + // If there's not a current inflight request, create one. + if r.inflight == nil { + r.inflight = newInflight() + + // This goroutine has exclusive ownership over the current inflight + // request. It releases the resource by nil'ing the inflight field + // once the goroutine is done. + go func() { + // Sync keys and finish inflight when that's done. + keys, expiry, err := r.updateKeys() + + r.inflight.done(keys, err) + + // Lock to update the keys and indicate that there is no longer an + // inflight request. + r.mu.Lock() + defer r.mu.Unlock() + + if err == nil { + r.cachedKeys = keys + r.expiry = expiry + } + + // Free inflight so a different request can run. + r.inflight = nil + }() } - - var inflightCtx *inflight - func() { - r.mu.Lock() - defer r.mu.Unlock() - - // If there's not a current inflight request, create one. - if r.inflightCtx == nil { - inflightCtx := &inflight{make(chan struct{}), nil} - r.inflightCtx = inflightCtx - - go func() { - // TODO(ericchiang): Upstream Kubernetes request that we recover every time - // we spawn a goroutine, because panics in a goroutine will bring down the - // entire program. There's no way to recover from another goroutine's panic. - // - // Most users actually want to let the panic propagate and bring down the - // program because it implies some unrecoverable state. - // - // Add a context key to allow the recover behavior. - // - // See: https://github.com/coreos/go-oidc/issues/89 - - // Sync keys and close inflightCtx when that's done. - // Use the remoteKeySet's context instead of the requests context - // because a re-sync is unique to the keys set and will span multiple - // requests. - inflightCtx.Cancel(r.updateKeys(r.ctx)) - - r.mu.Lock() - defer r.mu.Unlock() - r.inflightCtx = nil - }() - } - - inflightCtx = r.inflightCtx - }() + inflight := r.inflight + r.mu.Unlock() select { case <-ctx.Done(): return nil, ctx.Err() - case <-inflightCtx.Done(): - if err := inflightCtx.Err(); err != nil { - return nil, err - } + case <-inflight.wait(): + return inflight.result() } - - // Since we've just updated keys, we don't care about the cache miss. - keys, _ = r.keysWithIDFromCache(keyIDs) - return keys, nil } -func (r *remoteKeySet) updateKeys(ctx context.Context) error { +func (r *remoteKeySet) updateKeys() ([]jose.JSONWebKey, time.Time, error) { req, err := http.NewRequest("GET", r.jwksURL, nil) if err != nil { - return fmt.Errorf("oidc: can't create request: %v", err) + return nil, time.Time{}, fmt.Errorf("oidc: can't create request: %v", err) } - resp, err := doRequest(ctx, req) + resp, err := doRequest(r.ctx, req) if err != nil { - return fmt.Errorf("oidc: get keys failed %v", err) + return nil, time.Time{}, fmt.Errorf("oidc: get keys failed %v", err) } defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) if err != nil { - return fmt.Errorf("oidc: read response body: %v", err) + return nil, time.Time{}, fmt.Errorf("unable to read response body: %v", err) } + if resp.StatusCode != http.StatusOK { - return fmt.Errorf("oidc: get keys failed: %s %s", resp.Status, body) + return nil, time.Time{}, fmt.Errorf("oidc: get keys failed: %s %s", resp.Status, body) } var keySet jose.JSONWebKeySet - if err := json.Unmarshal(body, &keySet); err != nil { - return fmt.Errorf("oidc: failed to decode keys: %v %s", err, body) + err = unmarshalResp(resp, body, &keySet) + if err != nil { + return nil, time.Time{}, fmt.Errorf("oidc: failed to decode keys: %v %s", err, body) } // If the server doesn't provide cache control headers, assume the @@ -189,11 +224,5 @@ func (r *remoteKeySet) updateKeys(ctx context.Context) error { if err == nil && e.After(expiry) { expiry = e } - - r.mu.Lock() - defer r.mu.Unlock() - r.cachedKeys = keySet.Keys - r.expiry = expiry - - return nil + return keySet.Keys, expiry, nil } diff --git a/vendor/github.com/coreos/go-oidc/oidc.go b/vendor/github.com/coreos/go-oidc/oidc.go index a14dca0c..c82ba46b 100644 --- a/vendor/github.com/coreos/go-oidc/oidc.go +++ b/vendor/github.com/coreos/go-oidc/oidc.go @@ -3,10 +3,15 @@ package oidc import ( "context" + "crypto/sha256" + "crypto/sha512" + "encoding/base64" "encoding/json" "errors" "fmt" + "hash" "io/ioutil" + "mime" "net/http" "strings" "time" @@ -30,6 +35,11 @@ const ( ScopeOfflineAccess = "offline_access" ) +var ( + errNoAtHash = errors.New("id token did not have an access token hash") + errInvalidAtHash = errors.New("access token hash does not match value in ID token") +) + // ClientContext returns a new Context that carries the provided HTTP client. // // This method sets the same context key used by the golang.org/x/oauth2 package, @@ -63,7 +73,7 @@ type Provider struct { // Raw claims returned by the server. rawClaims []byte - remoteKeySet *remoteKeySet + remoteKeySet KeySet } type cachedKeys struct { @@ -93,18 +103,23 @@ func NewProvider(ctx context.Context, issuer string) (*Provider, error) { if err != nil { return nil, err } + defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, err + return nil, fmt.Errorf("unable to read response body: %v", err) } + if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("%s: %s", resp.Status, body) } - defer resp.Body.Close() + var p providerJSON - if err := json.Unmarshal(body, &p); err != nil { + err = unmarshalResp(resp, body, &p) + if err != nil { return nil, fmt.Errorf("oidc: failed to decode provider discovery object: %v", err) } + if p.Issuer != issuer { return nil, fmt.Errorf("oidc: issuer did not match the issuer returned by provider, expected %q got %q", issuer, p.Issuer) } @@ -114,7 +129,7 @@ func NewProvider(ctx context.Context, issuer string) (*Provider, error) { tokenURL: p.TokenURL, userInfoURL: p.UserInfoURL, rawClaims: body, - remoteKeySet: newRemoteKeySet(ctx, p.JWKSURL, time.Now), + remoteKeySet: NewRemoteKeySet(ctx, p.JWKSURL), }, nil } @@ -232,9 +247,18 @@ type IDToken struct { // Initial nonce provided during the authentication redirect. // - // If present, this package ensures this is a valid nonce. + // This package does NOT provided verification on the value of this field + // and it's the user's responsibility to ensure it contains a valid value. Nonce string + // at_hash claim, if set in the ID token. Callers can verify an access token + // that corresponds to the ID token using the VerifyAccessToken method. + AccessTokenHash string + + // signature algorithm used for ID token, needed to compute a verification hash of an + // access token + sigAlgorithm string + // Raw payload of the id_token. claims []byte } @@ -260,6 +284,34 @@ func (i *IDToken) Claims(v interface{}) error { return json.Unmarshal(i.claims, v) } +// VerifyAccessToken verifies that the hash of the access token that corresponds to the iD token +// matches the hash in the id token. It returns an error if the hashes don't match. +// It is the caller's responsibility to ensure that the optional access token hash is present for the ID token +// before calling this method. See https://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken +func (i *IDToken) VerifyAccessToken(accessToken string) error { + if i.AccessTokenHash == "" { + return errNoAtHash + } + var h hash.Hash + switch i.sigAlgorithm { + case RS256, ES256, PS256: + h = sha256.New() + case RS384, ES384, PS384: + h = sha512.New384() + case RS512, ES512, PS512: + h = sha512.New() + default: + return fmt.Errorf("oidc: unsupported signing algorithm %q", i.sigAlgorithm) + } + h.Write([]byte(accessToken)) // hash documents that Write will never return an error + sum := h.Sum(nil)[:h.Size()/2] + actual := base64.RawURLEncoding.EncodeToString(sum) + if actual != i.AccessTokenHash { + return errInvalidAtHash + } + return nil +} + type idToken struct { Issuer string `json:"iss"` Subject string `json:"sub"` @@ -267,6 +319,7 @@ type idToken struct { Expiry jsonTime `json:"exp"` IssuedAt jsonTime `json:"iat"` Nonce string `json:"nonce"` + AtHash string `json:"at_hash"` } type audience []string @@ -285,13 +338,6 @@ func (a *audience) UnmarshalJSON(b []byte) error { return nil } -func (a audience) MarshalJSON() ([]byte, error) { - if len(a) == 1 { - return json.Marshal(a[0]) - } - return json.Marshal([]string(a)) -} - type jsonTime time.Time func (j *jsonTime) UnmarshalJSON(b []byte) error { @@ -314,6 +360,15 @@ func (j *jsonTime) UnmarshalJSON(b []byte) error { return nil } -func (j jsonTime) MarshalJSON() ([]byte, error) { - return json.Marshal(time.Time(j).Unix()) +func unmarshalResp(r *http.Response, body []byte, v interface{}) error { + err := json.Unmarshal(body, &v) + if err == nil { + return nil + } + ct := r.Header.Get("Content-Type") + mediaType, _, parseErr := mime.ParseMediaType(ct) + if parseErr == nil && mediaType == "application/json" { + return fmt.Errorf("got Content-Type = application/json, but could not unmarshal as JSON: %v", err) + } + return fmt.Errorf("expected Content-Type = application/json, got %q: %v", ct, err) } diff --git a/vendor/github.com/coreos/go-oidc/test b/vendor/github.com/coreos/go-oidc/test index bbb5ed39..b262d0e7 100644 --- a/vendor/github.com/coreos/go-oidc/test +++ b/vendor/github.com/coreos/go-oidc/test @@ -11,5 +11,6 @@ LINTABLE=$( go list -tags=golint -f ' go test -v -i -race github.com/coreos/go-oidc/... go test -v -race github.com/coreos/go-oidc/... -golint $LINTABLE +golint -set_exit_status $LINTABLE go vet github.com/coreos/go-oidc/... +go build -v ./example/... diff --git a/vendor/github.com/coreos/go-oidc/verify.go b/vendor/github.com/coreos/go-oidc/verify.go index 2b67aa9c..24ff73ee 100644 --- a/vendor/github.com/coreos/go-oidc/verify.go +++ b/vendor/github.com/coreos/go-oidc/verify.go @@ -19,13 +19,54 @@ const ( issuerGoogleAccountsNoScheme = "accounts.google.com" ) +// KeySet is a set of publc JSON Web Keys that can be used to validate the signature +// of JSON web tokens. This is expected to be backed by a remote key set through +// provider metadata discovery or an in-memory set of keys delivered out-of-band. +type KeySet interface { + // VerifySignature parses the JSON web token, verifies the signature, and returns + // the raw payload. Header and claim fields are validated by other parts of the + // package. For example, the KeySet does not need to check values such as signature + // algorithm, issuer, and audience since the IDTokenVerifier validates these values + // independently. + // + // If VerifySignature makes HTTP requests to verify the token, it's expected to + // use any HTTP client associated with the context through ClientContext. + VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) +} + // IDTokenVerifier provides verification for ID Tokens. type IDTokenVerifier struct { - keySet *remoteKeySet + keySet KeySet config *Config issuer string } +// NewVerifier returns a verifier manually constructed from a key set and issuer URL. +// +// It's easier to use provider discovery to construct an IDTokenVerifier than creating +// one directly. This method is intended to be used with provider that don't support +// metadata discovery, or avoiding round trips when the key set URL is already known. +// +// This constructor can be used to create a verifier directly using the issuer URL and +// JSON Web Key Set URL without using discovery: +// +// keySet := oidc.NewRemoteKeySet(ctx, "https://www.googleapis.com/oauth2/v3/certs") +// verifier := oidc.NewVerifier("https://accounts.google.com", keySet, config) +// +// Since KeySet is an interface, this constructor can also be used to supply custom +// public key sources. For example, if a user wanted to supply public keys out-of-band +// and hold them statically in-memory: +// +// // Custom KeySet implementation. +// keySet := newStatisKeySet(publicKeys...) +// +// // Verifier uses the custom KeySet implementation. +// verifier := oidc.NewVerifier("https://auth.example.com", keySet, config) +// +func NewVerifier(issuerURL string, keySet KeySet, config *Config) *IDTokenVerifier { + return &IDTokenVerifier{keySet: keySet, config: config, issuer: issuerURL} +} + // Config is the configuration for an IDTokenVerifier. type Config struct { // Expected audience of the token. For a majority of the cases this is expected to be @@ -34,12 +75,6 @@ type Config struct { // // If not provided, users must explicitly set SkipClientIDCheck. ClientID string - // Method to verify the ID Token nonce. If a nonce is present and this method - // is nil, users must explicitly set SkipNonceCheck. - // - // If the ID Token nonce is empty, for example if the client didn't provide a nonce in - // the initial redirect, this may be nil. - ClaimNonce func(nonce string) error // If specified, only this set of algorithms may be used to sign the JWT. // // Since many providers only support RS256, SupportedSigningAlgs defaults to this value. @@ -49,8 +84,6 @@ type Config struct { SkipClientIDCheck bool // If true, token expiry is not checked. SkipExpiryCheck bool - // If true, nonce claim is not checked. Must be true if ClaimNonce field is empty. - SkipNonceCheck bool // Time function to check Token expiry. Defaults to time.Now Now func() time.Time @@ -61,21 +94,7 @@ type Config struct { // The returned IDTokenVerifier is tied to the Provider's context and its behavior is // undefined once the Provider's context is canceled. func (p *Provider) Verifier(config *Config) *IDTokenVerifier { - - return newVerifier(p.remoteKeySet, config, p.issuer) -} - -func newVerifier(keySet *remoteKeySet, config *Config, issuer string) *IDTokenVerifier { - // If SupportedSigningAlgs is empty defaults to only support RS256. - if len(config.SupportedSigningAlgs) == 0 { - config.SupportedSigningAlgs = []string{RS256} - } - - return &IDTokenVerifier{ - keySet: keySet, - config: config, - issuer: issuer, - } + return NewVerifier(p.issuer, p.remoteKeySet, config) } func parseJWT(p string) ([]byte, error) { @@ -102,6 +121,8 @@ func contains(sli []string, ele string) bool { // Verify parses a raw ID Token, verifies it's been signed by the provider, preforms // any additional checks depending on the Config, and returns the payload. // +// Verify does NOT do nonce validation, which is the callers responsibility. +// // See: https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation // // oauth2Token, err := oauth2Config.Exchange(ctx, r.URL.Query().Get("code")) @@ -120,7 +141,7 @@ func contains(sli []string, ele string) bool { func (v *IDTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*IDToken, error) { jws, err := jose.ParseSigned(rawIDToken) if err != nil { - return nil, fmt.Errorf("oidc: mallformed jwt: %v", err) + return nil, fmt.Errorf("oidc: malformed jwt: %v", err) } // Throw out tokens with invalid claims before trying to verify the token. This lets @@ -135,13 +156,14 @@ func (v *IDTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*IDTok } t := &IDToken{ - Issuer: token.Issuer, - Subject: token.Subject, - Audience: []string(token.Audience), - Expiry: time.Time(token.Expiry), - IssuedAt: time.Time(token.IssuedAt), - Nonce: token.Nonce, - claims: payload, + Issuer: token.Issuer, + Subject: token.Subject, + Audience: []string(token.Audience), + Expiry: time.Time(token.Expiry), + IssuedAt: time.Time(token.IssuedAt), + Nonce: token.Nonce, + AccessTokenHash: token.AtHash, + claims: payload, } // Check issuer. @@ -165,7 +187,7 @@ func (v *IDTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*IDTok return nil, fmt.Errorf("oidc: expected audience %q got %q", v.config.ClientID, t.Audience) } } else { - return nil, fmt.Errorf("oidc: Invalid configuration. ClientID must be provided or SkipClientIDCheck must be set.") + return nil, fmt.Errorf("oidc: invalid configuration, clientID must be provided or SkipClientIDCheck must be set") } } @@ -181,37 +203,29 @@ func (v *IDTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*IDTok } } - // If a set of required algorithms has been provided, ensure that the signatures use those. - var keyIDs, gotAlgs []string - for _, sig := range jws.Signatures { - if len(v.config.SupportedSigningAlgs) == 0 || contains(v.config.SupportedSigningAlgs, sig.Header.Algorithm) { - keyIDs = append(keyIDs, sig.Header.KeyID) - } else { - gotAlgs = append(gotAlgs, sig.Header.Algorithm) - } - } - if len(keyIDs) == 0 { - return nil, fmt.Errorf("oidc: no signatures use a supported algorithm, expected %q got %q", v.config.SupportedSigningAlgs, gotAlgs) + switch len(jws.Signatures) { + case 0: + return nil, fmt.Errorf("oidc: id token not signed") + case 1: + default: + return nil, fmt.Errorf("oidc: multiple signatures on id token not supported") } - // Get keys from the remote key set. This may trigger a re-sync. - keys, err := v.keySet.keysWithID(ctx, keyIDs) + sig := jws.Signatures[0] + supportedSigAlgs := v.config.SupportedSigningAlgs + if len(supportedSigAlgs) == 0 { + supportedSigAlgs = []string{RS256} + } + + if !contains(supportedSigAlgs, sig.Header.Algorithm) { + return nil, fmt.Errorf("oidc: id token signed with unsupported algorithm, expected %q got %q", supportedSigAlgs, sig.Header.Algorithm) + } + + t.sigAlgorithm = sig.Header.Algorithm + + gotPayload, err := v.keySet.VerifySignature(ctx, rawIDToken) if err != nil { - return nil, fmt.Errorf("oidc: get keys for id token: %v", err) - } - if len(keys) == 0 { - return nil, fmt.Errorf("oidc: no keys match signature ID(s) %q", keyIDs) - } - - // Try to use a key to validate the signature. - var gotPayload []byte - for _, key := range keys { - if p, err := jws.Verify(&key); err == nil { - gotPayload = p - } - } - if len(gotPayload) == 0 { - return nil, fmt.Errorf("oidc: failed to verify id token") + return nil, fmt.Errorf("failed to verify signature: %v", err) } // Ensure that the payload returned by the square actually matches the payload parsed earlier. @@ -219,19 +233,6 @@ func (v *IDTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*IDTok return nil, errors.New("oidc: internal error, payload parsed did not match previous payload") } - // Check the nonce after we've verified the token. We don't want to allow unverified - // payloads to trigger a nonce lookup. - // If SkipNonceCheck is not set ClaimNonce cannot be Nil. - if !v.config.SkipNonceCheck && t.Nonce != "" { - if v.config.ClaimNonce != nil { - if err := v.config.ClaimNonce(t.Nonce); err != nil { - return nil, err - } - } else { - return nil, fmt.Errorf("oidc: Invalid configuration. ClaimNonce must be provided or SkipNonceCheck must be set.") - } - } - return t, nil } diff --git a/vendor/modules.txt b/vendor/modules.txt index 7beaff88..6608d218 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -11,7 +11,7 @@ github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes github.com/coreos/etcd/etcdserver/etcdserverpb github.com/coreos/etcd/mvcc/mvccpb github.com/coreos/etcd/pkg/tlsutil -# github.com/coreos/go-oidc v0.0.0-20170307191026-be73733bb8cc +# github.com/coreos/go-oidc v2.0.0+incompatible github.com/coreos/go-oidc # github.com/felixge/httpsnoop v1.0.0 github.com/felixge/httpsnoop From 46f5726d119238a70eee82a7286d7d0193998bba Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Thu, 20 Jun 2019 13:15:59 -0400 Subject: [PATCH 06/11] Use oidc.Verifier to verify tokens --- server/handlers.go | 93 +++++++++---------------------------------- server/oauth2.go | 39 ++++++++++++++++++ server/oauth2_test.go | 87 ++++++++++++++++++++++++++++++++++++++++ server/server_test.go | 10 +++++ 4 files changed, 154 insertions(+), 75 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 573542c0..a931d23a 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -2,7 +2,6 @@ package server import ( "context" - "encoding/base64" "encoding/json" "errors" "fmt" @@ -15,6 +14,7 @@ import ( "sync" "time" + oidc "github.com/coreos/go-oidc" "github.com/gorilla/mux" jose "gopkg.in/square/go-jose.v2" @@ -23,10 +23,6 @@ import ( "github.com/dexidp/dex/storage" ) -var ( - errTokenExpired = errors.New("token has expired") -) - // newHealthChecker returns the healthz handler. The handler runs until the // provided context is canceled. func (s *Server) newHealthChecker(ctx context.Context) http.Handler { @@ -1055,84 +1051,31 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie } func (s *Server) handleUserInfo(w http.ResponseWriter, r *http.Request) { - authorization := r.Header.Get("Authorization") - parts := strings.Fields(authorization) + const prefix = "Bearer " - if len(parts) != 2 || !strings.EqualFold(parts[0], "bearer") { - msg := "invalid authorization header" - w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer realm="dex", error="%s", error_description="%s"`, errInvalidRequest, msg)) - s.tokenErrHelper(w, errInvalidRequest, msg, http.StatusBadRequest) + auth := r.Header.Get("authorization") + if len(auth) < len(prefix) || !strings.EqualFold(prefix, auth[:len(prefix)]) { + w.Header().Set("WWW-Authenticate", "Bearer") + s.tokenErrHelper(w, errAccessDenied, "Invalid bearer token.", http.StatusUnauthorized) + return + } + rawIDToken := auth[len(prefix):] + + verifier := oidc.NewVerifier(s.issuerURL.String(), &storageKeySet{s.storage}, &oidc.Config{SkipClientIDCheck: true}) + idToken, err := verifier.Verify(r.Context(), rawIDToken) + if err != nil { + s.tokenErrHelper(w, errAccessDenied, err.Error(), http.StatusForbidden) return } - token := parts[1] - - verified, err := s.verify(token) - if err != nil { - if err == errTokenExpired { - s.tokenErrHelper(w, errAccessDenied, err.Error(), http.StatusUnauthorized) - return - } - s.tokenErrHelper(w, errInvalidRequest, err.Error(), http.StatusBadRequest) + var claims json.RawMessage + if err := idToken.Claims(&claims); err != nil { + s.tokenErrHelper(w, errServerError, err.Error(), http.StatusInternalServerError) return } w.Header().Set("Content-Type", "application/json") - w.Write(verified) -} - -func (s *Server) verify(token string) ([]byte, error) { - keys, err := s.storage.GetKeys() - if err != nil { - return nil, fmt.Errorf("failed to get keys: %v", err) - } - - if keys.SigningKey == nil { - return nil, fmt.Errorf("no private keys found") - } - - object, err := jose.ParseSigned(token) - if err != nil { - return nil, fmt.Errorf("unable to parse signed message") - } - - parts := strings.Split(token, ".") - if len(parts) != 3 { - return nil, fmt.Errorf("compact JWS format must have three parts") - } - - payload, err := base64.RawURLEncoding.DecodeString(parts[1]) - if err != nil { - return nil, err - } - - // TODO: check other claims - var tokenInfo struct { - Expiry int64 `json:"exp"` - } - - if err := json.Unmarshal(payload, &tokenInfo); err != nil { - return nil, err - } - - if tokenInfo.Expiry < s.now().Unix() { - return nil, errTokenExpired - } - - var allKeys []*jose.JSONWebKey - - allKeys = append(allKeys, keys.SigningKeyPub) - for _, key := range keys.VerificationKeys { - allKeys = append(allKeys, key.PublicKey) - } - - for _, pubKey := range allKeys { - verified, err := object.Verify(pubKey) - if err == nil { - return verified, nil - } - } - return nil, errors.New("unable to verify jwt") + w.Write(claims) } func (s *Server) writeAccessToken(w http.ResponseWriter, idToken, accessToken, refreshToken string, expiry time.Time) { diff --git a/server/oauth2.go b/server/oauth2.go index 26d152f4..9effc174 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -1,6 +1,7 @@ package server import ( + "context" "crypto/ecdsa" "crypto/elliptic" "crypto/rsa" @@ -566,3 +567,41 @@ func validateRedirectURI(client storage.Client, redirectURI string) bool { host, _, err := net.SplitHostPort(u.Host) return err == nil && host == "localhost" } + +// storageKeySet implements the oidc.KeySet interface backed by Dex storage +type storageKeySet struct { + storage.Storage +} + +func (s *storageKeySet) VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) { + jws, err := jose.ParseSigned(jwt) + if err != nil { + return nil, err + } + + keyID := "" + for _, sig := range jws.Signatures { + keyID = sig.Header.KeyID + break + } + + skeys, err := s.Storage.GetKeys() + if err != nil { + return nil, err + } + + keys := []*jose.JSONWebKey{skeys.SigningKeyPub} + for _, vk := range skeys.VerificationKeys { + keys = append(keys, vk.PublicKey) + } + + for _, key := range keys { + if keyID == "" || key.KeyID == keyID { + if payload, err := jws.Verify(key); err == nil { + return payload, nil + } + } + } + + return nil, errors.New("failed to verify id token signature") +} diff --git a/server/oauth2_test.go b/server/oauth2_test.go index 8cad77a8..bb8d2723 100644 --- a/server/oauth2_test.go +++ b/server/oauth2_test.go @@ -2,6 +2,8 @@ package server import ( "context" + "crypto/rand" + "crypto/rsa" "net/http" "net/http/httptest" "net/url" @@ -11,6 +13,7 @@ import ( jose "gopkg.in/square/go-jose.v2" "github.com/dexidp/dex/storage" + "github.com/dexidp/dex/storage/memory" ) func TestParseAuthorizationRequest(t *testing.T) { @@ -259,3 +262,87 @@ func TestValidRedirectURI(t *testing.T) { } } } + +func TestStorageKeySet(t *testing.T) { + s := memory.New(logger) + if err := s.UpdateKeys(func(keys storage.Keys) (storage.Keys, error) { + keys.SigningKey = &jose.JSONWebKey{ + Key: testKey, + KeyID: "testkey", + Algorithm: "RS256", + Use: "sig", + } + keys.SigningKeyPub = &jose.JSONWebKey{ + Key: testKey.Public(), + KeyID: "testkey", + Algorithm: "RS256", + Use: "sig", + } + return keys, nil + }); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + tokenGenerator func() (jwt string, err error) + wantErr bool + }{ + { + name: "valid token", + tokenGenerator: func() (string, error) { + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: testKey}, nil) + if err != nil { + return "", err + } + + jws, err := signer.Sign([]byte("payload")) + if err != nil { + return "", err + } + + return jws.CompactSerialize() + }, + wantErr: false, + }, + { + name: "token signed by different key", + tokenGenerator: func() (string, error) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return "", err + } + + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: key}, nil) + if err != nil { + return "", err + } + + jws, err := signer.Sign([]byte("payload")) + if err != nil { + return "", err + } + + return jws.CompactSerialize() + }, + wantErr: true, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + jwt, err := tc.tokenGenerator() + if err != nil { + t.Fatal(err) + } + + keySet := &storageKeySet{s} + + _, err = keySet.VerifySignature(context.Background(), jwt) + if (err != nil && !tc.wantErr) || (err == nil && tc.wantErr) { + t.Fatalf("wantErr = %v, but got err = %v", tc.wantErr, err) + } + }) + } +} diff --git a/server/server_test.go b/server/server_test.go index 87c80fbb..12f29340 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -200,6 +200,16 @@ func TestOAuth2CodeFlow(t *testing.T) { return nil }, }, + { + name: "fetch userinfo", + handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { + _, err := p.UserInfo(ctx, config.TokenSource(ctx, token)) + if err != nil { + return fmt.Errorf("failed to fetch userinfo: %v", err) + } + return nil + }, + }, { name: "verify id token and oauth2 token expiry", handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { From 840065faaf0ff71d74802f044c8bb8879be33212 Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Mon, 24 Jun 2019 09:39:54 -0400 Subject: [PATCH 07/11] Assert something about the returned userinfo --- server/server_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/server_test.go b/server/server_test.go index 12f29340..581eb26a 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -203,10 +203,13 @@ func TestOAuth2CodeFlow(t *testing.T) { { name: "fetch userinfo", handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { - _, err := p.UserInfo(ctx, config.TokenSource(ctx, token)) + ui, err := p.UserInfo(ctx, config.TokenSource(ctx, token)) if err != nil { return fmt.Errorf("failed to fetch userinfo: %v", err) } + if conn.Identity.Email != ui.Email { + return fmt.Errorf("expected email to be %v, got %v", conn.Identity.Email, ui.Email) + } return nil }, }, From 21174c06a18e357c479aa1d4f4353fd7eae004c5 Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Mon, 24 Jun 2019 09:42:44 -0400 Subject: [PATCH 08/11] Remove comment We have a story around user info now --- server/handlers.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index a931d23a..7f942391 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -1079,9 +1079,6 @@ func (s *Server) handleUserInfo(w http.ResponseWriter, r *http.Request) { } func (s *Server) writeAccessToken(w http.ResponseWriter, idToken, accessToken, refreshToken string, expiry time.Time) { - // TODO(ericchiang): figure out an access token story and support the user info - // endpoint. For now use a random value so no one depends on the access_token - // holding a specific structure. resp := struct { AccessToken string `json:"access_token"` TokenType string `json:"token_type"` From 8959dc42750e1f574db74eec9f0fcc61b39b83ea Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Mon, 24 Jun 2019 09:43:12 -0400 Subject: [PATCH 09/11] ctx is not used --- server/oauth2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/oauth2.go b/server/oauth2.go index 9effc174..68a72f66 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -573,7 +573,7 @@ type storageKeySet struct { storage.Storage } -func (s *storageKeySet) VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) { +func (s *storageKeySet) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) { jws, err := jose.ParseSigned(jwt) if err != nil { return nil, err From 59b6595c37208beabbae4a05ead29d0529f5598b Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Tue, 25 Jun 2019 12:17:03 -0400 Subject: [PATCH 10/11] userinfo_endpoint is required --- server/server_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/server_test.go b/server/server_test.go index 581eb26a..1cc145a0 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -147,6 +147,7 @@ func TestDiscovery(t *testing.T) { "authorization_endpoint", "token_endpoint", "jwks_uri", + "userinfo_endpoint", } for _, field := range required { if _, ok := got[field]; !ok { From 5b66bf05c8a4a44e504d1021b8e9ba1926dc4329 Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Thu, 27 Jun 2019 19:12:18 -0400 Subject: [PATCH 11/11] Fixed shadowed variable declaration --- server/handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/handlers.go b/server/handlers.go index 7f942391..e7e15c27 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -600,7 +600,7 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe implicitOrHybrid = true var err error - accessToken, err := s.newAccessToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce, authReq.ConnectorID) + accessToken, err = s.newAccessToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce, authReq.ConnectorID) if err != nil { s.logger.Errorf("failed to create new access token: %v", err) s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)