Skip to content

Keystone V3: Support S3 tokens signature validation#1906

Merged
jtopjian merged 2 commits intogophercloud:masterfrom
kayrus:s3token
Mar 28, 2020
Merged

Keystone V3: Support S3 tokens signature validation#1906
jtopjian merged 2 commits intogophercloud:masterfrom
kayrus:s3token

Conversation

@kayrus
Copy link
Copy Markdown
Contributor

@kayrus kayrus commented Mar 26, 2020

Resolves #1905
@jtopjian this PR is a bit hacky, because if I put s3tokens into a separate package, I need to duplicate a lot of logic.

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 26, 2020

Coverage Status

Coverage decreased (-0.05%) to 78.697% when pulling 301b812 on kayrus:s3token into ab68fb5 on gophercloud:master.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Mar 26, 2020

Build failed.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Mar 26, 2020

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Mar 27, 2020

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Mar 27, 2020

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Mar 27, 2020

Build succeeded.

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

@kayrus Thanks for working on this. I see what you mean about it being hacky. It's been a long week for me and I can't think of a better way to do this, either. I'm okay with the implementation - it can be revisited later if someone is inclined.

I have a few comments - let me know if you have any questions.

Timestamp *time.Time `json:"-"`
// Token is a []byte string (encoded to base64 automatically) which was signed
// by an EC2 secret key. Used by S3 tokens for validation only.
Token []byte `json:"token,omitempty"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just so I understand: this field is for someone to supply a pre-generated signed string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Exactly.

c["signature"])
h["Authorization"] = EC2CredentialsBuildAuthorizationHeaderV4(*opts, signedHeaders, c["signature"].(string), date)

c["token"] = stringToSign
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If my previous comment is true, should there be some kind of check to make sure this isn't already set so it's not clobbered?

Additionally, let's add a comment above this line to say that while token is being set, it is only used for S3 and will be removed when using EC2 validation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Token must be set with a signature. If signature was not provided, token will be generated as well.


// ValidateS3Token authenticates an S3 request using EC2 credentials. Doesn't
// generate a new token ID, but returns a tokens.CreateResult.
func ValidateS3Token(c *gophercloud.ServiceClient, opts tokens.AuthOptionsBuilder) (r tokens.CreateResult) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about doing something similar to tokens.Validate where (bool, error) is returned: https://github.com/gophercloud/gophercloud/blob/master/openstack/identity/v3/tokens/requests.go#L145

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What if we need to fetch user/project/domain info from the token response?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure that's included? The debug output doesn't show it.

Copy link
Copy Markdown
Contributor Author

@kayrus kayrus Mar 28, 2020

Choose a reason for hiding this comment

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

It returns the same token body as usual, when keystone auths a user. The only difference is it doesn't return a token ID.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, right - the output is being redacted/removed in the acceptance tests. OK, I don't want to bike shed this one. Perhaps the Validate function I linked to should be renamed to IsValid at some point in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed debug doesn't show the json response, but content length is big and it should include the token data:

2020/03/27 13:58:13 [DEBUG] OpenStack Response Code: 200
2020/03/27 13:58:13 [DEBUG] OpenStack Response Headers: 
Content-Length: 6751
Content-Type: application/json
Date: Fri, 27 Mar 2020 13:58:13 GMT
Server: Apache/2.4.29 (Ubuntu)
Vary: X-Auth-Token
X-Openstack-Request-Id: req-07cf74fe-a252-4cbb-8107-0d263dc54b3e

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Mar 28, 2020

Build succeeded.

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

LGTM - thank you!

@jtopjian jtopjian merged commit b127e89 into gophercloud:master Mar 28, 2020
@kayrus kayrus deleted the s3token branch March 29, 2020 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keystone V3: Add support for S3 tokens signature verification

3 participants