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 AWS Secret Engine Root Credential Rotation #5140

Merged
merged 12 commits into from
Sep 26, 2018
6 changes: 5 additions & 1 deletion builtin/logical/aws/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ func Backend() *backend {
},

Paths: []*framework.Path{
pathConfigRoot(),
pathConfigRoot(&b),
pathConfigRotateRoot(&b),
pathConfigLease(&b),
pathRoles(&b),
pathListRoles(&b),
Expand All @@ -57,6 +58,9 @@ type backend struct {

// Mutex to protect access to reading and writing policies
roleMutex sync.RWMutex

// Mutex to protect access to reading and writing root creds
rootMutex sync.RWMutex
}

const backendHelp = `
Expand Down
39 changes: 39 additions & 0 deletions builtin/logical/aws/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func TestBackend_basicSTS(t *testing.T) {
Backend: getBackend(t),
Steps: []logicaltest.TestStep{
testAccStepConfigWithCreds(t, accessKey),
testAccStepRotateRoot(accessKey),
testAccStepWritePolicy(t, "test", testDynamoPolicy),
testAccStepRead(t, "sts", "test", []credentialTestFunc{listDynamoTablesTest}),
testAccStepWriteArnPolicyRef(t, "test", ec2PolicyArn),
Expand Down Expand Up @@ -368,6 +369,44 @@ func testAccStepConfigWithCreds(t *testing.T, accessKey *awsAccessKey) logicalte
}
}

func testAccStepRotateRoot(oldAccessKey *awsAccessKey) logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "config/rotate-root",
Check: func(resp *logical.Response) error {
if resp == nil {
return fmt.Errorf("received nil response from config/rotate-root")
}
newAccessKeyId := resp.Data["access_key"].(string)
if newAccessKeyId == oldAccessKey.AccessKeyId {
return fmt.Errorf("rotate-root didn't rotate access key")
}
awsConfig := &aws.Config{
Region: aws.String("us-east-1"),
HTTPClient: cleanhttp.DefaultClient(),
Credentials: credentials.NewStaticCredentials(oldAccessKey.AccessKeyId, oldAccessKey.SecretAccessKey, ""),
}
// sigh....
oldAccessKey.AccessKeyId = newAccessKeyId
log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...")
time.Sleep(10 * time.Second)
svc := sts.New(session.New(awsConfig))
params := &sts.GetCallerIdentityInput{}
_, err := svc.GetCallerIdentity(params)
if err == nil {
return fmt.Errorf("bad: old credentials succeeded after rotate")
}
if aerr, ok := err.(awserr.Error); ok {
if aerr.Code() != "InvalidClientTokenId" {
return fmt.Errorf("Unknown error returned from AWS: %#v", aerr)
}
return nil
}
return err
},
}
}

func testAccStepRead(t *testing.T, path, name string, credentialTests []credentialTestFunc) logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.ReadOperation,
Expand Down
12 changes: 7 additions & 5 deletions builtin/logical/aws/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import (
"github.com/hashicorp/vault/logical"
)

func getRootConfig(ctx context.Context, s logical.Storage, clientType string) (*aws.Config, error) {
func (b *backend) getRootConfig(ctx context.Context, s logical.Storage, clientType string) (*aws.Config, error) {
credsConfig := &awsutil.CredentialsConfig{}
var endpoint string
var maxRetries int = aws.UseServiceDefaultRetries

b.rootMutex.RLock()
defer b.rootMutex.RUnlock()
entry, err := s.Get(ctx, "config/root")
if err != nil {
return nil, err
Expand Down Expand Up @@ -68,8 +70,8 @@ func getRootConfig(ctx context.Context, s logical.Storage, clientType string) (*
}, nil
}

func clientIAM(ctx context.Context, s logical.Storage) (*iam.IAM, error) {
awsConfig, err := getRootConfig(ctx, s, "iam")
func (b *backend) clientIAM(ctx context.Context, s logical.Storage) (*iam.IAM, error) {
awsConfig, err := b.getRootConfig(ctx, s, "iam")
if err != nil {
return nil, err
}
Expand All @@ -82,8 +84,8 @@ func clientIAM(ctx context.Context, s logical.Storage) (*iam.IAM, error) {
return client, nil
}

func clientSTS(ctx context.Context, s logical.Storage) (*sts.STS, error) {
awsConfig, err := getRootConfig(ctx, s, "sts")
func (b *backend) clientSTS(ctx context.Context, s logical.Storage) (*sts.STS, error) {
awsConfig, err := b.getRootConfig(ctx, s, "sts")
if err != nil {
return nil, err
}
Expand Down
9 changes: 6 additions & 3 deletions builtin/logical/aws/path_config_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/hashicorp/vault/logical/framework"
)

func pathConfigRoot() *framework.Path {
func pathConfigRoot(b *backend) *framework.Path {
return &framework.Path{
Pattern: "config/root",
Fields: map[string]*framework.FieldSchema{
Expand Down Expand Up @@ -42,20 +42,23 @@ func pathConfigRoot() *framework.Path {
},

Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: pathConfigRootWrite,
logical.UpdateOperation: b.pathConfigRootWrite,
},

HelpSynopsis: pathConfigRootHelpSyn,
HelpDescription: pathConfigRootHelpDesc,
}
}

func pathConfigRootWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
func (b *backend) pathConfigRootWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
region := data.Get("region").(string)
iamendpoint := data.Get("iam_endpoint").(string)
stsendpoint := data.Get("sts_endpoint").(string)
maxretries := data.Get("max_retries").(int)

b.rootMutex.Lock()
defer b.rootMutex.Unlock()

entry, err := logical.StorageEntryJSON("config/root", rootConfig{
AccessKey: data.Get("access_key").(string),
SecretKey: data.Get("secret_key").(string),
Expand Down
121 changes: 121 additions & 0 deletions builtin/logical/aws/path_config_rotate_root.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package aws

import (
"context"
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
)

func pathConfigRotateRoot(b *backend) *framework.Path {
return &framework.Path{
Pattern: "config/rotate-root",
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: b.pathConfigRotateRootUpdate,
},

HelpSynopsis: pathConfigRotateRootHelpSyn,
HelpDescription: pathConfigRotateRootHelpDesc,
}
}

func (b *backend) pathConfigRotateRootUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
// have to get the client config first because that takes out a read lock
client, err := b.clientIAM(ctx, req.Storage)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf("error retrieving IAM client: %v", err)), nil
}
if client == nil {
return logical.ErrorResponse("nil IAM client"), nil
}

b.rootMutex.Lock()
defer b.rootMutex.Unlock()
rawRootConfig, err := req.Storage.Get(ctx, "config/root")
if err != nil {
return nil, err
}
if rawRootConfig == nil {
return logical.ErrorResponse("no configuration found for config/root"), nil
joelthompson marked this conversation as resolved.
Show resolved Hide resolved
}
var config rootConfig
if err := rawRootConfig.DecodeJSON(&config); err != nil {
return logical.ErrorResponse(fmt.Sprintf("error reading root configuration: %v", err)), nil
}

if config.AccessKey == "" || config.SecretKey == "" {
return logical.ErrorResponse("cannot call config/rotate-root when either access_key or secret_key is empty"), nil
}

var getUserInput iam.GetUserInput // empty input means get current user
getUserRes, err := client.GetUser(&getUserInput)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf("error calling GetUser: %v", err)), nil
}
if getUserRes == nil {
return logical.ErrorResponse("nil response from GetUser"), nil
}
if getUserRes.User == nil {
return logical.ErrorResponse("nil user returned from GetUser"), nil
}
if getUserRes.User.UserName == nil {
return logical.ErrorResponse("nil UserName returnjd from GetUser"), nil
joelthompson marked this conversation as resolved.
Show resolved Hide resolved
}

createAccessKeyInput := iam.CreateAccessKeyInput{
UserName: getUserRes.User.UserName,
}
createAccessKeyRes, err := client.CreateAccessKey(&createAccessKeyInput)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf("error calling CreateAccessKey: %v", err)), nil
}
if createAccessKeyRes.AccessKey == nil {
return logical.ErrorResponse("nil response from CreateAccessKey"), nil
}
if createAccessKeyRes.AccessKey.AccessKeyId == nil || createAccessKeyRes.AccessKey.SecretAccessKey == nil {
return logical.ErrorResponse("nil AccessKeyId or SecretAccessKey returned from CreateAccessKey"), nil
}

oldAccessKey := config.AccessKey

config.AccessKey = *createAccessKeyRes.AccessKey.AccessKeyId
config.SecretKey = *createAccessKeyRes.AccessKey.SecretAccessKey

newEntry, err := logical.StorageEntryJSON("config/root", config)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf("error generating new root config: %v", err)), nil
}
// TODO: Should we return the full error message? Are there any scenarios in which it could expose
joelthompson marked this conversation as resolved.
Show resolved Hide resolved
// the underlying creds? (The idea of rotate-root is that it should never expose credentials.)
if err := req.Storage.Put(ctx, newEntry); err != nil {
return logical.ErrorResponse(fmt.Sprintf("error saving new root config: %v", err)), nil
}

deleteAccessKeyInput := iam.DeleteAccessKeyInput{
AccessKeyId: aws.String(oldAccessKey),
UserName: getUserRes.User.UserName,
}
_, err = client.DeleteAccessKey(&deleteAccessKeyInput)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf("error deleting old access key: %v", err)), nil
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this just be a warning response instead of returning an err? In this event, the access key was successfully rotated, but it just failed to clean up.

Copy link
Member

Choose a reason for hiding this comment

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

It should be an error.


return &logical.Response{
Data: map[string]interface{}{
"access_key": config.AccessKey,
},
}, nil
}

const pathConfigRotateRootHelpSyn = `
Request to rotate the AWS credentials used by Vault
`

const pathConfigRotateRootHelpDesc = `
This path attempts to rotate the AWS credentials used by Vault for this mount.
It is only valid if Vault has been configured to use AWS IAM credentials via the
config/root endpoint.
`
4 changes: 2 additions & 2 deletions builtin/logical/aws/path_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,15 @@ func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *fr
}
}

func pathUserRollback(ctx context.Context, req *logical.Request, _kind string, data interface{}) error {
func (b *backend) pathUserRollback(ctx context.Context, req *logical.Request, _kind string, data interface{}) error {
var entry walUser
if err := mapstructure.Decode(data, &entry); err != nil {
return err
}
username := entry.UserName

// Get the client
client, err := clientIAM(ctx, req.Storage)
client, err := b.clientIAM(ctx, req.Storage)
if err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions builtin/logical/aws/rollback.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import (
"github.com/hashicorp/vault/logical/framework"
)

var walRollbackMap = map[string]framework.WALRollbackFunc{
"user": pathUserRollback,
}

func (b *backend) walRollback(ctx context.Context, req *logical.Request, kind string, data interface{}) error {
walRollbackMap := map[string]framework.WALRollbackFunc{
"user": b.pathUserRollback,
}

if !b.System().LocalMount() && b.System().ReplicationState().HasState(consts.ReplicationPerformancePrimary) {
return nil
}
Expand Down
12 changes: 6 additions & 6 deletions builtin/logical/aws/secret_access_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func secretAccessKeys(b *backend) *framework.Secret {
},

Renew: b.secretAccessKeysRenew,
Revoke: secretAccessKeysRevoke,
Revoke: b.secretAccessKeysRevoke,
}
}

Expand Down Expand Up @@ -67,7 +67,7 @@ func genUsername(displayName, policyName, userType string) (ret string, warning
func (b *backend) secretTokenCreate(ctx context.Context, s logical.Storage,
displayName, policyName, policy string,
lifeTimeInSeconds int64) (*logical.Response, error) {
STSClient, err := clientSTS(ctx, s)
STSClient, err := b.clientSTS(ctx, s)
if err != nil {
return logical.ErrorResponse(err.Error()), nil
}
Expand Down Expand Up @@ -112,7 +112,7 @@ func (b *backend) secretTokenCreate(ctx context.Context, s logical.Storage,
func (b *backend) assumeRole(ctx context.Context, s logical.Storage,
displayName, roleName, roleArn, policy string,
lifeTimeInSeconds int64) (*logical.Response, error) {
STSClient, err := clientSTS(ctx, s)
STSClient, err := b.clientSTS(ctx, s)
if err != nil {
return logical.ErrorResponse(err.Error()), nil
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func (b *backend) secretAccessKeysCreate(
ctx context.Context,
s logical.Storage,
displayName, policyName string, role *awsRoleEntry) (*logical.Response, error) {
client, err := clientIAM(ctx, s)
client, err := b.clientIAM(ctx, s)
if err != nil {
return logical.ErrorResponse(err.Error()), nil
}
Expand Down Expand Up @@ -281,7 +281,7 @@ func (b *backend) secretAccessKeysRenew(ctx context.Context, req *logical.Reques
return resp, nil
}

func secretAccessKeysRevoke(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
func (b *backend) secretAccessKeysRevoke(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {

// STS cleans up after itself so we can skip this if is_sts internal data
// element set to true. If is_sts is not set, assumes old version
Expand Down Expand Up @@ -309,7 +309,7 @@ func secretAccessKeysRevoke(ctx context.Context, req *logical.Request, d *framew
}

// Use the user rollback mechanism to delete this user
err := pathUserRollback(ctx, req, "user", map[string]interface{}{
err := b.pathUserRollback(ctx, req, "user", map[string]interface{}{
"username": username,
})
if err != nil {
Expand Down
Loading