From 52f9437adb36dfe6affc04e9e1032c85eb6bc30b Mon Sep 17 00:00:00 2001 From: Peng Li <86324823+penglimsft@users.noreply.github.com> Date: Mon, 19 Sep 2022 16:01:44 -0700 Subject: [PATCH] Update `AttestationClient::AttestTpm` API to match existing `AttestOpenEnclave` and `AttestSgxmEnclave` (#3928) * Fix broken link and typo in contributing.md * Use vector for attest instead of strings * remove options * fix comments * update release version * remove versionig * revert changelog * add the change * update comment * Update sdk/attestation/azure-security-attestation/CHANGELOG.md Co-authored-by: Larry Osterman * fix formatting * address pr comment * fix formating * update a comment * remove the attest tpm comment Co-authored-by: Peng Li Co-authored-by: Larry Osterman --- CONTRIBUTING.md | 6 +++--- .../azure-security-attestation/CHANGELOG.md | 5 +++++ .../azure/attestation/attestation_client.hpp | 20 +++++++------------ .../attestation/attestation_client_models.hpp | 5 +++-- .../attestation_client_options.hpp | 6 ------ .../src/attestation_client.cpp | 7 ++++--- .../attestation_deserializers_private.cpp | 12 +++++------ .../attestation_deserializers_private.hpp | 7 ++++--- .../test/ut/tpmattestation_test.cpp | 3 ++- 9 files changed, 34 insertions(+), 37 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6eab9f5cab..e35adf304c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,7 +15,7 @@ Thank you for your interest in contributing to Azure SDK for C++. - **DO** submit all code changes via pull requests (PRs) rather than through a direct commit. PRs will be reviewed and potentially merged by the repo maintainers after a peer review that includes at least one maintainer. - **DO** review your own PR to make sure there aren't any unintended changes or commits before submitting it. - **DO NOT** submit "work in progress" PRs. A PR should only be submitted when it is considered ready for review and subsequent merging by the contributor. - - If the change is work-in-progress or an experiment, **DO** start if off as a temporary draft PR. + - If the change is work-in-progress or an experiment, **DO** start it off as a temporary draft PR. - **DO** give PRs short-but-descriptive names (e.g. "Improve code coverage for Azure.Core by 10%", not "Fix #1234") and add a description which explains why the change is being made. - **DO** refer to any relevant issues, and include [keywords](https://docs.github.com/articles/closing-issues-via-commit-messages/) that automatically close issues when the PR is merged. - **DO** tag any users that should know about and/or review the change. @@ -48,13 +48,13 @@ Codespaces is new technology that allows you to use a container as your developm ### GitHub Codespaces 1. From the Azure SDK GitHub repo, click on the "Code -> Open with Codespaces" button. -1. Open a Terminal. The development environment will be ready for you. Continue to [Building and Testing](https://github.com/Azure/azure-sdk-for-cpp/blob/main/CONTRIBUTING.md#building-and-testing). +1. Open a Terminal. The development environment will be ready for you. Continue to [Building the project](#building-the-project). ### VS Code Codespaces 1. Install the [VS Code Remote Extension Pack](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.vscode-remote-extensionpack) 1. When you open the Azure SDK for C++ repo in VS Code, it will prompt you to open the project in the Dev Container. If it does not prompt you, then hit CTRL+P, and select "Remote-Containers: Open Folder in Container..." -1. Open a Terminal. The development environment will be ready for you. Continue to [Building and Testing](https://github.com/Azure/azure-sdk-for-cpp/blob/main/CONTRIBUTING.md#building-and-testing). +1. Open a Terminal. The development environment will be ready for you. Continue to [Building the project](#building-the-project). ## Full Local Setup diff --git a/sdk/attestation/azure-security-attestation/CHANGELOG.md b/sdk/attestation/azure-security-attestation/CHANGELOG.md index b032e5a8d2..e5c537269e 100644 --- a/sdk/attestation/azure-security-attestation/CHANGELOG.md +++ b/sdk/attestation/azure-security-attestation/CHANGELOG.md @@ -6,6 +6,11 @@ ### Breaking Changes +- Changed `AttestationClient::AttestTpm` to match `AttestOpenEnclave` and `AttestSgxEnclave` + - Added `std::vector` dataToAttest parameter. + - Removed `PayLoad` in `TpmAttestationOptions` + - Changed `TpmResult` in `TpmAttestationResult` to type `std::vector` + ### Bugs Fixed ### Other Changes diff --git a/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client.hpp b/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client.hpp index 4aa60061ed..d7c44d4941 100644 --- a/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client.hpp +++ b/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client.hpp @@ -223,26 +223,20 @@ namespace Azure { namespace Security { namespace Attestation { Azure::Core::Context const& context = Azure::Core::Context{}) const; /** - * @brief Perform a single leg - * - * Processes attestation evidence from a VBS enclave, producing an attestation result. - * + * @brief Sends TPM-based attestation data to the service. * The TPM attestation protocol is defined * [here](https://docs.microsoft.com/azure/attestation/virtualization-based-security-protocol') * - * Unlike OpenEnclave reports and SGX enclave quotes, TPM attestation is implemented using - * JSON encoded strings. * - * The client formats a string serialized JSON request to the - * service, which responds with a JSON response. The serialized JSON object exchange continues - * until the service responds with a JSON string with a property named {@code "report"}, whose - * value will be an attestation result token. + * @param dataToAttest - Attestation request data. + * @param options - Options to the attestation request. + * @param context - Context for the operation. * - * @param options sent to the service for Trusted Platform Module (TPM) attestation. - * @return attestation response for Trusted Platform Module (TPM) attestation. + * @return Response - The result of the attestation operation */ Response AttestTpm( - AttestTpmOptions const& options, + std::vector const& dataToAttest, + AttestTpmOptions const& options = AttestTpmOptions{}, Azure::Core::Context const& context = Azure::Core::Context{}) const; private: diff --git a/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_models.hpp b/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_models.hpp index 2fb15bcc51..c4db47e2ae 100644 --- a/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_models.hpp +++ b/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_models.hpp @@ -448,12 +448,13 @@ namespace Azure { namespace Security { namespace Attestation { namespace Models */ struct TpmAttestationResult final { - /** @brief The JSON encoded value returned from TPM attestation. + /** @brief Attestation response data. + * * The TPM attestation protocol is defined * [here](https://docs.microsoft.com/azure/attestation/virtualization-based-security-protocol') * */ - std::string TpmResult; + std::vector TpmResult; }; /** diff --git a/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_options.hpp b/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_options.hpp index a94cf6092e..b9f1a2450a 100644 --- a/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_options.hpp +++ b/sdk/attestation/azure-security-attestation/inc/azure/attestation/attestation_client_options.hpp @@ -261,12 +261,6 @@ namespace Azure { namespace Security { namespace Attestation { */ struct AttestTpmOptions final { - /** - * @brief JSON Data to send to the attestation service for TPM attestation. - * @details The TPM attestation protocol is defined - * [here](https://docs.microsoft.com/azure/attestation/virtualization-based-security-protocol') - */ - std::string Payload; }; /** @brief The AttestationSigningKey represents a tuple of asymmetric private cryptographic key diff --git a/sdk/attestation/azure-security-attestation/src/attestation_client.cpp b/sdk/attestation/azure-security-attestation/src/attestation_client.cpp index 1e0077adc3..8786466e1d 100644 --- a/sdk/attestation/azure-security-attestation/src/attestation_client.cpp +++ b/sdk/attestation/azure-security-attestation/src/attestation_client.cpp @@ -196,13 +196,14 @@ Azure::Response> AttestationClient::AttestOp } Azure::Response AttestationClient::AttestTpm( - AttestTpmOptions const& attestTpmOptions, + std::vector const& dataToAttest, + AttestTpmOptions const&, Azure::Core::Context const& context) const { auto tracingContext(m_tracingFactory.CreateTracingContext("AttestTpm", context)); try { - std::string jsonToSend = TpmDataSerializer::Serialize(attestTpmOptions.Payload); + std::string jsonToSend = TpmDataSerializer::Serialize(dataToAttest); auto encodedVector = std::vector(jsonToSend.begin(), jsonToSend.end()); Azure::Core::IO::MemoryBodyStream stream(encodedVector); @@ -212,7 +213,7 @@ Azure::Response AttestationClient::AttestTpm( // Send the request to the service. auto response = AttestationCommonRequest::SendRequest(*m_pipeline, request, tracingContext.Context); - std::string returnedBody(TpmDataSerializer::Deserialize(response)); + std::vector returnedBody{TpmDataSerializer::Deserialize(response)}; return Response(TpmAttestationResult{returnedBody}, std::move(response)); } catch (std::runtime_error const& ex) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.cpp b/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.cpp index 580fcdf503..684cec9432 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.cpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.cpp @@ -397,21 +397,21 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail returnValue.CertificateThumbprint, jsonResult, "x-ms-certificate-thumbprint"); return returnValue; } - std::string TpmDataSerializer::Serialize(std::string const& tpmData) + std::string TpmDataSerializer::Serialize(std::vector const& tpmData) { Azure::Core::Json::_internal::json jsonData; - jsonData["data"] = Azure::Core::_internal::Base64Url::Base64UrlEncode( - std::vector(tpmData.begin(), tpmData.end())); + jsonData["data"] = Azure::Core::_internal::Base64Url::Base64UrlEncode(tpmData); return jsonData.dump(); } - std::string TpmDataSerializer::Deserialize(Azure::Core::Json::_internal::json const& jsonData) + std::vector TpmDataSerializer::Deserialize( + Azure::Core::Json::_internal::json const& jsonData) { std::vector returnValue; JsonOptional::SetIfExists>( returnValue, jsonData, "data", Azure::Core::_internal::Base64Url::Base64UrlDecode); - return std::string(returnValue.begin(), returnValue.end()); + return returnValue; } - std::string TpmDataSerializer::Deserialize( + std::vector TpmDataSerializer::Deserialize( std::unique_ptr const& response) { return TpmDataSerializer::Deserialize( diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.hpp index 6cc7d1279b..9b72f35fac 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_deserializers_private.hpp @@ -136,9 +136,10 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail struct TpmDataSerializer { - static std::string Serialize(std::string const& tpmData); - static std::string Deserialize(Azure::Core::Json::_internal::json const& jsonData); - static std::string Deserialize(std::unique_ptr const& response); + static std::string Serialize(std::vector const& tpmData); + static std::vector Deserialize(Azure::Core::Json::_internal::json const& jsonData); + static std::vector Deserialize( + std::unique_ptr const& response); }; }}}} // namespace Azure::Security::Attestation::_detail diff --git a/sdk/attestation/azure-security-attestation/test/ut/tpmattestation_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/tpmattestation_test.cpp index dad0c78f55..2d0737e12c 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/tpmattestation_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/tpmattestation_test.cpp @@ -118,7 +118,8 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { { auto client(CreateClient(InstanceType::AAD)); - auto response(client.AttestTpm(AttestTpmOptions{R"({"payload": { "type": "aikcert" } })"})); + std::string tpmQuote = R"({"payload": { "type": "aikcert" } })"; + auto response(client.AttestTpm(std::vector(tpmQuote.begin(), tpmQuote.end()))); Azure::Core::Json::_internal::json parsedResponse( Azure::Core::Json::_internal::json::parse(response.Value.TpmResult));