-
Notifications
You must be signed in to change notification settings - Fork 156
docs: update resource URI and add plugin support #1468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0847fe4
5da4bc4
b5424e8
c7bcb8f
9e7932d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,15 +14,17 @@ const RESOURCE_ID_ERROR_INFO: &str = | |
| "invalid kbs resource uri, should be kbs://<addr-of-kbs>/<repo>/<type>/<tag>"; | ||
|
|
||
| const SCHEME: &str = "kbs"; | ||
| const DEFAULT_PLUGIN: &str = "resource"; | ||
|
|
||
| /// Resource Id document <https://github.com/confidential-containers/guest-components/blob/main/attestation-agent/docs/KBS_URI.md> | ||
| /// Resource Id document <https://github.com/confidential-containers/guest-components/blob/main/attestation-agent/docs/RESOURCE_URI.md> | ||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub struct ResourceUri { | ||
| pub kbs_addr: String, | ||
| pub repository: String, | ||
| pub r#type: String, | ||
| pub tag: String, | ||
|
Comment on lines
23
to
25
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that due to #1468 (comment), we could combine these fields into a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am open to this but I think we should do another PR for handling the variable length stuff. |
||
| pub query: Option<String>, | ||
| pub plugin: Option<String>, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can directly set this as
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes it a little tricky to reconstruct the URI in the |
||
| } | ||
|
|
||
| impl TryFrom<&str> for ResourceUri { | ||
|
|
@@ -47,9 +49,19 @@ impl TryFrom<url::Url> for ResourceUri { | |
| } | ||
| } | ||
|
|
||
| if value.scheme() != SCHEME { | ||
| return Err("scheme must be kbs"); | ||
| } | ||
| let scheme = value.scheme(); | ||
|
|
||
| let plugin = match scheme { | ||
| SCHEME => None, | ||
| s if s.starts_with("kbs+") => { | ||
| let plugin_name = s.trim_start_matches("kbs+"); | ||
| if plugin_name.is_empty() { | ||
| return Err("scheme kbs+ requires a plugin name, e.g. kbs+pkcs11"); | ||
| } | ||
| Some(plugin_name.to_string()) | ||
| } | ||
| _ => return Err("scheme must be kbs or kbs+<plugin>"), | ||
| }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a nit let plugin = match scheme.as_str() {
SCHEME => None,
s if s.starts_with("kbs+") => {
let plugin_name = s.trim_start_matches("kbs+");
if plugin_name.is_empty() {
return Err("scheme kbs+ requires a plugin name, e.g. kbs+pkcs11");
}
Some(plugin_name.to_string())
}
_ => return Err("scheme must be kbs or kbs+<plugin>"),
}; |
||
|
|
||
| if value.path().is_empty() { | ||
| return Err(RESOURCE_ID_ERROR_INFO); | ||
|
|
@@ -64,6 +76,7 @@ impl TryFrom<url::Url> for ResourceUri { | |
| r#type: values[1].into(), | ||
| tag: values[2].into(), | ||
| query: value.query().map(|s| s.to_string()), | ||
| plugin, | ||
| }) | ||
| } else { | ||
| Err(RESOURCE_ID_ERROR_INFO) | ||
|
|
@@ -106,6 +119,7 @@ impl ResourceUri { | |
| r#type: values[2].into(), | ||
| tag: values[3].into(), | ||
| query: None, | ||
| plugin: None, | ||
| }) | ||
| } else { | ||
| bail!( | ||
|
|
@@ -115,8 +129,12 @@ impl ResourceUri { | |
| } | ||
|
|
||
| pub fn whole_uri(&self) -> String { | ||
| let scheme = match &self.plugin { | ||
| Some(p) => format!("{SCHEME}+{p}"), | ||
| None => SCHEME.to_string(), | ||
| }; | ||
| let uri = format!( | ||
| "{SCHEME}://{}/{}/{}/{}", | ||
| "{scheme}://{}/{}/{}/{}", | ||
| self.kbs_addr, self.repository, self.r#type, self.tag | ||
| ); | ||
| match &self.query { | ||
|
|
@@ -125,6 +143,12 @@ impl ResourceUri { | |
| } | ||
| } | ||
|
|
||
| /// Returns the plugin name. If no plugin was specified in the URI, | ||
| /// returns the default plugin "resource". | ||
| pub fn plugin(&self) -> &str { | ||
| self.plugin.as_deref().unwrap_or(DEFAULT_PLUGIN) | ||
| } | ||
|
|
||
| /// Only return the resource path. This function is used | ||
| /// currently because up to now the kbs-uri is given | ||
| /// to create an AA instance. | ||
|
|
@@ -158,27 +182,58 @@ mod tests { | |
| use rstest::rstest; | ||
|
|
||
| #[rstest] | ||
| #[case("kbs:///alice/cosign-key/213", "alice", "cosign-key", "213", None)] | ||
| #[case( | ||
| "kbs:///plugin/plugname/resourcename?param1=value1¶m2=value2", | ||
| "plugin", | ||
| "plugname", | ||
| "resourcename", | ||
| Some("param1=value1¶m2=value2") | ||
| "kbs:///alice/cosign-key/213", | ||
| "alice", | ||
| "cosign-key", | ||
| "213", | ||
| None, | ||
| None | ||
| )] | ||
| #[case( | ||
| "kbs:///repo/type/tag?param1=value1¶m2=value2", | ||
| "repo", | ||
| "type", | ||
| "tag", | ||
| Some("param1=value1¶m2=value2"), | ||
| None | ||
| )] | ||
| #[case( | ||
| "kbs+pkcs11:///repo/type/tag", | ||
| "repo", | ||
| "type", | ||
| "tag", | ||
| None, | ||
| Some("pkcs11") | ||
| )] | ||
| #[case("kbs+myplugin:///a/b/c", "a", "b", "c", None, Some("myplugin"))] | ||
| #[case( | ||
| "kbs+custom://example.com:8080/repo/type/tag", | ||
| "repo", | ||
| "type", | ||
| "tag", | ||
| None, | ||
| Some("custom") | ||
| )] | ||
| fn test_resource_uri_serialization_conversion( | ||
| #[case] url: &str, | ||
| #[case] repository: &str, | ||
| #[case] r#type: &str, | ||
| #[case] tag: &str, | ||
| #[case] query: Option<&str>, | ||
| #[case] plugin: Option<&str>, | ||
| ) { | ||
| let resource = ResourceUri { | ||
| kbs_addr: "".into(), | ||
| kbs_addr: if url.contains("example.com") { | ||
| "example.com:8080".into() | ||
| } else { | ||
| "".into() | ||
| }, | ||
| repository: repository.into(), | ||
| r#type: r#type.into(), | ||
| tag: tag.into(), | ||
| query: query.map(|s| s.to_string()), | ||
| plugin: plugin.map(|s| s.to_string()), | ||
| }; | ||
|
|
||
| // Deserialization | ||
|
|
@@ -201,4 +256,21 @@ mod tests { | |
| ResourceUri::try_from(url_from_string).expect("failed to try from url"); | ||
| assert_eq!(resource_from_url, resource); | ||
| } | ||
|
|
||
| #[rstest] | ||
| #[case("kbs:///repo/type/tag", "resource")] | ||
| #[case("kbs+pkcs11:///repo/type/tag", "pkcs11")] | ||
| fn test_plugin_accessor(#[case] uri: &str, #[case] plugin: &str) { | ||
| let uri: ResourceUri = uri.try_into().expect("failed to parse uri"); | ||
| assert_eq!(uri.plugin(), plugin); | ||
| } | ||
|
|
||
| #[rstest] | ||
| #[case("http:///repo/type/tag", "scheme must be kbs")] | ||
| #[case("kbs+:///repo/type/tag", "requires a plugin name")] | ||
| fn test_invalid_scheme(#[case] uri: &str, #[case] error: &str) { | ||
| let result = ResourceUri::try_from(uri); | ||
| assert!(result.is_err()); | ||
| assert!(result.unwrap_err().contains(error)); | ||
| } | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| # Resource URIs | ||
|
|
||
| ## Introduction | ||
|
|
||
| Resource URIs are used across Confidential Containers to identify resources. | ||
| For example, the metadata of an encrypted image can contain a resource URI | ||
| referencing the image decryption key. | ||
| An image signature policy or a sealed secret can contain a resource URI. | ||
|
|
||
| Although these have been referred to as KBS Resource URIs, this abstraction | ||
| is implemented by the CDH. | ||
|
|
||
| The resource URI is not the same thing as the path for requesting a resource | ||
| from Trustee's REST API. The mapping between these is described below. | ||
|
|
||
| Technically, the Resource URI is not tied to any specific KBS, but this document | ||
| mainly focuses on Trustee and the CC_KBC and describes how the resource URI | ||
| relates to Trustee. | ||
| Some legacy code, such as the SEV KBC or the FS KBC may fulfill resource URIs | ||
| in different ways. | ||
|
|
||
| ## Specification | ||
|
|
||
| A Resource URI must comply with the following format: | ||
|
|
||
| ```plaintext | ||
| kbs+<plugin>://<kbs_host>:<kbs_port>/<repository>/<type>/<tag> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this design
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The asymmetry here is that with How the plugin URIs can be supported with api-server-rest queries?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. vault is not a plugin level conception, but a resource plugin level. Thus it's expected that the caller does not know that. for ASR, currently is
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would also that the caller doesn't get to request a concrete backing store for their resource is (say, vault or filesystem), this is up to the KBS, no? I also like this URI scheme 👍
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yup, my comment was just that the implementation detail is visible to the user and they don't necessarily understand: why
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the pkcs11 apis are different from the vaults. see https://github.com/confidential-containers/trustee/blob/main/kbs/src/plugins/implementations/pkcs11.rs#L135 for example
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, also worth noting that the resource plugin only supports one storage backend anyway. |
||
| ``` | ||
|
|
||
| ### Scheme | ||
|
|
||
| The scheme always begins with `kbs`. Typically the scheme is simply `kbs://`, but a plus sign | ||
| can be used to specify that the resource should be fulfilled by a particular plugin. | ||
|
|
||
| For instance, to represent a resource fulfilled by the Trustee `pkcs11` plugin, the | ||
| scheme would be `kbs+pkcs11://`. | ||
|
|
||
| If no plugin is specified, the CDH will request a resource from the `resource` plugin. | ||
|
|
||
| ### Host and Port | ||
|
|
||
| The host and port point to the KBS instance that will serve the resource. | ||
| Today, the CDH ignores these fields and instead gets the KBS URI and port | ||
| from the CDH config file. | ||
| This way, the resource URI does not need to be updated if the KBS URI changes. | ||
| This means that generally only one KBS serves resources for a pod, there are ways | ||
| to work around this with sealed secrets. | ||
| Multi-KBS workloads may be supported in the future. | ||
|
|
||
| Since these fields are ignored, most resource URIs leave them out. | ||
| This results in three slashes in a row. | ||
| For example, `kbs:///repository/type/tag`. | ||
|
|
||
| ### Repository, Type, and Tag | ||
|
|
||
| Currently resource URIs have three levels of identifiers/scope. | ||
| The terms `repository`, `type`, and `tag` are somewhat arbitrary. | ||
| These identifiers can be used in any way. | ||
|
Comment on lines
+54
to
+58
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, I was also thinking whether it is a good idea to require the same path segment for every plugin, for some this hierarchy might just not make any sense. but it keeps implementation complexity at bay for now and that requirement can be relaxed later once it becomes a nuisance
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes - and there is already some implementations that do not follow this, e.g. pkcs11
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we need to clarify this. Currently you cannot access the pkcs11 plugin via the resource uri, which is essentially a bug imo. I am open to allowing arbitrary length requests, like we do with rv uris, but that is not how the resource uri works today. |
||
|
|
||
| ### Query Strings | ||
|
|
||
| The resource URI also supports query strings. | ||
|
|
||
| ## Examples | ||
|
|
||
| * `kbs://example.cckbs.org:8081/alice/decryption-key/1` | ||
| * `kbs:///a/b/c` | ||
| * `kbs+pkcs11:///a/b/c` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw the original resource URI could also support query string, like |
||
|
|
||
| ## Mapping | ||
|
|
||
| The resource URI is transformed into an HTTP(S) request. | ||
| Specifically, the request will be made to `http://<kbs_host>:<kbs_port>/kbs/v0/<plugin>/alice/decryption-key/1`. | ||
|
|
||
| If no plugin is specified in the resource URI, `resource` will be used. | ||
| More information on the KBS API can be found [here](https://github.com/confidential-containers/trustee/blob/main/kbs/docs/kbs.yaml). | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can import
ResourceUriand parse, then extractrepository, .. etcThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function has slightly different behavior than the resource Uri parsing. It allows other schemes and even kids without any scheme. So I don't think it gives us much to use the resource uri crate here.