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

Add ability to disable an entity #4353

Merged
merged 3 commits into from
Apr 14, 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
89 changes: 50 additions & 39 deletions helper/identity/types.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions helper/identity/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ message Entity {
// MFASecrets holds the MFA secrets indexed by the identifier of the MFA
// method configuration.
//map<string, mfa.Secret> mfa_secrets = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m suspicious having this commented out might break if a user upgrades from oss -> ent

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be fine so long as the number is distinct.


// Disabled indicates whether tokens associated with the account should not
// be able to be used
bool disabled = 11;
}

// Alias represents the alias that gets stored inside of the
Expand Down
4 changes: 4 additions & 0 deletions logical/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ var (
// ErrPermissionDenied is returned if the client is not authorized
ErrPermissionDenied = errors.New("permission denied")

// ErrDisabledEntity is returned if the entity tied to a token is marked as
// disabled
ErrEntityDisabled = errors.New("entity associated with token is disabled")

// ErrMultiAuthzPending is returned if the the request needs more
// authorizations
ErrMultiAuthzPending = errors.New("request needs further approval")
Expand Down
29 changes: 23 additions & 6 deletions vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,10 @@ func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool
}
}

if entity != nil && entity.Disabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also be disallowing the entity from getting tokens?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

return nil, te, logical.ErrEntityDisabled
}

// Check if this is a root protected path
rootPath := c.router.RootPath(req.Path)

Expand Down Expand Up @@ -1319,6 +1323,13 @@ func (c *Core) sealInitCommon(ctx context.Context, req *logical.Request) (retErr
return retErr
}

if c.standby {
c.logger.Error("vault cannot seal when in standby mode; please restart instead")
retErr = multierror.Append(retErr, errors.New("vault cannot seal when in standby mode; please restart instead"))
c.stateLock.RUnlock()
return retErr
}

// Validate the token is a root token
acl, te, entity, err := c.fetchACLTokenEntryAndEntity(req.ClientToken)
if err != nil {
Expand All @@ -1327,12 +1338,6 @@ func (c *Core) sealInitCommon(ctx context.Context, req *logical.Request) (retErr
// for validation and the operation should be performed. But for now,
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove these comments?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I should move them down.

// just returning with an error and recommending a vault restart, which
// essentially does the same thing.
if c.standby {
c.logger.Error("vault cannot seal when in standby mode; please restart instead")
retErr = multierror.Append(retErr, errors.New("vault cannot seal when in standby mode; please restart instead"))
c.stateLock.RUnlock()
return retErr
}
retErr = multierror.Append(retErr, err)
c.stateLock.RUnlock()
return retErr
Expand All @@ -1358,6 +1363,12 @@ func (c *Core) sealInitCommon(ctx context.Context, req *logical.Request) (retErr
return retErr
}

if entity != nil && entity.Disabled {
retErr = multierror.Append(retErr, logical.ErrEntityDisabled)
c.stateLock.RUnlock()
return retErr
}

// Attempt to use the token (decrement num_uses)
// On error bail out; if the token has been revoked, bail out too
if te != nil {
Expand Down Expand Up @@ -1466,6 +1477,12 @@ func (c *Core) StepDown(req *logical.Request) (retErr error) {
return retErr
}

if entity != nil && entity.Disabled {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should do this before audit logging.

Copy link
Member Author

Choose a reason for hiding this comment

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

No -- for the same reason that we attempt to log whatever we get back from the token store before fully checking validity. The fact that a request is invalid doesn't mean it wasn't an attempted request.

retErr = multierror.Append(retErr, logical.ErrEntityDisabled)
c.stateLock.RUnlock()
return retErr
}

// Attempt to use the token (decrement num_uses)
if te != nil {
te, err = c.tokenStore.UseToken(ctx, te)
Expand Down
14 changes: 14 additions & 0 deletions vault/identity_store_entities.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ vault <command> <path> metadata=key1=value1 metadata=key2=value2
Type: framework.TypeCommaStringSlice,
Description: "Policies to be tied to the entity.",
},
"disabled": {
Type: framework.TypeBool,
Description: "If set true, tokens tied to this identity will not be able to be used (but will not be revoked).",
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: i.pathEntityRegister(),
Expand Down Expand Up @@ -76,6 +80,10 @@ vault <command> <path> metadata=key1=value1 metadata=key2=value2
Type: framework.TypeCommaStringSlice,
Description: "Policies to be tied to the entity.",
},
"disabled": {
Type: framework.TypeBool,
Description: "If set true, tokens tied to this identity will not be able to be used (but will not be revoked).",
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: i.pathEntityIDUpdate(),
Expand Down Expand Up @@ -354,6 +362,11 @@ func (i *IdentityStore) handleEntityUpdateCommon(req *logical.Request, d *framew
entity.Policies = entityPoliciesRaw.([]string)
}

disabledRaw, ok := d.GetOk("disabled")
if ok {
entity.Disabled = disabledRaw.(bool)
}

// Get the name
entityName := d.Get("name").(string)
if entityName != "" {
Expand Down Expand Up @@ -434,6 +447,7 @@ func (i *IdentityStore) handleEntityReadCommon(entity *identity.Entity) (*logica
respData["metadata"] = entity.Metadata
respData["merged_entity_ids"] = entity.MergedEntityIDs
respData["policies"] = entity.Policies
respData["disabled"] = entity.Disabled

// Convert protobuf timestamp into RFC3339 format
respData["creation_time"] = ptypes.TimestampString(entity.CreationTime)
Expand Down
Loading