Skip to content
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

Support the EC2 Metadata Server #115

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
411b67b
EC2Metadata server initial commit
sjperkins Sep 21, 2023
0479bdb
Fix header improt
sjperkins Sep 21, 2023
92285db
Consolidate path constants into url constants
sjperkins Sep 21, 2023
5b676b5
Remove unnecessary //tensorstore/internal/json_binding:bindable
sjperkins Sep 21, 2023
0219b84
Add headers
sjperkins Sep 21, 2023
5cbcb5b
Shorten calls to HttpRequestBuilder
sjperkins Sep 21, 2023
184c4bd
url in square brackets
sjperkins Sep 21, 2023
9d090be
Fix url
sjperkins Sep 23, 2023
80d1feb
Improve comment
sjperkins Sep 23, 2023
6d43437
url
sjperkins Sep 23, 2023
09fc335
raw string literal for json response
sjperkins Sep 23, 2023
30c50bf
Cater for EC2IamCode != "Success"
sjperkins Sep 23, 2023
5e0b844
Documentation
sjperkins Sep 23, 2023
9e31e18
kSuccess
sjperkins Sep 23, 2023
1012da1
Failure to retrieve credentials doesn't imply unauthenticated, but no…
sjperkins Sep 23, 2023
e148544
Test case copyright notice
sjperkins Sep 23, 2023
1e0d6dc
Improve test case
sjperkins Sep 23, 2023
94bf245
Formatting
sjperkins Sep 23, 2023
b9ec0e9
Add a test case for the Non-existent IAM role
sjperkins Sep 23, 2023
73df548
Add a test case for revoked IAM Role
sjperkins Sep 23, 2023
b568308
Clarify docs
sjperkins Sep 23, 2023
aae7d86
Test case name
sjperkins Sep 23, 2023
9f81367
formatting
sjperkins Sep 23, 2023
4141498
Formatting
sjperkins Sep 23, 2023
ff93dc5
formatting
sjperkins Sep 23, 2023
6fbad81
Merge branch 'master' into ec2-metadata-credential-provider
sjperkins Oct 2, 2023
7557ce7
IWYU
sjperkins Oct 2, 2023
802991a
HttpRequestBuilder now a class
sjperkins Oct 2, 2023
db2b917
Remove unnecessary json bindings
sjperkins Oct 2, 2023
520550d
Fix more http://
sjperkins Oct 2, 2023
9ec5bc2
Align EC2 Metadata retrieval further with go implementation
sjperkins Oct 3, 2023
97e7236
whitespace
sjperkins Oct 3, 2023
2ff9037
Fix include order
sjperkins Oct 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions tensorstore/kvstore/s3/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,18 @@ tensorstore_cc_test(

tensorstore_cc_library(
name = "aws_credential_provider",
srcs = ["aws_credential_provider.cc"],
hdrs = ["aws_credential_provider.h"],
srcs = [
"aws_credential_provider.cc",
"aws_metadata_credential_provider.cc"],
hdrs = [
"aws_credential_provider.h",
"aws_metadata_credential_provider.h"
],
deps = [
"//tensorstore/internal:env",
"//tensorstore/internal/json_binding",
"//tensorstore/internal/json_binding:absl_time",
"//tensorstore/internal/json_binding:bindable",
"//tensorstore/internal:no_destructor",
"//tensorstore/internal:path",
"//tensorstore/internal/http",
Expand All @@ -259,6 +267,21 @@ tensorstore_cc_test(
],
)


tensorstore_cc_test(
name = "aws_metadata_credential_provider_test",
srcs = ["aws_metadata_credential_provider_test.cc"],
deps = [
":aws_credential_provider",
"//tensorstore/internal:test_util",
"//tensorstore/internal/http:curl_transport",
"//tensorstore/util:result",
"//tensorstore/util:status_testutil",
"@com_google_googletest//:gtest_main",
],
)


tensorstore_cc_test(
name = "localstack_test",
srcs = ["localstack_test.cc"],
Expand Down
1 change: 1 addition & 0 deletions tensorstore/kvstore/s3/aws_credential_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "absl/strings/ascii.h"
#include "absl/strings/str_cat.h"
#include "absl/synchronization/mutex.h"
#include "tensorstore/kvstore/s3/aws_metadata_credential_provider.h"
#include "tensorstore/internal/env.h"
#include "tensorstore/internal/http/http_transport.h"
#include "tensorstore/internal/no_destructor.h"
Expand Down
23 changes: 4 additions & 19 deletions tensorstore/kvstore/s3/aws_credential_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef TENSORSTORE_KVSTORE_S3_S3_CREDENTIAL_PROVIDER_H
#define TENSORSTORE_KVSTORE_S3_S3_CREDENTIAL_PROVIDER_H
#ifndef TENSORSTORE_KVSTORE_S3_AWS_CREDENTIAL_PROVIDER_H
#define TENSORSTORE_KVSTORE_S3_AWS_CREDENTIAL_PROVIDER_H

#include <functional>
#include <memory>
Expand Down Expand Up @@ -56,6 +56,7 @@ class AwsCredentialProvider {
virtual Result<AwsCredentials> GetCredentials() = 0;
};


/// Provides credentials from the following environment variables:
/// AWS_ACCESS_KEY_ID, AWS_SECRET_KEY_ID, AWS_SESSION_TOKEN
class EnvironmentCredentialProvider : public AwsCredentialProvider {
Expand Down Expand Up @@ -89,22 +90,6 @@ class FileCredentialProvider : public AwsCredentialProvider {
Result<AwsCredentials> GetCredentials() override;
};

/// Provides S3 credentials from the EC2 Metadata server
/// if running within AWS
class EC2MetadataCredentialProvider : public AwsCredentialProvider {
private:
std::shared_ptr<internal_http::HttpTransport> transport_;

public:
EC2MetadataCredentialProvider(
std::shared_ptr<internal_http::HttpTransport> transport)
: transport_(std::move(transport)) {}

Result<AwsCredentials> GetCredentials() override {
return absl::UnimplementedError("EC2 Metadata Server");
}
};

using AwsCredentialProviderFn =
std::function<Result<std::unique_ptr<AwsCredentialProvider>>()>;

Expand All @@ -118,4 +103,4 @@ Result<std::unique_ptr<AwsCredentialProvider>> GetAwsCredentialProvider(
} // namespace internal_kvstore_s3
} // namespace tensorstore

#endif // TENSORSTORE_KVSTORE_S3_S3_CREDENTIAL_PROVIDER_H
#endif // TENSORSTORE_KVSTORE_S3_AWS_CREDENTIAL_PROVIDER_H
179 changes: 179 additions & 0 deletions tensorstore/kvstore/s3/aws_metadata_credential_provider.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
// Copyright 2023 The TensorStore Authors
//
// 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
//
// http://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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another include-order detail. If there is a .h file which directly correlates with the .cc file (a header with the declarations and the same name), then we include it before anything else.

#include "aws_metadata_credential_provider.h"
sjperkins marked this conversation as resolved.
Show resolved Hide resolved

#include "tensorstore/internal/json_binding/absl_time.h"
#include "tensorstore/internal/json_binding/bindable.h"
#include "tensorstore/internal/json_binding/json_binding.h"

using ::tensorstore::Result;
using ::tensorstore::internal::ParseJson;
using ::tensorstore::internal_http::HttpResponseCodeToStatus;
using ::tensorstore::internal_kvstore_s3::AwsCredentials;

namespace jb = tensorstore::internal_json_binding;

namespace tensorstore {
namespace internal_kvstore_s3 {

namespace {

// EC2 Metadata server url
static constexpr char kEc2MetadataUrl[] = "http://http://169.254.169.254";
// Token ttl header
static constexpr char kTokenTtlHeader[] = "x-aws-ec2-metadata-token-ttl-seconds";
// Token header
static constexpr char kMetadataTokenHeader[] = "x-aws-ec2-metadata-token";

static constexpr char kTokenPath[] = "/latest/api/token";
static constexpr char kIamPath[] = "/latest/meta-data/iam/";
static constexpr char kIamSecurityCredentialsPath[] = "/latest/meta-data/iam/security-credentials/";

// Requests to the above server block outside AWS
// Configure a timeout small enough not to degrade performance outside AWS
// but large enough inside AWS
static constexpr absl::Duration kConnectTimeout = absl::Milliseconds(200);
Copy link
Collaborator

Choose a reason for hiding this comment

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

include any directly named types. I notice at least, in this file:

string
absl/time
absl/strings/str_cat
tensorstore/json_binding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I think the existing tensorstore/internal/json_binding/* imports covers the last case.

Copy link
Collaborator

@laramiel laramiel Sep 21, 2023

Choose a reason for hiding this comment

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

There are a lot more includes missing.

Look at the using declarations, for example. Anything that you spell out, such as 'tensorstore::Result' needs an import in each file where it is spelled (include-what-you-use); we don't rely on transitive includes for any of those cases. We only permit transitive includes for non-spelled field access and auto type use.

json_binding is different, though. Those includes are needed to satisfy the need for type-specializations to be visible where they are used, such as bindings for absl::Time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks for pointing this out. I've gone through the file on a IWYU basis and adjusted the #includes accordingly.


/// Represents JSON returned from
/// http://http://169.254.169.254/latest/meta-data/iam/security-credentials/<iam-role>/
/// where <iam-role> is usually returned as a response from a request to
/// http://http://169.254.169.254/latest/meta-data/iam/security-credentials/
struct EC2CredentialsResponse {
std::string Code;
absl::Time LastUpdated;
std::string Type;
std::string AccessKeyId;
std::string SecretAccessKey;
std::string Token;
absl::Time Expiration;

using ToJsonOptions = IncludeDefaults;
sjperkins marked this conversation as resolved.
Show resolved Hide resolved
using FromJsonOptions = internal_json_binding::NoOptions;

TENSORSTORE_DECLARE_JSON_DEFAULT_BINDER(
EC2CredentialsResponse,
internal_kvstore_s3::EC2CredentialsResponse::FromJsonOptions,
internal_kvstore_s3::EC2CredentialsResponse::ToJsonOptions)
};

inline constexpr auto EC2CredentialsResponseBinder = jb::Object(
jb::Member("Code", jb::Projection(&EC2CredentialsResponse::Code)),
jb::Member("LastUpdated", jb::Projection(&EC2CredentialsResponse::LastUpdated)),
jb::Member("Type", jb::Projection(&EC2CredentialsResponse::Type)),
jb::Member("AccessKeyId", jb::Projection(&EC2CredentialsResponse::AccessKeyId)),
jb::Member("SecretAccessKey", jb::Projection(&EC2CredentialsResponse::SecretAccessKey)),
jb::Member("Token", jb::Projection(&EC2CredentialsResponse::Token)),
jb::Member("Expiration", jb::Projection(&EC2CredentialsResponse::Expiration))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect that, except for Code, all of these should be optional to encourage success in json parsing, when Code != "Success".

A sample from https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#instance-metadata-security-credentials

{
  "Code" : "Success",
  "LastUpdated" : "2012-04-26T16:39:16Z",
  "Type" : "AWS-HMAC",
  "AccessKeyId" : "ASIAIOSFODNN7EXAMPLE",
  "SecretAccessKey" : "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
  "Token" : "token",
  "Expiration" : "2017-05-17T15:09:54Z"
}

I'll try find the schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made all members optional except except Code and added a test case for the Code == SomethingOtherThanSuccess case.

);

TENSORSTORE_DEFINE_JSON_DEFAULT_BINDER(EC2CredentialsResponse,
[](auto is_loading, const auto& options,
auto* obj, ::nlohmann::json* j) {
return EC2CredentialsResponseBinder(
is_loading, options, obj, j);
})

} // namespace



Result<AwsCredentials> EC2MetadataCredentialProvider::GetCredentials() {
/// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-retrieval.html#instancedata-meta-data-retrieval-examples
/// https://hackingthe.cloud/aws/exploitation/ec2-metadata-ssrf/

/// Get a token for communicating with the EC2 Metadata server
auto token_request = internal_http::HttpRequestBuilder("POST",
absl::StrCat(kEc2MetadataUrl, kTokenPath))
.AddHeader(absl::StrCat(kTokenTtlHeader, ": 21600"))
.BuildRequest();

TENSORSTORE_ASSIGN_OR_RETURN(
auto token_response,
transport_->IssueRequest(token_request, {}, absl::InfiniteDuration(), kConnectTimeout).result());

TENSORSTORE_RETURN_IF_ERROR(HttpResponseCodeToStatus(token_response));

auto token_header = tensorstore::StrCat(kMetadataTokenHeader, ": ", token_response.payload);

auto iam_request = internal_http::HttpRequestBuilder("GET",
absl::StrCat(kEc2MetadataUrl, kIamPath))
.AddHeader(token_header)
.BuildRequest();

TENSORSTORE_ASSIGN_OR_RETURN(
auto iam_response,
transport_->IssueRequest(iam_request, {}).result()
)

// No associated IAM role, implies anonymous access?
if(iam_response.status_code == 404) {
return AwsCredentials{};
}

TENSORSTORE_RETURN_IF_ERROR(HttpResponseCodeToStatus(iam_response));

// IAM Role has been revoked, assume anonymous access?
if(iam_response.payload.empty()) {
return AwsCredentials{};
}

auto iam_role_request = internal_http::HttpRequestBuilder{"GET",
absl::StrCat(kEc2MetadataUrl, kIamSecurityCredentialsPath)}
sjperkins marked this conversation as resolved.
Show resolved Hide resolved
.AddHeader(token_header)
.BuildRequest();

TENSORSTORE_ASSIGN_OR_RETURN(
auto iam_role_response,
transport_->IssueRequest(iam_role_request, {}).result());

TENSORSTORE_RETURN_IF_ERROR(HttpResponseCodeToStatus(iam_role_response));

auto iam_credentials_request_url = tensorstore::StrCat(kEc2MetadataUrl,
kIamSecurityCredentialsPath,
iam_role_response.payload);

auto iam_credentials_request = internal_http::HttpRequestBuilder{"GET",
iam_credentials_request_url}
.AddHeader(token_header)
.BuildRequest();

TENSORSTORE_ASSIGN_OR_RETURN(
auto iam_credentials_response,
transport_->IssueRequest(iam_credentials_request, {}).result());

TENSORSTORE_RETURN_IF_ERROR(HttpResponseCodeToStatus(iam_credentials_response));

auto json_sv = iam_credentials_response.payload.Flatten();

TENSORSTORE_ASSIGN_OR_RETURN(
auto iam_credentials,
EC2CredentialsResponse::FromJson(ParseJson(json_sv)));

if(iam_credentials.Code != "Success") {
return absl::UnauthenticatedError(
absl::StrCat("EC2Metadata request to ",
iam_credentials_request_url,
" failed with ", json_sv));

}

return AwsCredentials{
iam_credentials.AccessKeyId,
iam_credentials.SecretAccessKey,
iam_credentials.Token};
}

} // namespace tensorstore
} // namespace internal_kvstore_s3
43 changes: 43 additions & 0 deletions tensorstore/kvstore/s3/aws_metadata_credential_provider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2023 The TensorStore Authors
//
// 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
//
// http://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.

#ifndef TENSORSTORE_KVSTORE_S3_AWS_METADATA_CREDENTIAL_PROVIDER_H
#define TENSORSTORE_KVSTORE_S3_AWS_METADATA_CREDENTIAL_PROVIDER_H

#include "tensorstore/kvstore/s3/aws_credential_provider.h"
#include "tensorstore/util/str_cat.h"


namespace tensorstore {
namespace internal_kvstore_s3 {

/// Provides S3 credentials from the EC2 Metadata server
/// if running within AWS
class EC2MetadataCredentialProvider : public AwsCredentialProvider {
private:
std::shared_ptr<internal_http::HttpTransport> transport_;

public:
EC2MetadataCredentialProvider(
std::shared_ptr<internal_http::HttpTransport> transport)
: transport_(std::move(transport)) {}

Result<AwsCredentials> GetCredentials() override;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like we may want to convert this from Result<AwsCredentials> GetCredentials() to Future<AwsCredentials> GetCredentials().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could do so. I guess this would imply returning a Future in the AwsCredentialProvider interface too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Those are separate issues, but perhaps. Also, defer that work if you want until we get this done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like we may want to convert this from Result<AwsCredentials> GetCredentials() to Future<AwsCredentials> GetCredentials().

I guess this so that the GetCredentials here can be converted into chained futures so as to not block the thread on which futures are submitted?

};

} // namespace internal_kvstore_s3
} // namespace tensorstore


#endif // TENSORSTORE_KVSTORE_S3_AWS_METADATA_CREDENTIAL_PROVIDER_H
Loading