KBS URI #129

Xynnn007 · 2023-02-06T06:16:40Z

As confidential-containers/guest-components#191, this PR includes the following

Introduce KBS URI scheme
implement cc-kbc's API
implement sample-kbc's API
implement offline-fs-kbc's API
implement offline-sev-kbc's API
implement online-sev-kbc's API
implement eaa-kbc's API
document about the KBS URI

Please notice that all the <kbs-addr> field in a kbs uri now will be ignored, all kbc will use its own member kbs_url instead.

Also, this PR modifies the gRPC API of get_resource, as duplicated information is carried in the kbs uri

jialez0

@Xynnn007 Thanks a lot!. This is an important PR and plays a great role in our follow-up work. Here are some small questions and comments...

BTW, the changes in this PR will make the current sample_keyprovider unusable. Is there a plan to modify the sample_keyprovider?

docs/KBS_URI.md

src/kbc_modules/cc_kbc/mod.rs

src/kbc_modules/eaa_kbc/mod.rs

src/kbc_modules/offline_fs_kbc/common.rs

src/kbc_modules/offline_fs_kbc/mod.rs

src/kbc_modules/uri.rs

Xynnn007 · 2023-02-08T08:44:09Z

@jialez0 Thanks a lot for deep diving... I will update the keyprovider and related documents in the next PR. For sample-key provider, there might be two ways:

give the new key provider a parameter to determine whether to use the hardcoded key of sample kbc
deprecate it

I prefer way 2, some related work has already been done in image-rs confidential-containers/guest-components#85 . And we can get rid of those encrypted images using sample-kbc then (if any). Finally we can mark sample kbc deprecated in code and give a compiling warning or directly deleted the related code

Xynnn007 · 2023-02-08T08:49:54Z

Hi @stevenhorsman @liudalibj this pr will influence the behavior of offline-kbs in resource indexing and we will need to remake some test images for kata-cc test. Please take a look if convenient to make sure I did not do anything stupid : )

Xynnn007 · 2023-02-09T03:26:57Z

protos/getresource.proto

-    string KbcName = 1;
-    string KbsUri = 2;
-    string ResourceDescription = 3;
+    string ResourceUri = 1;


I changed the Proto here, as kbsUri is part of the resourceUri. KbcName now still matters and I think we can open an issuer later to talk about https://github.com/confidential-containers/attestation-agent/issues/99 on both image-rs side and kata-agent side.

liudalibj

one offline_fs_kbc test case need be updated

src/kbc_modules/offline_fs_kbc/mod.rs

liudalibj

Thanks for the quick action, the whole PR looks good to me.

fitzthum

A few initial comments.

docs/KBS_URI.md

src/kbc_modules/cc_kbc/mod.rs

fitzthum · 2023-02-14T18:51:10Z

src/kbc_modules/offline_fs_kbc/common.rs

