Fixed Live Tests for Attestation SDK. #3366

ahsonkhan · 2022-03-30T03:32:17Z

This API addition, as a complement to ToLower makes sense to add.

However, unlike private APIs in the _detail namespace, we cannot make breaking changes to things in _internal. Therefore, we typically only add public surface area to such headers within the SDK/Azure Core when an actual upstream service SDK package within the repo needs it. In this case, this looks to only be used by unit tests.

In general, we'd avoid adding APIs to the SDK that are only used by unit tests :)

ahsonkhan · 2022-03-30T03:29:10Z

nit: Extra period

Suggested change

GetErrorAndThrow("Error while setting client cert context to ignore..");

GetErrorAndThrow("Error while setting client cert context to ignore.");

-Original file line number
+Diff line change
@@ Expand Up / @@ -149,6 +149,8 @@ jobs: @@
         CmakeArgs: ""
         AZURE_TEST_MODE: "LIVE"
         AZURE_LOG_LEVEL: "verbose"
+        # Surface the ServiceDirectory parameter as an environment variable so tests can take advantage of it.
+        AZURE_SERVICE_DIRECTORY: ${{ parameters.ServiceDirectory }}
       steps:
       - checkout: self
@@ Expand Down Expand Up / @@ -218,6 +220,7 @@ jobs: @@
         # This enables to run tests and samples at the same time as different matrix configuration.
         # Then unit-tests runs, samples should not run.
         condition: and(succeeded(), ne(variables['RunSamples'], '1'))
       - task: PublishTestResults@2
         inputs:
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     The `AttestationClientBuilder` class is used to create instances of the attestation client:
     ```cpp readme-sample-create-synchronous-client
+        std::string endpoint = std::getenv("ATTESTATION_AAD_URL");
+        AttestationClientOptions options;
+        return std::make_unique<Azure::Security::Attestation::AttestationClient>(m_endpoint, options);
     ```
+    If the attestation APIs require authentication, use the following:
+    ```cpp readme-sample-create-synchronous-client
+    std::string endpoint = std::getenv("ATTESTATION_AAD_URL");
+    AttestationClientOptions options;
+        std::shared_ptr<Azure::Core::Credentials::TokenCredential> credential
+            = std::make_shared<Azure::Identity::ClientSecretCredential>(
+                GetEnv("AZURE_TENANT_ID"), GetEnv("AZURE_CLIENT_ID"), GetEnv("AZURE_CLIENT_SECRET"));
+    return std::make_unique<Azure::Security::Attestation::AttestationClient>(m_endpoint, credential, options);
+    ```
+    The same pattern is used to create an `Azure::Security::Attestation::AttestationAdministrationClient`.
     #### Retrieve Token Certificates
-    Use `listAttestationSigners` to retrieve the set of certificates, which can be used to validate the token returned from the attestation service.
+    Use `GetAttestationSigningCertificates` to retrieve the set of certificates, which can be used to validate the token returned from the attestation service.
     Normally, this information is not required as the attestation SDK will perform the validation as a part of the interaction with the
     attestation service, however the APIs are provided for completeness and to facilitate customer's independently validating
     attestation results.
     ```cpp readme-sample-getSigningCertificates
+    auto attestationSigners = attestationClient->GetAttestationSigningCertificates();
+    // Enumerate the signers.
+    for (const auto& signer : attestationSigners.Value.Signers)
+    {
+    }
     ```
     #### Attest an SGX Enclave
@@ Expand All @@
     All administrative clients are authenticated.
-    ```cpp readme-sample-create-admin-client
-    AttestationAdministrationClientBuilder attestationBuilder = new AttestationAdministrationClientBuilder();
-    // Note that the "policy" calls require authentication.
-    AttestationAdministrationClient client = attestationBuilder
-        .endpoint(endpoint)
-        .credential(new DefaultAzureCredentialBuilder().build())
-        .buildClient();
+    ```cpp readme-sample-create-synchronous-client
+    std::string endpoint = std::getenv("ATTESTATION_AAD_URL");
+    AttestationClientOptions options;
+      std::shared_ptr<Azure::Core::Credentials::TokenCredential> credential
+            = std::make_shared<Azure::Identity::ClientSecretCredential>(
+                GetEnv("AZURE_TENANT_ID"), GetEnv("AZURE_CLIENT_ID"), GetEnv("AZURE_CLIENT_SECRET"));
+    auto adminClient =  std::make_unique<AttestationAdministrationClient>(m_endpoint, credential, options);
     ```
     #### Retrieve current attestation policy for OpenEnclave
     Use the `GetAttestationPolicy` API to retrieve the current attestation policy for a given TEE.
-    ```java readme-sample-getCurrentPolicy
-    String currentPolicy = client.getAttestationPolicy(AttestationType.OPEN_ENCLAVE);
-    System.out.printf("Current policy for OpenEnclave is: %s\n", currentPolicy);
+    ```cpp readme-sample-getCurrentPolicy
+    auto currentPolicy = adminClient->GetAttestationPolicy(AttestationType.OPEN_ENCLAVE);
+    std::cout << "Current policy for OpenEnclave is " << currentPolicy.Value.Body << std::endl;
     ```
     #### Set unsigned attestation policy (AAD clients only)
