add proposal cosign integration #172
Conversation
|
This is awesome and I would love to help out anyway possible! (I'm a maintainer on cosign) |
The proposal is for cosign and harbor integration story Signed-off-by: Wang Yan <wangyan@vmware.com>
proposals/new/cosign-integration.md
Outdated
| the type of the accessory, like signature.cosign. | ||
| */ | ||
| type varchar(1024), | ||
| digest varchar(1024), |
There was a problem hiding this comment.
is this column needed? it's a dup of artifact.digest?
There was a problem hiding this comment.
To show the pull command of signature, it can reduce the sql join.
proposals/new/cosign-integration.md
Outdated
| "subject_artifact_id":81, | ||
| "digest":"sha256:94788818ad901025c81f8696f3ee61619526b963b7dc36435ac284f4497aa7ca", | ||
| "type":"signature.cosign", | ||
| "icon":"sha256:0048162a053eef4d4ce3fe7518615bef084403614f8bca43b40ae2e762e11e06", |
There was a problem hiding this comment.
I may need to refresh my memory about this but do all the icons stored as blobs now, even the preset ones like container images?
There was a problem hiding this comment.
for the cosign scenario, we can add a default icon in Harbor.
proposals/new/cosign-integration.md
Outdated
|
|
||
| ```yaml | ||
|
|
||
| GET /api/v2.0/projects/library/repositories/hello-world/artifacts |
There was a problem hiding this comment.
Will need to add a querystring for accessories?
| **Delete artifact signature(specific)** | ||
|
|
||
| ```rest | ||
| DELETE /api/v2.0/projects/library/repositories/hello-world/artifacts/sha256:1b26826f602946860c279fce658f31050cff2c596583af237d971f4629b57792/accessory?type=signature.cosign&digtest=sha256:94788818ad901025c81f8696f3ee61619526b963b7dc36435ac284f4497aa7cb |
There was a problem hiding this comment.
what if the sha256:94788818ad901025c81f8696f3ee61619526b963b7dc36435ac284f4497aa7cb is not a accessory of sha256:1b26826f602946860c279fce658f31050cff2c596583af237d971f4629b57792?
Do you wanna support both digest and artifact_id when managing an accessory?
|
|
||
| - You will have an option indicate that whether you want to replicate the signature between Harbor instances. | ||
| 1. Replication the artifact to the target Harbor. | ||
| 2. Replication the signature to the target Harbor, then the target Harbor will build the reference. |
There was a problem hiding this comment.
Since you introduced the accessory abstraction layer you should describe the replication scenario in accessory terms.
For simplicity if possible IMO all accessories should be replicated considering artifact+accessories is one entity.
We also need to check if the "docker-reference" in the signature blob is always empty, if it's not, data will be inconsistent after copy/replication.
What if the target Harbor does not set to cosign mode? I think in that case the signature should not be replicated?
There was a problem hiding this comment.
If the root artifact is successfully replicated and the signature artifact replication is failed, is that counted as a success or a failure?
There was a problem hiding this comment.
we can assume that the transation of repliation should contain artifact and signature, and it's failure and the repliated artifact should be removed from the target.
|
|
||
|  | ||
|
|
||
| #### Tag Retention |
There was a problem hiding this comment.
I don't think this is needed, this is an enhancement to tag retention policy that is out of the scope of cosign integration.
|
|
||
| 1. Signed images: | ||
| - Notary: If user sign an image with notary and restart Harbor without notary enabled, the image will be acted as un-signed. | ||
| - Cosign: If user sign an image with cosign, unless user removes the signature from the artifact, the image will be acted as signed. |
There was a problem hiding this comment.
So we wanna support notary and cosign at the same time?
| ### Signature checker Interface | ||
|
|
||
| ```go | ||
| type PublicKey crypto.PublicKey |
There was a problem hiding this comment.
What is the PublicKey related stuff?
There was a problem hiding this comment.
Just for further design.
proposals/new/cosign-integration.md
Outdated
| - build the reference in the database | ||
|
|
||
|
|
||
| ### Request Artifact Accessory |
There was a problem hiding this comment.
I think we need a section for API change, note that we also need to determine what to do with the existing APIs:
Do we list accessories in GET /artifacts API?
What about DELETE ...
|
|
||
| 4. **Index Signing** | ||
|
|
||
| Cosign can sign a multi-platform `Index`, but the signature is attached to the index digest. Should we see all the references are signed? |
There was a problem hiding this comment.
It's hard to handle that when you check the signature in a pull command?
There was a problem hiding this comment.
I think it makes sense to check that any content you're using as a consumer is signed - i.e. the manifest list (index) and any arch-specific content too.
proposals/new/cosign-integration.md
Outdated
| /* | ||
| the artifact id of the accessory itself. | ||
| */ | ||
| artifact_id int, |
There was a problem hiding this comment.
I feel seeing accessory as an artifact make things more complicated in the API to handle artifacts you need to add a lot of if statements b/c accessories are different from regular artifacts.
| ### To be discussed | ||
|
|
||
| 1. Cosign attaches signature to digest by default, should we apply the signature to all tags of an artifact? | ||
| 2. Cosign can specify the signature repository, do we support it? |
There was a problem hiding this comment.
Need more context about signature repository
There was a problem hiding this comment.
I believe this is a client-side option only and wouldn't affect the server-side aspects of Harbor?
See https://github.com/sigstore/cosign#specifying-registry for info on what this does.
There was a problem hiding this comment.
For integration with Harbor, living in the same repository seems much better and simpler.
There was a problem hiding this comment.
to support it, it will involve the permission things(AZ).
|
|
||
| 1. Cosign attaches signature to digest by default, should we apply the signature to all tags of an artifact? | ||
| 2. Cosign can specify the signature repository, do we support it? | ||
| 3. Can the signature be added/removed tag or label like other artifact? |
There was a problem hiding this comment.
It's something to think about for all accessories, see my comment in the table schema for accessory.
There was a problem hiding this comment.
IMO, as it's an accessory of the root artifacts, it seems there are no use cases to require treating accessory artifacts as general ones。 They're just additional things of the root artifacts.
|
|
||
| User can specify anything for the tag attribute, even the tag does not exist. | ||
|
|
||
| Do we need to support this scenario? |
There was a problem hiding this comment.
We need to extract the blob to support it right? If the answer is yes, I suggest not.
There was a problem hiding this comment.
-1.
Conflict with the `To-be-discussed item 1?
|
|
||
| - Immutable Repository | ||
|
|
||
| Cosign's model of "appending" signatures won't work for multiple signatures. |
There was a problem hiding this comment.
Do you mean if a repository is immutable in Harbor you can't push any signatures?
There was a problem hiding this comment.
You'd be able to push an initial signature manifest, but not any additional signatures.
There was a problem hiding this comment.
All the signatures shares one tag name, if the immutable enabled, the following push will be rejected.
|
|
||
| As the multiple signatures share one tag, the previously pushed becomes the untagged artifact. If user run GC with untagged checked, all of these untagged signatures will be removed. | ||
|
|
||
| ```json |
There was a problem hiding this comment.
need more context about this JSON.
|
|
||
| ``` | ||
|
|
||
| Solution: For GC, the untagged artifact means that one artifact without any tag and are not an accessory. |
There was a problem hiding this comment.
Then you'll need to make sure the accessory table is in sync, which can be messy to handle.
proposals/new/cosign-integration.md
Outdated
| /* | ||
| Read all the public key from system. | ||
| */ | ||
| PublicKeyes(ctx context.Context) ([]PublicKey, error) |
There was a problem hiding this comment.
| PublicKeyes(ctx context.Context) ([]PublicKey, error) | |
| PublicKeys(ctx context.Context) ([]PublicKey, error) |
There was a problem hiding this comment.
What's the use case of this method?
| PublicKey(ctx context.Context, artifact Artifact) (PublicKey, error) | ||
|
|
||
| /* | ||
| Read all the public key from system. |
There was a problem hiding this comment.
Do you want this at the system level, or per repository, or maybe both?
There was a problem hiding this comment.
it's a draft design and for the phase 2. But, I suppose it's at the system level.
|
|
||
| 1. Signed images: | ||
| - Notary: If user sign an image with notary and restart Harbor without notary enabled, the image will be acted as un-signed. | ||
| - Cosign: If user sign an image with cosign, unless user removes the signature from the artifact, the image will be acted as signed. |
| ### To be discussed | ||
|
|
||
| 1. Cosign attaches signature to digest by default, should we apply the signature to all tags of an artifact? | ||
| 2. Cosign can specify the signature repository, do we support it? |
There was a problem hiding this comment.
I believe this is a client-side option only and wouldn't affect the server-side aspects of Harbor?
See https://github.com/sigstore/cosign#specifying-registry for info on what this does.
|
|
||
| 4. **Index Signing** | ||
|
|
||
| Cosign can sign a multi-platform `Index`, but the signature is attached to the index digest. Should we see all the references are signed? |
There was a problem hiding this comment.
I think it makes sense to check that any content you're using as a consumer is signed - i.e. the manifest list (index) and any arch-specific content too.
|
|
||
| - Immutable Repository | ||
|
|
||
| Cosign's model of "appending" signatures won't work for multiple signatures. |
There was a problem hiding this comment.
You'd be able to push an initial signature manifest, but not any additional signatures.
|
|
||
| ## Non Goal | ||
|
|
||
| 1. Do not manage Public keys in Harbor side. |
There was a problem hiding this comment.
Not likely unless tool chain supports push a public key as OCI artifacts.
| 4. As a project admin & user, I cannot GC an artifact's cosign signature individually. | ||
| 5. As a system admin, I can copy an artifact and its signature to another repository. | ||
| 6. As a system & project admin, I can reserve an artifact and its signature via retention policy. | ||
| 7. As a system & project admin, I can set the content trust policy to block the un-signed artifact pulling. |
There was a problem hiding this comment.
Without storing public keys, Harbor won't be able to verify the signatures. Would you allow a user to pull an image with a signature when Harbor can't tell if the signature is valid or not?
There was a problem hiding this comment.
Eventually, it depends on whether a client chooses to trust the public key or not.
At phase 1, this policy may only check if the signature exists and check if the metadata(if any) matches the artifact. With the cosign model whether the client should trust it or not is beyond the scope of Harbor.
There was a problem hiding this comment.
I think it could go either way. Harbor could block if no signature exists and allow if any signature exists.
It could also go further and block if it can't verify the signature. It mostly depends on if we want to require public keys to be uploaded to Harbor so Harbor can verify.
For a first step (and possibly only), we can go with the first approach (Harbor doesn't verify signatures because it doesn't know about public keys).
|
|
||
| ### To be discussed | ||
|
|
||
| 1. Cosign attaches signature to digest by default, should we apply the signature to all tags of an artifact? |
There was a problem hiding this comment.
I wouldn't do this. The signature was created for an image with a specific digest (potentially associated with a specific tag at a point in time).
There was a problem hiding this comment.
He means the case where different tags associate with one digest.
There was a problem hiding this comment.
Oh, thanks for the clarification. In that case, I would say yes, display the signature for all tags pointing to the same digest.
proposals/new/cosign-integration.md
Outdated
| /* | ||
| Read all the public key from system. | ||
| */ | ||
| PublicKeyes(ctx context.Context) ([]PublicKey, error) |
There was a problem hiding this comment.
What's the use case of this method?
|
|
||
| - You will have an option indicate that whether you want to replicate the signature between Harbor instances. | ||
| 1. Replication the artifact to the target Harbor. | ||
| 2. Replication the signature to the target Harbor, then the target Harbor will build the reference. |
There was a problem hiding this comment.
If the root artifact is successfully replicated and the signature artifact replication is failed, is that counted as a success or a failure?
| ### To be discussed | ||
|
|
||
| 1. Cosign attaches signature to digest by default, should we apply the signature to all tags of an artifact? | ||
| 2. Cosign can specify the signature repository, do we support it? |
There was a problem hiding this comment.
For integration with Harbor, living in the same repository seems much better and simpler.
|
|
||
| 1. Cosign attaches signature to digest by default, should we apply the signature to all tags of an artifact? | ||
| 2. Cosign can specify the signature repository, do we support it? | ||
| 3. Can the signature be added/removed tag or label like other artifact? |
There was a problem hiding this comment.
IMO, as it's an accessory of the root artifacts, it seems there are no use cases to require treating accessory artifacts as general ones。 They're just additional things of the root artifacts.
|
|
||
| User can specify anything for the tag attribute, even the tag does not exist. | ||
|
|
||
| Do we need to support this scenario? |
There was a problem hiding this comment.
-1.
Conflict with the `To-be-discussed item 1?
|
|
||
| ``` | ||
|
|
||
|  |
There was a problem hiding this comment.
Leverage the current metadata extracting processor framework?
Signed-off-by: Wang Yan <wangyan@vmware.com>
proposals/new/cosign-integration.md
Outdated
| 1. Do not manage Public keys in Harbor side. | ||
| 2. Do not support multiple signers in a deployment. | ||
| 3. Do not support cosign clean/delete(as the delete operation leverages the tag deletion API which is not a required implementation defined by OCI spec, and Harbor does not allow it). | ||
| 4. Do not support cosign copy(cosign pushes the signature firstly to the destination, and then the artifact, Harbor cannot build the reference.) |
There was a problem hiding this comment.
the description is already in the non-goal section, we don't need to add the " Do not" in the sub clause.
|
The biggest concern I have with this proposal right now is that the "accessory" that's defined here is very similar to the references work that's happening in the OCI right now (for example opencontainers/tob#96, which has a lot of links to other discussions too). I'd hate to see Harbor implement something that's very close to something the OCI eventually ratifies, but specific to |
|
I don't really have a preference either way on waiting, but I don't think it's safe to assume the OCI is actually close to ratifying anything. That link is to a draft proposal that's been open since April and still hasn't been submitted as a real proposal for a vote. After that vote, the WG would then need to actually start, design and prototype different options. Then the spec changes would need to get written, debated and approved. Then enough registries would need to implement those changes for it to be safe for clients to rely on them. I'm (pessimistically) guessing it's going to be mid to late 2022 at the earliest before there's enough widespread adoption of the new changes for clients to rely on them. The cosign spec is defined here, with a goal of working with existing registries; https://github.com/sigstore/cosign/blob/main/specs/SIGNATURE_SPEC.md and is already implemented by a few other clients so it's not really cosign specific. |
|
Thanks @dlorenc for providing your thoughts -- I guess that changes the question a little bit for me then. 😄 Why does adding "cosign support" to Harbor require creating this special "accessory" type? Is the goal of doing so to make it more usable/useful to end users, or is there a deeper reason? |
My understanding is that this is for extra features, like UI integration and policy support on top of the signatures. Basic sign/verify/upload/download operations all work with harbor out of the box. |
For the case of cosign, to add the Through the In addition, the |
|
Hey folks, This Artifact Reference Type support incorporates:
The manifest and API are part of the larger goals: https://stevelasker.blog/2021/08/26/artifact-services/ The reference implementation under CNCF distribution is also available at: https://github.com/oras-project/distribution which we're working to merge into https://github.com/distribution/distribution per: distribution/distribution#3482, which should facilitate distribution based projects like harbor to contribute and consume quickly. Please feel free to reach out to one of the Artifact owners who can provide accurate information to the projects status. |
Signed-off-by: Wang Yan <wangyan@vmware.com>
|
JFYI, the ORAS Artifacts Draft Specification has been released. It generalizes the way references can be pushed, discovered and pulled, enabling a standard across all distribution-spec based registries: |
Signed-off-by: Wang Yan <wangyan@vmware.com> format Signed-off-by: Wang Yan <wangyan@vmware.com>
|
|
||
| Beside to store an artifact into registry, there are requirements on storing secure chain artifacts, like Software Bill of Materials (SBoM) and signatures to provide rich rich functionality of an artifact. | ||
|
|
||
| The proposal is to introduce an abstract definition -- accessory -- to enable Harbor to extend the capabilities for storing artifact with additional stuff. |
There was a problem hiding this comment.
@tianon you as a native speaker, do you think the term accessory is correct here or would you use a better suited term?
I would like to critically question the "term", as it will stick on forever and generation of Harbor developers and users would need lookup the manual and the code just to understand what it means.
IMHO an artifact artifact attachment or attachment would be a better suited term because:
- It is a common and well know term for an object that can be (optionally) attached to another object. It is known to many user and developers from email, documents and files.
- A good word that denotes the common use correctly will save thousand hours of time for many many users and developers.
There was a problem hiding this comment.
I'm certainly not an expert, but I am "native" so I'll do my best! 😇
I'll start by saying I find it a little amusing that "attachment" is the very word I've been using to describe this feature to colleagues. 😅 🙈
To be clear, I think "accessory" is ok, but I do agree that "attachment" would be a better word for what this is.
An "accessory" is usually something that's only useful or thought about in the context of the thing it's connected to (for example, jewelry is commonly called an "accessory" because something like a necklace is not terribly useful in isolation). Another way to put that is that it "enhances" the thing it's connected to ("accessory to a crime" for example -- someone who helped the crime be committed but wasn't the person who did the crime directly), and this is the usage that makes me OK with this as a term for this feature.
Conversely, "attachment" only implies a connection between the things, not necessarily that the attached thing improves the thing it's attached to in any way - the attached item is (more) often useful/meaningful in isolation too.
(IMO the word "attachment" is also more closely related to the word "reference" that the OCI is working with/around than "accessory" is.)
So I guess my conclusion would be that if we expect this feature to grow beyond just things like signatures or SBOMs that "enhance" the thing they're attached to, then yes, "attachment" is a better term for it, but if we don't want this feature to grow into more use cases we haven't thought of yet (where there might be a softer relationship between the OCI artifact and the attachment), then "accessory" is probably fine.
There was a problem hiding this comment.
thanks @Vad1mo and @tianon, firstly, we'd like to extend the accessory into more cases, like SBoM, Nydus(converted images).
I know the OCI or Oras community uses the attachement to describe the linkage of subject manifest and its things. IMO, in Harbor, I'd like to use different term to descirbe, we can say, the same thing to avoid confusion. A typical example is label, in Harbor we have this term to help user to filter artifact. However, in docker community, labels are an key-value pair.
And since we've reached the RC stage and all the source code has been merged, we have to draw an conclusion soon.
The proposal is for cosign and harbor integration story
Signed-off-by: Wang Yan wangyan@vmware.com