-
Notifications
You must be signed in to change notification settings - Fork 546
fix: Missing line break in AWS credential canonical request #2264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| /* | ||
| Copyright 2022 Google LLC | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| https://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| using Google.Apis.Json; | ||
| using Google.Apis.Tests.Mocks; | ||
| using System; | ||
| using Xunit; | ||
| using static Google.Apis.Auth.OAuth2.AwsExternalAccountCredential; | ||
|
|
||
| namespace Google.Apis.Auth.Tests.OAuth2 | ||
| { | ||
| public class AwsSignedSubjectTokenTests | ||
| { | ||
| private const string AwsSecretAcessKeyId = "BSIARD4OQDT6ATWAVS5C"; | ||
| private const string AwsSecretAccessKey = "CDNMV7GOT4h+PHxB/03/Zh4X7XZFOZRydtoMwsRS"; | ||
| private const string AwsToken = "GwoGZXIvYXdzEDQaDAVwVIzhCkoSIt9J6yLeAeVnJt7alj0kfoGSAYzy2uYkeOmLHgz8BxpbiFR1NwKd6QXDo09ZRVSoP1eUI286GXEX5PuY/kX3lctI6L" + | ||
| "YWhAxLl7d3xMNqcT+4Pw+aRvLNX6rCs71/gwZzOs7dkktam8j7zy5TWrWDy8JVU61P+iuAg+0gCW6dHzis/rj0ProFQ9SSGD2CpO8CWoF144i+pAUMWLD+2s02YTRFASZX8wVokTv81cMiraH3" + | ||
| "FwPzdlXQ6+cYhz4/mvfVRp5RL2BHjMACOo860fmKRpyIi8maaDL8jcVJhWiPkZZkAXVMvSiipNibBjJYih+X9LcsNFAjkSjlhiSBYms4Z9Mrbf5PAaMIIokO7B5/1VsaQjNfKY41DfitS3+VUz" + | ||
| "u2iCayn6mysrSsAaWeXZarieae+cWG3ihKq71AhOsDQkWoh8a+zw=="; | ||
| private const string AwsRegion = "us-east-2"; | ||
| private const string AwsVerificationUrl = "https://sts.us-east-2.amazonaws.com/?Action=GetCallerIdentity&Version=2011-06-15"; | ||
| private const string Audience = "//iam.googleapis.com/projects/123456789012/locations/global/workloadIdentityPools/pool-abc123d4/providers/aws-abc123d4"; | ||
| private static readonly DateTime SigningTimestampUtc = new DateTime(1987, 5, 4, 6, 36, 45, DateTimeKind.Utc); | ||
|
|
||
| private const string SerializedSubjectToken = "%7B%22url%22%3A%22https%3A%2F%2Fsts.us-east-2.amazonaws.com%2F%3FAction%3DGetCallerIdentity%26Version%3D" + | ||
| "2011-06-15%22%2C%22method%22%3A%22POST%22%2C%22body%22%3A%22%22%2C%22headers%22%3A%5B%7B%22key%22%3A%22host%22%2C%22value%22%3A%22sts.us-east-2.am" + | ||
| "azonaws.com%22%7D%2C%7B%22key%22%3A%22x-amz-date%22%2C%22value%22%3A%2219870504T063645Z%22%7D%2C%7B%22key%22%3A%22x-goog-cloud-target-resource%22%" + | ||
| "2C%22value%22%3A%22%2F%2Fiam.googleapis.com%2Fprojects%2F123456789012%2Flocations%2Fglobal%2FworkloadIdentityPools%2Fpool-abc123d4%2Fproviders%2Fa" + | ||
| "ws-abc123d4%22%7D%2C%7B%22key%22%3A%22x-amz-security-token%22%2C%22value%22%3A%22GwoGZXIvYXdzEDQaDAVwVIzhCkoSIt9J6yLeAeVnJt7alj0kfoGSAYzy2uYkeOmLH" + | ||
| "gz8BxpbiFR1NwKd6QXDo09ZRVSoP1eUI286GXEX5PuY%2FkX3lctI6LYWhAxLl7d3xMNqcT%2B4Pw%2BaRvLNX6rCs71%2FgwZzOs7dkktam8j7zy5TWrWDy8JVU61P%2BiuAg%2B0gCW6dHzi" + | ||
| "s%2Frj0ProFQ9SSGD2CpO8CWoF144i%2BpAUMWLD%2B2s02YTRFASZX8wVokTv81cMiraH3FwPzdlXQ6%2BcYhz4%2FmvfVRp5RL2BHjMACOo860fmKRpyIi8maaDL8jcVJhWiPkZZkAXVMvSi" + | ||
| "ipNibBjJYih%2BX9LcsNFAjkSjlhiSBYms4Z9Mrbf5PAaMIIokO7B5%2F1VsaQjNfKY41DfitS3%2BVUzu2iCayn6mysrSsAaWeXZarieae%2BcWG3ihKq71AhOsDQkWoh8a%2Bzw%3D%3D%22" + | ||
| "%7D%2C%7B%22key%22%3A%22Authorization%22%2C%22value%22%3A%22AWS4-HMAC-SHA256%20Credential%3DBSIARD4OQDT6ATWAVS5C%2F19870504%2Fus-east-2%2Fsts%2Faw" + | ||
| "s4_request%2C%20SignedHeaders%3Dhost%3Bx-amz-date%3Bx-amz-security-token%3Bx-goog-cloud-target-resource%2C%20Signature%3D2e65b3fdfa86ba68f721bd458" + | ||
| "4f2fcbf8df9fad14ce3c7ea389c58af17bdc5a0%22%7D%5D%7D"; | ||
|
|
||
| [Fact] | ||
| public void Create() | ||
| { | ||
| // This is the reverse of what we do in production code, but it's harder to open all the implementation up | ||
| // for proper testing than to just repeat this step here. This is by far the simplest step of subject | ||
| // token generation as compared to building and signing the canonical request, etc. for which we are | ||
| // testing production code. | ||
| AwsSignedSubjectToken expected = NewtonsoftJsonSerializer.Instance.Deserialize<AwsSignedSubjectToken>(Uri.UnescapeDataString(SerializedSubjectToken)); | ||
|
|
||
| AwsSignedSubjectToken subjectToken = AwsSignedSubjectToken.Create( | ||
| new AwsSecurityCredentials(AwsSecretAcessKeyId, AwsSecretAccessKey, AwsToken), | ||
| new AwsRegion(AwsRegion), | ||
| new Uri(AwsVerificationUrl), | ||
| Audience, | ||
| new MockClock(SigningTimestampUtc)); | ||
|
|
||
| Assert.Equal(expected.Url, subjectToken.Url); | ||
| Assert.Equal(expected.HttpMethod, subjectToken.HttpMethod); | ||
| Assert.Equal(expected.Body, subjectToken.Body); | ||
|
|
||
| Assert.Equal(expected.Headers.Length, subjectToken.Headers.Length); | ||
|
|
||
| foreach(var expectedHeader in expected.Headers) | ||
| { | ||
| var actual = Assert.Single(subjectToken.Headers, header => header.Key == expectedHeader.Key); | ||
| Assert.Equal(expectedHeader.Value, actual.Value); | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,7 @@ public partial class AwsExternalAccountCredential | |
| /// access tokens. Google STS triggers the request as specified by the signed | ||
| /// request to verify the callers identity. | ||
| /// </summary> | ||
| private sealed class AwsSignedSubjectToken | ||
| internal sealed class AwsSignedSubjectToken | ||
| { | ||
| private const string StVerificationHttpMethod = "POST"; | ||
| private const string BasicFormatIso8601 = "yyyyMMdd'T'HHmmss'Z'"; | ||
|
|
@@ -43,22 +43,21 @@ private sealed class AwsSignedSubjectToken | |
| private const string AwsSha256Designation = "AWS4-HMAC-SHA256"; | ||
|
|
||
| [JsonProperty("url")] | ||
| public string Url { get; } | ||
| public string Url { get; set; } | ||
|
|
||
| [JsonProperty("method")] | ||
| public string HttpMethod { get; } | ||
| public string HttpMethod { get; set; } | ||
|
|
||
| [JsonProperty("body")] | ||
| public string Body { get; } | ||
| public string Body { get; set; } | ||
|
|
||
| [JsonProperty("headers")] | ||
| public AwsSignedSubjectTokenHeader[] Headers { get; } | ||
| public AwsSignedSubjectTokenHeader[] Headers { get; set; } | ||
|
|
||
| public AwsSignedSubjectToken() { } | ||
|
|
||
| private AwsSignedSubjectToken(string url, string httpMethod, string body, AwsSignedSubjectTokenHeader[] headers) | ||
| { | ||
| // We have no public parameterless constructor because | ||
| // we build this manually on the Create method and then | ||
| // serialize it. | ||
| Url = url; | ||
| HttpMethod = httpMethod; | ||
| Body = body; | ||
|
|
@@ -68,10 +67,12 @@ private AwsSignedSubjectToken(string url, string httpMethod, string body, AwsSig | |
| public sealed class AwsSignedSubjectTokenHeader | ||
| { | ||
| [JsonProperty("key")] | ||
| public string Key { get; } | ||
| public string Key { get; set; } | ||
|
|
||
| [JsonProperty("value")] | ||
| public string Value { get; } | ||
| public string Value { get; set; } | ||
|
|
||
| public AwsSignedSubjectTokenHeader() { } | ||
|
|
||
| internal AwsSignedSubjectTokenHeader(string key, string value) | ||
| { | ||
|
|
@@ -144,6 +145,7 @@ string GetHashedCanonicalRequest() | |
| canonicalRequest.Append(headerPair.Value); | ||
| canonicalRequest.Append(newLine); | ||
| } | ||
| canonicalRequest.Append(newLine); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I got what was wrong with my tests:
So I changed the code to add the line breaks after each element, not realizing that I was removing this line break with that. And I managed to cherry pick that change out of the last (and possibly more) local nuget package that I created, which I did rougly two hours before merging the original PR. I'm sorry about this! But I'm glad I understand what happened. The unit test that I have added will catch any such changes. And I've also run my manual tests with the code on this PR, and it's good. |
||
|
|
||
| // Now the list of headers that will be signed. | ||
| // See point 5 in https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| /* | ||
| Copyright 2022 Google LLC | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| https://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| using Google.Apis.Auth.OAuth2; | ||
| using Google.Apis.IAMCredentials.v1; | ||
| using Google.Apis.IAMCredentials.v1.Data; | ||
| using Google.Apis.Services; | ||
| using Google.Apis.Storage.v1; | ||
| using Google.Apis.Storage.v1.Data; | ||
| using IntegrationTests.Utils; | ||
| using System; | ||
| using System.Linq; | ||
| using System.Net.Http; | ||
| using System.Threading.Tasks; | ||
| using System.Xml.Linq; | ||
| using Xunit; | ||
|
|
||
| namespace IntegrationTests | ||
| { | ||
| public class ExternalAccountCredentialTests | ||
| { | ||
| [Fact(Skip = "Googlers see go/wif-integration-settings")] | ||
| public async Task AwsCredentials() | ||
| { | ||
| string gcpProjectNumber = ""; | ||
| string gcpPoolId = "pool-abc123d4"; | ||
|
|
||
| string awsAccountId = ""; | ||
| string awsProviderId = "aws-abc123d4"; | ||
| string awsRoleName = ""; | ||
| string awsAudience = $"//iam.googleapis.com/projects/{gcpProjectNumber}/locations/global/workloadIdentityPools/{gcpPoolId}/providers/{awsProviderId}"; | ||
| string awsArn = $"arn:aws:iam::{awsAccountId}:role/{awsRoleName}"; | ||
|
|
||
| // This really won't be running in AWS, so we need to obtain AWS credentials by using the AssumeRoleWithWebIdentity | ||
| // operation of AWS STS server. | ||
| // For this, we need an identity token to identify ourselves to AWS STS. | ||
| GoogleCredential gcpCredential = Helper.GetServiceCredential().CreateScoped("https://www.googleapis.com/auth/cloud-platform"); | ||
| string saEmail = (gcpCredential.UnderlyingCredential as ServiceAccountCredential).Id; | ||
| string saResourceName = $"projects/-/serviceAccounts/{saEmail}"; | ||
| IAMCredentialsService iamService = new IAMCredentialsService(new BaseClientService.Initializer | ||
| { | ||
| HttpClientInitializer = gcpCredential, | ||
| }); | ||
| var idToken = await iamService.Projects.ServiceAccounts.GenerateIdToken( | ||
| new GenerateIdTokenRequest | ||
| { | ||
| Audience = awsAudience, | ||
| IncludeEmail = true | ||
| }, | ||
| saResourceName).ExecuteAsync(); | ||
|
|
||
| // Using the id token, we can now obtain AWS credentials. | ||
| string assumeRoleUrl = $"https://sts.amazonaws.com/?" + | ||
| $"Action=AssumeRoleWithWebIdentity" + | ||
| $"&Version=2011-06-15" + | ||
| $"&DurationSeconds=3600" + | ||
| $"&RoleSessionName={awsRoleName}" + | ||
| $"&RoleArn={awsArn}" + | ||
| $"&WebIdentityToken={idToken.Token}"; | ||
|
|
||
| string content; | ||
| using (HttpClient httpClient = new HttpClient()) | ||
| { | ||
| HttpResponseMessage response = await httpClient.GetAsync(assumeRoleUrl); | ||
| content = await response.Content.ReadAsStringAsync(); | ||
| } | ||
| XElement awsCredentialXml = XElement.Parse(content); | ||
|
|
||
| // We store AWS credentials in environment variables to mimic an AWS environment. | ||
| Environment.SetEnvironmentVariable("AWS_ACCESS_KEY_ID", AwsCredentialValue("AccessKeyId")); | ||
| Environment.SetEnvironmentVariable("AWS_SECRET_ACCESS_KEY", AwsCredentialValue("SecretAccessKey")); | ||
| Environment.SetEnvironmentVariable("AWS_SESSION_TOKEN", AwsCredentialValue("SessionToken")); | ||
| // We store the AWS region in an environment variable to mimic an AWS environment. | ||
| Environment.SetEnvironmentVariable("AWS_REGION", "us-east-2"); | ||
|
|
||
| // Now we can create an AWS creential which will pick the relevant values from the environment variables. | ||
| AwsExternalAccountCredential awsToGcpCredential = new AwsExternalAccountCredential(new AwsExternalAccountCredential.Initializer( | ||
| tokenUrl: "https://sts.googleapis.com/v1/token", | ||
| audience: awsAudience, | ||
| subjectTokenType: "urn:ietf:params:aws:token-type:aws4_request", | ||
| regionalCredentialVerificationUrl: "https://sts.{region}.amazonaws.com?Action=GetCallerIdentity&Version=2011-06-15") | ||
| { | ||
| ServiceAccountImpersonationUrl = $"https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/{saEmail}:generateAccessToken", | ||
| Scopes = new string[] { StorageService.Scope.DevstorageFullControl } | ||
| }); | ||
|
|
||
| // And we make a request to Storage to confirm that our AWS credential is indeed valid. | ||
| StorageService client = new StorageService(new BaseClientService.Initializer | ||
| { | ||
| HttpClientInitializer = awsToGcpCredential | ||
| }); | ||
|
|
||
| // Following line will throw if authentication fails. | ||
| Buckets buckets = client.Buckets.List(Helper.GetProjectId()).Execute(); | ||
|
|
||
| // A final sanity-check. | ||
| Assert.NotNull(buckets.Items); | ||
|
|
||
| string AwsCredentialValue(string tagName) => | ||
| awsCredentialXml.Descendants().Single(element => element.Name.LocalName == tagName).Value; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like more than a refactor - this feels like it could be at the heart of the problem. (i.e. maybe we weren't deserializing properly). If this actually is the relevant change, maybe separate into a different commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is really just about testability. Production code doesn't deserialize at all, it only serializes.
Test code is now deserializing and that's why I had to make all these changes.