added AWS enpoint handling by ror6ax · Pull Request #3416 · hashicorp/vault

ror6ax · 2017-10-04T09:31:41Z

In AWS, IAM endpoint(API you actually talk to get users) is determined by the region.

There is a number of AWS clones out there, like Eucalyptus. IAM works in very similar way, but endpoints are not standardized. The only way to make clones work with Vault is to provide optional endpoint parameter.

joelthompson · 2017-10-04T15:33:10Z

builtin/logical/aws/path_config_root.go

+	endpoint := data.Get("endpoint").(string)
+
+	if endpoint == "" {
+		endpoint = "none"


You shouldn't use "none" as a placeholder value here. I'd just leave it as the empty string (which is the zero value for strings in golang) and compare against the empty string in client.go

joelthompson · 2017-10-04T15:33:34Z

builtin/logical/aws/client.go

+
 	client := iam.New(session.New(awsConfig))
+
+	if *awsConfig.Endpoint != "none" {


See note about not using "none" as a placeholder, use ""

joelthompson · 2017-10-04T15:33:49Z

builtin/logical/aws/client.go

 		return nil, err
 	}
 	client := sts.New(session.New(awsConfig))
+	if *awsConfig.Endpoint != "none" {


Same -- use ""

joelthompson · 2017-10-04T15:34:15Z

builtin/logical/aws/client.go

 	}
 	client := sts.New(session.New(awsConfig))
+	if *awsConfig.Endpoint != "none" {
+		client = sts.New(session.New(awsConfig.WithEndpoint(*awsConfig.Endpoint)))


You're configuring STS and IAM clients with the same endpoints, and I doubt that will work. They likely need different endpoints.

joelthompson · 2017-10-04T15:34:39Z

builtin/logical/aws/path_config_root.go

 				Type:        framework.TypeString,
 				Description: "Region for API calls.",
 			},
+			"endpoint": &framework.FieldSchema{


You probably want separate iam_endpoint and sts_endpoint here.

ror6ax · 2017-10-04T20:40:20Z

Not really sure why 2 endpoint fields don't get recognized, seems I've added both. @joelthompson - can you please have a look for what I missed? Thanks a ton.

joelthompson

Looking pretty good @ror6ax! Just the small bit about dealing with the annoying AWS SDK details....

joelthompson · 2017-10-05T18:56:09Z

builtin/logical/aws/path_config_root.go

 				Type:        framework.TypeString,
 				Description: "Region for API calls.",
 			},
+			"iamendpoint": &framework.FieldSchema{


The auth backend already has iam_endpoint and sts_endpoint; it'd be good to underscore-separate this here.

joelthompson · 2017-10-05T18:56:15Z

builtin/logical/aws/path_config_root.go

+				Type:        framework.TypeString,
+				Description: "Endpoint to custom IAM server URL",
+			},
+			"stsendpoint": &framework.FieldSchema{


joelthompson · 2017-10-05T18:56:55Z

builtin/logical/aws/client.go

 		Credentials: creds,
 		Region:      aws.String(credsConfig.Region),
+		IAMEndpoint: aws.String(credsConfig.IAMEndpoint),
+		STSEndpoint: aws.String(credsConfig.STSEndpoint),


Pretty sure the issue is because you're using the upstream aws.Config object and it only has an Endpoint member. Consider passing a clientType parameter to getRootConfig and if clientType is iam then you set Endpoint to credsConfig.IAMEndpoint and if it's sts then you set Endpoint to credsConfig.STSEndpoint. Largely the same code lives in the auth backend:

vault/builtin/credential/aws/client.go

Lines 24 to 68 in 1c4baa5

func (b *backend) getRawClientConfig(s logical.Storage, region, clientType string) (*aws.Config, error) {

credsConfig := &awsutil.CredentialsConfig{

Region: region,

}

// Read the configured secret key and access key

config, err := b.nonLockedClientConfigEntry(s)

if err != nil {

return nil, err

}

endpoint := aws.String("")

if config != nil {

// Override the default endpoint with the configured endpoint.

switch {

case clientType == "ec2" && config.Endpoint != "":

endpoint = aws.String(config.Endpoint)

case clientType == "iam" && config.IAMEndpoint != "":

endpoint = aws.String(config.IAMEndpoint)

case clientType == "sts" && config.STSEndpoint != "":

endpoint = aws.String(config.STSEndpoint)

}

credsConfig.AccessKey = config.AccessKey

credsConfig.SecretKey = config.SecretKey

}

credsConfig.HTTPClient = cleanhttp.DefaultClient()

creds, err := credsConfig.GenerateCredentialChain()

if err != nil {

return nil, err

}

if creds == nil {

return nil, fmt.Errorf("could not compile valid credential providers from static config, environment, shared, or instance metadata")

}

// Create a config that can be used to make the API calls.

return &aws.Config{

Credentials: creds,

Region: aws.String(region),

HTTPClient: cleanhttp.DefaultClient(),

Endpoint: endpoint,

}, nil

}

joelthompson · 2017-10-05T18:57:32Z

builtin/logical/aws/client.go

+
 	client := iam.New(session.New(awsConfig))
+
+	if *awsConfig.IAMEndpoint != "" {


Then this becomes *awsConfig.Endpoint

joelthompson · 2017-10-05T18:57:39Z

builtin/logical/aws/client.go

 		return nil, err
 	}
 	client := sts.New(session.New(awsConfig))
+	if *awsConfig.STSEndpoint != "" {


ror6ax · 2017-10-09T08:45:53Z

@joelthompson Is there anything else you want me to add here?

joelthompson

Sorry @ror6ax -- been a bit busy the past couple of days. Looking pretty good, just a couple of quick pieces of feedback.

joelthompson · 2017-10-10T03:50:52Z

helper/awsutil/generate_credentials.go

And then you can just remove these.

joelthompson · 2017-10-10T03:51:27Z

builtin/logical/aws/client.go

This is the only place credsConfig.IAMEndpoint and credsConfig.STSEndpoint are referenced. I don't think you need them in the CredentialsConfig struct at all because, below, you're putting the endpoint directly in the aws.Config object.

joelthompson · 2017-10-10T03:53:19Z

builtin/logical/aws/client.go

It would be more go-idiomatic to do something like:

switch { case clientType == "iam" && config.IAMEndpoint != "": endpoint = *aws.String(config.IAMEndpoint) case clientType == "sts" && config.STSEndpoint != "": endpoint = *aws.String(config.STSEndpoint) }

joelthompson · 2017-10-10T03:54:39Z

builtin/logical/aws/client.go

I don't think this block is necessary anymore, because the endpoint is already in the awsConfig object that gets passed in when creating the IAM client.

joelthompson · 2017-10-10T03:54:46Z

builtin/logical/aws/client.go

ror6ax · 2017-10-23T17:12:53Z

@joelthompson - thanks for those remarks, now fixed.

joelthompson

Code LGTM! Can you also these parameters to the API docs at https://github.com/hashicorp/vault/blob/master/website/source/api/secret/aws/index.html.md#parameters ?

ror6ax · 2017-10-24T08:01:31Z

@joelthompson Done. Did not add it to Sample Payload as it can be confusing in use with region.

ror6ax · 2017-11-02T09:36:37Z

Hi, @joelthompson sorry to bother, but will this make it into 0.8.4? Thanks!

ror6ax · 2017-11-06T18:22:05Z

Bumping this up.

jefferai · 2017-11-06T18:31:34Z

LGTM, thanks!

* oss/master: (30 commits) Handle 'not supplied' case for field type TypeNameString (#3546) Fix deprecated cassandra backend tests (#3543) changelog++ auth/aws: Make disallow_reauthentication and allow_instance_migration mutually exclusive (#3291) changelog++ More Mount Conflict Detection (#2919) Fix swallowed errors in TestRollbackManager_Join() (#3327) changelog++ added AWS enpoint handling (#3416) Seal wrap all root tokens and their leases (#3540) Return group memberships of entity during read (#3526) Add note on support for using rec keys on /sys/rekey (#3517) Add third party tools list to website (#3488) Minor client refactoring (#3539) changelog++ Add PKCS8 marshaling to PKI (#3518) Update SSH list roles docs (#3536) Update gocql dep changelog++ Return role info for each role on pathRoleList (#3532) ...

ror6ax force-pushed the endpoint_patch branch 2 times, most recently from fc7aa28 to 9064cfb Compare October 4, 2017 10:12

added AWS enpoint handling

57cf97d

ror6ax force-pushed the endpoint_patch branch from 9064cfb to 57cf97d Compare October 4, 2017 11:15

joelthompson reviewed Oct 4, 2017

View reviewed changes

joelthompson suggested changes Oct 4, 2017

View reviewed changes

ror6ax force-pushed the endpoint_patch branch 2 times, most recently from 53f1df5 to 96c6073 Compare October 4, 2017 19:50

added separate STS/IAM endpoints

c8c9de2

ror6ax force-pushed the endpoint_patch branch from 96c6073 to c8c9de2 Compare October 4, 2017 20:14

joelthompson suggested changes Oct 5, 2017

View reviewed changes

ror6ax force-pushed the endpoint_patch branch 3 times, most recently from 94b37c1 to 537e8b7 Compare October 6, 2017 10:40

joelthompson reviewed Oct 10, 2017

View reviewed changes

added clientType to getRootConfig

7762720

ror6ax force-pushed the endpoint_patch branch from 537e8b7 to 7762720 Compare October 23, 2017 15:14

joelthompson approved these changes Oct 24, 2017

View reviewed changes

added docs

db38462

jefferai merged commit 81e18ae into hashicorp:master Nov 6, 2017

jefferai added this to the 0.8.4 milestone Nov 6, 2017


		client := iam.New(session.New(awsConfig))

		if *awsConfig.Endpoint != "none" {

	func (b backend) getRawClientConfig(s logical.Storage, region, clientType string) (aws.Config, error) {
	credsConfig := &awsutil.CredentialsConfig{
	Region: region,
	}

	// Read the configured secret key and access key
	config, err := b.nonLockedClientConfigEntry(s)
	if err != nil {
	return nil, err
	}

	endpoint := aws.String("")
	if config != nil {
	// Override the default endpoint with the configured endpoint.
	switch {
	case clientType == "ec2" && config.Endpoint != "":
	endpoint = aws.String(config.Endpoint)
	case clientType == "iam" && config.IAMEndpoint != "":
	endpoint = aws.String(config.IAMEndpoint)
	case clientType == "sts" && config.STSEndpoint != "":
	endpoint = aws.String(config.STSEndpoint)
	}

	credsConfig.AccessKey = config.AccessKey
	credsConfig.SecretKey = config.SecretKey
	}

	credsConfig.HTTPClient = cleanhttp.DefaultClient()

	creds, err := credsConfig.GenerateCredentialChain()
	if err != nil {
	return nil, err
	}
	if creds == nil {
	return nil, fmt.Errorf("could not compile valid credential providers from static config, environment, shared, or instance metadata")
	}

	// Create a config that can be used to make the API calls.
	return &aws.Config{
	Credentials: creds,
	Region: aws.String(region),
	HTTPClient: cleanhttp.DefaultClient(),
	Endpoint: endpoint,
	}, nil
	}


		client := iam.New(session.New(awsConfig))

		if *awsConfig.IAMEndpoint != "" {

Conversation

ror6ax commented Oct 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ror6ax commented Oct 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joelthompson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ror6ax commented Oct 9, 2017

Uh oh!

joelthompson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ror6ax commented Oct 23, 2017

Uh oh!

joelthompson left a comment

Choose a reason for hiding this comment

Uh oh!

ror6ax commented Oct 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ror6ax commented Nov 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ror6ax commented Nov 6, 2017

Uh oh!

jefferai commented Nov 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ror6ax commented Oct 4, 2017 •

edited

Loading

ror6ax commented Oct 24, 2017 •

edited

Loading

ror6ax commented Nov 2, 2017 •

edited

Loading