CDH: add en/decrypt support for eHSM-KMS #359

1570005763 · 2023-09-15T03:14:24Z

Add eHSM-KMS client to support en/decrypt operation.

eHSM-KMS related tasks mentioned in #353 will be further developed based on it.

fitzthum · 2023-09-15T13:20:16Z

Cool, I will check this out once we are done with the release.

fitzthum · 2023-09-15T17:37:12Z

confidential-data-hub/kms/src/plugins/ehsm/client.rs

+        let key_spec = "EH_AES_GCM_256";
+        let provider_settings = json!({
+            "app_id": "86f0e9fe-7f05-4110-9f65-a224ddee1233",
+            "endpoint": "https://172.16.1.1:9002",


Is this a real instance running somewhere? Is it guaranteed to be running?

Sure! it is an eHSM-KMS service that I have deployed on my local machine. And all tests pass successfully.

By the way, it's hard to run these tests on CoCo CI, since eHSM-KMS service relies on SGX. But I add a readme file about how to set up eHSM-KMS service locally. So that anyone can test eHSM-KMS Client on a device with SGX capabilities.

Ok. I guess that's why these tests are marked as ignore. You might want to replace your IP with a phony one.

Xynnn007

Thanks @1570005763 pretty good code! Some nits

Xynnn007 · 2023-09-16T01:25:32Z

confidential-data-hub/kms/src/plugins/aliyun/client.rs

    #[case(b"this is a test plaintext")]
+    #[ignore]


The change upon this file seems not related to the PR.

Indeed, it was my mistake. I will open another PR about this change.

confidential-data-hub/kms/src/plugins/ehsm/client.rs

Xynnn007 · 2023-09-16T01:32:01Z

confidential-data-hub/kms/src/plugins/ehsm/client.rs

+
+        let credential_path = format!(
+            "{EHSM_IN_GUEST_DEFAULT_KEY_PATH}/credential_{}.json",
+            provider_settings.app_id


About the way to read credential. Currently we follow the steps:

CDH get credential from the KBS

CDH put the credential under /run/..

CDH reads the credential under /run/..

@fitzthum once mentioned that this will expose the plaintext of credentials to the guest and suggest that the credentials be kept in the memory of the CDH. I think it is a good way, and the steps could be

CDH get credential from the KBS

CDH sets its own process's env the credential with specific keys

Concrete KMS plugin reads the credential from the env

Which I'd like to put another PR and let's talk more about that then.

Glad to hear that. eHSM-KMS itself supports reading the credential from environment variables, which will make the transition easy to implement.

Xynnn007 · 2023-09-16T01:34:16Z

confidential-data-hub/kms/src/plugins/ehsm/client.rs

+        let key_spec = "EH_AES_GCM_256";
+        let provider_settings = json!({
+            "app_id": "86f0e9fe-7f05-4110-9f65-a224ddee1233",
+            "endpoint": "https://172.16.1.1:9002",


same as mentioned

Xynnn007 · 2023-09-16T01:35:43Z

confidential-data-hub/kms/src/plugins/mod.rs

@@ -14,6 +14,9 @@ pub mod aliyun;

 pub mod kbs;

+#[cfg(feature = "ehsm")]
+pub mod ehsm;
+
 #[derive(AsRefStr, EnumString)]
 pub enum DecryptorProvider {


We might also need to add eHSM-KMS to new_decryptor

1570005763 · 2023-09-18T03:24:42Z

Modifications to the Aliyun KMS client have been removed. @Xynnn007

Xynnn007 · 2023-09-18T09:46:30Z

confidential-data-hub/kms/Cargo.toml

@@ -13,6 +13,7 @@ bincode = { workspace = true, optional = true }
 chrono = { workspace = true, optional = true }
 const_format.workspace = true
 crypto = { path = "../../attestation-agent/deps/crypto", optional = true }
+ehsm_client = {git = "https://github.com/intel/ehsm", optional = true }


It is good to have a rev here to fix the version of the upstream version, which is helpful to avoid breaking caused by api changes

That's helpful. Thanks!

: ) Could you help to squash the commits into one?

Sure thing. : )

Xynnn007

lgtm.

Xynnn007 · 2023-09-20T08:40:26Z

confidential-data-hub/kms/src/plugins/ehsm/README.md

+
+EHSM-KMS is a SGX-based Key Managment Service (KMS) that provides the near-equivalent hardware protection level of cryptographic functionalities including key generation, management inside the SGX enclave. More information about EHSM-KMS can be found [here](https://github.com/intel/ehsm).
+
+In CDH, we provide the EHSM-KMS Client to interact with the EHSM-KMS Server.


Suggested change

In CDH, we provide the EHSM-KMS Client to interact with the EHSM-KMS Server.

In CDH, we provide the EHSM-KMS client to interact with the EHSM-KMS Server.

fitzthum

A couple of nits, but I think this is generally ready. It's unfortunate we don't have a better way to test. We might want to revisit the CI for this feature if it becomes widely used.

fitzthum · 2023-11-16T16:42:20Z

confidential-data-hub/kms/src/plugins/ehsm/README.md

+
+For EHSM-KMS client to run, you need to set up an EHSM-KMS service in advance. The following method is only a quick start, and you can find more deployment methods (e.g. with Kubernetes) at webpage of EHSM-KMS.
+
+> Prerequest: a sgx capable machine


nit: prerequest -> prerequisite

fitzthum · 2023-11-16T16:43:47Z

confidential-data-hub/kms/src/plugins/ehsm/README.md

@@ -0,0 +1,116 @@
+# EHSM-KMS


nit: should the E be lowercase?

fitzthum · 2023-11-16T17:20:32Z

confidential-data-hub/kms/src/plugins/ehsm/client.rs

+        let key_spec = "EH_AES_GCM_256";
+        let provider_settings = json!({
+            "app_id": "86f0e9fe-7f05-4110-9f65-a224ddee1233",
+            "endpoint": "https://172.16.1.1:9002",


Ok. I guess that's why these tests are marked as ignore. You might want to replace your IP with a phony one.

Signed-off-by: Daniel Duan <[email protected]>

1570005763 · 2023-12-04T07:35:50Z

@fitzthum I have fixed these issues, thanks for your review!

1570005763 requested a review from sameo as a code owner September 15, 2023 03:14

fitzthum reviewed Sep 15, 2023

View reviewed changes

Xynnn007 reviewed Sep 16, 2023

View reviewed changes

Xynnn007 mentioned this pull request Sep 11, 2023

Roadmap to leverage KMS abilities in Confidential Data Hub #353

Open

10 tasks

Xynnn007 reviewed Sep 18, 2023

View reviewed changes

Xynnn007 approved these changes Sep 20, 2023

View reviewed changes

1570005763 closed this by deleting the head repository Nov 16, 2023

1570005763 reopened this Nov 16, 2023

fitzthum approved these changes Nov 16, 2023

View reviewed changes

CDH: add en/decrypt support for eHSM-KMS

d71874f

Signed-off-by: Daniel Duan <[email protected]>

Xynnn007 merged commit 20ef805 into confidential-containers:main Dec 4, 2023
4 checks passed

1570005763 deleted the feat-cdh-kms-ehsm branch December 4, 2023 07:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CDH: add en/decrypt support for eHSM-KMS #359

CDH: add en/decrypt support for eHSM-KMS #359

1570005763 commented Sep 15, 2023

fitzthum commented Sep 15, 2023

fitzthum Sep 15, 2023

1570005763 Sep 18, 2023

1570005763 Sep 20, 2023

fitzthum Nov 16, 2023

Xynnn007 left a comment

Xynnn007 Sep 16, 2023

1570005763 Sep 18, 2023

Xynnn007 Sep 16, 2023

1570005763 Sep 18, 2023

Xynnn007 Sep 16, 2023

Xynnn007 Sep 16, 2023

1570005763 Sep 18, 2023

1570005763 commented Sep 18, 2023 •

edited

Loading

Xynnn007 Sep 18, 2023

1570005763 Sep 19, 2023

Xynnn007 Sep 19, 2023

1570005763 Sep 20, 2023

Xynnn007 left a comment

Xynnn007 Sep 20, 2023

fitzthum left a comment

fitzthum Nov 16, 2023

fitzthum Nov 16, 2023

fitzthum Nov 16, 2023

1570005763 commented Dec 4, 2023


		EHSM-KMS is a SGX-based Key Managment Service (KMS) that provides the near-equivalent hardware protection level of cryptographic functionalities including key generation, management inside the SGX enclave. More information about EHSM-KMS can be found [here](https://github.com/intel/ehsm).

		In CDH, we provide the EHSM-KMS Client to interact with the EHSM-KMS Server.


		For EHSM-KMS client to run, you need to set up an EHSM-KMS service in advance. The following method is only a quick start, and you can find more deployment methods (e.g. with Kubernetes) at webpage of EHSM-KMS.

		> Prerequest: a sgx capable machine

CDH: add en/decrypt support for eHSM-KMS #359

CDH: add en/decrypt support for eHSM-KMS #359

Conversation

1570005763 commented Sep 15, 2023

fitzthum commented Sep 15, 2023

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

Xynnn007 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

1570005763 commented Sep 18, 2023 • 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

Xynnn007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fitzthum 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

1570005763 commented Dec 4, 2023

1570005763 commented Sep 18, 2023 •

edited

Loading