From 025825dfe066608f94683070870b20371342c22b Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Tue, 13 Mar 2018 10:35:10 -0400 Subject: [PATCH] Accept temp creds in AWS secret backend acceptance tests (#4076) * Accept temp creds in AWS secret backend acceptance tests The AWS secret backend acceptance tests implicitly accepted long-lived AWS credentials (i.e., AWS IAM user and/or root credentials) in two ways: 1. It expected credentials to be passed in via the AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY environment variables. By not accepting AWS_SESSION_TOKEN or AWS_SECURITY_TOKEN, temporary credentials could not be passed in. (This also forced all credentials to be passed in via environment variables, which is a bit ugly). 2. The AWS sts:GetFederationToken call is only allowed from long-term credentials. This is called by the Vault code which the acceptance tests exercise. 1 is solved by deleting explicit references to credentials, which allows the SDK to do one of the things it does best -- find credentials via the default chain. 2 is a little more complicated. Rather than pass in whatever creds the acceptance test was run under to the backend, the acceptance test now creates a new IAM user and gets an access key from it, then passes the IAM user's creds back to the backend so that it can call sts:GetFederationToken (and then tries to clean up afterwards). * Fix Travis build failure The Travis build was failing because the user creation was happening regardless of whether it was running in acceptance test mode or not. This moves the user creation into the acceptance test precheck, which requires lazily evaluating the credentials when configuring the backend in the STS accetpance test, and so moving that to a PreFlight closure. * Reduce blind sleeps in AWS secret backend acceptance tests This removes a blind "sleep 10 seconds and then attempt to reuse the credential" codepath and instead just keeps attemtping to reuse the credential for 10 seconds and fails if there aren't any successful uses after 10 seconds. This adds a few seconds speedup of acceptance test runs from my experiments. --- builtin/logical/aws/backend_test.go | 232 ++++++++++++++++++++++------ 1 file changed, 182 insertions(+), 50 deletions(-) diff --git a/builtin/logical/aws/backend_test.go b/builtin/logical/aws/backend_test.go index ad919b1bee23..1973a2ef8197 100644 --- a/builtin/logical/aws/backend_test.go +++ b/builtin/logical/aws/backend_test.go @@ -7,7 +7,6 @@ import ( "fmt" "log" "os" - "strings" "testing" "time" @@ -16,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/iam" + "github.com/aws/aws-sdk-go/service/sts" "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/vault/logical" logicaltest "github.com/hashicorp/vault/logical/testing" @@ -41,15 +41,21 @@ func TestBackend_basic(t *testing.T) { } func TestBackend_basicSTS(t *testing.T) { + accessKey := &awsAccessKey{} logicaltest.Test(t, logicaltest.TestCase{ AcceptanceTest: true, PreCheck: func() { testAccPreCheck(t) + createUser(t, accessKey) createRole(t) + // Sleep sometime because AWS is eventually consistent + // Both the createUser and createRole depend on this + log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...") + time.Sleep(10 * time.Second) }, Backend: getBackend(t), Steps: []logicaltest.TestStep{ - testAccStepConfig(t), + testAccStepConfigWithCreds(t, accessKey), testAccStepWritePolicy(t, "test", testPolicy), testAccStepReadSTS(t, "test"), testAccStepWriteArnPolicyRef(t, "test", testPolicyArn), @@ -57,7 +63,9 @@ func TestBackend_basicSTS(t *testing.T) { testAccStepWriteArnRoleRef(t, testRoleName), testAccStepReadSTS(t, testRoleName), }, - Teardown: teardown, + Teardown: func() error { + return teardown(accessKey) + }, }) } @@ -81,14 +89,6 @@ func TestBackend_policyCrud(t *testing.T) { } func testAccPreCheck(t *testing.T) { - if v := os.Getenv("AWS_ACCESS_KEY_ID"); v == "" { - t.Fatal("AWS_ACCESS_KEY_ID must be set for acceptance tests") - } - - if v := os.Getenv("AWS_SECRET_ACCESS_KEY"); v == "" { - t.Fatal("AWS_SECRET_ACCESS_KEY must be set for acceptance tests") - } - if v := os.Getenv("AWS_DEFAULT_REGION"); v == "" { log.Println("[INFO] Test: Using us-west-2 as test region") os.Setenv("AWS_DEFAULT_REGION", "us-west-2") @@ -97,7 +97,7 @@ func testAccPreCheck(t *testing.T) { if v := os.Getenv("AWS_ACCOUNT_ID"); v == "" { accountId, err := getAccountId() if err != nil { - t.Fatal("AWS_ACCOUNT_ID could not be read from iam:GetUser for acceptance tests") + t.Fatalf("AWS_ACCOUNT_ID could not be read from iam:GetUser for acceptance tests: %#v", err) } log.Printf("[INFO] Test: Used %s as AWS_ACCOUNT_ID", accountId) os.Setenv("AWS_ACCOUNT_ID", accountId) @@ -105,27 +105,23 @@ func testAccPreCheck(t *testing.T) { } func getAccountId() (string, error) { - creds := credentials.NewStaticCredentials(os.Getenv("AWS_ACCESS_KEY_ID"), - os.Getenv("AWS_SECRET_ACCESS_KEY"), - "") - awsConfig := &aws.Config{ - Credentials: creds, - Region: aws.String("us-east-1"), - HTTPClient: cleanhttp.DefaultClient(), + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), } - svc := iam.New(session.New(awsConfig)) + svc := sts.New(session.New(awsConfig)) - params := &iam.GetUserInput{} - res, err := svc.GetUser(params) + params := &sts.GetCallerIdentityInput{} + res, err := svc.GetCallerIdentity(params) if err != nil { return "", err } + if res == nil { + return "", fmt.Errorf("got nil response from GetCallerIdentity") + } - // split "arn:aws:iam::012345678912:user/username" - accountId := strings.Split(*res.User.Arn, ":")[4] - return accountId, nil + return *res.Account, nil } const testRoleName = "Vault-Acceptance-Test-AWS-Assume-Role" @@ -144,12 +140,9 @@ func createRole(t *testing.T) { ] } ` - creds := credentials.NewStaticCredentials(os.Getenv("AWS_ACCESS_KEY_ID"), os.Getenv("AWS_SECRET_ACCESS_KEY"), "") - awsConfig := &aws.Config{ - Credentials: creds, - Region: aws.String("us-east-1"), - HTTPClient: cleanhttp.DefaultClient(), + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), } svc := iam.New(session.New(awsConfig)) trustPolicy := fmt.Sprintf(testRoleAssumePolicy, os.Getenv("AWS_ACCOUNT_ID")) @@ -176,19 +169,94 @@ func createRole(t *testing.T) { if err != nil { t.Fatalf("AWS CreateRole failed: %v", err) } - - // Sleep sometime because AWS is eventually consistent - log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...") - time.Sleep(10 * time.Second) } -func teardown() error { - creds := credentials.NewStaticCredentials(os.Getenv("AWS_ACCESS_KEY_ID"), os.Getenv("AWS_SECRET_ACCESS_KEY"), "") +const testUserName = "Vault-Acceptance-Test-AWS-FederationToken" + +func createUser(t *testing.T, accessKey *awsAccessKey) { + // The sequence of user creation actions is carefully chosen to minimize + // impact of stolen IAM user credentials + // 1. Create user, without any permissions or credentials. At this point, + // nobody cares if creds compromised because this user can do nothing. + // 2. Attach the timebomb policy. This grants no access but puts a time limit + // on validitity of compromised credentials. If this fails, nobody cares + // because the user has no permissions to do anything anyway + // 3. Attach the AdminAccess policy. The IAM user still has no credentials to + // do anything + // 4. Generate API creds to get an actual access key and secret key + timebombPolicyTemplate := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Deny", + "Action": "*", + "Resource": "*", + "Condition": { + "DateGreaterThan": { + "aws:CurrentTime": "%s" + } + } + } + ] + } + ` + validity := time.Duration(2 * time.Hour) + expiry := time.Now().Add(validity) + timebombPolicy := fmt.Sprintf(timebombPolicyTemplate, expiry.Format(time.RFC3339)) + awsConfig := &aws.Config{ + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), + } + svc := iam.New(session.New(awsConfig)) + + createUserInput := &iam.CreateUserInput{ + UserName: aws.String(testUserName), + } + log.Printf("[INFO] AWS CreateUser: %s", testUserName) + _, err := svc.CreateUser(createUserInput) + if err != nil { + t.Fatalf("AWS CreateUser failed: %v", err) + } + + putPolicyInput := &iam.PutUserPolicyInput{ + PolicyDocument: aws.String(timebombPolicy), + PolicyName: aws.String("SelfDestructionTimebomb"), + UserName: aws.String(testUserName), + } + _, err = svc.PutUserPolicy(putPolicyInput) + if err != nil { + t.Fatalf("AWS PutUserPolicy failed: %v", err) + } + + attachUserPolicyInput := &iam.AttachUserPolicyInput{ + PolicyArn: aws.String("arn:aws:iam::aws:policy/AdministratorAccess"), + UserName: aws.String(testUserName), + } + _, err = svc.AttachUserPolicy(attachUserPolicyInput) + if err != nil { + t.Fatalf("AWS AttachUserPolicy failed, %v", err) + } + + createAccessKeyInput := &iam.CreateAccessKeyInput{ + UserName: aws.String(testUserName), + } + createAccessKeyOutput, err := svc.CreateAccessKey(createAccessKeyInput) + if err != nil { + t.Fatalf("AWS CreateAccessKey failed: %v", err) + } + if createAccessKeyOutput == nil { + t.Fatalf("AWS CreateAccessKey returned nil") + } + genAccessKey := createAccessKeyOutput.AccessKey + + accessKey.AccessKeyId = *genAccessKey.AccessKeyId + accessKey.SecretAccessKey = *genAccessKey.SecretAccessKey +} +func teardown(accessKey *awsAccessKey) error { awsConfig := &aws.Config{ - Credentials: creds, - Region: aws.String("us-east-1"), - HTTPClient: cleanhttp.DefaultClient(), + Region: aws.String("us-east-1"), + HTTPClient: cleanhttp.DefaultClient(), } svc := iam.New(session.New(awsConfig)) @@ -214,6 +282,45 @@ func teardown() error { return err } + userDetachment := &iam.DetachUserPolicyInput{ + PolicyArn: aws.String("arn:aws:iam::aws:policy/AdministratorAccess"), + UserName: aws.String(testUserName), + } + _, err = svc.DetachUserPolicy(userDetachment) + if err != nil { + log.Printf("[WARN] AWS DetachUserPolicy failed: %v", err) + return err + } + + deleteAccessKeyInput := &iam.DeleteAccessKeyInput{ + AccessKeyId: aws.String(accessKey.AccessKeyId), + UserName: aws.String(testUserName), + } + _, err = svc.DeleteAccessKey(deleteAccessKeyInput) + if err != nil { + log.Printf("[WARN] AWS DeleteAccessKey failed: %v", err) + return err + } + + deleteUserPolicyInput := &iam.DeleteUserPolicyInput{ + PolicyName: aws.String("SelfDestructionTimebomb"), + UserName: aws.String(testUserName), + } + _, err = svc.DeleteUserPolicy(deleteUserPolicyInput) + if err != nil { + log.Printf("[WARN] AWS DeleteUserPolicy failed: %v", err) + return err + } + deleteUserInput := &iam.DeleteUserInput{ + UserName: aws.String(testUserName), + } + log.Printf("[INFO] AWS DeleteUser: %s", testUserName) + _, err = svc.DeleteUser(deleteUserInput) + if err != nil { + log.Printf("[WARN] AWS DeleteUser failed: %v", err) + return err + } + return nil } @@ -222,9 +329,26 @@ func testAccStepConfig(t *testing.T) logicaltest.TestStep { Operation: logical.UpdateOperation, Path: "config/root", Data: map[string]interface{}{ - "access_key": os.Getenv("AWS_ACCESS_KEY_ID"), - "secret_key": os.Getenv("AWS_SECRET_ACCESS_KEY"), - "region": os.Getenv("AWS_DEFAULT_REGION"), + "region": os.Getenv("AWS_DEFAULT_REGION"), + }, + } +} + +func testAccStepConfigWithCreds(t *testing.T, accessKey *awsAccessKey) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.UpdateOperation, + Path: "config/root", + Data: map[string]interface{}{ + "region": os.Getenv("AWS_DEFAULT_REGION"), + }, + PreFlight: func(req *logical.Request) error { + // Values in Data above get eagerly evaluated due to the testing framework. + // In particular, they get evaluated before accessKey gets set by CreateUser + // and thus would fail. By moving to a closure in a PreFlight, we ensure that + // the creds get evaluated lazily after they've been properly set + req.Data["access_key"] = accessKey.AccessKeyId + req.Data["secret_key"] = accessKey.SecretAccessKey + return nil }, } } @@ -243,10 +367,6 @@ func testAccStepReadUser(t *testing.T, name string) logicaltest.TestStep { } log.Printf("[WARN] Generated credentials: %v", d) - // Sleep sometime because AWS is eventually consistent - log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...") - time.Sleep(10 * time.Second) - // Build a client and verify that the credentials work creds := credentials.NewStaticCredentials(d.AccessKey, d.SecretKey, "") awsConfig := &aws.Config{ @@ -257,12 +377,19 @@ func testAccStepReadUser(t *testing.T, name string) logicaltest.TestStep { 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 + retryCount := 0 + success := false + var err error + for !success && retryCount < 10 { + _, err = client.DescribeInstances(&ec2.DescribeInstancesInput{}) + if err == nil { + return nil + } + time.Sleep(time.Second) + retryCount++ } - return nil + return err }, } } @@ -458,3 +585,8 @@ func testAccStepWriteArnRoleRef(t *testing.T, roleName string) logicaltest.TestS }, } } + +type awsAccessKey struct { + AccessKeyId string + SecretAccessKey string +}