Skip to content

Add plugin support for trustee #1208

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.

Sign up for GitHub

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

Open
salmanyam wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add plugin support for trustee #1208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions attestation-agent/kbs_protocol/src/client/rcar_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,21 @@ impl KbsClient<Box<dyn EvidenceProvider>> {
#[async_trait]
impl KbsClientCapabilities for KbsClient<Box<dyn EvidenceProvider>> {
async fn get_resource(&mut self, resource_uri: ResourceUri) -> Result<Vec<u8>> {
let mut remote_url = format!(
"{}/{KBS_PREFIX}/resource/{}/{}/{}",
self.kbs_host_url, resource_uri.repository, resource_uri.r#type, resource_uri.tag
);
let mut remote_url = if resource_uri.repository == "plugin" {
// plugin case → format: {host}/{KBS_PREFIX}/{plugin_name}/{tag}
Copy link
Member

@fitzthum fitzthum Dec 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm. I'm not sure we should be using the resource URI to denote a plugin. Note that the resource plugin is a plugin and it is accessed via {KBS_PREFIX}/resource/{RESOURCE_URI}. The full resource URI is provided to the plugin and the plugin name is not part of the resource URI. It seems like we should be doing the same thing for any other plugin. At least let's be very clear with the documentation here, because this is not clear.

Copy link
Member

@Xynnn007 Xynnn007 Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Currently, there are basically two design approaches:

  1. At the code level, implement a mechanism outside the KBS URI itself to support plugin selection received from user config, then let the <repo>/<type>/<tag> following the KBS URI be used by the specific plugin.

  2. Support plugin selection within the KBS URI itself. The current PR uses <repo> as a keyword filter; if it's plugin, then <type> is used as the plugin name, and tag and query are used as parameters.

Approach 2 seems to require the least code modification and flexible for users. Of course, we can also use other design methods based on approach 2, such as using a query to set the plugin, with the default query being plugin=resource, etc. As long as the default behavior is resource, it's acceptable.

Copy link
Member

@fitzthum fitzthum Dec 4, 2025

Choose a reason for hiding this comment

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

i am fine with either one, but we should be consistent between the resource plugins and all other plugins. It seems like in your other PR we are having the resource uri not include the plugin name. That means we should probably do number 1. We will probably need to introduce a new interface to allow users to set specify which plugin to use.

Copy link
Contributor

@mythi mythi Dec 5, 2025

Choose a reason for hiding this comment

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

We will probably need to introduce a new interface to allow users to set specify which plugin to use.

I think this would make more sense. Is it mandatory for the plugins to even support ResourceUri?

Copy link
Member

@Xynnn007 Xynnn007 Dec 8, 2025

Choose a reason for hiding this comment

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

Currently, we primarily use KBS Resource URIs for resource access requests, specifically HTTP GET requests via the KBS Resource plugin endpoint.

The mentioned solution 2 attempts to represent all HTTP GET requests to the plugin system using a unified KBS URI, allowing users to use a simple URI in typical scenarios.

Otherwise, with solution 1, we would need a separate field to distinguish the plugin name, resulting in configuration files like the image-rs configuration file.

[image.image_security_policy]
plugin = "resource"
uri = "kbs:///image/policy/1"

Or potential nebula

..
[xxx.nebula.cert]
plugin = "nebula"
uri = "..."

While it can be simplified as follow to indicate that there should be a HTTP GET upon the ../kbs/v0/nebula...

[xxx.nebula]
cert = "kbs:///plugin/nebula/..."

Copy link
Member

@Xynnn007 Xynnn007 Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this would make more sense. Is it mandatory for the plugins to even support ResourceUri?

No need. The plugin actually receives a GET request to /kbs/v0/<plugin>/xxx?<query> and xxx does not to have a fixed number of /.

According to this PR, the client's syntax will be kbs:///plugin/xxx?<query>, but the actual request remains unchanged, and the plugin still needs to respond to requests at the /kbs/v0/<plugin>/xxx?<query> address.

2 in some ways is more convenient, but if we do that we have to change the resource plugin implementation to match. We've already been using the resource URI for a while, so I don't think that's ideal.

We don't need to modify the plugin implementation as the request received by the plugin is not changed, nor do we need to change the original user experience; we just extend in a way.

We keep the URI syntax unchanged, letting it default to resource access. However, once the keyword "plugin" is detected, the second field is interpreted as the plugin name, followed by content of any length (and can include more /s). This explicit method is used to specify other plugins.

The URI will generate the corresponding access address, so the server doesn't need to make any changes.

You are right that in most cases the context will tell us what plugin is being used. Like get_resource API explicitly shows. So if we want to follow way 1, we might need to add a new low-level function for kbs_protocol and a high level RPC for CDH named call_plugin, both of which receives plugin_name, method (default to GET), subpath+query and an optional body bytes slice.

Anyway I will not insist on my original idea anymore.

Could you share your more comments? @salmanyam @fitzthum @mythi

Copy link
Author

@salmanyam salmanyam Dec 9, 2025

Choose a reason for hiding this comment

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

I agree with @Xynnn007. There's no need to change the plugin implementation. It is only the user who needs to specify the correct resource URI. Also, if you look into the source code of the ResourceURI in here, you can see the test cases of ResourceURI have both formats of specifying resource in trustee. This PR is also sort of inspired by those test cases.

Copy link
Member

@fitzthum fitzthum Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

All I want is consistency between how different plugins are implemented and clarity about the format of the resource URI. The resource plugin is a plugin. It is accessed via /kbs/v0/resource/resource-uri. This implies that the plugin name is separate from the resource URI. Of course we could say that the resource URI is only for the resource plugin, but in this PR we are clearly using the resource URI constructs on the client side for all requests. The input to get_resource is a resource URI.

This is inconsistent. In some cases the first field of the resource URI is a plugin, but in other cases it isn't. To me either the first field of the URI should always be the plugin name or it should never be. I lean towards the second one. Add an argument to get_resource that is the plugin name. It will take a little bit more work to propagate this up the stack, but I think this will be more clear than having a special case buried in the code.

As an aside, I think the source of this confusion is that originally the resource backend was not going to be a plugin. The plugin PR was going to serve everything with the resource backend unless it found a special case in the resource URI, which would trigger a plugin. Here maybe it would have made more sense to treat a plugin as a special case. In fact, I think I suggested doing that. Later in the process, we changed the PR to serve resources as a plugin. Here, we don't really want the resource plugin to be a special case. It should just be another plugin. I think we did the right thing by making the resource plugin a plugin, but since we changed approaches mid-PR there are a few inconsistencies that snuck in. I have seen most people who work with plugins get confused here so hopefully we can clarify things with this PR.

Copy link
Member

@fitzthum fitzthum Jan 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, we should try to come back to this. Not having plugin support in guest-components is a gap. We should decide if we want to make the plugin officially part of the resource URI or if we want to introduce a new field preceding the URI to keep track of the plugin. I am fine with either, but I am wary of other approaches that are less consistent.

Copy link
Member

@Xynnn007 Xynnn007 Jan 8, 2026

Choose a reason for hiding this comment

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

I vote for URI to support plugin as different numbers of / would be treated in different ways. Also, URI supporting way can only cover GET requests and we should know the shortage ahead of time

format!(
"{}/{KBS_PREFIX}/{}/{}",
self.kbs_host_url,
resource_uri.r#type, // plugin name
resource_uri.tag
)
} else {
// resource plugin case → format: {host}/{KBS_PREFIX}/resource/{repository}/{type}/{tag}
format!(
"{}/{KBS_PREFIX}/resource/{}/{}/{}",
self.kbs_host_url, resource_uri.repository, resource_uri.r#type, resource_uri.tag
Copy link
Member

@Xynnn007 Xynnn007 Dec 3, 2025

Choose a reason for hiding this comment

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

Note that we also have query field here

Copy link
Author

@salmanyam salmanyam Dec 3, 2025

Choose a reason for hiding this comment

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

The query field (if not None) is being added to the URL using these following lines I believe.

if let Some(ref q) = resource_uri.query {
    remote_url = format!("{remote_url}?{q}");
}

Copy link
Member

@Xynnn007 Xynnn007 Dec 4, 2025

Choose a reason for hiding this comment

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

Oh yes. L327-L329 I just missed 😓

)
};
if let Some(ref q) = resource_uri.query {
remote_url = format!("{remote_url}?{q}");
}
Expand Down
19 changes: 15 additions & 4 deletions attestation-agent/kbs_protocol/src/client/token_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,21 @@ impl KbsClient<Box<dyn TokenProvider>> {
#[async_trait]
impl KbsClientCapabilities for KbsClient<Box<dyn TokenProvider>> {
async fn get_resource(&mut self, resource_uri: ResourceUri) -> Result<Vec<u8>> {
let mut remote_url = format!(
"{}/{KBS_PREFIX}/resource/{}/{}/{}",
self.kbs_host_url, resource_uri.repository, resource_uri.r#type, resource_uri.tag
);
let mut remote_url = if resource_uri.repository == "plugin" {
// plugin case → format: {host}/{KBS_PREFIX}/{plugin_name}/{tag}
format!(
"{}/{KBS_PREFIX}/{}/{}",
self.kbs_host_url,
resource_uri.r#type, // plugin name
resource_uri.tag
)
} else {
// resource plugin case → format: {host}/{KBS_PREFIX}/resource/{repository}/{type}/{tag}
format!(
"{}/{KBS_PREFIX}/resource/{}/{}/{}",
self.kbs_host_url, resource_uri.repository, resource_uri.r#type, resource_uri.tag
)
};
if let Some(ref q) = resource_uri.query {
remote_url = format!("{remote_url}?{q}");
}
Expand Down