Skip to content

Conversation

@veenasai2
Copy link
Contributor

@veenasai2 veenasai2 commented Sep 14, 2021

Signed-off-by: Veena Saini [email protected]

Description of the changes

This PR provides a reference implementation to show how gramine attestation (DCAP) samples works inside AKS cluster. We have created two docker images for ra-tls-secret-prov server and ra-tls-secret-prov client. Both images are deployed as part of AKS confidential compute cluster and both quote generation and quote verification are successful inside AKS cluster.

For client deployment inside AKS cluster, we have gsc/examples/aks-attestation/aks-secret-prov-client-deployment.yaml and for server deployment gsc/examples/aks-attestation/aks-secret-prov-server-deployment.yaml file.

For more details, we have created a readme file.

This PR is an updated version of gramineproject/graphene#2473. Once this is merged, I will delete 2473 PR.

How to test this PR?

Please follow gsc/examples/aks-attestation/gramine_attestation_inside_aks_readme.md


This change is Reviewable

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 32 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @veenasai2)


-- commits, line 3 at r1:
This should have a better name like Add example of AKS attestation and secret provisioning


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 24 at r1 (raw file):

# Build environment of this Dockerfile should point to the root of Gramine directory

# Before copy do "make clean && make secret_prov_min_client" in CI-Examples/ra-tls-secret-prov directory

This step should be in the README, not here


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 26 at r1 (raw file):

# Before copy do "make clean && make secret_prov_min_client" in CI-Examples/ra-tls-secret-prov directory

COPY CI-Examples/ra-tls-secret-prov /gramine/Examples/ra-tls-secret-prov

Why do you need this complex dir /gramine/Examples/ra-tls-secret-prov? All Gramine binaries/utilities/libraries (including RA-TLS, Secret Provisioning, MbedTLS) are installed correctly in your Ubuntu OS inside this Docker image. So you don't need any references to gramine/Examples.

Please replace /gramine/Examples/ra-tls-secret-prov to simply /ra-tls-secret-prov everywhere in this PR.


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r1 (raw file):

WORKDIR /gramine/Examples/ra-tls-secret-prov

ENV PATH = "${PATH}:/gramine/Examples/ra-tls-secret-prov"

Why is this needed? With the latest Gramine version, we have all libraries and utilities installed in the correct dir, and there is no need to adjust PATH


examples/aks-attestation/aks-secret-prov-client.manifest, line 4 at r1 (raw file):


# Secret Provisioning library (client-side) is preloaded
loader.env.LD_PRELOAD = "libs/libsecret_prov_attest.so"

With the latest Gramine, the directory is different... It is one of the common dirs on Ubuntu.


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 15 at r1 (raw file):

# Here, the version of az-dcap-client should be in sync with the
# az-dcap-client version used for quote generation.
# User can replace the below package with the latest package.

Why this strange char limit per line? Please re-wrap to 80 chars.


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 30 at r1 (raw file):

# Build environment of this Dockerfile should point to the root of Gramine directory

# Before copy do "make clean && make dcap" in CI-Examples/ra-tls-secret-prov directory

This step should be in the README, not here


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 32 at r1 (raw file):

# Before copy do "make clean && make dcap" in CI-Examples/ra-tls-secret-prov directory

COPY CI-Examples/ra-tls-secret-prov /gramine/Examples/ra-tls-secret-prov

ditto (Please replace /gramine/Examples/ra-tls-secret-prov to simply /ra-tls-secret-prov everywhere in this PR.)


examples/aks-attestation/README.md, line 5 at r1 (raw file):

This guide demonstrates how Gramine DCAP attestation quote can be generated and verified from
within an AKS cluster. Here, we provide an end-to-end example to help Cloud Solution Providers
integrate gramine’s RA-TLS attestation and secret provisioning feature with a confidential compute

Please upper-case all gramine words in your README


examples/aks-attestation/README.md, line 11 at r1 (raw file):

deployed to the AKS cluster.

## Create client and server images for gramine attestation samples

Please remove for gramine attestation samples -- it's obvious from the context.

I suggest ## Preparing client and server images


examples/aks-attestation/README.md, line 14 at r1 (raw file):

