From a14529faa92cb1b56a32841a8e9654aa1cf17886 Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Sat, 14 Apr 2018 20:33:18 -0400 Subject: [PATCH 1/7] Make AWS credential types more explicit The AWS secret engine had a lot of confusing overloading with role paramemters and how they mapped to each of the three credential types supported. This now adds parameters to remove the overloading while maintaining backwards compatibility. With the change, it also becomes easier to add other feature requests. Attaching multiple managed policies to IAM users and adding a policy document to STS AssumedRole credentials is now also supported. Fixes #4229 Fixes #3751 Fixes #2817 --- builtin/logical/aws/backend.go | 7 +- builtin/logical/aws/backend_test.go | 336 ++++++++++++---- builtin/logical/aws/path_roles.go | 380 +++++++++++++++--- builtin/logical/aws/path_roles_test.go | 91 ++++- builtin/logical/aws/path_sts.go | 95 ----- builtin/logical/aws/path_user.go | 94 ++++- builtin/logical/aws/secret_access_keys.go | 36 +- website/source/api/secret/aws/index.html.md | 125 +++--- website/source/docs/secrets/aws/index.html.md | 120 ++++-- 9 files changed, 925 insertions(+), 359 deletions(-) delete mode 100644 builtin/logical/aws/path_sts.go diff --git a/builtin/logical/aws/backend.go b/builtin/logical/aws/backend.go index 6b0b5e4878a6..1c1f04b6807f 100644 --- a/builtin/logical/aws/backend.go +++ b/builtin/logical/aws/backend.go @@ -3,6 +3,7 @@ package aws import ( "context" "strings" + "sync" "time" "github.com/hashicorp/vault/logical" @@ -34,10 +35,9 @@ func Backend() *backend { Paths: []*framework.Path{ pathConfigRoot(), pathConfigLease(&b), - pathRoles(), + pathRoles(&b), pathListRoles(&b), pathUser(&b), - pathSTS(&b), }, Secrets: []*framework.Secret{ @@ -54,6 +54,9 @@ func Backend() *backend { type backend struct { *framework.Backend + + // Mutex to protect access to reading and writing policies + roleMutex sync.RWMutex } const backendHelp = ` diff --git a/builtin/logical/aws/backend_test.go b/builtin/logical/aws/backend_test.go index 1cdd2c884169..5ba395d092b9 100644 --- a/builtin/logical/aws/backend_test.go +++ b/builtin/logical/aws/backend_test.go @@ -1,18 +1,19 @@ package aws import ( - "bytes" "context" - "encoding/json" "fmt" "log" "os" + "reflect" "testing" "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/dynamodb" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/sts" @@ -34,8 +35,8 @@ func TestBackend_basic(t *testing.T) { Backend: getBackend(t), Steps: []logicaltest.TestStep{ testAccStepConfig(t), - testAccStepWritePolicy(t, "test", testPolicy), - testAccStepReadUser(t, "test"), + testAccStepWritePolicy(t, "test", testDynamoPolicy), + testAccStepReadUser(t, "test", []credentialTestFunc{listDynamoTablesTest}), }, }) } @@ -56,12 +57,12 @@ func TestBackend_basicSTS(t *testing.T) { Backend: getBackend(t), Steps: []logicaltest.TestStep{ testAccStepConfigWithCreds(t, accessKey), - testAccStepWritePolicy(t, "test", testPolicy), - testAccStepReadSTS(t, "test"), - testAccStepWriteArnPolicyRef(t, "test", testPolicyArn), + testAccStepWritePolicy(t, "test", testDynamoPolicy), + testAccStepReadSTS(t, "test", []credentialTestFunc{listDynamoTablesTest}), + testAccStepWriteArnPolicyRef(t, "test", ec2PolicyArn), testAccStepReadSTSWithArnPolicy(t, "test"), testAccStepWriteArnRoleRef(t, testRoleName), - testAccStepReadSTS(t, testRoleName), + testAccStepReadSTS(t, testRoleName, []credentialTestFunc{describeInstancesTest}), }, Teardown: func() error { return teardown(accessKey) @@ -70,8 +71,8 @@ func TestBackend_basicSTS(t *testing.T) { } func TestBackend_policyCrud(t *testing.T) { - var compacted bytes.Buffer - if err := json.Compact(&compacted, []byte(testPolicy)); err != nil { + compacted, err := compactJSON(testDynamoPolicy) + if err != nil { t.Fatalf("bad: %s", err) } @@ -80,8 +81,8 @@ func TestBackend_policyCrud(t *testing.T) { Backend: getBackend(t), Steps: []logicaltest.TestStep{ testAccStepConfig(t), - testAccStepWritePolicy(t, "test", testPolicy), - testAccStepReadPolicy(t, "test", compacted.String()), + testAccStepWritePolicy(t, "test", testDynamoPolicy), + testAccStepReadPolicy(t, "test", compacted), testAccStepDeletePolicy(t, "test"), testAccStepReadPolicy(t, "test", ""), }, @@ -162,7 +163,7 @@ func createRole(t *testing.T) { } attachment := &iam.AttachRolePolicyInput{ - PolicyArn: aws.String(testPolicyArn), + PolicyArn: aws.String(ec2PolicyArn), RoleName: aws.String(testRoleName), // Required } _, err = svc.AttachRolePolicy(attachment) @@ -254,7 +255,7 @@ func createUser(t *testing.T, accessKey *awsAccessKey) { accessKey.SecretAccessKey = *genAccessKey.SecretAccessKey } -func teardown(accessKey *awsAccessKey) error { +func deleteTestRole() error { awsConfig := &aws.Config{ Region: aws.String("us-east-1"), HTTPClient: cleanhttp.DefaultClient(), @@ -262,7 +263,7 @@ func teardown(accessKey *awsAccessKey) error { svc := iam.New(session.New(awsConfig)) attachment := &iam.DetachRolePolicyInput{ - PolicyArn: aws.String(testPolicyArn), + PolicyArn: aws.String(ec2PolicyArn), RoleName: aws.String(testRoleName), // Required } _, err := svc.DetachRolePolicy(attachment) @@ -282,12 +283,25 @@ func teardown(accessKey *awsAccessKey) error { log.Printf("[WARN] AWS DeleteRole failed: %v", err) return err } + return nil +} + +func teardown(accessKey *awsAccessKey) error { + + if err := deleteTestRole(); err != nil { + return err + } + awsConfig := &aws.Config{ + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), + } + svc := iam.New(session.New(awsConfig)) userDetachment := &iam.DetachUserPolicyInput{ PolicyArn: aws.String("arn:aws:iam::aws:policy/AdministratorAccess"), UserName: aws.String(testUserName), } - _, err = svc.DetachUserPolicy(userDetachment) + _, err := svc.DetachUserPolicy(userDetachment) if err != nil { log.Printf("[WARN] AWS DetachUserPolicy failed: %v", err) return err @@ -354,7 +368,7 @@ func testAccStepConfigWithCreds(t *testing.T, accessKey *awsAccessKey) logicalte } } -func testAccStepReadUser(t *testing.T, name string) logicaltest.TestStep { +func testAccStepReadUser(t *testing.T, name string, credentialTests []credentialTestFunc) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.ReadOperation, Path: "creds/" + name, @@ -362,40 +376,108 @@ func testAccStepReadUser(t *testing.T, name string) logicaltest.TestStep { var d struct { AccessKey string `mapstructure:"access_key"` SecretKey string `mapstructure:"secret_key"` + STSToken string `mapstructure:"security_token"` } if err := mapstructure.Decode(resp.Data, &d); err != nil { return err } log.Printf("[WARN] Generated credentials: %v", d) - - // Build a client and verify that the credentials work - creds := credentials.NewStaticCredentials(d.AccessKey, d.SecretKey, "") - awsConfig := &aws.Config{ - Credentials: creds, - Region: aws.String("us-east-1"), - HTTPClient: cleanhttp.DefaultClient(), - } - client := ec2.New(session.New(awsConfig)) - - log.Printf("[WARN] Verifying that the generated credentials work...") - retryCount := 0 - success := false - var err error - for !success && retryCount < 10 { - _, err = client.DescribeInstances(&ec2.DescribeInstancesInput{}) - if err == nil { - return nil + for _, test := range credentialTests { + err := test(d.AccessKey, d.SecretKey, d.STSToken) + if err != nil { + return err } - time.Sleep(time.Second) - retryCount++ } - - return err + return nil }, } } -func testAccStepReadSTS(t *testing.T, name string) logicaltest.TestStep { +func describeInstancesTest(accessKey, secretKey, token string) error { + creds := credentials.NewStaticCredentials(accessKey, secretKey, token) + awsConfig := &aws.Config{ + Credentials: creds, + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), + } + client := ec2.New(session.New(awsConfig)) + log.Printf("[WARN] Verifying that the generated credentials work with ec2:DescribeInstances...") + return retryUntilSuccess(func() error { + _, err := client.DescribeInstances(&ec2.DescribeInstancesInput{}) + return err + }) +} + +func describeAzsTestUnauthorized(accessKey, secretKey, token string) error { + creds := credentials.NewStaticCredentials(accessKey, secretKey, token) + awsConfig := &aws.Config{ + Credentials: creds, + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), + } + client := ec2.New(session.New(awsConfig)) + log.Printf("[WARN] Verifying that the generated credentials don't work with ec2:DescribeAvailabilityZones...") + return retryUntilSuccess(func() error { + _, err := client.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{}) + // Need to make sure AWS authenticates the generated credentials but does not authorize the operation + if err == nil { + return fmt.Errorf("operation succeeded when expected failure") + } + if aerr, ok := err.(awserr.Error); ok { + if aerr.Code() == "UnauthorizedOperation" { + return nil + } + } + return err + }) +} + +func listIamUsersTest(accessKey, secretKey, token string) error { + creds := credentials.NewStaticCredentials(accessKey, secretKey, token) + awsConfig := &aws.Config{ + Credentials: creds, + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), + } + client := iam.New(session.New(awsConfig)) + log.Printf("[WARN] Verifying that the generated credentials work with iam:ListUsers...") + return retryUntilSuccess(func() error { + _, err := client.ListUsers(&iam.ListUsersInput{}) + return err + }) +} + +func listDynamoTablesTest(accessKey, secretKey, token string) error { + creds := credentials.NewStaticCredentials(accessKey, secretKey, token) + awsConfig := &aws.Config{ + Credentials: creds, + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), + } + client := dynamodb.New(session.New(awsConfig)) + log.Printf("[WARN] Verifying that the generated credentials work with dynamodb:ListTables...") + return retryUntilSuccess(func() error { + _, err := client.ListTables(&dynamodb.ListTablesInput{}) + return err + }) +} + +func retryUntilSuccess(op func() error) error { + retryCount := 0 + success := false + var err error + for !success && retryCount < 10 { + err = op() + if err == nil { + return nil + } + time.Sleep(time.Second) + retryCount++ + } + return err +} + +func testAccStepReadSTS(t *testing.T, name string, credentialTests []credentialTestFunc) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.ReadOperation, Path: "sts/" + name, @@ -409,22 +491,12 @@ func testAccStepReadSTS(t *testing.T, name string) logicaltest.TestStep { return err } log.Printf("[WARN] Generated credentials: %v", d) - - // Build a client and verify that the credentials work - creds := credentials.NewStaticCredentials(d.AccessKey, d.SecretKey, d.STSToken) - awsConfig := &aws.Config{ - Credentials: creds, - Region: aws.String("us-east-1"), - HTTPClient: cleanhttp.DefaultClient(), - } - client := ec2.New(session.New(awsConfig)) - - log.Printf("[WARN] Verifying that the generated credentials work...") - _, err := client.DescribeInstances(&ec2.DescribeInstancesInput{}) - if err != nil { - return err + for _, test := range credentialTests { + err := test(d.AccessKey, d.SecretKey, d.STSToken) + if err != nil { + return err + } } - return nil }, } @@ -437,7 +509,7 @@ func testAccStepReadSTSWithArnPolicy(t *testing.T, name string) logicaltest.Test ErrorOk: true, Check: func(resp *logical.Response) error { if resp.Data["error"] != - "Can't generate STS credentials for a managed policy; use a role to assume or an inline policy instead" { + "attempted to retrieve iam_user credentials through the sts path; this is not allowed for legacy roles" { t.Fatalf("bad: %v", resp) } return nil @@ -450,7 +522,7 @@ func testAccStepWritePolicy(t *testing.T, name string, policy string) logicaltes Operation: logical.UpdateOperation, Path: "roles/" + name, Data: map[string]interface{}{ - "policy": testPolicy, + "policy": policy, }, } } @@ -475,31 +547,28 @@ func testAccStepReadPolicy(t *testing.T, name string, value string) logicaltest. return fmt.Errorf("bad: %#v", resp) } - var d struct { - Policy string `mapstructure:"policy"` + expected := map[string]interface{}{ + "policy_arns": []string(nil), + "role_arns": []string(nil), + "policy_document": value, + "credential_types": []string{iamUserCred, federationTokenCred}, } - if err := mapstructure.Decode(resp.Data, &d); err != nil { - return err + if !reflect.DeepEqual(resp.Data, expected) { + return fmt.Errorf("bad: got: %#v\nexpected: %#v", resp.Data, expected) } - - if d.Policy != value { - return fmt.Errorf("bad: %#v", resp) - } - return nil }, } } -const testPolicy = ` -{ +const testDynamoPolicy = `{ "Version": "2012-10-17", "Statement": [ { "Sid": "Stmt1426528957000", "Effect": "Allow", "Action": [ - "ec2:*" + "dynamodb:List*" ], "Resource": [ "*" @@ -509,14 +578,42 @@ const testPolicy = ` } ` -const testPolicyArn = "arn:aws:iam::aws:policy/AmazonEC2ReadOnlyAccess" +const ec2PolicyArn = "arn:aws:iam::aws:policy/AmazonEC2ReadOnlyAccess" +const iamPolicyArn = "arn:aws:iam::aws:policy/IAMReadOnlyAccess" + +func testAccStepWriteRole(t *testing.T, name string, data map[string]interface{}) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.UpdateOperation, + Path: "roles/" + name, + Data: data, + } +} + +func testAccStepReadRole(t *testing.T, name string, expected map[string]interface{}) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.ReadOperation, + Path: "roles/" + name, + Check: func(resp *logical.Response) error { + if resp == nil { + if expected == nil { + return nil + } + return fmt.Errorf("bad: nil response") + } + if !reflect.DeepEqual(resp.Data, expected) { + return fmt.Errorf("bad: got %#v\nexpected: %#v", resp.Data, expected) + } + return nil + }, + } +} func testAccStepWriteArnPolicyRef(t *testing.T, name string, arn string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, Path: "roles/" + name, Data: map[string]interface{}{ - "arn": testPolicyArn, + "arn": ec2PolicyArn, }, } } @@ -528,20 +625,92 @@ func TestBackend_basicPolicyArnRef(t *testing.T) { Backend: getBackend(t), Steps: []logicaltest.TestStep{ testAccStepConfig(t), - testAccStepWriteArnPolicyRef(t, "test", testPolicyArn), - testAccStepReadUser(t, "test"), + testAccStepWriteArnPolicyRef(t, "test", ec2PolicyArn), + testAccStepReadUser(t, "test", []credentialTestFunc{describeInstancesTest}), }, }) } +func TestBackend_iamUserManagedInlinePolicies(t *testing.T) { + compacted, err := compactJSON(testDynamoPolicy) + if err != nil { + t.Fatalf("bad: %#v", err) + } + roleData := map[string]interface{}{ + "policy_document": testDynamoPolicy, + "policy_arns": []string{ec2PolicyArn, iamPolicyArn}, + "credential_type": iamUserCred, + } + expectedRoleData := map[string]interface{}{ + "policy_document": compacted, + "policy_arns": []string{ec2PolicyArn, iamPolicyArn}, + "credential_types": []string{iamUserCred}, + "role_arns": []string(nil), + } + logicaltest.Test(t, logicaltest.TestCase{ + AcceptanceTest: true, + PreCheck: func() { testAccPreCheck(t) }, + Backend: getBackend(t), + Steps: []logicaltest.TestStep{ + testAccStepConfig(t), + testAccStepWriteRole(t, "test", roleData), + testAccStepReadRole(t, "test", expectedRoleData), + testAccStepReadUser(t, "test", []credentialTestFunc{describeInstancesTest, listIamUsersTest, listDynamoTablesTest}), + testAccStepReadSTS(t, "test", []credentialTestFunc{describeInstancesTest, listIamUsersTest, listDynamoTablesTest}), + }, + }) +} + +func TestBackend_AssumedRoleWithPolicyDoc(t *testing.T) { + // This looks a bit curious. The policy document and the role document act + // as a logical intersection of policies. The role allows ec2:Describe* + // (among other permissions). This policy allows everything BUT + // ec2:DescribeAvailabilityZones. Thus, the logical intersection of the two + // is all ec2:Describe* EXCEPT ec2:DescribeAvailabilityZones, and so the + // describeAZs call should fail + allowAllButDescribeAzs := ` +{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "NotAction": "ec2:DescribeAvailabilityZones", + "Resource": "*" + }] +} +` + roleData := map[string]interface{}{ + "policy_document": allowAllButDescribeAzs, + "role_arns": []string{fmt.Sprintf("arn:aws:iam::%s:role/%s", os.Getenv("AWS_ACCOUNT_ID"), testRoleName)}, + "credential_type": assumedRoleCred, + } + logicaltest.Test(t, logicaltest.TestCase{ + AcceptanceTest: true, + PreCheck: func() { + testAccPreCheck(t) + createRole(t) + // Sleep sometime because AWS is eventually consistent + log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...") + time.Sleep(10 * time.Second) + }, + Backend: getBackend(t), + Steps: []logicaltest.TestStep{ + testAccStepConfig(t), + testAccStepWriteRole(t, "test", roleData), + testAccStepReadSTS(t, "test", []credentialTestFunc{describeInstancesTest, describeAzsTestUnauthorized}), + testAccStepReadUser(t, "test", []credentialTestFunc{describeInstancesTest, describeAzsTestUnauthorized}), + }, + Teardown: deleteTestRole, + }) +} + func TestBackend_policyArnCrud(t *testing.T) { logicaltest.Test(t, logicaltest.TestCase{ AcceptanceTest: true, Backend: getBackend(t), Steps: []logicaltest.TestStep{ testAccStepConfig(t), - testAccStepWriteArnPolicyRef(t, "test", testPolicyArn), - testAccStepReadArnPolicy(t, "test", testPolicyArn), + testAccStepWriteArnPolicyRef(t, "test", ec2PolicyArn), + testAccStepReadArnPolicy(t, "test", ec2PolicyArn), testAccStepDeletePolicy(t, "test"), testAccStepReadArnPolicy(t, "test", ""), }, @@ -561,15 +730,14 @@ func testAccStepReadArnPolicy(t *testing.T, name string, value string) logicalte return fmt.Errorf("bad: %#v", resp) } - var d struct { - Policy string `mapstructure:"arn"` + expected := map[string]interface{}{ + "policy_arns": []string{value}, + "role_arns": []string(nil), + "policy_document": "", + "credential_types": []string{iamUserCred}, } - if err := mapstructure.Decode(resp.Data, &d); err != nil { - return err - } - - if d.Policy != value { - return fmt.Errorf("bad: %#v", resp) + if !reflect.DeepEqual(resp.Data, expected) { + return fmt.Errorf("bad: got: %#v\nexpected: %#v", resp.Data, expected) } return nil @@ -591,3 +759,5 @@ type awsAccessKey struct { AccessKeyId string SecretAccessKey string } + +type credentialTestFunc func(string, string, string) error diff --git a/builtin/logical/aws/path_roles.go b/builtin/logical/aws/path_roles.go index 128e5b59fa48..58fa8666d8c6 100644 --- a/builtin/logical/aws/path_roles.go +++ b/builtin/logical/aws/path_roles.go @@ -4,11 +4,13 @@ import ( "bytes" "context" "encoding/json" - "fmt" - "errors" + "fmt" + "regexp" "strings" + "github.com/hashicorp/vault/helper/consts" + "github.com/hashicorp/vault/helper/strutil" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" ) @@ -26,7 +28,7 @@ func pathListRoles(b *backend) *framework.Path { } } -func pathRoles() *framework.Path { +func pathRoles(b *backend) *framework.Path { return &framework.Path{ Pattern: "roles/" + framework.GenericNameRegex("name"), Fields: map[string]*framework.FieldSchema{ @@ -35,21 +37,46 @@ func pathRoles() *framework.Path { Description: "Name of the policy", }, - "arn": &framework.FieldSchema{ + "credential_type": &framework.FieldSchema{ Type: framework.TypeString, - Description: "ARN Reference to a managed policy", + Description: fmt.Sprintf("Type of credential to retrieve. Must be one of %s, %s, or %s", assumedRoleCred, iamUserCred, federationTokenCred), + }, + + "role_arns": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: "ARNs of AWS roles allowed to be assumed. Only valid when credential_type is " + assumedRoleCred, + }, + + "policy_arns": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: "ARNs of AWS policies to attach to IAM users. Only valid when credential_type is " + iamUserCred, + }, + + "policy_document": &framework.FieldSchema{ + Type: framework.TypeString, // TODO: Investigate adding a TypeJSON + Description: `JSON-encoded IAM policy document. Behavior varies by credential_type. When credential_type is +iam_user, then it will attach the contents of the policy_document to the IAM +user generated. When credential_type is assumed_role or federation_token, this +will be passed in as the Policy parameter to the AssumeRole or +GetFederationToken API call, acting as a filter on permissions available.`, + }, + + "arn": &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Deprecated; use role_arns or policy_arns instead. ARN Reference to a managed policy +or IAM role to assume`, }, "policy": &framework.FieldSchema{ Type: framework.TypeString, - Description: "IAM policy document", + Description: "Deprecated; use policy_document instead. IAM policy document", }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.DeleteOperation: pathRolesDelete, - logical.ReadOperation: pathRolesRead, - logical.UpdateOperation: pathRolesWrite, + logical.DeleteOperation: b.pathRolesDelete, + logical.ReadOperation: b.pathRolesRead, + logical.UpdateOperation: b.pathRolesWrite, }, HelpSynopsis: pathRolesHelpSyn, @@ -58,24 +85,33 @@ func pathRoles() *framework.Path { } func (b *backend) pathRoleList(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - entries, err := req.Storage.List(ctx, "policy/") + b.roleMutex.RLock() + defer b.roleMutex.RUnlock() + entries, err := req.Storage.List(ctx, "role/") if err != nil { return nil, err } - return logical.ListResponse(entries), nil -} - -func pathRolesDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - err := req.Storage.Delete(ctx, "policy/"+d.Get("name").(string)) + legacyEntries, err := req.Storage.List(ctx, "policy/") if err != nil { return nil, err } + return logical.ListResponse(append(entries, legacyEntries...)), nil +} + +func (b *backend) pathRolesDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + for _, prefix := range []string{"policy/", "role/"} { + err := req.Storage.Delete(ctx, prefix+d.Get("name").(string)) + if err != nil { + return nil, err + } + } + return nil, nil } -func pathRolesRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - entry, err := req.Storage.Get(ctx, "policy/"+d.Get("name").(string)) +func (b *backend) pathRolesRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + entry, err := b.lockedRoleRead(ctx, req.Storage, d.Get("name").(string)) if err != nil { return nil, err } @@ -83,69 +119,303 @@ func pathRolesRead(ctx context.Context, req *logical.Request, d *framework.Field return nil, nil } - val := string(entry.Value) - if strings.HasPrefix(val, "arn:") { - return &logical.Response{ - Data: map[string]interface{}{ - "arn": val, - }, - }, nil - } return &logical.Response{ - Data: map[string]interface{}{ - "policy": val, - }, + Data: entry.toResponseData(), }, nil } -func useInlinePolicy(d *framework.FieldData) (bool, error) { - bp := d.Get("policy").(string) != "" - ba := d.Get("arn").(string) != "" +func legacyRoleData(d *framework.FieldData) (string, error) { + policy := d.Get("policy").(string) + arn := d.Get("arn").(string) - if !bp && !ba { - return false, errors.New("either policy or arn must be provided") + switch { + case policy == "" && arn == "": + return "", nil + case policy != "" && arn != "": + return "", errors.New("only one of policy or arn should be provided") + case policy != "": + return policy, nil + default: + return arn, nil } - if bp && ba { - return false, errors.New("only one of policy or arn should be provided") +} + +func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + var resp logical.Response + + roleName := d.Get("name").(string) + if roleName == "" { + return logical.ErrorResponse("missing role name"), nil } - return bp, nil + + b.roleMutex.Lock() + defer b.roleMutex.Unlock() + roleEntry, err := nonLockedRoleRead(ctx, req.Storage, roleName) + if err != nil { + return nil, err + } + if roleEntry == nil { + legacyEntry, err := req.Storage.Get(ctx, "policy/"+roleName) + if err != nil { + return nil, err + } + if legacyEntry == nil { + roleEntry = &awsRoleEntry{} + } else { + roleEntry = upgradeLegacyPolicyEntry(string(legacyEntry.Value)) + if roleEntry.InvalidData != "" { + resp.AddWarning(fmt.Sprintf("Invalid data of %q cleared out of role", roleEntry.InvalidData)) + roleEntry.InvalidData = "" + } + err = nonLockedSetAwsRole(ctx, req.Storage, roleName, roleEntry) + if err != nil { + return nil, err + } + err = req.Storage.Delete(ctx, "policy/"+roleName) + if err != nil { + return nil, err + } + } + } + + legacyRole, err := legacyRoleData(d) + if err != nil { + return nil, err + } + + if credentialTypeRaw, ok := d.GetOk("credential_type"); ok { + if legacyRole != "" { + return logical.ErrorResponse("cannot supply deprecated role or policy parameters with an explicit credential_type"), nil + } + credentialType := credentialTypeRaw.(string) + allowedCredentialTypes := []string{iamUserCred, assumedRoleCred, federationTokenCred} + if !strutil.StrListContains(allowedCredentialTypes, credentialType) { + return logical.ErrorResponse(fmt.Sprintf("unrecognized credential_type: %q, not one of %#v", credentialType, allowedCredentialTypes)), nil + } + roleEntry.CredentialTypes = []string{credentialType} + } + + if roleArnsRaw, ok := d.GetOk("role_arns"); ok { + if legacyRole != "" { + return logical.ErrorResponse("cannot supply deprecated role or policy parameters with role_arns"), nil + } + roleEntry.RoleArns = roleArnsRaw.([]string) + } + + if policyArnsRaw, ok := d.GetOk("policy_arns"); ok { + if legacyRole != "" { + return logical.ErrorResponse("cannot supply deprecated role or policy parameters with policy_arns"), nil + } + roleEntry.PolicyArns = policyArnsRaw.([]string) + } + + if policyDocumentRaw, ok := d.GetOk("policy_document"); ok { + if legacyRole != "" { + return logical.ErrorResponse("cannot supply deprecated role or policy parameters with policy_document"), nil + } + compacted, err := compactJSON(policyDocumentRaw.(string)) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("cannot parse policy document: %q", policyDocumentRaw.(string))), nil + } + roleEntry.PolicyDocument = compacted + } + + if legacyRole != "" { + roleEntry = upgradeLegacyPolicyEntry(legacyRole) + if roleEntry.InvalidData != "" { + return logical.ErrorResponse(fmt.Sprintf("unable to parse supplied data: %q", roleEntry.InvalidData)), nil + } + resp.AddWarning("Detected use of legacy role or policy paramemter. Please upgrade to use the new parameters.") + } else { + roleEntry.ProhibitFlexibleCredPath = false + } + + if len(roleEntry.CredentialTypes) < 1 { + return logical.ErrorResponse("did not supply credential_type"), nil + } + + if len(roleEntry.RoleArns) > 0 && !strutil.StrListContains(roleEntry.CredentialTypes, assumedRoleCred) { + return logical.ErrorResponse(fmt.Sprintf("cannot supply role_arns when credential_type isn't %s", assumedRoleCred)), nil + } + if len(roleEntry.PolicyArns) > 0 && !strutil.StrListContains(roleEntry.CredentialTypes, iamUserCred) { + return logical.ErrorResponse(fmt.Sprintf("cannot supply policy_arns when credential_type isn't %s", iamUserCred)), nil + } + + err = nonLockedSetAwsRole(ctx, req.Storage, roleName, roleEntry) + if err != nil { + return nil, err + } + + return &resp, nil } -func pathRolesWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - var buf bytes.Buffer +func nonLockedRoleRead(ctx context.Context, s logical.Storage, roleName string) (*awsRoleEntry, error) { + if roleName == "" { + return nil, fmt.Errorf("missing role name") + } + entry, err := s.Get(ctx, "role/"+roleName) + if err != nil { + return nil, err + } + if entry == nil { + return nil, nil + } + + var result awsRoleEntry + if err := entry.DecodeJSON(&result); err != nil { + return nil, err + } + return &result, nil +} + +func (b *backend) lockedRoleRead(ctx context.Context, s logical.Storage, roleName string) (*awsRoleEntry, error) { + b.roleMutex.RLock() + roleEntry, err := nonLockedRoleRead(ctx, s, roleName) + b.roleMutex.RUnlock() - uip, err := useInlinePolicy(d) if err != nil { return nil, err } + if roleEntry != nil { + return roleEntry, nil + } + + b.roleMutex.Lock() + defer b.roleMutex.Unlock() + legacyEntry, err := s.Get(ctx, "policy/"+roleName) + if err != nil { + return nil, err + } + if legacyEntry == nil { + return nil, nil + } - if uip { - if err := json.Compact(&buf, []byte(d.Get("policy").(string))); err != nil { - return logical.ErrorResponse(fmt.Sprintf( - "Error compacting policy: %s", err)), nil + newRoleEntry := upgradeLegacyPolicyEntry(string(legacyEntry.Value)) + if b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary) { + err = nonLockedSetAwsRole(ctx, s, roleName, newRoleEntry) + if err != nil { + return nil, err } - // Write the policy into storage - err := req.Storage.Put(ctx, &logical.StorageEntry{ - Key: "policy/" + d.Get("name").(string), - Value: buf.Bytes(), - }) + // This can leave legacy data around in the policy/ path if it fails for some reason, + // but should be pretty rare for this to fail but prior writes to succeed, so not worrying + // about cleaning it up in case of error + err = s.Delete(ctx, "policy/"+roleName) if err != nil { return nil, err } + } + return newRoleEntry, nil +} + +func upgradeLegacyPolicyEntry(entry string) *awsRoleEntry { + var newRoleEntry *awsRoleEntry + if strings.HasPrefix(entry, "arn:") { + arnRegex := regexp.MustCompile(`^arn:aws[a-z-]*:iam::(?P\d{12}|aws):(?P\w+)/(?P.+)$`) + parsedArn := arnRegex.FindStringSubmatch(entry) + if parsedArn == nil { + newRoleEntry = &awsRoleEntry{ + InvalidData: entry, + Version: 1, + } + return newRoleEntry + } + switch parsedArn[2] { + case "role": + newRoleEntry = &awsRoleEntry{ + CredentialTypes: []string{assumedRoleCred}, + RoleArns: []string{entry}, + ProhibitFlexibleCredPath: true, + Version: 1, + } + case "policy": + newRoleEntry = &awsRoleEntry{ + CredentialTypes: []string{iamUserCred}, + PolicyArns: []string{entry}, + ProhibitFlexibleCredPath: true, + Version: 1, + } + default: + newRoleEntry = &awsRoleEntry{ + InvalidData: entry, + Version: 1, + } + } } else { - // Write the arn ref into storage - err := req.Storage.Put(ctx, &logical.StorageEntry{ - Key: "policy/" + d.Get("name").(string), - Value: []byte(d.Get("arn").(string)), - }) + compacted, err := compactJSON(entry) if err != nil { - return nil, err + newRoleEntry = &awsRoleEntry{ + InvalidData: entry, + Version: 1, + } + } else { + // unfortunately, this is ambiguous between the cred types, so allow both + newRoleEntry = &awsRoleEntry{ + CredentialTypes: []string{iamUserCred, federationTokenCred}, + PolicyDocument: compacted, + ProhibitFlexibleCredPath: true, + Version: 1, + } } } - return nil, nil + return newRoleEntry } +func nonLockedSetAwsRole(ctx context.Context, s logical.Storage, roleName string, roleEntry *awsRoleEntry) error { + if roleName == "" { + return fmt.Errorf("empty role name") + } + if roleEntry == nil { + return fmt.Errorf("nil roleEntry") + } + entry, err := logical.StorageEntryJSON("role/"+roleName, roleEntry) + if err != nil { + return err + } + if entry == nil { + return fmt.Errorf("nil result when writing to storage") + } + if err := s.Put(ctx, entry); err != nil { + return err + } + return nil +} + +type awsRoleEntry struct { + CredentialTypes []string `json:"credential_types"` // Entries must all be in the set of ("iam_user", "assumed_role", "federation_token") + PolicyArns []string `json:"policy_arns"` // ARNs of managed policies to attach to an IAM user + RoleArns []string `json:"role_arns"` // ARNs of roles to assume for AssumedRole credentials + PolicyDocument string `json:"policy_document"` // JSON-serialized inline policy to attach to IAM users and/or to specify as the Policy parameter in AssumeRole calls + InvalidData string `json:"invalid_data,omitempty"` // Invalid role data. Exists to support converting the legacy role data into the new format + ProhibitFlexibleCredPath bool `json:"prohibit_flexible_cred_path,omitempty"` // Disallow accessing STS credentials via the creds path and vice verse + Version int `json:"version"` // Version number of the role format +} + +func (r *awsRoleEntry) toResponseData() map[string]interface{} { + respData := map[string]interface{}{ + "credential_types": r.CredentialTypes, + "policy_arns": r.PolicyArns, + "role_arns": r.RoleArns, + "policy_document": r.PolicyDocument, + } + if r.InvalidData != "" { + respData["invalid_data"] = r.InvalidData + } + return respData +} + +func compactJSON(input string) (string, error) { + var compacted bytes.Buffer + err := json.Compact(&compacted, []byte(input)) + return compacted.String(), err +} + +const ( + assumedRoleCred = "assumed_role" + iamUserCred = "iam_user" + federationTokenCred = "federation_token" +) + const pathListRolesHelpSyn = `List the existing roles in this backend` const pathListRolesHelpDesc = `Roles will be listed by the role name.` diff --git a/builtin/logical/aws/path_roles_test.go b/builtin/logical/aws/path_roles_test.go index 54c8019e6578..d3705c0bd97d 100644 --- a/builtin/logical/aws/path_roles_test.go +++ b/builtin/logical/aws/path_roles_test.go @@ -2,6 +2,7 @@ package aws import ( "context" + "reflect" "strconv" "testing" @@ -20,7 +21,8 @@ func TestBackend_PathListRoles(t *testing.T) { } roleData := map[string]interface{}{ - "arn": "testarn", + "role_arns": []string{"arn:aws:iam::123456789012:role/path/RoleName"}, + "credential_type": assumedRoleCred, } roleReq := &logical.Request{ @@ -63,3 +65,90 @@ func TestBackend_PathListRoles(t *testing.T) { t.Fatalf("failed to list all 10 roles") } } + +func TestUpgradeLegacyPolicyEntry(t *testing.T) { + var input string + var expected awsRoleEntry + var output *awsRoleEntry + + input = "arn:aws:iam::123456789012:role/path/RoleName" + expected = awsRoleEntry{ + CredentialTypes: []string{assumedRoleCred}, + RoleArns: []string{input}, + ProhibitFlexibleCredPath: true, + Version: 1, + } + output = upgradeLegacyPolicyEntry(input) + if output.InvalidData != "" { + t.Fatalf("bad: error processing upgrade of %q: got invalid data of %v", input, output.InvalidData) + } + if !reflect.DeepEqual(*output, expected) { + t.Fatalf("bad: expected %#v; received %#v", expected, *output) + } + + input = "arn:aws:iam::123456789012:policy/MyPolicy" + expected = awsRoleEntry{ + CredentialTypes: []string{iamUserCred}, + PolicyArns: []string{input}, + ProhibitFlexibleCredPath: true, + Version: 1, + } + output = upgradeLegacyPolicyEntry(input) + if output.InvalidData != "" { + t.Fatalf("bad: error processing upgrade of %q: got invalid data of %v", input, output.InvalidData) + } + if !reflect.DeepEqual(*output, expected) { + t.Fatalf("bad: expected %#v; received %#v", expected, *output) + } + + input = "arn:aws:iam::aws:policy/AWSManagedPolicy" + expected.PolicyArns = []string{input} + output = upgradeLegacyPolicyEntry(input) + if output.InvalidData != "" { + t.Fatalf("bad: error processing upgrade of %q: got invalid data of %v", input, output.InvalidData) + } + if !reflect.DeepEqual(*output, expected) { + t.Fatalf("bad: expected %#v; received %#v", expected, *output) + } + + input = ` +{ + "Version": "2012-10-07", + "Statement": [ + { + "Effect": "Allow", + "Action": "ec2:Describe*", + "Resource": "*" + } + ] +}` + compacted, err := compactJSON(input) + if err != nil { + t.Fatalf("error parsing JSON: %v", err) + } + expected = awsRoleEntry{ + CredentialTypes: []string{iamUserCred, federationTokenCred}, + PolicyDocument: compacted, + ProhibitFlexibleCredPath: true, + Version: 1, + } + output = upgradeLegacyPolicyEntry(input) + if output.InvalidData != "" { + t.Fatalf("bad: error processing upgrade of %q: got invalid data of %v", input, output.InvalidData) + } + if !reflect.DeepEqual(*output, expected) { + t.Fatalf("bad: expected %#v; received %#v", expected, *output) + } + + // Due to lack of prior input validation, this could exist in the storage, and we need + // to be able to read it out in some fashion, so have to handle this in a poor fashion + input = "arn:gobbledygook" + expected = awsRoleEntry{ + InvalidData: input, + Version: 1, + } + output = upgradeLegacyPolicyEntry(input) + if !reflect.DeepEqual(*output, expected) { + t.Fatalf("bad: expected %#v; received %#v", expected, *output) + } +} diff --git a/builtin/logical/aws/path_sts.go b/builtin/logical/aws/path_sts.go deleted file mode 100644 index 2fe195359018..000000000000 --- a/builtin/logical/aws/path_sts.go +++ /dev/null @@ -1,95 +0,0 @@ -package aws - -import ( - "context" - "fmt" - "strings" - - "github.com/hashicorp/errwrap" - "github.com/hashicorp/vault/logical" - "github.com/hashicorp/vault/logical/framework" -) - -func pathSTS(b *backend) *framework.Path { - return &framework.Path{ - Pattern: "sts/" + framework.GenericNameRegex("name"), - Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ - Type: framework.TypeString, - Description: "Name of the role", - }, - "ttl": &framework.FieldSchema{ - Type: framework.TypeDurationSecond, - Description: `Lifetime of the token in seconds. -AWS documentation excerpt: The duration, in seconds, that the credentials -should remain valid. Acceptable durations for IAM user sessions range from 900 -seconds (15 minutes) to 129600 seconds (36 hours), with 43200 seconds (12 -hours) as the default. Sessions for AWS account owners are restricted to a -maximum of 3600 seconds (one hour). If the duration is longer than one hour, -the session for AWS account owners defaults to one hour.`, - Default: 3600, - }, - }, - - Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.ReadOperation: b.pathSTSRead, - logical.UpdateOperation: b.pathSTSRead, - }, - - HelpSynopsis: pathSTSHelpSyn, - HelpDescription: pathSTSHelpDesc, - } -} - -func (b *backend) pathSTSRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - policyName := d.Get("name").(string) - ttl := int64(d.Get("ttl").(int)) - - // Read the policy - policy, err := req.Storage.Get(ctx, "policy/"+policyName) - if err != nil { - return nil, errwrap.Wrapf("error retrieving role: {{err}}", err) - } - if policy == nil { - return logical.ErrorResponse(fmt.Sprintf( - "Role '%s' not found", policyName)), nil - } - policyValue := string(policy.Value) - if strings.HasPrefix(policyValue, "arn:") { - if strings.Contains(policyValue, ":role/") { - return b.assumeRole( - ctx, - req.Storage, - req.DisplayName, policyName, policyValue, - ttl, - ) - } else { - return logical.ErrorResponse( - "Can't generate STS credentials for a managed policy; use a role to assume or an inline policy instead"), - logical.ErrInvalidRequest - } - } - // Use the helper to create the secret - return b.secretTokenCreate( - ctx, - req.Storage, - req.DisplayName, policyName, policyValue, - ttl, - ) -} - -const pathSTSHelpSyn = ` -Generate an access key pair + security token for a specific role. -` - -const pathSTSHelpDesc = ` -This path will generate a new, never before used key pair + security token for -accessing AWS. The IAM policy used to back this key pair will be -the "name" parameter. For example, if this backend is mounted at "aws", -then "aws/sts/deploy" would generate access keys for the "deploy" role. - -Note, these credentials are instantiated using the AWS STS backend. - -The access keys will have a lease associated with them, but revoking the lease -does not revoke the access keys. -` diff --git a/builtin/logical/aws/path_user.go b/builtin/logical/aws/path_user.go index 74a4ae0bfe5c..92587bc67c0c 100644 --- a/builtin/logical/aws/path_user.go +++ b/builtin/logical/aws/path_user.go @@ -3,10 +3,12 @@ package aws import ( "context" "fmt" + "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" "github.com/hashicorp/errwrap" + "github.com/hashicorp/vault/helper/strutil" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" "github.com/mitchellh/mapstructure" @@ -14,16 +16,24 @@ import ( func pathUser(b *backend) *framework.Path { return &framework.Path{ - Pattern: "creds/" + framework.GenericNameRegex("name"), + Pattern: "(creds|sts)/" + framework.GenericNameRegex("name"), Fields: map[string]*framework.FieldSchema{ "name": &framework.FieldSchema{ Type: framework.TypeString, Description: "Name of the role", }, + "role_arn": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "ARN of role to assume when credential_type is " + assumedRoleCred, + }, + "ttl": &framework.FieldSchema{ + Type: framework.TypeDurationSecond, + Description: "Lifetime of the returned credentials in seconds", + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.ReadOperation: b.pathUserRead, + logical.ReadOperation: b.pathCredsRead, }, HelpSynopsis: pathUserHelpSyn, @@ -31,22 +41,78 @@ func pathUser(b *backend) *framework.Path { } } -func (b *backend) pathUserRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - policyName := d.Get("name").(string) +func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + roleName := d.Get("name").(string) // Read the policy - policy, err := req.Storage.Get(ctx, "policy/"+policyName) + role, err := b.lockedRoleRead(ctx, req.Storage, roleName) if err != nil { return nil, errwrap.Wrapf("error retrieving role: {{err}}", err) } - if policy == nil { + if role == nil { return logical.ErrorResponse(fmt.Sprintf( - "Role '%s' not found", policyName)), nil + "Role '%s' not found", roleName)), nil + } + + ttl := int64(d.Get("ttl").(int)) + roleArn := d.Get("role_arn").(string) + + var credentialType string + switch { + case len(role.CredentialTypes) == 1: + credentialType = role.CredentialTypes[0] + // There is only one way for the CredentialTypes to contain more than one entry, and that's an upgrade path + // where it contains iamUserCred and federationTokenCred + // This ambiguity can be resolved based on req.Path, so resolve it assuming CredentialTypes only has those values + case len(role.CredentialTypes) > 1: + if strings.HasPrefix(req.Path, "creds") { + credentialType = iamUserCred + } else { + credentialType = federationTokenCred + } + // sanity check on the assumption above + if !strutil.StrListContains(role.CredentialTypes, credentialType) { + return logical.ErrorResponse(fmt.Sprintf("requested credential type %q not in allowed credential types %#v", credentialType, role.CredentialTypes)), nil + } } - // Use the helper to create the secret - return b.secretAccessKeysCreate( - ctx, req.Storage, req.DisplayName, policyName, string(policy.Value)) + // creds requested through the sts path shouldn't be allowed to get iamUserCred type creds + // when the role is created from legacy data because they might have more privileges in AWS. + // See https://github.com/hashicorp/vault/issues/4229#issuecomment-380316788 for details. + if role.ProhibitFlexibleCredPath { + if credentialType == iamUserCred && strings.HasPrefix(req.Path, "sts") { + return logical.ErrorResponse(fmt.Sprintf("attempted to retrieve %s credentials through the sts path; this is not allowed for legacy roles", iamUserCred)), nil + } + if credentialType != iamUserCred && strings.HasPrefix(req.Path, "creds") { + return logical.ErrorResponse(fmt.Sprintf("attempted to retrieve %s credentials through the creds path; this is not allowed for legacy roles", credentialType)), nil + } + } + + switch credentialType { + case iamUserCred: + return b.secretAccessKeysCreate(ctx, req.Storage, req.DisplayName, roleName, role) + case assumedRoleCred: + if ttl == 0 { + ttl = 3600 + } + switch { + case roleArn == "": + if len(role.RoleArns) != 1 { + return logical.ErrorResponse("did not supply a role_arn parameter and unable to determine one"), nil + } + roleArn = role.RoleArns[0] + case !strutil.StrListContains(role.RoleArns, roleArn): + return logical.ErrorResponse(fmt.Sprintf("role_arn %q not in allowed role arns for Vault role %q", roleArn, roleName)), nil + } + return b.assumeRole(ctx, req.Storage, req.DisplayName, roleName, roleArn, role.PolicyDocument, ttl) + case federationTokenCred: + if ttl == 0 { + ttl = 3600 + } + return b.secretTokenCreate(ctx, req.Storage, req.DisplayName, roleName, role.PolicyDocument, ttl) + default: + return logical.ErrorResponse(fmt.Sprintf("unknown credential_type: %q", credentialType)), nil + } } func pathUserRollback(ctx context.Context, req *logical.Request, _kind string, data interface{}) error { @@ -161,15 +227,17 @@ type walUser struct { } const pathUserHelpSyn = ` -Generate an access key pair for a specific role. +Generate AWS credentials from a specific Vault role. ` const pathUserHelpDesc = ` -This path will generate a new, never before used key pair for +This path will generate new, never before used AWS credentials for accessing AWS. The IAM policy used to back this key pair will be the "name" parameter. For example, if this backend is mounted at "aws", then "aws/creds/deploy" would generate access keys for the "deploy" role. The access keys will have a lease associated with them. The access keys -can be revoked by using the lease ID. +can be revoked by using the lease ID when using the iam_user credential type. +When using AWS STS credential types (assumed_role or federation_token), +revoking the lease does not revoke the access keys. ` diff --git a/builtin/logical/aws/secret_access_keys.go b/builtin/logical/aws/secret_access_keys.go index c45b7eb5a6c2..57e1105da69b 100644 --- a/builtin/logical/aws/secret_access_keys.go +++ b/builtin/logical/aws/secret_access_keys.go @@ -7,8 +7,6 @@ import ( "regexp" "time" - "strings" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/sts" @@ -112,21 +110,24 @@ func (b *backend) secretTokenCreate(ctx context.Context, s logical.Storage, } func (b *backend) assumeRole(ctx context.Context, s logical.Storage, - displayName, policyName, policy string, + displayName, roleName, roleArn, policy string, lifeTimeInSeconds int64) (*logical.Response, error) { STSClient, err := clientSTS(ctx, s) if err != nil { return logical.ErrorResponse(err.Error()), nil } - username, usernameWarning := genUsername(displayName, policyName, "iam_user") + username, usernameWarning := genUsername(displayName, roleName, "iam_user") - tokenResp, err := STSClient.AssumeRole( - &sts.AssumeRoleInput{ - RoleSessionName: aws.String(username), - RoleArn: aws.String(policy), - DurationSeconds: &lifeTimeInSeconds, - }) + assumeRoleInput := &sts.AssumeRoleInput{ + RoleSessionName: aws.String(username), + RoleArn: aws.String(roleArn), + DurationSeconds: &lifeTimeInSeconds, + } + if policy != "" { + assumeRoleInput.SetPolicy(policy) + } + tokenResp, err := STSClient.AssumeRole(assumeRoleInput) if err != nil { return logical.ErrorResponse(fmt.Sprintf( @@ -139,7 +140,7 @@ func (b *backend) assumeRole(ctx context.Context, s logical.Storage, "security_token": *tokenResp.Credentials.SessionToken, }, map[string]interface{}{ "username": username, - "policy": policy, + "policy": roleArn, "is_sts": true, }) @@ -159,7 +160,7 @@ func (b *backend) assumeRole(ctx context.Context, s logical.Storage, func (b *backend) secretAccessKeysCreate( ctx context.Context, s logical.Storage, - displayName, policyName string, policy string) (*logical.Response, error) { + displayName, policyName string, role *awsRoleEntry) (*logical.Response, error) { client, err := clientIAM(ctx, s) if err != nil { return logical.ErrorResponse(err.Error()), nil @@ -187,23 +188,24 @@ func (b *backend) secretAccessKeysCreate( "Error creating IAM user: %s", err)), nil } - if strings.HasPrefix(policy, "arn:") { + for _, arn := range role.PolicyArns { // Attach existing policy against user _, err = client.AttachUserPolicy(&iam.AttachUserPolicyInput{ UserName: aws.String(username), - PolicyArn: aws.String(policy), + PolicyArn: aws.String(arn), }) if err != nil { return logical.ErrorResponse(fmt.Sprintf( "Error attaching user policy: %s", err)), nil } - } else { + } + if role.PolicyDocument != "" { // Add new inline user policy against user _, err = client.PutUserPolicy(&iam.PutUserPolicyInput{ UserName: aws.String(username), PolicyName: aws.String(policyName), - PolicyDocument: aws.String(policy), + PolicyDocument: aws.String(role.PolicyDocument), }) if err != nil { return logical.ErrorResponse(fmt.Sprintf( @@ -234,7 +236,7 @@ func (b *backend) secretAccessKeysCreate( "security_token": nil, }, map[string]interface{}{ "username": username, - "policy": policy, + "policy": role, "is_sts": false, }) diff --git a/website/source/api/secret/aws/index.html.md b/website/source/api/secret/aws/index.html.md index 666a5f93f1e1..8b4f97845396 100644 --- a/website/source/api/secret/aws/index.html.md +++ b/website/source/api/secret/aws/index.html.md @@ -159,6 +159,31 @@ updated with the new attributes. - `name` `(string: )` – Specifies the name of the role to create. This is part of the request URL. +- `credential_type` `(string: )` – Specifies the type of credential to be used when + retrieving credentials from the role. Must be one of `iam_user`, + `assumed_role`, or `federation_token`. + +- `role_arns` `(list: [])` – Specifies the ARNs of the AWS roles this Vault role + is allowed to assume. Required when `credential_type` is `assumed_role` and + prohibited otherwise. This is a comma-separated string or JSON array. + +- `policy_arns` `(list: [])` – Specifies the ARNs of the AWS managed policies to + be attached to IAM users when they are requsted. Valid only when + `credential_type` is `iam_user`. When `credential_type` is `iam_user`, at + least one of `policy_arns` or `policy_document` must be specified. This is a + comma-separated string or JSON array. + +- `policy_document` `(string)` – The IAM policy document for the role. The + behavior depends on the credential type. With `iam_user`, the policy document + will be attached to the IAM user generated and augment the permissions the IAM + user has. With `assumed_role` and `federation_token`, the policy document will + act as a filter on what the credentials can do. + +Legacy parameters: + +These parameters are supported for backwards compatibility only. They cannot be +mixed with the parameters listed above. + - `policy` `(string: )` – Specifies the IAM policy in JSON format. @@ -181,7 +206,8 @@ Using an inline IAM policy: ```json { - "policy": "{\"Version\": \"...\"}", + "credential_type": "federation_token", + "policy_document": "{\"Version\": \"...\"}" } ``` @@ -189,7 +215,8 @@ Using an ARN: ```json { - "arn": "arn:aws:iam::123456789012:user/David" + "credential_type": "assumed_role", + "role_arns": "arn:aws:iam::123456789012:role/DeveloperRole" } ``` @@ -202,6 +229,9 @@ exist, a 404 is returned. | :------- | :--------------------------- | :--------------------- | | `GET` | `/aws/roles/:name` | `200 application/json` | +If invalid role data was supplied to the role from an earlier version of Vault, +then it will show up in the response as `invalid_data`. + ### Parameters - `name` `(string: )` – Specifies the name of the role to read. This @@ -222,17 +252,23 @@ For an inline IAM policy: ```json { "data": { - "policy": "{\"Version\": \"...\"}" + "policy_document": "{\"Version\": \"...\"}", + "policy_arns": [], + "credential_types": ["assumed_role"], + "role_arns": [], } } ``` -For an ARN: +For a role ARN: ```json { "data": { - "arn": "arn:aws:iam::123456789012:user/David" + "policy_document": "", + "policy_arns": [], + "credential_types": ["assumed_role"], + "role_arns": ["arn:aws:iam::123456789012:role/example-role"] } } ``` @@ -289,19 +325,39 @@ $ curl \ http://127.0.0.1:8200/v1/aws/roles/example-role ``` -## Generate IAM Credentials +## Generate Credentials -This endpoint generates dynamic IAM credentials based on the named role. This -role must be created before queried. +This endpoint generates credentials based on the named role. This role must be +created before queried. | Method | Path | Produces | | :------- | :--------------------------- | :--------------------- | | `GET` | `/aws/creds/:name` | `200 application/json` | +| `GET` | `/aws/sts/:name` | `200 application/json` | + +The `/aws/creds` and `/aws/sts` endpoints are almost identical. The exception is +when retrieving credentials for a role that was specified with the legacy `arn` +or `policy` parameter. In this case, credentials retrieved through `/aws/sts` +must be of either the `assumed_role` or `federation_token` types, and +credentials retrieved through `/aws/creds` must be of the `iam_user` type. ### Parameters - `name` `(string: )` – Specifies the name of the role to generate credentials against. This is part of the request URL. +- `role_arn` `(string)` – The ARN of the role to assume if `credential_type` on + the Vault role is `assumed_role`. Must match one of the allowed role ARNs in + the Vault role. Optional if the Vault role only allows a single AWS role ARN; + required otherwise. +- `ttl` `(string: "3600s")` – Specifies the TTL for the use of the STS token. + This is specified as a string with a duration suffix. Valid only when + `credential_type` is `assumed_role` or `federation_token`. AWS places limits + on the maximum TTL allowed. See the AWS documentation on the `DurationSeconds` + parameter for + [AssumeRole](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html) + (for `assumed_role` credential types) and + [GetFederationToken](https://docs.aws.amazon.com/STS/latest/APIReference/API_GetFederationToken.html) + (for `federation_token` credential types) for more details. ### Sample Request @@ -322,56 +378,3 @@ $ curl \ } } ``` - -## Generate IAM with STS - -This generates a dynamic IAM credential with an STS token based on the named -role. - -| Method | Path | Produces | -| :------- | :--------------------------- | :--------------------- | -| `POST` | `/aws/sts/:name` | `204 (empty body)` | - -### Parameters - -- `name` `(string: )` – Specifies the name of the role against which - to create this STS credential. This is part of the request URL. - -- `ttl` `(string: "3600s")` – Specifies the TTL for the use of the STS token. - This is specified as a string with a duration suffix. AWS documentation - excerpt: `The duration, in seconds, that the credentials should remain valid. - Acceptable durations for IAM user sessions range from 900 seconds (15 - minutes) to 129600 seconds (36 hours), with 43200 seconds (12 hours) as the - default. Sessions for AWS account owners are restricted to a maximum of 3600 - seconds (one hour). If the duration is longer than one hour, the session for - AWS account owners defaults to one hour.` - -### Sample Payload - -```json -{ - "ttl": "15m" -} -``` - -### Sample Request - -``` -$ curl \ - --header "X-Vault-Token: ..." \ - --request POST \ - --data @payload.json \ - http://127.0.0.1:8200/v1/aws/sts/example-role -``` - -### Sample Response - -```json -{ - "data": { - "access_key": "AKIA...", - "secret_key": "xlCs...", - "security_token": "429255" - } -} -``` diff --git a/website/source/docs/secrets/aws/index.html.md b/website/source/docs/secrets/aws/index.html.md index fb2948ef73da..a625d7ee61ba 100644 --- a/website/source/docs/secrets/aws/index.html.md +++ b/website/source/docs/secrets/aws/index.html.md @@ -15,6 +15,20 @@ involve clicking in the web UI. Additionally, the process is codified and mapped to internal auth methods (such as LDAP). The AWS IAM credentials are time-based and are automatically revoked when the Vault lease expires. +Vault supports three different types of credentials to retrieve from AWS: + +1. `iam_user`: Vault will create an IAM user for each lease, attach the managed + and inline IAM policies as specified in the role to the user, and then return + the access key and secret key to the caller. IAM users have no session tokens + and so no session token will be returned. +2. `assumed_role`: Vault will call + [sts:AssumeRole](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html) + and return the access key, secret key, and session token to the caller. +3. `federation_token`: Vault will call + [sts:GetFederationToken](https://docs.aws.amazon.com/STS/latest/APIReference/API_GetFederationToken.html) + passing in the supplied AWS policy document and return the access key, secret + key, and session token to the caller. + ## Setup Most secrets engines must be configured in advance before they can perform their @@ -54,12 +68,14 @@ the IAM credentials: your AWS root account credentials. Instead generate a dedicated user or role. -1. Configure a role that maps a name in Vault to a policy or policy file in AWS. -When users generate credentials, they are generated against this role: +1. Configure a Vault role that maps to a set of permissions in AWS as well as an + AWS credential type. When users generate credentials, they are generated + against this role. An example: ```text $ vault write aws/roles/my-role \ - policy=-< **Notice:** Due to limitations in AWS, in order to use the `federation_token` +credential type, Vault **must** be configured with IAM user credentials. AWS +does not allow temporary credentials (such as those from an IAM instance +profile) to be used. + An STS federation token inherits a set of permissions that are the combination (intersection) of three sets of permissions: @@ -171,13 +202,15 @@ An STS federation token inherits a set of permissions that are the combination 2. The user inline policy configured for the `aws/role` 3. An implicit deny policy on IAM or STS operations. -STS federation token credentials can only be generated for user inline -policies; the AWS GetFederationToken API does not support managed policies. +Roles with a `credential_type` of `federation_token` can only specify a +`policy_document` in the Vault role. AWS does not support support managed +policies. The `aws/config/root` credentials require IAM permissions for `sts:GetFederationToken` and the permissions to delegate to the STS federation token. For example, this policy on the `aws/config/root` credentials -would allow creation of an STS federated token with delegated `ec2:*` permissions: +would allow creation of an STS federated token with delegated `ec2:*` +permissions (or any subset of `ec2:*` permissions): ```javascript { @@ -193,17 +226,19 @@ would allow creation of an STS federated token with delegated `ec2:*` permission } ``` -Our "deploy" role would then assign an inline user policy with the same `ec2:*` +An `ec2_admin` role would then assign an inline policy with the same `ec2:*` permissions. ```text -$ vault write aws/roles/deploy \ - policy=@policy.json +$ vault write aws/roles/ec2_admin \ + credential_type=federation_token \ + policy_document=@policy.json ``` The policy.json file would contain an inline policy with similar permissions, -less the `sts:GetFederationToken` permission. (We could grant `sts` permissions, -but STS would attach an implicit deny on `sts` that overrides the allow.) +less the `sts:GetFederationToken` permission. (We could grant +`sts:GetFederationToken` permissions, but STS attaches attach an implicit deny +that overrides the allow.) ```javascript { @@ -220,9 +255,9 @@ To generate a new set of STS federation token credentials, we simply write to the role using the aws/sts endpoint: ```text -$vault write aws/sts/deploy ttl=60m +$vault write aws/sts/ec2_admin ttl=60m Key Value -lease_id aws/sts/deploy/31d771a6-fb39-f46b-fdc5-945109106422 +lease_id aws/sts/ec2_admin/31d771a6-fb39-f46b-fdc5-945109106422 lease_duration 60m0s lease_renewable true access_key ASIAJYYYY2AA5K4WIXXX @@ -232,19 +267,23 @@ security_token AQoDYXdzEEwasAKwQyZUtZaCjVNDiXXXXXXXXgUgBBVUUbSyujLjsw6jYzboOQ89 ### STS AssumeRole -STS AssumeRole is typically used for cross-account authentication or single sign-on (SSO) -scenarios. AssumeRole has additional complexity compared STS federation tokens: +The `assumed_role` credential type is typically used for cross-account +authentication or single sign-on (SSO) scenarios. In order to use an +`assumed_role` credential type, you must configure outside of Vault: -1. The ARN of a IAM role to assume +1. An IAM role 2. IAM inline policies and/or managed policies attached to the IAM role -3. IAM trust policy attached to the IAM role to grant privileges for one identity - to assume the role. +3. IAM trust policy attached to the IAM role to grant privileges for Vault to + assume the role -AssumeRole adds a few benefits over federation tokens: +`assumed_role` credentials offer a few benefits over `federation_token`: 1. Assumed roles can invoke IAM and STS operations, if granted by the role's IAM policies. 2. Assumed roles support cross-account authentication +3. Temporary credentials (such as those granted by running Vault on an EC2 + instance in an IAM instance profile) can retrieve `assumed_role` credentials + (but cannot retrieve `federation_token` credentials). The `aws/config/root` credentials must have an IAM policy that allows `sts:AssumeRole` against the target role: @@ -278,11 +317,28 @@ the aws/root/config credentials to assume the role. } ``` -Finally, let's create a "deploy" policy using the arn of our role to assume: +When specifying a Vault role with a `credential_type` of `assumed_role`, you can +specify more than one IAM role ARN. If you do so, Vault clients can select which +role ARN they would like to assume when retrieving credentials from that role. +You can further specify a `policy_document` which, if specified, acts as a +filter on the IAM permissions granted to the assumed role. For an action to be +allowed, it must be permitted by both the IAM policy on the AWS role that is +assumed as well as the `policy_document` specified on the Vault role. (The +`policy_document` parameter is passed in as the `Policy` parameter to the +[sts:AssumeRole](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html) +API call.) + +Note: When multiple `role_arns` are specified, clients requesting credentials +can specify any of the role ARNs that are defined on the Vault role in order to +retrieve credentials. However, when a `policy_document` is specified, that will +apply to ALL role credentials retrieved from AWS. + +Let's create a "deploy" policy using the arn of our role to assume: ```text $ vault write aws/roles/deploy \ - arn=arn:aws:iam::ACCOUNT-ID-WITHOUT-HYPHENS:role/RoleNameToAssume + role_arns=arn:aws:iam::ACCOUNT-ID-WITHOUT-HYPHENS:role/RoleNameToAssume \ + credential_type=assumed_role ``` To generate a new set of STS assumed role credentials, we again write to From cc2933c68d4d92833ff27bf187341f9dd704c7f6 Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Sun, 15 Apr 2018 16:33:27 -0400 Subject: [PATCH 2/7] Add missing write action to STS endpoint --- builtin/logical/aws/path_user.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/logical/aws/path_user.go b/builtin/logical/aws/path_user.go index 92587bc67c0c..7e4a21687074 100644 --- a/builtin/logical/aws/path_user.go +++ b/builtin/logical/aws/path_user.go @@ -33,7 +33,8 @@ func pathUser(b *backend) *framework.Path { }, Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.ReadOperation: b.pathCredsRead, + logical.ReadOperation: b.pathCredsRead, + logical.UpdateOperation: b.pathCredsRead, }, HelpSynopsis: pathUserHelpSyn, From 42b41da6cfeccbdc0442ab6bd9ef9938d6032922 Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Sun, 15 Apr 2018 23:09:52 -0400 Subject: [PATCH 3/7] Allow unsetting policy_document with empty string This allows unsetting the policy_document by passing in an empty string. Previously, it would fail because the empty string isn't a valid JSON document. --- builtin/logical/aws/path_roles.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/logical/aws/path_roles.go b/builtin/logical/aws/path_roles.go index 58fa8666d8c6..3b6b43148edf 100644 --- a/builtin/logical/aws/path_roles.go +++ b/builtin/logical/aws/path_roles.go @@ -213,9 +213,14 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f if legacyRole != "" { return logical.ErrorResponse("cannot supply deprecated role or policy parameters with policy_document"), nil } - compacted, err := compactJSON(policyDocumentRaw.(string)) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("cannot parse policy document: %q", policyDocumentRaw.(string))), nil + var compacted string + if policyDocumentRaw.(string) == "" { + compacted = "" + } else { + compacted, err = compactJSON(policyDocumentRaw.(string)) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("cannot parse policy document: %q", policyDocumentRaw.(string))), nil + } } roleEntry.PolicyDocument = compacted } From 1d77a85333534880f0491549937e48d3d6d892a8 Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Sat, 4 Aug 2018 15:32:37 -0400 Subject: [PATCH 4/7] Respond to some PR feedback --- builtin/logical/aws/path_roles.go | 26 ++++++++++++++++---------- builtin/logical/aws/path_user.go | 7 +------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/builtin/logical/aws/path_roles.go b/builtin/logical/aws/path_roles.go index 3b6b43148edf..1b2fa0de9146 100644 --- a/builtin/logical/aws/path_roles.go +++ b/builtin/logical/aws/path_roles.go @@ -53,7 +53,7 @@ func pathRoles(b *backend) *framework.Path { }, "policy_document": &framework.FieldSchema{ - Type: framework.TypeString, // TODO: Investigate adding a TypeJSON + Type: framework.TypeString, Description: `JSON-encoded IAM policy document. Behavior varies by credential_type. When credential_type is iam_user, then it will attach the contents of the policy_document to the IAM user generated. When credential_type is assumed_role or federation_token, this @@ -167,7 +167,7 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f resp.AddWarning(fmt.Sprintf("Invalid data of %q cleared out of role", roleEntry.InvalidData)) roleEntry.InvalidData = "" } - err = nonLockedSetAwsRole(ctx, req.Storage, roleName, roleEntry) + err = setAwsRole(ctx, req.Storage, roleName, roleEntry) if err != nil { return nil, err } @@ -213,10 +213,8 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f if legacyRole != "" { return logical.ErrorResponse("cannot supply deprecated role or policy parameters with policy_document"), nil } - var compacted string - if policyDocumentRaw.(string) == "" { - compacted = "" - } else { + compacted := policyDocumentRaw.(string) + if len(compacted) > 0 { compacted, err = compactJSON(policyDocumentRaw.(string)) if err != nil { return logical.ErrorResponse(fmt.Sprintf("cannot parse policy document: %q", policyDocumentRaw.(string))), nil @@ -235,7 +233,7 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f roleEntry.ProhibitFlexibleCredPath = false } - if len(roleEntry.CredentialTypes) < 1 { + if len(roleEntry.CredentialTypes) == 0 { return logical.ErrorResponse("did not supply credential_type"), nil } @@ -246,7 +244,7 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f return logical.ErrorResponse(fmt.Sprintf("cannot supply policy_arns when credential_type isn't %s", iamUserCred)), nil } - err = nonLockedSetAwsRole(ctx, req.Storage, roleName, roleEntry) + err = setAwsRole(ctx, req.Storage, roleName, roleEntry) if err != nil { return nil, err } @@ -287,6 +285,14 @@ func (b *backend) lockedRoleRead(ctx context.Context, s logical.Storage, roleNam b.roleMutex.Lock() defer b.roleMutex.Unlock() + roleEntry, err = nonLockedRoleRead(ctx, s, roleName) + + if err != nil { + return nil, err + } + if roleEntry != nil { + return roleEntry, nil + } legacyEntry, err := s.Get(ctx, "policy/"+roleName) if err != nil { return nil, err @@ -297,7 +303,7 @@ func (b *backend) lockedRoleRead(ctx context.Context, s logical.Storage, roleNam newRoleEntry := upgradeLegacyPolicyEntry(string(legacyEntry.Value)) if b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary) { - err = nonLockedSetAwsRole(ctx, s, roleName, newRoleEntry) + err = setAwsRole(ctx, s, roleName, newRoleEntry) if err != nil { return nil, err } @@ -366,7 +372,7 @@ func upgradeLegacyPolicyEntry(entry string) *awsRoleEntry { return newRoleEntry } -func nonLockedSetAwsRole(ctx context.Context, s logical.Storage, roleName string, roleEntry *awsRoleEntry) error { +func setAwsRole(ctx context.Context, s logical.Storage, roleName string, roleEntry *awsRoleEntry) error { if roleName == "" { return fmt.Errorf("empty role name") } diff --git a/builtin/logical/aws/path_user.go b/builtin/logical/aws/path_user.go index 7e4a21687074..6fc4760333c6 100644 --- a/builtin/logical/aws/path_user.go +++ b/builtin/logical/aws/path_user.go @@ -29,6 +29,7 @@ func pathUser(b *backend) *framework.Path { "ttl": &framework.FieldSchema{ Type: framework.TypeDurationSecond, Description: "Lifetime of the returned credentials in seconds", + Default: 3600, }, }, @@ -93,9 +94,6 @@ func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *fr case iamUserCred: return b.secretAccessKeysCreate(ctx, req.Storage, req.DisplayName, roleName, role) case assumedRoleCred: - if ttl == 0 { - ttl = 3600 - } switch { case roleArn == "": if len(role.RoleArns) != 1 { @@ -107,9 +105,6 @@ func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *fr } return b.assumeRole(ctx, req.Storage, req.DisplayName, roleName, roleArn, role.PolicyDocument, ttl) case federationTokenCred: - if ttl == 0 { - ttl = 3600 - } return b.secretTokenCreate(ctx, req.Storage, req.DisplayName, roleName, role.PolicyDocument, ttl) default: return logical.ErrorResponse(fmt.Sprintf("unknown credential_type: %q", credentialType)), nil From db0e9e27c18aa210ac6069c8432a52096bfa6e0f Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Sat, 4 Aug 2018 16:06:02 -0400 Subject: [PATCH 5/7] Refactor and simplify role reading/upgrading This gets rid of the duplicated role upgrade code between both role reading and role writing by handling the upgrade all in the role reading. --- builtin/logical/aws/path_roles.go | 78 ++++++++++++------------------- builtin/logical/aws/path_user.go | 2 +- 2 files changed, 30 insertions(+), 50 deletions(-) diff --git a/builtin/logical/aws/path_roles.go b/builtin/logical/aws/path_roles.go index 1b2fa0de9146..528395a40e21 100644 --- a/builtin/logical/aws/path_roles.go +++ b/builtin/logical/aws/path_roles.go @@ -111,7 +111,7 @@ func (b *backend) pathRolesDelete(ctx context.Context, req *logical.Request, d * } func (b *backend) pathRolesRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - entry, err := b.lockedRoleRead(ctx, req.Storage, d.Get("name").(string)) + entry, err := b.roleRead(ctx, req.Storage, d.Get("name").(string), true) if err != nil { return nil, err } @@ -150,32 +150,15 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f b.roleMutex.Lock() defer b.roleMutex.Unlock() - roleEntry, err := nonLockedRoleRead(ctx, req.Storage, roleName) + roleEntry, err := b.roleRead(ctx, req.Storage, roleName, false) if err != nil { return nil, err } if roleEntry == nil { - legacyEntry, err := req.Storage.Get(ctx, "policy/"+roleName) - if err != nil { - return nil, err - } - if legacyEntry == nil { - roleEntry = &awsRoleEntry{} - } else { - roleEntry = upgradeLegacyPolicyEntry(string(legacyEntry.Value)) - if roleEntry.InvalidData != "" { - resp.AddWarning(fmt.Sprintf("Invalid data of %q cleared out of role", roleEntry.InvalidData)) - roleEntry.InvalidData = "" - } - err = setAwsRole(ctx, req.Storage, roleName, roleEntry) - if err != nil { - return nil, err - } - err = req.Storage.Delete(ctx, "policy/"+roleName) - if err != nil { - return nil, err - } - } + roleEntry = &awsRoleEntry{} + } else if roleEntry.InvalidData != "" { + resp.AddWarning(fmt.Sprintf("Invalid data of %q cleared out of role", roleEntry.InvalidData)) + roleEntry.InvalidData = "" } legacyRole, err := legacyRoleData(d) @@ -252,47 +235,44 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f return &resp, nil } -func nonLockedRoleRead(ctx context.Context, s logical.Storage, roleName string) (*awsRoleEntry, error) { +func (b *backend) roleRead(ctx context.Context, s logical.Storage, roleName string, shouldLock bool) (*awsRoleEntry, error) { if roleName == "" { return nil, fmt.Errorf("missing role name") } + if shouldLock { + b.roleMutex.RLock() + } entry, err := s.Get(ctx, "role/"+roleName) + if shouldLock { + b.roleMutex.RUnlock() + } if err != nil { return nil, err } - if entry == nil { - return nil, nil + var roleEntry awsRoleEntry + if entry != nil { + if err := entry.DecodeJSON(&roleEntry); err != nil { + return nil, err + } + return &roleEntry, nil } - var result awsRoleEntry - if err := entry.DecodeJSON(&result); err != nil { - return nil, err + if shouldLock { + b.roleMutex.Lock() + defer b.roleMutex.Unlock() } - return &result, nil -} - -func (b *backend) lockedRoleRead(ctx context.Context, s logical.Storage, roleName string) (*awsRoleEntry, error) { - b.roleMutex.RLock() - roleEntry, err := nonLockedRoleRead(ctx, s, roleName) - b.roleMutex.RUnlock() - + entry, err = s.Get(ctx, "role/"+roleName) if err != nil { return nil, err } - if roleEntry != nil { - return roleEntry, nil - } - b.roleMutex.Lock() - defer b.roleMutex.Unlock() - roleEntry, err = nonLockedRoleRead(ctx, s, roleName) - - if err != nil { - return nil, err - } - if roleEntry != nil { - return roleEntry, nil + if entry != nil { + if err := entry.DecodeJSON(&roleEntry); err != nil { + return nil, err + } + return &roleEntry, nil } + legacyEntry, err := s.Get(ctx, "policy/"+roleName) if err != nil { return nil, err diff --git a/builtin/logical/aws/path_user.go b/builtin/logical/aws/path_user.go index 6fc4760333c6..0d541546f70f 100644 --- a/builtin/logical/aws/path_user.go +++ b/builtin/logical/aws/path_user.go @@ -47,7 +47,7 @@ func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *fr roleName := d.Get("name").(string) // Read the policy - role, err := b.lockedRoleRead(ctx, req.Storage, roleName) + role, err := b.roleRead(ctx, req.Storage, roleName, true) if err != nil { return nil, errwrap.Wrapf("error retrieving role: {{err}}", err) } From 6bb274ff034f22f6f49b59ae422331c69e12cbb7 Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Sun, 5 Aug 2018 18:05:44 -0400 Subject: [PATCH 6/7] Eliminate duplicated AWS secret test code The testAccStepReadUser and testAccStepReadSTS were virtually identical, so they are consolidated into a single method with the path passed in. --- builtin/logical/aws/backend_test.go | 45 +++++++---------------------- 1 file changed, 10 insertions(+), 35 deletions(-) diff --git a/builtin/logical/aws/backend_test.go b/builtin/logical/aws/backend_test.go index 5ba395d092b9..85bc70af35f4 100644 --- a/builtin/logical/aws/backend_test.go +++ b/builtin/logical/aws/backend_test.go @@ -36,7 +36,7 @@ func TestBackend_basic(t *testing.T) { Steps: []logicaltest.TestStep{ testAccStepConfig(t), testAccStepWritePolicy(t, "test", testDynamoPolicy), - testAccStepReadUser(t, "test", []credentialTestFunc{listDynamoTablesTest}), + testAccStepRead(t, "creds", "test", []credentialTestFunc{listDynamoTablesTest}), }, }) } @@ -58,11 +58,11 @@ func TestBackend_basicSTS(t *testing.T) { Steps: []logicaltest.TestStep{ testAccStepConfigWithCreds(t, accessKey), testAccStepWritePolicy(t, "test", testDynamoPolicy), - testAccStepReadSTS(t, "test", []credentialTestFunc{listDynamoTablesTest}), + testAccStepRead(t, "sts", "test", []credentialTestFunc{listDynamoTablesTest}), testAccStepWriteArnPolicyRef(t, "test", ec2PolicyArn), testAccStepReadSTSWithArnPolicy(t, "test"), testAccStepWriteArnRoleRef(t, testRoleName), - testAccStepReadSTS(t, testRoleName, []credentialTestFunc{describeInstancesTest}), + testAccStepRead(t, "sts", testRoleName, []credentialTestFunc{describeInstancesTest}), }, Teardown: func() error { return teardown(accessKey) @@ -368,10 +368,10 @@ func testAccStepConfigWithCreds(t *testing.T, accessKey *awsAccessKey) logicalte } } -func testAccStepReadUser(t *testing.T, name string, credentialTests []credentialTestFunc) logicaltest.TestStep { +func testAccStepRead(t *testing.T, path, name string, credentialTests []credentialTestFunc) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.ReadOperation, - Path: "creds/" + name, + Path: path + "/" + name, Check: func(resp *logical.Response) error { var d struct { AccessKey string `mapstructure:"access_key"` @@ -477,31 +477,6 @@ func retryUntilSuccess(op func() error) error { return err } -func testAccStepReadSTS(t *testing.T, name string, credentialTests []credentialTestFunc) logicaltest.TestStep { - return logicaltest.TestStep{ - Operation: logical.ReadOperation, - Path: "sts/" + name, - Check: func(resp *logical.Response) error { - var d struct { - AccessKey string `mapstructure:"access_key"` - SecretKey string `mapstructure:"secret_key"` - STSToken string `mapstructure:"security_token"` - } - if err := mapstructure.Decode(resp.Data, &d); err != nil { - return err - } - log.Printf("[WARN] Generated credentials: %v", d) - for _, test := range credentialTests { - err := test(d.AccessKey, d.SecretKey, d.STSToken) - if err != nil { - return err - } - } - return nil - }, - } -} - func testAccStepReadSTSWithArnPolicy(t *testing.T, name string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.ReadOperation, @@ -626,7 +601,7 @@ func TestBackend_basicPolicyArnRef(t *testing.T) { Steps: []logicaltest.TestStep{ testAccStepConfig(t), testAccStepWriteArnPolicyRef(t, "test", ec2PolicyArn), - testAccStepReadUser(t, "test", []credentialTestFunc{describeInstancesTest}), + testAccStepRead(t, "creds", "test", []credentialTestFunc{describeInstancesTest}), }, }) } @@ -655,8 +630,8 @@ func TestBackend_iamUserManagedInlinePolicies(t *testing.T) { testAccStepConfig(t), testAccStepWriteRole(t, "test", roleData), testAccStepReadRole(t, "test", expectedRoleData), - testAccStepReadUser(t, "test", []credentialTestFunc{describeInstancesTest, listIamUsersTest, listDynamoTablesTest}), - testAccStepReadSTS(t, "test", []credentialTestFunc{describeInstancesTest, listIamUsersTest, listDynamoTablesTest}), + testAccStepRead(t, "creds", "test", []credentialTestFunc{describeInstancesTest, listIamUsersTest, listDynamoTablesTest}), + testAccStepRead(t, "sts", "test", []credentialTestFunc{describeInstancesTest, listIamUsersTest, listDynamoTablesTest}), }, }) } @@ -696,8 +671,8 @@ func TestBackend_AssumedRoleWithPolicyDoc(t *testing.T) { Steps: []logicaltest.TestStep{ testAccStepConfig(t), testAccStepWriteRole(t, "test", roleData), - testAccStepReadSTS(t, "test", []credentialTestFunc{describeInstancesTest, describeAzsTestUnauthorized}), - testAccStepReadUser(t, "test", []credentialTestFunc{describeInstancesTest, describeAzsTestUnauthorized}), + testAccStepRead(t, "sts", "test", []credentialTestFunc{describeInstancesTest, describeAzsTestUnauthorized}), + testAccStepRead(t, "creds", "test", []credentialTestFunc{describeInstancesTest, describeAzsTestUnauthorized}), }, Teardown: deleteTestRole, }) From 523d206d5d9c2d127f39e6a449c58a1dc05f487a Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Mon, 6 Aug 2018 21:06:01 -0400 Subject: [PATCH 7/7] Switch to use AWS ARN parser --- builtin/logical/aws/path_roles.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/logical/aws/path_roles.go b/builtin/logical/aws/path_roles.go index 528395a40e21..d8d128e1f188 100644 --- a/builtin/logical/aws/path_roles.go +++ b/builtin/logical/aws/path_roles.go @@ -6,9 +6,9 @@ import ( "encoding/json" "errors" "fmt" - "regexp" "strings" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/hashicorp/vault/helper/consts" "github.com/hashicorp/vault/helper/strutil" "github.com/hashicorp/vault/logical" @@ -301,16 +301,17 @@ func (b *backend) roleRead(ctx context.Context, s logical.Storage, roleName stri func upgradeLegacyPolicyEntry(entry string) *awsRoleEntry { var newRoleEntry *awsRoleEntry if strings.HasPrefix(entry, "arn:") { - arnRegex := regexp.MustCompile(`^arn:aws[a-z-]*:iam::(?P\d{12}|aws):(?P\w+)/(?P.+)$`) - parsedArn := arnRegex.FindStringSubmatch(entry) - if parsedArn == nil { + parsedArn, err := arn.Parse(entry) + if err != nil { newRoleEntry = &awsRoleEntry{ InvalidData: entry, Version: 1, } return newRoleEntry } - switch parsedArn[2] { + resourceParts := strings.Split(parsedArn.Resource, "/") + resourceType := resourceParts[0] + switch resourceType { case "role": newRoleEntry = &awsRoleEntry{ CredentialTypes: []string{assumedRoleCred},