@@ Expand Down Expand Up @@
     [cloud_shell_bash]: https://shell.azure.com/bash
     ![Impressions](https://azure-sdk-impressions.azurewebsites.net/api/impressions/azure-sdk-for-cpp%2Fsdk%2Fattestation%2Fazure-security-attestation%2FREADME.png)

-Original file line number
+Diff line change
@@ Expand Up @@
           ValidateTokenIssuer(validationOptions);
         }
-        operator Models::AttestationToken<T>&&() { return std::move(m_token); }
+        /**
+         * @brief Convert the internal attestation token to a public AttestationToken object.
+         */
+        operator Models::AttestationToken<T>&() { return m_token; }
       };
     }}}} // namespace Azure::Security::Attestation::_detail

-Original file line number
+Diff line change
@@ Expand Up @@
           {
             throw OpenSSLException("i2d_X509");
           }
-          if (EVP_DigestUpdate(hash.get(), buf, thumbprintBuffer.size()) != 1)
+          if (EVP_DigestUpdate(hash.get(), thumbprintBuffer.data(), thumbprintBuffer.size()) != 1)
           {
             throw OpenSSLException("EVP_DigestUpdate");
           }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,7 @@ @@
     #include "azure/identity/client_secret_credential.hpp"
     #include <azure/attestation/attestation_client_models.hpp>
     #include <azure/core/test/test_base.hpp>
+    #include <cstdlib>
     #include <gtest/gtest.h>
     #include <string>
     #include <tuple>
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed Live Tests for Attestation SDK. #3366

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

ahsonkhan Mar 30, 2022

Uh oh!

ahsonkhan Mar 30, 2022

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -243,13 +243,62 @@ namespace Azure { namespace Core { namespace Test { @@
               "Test Log from: [ " + m_testContext.GetTestPlaybackRecordingName() + " ] - " + message);
         }
-        // Util for tests getting env vars
-        std::string GetEnv(const std::string& name)
+        /**
+         * @brief Utility function used by tests to retrieve env vars
+         *
+         * @param name Environment variable name to retrieve.
+         *
+         * @return The value of the environment variable retrieved.
+         *
+         * @note If AZURE_TENANT_ID, AZURE_CLIENT_ID, or AZURE_CLIENT_SECRET are not available in the
+         * environment, the AZURE_SERVICE_DIRECTORY environment variable is used to set those values
+         * with the values emitted by the New-TestResources.ps1 script.
+         *
+         * @note The Azure CI pipeline upper cases all environment variables defined in the pipeline.
+         * Since some operating systems have case sensitive environment variables, on debug builds, this
+         * function ensures that the environment variable being retrieved is all upper case.
+         *
+         */
+        std::string GetEnv(std::string const& name)
         {
-          const auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str());
+    #if !defined(NDEBUG)
+          // The azure CI pipeline uppercases all EnvVar values from ci.yml files.
+          // That means that any mixed case strings will not be found when run from the CI
+          // pipeline. Check to make sure that the developer only passed in an upper case environment
+          // variable.
+          {
+            if (name != Azure::Core::_internal::StringExtensions::ToUpper(name))
+            {
+              throw std::runtime_error("All Azure SDK environment variables must be all upper case.");
+            }
+          }
+    #endif
+          auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str());
           if (ret.empty())
           {
+            static const char azurePrefix[] = "AZURE_";
+            if (!m_testContext.IsPlaybackMode() && name.find(azurePrefix) == 0)
+            {
+              std::string serviceDirectory
+                  = Azure::Core::_internal::Environment::GetVariable("AZURE_SERVICE_DIRECTORY");
+              if (serviceDirectory.empty())
+              {
+                throw std::runtime_error(
+                    "Could not find a value for " + name
+                    + " and AZURE_SERVICE_DIRECTORY was not defined. Define either " + name
+                    + " or AZURE_SERVICE_DIRECTORY to resolve.");
+              }
+              // Upper case the serviceName environment variable because all ci.yml environment
+              // variables are upper cased.
+              std::string serviceDirectoryEnvVar
+                  = Azure::Core::_internal::StringExtensions::ToUpper(serviceDirectory);
+              serviceDirectoryEnvVar += name.substr(sizeof(azurePrefix) - 2);
+              ret = Azure::Core::_internal::Environment::GetVariable(serviceDirectoryEnvVar.c_str());
+              if (!ret.empty())
+              {
+                return ret;
+              }
+            }
             throw std::runtime_error("Missing required environment variable: " + name);
           }
