From 1e8c9d0c0247f3c4c44e7b83723013e6f9708615 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 11 Jun 2024 15:25:02 -0700 Subject: [PATCH] Prepare for the June Identity GA release. (#5695) * Prepare for the June Identity GA release. * Validate azure arc. * Update changelog entry. * Update cspell, fixup gtest skip, and remove unnecessary logging. * Move gtest_skip call inside the gtest. * Use system command due to permissions on creating a directory, on linux. * Pass in a c_str() to system. * Update permissions to create keys and address pr feedback (rename test var and method to remove 'valid'). * Address PR feedback - nits. * Fix remaining rename of local variable. --- .vscode/cspell.json | 1 + sdk/identity/azure-identity/CHANGELOG.md | 7 +- .../src/managed_identity_source.cpp | 76 ++++- .../src/private/package_version.hpp | 4 +- .../ut/managed_identity_credential_test.cpp | 322 +++++++++++++++++- 5 files changed, 398 insertions(+), 12 deletions(-) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 48733b4cf..4a88b7b53 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -70,6 +70,7 @@ "authcid", "avro", "antkmsft", + "azcmagent", "azcore", "azsdk", "azsdkengsys", diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 23b083466..bc3df34ff 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -1,14 +1,15 @@ # Release History -## 1.7.0-beta.3 (Unreleased) +## 1.8.0 (2024-06-11) ### Features Added -### Breaking Changes +- [[#4474]](https://github.com/Azure/azure-sdk-for-cpp/issues/4474) Enable proactive renewal of Managed Identity tokens. +- [[#5116]](https://github.com/Azure/azure-sdk-for-cpp/issues/5116) `AzureCliCredential`: Added support for the new response field which represents token expiration timestamp as time zone agnostic value. ### Bugs Fixed -### Other Changes +- Managed identity bug fixes. ## 1.7.0-beta.2 (2024-02-09) diff --git a/sdk/identity/azure-identity/src/managed_identity_source.cpp b/sdk/identity/azure-identity/src/managed_identity_source.cpp index 4f9290224..aa1b26a44 100644 --- a/sdk/identity/azure-identity/src/managed_identity_source.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_source.cpp @@ -6,12 +6,15 @@ #include "private/identity_log.hpp" #include +#include #include #include #include #include +#include // for stat() used to check file size + using namespace Azure::Identity::_detail; using Azure::Core::_internal::Environment; @@ -30,6 +33,73 @@ void PrintEnvNotSetUpMessage(std::string const& credName, std::string const& cre credName + ": Environment is not set up for the credential to be created" + WithSourceMessage(credSource) + '.'); } + +// ExpectedArcKeyDirectory returns the directory expected to contain Azure Arc keys. +std::string ExpectedArcKeyDirectory() +{ + using Azure::Core::Credentials::AuthenticationException; + +#if defined(AZ_PLATFORM_LINUX) + return "/var/opt/azcmagent/tokens"; +#elif defined(AZ_PLATFORM_WINDOWS) + const std::string programDataPath{ + Azure::Core::_internal::Environment::GetVariable("ProgramData")}; + if (programDataPath.empty()) + { + throw AuthenticationException("Unable to get ProgramData folder path."); + } + return programDataPath + "\\AzureConnectedMachineAgent\\Tokens"; +#else + throw AuthenticationException("Unsupported OS. Arc supports only Linux and Windows."); +#endif +} + +static constexpr off_t MaximumAzureArcKeySize = 4096; + +#if defined(AZ_PLATFORM_WINDOWS) +static constexpr char DirectorySeparator = '\\'; +#else +static constexpr char DirectorySeparator = '/'; +#endif + +// Validates that a given Azure Arc MSI file path is valid for use. +// The specified file must: +// - be in the expected directory for the OS +// - have a .key extension +// - contain at most 4096 bytes +void ValidateArcKeyFile(std::string fileName) +{ + using Azure::Core::Credentials::AuthenticationException; + + std::string directory; + const size_t lastSlashIndex = fileName.rfind(DirectorySeparator); + if (std::string::npos != lastSlashIndex) + { + directory = fileName.substr(0, lastSlashIndex); + } + if (directory != ExpectedArcKeyDirectory() || fileName.size() < 5 + || fileName.substr(fileName.size() - 4) != ".key") + { + throw AuthenticationException( + "The file specified in the 'WWW-Authenticate' header in the response from Azure Arc " + "Managed Identity Endpoint has an unexpected file path."); + } + + struct stat s; + if (!stat(fileName.c_str(), &s)) + { + if (s.st_size > MaximumAzureArcKeySize) + { + throw AuthenticationException( + "The file specified in the 'WWW-Authenticate' header in the response from Azure Arc " + "Managed Identity Endpoint is larger than 4096 bytes."); + } + } + else + { + throw AuthenticationException("Failed to get file size for '" + fileName + "'."); + } +} } // namespace Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( @@ -352,7 +422,11 @@ Azure::Core::Credentials::AccessToken AzureArcManagedIdentitySource::GetToken( } auto request = createRequest(); - std::ifstream secretFile(challenge.substr(eq + 1)); + + const std::string fileName = challenge.substr(eq + 1); + ValidateArcKeyFile(fileName); + + std::ifstream secretFile(fileName); request->HttpRequest.SetHeader( "Authorization", "Basic " diff --git a/sdk/identity/azure-identity/src/private/package_version.hpp b/sdk/identity/azure-identity/src/private/package_version.hpp index 167cc2c1c..9beeb23dd 100644 --- a/sdk/identity/azure-identity/src/private/package_version.hpp +++ b/sdk/identity/azure-identity/src/private/package_version.hpp @@ -11,9 +11,9 @@ #include #define AZURE_IDENTITY_VERSION_MAJOR 1 -#define AZURE_IDENTITY_VERSION_MINOR 7 +#define AZURE_IDENTITY_VERSION_MINOR 8 #define AZURE_IDENTITY_VERSION_PATCH 0 -#define AZURE_IDENTITY_VERSION_PRERELEASE "beta.3" +#define AZURE_IDENTITY_VERSION_PRERELEASE "" #define AZURE_IDENTITY_VERSION_ITOA_HELPER(i) #i #define AZURE_IDENTITY_VERSION_ITOA(i) AZURE_IDENTITY_VERSION_ITOA_HELPER(i) diff --git a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp index 2f7f44636..edad7e6f4 100644 --- a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp @@ -5,11 +5,27 @@ #include "credential_test_helper.hpp" #include +#include #include #include +#if defined(AZ_PLATFORM_LINUX) +#include + +#include // for mkdir +#include +#elif defined(AZ_PLATFORM_WINDOWS) +#if !defined(WIN32_LEAN_AND_MEAN) +#define WIN32_LEAN_AND_MEAN +#endif +#if !defined(NOMINMAX) +#define NOMINMAX +#endif +#include +#endif + using Azure::Core::Credentials::TokenCredentialOptions; using Azure::Core::Http::HttpMethod; using Azure::Core::Http::HttpStatusCode; @@ -785,6 +801,58 @@ TEST(ManagedIdentityCredential, CloudShellInvalidUrl) {"{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"})); } +// This helper creates the necessary directories required for the Azure Arc tests, and returns where +// we expect the valid arc key to exist. +std::string CreateDirectoryAndGetKeyPath() +{ + std::string keyPath; +#if defined(AZ_PLATFORM_LINUX) + keyPath = "/var/opt/azcmagent/tokens"; + int result = system(std::string("sudo mkdir -p ").append(keyPath).c_str()); + if (result != 0 && errno != EEXIST) + { + GTEST_LOG_(ERROR) << "Directory creation failure in an AzureArc test: " << keyPath + << " Result: " << result << " Error : " << errno; + EXPECT_TRUE(false); + } + // Add write permission for owner, group, and others + result = system(std::string("sudo chmod -R 0777 ").append(keyPath).c_str()); + if (result != 0) + { + GTEST_LOG_(ERROR) << "Failed to change permissions for " << keyPath << ": " << strerror(errno) + << std::endl; + EXPECT_TRUE(false); + } + keyPath += "/"; +#elif defined(AZ_PLATFORM_WINDOWS) + keyPath = Azure::Core::_internal::Environment::GetVariable("ProgramData"); + if (keyPath.empty()) + { + GTEST_LOG_(ERROR) << "We can't get ProgramData folder path in an AzureArc test."; + EXPECT_TRUE(false); + } + // Unlike linux, we can't use mkdir on Windows, since it is deprecated. We will use + // CreateDirectory instead. + // https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/mkdir?view=msvc-170 + keyPath += "\\AzureConnectedMachineAgent"; + if (!CreateDirectory(keyPath.c_str(), NULL) && GetLastError() != ERROR_ALREADY_EXISTS) + { + GTEST_LOG_(ERROR) << "Directory creation failure in an AzureArc test: " << keyPath + << " Error: " << GetLastError(); + EXPECT_TRUE(false); + } + keyPath += "\\Tokens"; + if (!CreateDirectory(keyPath.c_str(), NULL) && GetLastError() != ERROR_ALREADY_EXISTS) + { + GTEST_LOG_(ERROR) << "Directory creation failure in an an AzureArc test: " << keyPath + << " Error: " << GetLastError(); + EXPECT_TRUE(false); + } + keyPath += "\\"; +#endif + return keyPath; +} + TEST(ManagedIdentityCredential, AzureArc) { using Azure::Core::Diagnostics::Logger; @@ -793,23 +861,32 @@ TEST(ManagedIdentityCredential, AzureArc) Logger::SetLevel(Logger::Level::Verbose); Logger::SetListener([&](auto lvl, auto msg) { log.push_back(std::make_pair(lvl, msg)); }); + std::string keyPath = CreateDirectoryAndGetKeyPath(); + if (keyPath.empty()) + { + GTEST_SKIP_("Skipping AzureArc test on unsupported OSes."); + } + { std::ofstream secretFile( - "managed_identity_credential_test1.txt", std::ios_base::out | std::ios_base::trunc); + keyPath + "managed_identity_credential_test1.key", + std::ios_base::out | std::ios_base::trunc); secretFile << "SECRET1"; } { std::ofstream secretFile( - "managed_identity_credential_test2.txt", std::ios_base::out | std::ios_base::trunc); + keyPath + "managed_identity_credential_test2.key", + std::ios_base::out | std::ios_base::trunc); secretFile << "SECRET2"; } { std::ofstream secretFile( - "managed_identity_credential_test3.txt", std::ios_base::out | std::ios_base::trunc); + keyPath + "managed_identity_credential_test3.key", + std::ios_base::out | std::ios_base::trunc); secretFile << "SECRET3"; } @@ -862,15 +939,15 @@ TEST(ManagedIdentityCredential, AzureArc) {{"https://azure.com/.default"}, {"https://outlook.com/.default"}, {}}, {{HttpStatusCode::Unauthorized, "", - {{"WWW-Authenticate", "ABC ABC=managed_identity_credential_test1.txt"}}}, + {{"WWW-Authenticate", "ABC ABC=" + keyPath + "managed_identity_credential_test1.key"}}}, {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}, {HttpStatusCode::Unauthorized, "", - {{"WWW-Authenticate", "XYZ XYZ=managed_identity_credential_test2.txt"}}}, + {{"WWW-Authenticate", "XYZ XYZ=" + keyPath + "managed_identity_credential_test2.key"}}}, {HttpStatusCode::Ok, "{\"expires_in\":7200, \"access_token\":\"ACCESSTOKEN2\"}", {}}, {HttpStatusCode::Unauthorized, "", - {{"WWW-Authenticate", "ABC ABC=managed_identity_credential_test3.txt"}}}, + {{"WWW-Authenticate", "ABC ABC=" + keyPath + "managed_identity_credential_test3.key"}}}, {HttpStatusCode::Ok, "{\"expires_in\":9999, \"access_token\":\"ACCESSTOKEN3\"}", {}}}); EXPECT_EQ(actual.Requests.size(), 6U); @@ -1142,6 +1219,239 @@ TEST(ManagedIdentityCredential, AzureArcAuthHeaderTwoEquals) })); } +TEST(ManagedIdentityCredential, AzureArcInvalidKey) +{ + using Azure::Core::Credentials::AccessToken; + using Azure::Core::Credentials::AuthenticationException; + + std::string keyPath; + +#if defined(AZ_PLATFORM_LINUX) + keyPath = "/var/opt/azcmagent/tokens/"; +#elif defined(AZ_PLATFORM_WINDOWS) + keyPath = Azure::Core::_internal::Environment::GetVariable("ProgramData"); + if (keyPath.empty()) + { + GTEST_LOG_(ERROR) << "We can't get ProgramData folder path in AzureArcInvalidKey test."; + EXPECT_TRUE(false); + } + keyPath += "\\AzureConnectedMachineAgent\\Tokens\\"; +#else + // Unsupported OS + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + return std::make_unique(options); + }, + {{"https://azure.com/.default"}}, + {{HttpStatusCode::Unauthorized, "", {{"WWW-Authenticate", "ABC ABC=foo.key"}}}, + {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}}, + [](auto& credential, auto& tokenRequestContext, auto& context) { + AccessToken token; + EXPECT_THROW( + token = credential.GetToken(tokenRequestContext, context), AuthenticationException); + return token; + })); + + GTEST_SKIP_("Skipping the rest of AzureArcInvalidKey tests on unsupported OSes."); +#endif + + // Invalid Key Path - empty directory + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + return std::make_unique(options); + }, + {{"https://azure.com/.default"}}, + {{HttpStatusCode::Unauthorized, "", {{"WWW-Authenticate", "ABC ABC=foo.key"}}}, + {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}}, + [](auto& credential, auto& tokenRequestContext, auto& context) { + AccessToken token; + EXPECT_THROW( + token = credential.GetToken(tokenRequestContext, context), AuthenticationException); + return token; + })); + + // Invalid Key Path - unexpected directory + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + return std::make_unique(options); + }, + {{"https://azure.com/.default"}}, + {{HttpStatusCode::Unauthorized, "", {{"WWW-Authenticate", "ABC ABC=C:\\Foo\\foo.key"}}}, + {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}}, + [](auto& credential, auto& tokenRequestContext, auto& context) { + AccessToken token; + EXPECT_THROW( + token = credential.GetToken(tokenRequestContext, context), AuthenticationException); + return token; + })); + + // Invalid Key Path - unexpected extension, filename is short + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + return std::make_unique(options); + }, + {{"https://azure.com/.default"}}, + {{HttpStatusCode::Unauthorized, "", {{"WWW-Authenticate", "ABC ABC=" + keyPath + "a.b"}}}, + {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}}, + [](auto& credential, auto& tokenRequestContext, auto& context) { + AccessToken token; + EXPECT_THROW( + token = credential.GetToken(tokenRequestContext, context), AuthenticationException); + return token; + })); + + // Invalid Key Path - unexpected extension + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + return std::make_unique(options); + }, + {{"https://azure.com/.default"}}, + {{HttpStatusCode::Unauthorized, "", {{"WWW-Authenticate", "ABC ABC=" + keyPath + "foo.txt"}}}, + {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}}, + [](auto& credential, auto& tokenRequestContext, auto& context) { + AccessToken token; + EXPECT_THROW( + token = credential.GetToken(tokenRequestContext, context), AuthenticationException); + return token; + })); + + // Invalid Key Path - file missing + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + return std::make_unique(options); + }, + {{"https://azure.com/.default"}}, + {{HttpStatusCode::Unauthorized, "", {{"WWW-Authenticate", "ABC ABC=" + keyPath + "foo.key"}}}, + {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}}, + [](auto& credential, auto& tokenRequestContext, auto& context) { + AccessToken token; + EXPECT_THROW( + token = credential.GetToken(tokenRequestContext, context), AuthenticationException); + return token; + })); + + keyPath = CreateDirectoryAndGetKeyPath(); + if (keyPath.empty()) + { + GTEST_SKIP_("Skipping AzureArcInvalidKey test on unsupported OSes."); + } + + { + std::ofstream secretFile(keyPath + "toolarge.key", std::ios_base::out | std::ios_base::trunc); + + if (!secretFile.is_open()) + { + GTEST_LOG_(ERROR) << "Failed to create a test file required in AzureArcInvalidKey test."; + EXPECT_TRUE(false); + } + + std::string fileContents(4097, '.'); + + secretFile << fileContents; + } + + // Invalid Key Path - file too large + static_cast(CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + TokenCredentialOptions options; + options.Transport.Transport = transport; + + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", "https://xbox.com/"}, + {"IDENTITY_HEADER", ""}, + {"IDENTITY_SERVER_THUMBPRINT", "0123456789abcdef0123456789abcdef01234567"}, + }); + + return std::make_unique(options); + }, + {{"https://azure.com/.default"}}, + {{HttpStatusCode::Unauthorized, + "", + {{"WWW-Authenticate", "ABC ABC=" + keyPath + "toolarge.key"}}}, + {HttpStatusCode::Ok, "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}", {}}}, + [](auto& credential, auto& tokenRequestContext, auto& context) { + AccessToken token; + EXPECT_THROW( + token = credential.GetToken(tokenRequestContext, context), AuthenticationException); + return token; + })); +} + TEST(ManagedIdentityCredential, AzureArcInvalidUrl) { using Azure::Core::Credentials::AccessToken;