From 462a9247221b86cdbc5172eed90273b74ba1bf9d Mon Sep 17 00:00:00 2001 From: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> Date: Tue, 22 Feb 2022 08:33:19 -0800 Subject: [PATCH] identity/oidc: Adds default provider, key, and allow_all assignment (#14119) --- changelog/14119.txt | 9 + .../identity/oidc_provider_test.go | 222 ++++++++++++++++++ vault/identity_store.go | 4 + vault/identity_store_oidc.go | 5 + vault/identity_store_oidc_provider.go | 162 ++++++++++++- vault/identity_store_oidc_provider_test.go | 36 +-- vault/identity_store_util.go | 33 --- 7 files changed, 416 insertions(+), 55 deletions(-) create mode 100644 changelog/14119.txt diff --git a/changelog/14119.txt b/changelog/14119.txt new file mode 100644 index 000000000..3056bc2c4 --- /dev/null +++ b/changelog/14119.txt @@ -0,0 +1,9 @@ +```release-note:improvement +identity/oidc: Adds a default OIDC provider +``` +```release-note:improvement +identity/oidc: Adds a default key for OIDC clients +``` +```release-note:improvement +identity/oidc: Adds an `allow_all` assignment that permits all entities to authenticate via an OIDC client +``` diff --git a/vault/external_tests/identity/oidc_provider_test.go b/vault/external_tests/identity/oidc_provider_test.go index 30350bfc8..3b8495660 100644 --- a/vault/external_tests/identity/oidc_provider_test.go +++ b/vault/external_tests/identity/oidc_provider_test.go @@ -39,6 +39,228 @@ const ( ` ) +// TestOIDC_Auth_Code_Flow_Default_Resources tests the authorization +// code flow using the default OIDC provider, default key, and allow_all +// assignment. This ensures that the resources are created and usable with +// an initial setup of Vault. +func TestOIDC_Auth_Code_Flow_Default_Resources(t *testing.T) { + cluster := setupOIDCTestCluster(t, 2) + defer cluster.Cleanup() + active := cluster.Cores[0].Client + standby := cluster.Cores[1].Client + + // Enable userpass auth and create a user + err := active.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{ + Type: "userpass", + }) + require.NoError(t, err) + _, err = active.Logical().Write("auth/userpass/users/end-user", map[string]interface{}{ + "password": testPassword, + }) + require.NoError(t, err) + + // Create a confidential client + _, err = active.Logical().Write("identity/oidc/client/confidential", map[string]interface{}{ + "redirect_uris": []string{testRedirectURI}, + "assignments": []string{"allow_all"}, + "id_token_ttl": "1h", + "access_token_ttl": "30m", + }) + require.NoError(t, err) + + // Read the client ID and secret in order to configure the OIDC client + resp, err := active.Logical().Read("identity/oidc/client/confidential") + require.NoError(t, err) + clientID := resp.Data["client_id"].(string) + clientSecret := resp.Data["client_secret"].(string) + + // We aren't going to open up a browser to facilitate the login and redirect + // from this test, so we'll log in via userpass and set the client's token as + // the token that results from the authentication. + resp, err = active.Logical().Write("auth/userpass/login/end-user", map[string]interface{}{ + "password": testPassword, + }) + require.NoError(t, err) + clientToken := resp.Auth.ClientToken + entityID := resp.Auth.EntityID + + // Look up the token to get its creation time. This will be used for test + // cases that make assertions on the max_age parameter and auth_time claim. + resp, err = active.Logical().Write("auth/token/lookup", map[string]interface{}{ + "token": clientToken, + }) + require.NoError(t, err) + expectedAuthTime, err := strconv.Atoi(string(resp.Data["creation_time"].(json.Number))) + require.NoError(t, err) + + // Read the issuer from the OIDC provider's discovery document + var discovery struct { + Issuer string `json:"issuer"` + } + decodeRawRequest(t, active, http.MethodGet, + "/v1/identity/oidc/provider/default/.well-known/openid-configuration", + nil, &discovery) + + // Create the client-side OIDC provider config + pc, err := oidc.NewConfig(discovery.Issuer, clientID, + oidc.ClientSecret(clientSecret), []oidc.Alg{oidc.RS256}, + []string{testRedirectURI}, oidc.WithProviderCA(string(cluster.CACertPEM))) + require.NoError(t, err) + + // Create the client-side OIDC provider + p, err := oidc.NewProvider(pc) + 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 + } + tests := []struct { + name string + args args + expected string + }{ + { + name: "active: authorization code flow", + args: args{ + options: []oidc.Option{ + oidc.WithScopes("openid"), + }, + }, + expected: fmt.Sprintf(`{ + "iss": "%s", + "aud": "%s", + "sub": "%s", + "namespace": "root" + }`, discovery.Issuer, clientID, entityID), + }, + { + name: "active: authorization code flow with max_age parameter", + args: args{ + options: []oidc.Option{ + oidc.WithScopes("openid"), + oidc.WithMaxAge(60), + }, + }, + expected: fmt.Sprintf(`{ + "iss": "%s", + "aud": "%s", + "sub": "%s", + "namespace": "root", + "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", + args: args{ + useStandby: true, + options: []oidc.Option{ + oidc.WithScopes("openid"), + }, + }, + expected: fmt.Sprintf(`{ + "iss": "%s", + "aud": "%s", + "sub": "%s", + "namespace": "root" + }`, discovery.Issuer, clientID, entityID), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := active + if tt.args.useStandby { + client = standby + } + client.SetToken(clientToken) + + // Create the client-side OIDC request state + oidcRequest, err := oidc.NewRequest(10*time.Minute, testRedirectURI, tt.args.options...) + require.NoError(t, err) + + // Get the URL for the authorization endpoint from the OIDC client + authURL, err := p.AuthURL(context.Background(), oidcRequest) + require.NoError(t, err) + parsedAuthURL, err := url.Parse(authURL) + require.NoError(t, err) + + // This replace only occurs because we're not using the browser in this test + authURLPath := strings.Replace(parsedAuthURL.Path, "/ui/vault/", "/v1/", 1) + + // Kick off the authorization code flow + var authResp struct { + Code string `json:"code"` + State string `json:"state"` + } + decodeRawRequest(t, client, http.MethodGet, authURLPath, parsedAuthURL.Query(), &authResp) + + // The returned state must match the OIDC client state + require.Equal(t, oidcRequest.State(), authResp.State) + + // Exchange the authorization code for an ID token and access token. + // The ID token signature is verified using the provider's public keys after + // the exchange takes place. The ID token is also validated according to the + // client-side requirements of the OIDC spec. See the validation code at: + // - https://github.com/hashicorp/cap/blob/main/oidc/provider.go#L240 + // - https://github.com/hashicorp/cap/blob/main/oidc/provider.go#L441 + token, err := p.Exchange(context.Background(), oidcRequest, authResp.State, authResp.Code) + require.NoError(t, err) + require.NotNil(t, token) + idToken := token.IDToken() + accessToken := token.StaticTokenSource() + + // Get the ID token claims + allClaims := make(map[string]interface{}) + require.NoError(t, idToken.Claims(&allClaims)) + + // Get the sub claim for userinfo validation + require.NotEmpty(t, allClaims["sub"]) + subject := allClaims["sub"].(string) + + // Request userinfo using the access token + err = p.UserInfo(context.Background(), accessToken, subject, &allClaims) + require.NoError(t, err) + + // Assert that claims computed during the flow (i.e., not known + // ahead of time in this test) are present as top-level keys + for _, claim := range []string{"iat", "exp", "nonce", "at_hash", "c_hash"} { + _, ok := allClaims[claim] + require.True(t, ok) + } + + // Assert that all other expected claims are populated + expectedClaims := make(map[string]interface{}) + require.NoError(t, json.Unmarshal([]byte(tt.expected), &expectedClaims)) + for k, expectedVal := range expectedClaims { + actualVal, ok := allClaims[k] + require.True(t, ok) + require.EqualValues(t, expectedVal, actualVal) + } + }) + } +} + // TestOIDC_Auth_Code_Flow_Confidential_CAP_Client tests the authorization code // flow using a Vault OIDC provider. The test uses the CAP OIDC client to verify // that the Vault OIDC provider's responses pass the various client-side validation diff --git a/vault/identity_store.go b/vault/identity_store.go index f0b6de797..6f1da957b 100644 --- a/vault/identity_store.go +++ b/vault/identity_store.go @@ -492,6 +492,10 @@ func (i *IdentityStore) initialize(ctx context.Context, req *logical.Initializat return nil } + if err := i.storeOIDCDefaultResources(ctx, req.Storage); err != nil { + return err + } + entry, err := logical.StorageEntryJSON(caseSensitivityKey, &casesensitivity{ DisableLowerCasedNames: i.disableLowerCasedNames, }) diff --git a/vault/identity_store_oidc.go b/vault/identity_store_oidc.go index bbf7c0f54..26e5920cb 100644 --- a/vault/identity_store_oidc.go +++ b/vault/identity_store_oidc.go @@ -683,6 +683,11 @@ func (i *IdentityStore) pathOIDCDeleteKey(ctx context.Context, req *logical.Requ targetKeyName := d.Get("name").(string) + if targetKeyName == defaultKeyName { + return logical.ErrorResponse("deletion of key %q not allowed", + defaultKeyName), nil + } + i.oidcLock.Lock() roleNames, err := i.roleNamesReferencingTargetKeyName(ctx, req, targetKeyName) diff --git a/vault/identity_store_oidc_provider.go b/vault/identity_store_oidc_provider.go index ca2901d5e..3867c2095 100644 --- a/vault/identity_store_oidc_provider.go +++ b/vault/identity_store_oidc_provider.go @@ -35,6 +35,9 @@ const ( clientSecretPrefix = "hvo_secret_" codeChallengeMethodPlain = "plain" codeChallengeMethodS256 = "S256" + defaultProviderName = "default" + defaultKeyName = "default" + allowAllAssignmentName = "allow_all" // Storage path constants oidcProviderPrefix = "oidc_provider/" @@ -266,8 +269,8 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path { }, "key": { Type: framework.TypeString, - Description: "A reference to a named key resource. Cannot be modified after creation.", - Required: true, + Description: "A reference to a named key resource. Cannot be modified after creation. Defaults to the 'default' key.", + Default: "default", }, "id_token_ttl": { Type: framework.TypeDurationSecond, @@ -654,6 +657,11 @@ func (i *IdentityStore) providersReferencingTargetScopeName(ctx context.Context, func (i *IdentityStore) pathOIDCCreateUpdateAssignment(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) + if name == allowAllAssignmentName { + return logical.ErrorResponse("modification of assignment %q not allowed", + allowAllAssignmentName), nil + } + i.oidcLock.Lock() defer i.oidcLock.Unlock() @@ -749,6 +757,11 @@ func (i *IdentityStore) getOIDCAssignment(ctx context.Context, s logical.Storage func (i *IdentityStore) pathOIDCDeleteAssignment(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) + if name == allowAllAssignmentName { + return logical.ErrorResponse("deletion of assignment %q not allowed", + allowAllAssignmentName), nil + } + i.oidcLock.Lock() defer i.oidcLock.Unlock() @@ -1005,10 +1018,6 @@ func (i *IdentityStore) pathOIDCCreateUpdateClient(ctx context.Context, req *log client.Key = d.Get("key").(string) } - if client.Key == "" { - return logical.ErrorResponse("the key parameter is required"), nil - } - // enforce key existence on client creation key, err := i.getNamedKey(ctx, req.Storage, client.Key) if err != nil { @@ -1348,6 +1357,12 @@ func (i *IdentityStore) getOIDCProvider(ctx context.Context, s logical.Storage, // pathOIDCDeleteProvider is used to delete a provider func (i *IdentityStore) pathOIDCDeleteProvider(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) + + if name == defaultProviderName { + return logical.ErrorResponse("deletion of OIDC provider %q not allowed", + defaultProviderName), nil + } + return nil, req.Storage.Delete(ctx, providerPath+name) } @@ -1388,7 +1403,11 @@ func (i *IdentityStore) pathOIDCProviderDiscovery(ctx context.Context, req *logi ResponseTypes: []string{"code"}, Subjects: []string{"public"}, GrantTypes: []string{"authorization_code"}, - AuthMethods: []string{"client_secret_basic"}, + AuthMethods: []string{ + // PKCE is required for auth method "none" + "none", + "client_secret_basic", + }, } data, err := json.Marshal(disc) @@ -2294,6 +2313,10 @@ func (i *IdentityStore) entityHasAssignment(ctx context.Context, s logical.Stora return false, nil } + if strutil.StrListContains(assignments, allowAllAssignmentName) { + return true, nil + } + // Get the group IDs that the entity is a member of groups, inheritedGroups, err := i.groupsByEntityID(entity.GetID()) if err != nil { @@ -2329,6 +2352,131 @@ func (i *IdentityStore) entityHasAssignment(ctx context.Context, s logical.Stora return false, nil } +func defaultOIDCProvider() provider { + return provider{ + AllowedClientIDs: []string{"*"}, + ScopesSupported: []string{}, + } +} + +func defaultOIDCKey() namedKey { + return namedKey{ + Algorithm: "RS256", + VerificationTTL: 24 * time.Hour, + RotationPeriod: 24 * time.Hour, + NextRotation: time.Now().Add(24 * time.Hour), + AllowedClientIDs: []string{"*"}, + } +} + +func allowAllAssignment() assignment { + return assignment{ + EntityIDs: []string{"*"}, + GroupIDs: []string{"*"}, + } +} + +func (i *IdentityStore) storeOIDCDefaultResources(ctx context.Context, view logical.Storage) error { + // Store the default provider + storageKey := providerPath + defaultProviderName + entry, err := view.Get(ctx, storageKey) + if err != nil { + return err + } + if entry == nil { + entry, err := logical.StorageEntryJSON(storageKey, defaultOIDCProvider()) + if err != nil { + return err + } + if err := view.Put(ctx, entry); err != nil { + return err + } + i.Logger().Debug("wrote OIDC default provider") + } + + // Store the default key + storageKey = namedKeyConfigPath + defaultKeyName + entry, err = view.Get(ctx, storageKey) + if err != nil { + return err + } + if entry == nil { + defaultKey := defaultOIDCKey() + + // Generate initial key material for current and next keys + err = defaultKey.generateAndSetKey(ctx, i.Logger(), view) + if err != nil { + return err + } + err = defaultKey.generateAndSetNextKey(ctx, i.Logger(), view) + if err != nil { + return err + } + + // Store the entry + entry, err := logical.StorageEntryJSON(storageKey, defaultKey) + if err != nil { + return err + } + if err := view.Put(ctx, entry); err != nil { + return err + } + i.Logger().Debug("wrote OIDC default key") + } + + // Store the allow all assignment + storageKey = assignmentPath + allowAllAssignmentName + entry, err = view.Get(ctx, storageKey) + if err != nil { + return err + } + if entry == nil { + entry, err := logical.StorageEntryJSON(storageKey, allowAllAssignment()) + if err != nil { + return err + } + if err := view.Put(ctx, entry); err != nil { + return err + } + i.Logger().Debug("wrote OIDC allow_all assignment") + } + + return nil +} + +func (i *IdentityStore) loadOIDCClients(ctx context.Context) error { + i.logger.Debug("identity loading OIDC clients") + + clients, err := i.view.List(ctx, clientPath) + if err != nil { + return err + } + + txn := i.db.Txn(true) + defer txn.Abort() + for _, name := range clients { + entry, err := i.view.Get(ctx, clientPath+name) + if err != nil { + return err + } + if entry == nil { + continue + } + + var client client + if err := entry.DecodeJSON(&client); err != nil { + return err + } + + if err := i.memDBUpsertClientInTxn(txn, &client); err != nil { + return err + } + } + txn.Commit() + + return nil +} + // clientByID returns the client with the given ID. func (i *IdentityStore) clientByID(ctx context.Context, s logical.Storage, id string) (*client, error) { // Read the client from memdb diff --git a/vault/identity_store_oidc_provider_test.go b/vault/identity_store_oidc_provider_test.go index 2d48c2a98..080f4cb58 100644 --- a/vault/identity_store_oidc_provider_test.go +++ b/vault/identity_store_oidc_provider_test.go @@ -1647,25 +1647,31 @@ func TestOIDC_Path_OIDC_Client_Type(t *testing.T) { } } -// TestOIDC_Path_OIDC_ProviderClient_NoKeyParameter tests that a client cannot -// be created without a key parameter -func TestOIDC_Path_OIDC_ProviderClient_NoKeyParameter(t *testing.T) { +// TestOIDC_Path_OIDC_ProviderClient_DefaultKey tests that a +// client uses the default key if none provided at creation time. +func TestOIDC_Path_OIDC_ProviderClient_DefaultKey(t *testing.T) { c, _, _ := TestCoreUnsealed(t) ctx := namespace.RootContext(nil) - storage := &logical.InmemStorage{} + require.NoError(t, c.identityStore.storeOIDCDefaultResources(ctx, c.identityStore.view)) - // Create a test client "test-client1" without a key param -- should fail + // Create a test client "test-client" without a key param resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{ - Path: "oidc/client/test-client1", + Path: "oidc/client/test-client", Operation: logical.CreateOperation, - Storage: storage, + Storage: c.identityStore.view, }) - expectError(t, resp, err) - // validate error message - expectedStrings := map[string]interface{}{ - "the key parameter is required": true, - } - expectStrings(t, []string{resp.Data["error"].(string)}, expectedStrings) + expectSuccess(t, resp, err) + + // Read "test-client" to validate it uses the default key + resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/client/test-client", + Operation: logical.ReadOperation, + Storage: c.identityStore.view, + }) + expectSuccess(t, resp, err) + + // Assert that the client uses the default key + require.Equal(t, defaultKeyName, resp.Data["key"].(string)) } // TestOIDC_Path_OIDC_ProviderClient_NilKeyEntry tests that a client cannot be @@ -3394,7 +3400,7 @@ func TestOIDC_Path_OpenIDProviderConfig(t *testing.T) { TokenEndpoint: basePath + "/token", UserinfoEndpoint: basePath + "/userinfo", GrantTypes: []string{"authorization_code"}, - AuthMethods: []string{"client_secret_basic"}, + AuthMethods: []string{"none", "client_secret_basic"}, RequestURIParameter: false, } discoveryResp := &providerDiscovery{} @@ -3448,7 +3454,7 @@ func TestOIDC_Path_OpenIDProviderConfig(t *testing.T) { TokenEndpoint: basePath + "/token", UserinfoEndpoint: basePath + "/userinfo", GrantTypes: []string{"authorization_code"}, - AuthMethods: []string{"client_secret_basic"}, + AuthMethods: []string{"none", "client_secret_basic"}, RequestURIParameter: false, } discoveryResp = &providerDiscovery{} diff --git a/vault/identity_store_util.go b/vault/identity_store_util.go index 4ac997ff8..bf5c0dba5 100644 --- a/vault/identity_store_util.go +++ b/vault/identity_store_util.go @@ -90,39 +90,6 @@ func (i *IdentityStore) sanitizeName(name string) string { return strings.ToLower(name) } -func (i *IdentityStore) loadOIDCClients(ctx context.Context) error { - i.logger.Debug("identity loading OIDC clients") - - clients, err := i.view.List(ctx, clientPath) - if err != nil { - return err - } - - txn := i.db.Txn(true) - defer txn.Abort() - for _, name := range clients { - entry, err := i.view.Get(ctx, clientPath+name) - if err != nil { - return err - } - if entry == nil { - continue - } - - var client client - if err := entry.DecodeJSON(&client); err != nil { - return err - } - - if err := i.memDBUpsertClientInTxn(txn, &client); err != nil { - return err - } - } - txn.Commit() - - return nil -} - func (i *IdentityStore) loadGroups(ctx context.Context) error { i.logger.Debug("identity loading groups") existing, err := i.groupPacker.View().List(ctx, groupBucketsPrefix)