@@ Expand All / @@ -261,6 +310,15 @@ namespace Azure { namespace Core { namespace Test { @@
         /**
          * @brief Run before each test.
          *
+         * @param baseRecordingPath - the base recording path to be used for this test. Normally this is
+         * `AZURE_TEST_RECORDING_DIR`.
+         *
+         * For example:
+         *
+         * \code{.cpp}
+         *  Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR);
+         * \endcode
+         *
          */
         void SetUpTestBase(std::string const& baseRecordingPath)
         {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -33,8 +33,10 @@ namespace Azure { namespace Core { namespace _internal { @@
         static bool LocaleInvariantCaseInsensitiveEqual(
             const std::string& lhs,
             const std::string& rhs) noexcept;
-        static std::string const ToLower(const std::string& src) noexcept;
-        static unsigned char ToLower(const unsigned char src) noexcept;
+        static std::string const ToLower(std::string const& src) noexcept;
+        static unsigned char ToLower(unsigned char const src) noexcept;
+        static std::string const ToUpper(std::string const& src) noexcept;
+        static unsigned char ToUpper(unsigned char const src) noexcept;
       };
     }}} // namespace Azure::Core::_internal

-Original file line number
+Diff line change
@@ Expand Up / @@ -65,22 +65,53 @@ const unsigned char LocaleInvariantLowercaseTable[256] = { @@
 xE0, 0xE1, 0xE2, 0xE3, 0xE4, 0xE5, 0xE6, 0xE7, 0xE8, 0xE9, 0xEA, 0xEB, 0xEC, 0xED, 0xEE, 0xEF,
 xF0, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF,
     };
+    const unsigned char LocaleInvariantUppercaseTable[256] = {
+x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F,
+x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F,
+x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3A, 0x3B, 0x3C, 0x3D, 0x3E, 0x3F,
+x40, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x4E, 0x4F,
+x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5A, 0x5B, 0x5C, 0x5D, 0x5E, 0x5F,
+x60, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x4E, 0x4F,
+x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5A, 0x7B, 0x7C, 0x7D, 0x7E, 0x7F,
+x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8A, 0x8B, 0x8C, 0x8D, 0x8E, 0x8F,
+x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, 0x98, 0x99, 0x9A, 0x9B, 0x9C, 0x9D, 0x9E, 0x9F,
+xA0, 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7, 0xA8, 0xA9, 0xAA, 0xAB, 0xAC, 0xAD, 0xAE, 0xAF,
+xB0, 0xB1, 0xB2, 0xB3, 0xB4, 0xB5, 0xB6, 0xB7, 0xB8, 0xB9, 0xBA, 0xBB, 0xBC, 0xBD, 0xBE, 0xBF,
+xC0, 0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7, 0xC8, 0xC9, 0xCA, 0xCB, 0xCC, 0xCD, 0xCE, 0xCF,
+xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, 0xD7, 0xD8, 0xD9, 0xDA, 0xDB, 0xDC, 0xDD, 0xDE, 0xDF,
+xE0, 0xE1, 0xE2, 0xE3, 0xE4, 0xE5, 0xE6, 0xE7, 0xE8, 0xE9, 0xEA, 0xEB, 0xEC, 0xED, 0xEE, 0xEF,
+xF0, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF,
+    };
     } // unnamed namespace
     namespace Azure { namespace Core { namespace _internal {
-      unsigned char StringExtensions::ToLower(const unsigned char symbol) noexcept
+      unsigned char StringExtensions::ToLower(unsigned char const symbol) noexcept
       {
         return LocaleInvariantLowercaseTable[symbol];
       }
       std::string const StringExtensions::ToLower(const std::string& src) noexcept
       {
         auto result = std::string(src);
-        for (auto i = result.begin(); i < result.end(); i++)
-        {
-          *i = ToLower(static_cast<unsigned char>(*i));
-        }
+        std::transform(result.begin(), result.end(), result.begin(), [](char const ch) {
+          return StringExtensions::ToLower(ch);
+        });
+        return result;
+      }
+      unsigned char StringExtensions::ToUpper(unsigned char const symbol) noexcept
+      {
+        return LocaleInvariantUppercaseTable[symbol];
+      }
+      std::string const StringExtensions::ToUpper(const std::string& src) noexcept
+      {
+        auto result = std::string(src);
+        std::transform(result.begin(), result.end(), result.begin(), [](char const ch) {
+          return StringExtensions::ToUpper(ch);
+        });
         return result;
       }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -20,6 +20,24 @@ TEST(String, invariantCompare) @@
       EXPECT_FALSE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("ABC", "abcd"));
     }
+    TEST(String, toLowerC)
+    {
+      using Azure::Core::_internal::StringExtensions;
+      for (unsigned char ch = 0; ch < 255; ch += 1)
+      {
+        EXPECT_TRUE(StringExtensions::ToLower(ch) == std::tolower(ch));
+      }
+    }
+    TEST(String, toUpperC)
+    {
+      using Azure::Core::_internal::StringExtensions;
+      for (unsigned char ch = 0; ch < 255; ch += 1)
+      {
+        EXPECT_TRUE(StringExtensions::ToUpper(ch) == std::toupper(ch));
+      }
+    }
     TEST(String, toLower)
     {
       using Azure::Core::_internal::StringExtensions;
@@ Expand All / @@ -29,9 +47,33 @@ TEST(String, toLower) @@
       EXPECT_TRUE(StringExtensions::ToLower("AA") == "aa");
       EXPECT_TRUE(StringExtensions::ToLower("aA") == "aa");
       EXPECT_TRUE(StringExtensions::ToLower("ABC") == "abc");
+      EXPECT_TRUE(
+          StringExtensions::ToLower("abcdefghijklmnopqrstuvwxyz") == "abcdefghijklmnopqrstuvwxyz");
+      EXPECT_TRUE(
+          StringExtensions::ToLower("ABCDEFGHIJKLMNOPQRSTUVWXYZ") == "abcdefghijklmnopqrstuvwxyz");
       EXPECT_TRUE(StringExtensions::ToLower("ABC-1-,!@#$%^&*()_+=ABC") == "abc-1-,!@#$%^&*()_+=abc");
       EXPECT_FALSE(StringExtensions::ToLower("") == "a");
       EXPECT_FALSE(StringExtensions::ToLower("a") == "");
       EXPECT_FALSE(StringExtensions::ToLower("a") == "aA");
       EXPECT_FALSE(StringExtensions::ToLower("abc") == "abcd");
     }
+    TEST(String, toUpper)
+    {
+      using Azure::Core::_internal::StringExtensions;
+      EXPECT_TRUE(StringExtensions::ToUpper("") == "");
+      EXPECT_TRUE(StringExtensions::ToUpper("a") == "A");
+      EXPECT_TRUE(StringExtensions::ToUpper("A") == "A");
+      EXPECT_TRUE(StringExtensions::ToUpper("AA") == "AA");
+      EXPECT_TRUE(StringExtensions::ToUpper("aA") == "AA");
+      EXPECT_TRUE(
+          StringExtensions::ToUpper("ABCDEFGHIJKLMNOPQRSTUVWXYZ") == "ABCDEFGHIJKLMNOPQRSTUVWXYZ");
+      EXPECT_TRUE(StringExtensions::ToUpper("ABC") == "ABC");
+      EXPECT_TRUE(
+          StringExtensions::ToUpper("ABCDEFGHIJKLMNOPQRSTUVWXYZ") == "ABCDEFGHIJKLMNOPQRSTUVWXYZ");
+      EXPECT_TRUE(StringExtensions::ToUpper("ABC-1-,!@#$%^&*()_+=ABC") == "ABC-1-,!@#$%^&*()_+=ABC");
+      EXPECT_FALSE(StringExtensions::ToUpper("") == "A");
+      EXPECT_FALSE(StringExtensions::ToUpper("a") == "");
+      EXPECT_FALSE(StringExtensions::ToUpper("a") == "aA");
+      EXPECT_FALSE(StringExtensions::ToUpper("abc") == "abcd");
+    }

Fixed Live Tests for Attestation SDK. #3366

Uh oh!

Fixed Live Tests for Attestation SDK. #3366

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

ahsonkhan Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

ahsonkhan Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!