README.md: refine readme #255

ruomengh · 2024-01-29T02:00:56Z

Move design section to wiki
Add recommended configuration in installation section
Other small adjustment

eadamsintel

"provides an isolated encyryption" would be more accurate

eadamsintel

Here is my full review

eadamsintel · 2024-01-29T22:11:29Z

README.md

 environment to protect data-in-use based on hardware Trusted Execution Environment (TEE).
 It requires a full chain integrity measurement on the launch-time or runtime environment
-to guarantee "consistently behavior in expected way" (defined by
-[Trusted Computing](https://en.wikipedia.org/wiki/Trusted_Computing)) of confidential
+to guarantee "consistently behavior in expected way" of confidential


guarantee "consistent behavior in an expected way"

eadamsintel · 2024-01-29T22:12:01Z

README.md

 computing environment for tenant's zero-trust use case.

 This project is designed to provide cloud native measurement for the full measurement
 chain from TEE TCB -> Firmware TCB -> Guest OS TCB -> Cloud Native TCB as follows:

 ![](/docs/cc-full-meaurement-chain.png)

-_NOTE: Different with traditional trusted computing on non-confidential environment,
+_NOTE: Different from traditional trusted computing on non-confidential environment,
 the measurement chain is not only started with Guest's `SRTM` (Static Root Of Measurement)
 but also need include the TEE TCB, because the CC VM environment is created by TEE


"but it also needs to include"

eadamsintel · 2024-01-29T22:12:24Z

README.md

 computing environment for tenant's zero-trust use case.

 This project is designed to provide cloud native measurement for the full measurement
 chain from TEE TCB -> Firmware TCB -> Guest OS TCB -> Cloud Native TCB as follows:

 ![](/docs/cc-full-meaurement-chain.png)

-_NOTE: Different with traditional trusted computing on non-confidential environment,
+_NOTE: Different from traditional trusted computing on non-confidential environment,
 the measurement chain is not only started with Guest's `SRTM` (Static Root Of Measurement)
 but also need include the TEE TCB, because the CC VM environment is created by TEE
 via `DRTM` (Dynamic Root of Measurement) like Intel TXT on the host._

 From the perspective of tenant's workload, `CCNP` will expose the [CC Trusted API](https://github.com/cc-api/cc-trusted-api)


"of a tenant's workload"

eadamsintel · 2024-01-29T22:12:56Z

README.md

-refer [detail deployment steps](/deployment/README.md)
-
-![](/docs/ccnp-landing-confidential-cluster.png)
-
 Finally, the full trusted chain will be measured into CC report as follows using


into a CC report

eadamsintel · 2024-01-29T22:13:12Z

README.md

-refer [detail deployment steps](/deployment/README.md)
-
-![](/docs/ccnp-landing-confidential-cluster.png)
-
 Finally, the full trusted chain will be measured into CC report as follows using
 TDX as example:


as an example

eadamsintel · 2024-01-29T22:23:24Z

README.md

- A `CCNP` device plugin is provided as the dependency for services such as Quote
-Server and Measurement Server. It will help with device mount and folder injection
-within the service.
+CCNP collects primitives of confidential cloud native environments running in confidential VMs, such as Intel® TD. You can setup an Intel® TDX enlightened host and then boot TD guest on it. The feasible configurations are as below.


I don't think Intel((r) TD is a valid product name. I could not find it in the brands database.

Also it should be "then boot a TD guest"

eadamsintel · 2024-01-29T22:38:28Z

README.md

-SDK PyPI package can be found [here](https://pypi.org/project/ccnp/). Please check our [documentation](https://intel.github.io/confidential-cloud-native-primitives/) for more details.
+|  CPU | Host OS  | Host packages  | Guest OS  | Guest packages  | DCAP packages |
+|---|---|---|---|---|---|
+|  Intel® Emerald Rapids | Ubuntu 22.04| Build packages referring to [here](https://github.com/intel/tdx-tools/tree/tdx-1.5/build/ubuntu-22.04) | Ubuntu 22.04 | Build packages referring to [here](https://github.com/intel/tdx-tools/tree/tdx-1.5/build/ubuntu-22.04) | [here](https://download.01.org/intel-sgx/sgx-dcap/1.19/linux/distro/ubuntu22.04-server/)


It is a bit odd to have a column for the CPU and just repeat it twice. You can't merge the columns in markdown so I can't think of a better way to do this. Is RHEL not supported? Also, DCAP and attestation is one of the trickiest things to implement and understand. At the very least I would add "Attestation" to the DCAP column. Not everyone will know that DCAP means attestation. The two links go to very different places. The top link is going to be hard for people to follow. Linking to the linux pdf install guide and also linking to the package should be adequate. Do the docker instructions on the CCNP site for PCCS and QGS also work with the MVP stack? If so I would use the same links. I also think a very brief explanation of what PCCS does and what the QGS is used for right before this table would help a lot. You could say something like "The Platform certificate caching service (PCCS) is used to retrieve and cache PCK certificates locally to your cluster from Intel's Platform Certificate Service. This is necessary to attest the authenticity of a TDVM before a workload is started in it. The Quote Generate Service (QGS) runs on the host in a specialized enclave to generate and use TD quotes. For convenient setup these can run inside a Docker container. Learn more at https://download.01.org/intel-sgx/sgx-dcap/1.17/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf" I would embed this link in the text.

The MVP related instructions will go away eventually so I don't think it is worth a lot of effort to update this section. If the docker CCNP instructions work for that section then link that instead.

Mid-stream distro also works for CentOS Stream 9, but it will not be put here until CCNP is validated with it.
Containerized PCCS and QGS work for both rows. However, it's just an alternative for mid-stream because they may release PCCS and QGS packages for Ubuntu 23.10 in future. The configuration table will be evolved along with mid-stream status.

If the same Docker instructions work for both TDX integrations then I would just use that. This table will evolve as distros update their TDX offerings.

eadamsintel · 2024-01-29T22:43:09Z

README.md

- SDK is provided to simplify the use of the service interface for development,
-it covers communication to the service and parses the results from the services.
-With such SDK, users can perform related actions with one simple API call.
+### 2.1 Configuration


Could each of these sections apply 100% to either the host or the confidential VM? If so I would put "2.1 Configuration (HOST)"

"2.2 Deploy CCNP Service (Confidential VM"

"2.3 Install SDK" I am not sure if this would be confidential VM or container I would try to write each section so that all the instructions are run 100% on a host or 100% on a VM. It would help the user know where they should be installing things. I don't know if this can be cleanly done when comparing confidential cluster in a VM vs confidential node as a VM or confidential workload as a VM. The key thing about my comment is it gets confusing for the user to know where to run the commands.

eadamsintel · 2024-01-29T22:44:08Z

README.md


-For SDK, user can simply install from PyPI using command:
+CCNP SDK can be used by workload for cloud native primitives collecting. It needs to be installed within the workload container image and called whenever the primitives are required. The SDK can be installed from PyPI using command:


"used by a workload"

"The SDK can be installed from PyPi using the command:"

For the next section I would indicate that the repository needs to be cloned into the VM if you want to install that way. I would rephrase as "Alternatively, the CCNP can be installed from source code with the following command. Make sure to clone the repository into your confidential VM."

eadamsintel · 2024-01-29T22:49:16Z

README.md

@@ -108,10 +87,11 @@ cd sdk/python3
 pip install -e .
 ```

+### 2.4 Install CCNP Device Plugin
 For the ccnp device plugin, user can find the installation guide under the 'Installation'


Follow the CCNP device plugin installation guide .

1. Move design section to wiki 2. Add recommended configuration in installation section 3. Other small adjustment Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com>

ruomengh · 2024-01-30T06:11:53Z

Thanks @eadamsintel for the detailed review comments! Most of the comments have been dealt with in the latest commit. Could you please review it again?

Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com>

eadamsintel

Everything is looking a lot better. I only had some minor additional feedback.

eadamsintel · 2024-01-30T17:04:42Z

README.md


 ## 1. Introduction

-Confidential Computing technology like Intel TDX provides isolated encryption runtime
+Confidential Computing technology like Intel® TDX provides an isolated encryption runtime


I just noticed this should be "Confidential Computer technologies" Another way to say it would be "A Confidential Computing technology like" .. I prefer the first

eadamsintel · 2024-01-30T17:07:03Z

README.md

 environment to protect data-in-use based on hardware Trusted Execution Environment (TEE).
 It requires a full chain integrity measurement on the launch-time or runtime environment
-to guarantee "consistently behavior in expected way" (defined by
-[Trusted Computing](https://en.wikipedia.org/wiki/Trusted_Computing)) of confidential
+to guarantee "consistent behavior in an expected way" of confidential


"of a confidential computing environment"

eadamsintel · 2024-01-30T17:11:23Z

README.md

-Finally, the full trusted chain will be measured into CC report as follows using
-TDX as example:
+Finally, the full trusted chain will be measured into a CC report as follows using
+TDX as an example:


"Intel TDX"

eadamsintel · 2024-01-30T17:15:37Z

README.md

- A `CCNP` device plugin is provided as the dependency for services such as Quote
-Server and Measurement Server. It will help with device mount and folder injection
-within the service.
+CCNP collects primitives of confidential cloud native environments running in confidential VMs(CVM), such as Intel® TDX guest. The primitives are not only from the TEE + CVM boot process + CVM OS but also from the environments running workloads, e.g. Kubernetes cluster or Docker containers. Thus, you need to check below configuration for both hosts and guests.


I would add a space after "confidential VMs" and before the acronym is shown (CVM). Also, you only have to use the circle R after Intel the first time you define Intel(R) TDX. Then you can refer to it as Intel TDX.

ruomengh · 2024-01-31T01:46:13Z

@eadamsintel The document is updated according to your comments, except that I keep both ways of installing PCCS and QGS in the configuration table. Could you please review it again? Thanks.

eadamsintel

I noticed two very small grammar issues. Otherwise this is good to go.

eadamsintel · 2024-01-31T18:17:02Z

README.md

 the measurement chain is not only started with Guest's `SRTM` (Static Root Of Measurement)
-but also need include the TEE TCB, because the CC VM environment is created by TEE
-via `DRTM` (Dynamic Root of Measurement) like Intel TXT on the host._
+but it also needs to include the TEE TCB, because the CC VM environment is created by TEE


This is a small grammar correction I missed before --> "(Static Root of Mesurement), but it also needs to include the TEE TCB because the" The comma should be after the parenthesis and before "but" because it is a compound sentence. The "because" is a subordinating conjunction (had to look that terminology up) so no comma is needed. English can be very nuanced.

Thanks for the English learning tips :-)

I have to look these things up myself to re-remember :)

eadamsintel · 2024-01-31T18:18:02Z

README.md

-
-
-## 2. Design
+- Please refer to structure [`TDREPORT`](https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/Tdx.h)


The other two bullet points ended with a period . so for consistency I would add a period here too.

Good catch.

Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com>

ruomengh · 2024-02-02T07:11:49Z

@eadamsintel Thanks a lot for all the review comments. The file has been updated.

eadamsintel · 2024-02-02T17:51:16Z

I think it is GTG.

* README.md: refine readme 1. Move design section to wiki 2. Add recommended configuration in installation section 3. Other small adjustment Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com> * Doc: add OpenSSF badge in README (intel#252) * Add CCNP overall document link in README. Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com> * More update on configuration and deployment Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com> * More update on configuration and deployment Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com> --------- Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com> Co-authored-by: Ruoyu Ying <ruoyu.ying@intel.com>

ruomengh requested review from kenplusplus, Ruoyu-y, eadamsintel and dongx1x January 29, 2024 02:03

eadamsintel reviewed Jan 29, 2024

View reviewed changes

README.md: refine readme

ff490ec

1. Move design section to wiki 2. Add recommended configuration in installation section 3. Other small adjustment Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com>

Ruoyu-y and others added 3 commits January 30, 2024 15:38

Doc: add OpenSSF badge in README (#252)

cc4155e

Add CCNP overall document link in README.

e22f2e1

Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com>

More update on configuration and deployment

f2895e5

Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com>

eadamsintel reviewed Jan 30, 2024

View reviewed changes

eadamsintel reviewed Jan 31, 2024

View reviewed changes

More update on configuration and deployment

f35d2f8

Signed-off-by: Hao, Ruomeng <ruomeng.hao@intel.com>

dongx1x approved these changes Feb 4, 2024

View reviewed changes

ruomengh merged commit 461c604 into intel:main Feb 4, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

README.md: refine readme #255

README.md: refine readme #255

ruomengh commented Jan 29, 2024

eadamsintel left a comment

eadamsintel left a comment

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

ruomengh Jan 30, 2024

eadamsintel Jan 30, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

eadamsintel Jan 29, 2024

ruomengh commented Jan 30, 2024 •

edited

Loading

eadamsintel left a comment

eadamsintel Jan 30, 2024

eadamsintel Jan 30, 2024

eadamsintel Jan 30, 2024

eadamsintel Jan 30, 2024

ruomengh commented Jan 31, 2024

eadamsintel left a comment

eadamsintel Jan 31, 2024 •

edited

Loading

ruomengh Feb 2, 2024

eadamsintel Feb 2, 2024

eadamsintel Jan 31, 2024

ruomengh Feb 2, 2024

ruomengh commented Feb 2, 2024

eadamsintel commented Feb 2, 2024


		For SDK, user can simply install from PyPI using command:
		CCNP SDK can be used by workload for cloud native primitives collecting. It needs to be installed within the workload container image and called whenever the primitives are required. The SDK can be installed from PyPI using command:



		## 2. Design
		- Please refer to structure [`TDREPORT`](https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/Tdx.h)

README.md: refine readme #255

README.md: refine readme #255

Conversation

ruomengh commented Jan 29, 2024

eadamsintel left a comment

Choose a reason for hiding this comment

eadamsintel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruomengh commented Jan 30, 2024 • edited Loading

eadamsintel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruomengh commented Jan 31, 2024

eadamsintel left a comment

Choose a reason for hiding this comment

eadamsintel Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruomengh commented Feb 2, 2024

eadamsintel commented Feb 2, 2024

ruomengh commented Jan 30, 2024 •

edited

Loading

eadamsintel Jan 31, 2024 •

edited

Loading