Add plugin support for trustee

[salmanyam]

The guest-components should have support for plugins, not only for the resource plugin, but also for other plugins.

Resource plugin format: kbs:///alice/cosign-key/213
Other plugin format: kbs:///plugin/plugname/resourcename?param1=value1&param2=value2

Fixes #1207

[Xynnn007]

Thanks @salmanyam , I think this way to support compability for both resource and plugins is good. Note that the original code would only support POST to the target.

Also, we might need to update the document https://github.com/confidential-containers/guest-components/blob/main/attestation-agent/docs/KBS_URI.md to mention that plugin mechanism is also supported in a way.

[Xynnn007]

Note that we also have query field here

[salmanyam]

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}");
}

[Xynnn007]

Oh yes. L327-L329 I just missed 😓

[fitzthum]

One comment

[fitzthum]

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.

[Xynnn007]

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.

[fitzthum]

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.

[mythi]

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?

[Xynnn007]

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/..."

[Xynnn007]

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

[salmanyam]

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.

[fitzthum]

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.

[fitzthum]

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.

[Xynnn007]

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