Azure-Samples / microsoft-azure-attestation

Microsoft Azure Attestation is a solution for attesting Trusted Execution Environments (TEEs)
MIT License
30 stars 17 forks source link

Why do examples use plain hex representations instead of Base64Url? #4

Closed dimakuv closed 3 years ago

dimakuv commented 4 years ago

The current two examples use plain hex representations of the SGX quote and EnclaveHeldData:

However, the MAA defines the Attestation Request's JSON fields as Base64Url encoded:

How can these examples work if they submit Attestation Requests with plain-hex values, even though MAA expects Base64Url-encoded values?

dimakuv commented 4 years ago

Ok, figured it out.

The "ValidateQuotes" utility program actually modifies the values in the JSON file: https://github.com/Azure-Samples/microsoft-azure-attestation/blob/b51c146d5246683abfc6253b4a82e5d0d52c28fb/intel.sdk.attest.sample/validatequotes.core/EnclaveInfo.cs#L25

And only then it sends the modified values (now encoded in Base64Url) to the MAA service.

Why was it implemented this way? Seems like an unnecessary step.

olkroshk commented 4 years ago

We encode the quote and data in https://github.com/Azure-Samples/microsoft-azure-attestation/blob/b51c146d5246683abfc6253b4a82e5d0d52c28fb/intel.sdk.attest.sample/validatequotes.core/EnclaveInfo.cs#L21-L29

dimakuv commented 4 years ago

I would like to re-use the validatequotes.core program for Graphene. This particular gimmick doesn't make sense to me. Could we do the following:

Or am I missing some important reason to have this intermediate conversion?

olkroshk commented 4 years ago

Good point. I don't see a reason why you cannot save QuoteHex and EnclaveHeldDataHex in the JSON file as base64Url and remove the step from the validatequotes.core application.

The purpose of the samples is to demo basics of the quote generation, and validation with MAA service.

@gkostal could shed some light on whether there was a particular reason to separate encoding from writing to JSON. My guess would be that it was easier to verify genquote_host output.

dimakuv commented 4 years ago

Given that we want to add support for MAA to Graphene, it would be great to agree on a particular format. We (Graphene developers) can go either way, but seems like Base64Url-encoded values in the JSON file is more straight-forward.

You can also check my draft PR for MAA support in Graphene: https://github.com/oscarlab/graphene/pull/1793.

olkroshk commented 4 years ago

Looking at https://github.com/oscarlab/graphene/pull/1793/files#diff-e85fb2d6fe3934933684754ee6a71705R32 gives me an idea about why encoding was implemented in the validatequotes.core.

Anyhow, it makes sense to write encoded values to JSON. I'd like to clarify some details before making any changes. Is the plan to publish the Graphene sample into this repo? If yes, how do you want to lay out the samples? In other words, do you plan to use this app https://github.com/Azure-Samples/microsoft-azure-attestation/tree/master/intel.sdk.attest.sample/validatequotes.core or do you prefer to create a new folder similar to https://github.com/Azure-Samples/microsoft-azure-attestation/tree/master/intel.sdk.attest.sample or https://github.com/Azure-Samples/microsoft-azure-attestation/tree/master/sgx.attest.sample?

gkostal commented 3 years ago

@dimakuv - the reason that the quote is hex formatted in the internal enclave info JSON file is simply because I was interested in the human readability of the enclave attribute fields in JSON file. This file is an implementation detail for this sample since it separates the generation of a quote in one application (must be written in C/C++ due to OE SDK integration) and the verification of the quote with MAA (which originally was much simpler to write in C# due to the need to perform AAD authentication).

The MAA service no longer requires AAD authentication for making attest API calls. Hence, if I were rewriting this sample app today, I'd combine both the genquote and validatequotes applications into one written in C/C++.

That said, I don't have any issue with switching the format of the strings in the enclave info JSON files to be base64url, so feel free to submit a PR with these changes if you like.

Closing this issue now since I believe the questions are all answered now.