rust-oci-client icon indicating copy to clipboard operation
rust-oci-client copied to clipboard

Add support for the signatures registry API extension

Open mattarnoatibm opened this issue 2 years ago • 9 comments

OpenShift and IBM Cloud container registries support a signatures API extension where Red Hat Simple Signing signatures for an image digest can be stored in and fetched from the container registry.

We could add support for this extension to the OCI distribution client.

Under this issue we might also want to design how the oci-distribution Rust library should support extensions to the OCI distribution spec geerally.

Refs:

  • https://access.redhat.com/documentation/en-us/openshift_container_platform/3.10/html/cluster_administration/admin-guide-image-signatures#accessing-image-signatures-using-registry-api
  • https://github.com/containers/image/blob/main/docs/signature-protocols.md#openshift-dockerdistribution-api-extension

mattarnoatibm avatar Oct 10 '23 08:10 mattarnoatibm

So I am not personally familiar with the RSS (RHSS?) extension, but I am definitely not opposed to adding in registry extensions. The main thing I'd want to figure out before reviewing #98 is how we decide on which extensions to include. Given that this specific extension seems to be relatively well-known, it feels like it would fit, but I also don't want to get to a point where we include every extension anyone would ever use

@flavio I'm curious if you have any thoughts here on how we should weigh extensions for inclusion in the main crate or if there is another way we could possibly handle them

thomastaylor312 avatar Oct 20 '23 18:10 thomastaylor312

I don't think I see extensions here https://github.com/google/go-containerregistry/tree/v0.16.1/pkg/v1 or here: https://pkg.go.dev/github.com/docker/[email protected]+incompatible/distribution, so I am not quite certain how this is done in other clients

thomastaylor312 avatar Oct 20 '23 18:10 thomastaylor312

I didn't know this mechanism of registry API extensions. From what I understand this is something specific to the registry used by OpenShift. @mattarnoatibm am I right?

I have mixed feeling about getting this code into the main crate. Right now the code from the PR is pretty small and enabled only via a new dedicated feature flag.

The PR adds support for pulling and validating the signatures, but it doesn't seem to allow the creation/push of these objects. Am I right? I wonder how big this specific code could become in the future.

As for having this feature into a separate crate. I looked at the code added by #98. I think this could be moved to a different crate. We would have to extend the Client::auth return signature to return the authentication token. All the other operations done by the PR can be performed with a vanilla reqwest.Client and the auth token.

What do you think?

flavio avatar Oct 25 '23 08:10 flavio

That seems to be the simplest way forward to me and avoids taking on all the code into this crate. As things evolve, we could also make this easier by creating some sort of trait that other crates could implement

thomastaylor312 avatar Oct 25 '23 22:10 thomastaylor312

Hi Flavio, the signatures API extension is used by OpenShift container registries and IBM Cloud container registries. For API extensions to the OCI Distribution Spec in general, I think anyone can write one but I must admit I'm not aware of any extensions apart from the one for signatures.

The signatures API extension supports a PUT endpoint, where you can push a Simple Signing signature to a container registry. I think the PUT and GET (for retrieving the signatures for a digest) are the only endpoints the signatures API extension introduces.

For moving the feature to a separate crate, is there more than Client::auth returning the authentication token that should be added to the oci-distribution crate and exposed for other crates to use? For example, would it be better to use the RequestBuilderWrapper pattern from the oci-distribution crate client.rs than a vanilla reqwest.Client?

mattarnoatibm avatar Oct 26 '23 10:10 mattarnoatibm

Sorry for the late response :disappointed:

For moving the feature to a separate crate, is there more than Client::auth returning the authentication token that should be added to the oci-distribution crate and exposed for other crates to use? For example, would it be better to use the RequestBuilderWrapper pattern from the oci-distribution crate client.rs than a vanilla reqwest.Client?

I think the RequestBuilderWrapper isn't doing anything fancy, you could achieve all of that with a basic reqwest.Client. Moreover, making RequestBuilderWrapper public doesn't look good to me.

I would propose to create a feature branch where we implement the changes proposed above. You can then try to consume this feature branch and see if you have all the things needed.

Once you're happy with that we can merge the feature branch into main and tag a new release of oci-distribution.

Does it sound reasonable to you?

flavio avatar Nov 15 '23 09:11 flavio

Hi Flavio.

Also sorry for the late response, due to vacation I've been taking.

I'd be happy to update the oci-distribution crate as you propose so Client::auth returns an authentication token. I'll try to consume that updated Client::auth function from my feature branch in the image-rs crate of the confidential-containers/guest-components repo.

mattarnoatibm avatar Nov 23 '23 10:11 mattarnoatibm

Opened a PR and new issue for Client::auth returning an auth token.

mattarnoatibm avatar Nov 23 '23 15:11 mattarnoatibm

@mattarnoatibm good, if this is all you need to be unblocked I'll be happy to merge it and tag a new release of the crate

flavio avatar Nov 23 '23 17:11 flavio