Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

identity/oidc: Adds proof key for code exchange (PKCE) support #13917

Merged
merged 14 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/13917.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
identity/oidc: Adds proof key for code exchange (PKCE) support to OIDC providers.
```
19 changes: 19 additions & 0 deletions vault/external_tests/identity/oidc_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ func TestOIDC_Auth_Code_Flow_CAP_Client(t *testing.T) {
require.NoError(t, err)
defer p.Done()

// Create the client-side PKCE code verifier
v, err := oidc.NewCodeVerifier()
require.NoError(t, err)

type args struct {
useStandby bool
options []oidc.Option
Expand Down Expand Up @@ -255,6 +259,21 @@ func TestOIDC_Auth_Code_Flow_CAP_Client(t *testing.T) {
"auth_time": %d
}`, discovery.Issuer, clientID, entityID, expectedAuthTime),
},
{
name: "active: authorization code flow with Proof Key for Code Exchange (PKCE)",
args: args{
options: []oidc.Option{
oidc.WithScopes("openid"),
oidc.WithPKCE(v),
},
},
expected: fmt.Sprintf(`{
"iss": "%s",
"aud": "%s",
"sub": "%s",
"namespace": "root"
}`, discovery.Issuer, clientID, entityID),
},
{
name: "standby: authorization code flow with additional scopes",
args: args{
Expand Down
90 changes: 76 additions & 14 deletions vault/identity_store_oidc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ import (

const (
// OIDC-related constants
openIDScope = "openid"
scopesDelimiter = " "
accessTokenScopesMeta = "scopes"
accessTokenClientIDMeta = "client_id"
clientIDLength = 32
clientSecretLength = 64
clientSecretPrefix = "hvo_secret_"
openIDScope = "openid"
scopesDelimiter = " "
accessTokenScopesMeta = "scopes"
accessTokenClientIDMeta = "client_id"
clientIDLength = 32
clientSecretLength = 64
clientSecretPrefix = "hvo_secret_"
codeChallengeMethodPlain = "plain"
codeChallengeMethodS256 = "S256"

// Storage path constants
oidcProviderPrefix = "oidc_provider/"
Expand Down Expand Up @@ -127,13 +129,15 @@ type providerDiscovery struct {
}

type authCodeCacheEntry struct {
provider string
clientID string
entityID string
redirectURI string
nonce string
scopes []string
authTime time.Time
provider string
clientID string
entityID string
redirectURI string
nonce string
scopes []string
authTime time.Time
codeChallenge string
codeChallengeMethod string
}

func oidcProviderPaths(i *IdentityStore) []*framework.Path {
Expand Down Expand Up @@ -405,6 +409,14 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path {
Type: framework.TypeInt,
Description: "The allowable elapsed time in seconds since the last time the end-user was actively authenticated.",
},
"code_challenge": {
Type: framework.TypeString,
Description: "The code challenge derived from the code verifier.",
},
"code_challenge_method": {
Type: framework.TypeString,
Description: "The method that was used to derive the code challenge. The following methods are supported: 'S256', 'plain'. Defaults to 'plain'.",
austingebauer marked this conversation as resolved.
Show resolved Hide resolved
},
},
Operations: map[logical.Operation]framework.OperationHandler{
logical.ReadOperation: &framework.PathOperation{
Expand Down Expand Up @@ -443,6 +455,10 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path {
Description: "The callback location where the authentication response was sent.",
Required: true,
},
"code_verifier": {
Type: framework.TypeString,
Description: "The code verifier associated with the authorization code.",
},
// The client_id and client_secret are provided to the token endpoint via
// the client_secret_basic authentication method, which uses the HTTP Basic
// authentication scheme. See the OIDC spec for details at:
Expand Down Expand Up @@ -1561,6 +1577,33 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ
scopes: scopes,
}

// Validate the optional Proof Key for Code Exchange (PKCE) if a code
austingebauer marked this conversation as resolved.
Show resolved Hide resolved
// challenge and code challenge method are provided. See details at
// https://datatracker.ietf.org/doc/html/rfc7636.
if codeChallengeRaw, ok := d.GetOk("code_challenge"); ok {
codeChallenge := codeChallengeRaw.(string)

// Validate the code challenge method
codeChallengeMethod := d.Get("code_challenge_method").(string)
switch codeChallengeMethod {
case codeChallengeMethodPlain, codeChallengeMethodS256:
case "":
codeChallengeMethod = codeChallengeMethodPlain
austingebauer marked this conversation as resolved.
Show resolved Hide resolved
default:
return authResponse("", state, ErrAuthInvalidRequest, "invalid code_challenge_method")
}

// Validate the code challenge
if codeChallenge == "" {
austingebauer marked this conversation as resolved.
Show resolved Hide resolved
return authResponse("", state, ErrAuthInvalidRequest, "invalid code_challenge")
}

// Associate the code challenge and method with the authorization code.
// This will be used to verify the code verifier in the token exchange.
authCodeEntry.codeChallenge = codeChallenge
authCodeEntry.codeChallengeMethod = codeChallengeMethod
}

// Validate the optional max_age parameter to check if an active re-authentication
// of the user should occur. Re-authentication will be requested if the last time
// the token actively authenticated exceeds the given max_age requirement. Returning
Expand Down Expand Up @@ -1771,6 +1814,25 @@ func (i *IdentityStore) pathOIDCToken(ctx context.Context, req *logical.Request,
return tokenResponse(nil, ErrTokenInvalidRequest, "identity entity not authorized by client assignment")
}

// Validate the code verifier if a code challenge and code challenge
// method are associated with the authorization code. See details at
// https://datatracker.ietf.org/doc/html/rfc7636#section-4.6
if authCodeEntry.codeChallenge != "" && authCodeEntry.codeChallengeMethod != "" {
austingebauer marked this conversation as resolved.
Show resolved Hide resolved
codeVerifier := d.Get("code_verifier").(string)
if codeVerifier == "" {
return tokenResponse(nil, ErrTokenInvalidGrant, "invalid code_verifier for token exchange")
}

codeChallenge, err := computeCodeChallenge(codeVerifier, authCodeEntry.codeChallengeMethod)
if err != nil {
return tokenResponse(nil, ErrTokenInvalidGrant, err.Error())
}

if codeChallenge != authCodeEntry.codeChallenge {
austingebauer marked this conversation as resolved.
Show resolved Hide resolved
return tokenResponse(nil, ErrTokenInvalidGrant, "invalid code_verifier for token exchange")
}
}

// The access token is a Vault batch token with a policy that only
// provides access to the issuing provider's userinfo endpoint.
accessTokenIssuedAt := time.Now()
Expand Down
130 changes: 130 additions & 0 deletions vault/identity_store_oidc_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,104 @@ func TestOIDC_Path_OIDC_Token(t *testing.T) {
},
wantErr: ErrTokenInvalidRequest,
},
{
name: "invalid token request with empty code_verifier",
args: args{
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: func() *logical.Request {
req := testAuthorizeReq(s, clientID)
req.Data["code_challenge_method"] = "plain"
req.Data["code_challenge"] = "a1b2c3d4"
return req
}(),
tokenReq: func() *logical.Request {
req := testTokenReq(s, "", clientID, clientSecret)
req.Data["code_verifier"] = ""
return req
}(),
},
wantErr: ErrTokenInvalidGrant,
},
{
name: "invalid token request with incorrect plain code_verifier",
args: args{
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: func() *logical.Request {
req := testAuthorizeReq(s, clientID)
req.Data["code_challenge_method"] = "plain"
req.Data["code_challenge"] = "a1b2c3d4"
return req
}(),
tokenReq: func() *logical.Request {
req := testTokenReq(s, "", clientID, clientSecret)
req.Data["code_verifier"] = "wont_match_challenge"
return req
}(),
},
wantErr: ErrTokenInvalidGrant,
},
{
name: "invalid token request with incorrect S256 code_verifier",
args: args{
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: func() *logical.Request {
req := testAuthorizeReq(s, clientID)
req.Data["code_challenge_method"] = "S256"
req.Data["code_challenge"] = "a1b2c3d4"
return req
}(),
tokenReq: func() *logical.Request {
req := testTokenReq(s, "", clientID, clientSecret)
req.Data["code_verifier"] = "wont_hash_to_challenge"
return req
}(),
},
wantErr: ErrTokenInvalidGrant,
},
{
name: "valid token request with plain code_challenge_method",
args: args{
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: func() *logical.Request {
req := testAuthorizeReq(s, clientID)
req.Data["code_challenge_method"] = "plain"
req.Data["code_challenge"] = "a1b2c3d4"
return req
}(),
tokenReq: func() *logical.Request {
req := testTokenReq(s, "", clientID, clientSecret)
req.Data["code_verifier"] = "a1b2c3d4"
return req
}(),
},
},
{
name: "valid token request with S256 code_challenge_method",
args: args{
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: func() *logical.Request {
req := testAuthorizeReq(s, clientID)
req.Data["code_challenge_method"] = "S256"
req.Data["code_challenge"] = "fc9Af6hKDgUZx5kRVMQUjeAkTXWJAgwNmELbnvrYIJQ"
return req
}(),
tokenReq: func() *logical.Request {
req := testTokenReq(s, "", clientID, clientSecret)
req.Data["code_verifier"] = "a1b2c3d4"
return req
}(),
},
},
{
name: "valid token request with max_age and auth_time claim",
args: args{
Expand Down Expand Up @@ -712,6 +810,38 @@ func TestOIDC_Path_OIDC_Authorize(t *testing.T) {
},
wantErr: ErrAuthInvalidRequest,
},
{
austingebauer marked this conversation as resolved.
Show resolved Hide resolved
name: "invalid authorize request with invalid code_challenge_method",
args: args{
entityID: entityID,
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: func() *logical.Request {
req := testAuthorizeReq(s, clientID)
req.Data["code_challenge_method"] = "S512"
req.Data["code_challenge"] = "a1b2c3d4"
return req
}(),
},
wantErr: ErrAuthInvalidRequest,
},
{
name: "invalid authorize request with valid code_challenge_method and empty code_challenge",
args: args{
entityID: entityID,
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: func() *logical.Request {
req := testAuthorizeReq(s, clientID)
req.Data["code_challenge_method"] = "S256"
req.Data["code_challenge"] = ""
return req
}(),
},
wantErr: ErrAuthInvalidRequest,
},
{
name: "valid authorize request with empty nonce",
args: args{
Expand Down
15 changes: 15 additions & 0 deletions vault/identity_store_oidc_provider_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,18 @@ func computeHashClaim(alg string, input string) (string, error) {
sum := h.Sum(nil)
return base64.RawURLEncoding.EncodeToString(sum[:len(sum)/2]), nil
}

// computeCodeChallenge computes a Proof Key for Code Exchange (PKCE)
// code challenge given a code verifier and code challenge method.
func computeCodeChallenge(verifier string, method string) (string, error) {
switch method {
case codeChallengeMethodPlain:
return verifier, nil
case codeChallengeMethodS256:
hf := sha256.New()
hf.Write([]byte(verifier))
return base64.RawURLEncoding.EncodeToString(hf.Sum(nil)), nil
default:
return "", fmt.Errorf("invalid code challenge method %q", method)
}
}