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

Disable identity for local mounts #4407

Merged
merged 4 commits into from
Apr 23, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 4 additions & 0 deletions vault/identity_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,10 @@ func (i *IdentityStore) CreateOrFetchEntity(alias *logical.Alias) (*identity.Ent
return nil, fmt.Errorf("invalid mount accessor %q", alias.MountAccessor)
}

if mountValidationResp.MountLocal {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this happen later so that existing values are returned?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't matter. This function will not be called for the new tokens.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I just figured from the name (Create or Fetch) that it could end up creating new entities...which it can... :-)

return nil, fmt.Errorf("mount_accessor %q is of a local mount", alias.MountAccessor)
}

if mountValidationResp.MountType != alias.MountType {
return nil, fmt.Errorf("mount accessor %q is not a mount of type %q", alias.MountAccessor, alias.MountType)
}
Expand Down
4 changes: 4 additions & 0 deletions vault/identity_store_aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ func (i *IdentityStore) handleAliasUpdateCommon(req *logical.Request, d *framewo
return logical.ErrorResponse(fmt.Sprintf("invalid mount accessor %q", mountAccessor)), nil
}

if mountValidationResp.MountLocal {
return logical.ErrorResponse(fmt.Sprintf("mount_accessor %q is of a local mount", mountAccessor)), nil
}

// Get alias metadata
metadata, ok, err := d.GetOkErr("metadata")
if err != nil {
Expand Down
62 changes: 62 additions & 0 deletions vault/identity_store_aliases_ext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package vault_test

import (
"testing"

"github.com/hashicorp/vault/api"
vaulthttp "github.com/hashicorp/vault/http"
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/vault"

credLdap "github.com/hashicorp/vault/builtin/credential/ldap"
)

func TestIdentityStore_EntityAliasLocalMount(t *testing.T) {
coreConfig := &vault.CoreConfig{
CredentialBackends: map[string]logical.Factory{
"ldap": credLdap.Factory,
},
}
cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
HandlerFunc: vaulthttp.Handler,
})
cluster.Start()
defer cluster.Cleanup()

core := cluster.Cores[0].Core
vault.TestWaitActive(t, core)
client := cluster.Cores[0].Client

// Create a local auth mount
err := client.Sys().EnableAuthWithOptions("ldap", &api.EnableAuthOptions{
Type: "ldap",
Local: true,
})
if err != nil {
t.Fatal(err)
}

// Extract out the mount accessor for LDAP auth
auths, err := client.Sys().ListAuth()
if err != nil {
t.Fatal(err)
}
ldapMountAccessor := auths["ldap/"].Accessor

// Create an entity
secret, err := client.Logical().Write("identity/entity", nil)
if err != nil {
t.Fatal(err)
}
entityID := secret.Data["id"].(string)

// Attempt to create an entity alias against a local mount should fail
secret, err = client.Logical().Write("identity/entity-alias", map[string]interface{}{
"name": "testuser",
"mount_accessor": ldapMountAccessor,
"canonical_id": entityID,
})
if err == nil {
t.Fatalf("expected error since mount is local")
}
}
4 changes: 4 additions & 0 deletions vault/identity_store_group_aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ func (i *IdentityStore) handleGroupAliasUpdateCommon(req *logical.Request, d *fr
return logical.ErrorResponse(fmt.Sprintf("invalid mount accessor %q", mountAccessor)), nil
}

if mountValidationResp.MountLocal {
return logical.ErrorResponse(fmt.Sprintf("mount_accessor %q is of a local mount", mountAccessor)), nil
}

groupAliasByFactors, err := i.MemDBAliasByFactors(mountValidationResp.MountAccessor, groupAliasName, false, true)
if err != nil {
return nil, err
Expand Down
64 changes: 64 additions & 0 deletions vault/identity_store_group_aliases_ext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package vault_test

import (
"testing"

"github.com/hashicorp/vault/api"
vaulthttp "github.com/hashicorp/vault/http"
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/vault"

credLdap "github.com/hashicorp/vault/builtin/credential/ldap"
)

func TestIdentityStore_GroupAliasLocalMount(t *testing.T) {
coreConfig := &vault.CoreConfig{
CredentialBackends: map[string]logical.Factory{
"ldap": credLdap.Factory,
},
}
cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
HandlerFunc: vaulthttp.Handler,
})
cluster.Start()
defer cluster.Cleanup()

core := cluster.Cores[0].Core
vault.TestWaitActive(t, core)
client := cluster.Cores[0].Client

// Create a local auth mount
err := client.Sys().EnableAuthWithOptions("ldap", &api.EnableAuthOptions{
Type: "ldap",
Local: true,
})
if err != nil {
t.Fatal(err)
}

// Extract out the mount accessor for LDAP auth
auths, err := client.Sys().ListAuth()
if err != nil {
t.Fatal(err)
}
ldapMountAccessor := auths["ldap/"].Accessor

// Create an external group
secret, err := client.Logical().Write("identity/group", map[string]interface{}{
"type": "external",
})
if err != nil {
t.Fatal(err)
}
groupID := secret.Data["id"].(string)

// Attempt to create a group alias against a local mount should fail
secret, err = client.Logical().Write("identity/group-alias", map[string]interface{}{
"name": "testuser",
"mount_accessor": ldapMountAccessor,
"canonical_id": groupID,
})
if err == nil {
t.Fatalf("expected error since mount is local")
}
}
4 changes: 3 additions & 1 deletion vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,9 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
var entity *identity.Entity
auth = resp.Auth

if auth.Alias != nil {
mEntry := c.router.MatchingMountEntry(req.Path)

if auth.Alias != nil && mEntry != nil && !mEntry.Local {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing the auth/token/renew path earlier on, which can also end up updating memberships.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left the renewal flow as it is to allow it to work if the tokens were created before this fix. The thought I had was to only disallow entity attachment for the newly created tokens.

// Overwrite the mount type and mount path in the alias
// information
auth.Alias.MountType = req.MountType
Expand Down
2 changes: 2 additions & 0 deletions vault/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type validateMountResponse struct {
MountType string `json:"mount_type" structs:"mount_type" mapstructure:"mount_type"`
MountAccessor string `json:"mount_accessor" structs:"mount_accessor" mapstructure:"mount_accessor"`
MountPath string `json:"mount_path" structs:"mount_path" mapstructure:"mount_path"`
MountLocal bool `json:"mount_local" structs:"mount_local" mapstructure:"mount_local"`
}

// validateMountByAccessor returns the mount type and ID for a given mount
Expand All @@ -84,6 +85,7 @@ func (r *Router) validateMountByAccessor(accessor string) *validateMountResponse
MountAccessor: mountEntry.Accessor,
MountType: mountEntry.Type,
MountPath: mountPath,
MountLocal: mountEntry.Local,
}
}

Expand Down