This demonstration is created for ``gramine/CI-Examples/ra-tls-secret-prov`` sample.
In order to create the below two images, user needs to download core [gramine repository](https://github.com/gramineproject/gramine).

gramine repository -> Gramine repository


examples/aks-attestation/README.md, line 16 at r1 (raw file):

In order to create the below two images, user needs to download core [gramine repository](https://github.com/gramineproject/gramine).

### Steps to create ra-tls-secret-prov server image for AKS

Please remove Steps to create ... -- it's obvious from the context.

I suggest ### Creating server image


examples/aks-attestation/README.md, line 18 at r1 (raw file):

### Steps to create ra-tls-secret-prov server image for AKS

1. Prepare server certificate

Please add : at the end of each step title


examples/aks-attestation/README.md, line 19 at r1 (raw file):

1. Prepare server certificate
    - Create server certificate signed by your trusted root CA. Ensure Common Name

Common Name -> "Common Name"


examples/aks-attestation/README.md, line 20 at r1 (raw file):

1. Prepare server certificate
    - Create server certificate signed by your trusted root CA. Ensure Common Name
      field in the server certificate corresponds to `<AKS-DNS-NAME>` used in STEP 5.

STEP 5 -> step 5


examples/aks-attestation/README.md, line 22 at r1 (raw file):

      field in the server certificate corresponds to `<AKS-DNS-NAME>` used in STEP 5.
    - Put trusted root CA certificate, server certificate, and server key in
      gramine/CI-Examples/ra-tls-secret-prov/certs directory with existing naming convention.

Add backticks around the path


examples/aks-attestation/README.md, line 24 at r1 (raw file):

      gramine/CI-Examples/ra-tls-secret-prov/certs directory with existing naming convention.

2. Make sure Gramine is built with -Ddcap=enabled option `meson setup ... -Ddcap=enabled`

You can just remove -Ddcap=enabled option phrase -- it duplicates the command itself


examples/aks-attestation/README.md, line 32 at r1 (raw file):

    $ docker build -t <aks-secret-prov-server-img> \
        -f <path-to-gsc>/examples/aks-attestation/aks-secret-prov-server.dockerfile .

Please remove this empty line


examples/aks-attestation/README.md, line 41 at r1 (raw file):

        <dockerhubusername>/<aks-secret-prov-server-img>
    $ docker push <dockerhubusername>/<aks-secret-prov-server-img>

Please remove this empty line


examples/aks-attestation/README.md, line 43 at r1 (raw file):

    ```
5. Deploy `<aks-secret-prov-server-img>` in AKS confidential compute cluster

Please add an empty line before step 5


examples/aks-attestation/README.md, line 45 at r1 (raw file):

5. Deploy `<aks-secret-prov-server-img>` in AKS confidential compute cluster
    - Reference deployment file:
        gsc/examples/aks-attestation/aks-secret-prov-server-deployment.yaml

Please add backticks around this path


examples/aks-attestation/README.md, line 48 at r1 (raw file):

NOTE:  Server can be deployed at a non-confidential compute node as well. However, in that case
       QVE-based dcap verification will fail.

What's the point of this note then? If the server will just fail?


examples/aks-attestation/README.md, line 50 at r1 (raw file):

       QVE-based dcap verification will fail.

### Steps to create ra-tls-secret-prov client image for AKS

Please remove Steps to create ... -- it's obvious from the context.

I suggest ### Creating client image


examples/aks-attestation/README.md, line 52 at r1 (raw file):

### Steps to create ra-tls-secret-prov client image for AKS

1. Make sure Gramine is built with -Ddcap=enabled option `meson setup ... -Ddcap=enabled`

ditto (please fix this section similarly to my comments for the other section)


examples/aks-attestation/README.md, line 89 at r1 (raw file):

NOTE: We recommend deploying gsc images on Ubuntu with Linux kernel version 5.11 or higher.
For kernel version lower than 5.11, please uncomment line9 at gsc/templates/apploader.template.

This is wrong. The user should change their config.yaml file for that...


examples/aks-attestation/README.md, line 91 at r1 (raw file):

For kernel version lower than 5.11, please uncomment line9 at gsc/templates/apploader.template.

## Deploy both client and server images inside AKS confidential compute cluster

I suggest ## Deploying client and server images inside AKS Confidential Compute cluster


examples/aks-attestation/README.md, line 110 at r1 (raw file):

 AKS cluster.

**Deployment**<br>

What's the deal with this? Why not simply ### Deployment


examples/aks-attestation/README.md, line 125 at r1 (raw file):

provisioned from the server to the client container.

## Steps to verify successful quote generation and quote verification using logs

## Checking SGX quote generation and verification


examples/aks-attestation/README.md, line 127 at r1 (raw file):

## Steps to verify successful quote generation and quote verification using logs

Verify the client job is completed

Please put : at the ends of lines, where needed


examples/aks-attestation/README.md, line 137 at r1 (raw file):

Expected Output

Simply ### Expected output


examples/aks-attestation/README.md, line 139 at r1 (raw file):

**Expected Output**<br>

--- Received secret = 'XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX'

Put this into backticks, currently looks ugly: https://github.com/veenasai2/gsc/tree/veenasai/gramine-aks-attestation/examples/aks-attestation


templates/apploader.template, line 9 at r1 (raw file):


# Uncomment below for kernel version lower than 5.11
# ln -s /dev/sgx/enclave /dev/sgx_enclave

Why does it matter? The SGX driver used depends on the config.yaml configuration file.

This change should be reverted.

@veenasai2 veenasai2 changed the title Gramine Attestation Inside AKS Add example of AKS attestation and secret provisioning Sep 24, 2021
Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 7 files reviewed, 32 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


-- commits, line 3 at r1:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This should have a better name like Add example of AKS attestation and secret provisioning

done, thanks


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 24 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This step should be in the README, not here

done, thanks.


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 26 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why do you need this complex dir /gramine/Examples/ra-tls-secret-prov? All Gramine binaries/utilities/libraries (including RA-TLS, Secret Provisioning, MbedTLS) are installed correctly in your Ubuntu OS inside this Docker image. So you don't need any references to gramine/Examples.

Please replace /gramine/Examples/ra-tls-secret-prov to simply /ra-tls-secret-prov everywhere in this PR.

Done, thanks


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why is this needed? With the latest Gramine version, we have all libraries and utilities installed in the correct dir, and there is no need to adjust PATH

Done, thanks


examples/aks-attestation/aks-secret-prov-client.manifest, line 4 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

With the latest Gramine, the directory is different... It is one of the common dirs on Ubuntu.

Done, thanks.


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 15 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why this strange char limit per line? Please re-wrap to 80 chars.

Done, thanks


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 30 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This step should be in the README, not here

Done, thanks


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 32 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

ditto (Please replace /gramine/Examples/ra-tls-secret-prov to simply /ra-tls-secret-prov everywhere in this PR.)

done, thanks


examples/aks-attestation/README.md, line 5 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please upper-case all gramine words in your README

Done, thanks.


examples/aks-attestation/README.md, line 11 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove for gramine attestation samples -- it's obvious from the context.

I suggest ## Preparing client and server images

Done, thanks


examples/aks-attestation/README.md, line 14 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

gramine repository -> Gramine repository

Done, thanks.


examples/aks-attestation/README.md, line 16 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove Steps to create ... -- it's obvious from the context.

I suggest ### Creating server image

Done, thanks


examples/aks-attestation/README.md, line 18 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please add : at the end of each step title

Done, thanks.


examples/aks-attestation/README.md, line 19 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Common Name -> "Common Name"

Done, thanks.


examples/aks-attestation/README.md, line 20 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

STEP 5 -> step 5

Done, thanks.


examples/aks-attestation/README.md, line 22 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Add backticks around the path

Done, thanks.


examples/aks-attestation/README.md, line 24 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

You can just remove -Ddcap=enabled option phrase -- it duplicates the command itself

Done, thanks.


examples/aks-attestation/README.md, line 32 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove this empty line

Done, thanks


examples/aks-attestation/README.md, line 41 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove this empty line

Done, thanks


examples/aks-attestation/README.md, line 43 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please add an empty line before step 5

Done, thanks


examples/aks-attestation/README.md, line 45 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please add backticks around this path

Done, thanks.


examples/aks-attestation/README.md, line 48 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

What's the point of this note then? If the server will just fail?

The non-qve based verification will pass. QVE-based verification will fail, if we don't use confidential compute node for server.


examples/aks-attestation/README.md, line 50 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove Steps to create ... -- it's obvious from the context.

I suggest ### Creating client image

Done, thanks


examples/aks-attestation/README.md, line 52 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

ditto (please fix this section similarly to my comments for the other section)

Done, thanks


examples/aks-attestation/README.md, line 89 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This is wrong. The user should change their config.yaml file for that...

Yeah correct. But what if the user uses "in-kernel driver option" in config.yaml file and the in-kernel driver is lower than 5.11. In that case we have to explicitely set the symbolic links, right?


examples/aks-attestation/README.md, line 91 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I suggest ## Deploying client and server images inside AKS Confidential Compute cluster

Done, thanks.


examples/aks-attestation/README.md, line 110 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

What's the deal with this? Why not simply ### Deployment

Done, thanks.


examples/aks-attestation/README.md, line 125 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

## Checking SGX quote generation and verification

Done, thanks.


examples/aks-attestation/README.md, line 127 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please put : at the ends of lines, where needed

Done, thanks.


examples/aks-attestation/README.md, line 137 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Simply ### Expected output

Done, thanks.


examples/aks-attestation/README.md, line 139 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Put this into backticks, currently looks ugly: https://github.com/veenasai2/gsc/tree/veenasai/gramine-aks-attestation/examples/aks-attestation

Done, thanks.


templates/apploader.template, line 9 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why does it matter? The SGX driver used depends on the config.yaml configuration file.

This change should be reverted.

Done, thanks.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 7 files at r2, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @veenasai2)


-- commits, line 3 at r1:

Previously, veenasai2 wrote…

done, thanks

Not done, but we'll change the name during the final rebase.


examples/aks-attestation/README.md, line 48 at r1 (raw file):

Previously, veenasai2 wrote…

The non-qve based verification will pass. QVE-based verification will fail, if we don't use confidential compute node for server.

Ah, I understand the point now. The server doesn't really need to be run on the SGX-enabled node because it can use the non-QVE (non-Quote-Verification-Enclave) flow. But to use the QVE flow, it must run on the SGX-enabled node.

Could you add a link to some Microsoft Azure doc about these two flows? I can't find any... (If you also can't find any doc, then I'll resolve this comment. Surprisingly hard to find any good documentation on QvE.)


examples/aks-attestation/README.md, line 89 at r1 (raw file):

Previously, veenasai2 wrote…

Yeah correct. But what if the user uses "in-kernel driver option" in config.yaml file and the in-kernel driver is lower than 5.11. In that case we have to explicitely set the symbolic links, right?

But why would the user specify wrong driver option in config.yaml? I am missing something here.


examples/aks-attestation/README.md, line 48 at r2 (raw file):

        `gsc/examples/aks-attestation/aks-secret-prov-server-deployment.yaml`

NOTE:  Server can be deployed at a non-confidential compute node as well. However, in that case

Please make this NOTE bold or italic.


examples/aks-attestation/README.md, line 69 at r2 (raw file):

      inside `gsc/examples/aks-attestation/aks-secret-prov-client.manifest` file.

4. Create gsc image for ra-tls-secret-prov min client:

gsc -> GSC


examples/aks-attestation/README.md, line 91 at r2 (raw file):

        `gsc/examples/aks-attestation/aks-secret-prov-client-deployment.yaml`

NOTE: We recommend deploying gsc images on Ubuntu with Linux kernel version 5.11 or higher.

Please make this NOTE bold or italic.

Also, gsc -> GSC.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 6 of 7 files reviewed, 6 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


-- commits, line 3 at r1:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Not done, but we'll change the name during the final rebase.

Looks like directly updating at github does not show up.


examples/aks-attestation/README.md, line 48 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Ah, I understand the point now. The server doesn't really need to be run on the SGX-enabled node because it can use the non-QVE (non-Quote-Verification-Enclave) flow. But to use the QVE flow, it must run on the SGX-enabled node.

Could you add a link to some Microsoft Azure doc about these two flows? I can't find any... (If you also can't find any doc, then I'll resolve this comment. Surprisingly hard to find any good documentation on QvE.)

Sure, I will search. Thanks.


examples/aks-attestation/README.md, line 89 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

But why would the user specify wrong driver option in config.yaml? I am missing something here.

This happened with me. I ran gsc with default (i.e. in-kernel driver) config.yaml option. That will assume, I have in-kernel driver on my host system. This in-kernel driver can be 5.11 or lower. When I ran the same gsc image as part of AKS cluster, there it failed with the symbolic link error. I don't know which in-kernel driver is installed on Azure host system where the container is running, when I create an AKS cluster. So only for the AKS environment, I thought providing symbolic link as part of apploader.sh would be good, until they upgrade their in-kernel driver.


examples/aks-attestation/README.md, line 48 at r2 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please make this NOTE bold or italic.

done, thanks


examples/aks-attestation/README.md, line 69 at r2 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

gsc -> GSC

done, thanks


examples/aks-attestation/README.md, line 91 at r2 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please make this NOTE bold or italic.

Also, gsc -> GSC.

done, thanks

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 6 of 7 files reviewed, 6 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


examples/aks-attestation/README.md, line 48 at r1 (raw file):

Previously, veenasai2 wrote…

Sure, I will search. Thanks.

Just to correct my previous statement, in gramine we are doing "non-qve based" dcap verification only. As we are passing /p_qve_report_info=/NULL in sgx_qv_verify_quote() . So, I think, I can remove this comment. Also, in server deployment file there should not be any need for epc reservation. I will check again by running the server in non-confidential compute node and get back soon. Thanks.

@veenasai2
Copy link
Contributor Author


examples/aks-attestation/README.md, line 48 at r1 (raw file):

Previously, veenasai2 wrote…

Just to correct my previous statement, in gramine we are doing "non-qve based" dcap verification only. As we are passing /p_qve_report_info=/NULL in sgx_qv_verify_quote() . So, I think, I can remove this comment. Also, in server deployment file there should not be any need for epc reservation. I will check again by running the server in non-confidential compute node and get back soon. Thanks.

I just ran the server image as part of AKS cluster without epc reservation. The verification succeeds and the client is receiving the secret. But the server container prints error message

"[get_driver_type /home/sgx/jenkins/linux-ubuntuServer-release-build-trunk-214/build_target/PROD/label/Builder-UbuntuSrv18/label_exp/ubuntu64/linux-trunk-opensource/psw/urts/linux/edmm_utility.cpp:111] Failed to open Intel SGX device."

Hence, we need to use the confidential compute node for server as well. Thanks.

dimakuv
dimakuv previously approved these changes Sep 27, 2021
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @veenasai2)

a discussion (no related file):
I'm approving this PR (we'll fix the commit message during final rebase), but please double-check that this latest version works correctly on AKS.



-- commits, line 3 at r1:

Previously, veenasai2 wrote…

Looks like directly updating at github does not show up.

Yes, git and GitHub have a one-way relationship: all changes in your git repo are visible on GitHub (after you git-push), but GitHub changes do not affect your git repo.


examples/aks-attestation/README.md, line 89 at r1 (raw file):

Previously, veenasai2 wrote…

This happened with me. I ran gsc with default (i.e. in-kernel driver) config.yaml option. That will assume, I have in-kernel driver on my host system. This in-kernel driver can be 5.11 or lower. When I ran the same gsc image as part of AKS cluster, there it failed with the symbolic link error. I don't know which in-kernel driver is installed on Azure host system where the container is running, when I create an AKS cluster. So only for the AKS environment, I thought providing symbolic link as part of apploader.sh would be good, until they upgrade their in-kernel driver.

I'm resolving this issue because you reverted that change in apploader.sh. But in general, you shouldn't re-use the same config.yaml file for your local environment and for the AKS environment. These are two different deployments, so you should use two different configuration files. In other words, you're not supposed to use the same GSC image in AKS as you would do on your local machine.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I'm approving this PR (we'll fix the commit message during final rebase), but please double-check that this latest version works correctly on AKS.

Pushing couple of fixes in few hours. Thanks.



-- commits, line 3 at r1:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Yes, git and GitHub have a one-way relationship: all changes in your git repo are visible on GitHub (after you git-push), but GitHub changes do not affect your git repo.

Ok, thanks.


examples/aks-attestation/README.md, line 89 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I'm resolving this issue because you reverted that change in apploader.sh. But in general, you shouldn't re-use the same config.yaml file for your local environment and for the AKS environment. These are two different deployments, so you should use two different configuration files. In other words, you're not supposed to use the same GSC image in AKS as you would do on your local machine.

ok, thanks.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 4 of 7 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r4 (raw file):

WORKDIR /ra-tls-secret-prov

ENV PATH = "${PATH}:/ra-tls-secret-prov"

I had to add the path of ra-tls-secret-prov to PATH ENV variable, because I was facing errors "Error during shim_init() in init_important_handles, DkProcessExit: Returning exit code 2" during docker run. This is similar to the issue reported here: gramineproject/graphene#2632


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 34 at r4 (raw file):

COPY CI-Examples/ra-tls-secret-prov /ra-tls-secret-prov

RUN mkdir -p /ra-tls-secret-prov/libs

The below steps are to avoid "error while loading shared libraries: " error.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @veenasai2)

a discussion (no related file):
@veenasai2 Could you rebase this PR to the latest master? Now we have the final v1.0 Gramine release, and a lot of build/install steps and workarounds are not needed anymore. In particular, all executables, utilities, libraries should be found in the Gramine install directory. No need for ENV and COPY Dockerfile commands.



examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r4 (raw file):

Previously, veenasai2 wrote…

I had to add the path of ra-tls-secret-prov to PATH ENV variable, because I was facing errors "Error during shim_init() in init_important_handles, DkProcessExit: Returning exit code 2" during docker run. This is similar to the issue reported here: gramineproject/graphene#2632

With the latest Gramine install process, this is not needed. Please remove.


examples/aks-attestation/aks-secret-prov-client.manifest, line 6 at r4 (raw file):

loader.env.LD_PRELOAD = "libsecret_prov_attest.so"

loader.env.SECRET_PROVISION_SERVERS = "<AKS-DNS-NAME.*.cloudapp.azure.com>:4433"

We now have the ability to propagate env vars from host: https://gramine.readthedocs.io/en/latest/manifest-syntax.html#environment-variables

I suggest to use it here:

loader.env.SECRET_PROVISION_SERVERS = { passthrough = true }

Then in your aks-secret-prov-client-deployment.yaml, you can specify this SECRET_PROVISION_SERVERS variable in env. This will be much cleaner than asking users to modify the manifest file (please also change the README!).


examples/aks-attestation/aks-secret-prov-client.manifest, line 21 at r4 (raw file):

  "file:/etc/passwd",
  "file:/etc/gai.conf",
  "file:/etc/resolv.conf",

Could you please sort these filenames?


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 34 at r4 (raw file):

Previously, veenasai2 wrote…

The below steps are to avoid "error while loading shared libraries: " error.

With the latest Gramine install process, this is not needed. Please remove.


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 42 at r4 (raw file):

COPY build/subprojects/mbedtls-mbedtls-2.26.0/libmbedx509_gramine.so.1 /ra-tls-secret-prov/libs

ENV LD_LIBRARY_PATH = "${LD_LIBRARY_PATH}:/ra-tls-secret-prov/libs"

With the latest Gramine install process, this is not needed. Please remove all of this.


examples/aks-attestation/README.md, line 30 at r4 (raw file):

    ```sh
    $ cd gramine/CI-Examples/ra-tls-secret-prov
    $ make clean && make dcap

Can we remove make clean command from here? It's kinda obvious that you need to rebuild from clean state.


examples/aks-attestation/README.md, line 31 at r4 (raw file):

    $ cd gramine/CI-Examples/ra-tls-secret-prov
    $ make clean && make dcap
    $ cd gramine

Please change to cd ../../ (otherwise people cannot just copy-paste this command)


examples/aks-attestation/README.md, line 56 at r4 (raw file):

    ```sh
    $ cd gramine/CI-Examples/ra-tls-secret-prov
    $ make clean && make secret_prov_min_client

Can we remove make clean command from here? It's kinda obvious that you need to rebuild from clean state.


examples/aks-attestation/README.md, line 57 at r4 (raw file):

    $ cd gramine/CI-Examples/ra-tls-secret-prov
    $ make clean && make secret_prov_min_client
    $ cd gramine

Please change to cd ../../ (otherwise people cannot just copy-paste this command)

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

@veenasai2 Could you rebase this PR to the latest master? Now we have the final v1.0 Gramine release, and a lot of build/install steps and workarounds are not needed anymore. In particular, all executables, utilities, libraries should be found in the Gramine install directory. No need for ENV and COPY Dockerfile commands.

ok, thanks.



examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

With the latest Gramine install process, this is not needed. Please remove.

I will check this, thanks.


examples/aks-attestation/aks-secret-prov-client.manifest, line 6 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

We now have the ability to propagate env vars from host: https://gramine.readthedocs.io/en/latest/manifest-syntax.html#environment-variables

I suggest to use it here:

loader.env.SECRET_PROVISION_SERVERS = { passthrough = true }

Then in your aks-secret-prov-client-deployment.yaml, you can specify this SECRET_PROVISION_SERVERS variable in env. This will be much cleaner than asking users to modify the manifest file (please also change the README!).

sure, thanks.


examples/aks-attestation/aks-secret-prov-client.manifest, line 21 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Could you please sort these filenames?

Sure, thanks.


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 34 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

With the latest Gramine install process, this is not needed. Please remove.

Yes, you are correct! With the latest Gramine install process, these libraries' location is set to the Path. But as part of dockerfile, I am not building Gramine. I just copied the contents of ra-tls-secret-prov example directory. So, the ninja installation paths are not visible inside docker environment and I am still facing the error "/ra-tls-secret-prov/secret_prov_server_dcap: error while loading shared libraries: libsecret_prov_verify_dcap.so: cannot open shared object file: No such file or directory" with the latest Gramine too.

Please correct me, if I am missing something here. Thanks.


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 42 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

With the latest Gramine install process, this is not needed. Please remove all of this.

Same reason as above.


examples/aks-attestation/README.md, line 30 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Can we remove make clean command from here? It's kinda obvious that you need to rebuild from clean state.

sure, thanks.


examples/aks-attestation/README.md, line 31 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please change to cd ../../ (otherwise people cannot just copy-paste this command)

sure, thanks.


examples/aks-attestation/README.md, line 56 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Can we remove make clean command from here? It's kinda obvious that you need to rebuild from clean state.

sure, thanks.


examples/aks-attestation/README.md, line 57 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please change to cd ../../ (otherwise people cannot just copy-paste this command)

Ok, thanks.

Copy link
Contributor

@aneessahib aneessahib left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


examples/aks-attestation/README.md, line 30 at r4 (raw file):

Previously, veenasai2 wrote…

sure, thanks.

Also - would it be better to put a script here that automates the gramine and ratls example build? This would make it more automated for the user. @dimakuv - thoughts? we could enforce this for all gsc examples that leverages the examples repo.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r4 (raw file):

Previously, veenasai2 wrote…

I will check this, thanks.

if I don't update PATH, I am still facing the same error "Error during shim_init() in init_important_handles " with the latest Gramine too. Thanks.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 11 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @aneessahib, @dimakuv, and @veenasai2)

a discussion (no related file):
So I finally got the main idea/problem of this PR: we first need to have Gramine built on the user machine to generate two base Dockerfiles, and then one of them (client) is run through GSC.

I highly dislike this requirement of "have Gramine built on the user machine". Why can't we put all the logic of "download Gramine, build Gramine and build the ra-tls-secret-prov example" inside each of these two base Dockerfiles? Then the user won't need to install Gramine on her own machine just to test this example. Also, the example will be more self-contained.

Are there any obstacles to this implementation?



examples/aks-attestation/aks-secret-prov-client.dockerfile, line 22 at r4 (raw file):

    libsgx-quote-ex

# Build environment of this Dockerfile should point to the root of Gramine directory

Please add that Also, the ra-tls-secret-prov example must be built.


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r4 (raw file):

Previously, veenasai2 wrote…

if I don't update PATH, I am still facing the same error "Error during shim_init() in init_important_handles " with the latest Gramine too. Thanks.

Ok, I finally understand why this is needed.

So, we put the symlink to the actual executable under root of the final GSC image: https://github.com/gramineproject/gsc/blob/master/templates/Dockerfile.ubuntu.build.template#L53-L55

And then GSC actually overwrites the GSC image entrypoint: https://github.com/gramineproject/gsc/blob/master/gsc.py#L65 and https://github.com/gramineproject/gsc/blob/master/gsc.py#L82

So the final GSC image has ENTRYPOINT ["/secret_prov_min_client]".

To make the symlink /secret_prov_min_client -> /ra-tls-secret-prov/secret_prov_min_client, we execute which secret_prov_min_client (see my first link). So for Bash to know where to find secret_prov_min_client binary, we need this PATH.

@veenasai2 I don't like this hack... Can we move this /ra-tls-secret-prov/secret_prov_min_client under /usr/bin or /usr/local/bin (which are the default paths for executables)? Then we won't need to play this trick with changing PATH.


examples/aks-attestation/aks-secret-prov-client.manifest, line 6 at r4 (raw file):

Previously, veenasai2 wrote…

sure, thanks.

I don't see any change.


examples/aks-attestation/aks-secret-prov-client.manifest, line 21 at r4 (raw file):

Previously, veenasai2 wrote…

Sure, thanks.

Not done?


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 28 at r4 (raw file):

RUN apt-get update && apt-get install -y libsgx-dcap-quote-verify

# Build environment of this Dockerfile should point to the root of Gramine directory

You need to add that

Also, Gramine must be built under the build/ directory (via `meson setup build/`).

examples/aks-attestation/aks-secret-prov-server.dockerfile, line 34 at r4 (raw file):

Previously, veenasai2 wrote…

Yes, you are correct! With the latest Gramine install process, these libraries' location is set to the Path. But as part of dockerfile, I am not building Gramine. I just copied the contents of ra-tls-secret-prov example directory. So, the ninja installation paths are not visible inside docker environment and I am still facing the error "/ra-tls-secret-prov/secret_prov_server_dcap: error while loading shared libraries: libsecret_prov_verify_dcap.so: cannot open shared object file: No such file or directory" with the latest Gramine too.

Please correct me, if I am missing something here. Thanks.

Ah, now I understand. This is a bit brittle (imagine we update from mbedTLS 2.26.0 to say 2.27), I wonder if we can do anything better?

Actually, why not git-clone Gramine in this Dockerfile, build it here (and also build the ra-tls-secret-prov server app) and install? Then we won't need to require the user to "point to the root of Gramine directory" and perform any kinds of COPY.


examples/aks-attestation/README.md, line 30 at r4 (raw file):

Previously, aneessahib (Anees Sahib) wrote…

Also - would it be better to put a script here that automates the gramine and ratls example build? This would make it more automated for the user. @dimakuv - thoughts? we could enforce this for all gsc examples that leverages the examples repo.

Yes, I don't mind the automated Bash script.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 12 files reviewed, 11 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

So I finally got the main idea/problem of this PR: we first need to have Gramine built on the user machine to generate two base Dockerfiles, and then one of them (client) is run through GSC.

I highly dislike this requirement of "have Gramine built on the user machine". Why can't we put all the logic of "download Gramine, build Gramine and build the ra-tls-secret-prov example" inside each of these two base Dockerfiles? Then the user won't need to install Gramine on her own machine just to test this example. Also, the example will be more self-contained.

Are there any obstacles to this implementation?

Added script gsc/examples/aks-attestation/base-image-generation-script.sh for this purpose. Thanks.



examples/aks-attestation/aks-secret-prov-client.dockerfile, line 22 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please add that Also, the ra-tls-secret-prov example must be built.

Removed this part from this file. Thanks.


examples/aks-attestation/aks-secret-prov-client.dockerfile, line 30 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Ok, I finally understand why this is needed.

So, we put the symlink to the actual executable under root of the final GSC image: https://github.com/gramineproject/gsc/blob/master/templates/Dockerfile.ubuntu.build.template#L53-L55

And then GSC actually overwrites the GSC image entrypoint: https://github.com/gramineproject/gsc/blob/master/gsc.py#L65 and https://github.com/gramineproject/gsc/blob/master/gsc.py#L82

So the final GSC image has ENTRYPOINT ["/secret_prov_min_client]".

To make the symlink /secret_prov_min_client -> /ra-tls-secret-prov/secret_prov_min_client, we execute which secret_prov_min_client (see my first link). So for Bash to know where to find secret_prov_min_client binary, we need this PATH.

@veenasai2 I don't like this hack... Can we move this /ra-tls-secret-prov/secret_prov_min_client under /usr/bin or /usr/local/bin (which are the default paths for executables)? Then we won't need to play this trick with changing PATH.

done, thanks.


examples/aks-attestation/aks-secret-prov-client.manifest, line 6 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I don't see any change.

done, thanks


examples/aks-attestation/aks-secret-prov-client.manifest, line 21 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Not done?

done, thanks


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 28 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

You need to add that

Also, Gramine must be built under the build/ directory (via `meson setup build/`).

moved gramine build steps to base-image-generation-script.sh . Thanks.


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 34 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Ah, now I understand. This is a bit brittle (imagine we update from mbedTLS 2.26.0 to say 2.27), I wonder if we can do anything better?

Actually, why not git-clone Gramine in this Dockerfile, build it here (and also build the ra-tls-secret-prov server app) and install? Then we won't need to require the user to "point to the root of Gramine directory" and perform any kinds of COPY.

I thought gramine build steps may make the image heavy in size and also the image creation may take longer time. How about keeping "COPY gramine/build/subprojects/mbedtls-mbedtls-/libmbedcrypto_gramine.so. /ra-tls-secret-prov/libs" ? Thanks.


examples/aks-attestation/README.md, line 30 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Yes, I don't mind the automated Bash script.

ok, thanks

cd gramine/Pal/src/host/Linux-SGX/signer/
openssl genrsa -3 -out enclave-key.pem 3072

# Build Gramine with DCAP enabled mode
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention that in-kernel driver is assumed. Also point to the build page if the user intends to build with other sgx driver options


## Preparing client and server images

This demonstration is created for ``gramine/CI-Examples/ra-tls-secret-prov`` sample. The sample
Copy link
Contributor

Choose a reason for hiding this comment

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

This demonstration is based on the ra-tls-secret-prov example from path . Familiarity with this sample is highly recommended before proceeding.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r14, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 39 at r14 (raw file):

Previously, veenasai2 wrote…

@dimakuv , I put a * here, considering if the lib version changes in future.

Sounds good.


examples/aks-attestation/base-image-generation-script.sh, line 1 at r14 (raw file):

# Download and build Gramine directory

Please append smth like: : we need it to copy ra-tls-secret-prov files and relevant libraries into the server and client Dockerfiles

Also add a line like This script also expects Gramine to be installed on the system.


examples/aks-attestation/base-image-generation-script.sh, line 12 at r14 (raw file):

cd gramine/CI-Examples/ra-tls-secret-prov
make clean && make dcap

make dcap actually invokes gramine-manifest, gramine-sgx-sign and so on. These tools will be taken from Gramine which is installed system-wide. (Because you're not re-defining PATH, PYTHON_PATH and PKG_CONFIG_PATH.)

Actually, I don't see a big problem in this, because the user should have Gramine installed on the system (system-wide) anyway, if he is looking at this AKS example.

However, I want to make sure this was your intention: do you assume that Gramine is installed system-wide on the machine? Then the only purpose of building Gramine and installing into meson_build_output is to copy the libsgx*, libmbed* libraries into the Dockerfile.


examples/aks-attestation/gramine_build.sh, line 40 at r13 (raw file):

Previously, veenasai2 wrote…

done, thanks. Base images does not require updating the PATH, as gramine specific executables are being used during gsc build.

Still not enough. See my other comment.


examples/aks-attestation/gramine_build.sh, line 31 at r14 (raw file):

git clone https://github.com/gramineproject/gramine.git
cd gramine
mkdir -p $PWD/meson_build_output

Please remove $PWD/, it is not needed. Simply mkdir -p meson_build_output

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 12 of 14 files reviewed, 12 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)


examples/aks-attestation/aks-secret-prov-server.dockerfile, line 39 at r14 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Sounds good.

Thanks.


examples/aks-attestation/base-image-generation-script.sh, line 1 at r14 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please append smth like: : we need it to copy ra-tls-secret-prov files and relevant libraries into the server and client Dockerfiles

Also add a line like This script also expects Gramine to be installed on the system.

done, thanks.


examples/aks-attestation/base-image-generation-script.sh, line 12 at r14 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

make dcap actually invokes gramine-manifest, gramine-sgx-sign and so on. These tools will be taken from Gramine which is installed system-wide. (Because you're not re-defining PATH, PYTHON_PATH and PKG_CONFIG_PATH.)

Actually, I don't see a big problem in this, because the user should have Gramine installed on the system (system-wide) anyway, if he is looking at this AKS example.

However, I want to make sure this was your intention: do you assume that Gramine is installed system-wide on the machine? Then the only purpose of building Gramine and installing into meson_build_output is to copy the libsgx*, libmbed* libraries into the Dockerfile.

@dimakuv I have put all the three paths. But while building , I am getting error, only if I don't have PKG_CONFIG_PATH set.

Reason:

LFLAGS += $(shell pkg-config --libs mbedtls_gramine)

https://github.com/gramineproject/gramine/blob/master/CI-Examples/ra-tls-secret-prov/Makefile#L53

PATH and PYTHON_PATH are not required for building secret_prov_server_dcap and secret_prov_min_client.


examples/aks-attestation/gramine_build.sh, line 40 at r13 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Still not enough. See my other comment.

ok, got it. Thanks.


examples/aks-attestation/gramine_build.sh, line 31 at r14 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove $PWD/, it is not needed. Simply mkdir -p meson_build_output

done, thanks.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r15, all commit messages.
Reviewable status: 13 of 14 files reviewed, 13 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)


examples/aks-attestation/base-image-generation-script.sh, line 12 at r14 (raw file):
I think you misunderstood my point.

There are two possibilities:

  1. Your AKS example requires Gramine to be installed system-wide on the machine. Then you can use gramine-manifest, gramine-sgx-get-token and other Gramine utilities, without the need to download and build Gramine from GitHub. Then the only purpose of downloading and building a separate Gramine version is to copy mbedTLS files and CI-Examples files.
  2. Your AKS example is self-contained and does not require Gramine to be installed system-wide on the machine. Then you must use gramine-manifest, gramine-sgx-get-token and other Gramine utilities from the downloaded and built Gramine version (for this, you need to specify PATH, PYTHONPATH and PKGCONFIGPATH). And you also use this downloaded and built Gramine version to copy mbedTLS files and CI-Examples files.

I was under impression that you wanted to go with option 1. And so I left my comments about this option 1.

But now you changed your PR to option 2. I'm fine with this, though I didn't expect this.

PATH and PYTHON_PATH are not required for building secret_prov_server_dcap and secret_prov_min_client.

They are required if you don't have Gramine installed system-wide. You were only able to build ra-tls-secret-prov files because you have Gramine installed system-wide. To check it, just find a clean Ubuntu machine and try the old version of your PR -- it will fail during steps make dcap and make secret_prov_min_client.


examples/aks-attestation/base-image-generation-script.sh, line 2 at r15 (raw file):

# Download and build Gramine directory. We need it to copy ra-tls-secret-prov files and relevant
# libraries into the server and client Dockerfiles. This script also expects Gramine to be installed

Please expand: We need it to copy ... -> we need it to build and copy ...


examples/aks-attestation/base-image-generation-script.sh, line 3 at r15 (raw file):

# Download and build Gramine directory. We need it to copy ra-tls-secret-prov files and relevant
# libraries into the server and client Dockerfiles. This script also expects Gramine to be installed
# on the system.

Well now that you added PATH, PYTHONPATH, PKGCONFIGPATH, this last sentence (This script also expects ...) is not needed. Now your example is completely self-contained (it uses the just-built Gramine version, and doesn't use the system-wide version). So please remove this last sentence.

Or am I missing something?


examples/aks-attestation/base-image-generation-script.sh, line 15 at r15 (raw file):

# Include Meson build output Python dir in $PYTHONPATH, needed by gramine-sgx-get-token
export PYTHONPATH="${PYTHONPATH}:$(find $PWD/gramine/meson_build_output/lib -type d -path '*/site-packages')"

You should swap these two parts:

export PYTHONPATH="$(find $PWD/gramine/meson_build_output/lib -type d -path '*/site-packages'):${PYTHONPATH}"

This forces Python to first look at the gramine/meson_build_output/... directory first -- exactly what we want. So even if you have Gramine installed system-wide, this script will force Python to use your just-built Gramine version.


examples/aks-attestation/base-image-generation-script.sh, line 18 at r15 (raw file):

# Include Meson build output packages dir in $PKG_CONFIG_PATH, contains mbedTLS and util libs
export PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:$(find $PWD/gramine/meson_build_output/lib -type d -path '*/pkgconfig')"

You should swap these two parts:

export PKG_CONFIG_PATH="$(find $PWD/gramine/meson_build_output/lib -type d -path '*/pkgconfig'):${PKG_CONFIG_PATH}"

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 13 of 14 files reviewed, 13 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)


examples/aks-attestation/base-image-generation-script.sh, line 12 at r14 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I think you misunderstood my point.

There are two possibilities:

  1. Your AKS example requires Gramine to be installed system-wide on the machine. Then you can use gramine-manifest, gramine-sgx-get-token and other Gramine utilities, without the need to download and build Gramine from GitHub. Then the only purpose of downloading and building a separate Gramine version is to copy mbedTLS files and CI-Examples files.
  2. Your AKS example is self-contained and does not require Gramine to be installed system-wide on the machine. Then you must use gramine-manifest, gramine-sgx-get-token and other Gramine utilities from the downloaded and built Gramine version (for this, you need to specify PATH, PYTHONPATH and PKGCONFIGPATH). And you also use this downloaded and built Gramine version to copy mbedTLS files and CI-Examples files.

I was under impression that you wanted to go with option 1. And so I left my comments about this option 1.

But now you changed your PR to option 2. I'm fine with this, though I didn't expect this.

PATH and PYTHON_PATH are not required for building secret_prov_server_dcap and secret_prov_min_client.

They are required if you don't have Gramine installed system-wide. You were only able to build ra-tls-secret-prov files because you have Gramine installed system-wide. To check it, just find a clean Ubuntu machine and try the old version of your PR -- it will fail during steps make dcap and make secret_prov_min_client.

Earlier, I just looked at the Makefile and also renamed some systemwide gramine installation paths and only error I got was for PKGCONFIGPATH. I thought since we are doing just native build (neither gramine-sgx, not gramine), so we will not require gramine-sgx-get-token and other gramine specific utilities.

But, you are right, I will check on a clean ubuntu machine and get back. Thanks.


examples/aks-attestation/base-image-generation-script.sh, line 2 at r15 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please expand: We need it to copy ... -> we need it to build and copy ...

done, thanks.


examples/aks-attestation/base-image-generation-script.sh, line 3 at r15 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Well now that you added PATH, PYTHONPATH, PKGCONFIGPATH, this last sentence (This script also expects ...) is not needed. Now your example is completely self-contained (it uses the just-built Gramine version, and doesn't use the system-wide version). So please remove this last sentence.

Or am I missing something?

@dimakuv Yes, you are right. Earlier I understood that "we not only download and build gramine, but we also install it". That's why I kept that statement.


examples/aks-attestation/base-image-generation-script.sh, line 15 at r15 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

You should swap these two parts:

export PYTHONPATH="$(find $PWD/gramine/meson_build_output/lib -type d -path '*/site-packages'):${PYTHONPATH}"

This forces Python to first look at the gramine/meson_build_output/... directory first -- exactly what we want. So even if you have Gramine installed system-wide, this script will force Python to use your just-built Gramine version.

Understood!! Done, thanks.


examples/aks-attestation/base-image-generation-script.sh, line 18 at r15 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

You should swap these two parts:

export PKG_CONFIG_PATH="$(find $PWD/gramine/meson_build_output/lib -type d -path '*/pkgconfig'):${PKG_CONFIG_PATH}"

done, thanks.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r16, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)

a discussion (no related file):
I think I'm finally happy with this PR (bar the last few comments) :)



examples/aks-attestation/base-image-generation-script.sh, line 12 at r14 (raw file):

Previously, veenasai2 wrote…

Earlier, I just looked at the Makefile and also renamed some systemwide gramine installation paths and only error I got was for PKGCONFIGPATH. I thought since we are doing just native build (neither gramine-sgx, not gramine), so we will not require gramine-sgx-get-token and other gramine specific utilities.

But, you are right, I will check on a clean ubuntu machine and get back. Thanks.

@veenasai2 I was wrong, you were right. I take my words back.

What you do in this bash script is make dcap and then make secret_prov_min_client. These correspond to this in the ra-tls-secret-prov Makefile:

These Makefile targets indeed only perform a native build (simply gcc <output> <input>, no Gramine involvement). I was under the impression that these Makefile targets also invoke Gramine utilities like gramine-manifest and gramine-sgx-get-token.

Thanks for explaining my error to me. You are right, we only need PKGCONFIGPATH and don't need the other two variables.


examples/aks-attestation/base-image-generation-script.sh, line 1 at r16 (raw file):

# Download and build Gramine directory. We need it to build and copy ra-tls-secret-prov files and

Please remove directory. One can't "build directory", it's broken English.


examples/aks-attestation/base-image-generation-script.sh, line 11 at r16 (raw file):

# Include Meson build output directory in $PATH
export PATH="$PWD/gramine/meson_build_output/bin:$PATH"

Please remove these two lines, we don't need PATH


examples/aks-attestation/base-image-generation-script.sh, line 14 at r16 (raw file):

# Include Meson build output Python dir in $PYTHONPATH, needed by gramine-sgx-get-token
export PYTHONPATH="$(find $PWD/gramine/meson_build_output/lib -type d -path '*/site-packages'):${PYTHONPATH}"

Please remove these two lines, we don't need PYTHONPATH

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 12 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)


examples/aks-attestation/base-image-generation-script.sh, line 12 at r14 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

@veenasai2 I was wrong, you were right. I take my words back.

What you do in this bash script is make dcap and then make secret_prov_min_client. These correspond to this in the ra-tls-secret-prov Makefile:

These Makefile targets indeed only perform a native build (simply gcc <output> <input>, no Gramine involvement). I was under the impression that these Makefile targets also invoke Gramine utilities like gramine-manifest and gramine-sgx-get-token.

Thanks for explaining my error to me. You are right, we only need PKGCONFIGPATH and don't need the other two variables.

ok, thanks.


examples/aks-attestation/base-image-generation-script.sh, line 1 at r16 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove directory. One can't "build directory", it's broken English.

done, thanks.


examples/aks-attestation/base-image-generation-script.sh, line 11 at r16 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove these two lines, we don't need PATH

done, thanks.


examples/aks-attestation/base-image-generation-script.sh, line 14 at r16 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove these two lines, we don't need PYTHONPATH

done, thanks.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 10 of 14 files reviewed, 13 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)


examples/aks-attestation/base-image-generation-script.sh, line 2 at r17 (raw file):

# Download and build Gramine. We need it to build and copy ra-tls-secret-prov files and
# relevant libraries into the server and client Dockerfiles.

@dimakuv I removed spaces after comment statements to follow gsc and gramine convention for script and dockerfiles

https://github.com/gramineproject/gsc/blob/master/templates/apploader.template#L11

https://github.com/gramineproject/gramine/blob/master/.ci/ubuntu18.04.dockerfile#L4

Thanks.

dimakuv
dimakuv previously approved these changes Dec 3, 2021
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 3 files at r17, 2 of 2 files at r18, all commit messages.
Dismissed @veenasai2 from a discussion.
Reviewable status: all files reviewed, 8 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mythi, and @veenasai2)


examples/aks-attestation/base-image-generation-script.sh, line 2 at r17 (raw file):

Previously, veenasai2 wrote…

@dimakuv I removed spaces after comment statements to follow gsc and gramine convention for script and dockerfiles

https://github.com/gramineproject/gsc/blob/master/templates/apploader.template#L11

https://github.com/gramineproject/gramine/blob/master/.ci/ubuntu18.04.dockerfile#L4

Thanks.

Thanks, looks good

@dimakuv dimakuv requested a review from mkow December 3, 2021 08:56
Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 13 of 14 files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/gramine_build.sh, line 34 at r19 (raw file):

openssl genrsa -3 -out Pal/src/host/Linux-SGX/signer/enclave-key.pem 3072

# Install DCAP dependencies

@dimakuv after running the script on a clean system with fresh ubuntu installation, I found that dcap headers (that's why libsgx-dcap-quote-verify-dev, instead of just libsgx-dcap-quote-verify) are also required for meson setup build to proceed with -Ddcap=enabled option. Thanks.


examples/aks-attestation/gramine_build.sh, line 39 at r19 (raw file):

wget https://download.01.org/intel-sgx/sgx_repo/ubuntu/intel-sgx-deb.key
sudo apt-key add intel-sgx-deb.key
sudo apt-get install -y libsgx-dcap-quote-verify-dev

@dimakuv Here, I have question that should we go with "--no-install-recommends " , because so many other sgx libraries are getting installed with this command?

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 13 of 14 files reviewed, 11 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/README.md, line 33 at r19 (raw file):

server for verification, therefore the user needs to graminize the client application. Hence, the
following two steps create a native Docker server image and a graminized GSC client image for the
AKS cluster.

@dimakuv Should we mention that "Before proceeding further user should check gsc prerequisites at "https://graphene.readthedocs.io/en/latest/manpages/gsc.html." ?

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 13 of 14 files reviewed, 11 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I think I'm finally happy with this PR (bar the last few comments) :)

@dimakuv thanks. Now I tested the PR completely on a clean system and everything is working smooth. Thanks.


Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r19, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/gramine_build.sh, line 34 at r19 (raw file):

Previously, veenasai2 wrote…

@dimakuv after running the script on a clean system with fresh ubuntu installation, I found that dcap headers (that's why libsgx-dcap-quote-verify-dev, instead of just libsgx-dcap-quote-verify) are also required for meson setup build to proceed with -Ddcap=enabled option. Thanks.

Ok.


examples/aks-attestation/gramine_build.sh, line 39 at r19 (raw file):

Previously, veenasai2 wrote…

@dimakuv Here, I have question that should we go with "--no-install-recommends " , because so many other sgx libraries are getting installed with this command?

Does it work correctly with your suggested flag --no-install-recommends on the clean system? If everything works correctly, then sure, add this flag. From what I've read on the internet, adding this flag may make the installed packages to behave incorrectly.


examples/aks-attestation/README.md, line 33 at r19 (raw file):

Previously, veenasai2 wrote…

@dimakuv Should we mention that "Before proceeding further user should check gsc prerequisites at "https://graphene.readthedocs.io/en/latest/manpages/gsc.html." ?

I don't think so. It's quite obvious that if the user wants to try this GSC example, the user is supposed to know how GSC works and is supposed to install all prerequisites.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 13 of 14 files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/gramine_build.sh, line 39 at r19 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Does it work correctly with your suggested flag --no-install-recommends on the clean system? If everything works correctly, then sure, add this flag. From what I've read on the internet, adding this flag may make the installed packages to behave incorrectly.

yes with --no-install-recommends everything works fine. One additional library "libsgx-urts" required due to the below dependency

https://github.com/gramineproject/gramine/blob/master/CI-Examples/ra-tls-secret-prov/Makefile#L54

thanks.


examples/aks-attestation/README.md, line 33 at r19 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I don't think so. It's quite obvious that if the user wants to try this GSC example, the user is supposed to know how GSC works and is supposed to install all prerequisites.

Ok, thanks. Actually, if someone just follows the README.md here and then at step "Create the GSC client image:" without gsc prerequisites, gsc build will fail on a clean system. Thanks.

dimakuv
dimakuv previously approved these changes Dec 6, 2021
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r20, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/README.md, line 33 at r19 (raw file):

Previously, veenasai2 wrote…

Ok, thanks. Actually, if someone just follows the README.md here and then at step "Create the GSC client image:" without gsc prerequisites, gsc build will fail on a clean system. Thanks.

It's ok if gsc build fails. If someone didn't install all pre-requisites of GSC before running this example, they will go to the GSC documentation and will quickly find that they need to install some pre-requisites. So I don't see a problem.

Copy link
Contributor

@aneessahib aneessahib left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 11 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/README.md, line 82 at r20 (raw file):

        `aks-secret-prov-client-deployment.yaml`

Note: We tested this example with DCAP driver 1.11 specified in the GSC configuration file.

Shouldn't we be putting this note just when step 2 starts? So that the user can modify the config.yaml file accordingly. @dimakuv thoughts?

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 11 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/README.md, line 82 at r20 (raw file):

Previously, aneessahib (Anees Sahib) wrote…

Shouldn't we be putting this note just when step 2 starts? So that the user can modify the config.yaml file accordingly. @dimakuv thoughts?

Sure, let's move this note to step 2.

Copy link
Contributor Author

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 11 of 14 files reviewed, 11 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @aneessahib, @dimakuv, @mkow, @mythi, and @veenasai2)


examples/aks-attestation/base-image-generation-script.sh, line 2 at r17 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Thanks, looks good

Thanks.


examples/aks-attestation/gramine_build.sh, line 34 at r19 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Ok.

Thanks


examples/aks-attestation/README.md, line 33 at r19 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

It's ok if gsc build fails. If someone didn't install all pre-requisites of GSC before running this example, they will go to the GSC documentation and will quickly find that they need to install some pre-requisites. So I don't see a problem.

ok, thanks.


examples/aks-attestation/README.md, line 82 at r20 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Sure, let's move this note to step 2.

done, thanks.

@veenasai2
Copy link
Contributor Author

This PR is superseded by #38. Thanks

@veenasai2 veenasai2 closed this Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants