Skip to content

Commit

Permalink
Set and verify a nonce with OIDC
Browse files Browse the repository at this point in the history
  • Loading branch information
Nick Meves committed Dec 25, 2020
1 parent 89e0a77 commit e99479d
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 7 deletions.
17 changes: 17 additions & 0 deletions pkg/apis/sessions/session_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package sessions

import (
"bytes"
"crypto/hmac"
"crypto/sha256"
"errors"
"fmt"
"io"
Expand All @@ -24,6 +26,8 @@ type SessionState struct {
IDToken string `msgpack:"it,omitempty"`
RefreshToken string `msgpack:"rt,omitempty"`

Nonce []byte `msgpack:"n,omitempty"`

Email string `msgpack:"e,omitempty"`
User string `msgpack:"u,omitempty"`
Groups []string `msgpack:"g,omitempty"`
Expand Down Expand Up @@ -55,6 +59,9 @@ func (s *SessionState) String() string {
if s.IDToken != "" {
o += " id_token:true"
}
if s.Nonce != nil {
o += "nonce:true"
}
if s.CreatedAt != nil && !s.CreatedAt.IsZero() {
o += fmt.Sprintf(" created:%s", s.CreatedAt)
}
Expand Down Expand Up @@ -100,6 +107,16 @@ func (s *SessionState) GetClaim(claim string) []string {
}
}

func (s *SessionState) AddNonce(nonce string) {
nonceHMAC := sha256.Sum256([]byte(nonce))
s.Nonce = nonceHMAC[:]
}

func (s *SessionState) CheckNonce(nonce string) bool {
nonceHMAC := sha256.Sum256([]byte(nonce))
return hmac.Equal(s.Nonce, nonceHMAC[:])
}

// EncodeSessionState returns an encrypted, lz4 compressed, MessagePack encoded session
func (s *SessionState) EncodeSessionState(c encryption.Cipher, compress bool) ([]byte, error) {
packed, err := msgpack.Marshal(s)
Expand Down
66 changes: 59 additions & 7 deletions providers/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests"
"golang.org/x/oauth2"
Expand All @@ -16,12 +17,17 @@ import (
// OIDCProvider represents an OIDC based Identity Provider
type OIDCProvider struct {
*ProviderData

SkipNonce bool
}

// NewOIDCProvider initiates a new OIDCProvider
func NewOIDCProvider(p *ProviderData) *OIDCProvider {
p.ProviderName = "OpenID Connect"
return &OIDCProvider{ProviderData: p}
return &OIDCProvider{
ProviderData: p,
SkipNonce: true,
}
}

var _ Provider = (*OIDCProvider)(nil)
Expand All @@ -41,12 +47,33 @@ func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code string) (*s
},
RedirectURL: redirectURL,
}
token, err := c.Exchange(ctx, code)

// Make a nonce even if `SkipNonce` is true to force a validation
// failure and reauthentication if `SkipNonce` is switched to false.
nonce, err := encryption.Nonce()
if err != nil {
return nil, fmt.Errorf("error creating nonce: %v", err)
}

var opts []oauth2.AuthCodeOption
if !p.SkipNonce {
opts = append(opts, oauth2.SetAuthURLParam("nonce", nonce))
}

token, err := c.Exchange(ctx, code, opts...)
if err != nil {
return nil, fmt.Errorf("token exchange failed: %v", err)
}

return p.createSession(ctx, token, false)
ss, err := p.createSession(ctx, token, nonce, false)

// TODO (@NickMeves): Do ValidateSession after Redeem in OAuthProxy
// Once all providers are refactored, make this pattern universal
if err == nil && !p.ValidateSession(ctx, ss) {
return nil, errors.New("id_token validation failed")
}

return ss, err
}

// EnrichSession is called after Redeem to allow providers to enrich session fields
Expand Down Expand Up @@ -109,8 +136,24 @@ func (p *OIDCProvider) enrichFromProfileURL(ctx context.Context, s *sessions.Ses

// ValidateSession checks that the session's IDToken is still valid
func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool {
_, err := p.Verifier.Verify(ctx, s.IDToken)
return err == nil
idToken, err := p.Verifier.Verify(ctx, s.IDToken)
if err != nil {
return false
}

if !p.SkipNonce {
claims, err := p.getClaims(idToken)
if err != nil {
logger.Errorf("id_token claims extraction failed: %v", err)
return false
}
if !s.CheckNonce(claims.Nonce) {
logger.Errorf("id_token nonce claim does not match the session nonce")
return false
}
}

return true
}

// RefreshSessionIfNeeded checks if the session has expired and uses the
Expand Down Expand Up @@ -153,7 +196,7 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi
return fmt.Errorf("failed to get token: %v", err)
}

newSession, err := p.createSession(ctx, token, true)
newSession, err := p.createSession(ctx, token, "", true)
if err != nil {
return fmt.Errorf("unable create new session state from response: %v", err)
}
Expand All @@ -162,6 +205,13 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi
// session will not contain an id token.
// If it doesn't it's probably better to retain the old one
if newSession.IDToken != "" {
// TODO (@NickMeves): Do ValidateSession after Refresh in OAuthProxy
// Once all providers are refactored, make this pattern universal
newSession.Nonce = s.Nonce
if !p.ValidateSession(ctx, newSession) {
return errors.New("refreshed id_token validation failed")
}

s.IDToken = newSession.IDToken
s.Email = newSession.Email
s.User = newSession.User
Expand Down Expand Up @@ -204,7 +254,7 @@ func (p *OIDCProvider) CreateSessionFromToken(ctx context.Context, token string)

// createSession takes an oauth2.Token and creates a SessionState from it.
// It alters behavior if called from Redeem vs Refresh
func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, refresh bool) (*sessions.SessionState, error) {
func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, nonce string, refresh bool) (*sessions.SessionState, error) {
idToken, err := p.verifyIDToken(ctx, token)
if err != nil {
switch err {
Expand All @@ -231,5 +281,7 @@ func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, r
ss.CreatedAt = &created
ss.ExpiresOn = &token.Expiry

ss.AddNonce(nonce)

return ss, nil
}
1 change: 1 addition & 0 deletions providers/provider_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ type OIDCClaims struct {
Email string `json:"-"`
Groups []string `json:"-"`
Verified *bool `json:"email_verified"`
Nonce string `json:"nonce"`

raw map[string]interface{}
}
Expand Down

0 comments on commit e99479d

Please sign in to comment.