From bf0b7f218e1f779abcd3def9771d997abd4ce312 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Fri, 23 Sep 2016 12:47:35 -0400 Subject: [PATCH] Implemented bound_iam_role_arn constraint --- builtin/credential/aws-ec2/backend.go | 8 ++ builtin/credential/aws-ec2/client.go | 62 ++++++++++++--- .../credential/aws-ec2/path_config_client.go | 4 + builtin/credential/aws-ec2/path_login.go | 76 ++++++++++++++++++- builtin/credential/aws-ec2/path_role.go | 44 ++++++----- 5 files changed, 159 insertions(+), 35 deletions(-) diff --git a/builtin/credential/aws-ec2/backend.go b/builtin/credential/aws-ec2/backend.go index 5e63eac32..ff8b71e2f 100644 --- a/builtin/credential/aws-ec2/backend.go +++ b/builtin/credential/aws-ec2/backend.go @@ -5,6 +5,7 @@ import ( "time" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/iam" "github.com/hashicorp/vault/helper/salt" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" @@ -49,6 +50,12 @@ type backend struct { // the credentials are modified or deleted, all the cached client objects // will be flushed. EC2ClientsMap map[string]*ec2.EC2 + + // Map to hold the IAM client objects indexed by region. This avoids + // the overhead of creating a client object for every login request. + // When the credentials are modified or deleted, all the cached client + // objects will be flushed. + IAMClientsMap map[string]*iam.IAM } func Backend(conf *logical.BackendConfig) (*backend, error) { @@ -65,6 +72,7 @@ func Backend(conf *logical.BackendConfig) (*backend, error) { tidyCooldownPeriod: time.Hour, Salt: salt, EC2ClientsMap: make(map[string]*ec2.EC2), + IAMClientsMap: make(map[string]*iam.IAM), } b.Backend = &framework.Backend{ diff --git a/builtin/credential/aws-ec2/client.go b/builtin/credential/aws-ec2/client.go index 59120eda1..1434c9ceb 100644 --- a/builtin/credential/aws-ec2/client.go +++ b/builtin/credential/aws-ec2/client.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "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/hashicorp/go-cleanhttp" "github.com/hashicorp/vault/helper/awsutil" "github.com/hashicorp/vault/logical" @@ -61,43 +62,82 @@ func (b *backend) getClientConfig(s logical.Storage, region string) (*aws.Config // flushCachedEC2Clients deletes all the cached ec2 client objects from the backend. // If the client credentials configuration is deleted or updated in the backend, all -// the cached EC2 client objects will be flushed. -// -// Write lock should be acquired using b.configMutex.Lock() before calling this method -// and lock should be released using b.configMutex.Unlock() after the method returns. +// the cached EC2 client objects will be flushed. Config mutex lock should be +// acquired for write operation before calling this method. func (b *backend) flushCachedEC2Clients() { - // deleting items in map during iteration is safe. + // deleting items in map during iteration is safe for region, _ := range b.EC2ClientsMap { delete(b.EC2ClientsMap, region) } } -// clientEC2 creates a client to interact with AWS EC2 API. +// flushCachedIAMClients deletes all the cached iam client objects from the +// backend. If the client credentials configuration is deleted or updated in +// the backend, all the cached IAM client objects will be flushed. Config mutex +// lock should be acquired for write operation before calling this method. +func (b *backend) flushCachedIAMClients() { + // deleting items in map during iteration is safe + for region, _ := range b.IAMClientsMap { + delete(b.IAMClientsMap, region) + } +} + +// clientEC2 creates a client to interact with AWS EC2 API func (b *backend) clientEC2(s logical.Storage, region string) (*ec2.EC2, error) { b.configMutex.RLock() if b.EC2ClientsMap[region] != nil { defer b.configMutex.RUnlock() - // If the client object was already created, return it. + // If the client object was already created, return it return b.EC2ClientsMap[region], nil } - // Release the read lock and acquire the write lock. + // Release the read lock and acquire the write lock b.configMutex.RUnlock() b.configMutex.Lock() defer b.configMutex.Unlock() - // If the client gets created while switching the locks, return it. + // If the client gets created while switching the locks, return it if b.EC2ClientsMap[region] != nil { return b.EC2ClientsMap[region], nil } - // Create a AWS config object using a chain of providers. + // Create an AWS config object using a chain of providers awsConfig, err := b.getClientConfig(s, region) if err != nil { return nil, err } - // Create a new EC2 client object, cache it and return the same. + // Create a new EC2 client object, cache it and return the same b.EC2ClientsMap[region] = ec2.New(session.New(awsConfig)) return b.EC2ClientsMap[region], nil } + +// clientIAM creates a client to interact with AWS IAM API +func (b *backend) clientIAM(s logical.Storage, region string) (*iam.IAM, error) { + b.configMutex.RLock() + if b.IAMClientsMap[region] != nil { + defer b.configMutex.RUnlock() + // If the client object was already created, return it + return b.IAMClientsMap[region], nil + } + + // Release the read lock and acquire the write lock + b.configMutex.RUnlock() + b.configMutex.Lock() + defer b.configMutex.Unlock() + + // If the client gets created while switching the locks, return it + if b.IAMClientsMap[region] != nil { + return b.IAMClientsMap[region], nil + } + + // Create an AWS config object using a chain of providers + awsConfig, err := b.getClientConfig(s, region) + if err != nil { + return nil, err + } + + // Create a new IAM client object, cache it and return the same + b.IAMClientsMap[region] = iam.New(session.New(awsConfig)) + return b.IAMClientsMap[region], nil +} diff --git a/builtin/credential/aws-ec2/path_config_client.go b/builtin/credential/aws-ec2/path_config_client.go index 008e3e69a..8b973a6d7 100644 --- a/builtin/credential/aws-ec2/path_config_client.go +++ b/builtin/credential/aws-ec2/path_config_client.go @@ -108,6 +108,9 @@ func (b *backend) pathConfigClientDelete( // Remove all the cached EC2 client objects in the backend. b.flushCachedEC2Clients() + // Remove all the cached EC2 client objects in the backend. + b.flushCachedIAMClients() + return nil, nil } @@ -175,6 +178,7 @@ func (b *backend) pathConfigClientCreateUpdate( if changedCreds { b.flushCachedEC2Clients() + b.flushCachedIAMClients() } return nil, nil diff --git a/builtin/credential/aws-ec2/path_login.go b/builtin/credential/aws-ec2/path_login.go index 1d90525cb..32320f7b8 100644 --- a/builtin/credential/aws-ec2/path_login.go +++ b/builtin/credential/aws-ec2/path_login.go @@ -4,10 +4,12 @@ import ( "crypto/subtle" "encoding/pem" "fmt" + "strings" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/iam" "github.com/fullsailor/pkcs7" "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/jsonutil" @@ -61,6 +63,39 @@ on either the role or the role tag, the 'nonce' holds no significance.`, } } +// instanceIamRoleARN fetches the IAM role ARN associated with the given +// instance profile name +func (b *backend) instanceIamRoleARN(s logical.Storage, instanceProfileName, region string) (string, error) { + iamClient, err := b.clientIAM(s, region) + if err != nil { + return "", err + } + + profile, err := iamClient.GetInstanceProfile(&iam.GetInstanceProfileInput{ + InstanceProfileName: aws.String(instanceProfileName), + }) + if err != nil { + return "", err + } + if profile == nil { + return "", fmt.Errorf("nil output while getting instance profile details") + } + + if profile.InstanceProfile == nil { + return "", fmt.Errorf("nil instance profile in the output of instance profile details") + } + + if profile.InstanceProfile.Roles == nil || len(profile.InstanceProfile.Roles) != 1 { + return "", fmt.Errorf("invalid roles in the output of instance profile details") + } + + if profile.InstanceProfile.Roles[0].Arn == nil { + return "", fmt.Errorf("nil role ARN in the output of instance profile details") + } + + return *profile.InstanceProfile.Roles[0].Arn, nil +} + // validateInstance queries the status of the EC2 instance using AWS EC2 API and // checks if the instance is running and is healthy. func (b *backend) validateInstance(s logical.Storage, instanceID, region string) (*ec2.DescribeInstancesOutput, error) { @@ -83,6 +118,9 @@ func (b *backend) validateInstance(s logical.Storage, instanceID, region string) if err != nil { return nil, fmt.Errorf("error fetching description for instance ID %s: %s\n", instanceID, err) } + if status == nil { + return nil, fmt.Errorf("nil output from describe instances") + } if len(status.Reservations) == 0 { return nil, fmt.Errorf("no reservations found in instance description") @@ -284,9 +322,41 @@ func (b *backend) pathLoginUpdate( if instanceDesc.Reservations[0].Instances[0].IamInstanceProfile.Arn == nil { return nil, fmt.Errorf("IAM instance profile ARN in the instance description is nil") } - iamInstanceProfileArn := *instanceDesc.Reservations[0].Instances[0].IamInstanceProfile.Arn - if iamInstanceProfileArn != roleEntry.BoundIamInstanceProfileARN { - return logical.ErrorResponse(fmt.Sprintf("IAM instance profile ARN %q does not satisfy the constraint role %q", iamInstanceProfileArn, roleName)), nil + iamInstanceProfileARN := *instanceDesc.Reservations[0].Instances[0].IamInstanceProfile.Arn + if iamInstanceProfileARN != roleEntry.BoundIamInstanceProfileARN { + return logical.ErrorResponse(fmt.Sprintf("IAM instance profile ARN %q does not satisfy the constraint role %q", iamInstanceProfileARN, roleName)), nil + } + } + + // Check if the IAM role ARN of the instance trying to login, matches + // the IAM role ARN specified as a constraint on the role. + if roleEntry.BoundIamRoleARN != "" { + if instanceDesc.Reservations[0].Instances[0].IamInstanceProfile == nil { + return nil, fmt.Errorf("IAM instance profile in the instance description is nil") + } + if instanceDesc.Reservations[0].Instances[0].IamInstanceProfile.Id == nil { + return nil, fmt.Errorf("IAM instance profile identifier in the instance description is nil") + } + + // Fetch the instance profile ARN from the instance description + iamInstanceProfileARN := *instanceDesc.Reservations[0].Instances[0].IamInstanceProfile.Arn + + // Extract out the instance profile name from the instance + // profile ARN + iamInstanceProfileARNSlice := strings.SplitAfter(iamInstanceProfileARN, ":instance-profile/") + iamInstanceProfileName := iamInstanceProfileARNSlice[len(iamInstanceProfileARNSlice)-1] + + // Use instance profile ARN to fetch the associated role ARN + iamRoleARN, err := b.instanceIamRoleARN(req.Storage, iamInstanceProfileName, identityDoc.Region) + if err != nil { + return nil, fmt.Errorf("IAM role ARN could not be fetched: %v", err) + } + if iamRoleARN == "" { + return nil, fmt.Errorf("IAM role ARN could not be fetched") + } + + if iamRoleARN != roleEntry.BoundIamRoleARN { + return logical.ErrorResponse(fmt.Sprintf("IAM role ARN %q does not satisfy the constraint role %q", iamRoleARN, roleName)), nil } } diff --git a/builtin/credential/aws-ec2/path_role.go b/builtin/credential/aws-ec2/path_role.go index be16a312f..add40aa19 100644 --- a/builtin/credential/aws-ec2/path_role.go +++ b/builtin/credential/aws-ec2/path_role.go @@ -29,6 +29,11 @@ using the AMI ID specified by this parameter.`, Type: framework.TypeString, Description: `If set, defines a constraint on the EC2 instances that the account ID in its identity document to match the one specified by this parameter.`, + }, + "bound_iam_role_arn": { + Type: framework.TypeString, + Description: `If set, defines a constraint on the EC2 instances that they should be using the +IAM role ARN specified by this parameter.`, }, "bound_iam_instance_profile_arn": { Type: framework.TypeString, @@ -194,22 +199,22 @@ func (b *backend) nonLockedAWSRole(s logical.Storage, roleName string) (*awsRole return nil, err } - // Upgrade code to use proper field for bound_iam_instance_profile_arn - if result.DeprecatedBoundIamARN != "" { + // Check if the value held by role ARN field is actually an instance profile ARN + if result.BoundIamRoleARN != "" && strings.Contains(result.BoundIamRoleARN, ":instance-profile/") { // For sanity if result.BoundIamInstanceProfileARN != "" { - return nil, fmt.Errorf("both bound_iam_role_arn and bound_iam_instance_profile_arn are set") + return nil, fmt.Errorf("bound_iam_role_arn contains instance profile ARN and bound_iam_instance_profile_arn is non empty") } - // Fill in the new field - result.BoundIamInstanceProfileARN = result.DeprecatedBoundIamARN + // If yes, move it to the correct field + result.BoundIamInstanceProfileARN = result.BoundIamRoleARN // Reset the old field - result.DeprecatedBoundIamARN = "" + result.BoundIamRoleARN = "" // Save the update if err = b.nonLockedSetAWSRole(s, roleName, &result); err != nil { - return nil, fmt.Errorf("failed to upgrade bound_iam_role_arn to bound_iam_instance_profile_arn") + return nil, fmt.Errorf("failed to move instance profile ARN to bound_iam_instance_profile_arn field") } } @@ -246,8 +251,6 @@ func (b *backend) pathRoleList( // pathRoleRead is used to view the information registered for a given AMI ID. func (b *backend) pathRoleRead( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - resp := &logical.Response{} - roleEntry, err := b.lockedAWSRole(req.Storage, strings.ToLower(data.Get("role").(string))) if err != nil { return nil, err @@ -267,15 +270,9 @@ func (b *backend) pathRoleRead( // Display the max_ttl in seconds. respData["max_ttl"] = roleEntry.MaxTTL / time.Second - // To be removed in the coming releases - if respData["bound_iam_instance_profile_arn"] != "" { - respData["bound_iam_role_arn"] = respData["bound_iam_instance_profile_arn"] - resp.AddWarning("The field bound_iam_role_arn is deprecated and will be removed in future releases; refer bound_iam_instance_profile_arn instead.") - } - - resp.Data = respData - - return resp, nil + return &logical.Response{ + Data: respData, + }, nil } // pathRoleCreateUpdate is used to associate Vault policies to a given AMI ID. @@ -298,16 +295,20 @@ func (b *backend) pathRoleCreateUpdate( roleEntry = &awsRoleEntry{} } - // Set BoundAmiID only if it is supplied. There can't be a default value. + // Fetch and set the bound parameters. There can't be default values + // for these. if boundAmiIDRaw, ok := data.GetOk("bound_ami_id"); ok { roleEntry.BoundAmiID = boundAmiIDRaw.(string) } - // Set BoundAccountID only if it is supplied. There can't be a default value. if boundAccountIDRaw, ok := data.GetOk("bound_account_id"); ok { roleEntry.BoundAccountID = boundAccountIDRaw.(string) } + if boundIamRoleARNRaw, ok := data.GetOk("bound_iam_role_arn"); ok { + roleEntry.BoundIamRoleARN = boundIamRoleARNRaw.(string) + } + if boundIamInstanceProfileARNRaw, ok := data.GetOk("bound_iam_instance_profile_arn"); ok { roleEntry.BoundIamInstanceProfileARN = boundIamInstanceProfileARNRaw.(string) } @@ -317,6 +318,7 @@ func (b *backend) pathRoleCreateUpdate( case roleEntry.BoundAccountID != "": case roleEntry.BoundAmiID != "": case roleEntry.BoundIamInstanceProfileARN != "": + case roleEntry.BoundIamRoleARN != "": default: return logical.ErrorResponse("at least be one bound parameter should be specified on the role"), nil @@ -412,7 +414,7 @@ func (b *backend) pathRoleCreateUpdate( type awsRoleEntry struct { BoundAmiID string `json:"bound_ami_id" structs:"bound_ami_id" mapstructure:"bound_ami_id"` BoundAccountID string `json:"bound_account_id" structs:"bound_account_id" mapstructure:"bound_account_id"` - DeprecatedBoundIamARN string `json:"bound_iam_role_arn" structs:"bound_iam_role_arn" mapstructure:"bound_iam_role_arn"` + BoundIamRoleARN string `json:"bound_iam_role_arn" structs:"bound_iam_role_arn" mapstructure:"bound_iam_role_arn"` BoundIamInstanceProfileARN string `json:"bound_iam_instance_profile_arn" structs:"bound_iam_instance_profile_arn" mapstructure:"bound_iam_instance_profile_arn"` RoleTag string `json:"role_tag" structs:"role_tag" mapstructure:"role_tag"` AllowInstanceMigration bool `json:"allow_instance_migration" structs:"allow_instance_migration" mapstructure:"allow_instance_migration"`