@@ -62,6 +63,18 @@ pub mod tests {
    #[allow(dead_code)]
    pub const RESOURCES_NAME: &str = "aa-offline_fs_kbc-resources.json";

+    pub const KBS_URI_PREFIX: &str = "kbs://example.org/";


So if we use the offline_fs_kbs the resource request must have kbs://example.org/ in it? That seems a bit inconvenient. Wouldn't it be better to just ignore whatever KBS address is provided?

We do not need the kbs://example.org/ in it. We will ignore the KBS address if offline_fs_kbc is used. Here the const is defined to work with resource_path! macro which is to convert a whole uri into a path without KBS address.

fitzthum · 2023-02-14T19:51:46Z

src/kbc_modules/online_sev_kbc/mod.rs

@@ -49,7 +49,9 @@ impl KbcInterface for OnlineSevKbc {
    }

    async fn decrypt_payload(&mut self, annotation_packet: AnnotationPacket) -> Result<Vec<u8>> {
-        let key = self.get_key_from_kbs(annotation_packet.kid).await?;
+        let key = self
+            .get_key_from_kbs(annotation_packet.kid.resource_path())


So we aren't using the resource uri for image decryption keys?

Yes, we are. In online-sev-kbc, it also defines its own kbs address like cc-kbc and the get_key_from_kbs only makes a gRPC request containing the key id to kbs. So here we only need to extract the <repository>/<type>/<tag> (what resource_path() does) and ignore the uri of kbs. (like get_resource_from_kbs does)

Also, get_resource_from_kbs is a little different from get_key_from_kbs. The later will flush the memory after using the key using Zeroizing

There is some different of both api which confuses, let me fix this.

src/kbc_modules/uri.rs

src/kbc_modules/sample_kbc/mod.rs

Xynnn007 · 2023-02-16T06:33:28Z

Hi @fitzthum @jialez0 I've changed the name ResourceId into ResourceUri. Please take another look to make sure I didnot do anything wrong. Thanks!

jialez0

Note: To be compatible with this PR, image-rs may need some updates and its existing resource acquisition format will no longer be available, right? @Xynnn007

src/kbc_modules/uri.rs

Xynnn007 · 2023-02-21T01:51:36Z

Note: To be compatible with this PR, image-rs may need some updates and its existing resource acquisition format will no longer be available, right? @Xynnn007

Yes, we need to remove the hardcodes in image-rs and give a configuration of image-rs for the resources needed. This work should be delayed after 0.4.0 is released. Let's perfect the system for 0.5.0 : )

Xynnn007 · 2023-02-21T01:57:07Z

There are some conflicts in the code for ttrpc things.. let me finish some tiny fixes and then rebase the code.

jialez0 · 2023-02-23T09:27:18Z

Yes, we need to remove the hardcodes in image-rs and give a configuration of image-rs for the resources needed. This work should be delayed after 0.4.0 is released. Let's perfect the system for 0.5.0 : )

@Xynnn007 Sounds great! Since the changes in this PR will affect the code availability of image-rs and sample keyprovider, we should wait for the above two adaptations are ready, then merge the PRs of AA, image-rs and sample keyprovider at the same time, so that we can ensure that the existing upstream code is available as much as possible.

cc @fitzthum

jialez0 · 2023-03-03T07:19:23Z

@Xynnn007 In view of the fact that the KBS address part of the KBS URI is not actually consumed at present, we actually use the KBS address specified in the aa_kbc_params field in the config of kata-agent. Therefore, in the current version, I suggest that we temporarily directly set the format of the KBS URI to:

kbs:///<repository>/<type>/<tag>

This can avoid confusion for users now.

Xynnn007 · 2023-03-03T07:42:23Z

@Xynnn007 In view of the fact that the KBS address part of the KBS URI is not actually consumed at present, we actually use the KBS address specified in the aa_kbc_params field in the config of kata-agent. Therefore, in the current version, I suggest that we temporarily directly set the format of the KBS URI to:
kbs:///<repository>/<type>/<tag>
This can avoid confusion for users now.

Yea, I have no preference for either option kbs:///<repository>/<type>/<tag> or kbs://<kbs-addr>/<repository>/<type>/<tag>. I just want to say that <kbs-addr> could be used in future potentially to distinguish resources from different KBS. Nowkbs:///<repository>/<type>/<tag> presentation may influence the following scenarios

Test uris in kbc_modules::tests::ResourcePath
Documents
Guide for CoCo Key Provider

Does it look good?

jialez0 · 2023-03-07T07:19:02Z

Nowkbs:///// presentation may influence the following scenarios

Test uris in kbc_modules::tests::ResourcePath

Documents

Guide for CoCo Key Provider
Does it look good?

@Xynnn007 That's OK. In fact, whether we use example.org or null, they will not be actually read under the current system. I think it would be better to use null values at present, so I suggest to implement this modification.

Meanwhile, in coco keyprovider and image-rs, where example.org is used, I think null values can be used instead

fitzthum

LGTM. I also prefer the adjustment that @jialez0 mentions regarding the URI, but we can do that later.

key_id in the AnnotationPacket will follow the KBS URI format. Different KBCs will index the corresponding keu due to the uri. Signed-off-by: Xynnn007 <[email protected]>

now get_resource API receives a kbs uri to indicate the confidential resource to be requested Signed-off-by: Xynnn007 <[email protected]>

Signed-off-by: Xynnn007 <[email protected]>

Now if the request resource `type` in kbs uri is `"client-id"`, the responsed resource will be the client id. Signed-off-by: Xynnn007 <[email protected]>

Signed-off-by: Xynnn007 <[email protected]>

please refer to https://github.com/confidential-containers/attestation-agent/issues/130 for more information Signed-off-by: Xynnn007 <[email protected]>

Signed-off-by: Xynnn007 <[email protected]>

To avoid confusing, a uri pointing to a resource is named kbs resource uri. A kbs uri stands for the uri of the kbs. Signed-off-by: Xynnn007 <[email protected]>

Signed-off-by: Xynnn007 <[email protected]>

Xynnn007 mentioned this pull request Jun 30, 2023

Roadmap to unify confidential images and use KBS URI mechinism confidential-containers/guest-components#191

Closed

8 tasks

Xynnn007 force-pushed the feat-uri branch from 54dfb82 to 6002ba2 Compare February 6, 2023 06:35

jialez0 requested a review from fitzthum February 6, 2023 07:10

Xynnn007 force-pushed the feat-uri branch from 6002ba2 to 5414b1f Compare February 6, 2023 10:09

jialez0 reviewed Feb 8, 2023

View reviewed changes

Xynnn007 mentioned this pull request Feb 8, 2023

Refactoring the interact between eaa-kbc and verdictd inclavare-containers/verdictd#29

Closed

Xynnn007 force-pushed the feat-uri branch 2 times, most recently from d215eca to 0050dcd Compare February 8, 2023 08:38

Xynnn007 commented Feb 9, 2023

View reviewed changes

liudalibj suggested changes Feb 9, 2023

View reviewed changes

src/kbc_modules/offline_fs_kbc/mod.rs Outdated Show resolved Hide resolved

Xynnn007 force-pushed the feat-uri branch from 0050dcd to cc925be Compare February 9, 2023 06:00

liudalibj approved these changes Feb 9, 2023

View reviewed changes

jialez0 approved these changes Feb 13, 2023

View reviewed changes

Xynnn007 force-pushed the feat-uri branch 2 times, most recently from e01a7bf to 6febca2 Compare February 13, 2023 02:35

fitzthum reviewed Feb 14, 2023

View reviewed changes

Xynnn007 force-pushed the feat-uri branch from 6febca2 to d330b04 Compare February 15, 2023 03:33

jialez0 approved these changes Feb 20, 2023

View reviewed changes

src/kbc_modules/uri.rs Outdated Show resolved Hide resolved

Xynnn007 force-pushed the feat-uri branch 2 times, most recently from 97806ac to 76d6acd Compare February 21, 2023 01:49

Xynnn007 force-pushed the feat-uri branch 2 times, most recently from 1bc6918 to c2a99f0 Compare February 21, 2023 05:55

Xynnn007 mentioned this pull request Feb 26, 2023

Bring KBS Resource URI into Image-rs confidential-containers/guest-components#119

Merged

Xynnn007 force-pushed the feat-uri branch from c2a99f0 to 4ca3509 Compare February 27, 2023 08:18

Xynnn007 force-pushed the feat-uri branch from 4ca3509 to 992ce55 Compare February 28, 2023 03:02

Xynnn007 mentioned this pull request Feb 28, 2023

CoCo Keyprovider #143

Merged

fitzthum approved these changes Mar 7, 2023

View reviewed changes

Xynnn007 added 14 commits March 8, 2023 13:44

feat: kbs uri support for decrypt_payload

ecefcec

key_id in the AnnotationPacket will follow the KBS URI format. Different KBCs will index the corresponding keu due to the uri. Signed-off-by: Xynnn007 <[email protected]>

feat: kbs uri support for get_resource

e6d5096

now get_resource API receives a kbs uri to indicate the confidential resource to be requested Signed-off-by: Xynnn007 <[email protected]>

feat: eaa-kbc support get_resource kbs uri

c8317a7

Signed-off-by: Xynnn007 <[email protected]>

feat: cc kbc support get_resource kbs uri

57ee280

Signed-off-by: Xynnn007 <[email protected]>

feat: offline-fs-kbc support get_resource kbs uri

60482be

Signed-off-by: Xynnn007 <[email protected]>

feat: online-sev-kbc support get_resource kbs uri

dc2925e

Now if the request resource `type` in kbs uri is `"client-id"`, the responsed resource will be the client id. Signed-off-by: Xynnn007 <[email protected]>

feat: sample kbc support get_resource kbs uri

f9e4778

Signed-off-by: Xynnn007 <[email protected]>

docs: kbs uri guide and specification

a6b2584

Signed-off-by: Xynnn007 <[email protected]>

note: kbs url usage warning in note

bd6451d

please refer to https://github.com/confidential-containers/attestation-agent/issues/130 for more information Signed-off-by: Xynnn007 <[email protected]>

fix: use wraptype defined in RFC7518

1c1fa06

Signed-off-by: Xynnn007 <[email protected]>

ci: let basic ci cover offline-fs-kbc

f43eefa

Signed-off-by: Xynnn007 <[email protected]>

fix: rename kbs uri to kbs resource uri

d97db23

To avoid confusing, a uri pointing to a resource is named kbs resource uri. A kbs uri stands for the uri of the kbs. Signed-off-by: Xynnn007 <[email protected]>

fix: delete lib Cargo.lock file

7e39e6e

Signed-off-by: Xynnn007 <[email protected]>

App: apply the new get_resource API

a023856

Signed-off-by: Xynnn007 <[email protected]>

Xynnn007 force-pushed the feat-uri branch from 992ce55 to a023856 Compare March 8, 2023 05:45

jialez0 merged commit beab1ba into confidential-containers:main Mar 8, 2023

stevenhorsman mentioned this pull request Mar 9, 2023

podvm: Pinning attestation-agent version, whilst there are instablilities confidential-containers/cloud-api-adaptor#697

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KBS URI #129

KBS URI #129

Xynnn007 commented Feb 6, 2023 •

edited

Loading

jialez0 left a comment

Xynnn007 commented Feb 8, 2023

Xynnn007 commented Feb 8, 2023

Xynnn007 Feb 9, 2023

liudalibj left a comment

liudalibj left a comment

fitzthum left a comment

fitzthum Feb 14, 2023

Xynnn007 Feb 15, 2023 •

edited

Loading

fitzthum Feb 14, 2023

Xynnn007 Feb 15, 2023 •

edited

Loading

Xynnn007 commented Feb 16, 2023

jialez0 left a comment •

edited

Loading

Xynnn007 commented Feb 21, 2023 •

edited

Loading

Xynnn007 commented Feb 21, 2023

jialez0 commented Feb 23, 2023 •

edited

Loading

jialez0 commented Mar 3, 2023

Xynnn007 commented Mar 3, 2023 •

edited

Loading

jialez0 commented Mar 7, 2023

fitzthum left a comment

KBS URI #129

KBS URI #129

Conversation

Xynnn007 commented Feb 6, 2023 • edited Loading

jialez0 left a comment

Choose a reason for hiding this comment

Xynnn007 commented Feb 8, 2023

Xynnn007 commented Feb 8, 2023

Xynnn007 Feb 9, 2023

Choose a reason for hiding this comment

liudalibj left a comment

Choose a reason for hiding this comment

liudalibj left a comment

Choose a reason for hiding this comment

fitzthum left a comment

Choose a reason for hiding this comment

fitzthum Feb 14, 2023

Choose a reason for hiding this comment

Xynnn007 Feb 15, 2023 • edited Loading

Choose a reason for hiding this comment

fitzthum Feb 14, 2023

Choose a reason for hiding this comment

Xynnn007 Feb 15, 2023 • edited Loading

Choose a reason for hiding this comment

Xynnn007 commented Feb 16, 2023

jialez0 left a comment • edited Loading

Choose a reason for hiding this comment

Xynnn007 commented Feb 21, 2023 • edited Loading

Xynnn007 commented Feb 21, 2023

jialez0 commented Feb 23, 2023 • edited Loading

jialez0 commented Mar 3, 2023

Xynnn007 commented Mar 3, 2023 • edited Loading

jialez0 commented Mar 7, 2023

fitzthum left a comment

Choose a reason for hiding this comment

Xynnn007 commented Feb 6, 2023 •

edited

Loading

Xynnn007 Feb 15, 2023 •

edited

Loading

Xynnn007 Feb 15, 2023 •

edited

Loading

jialez0 left a comment •

edited

Loading

Xynnn007 commented Feb 21, 2023 •

edited

Loading

jialez0 commented Feb 23, 2023 •

edited

Loading

Xynnn007 commented Mar 3, 2023 •

edited

Loading