-
Notifications
You must be signed in to change notification settings - Fork 58
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
docs: Create proposal for annotation-based filtering #1797
base: dev
Are you sure you want to change the base?
Conversation
spec: | ||
name: # REQUIRED: [string], the unique type of the verifier (notation, cosign) | ||
artifactType: # REQUIRED: [string], comma seperated list, artifact type this verifier handles | ||
artifactAgeFilter: # Optional: [string], one of the following 'latest' to verify only the last artifact, or all to verify all artifacts. defaults to all. |
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.
if artifactAgeFilter may contain different values based on verifier, should this be moved to "parameters"
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.
I've added it here because each verifier/namespaced verifier configuration is evaluated as a different "verifier" by ratify.
Since the properties part is parsed by the verifier code itself, and as you said, we wish to filter in the executor, then it would be better to place those fields outside the scope of parameters.
spec: | ||
name: # REQUIRED: [string], the unique type of the verifier (notation, cosign) | ||
artifactType: # REQUIRED: [string], comma seperated list, artifact type this verifier handles | ||
artifactAgeFilter: # Optional: [string], one of the following 'latest' to verify only the last artifact, or all to verify all artifacts. defaults to all. |
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.
will the value of artifactAgeFilter
be a define set of string? an expression or key:value pair
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.
It would be as a single string, either "latest" or "all"
|
||
### Defining artifact age based filtering | ||
|
||
To support artifact age based filtering, we would add an additional field to the verifier configuration: |
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.
will the filtering capability be available for both k8s and cli scenario.
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.
No, it should be available on both methods, the examples for K8s are perhaps just out of lazyness :)
I'll update the proposal to include examples of CLI configurations.
Hi @asafalgawi , thank you for the proposal. We have discussed this during the community meeting. We agree with the "latest" vulnerability report user scenario, however we need to understand the feasibility and scope of change. @binbin-li brought up good point today, the individual verifier might not have filtering abilities. Major refactoring might be required in executor code path. We still require more time to understand the notary scenarios a bit more, thankyou for your patience. |
Hi @susanshi, thanks for the reviewing the proposal and for the update :) |
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.
Thanks @asafalgawi . I left some comments.
## Problem/Motivation | ||
|
||
When configuring a verifier in Ratify, we set the artifact type the verifier should work on. In such case, Ratify will verify all referrers of a given subject that have a matching artifact type using the verifier. | ||
In some cases, this could lead to a wrong behavior. For instance, Vulnerability Artifacts are outdated once a new artifact is written to the repository, as such there is no use for verifying both the new one and the old one. On the other hand, in performing signature verification using Notary, we may wish to attest more than one signature, because there may be multiple signatures attached to a given artifact, but our "tenant" only trusts a subset of them. |
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.
In some cases, this could lead to a wrong behavior. For instance, Vulnerability Artifacts are outdated once a new artifact is written to the repository, as such there is no use for verifying both the new one and the old one. On the other hand, in performing signature verification using Notary, we may wish to attest more than one signature, because there may be multiple signatures attached to a given artifact, but our "tenant" only trusts a subset of them. | |
In some cases, this could lead to a wrong behavior. For instance, Vulnerability Artifacts are outdated once a new artifact is written to the repository, as such there is no use for verifying both the new one and the old one. On the other hand, in performing Notary Project signature verification, we may wish to attest more than one signature, because there may be multiple signatures attached to a given artifact, but our "tenant" only trusts a subset of them. |
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.
The current behavior of Notary Project signature verification is:
- if 1 out of N signatures passes verification, then the overall signature verification succeeds.
- If all signatures failed validation, then the overall signature verification fails
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.
@binbin-li If I remembered correctly, verifying multiple signatures can be achieved through Rego policies?
"digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b864", | ||
"signatures": [ | ||
{ | ||
"mediaType": "applicaiton/vnd.cncf.notary.x509-signature.config.v2", |
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.
Is this media type applicaiton/vnd.cncf.notary.x509-signature.config.v2
correct?
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.
I copied it from their website, perhaps it is not up to date, i'll check again and update the doc.
|
||
# Filtration Methods | ||
|
||
## Age Based Filtration |
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.
This is still annotation-based filtration, and it just uses the annotation org.opencontainers.image.created
to achieve the goal. So I think there is only one filtration method, but currently there are two scenarios, one is for filtering out latest artifact, another is for filtering out a subset of artifacts.
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.
It is, but the verification process of both is different.
Age based verification requires context, you need to be aware of all the referrers, while annotation based in specific to the artifact that is currently being verified.
While age based has to be implemented in the executor, the annotation filtering can be done even in the skel method of the verifier, it could return a new type of response to show that verification was skipped and the artifact won't be included in the final verification response.
Perhaps we should not bundle both features in the same proposal ?
``` | ||
|
||
We could define a notation verifier configuration that would require the artifact manifest to contain the annotation `org.opencontainers.image.signature.fingerprint` and that is should have a known fingerprint value. this would enable a scenario where the user configures three notation verifiers, one in each namespace: | ||
* Namepsace A -> Fingerprint should match [X] |
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.
Could you elaborate more about the problem that fingerprint filtering is going to be solved? If it is to distinguish different signers, we can configure Notation Verifier with different trusted identities for different namespaces. For example, images pulled in namespace A are signed by Signer-A, then we can configure the trusted identities that Signer-A used for namespace A.
Or is your scenario to distinguish signatures that are generated by the same Signer (trusted identity)? Then what is the difference in signatures generated by the same Signer? If it is about age, maybe we can use aged-filtering
to verify only the latest signature?
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.
Sure. Basically, I referred to a scenario in which I as a namespace owner I require that all certificate chains will contains a specific signer, since the signature chain is part of the artifact annotations, I can verify it before even going through all whole process of verifying the actual signature.
Description
What this PR does / why we need it:
This PR adds a proposal for filtering artifacts based on their annotations before forwarding them to verification by one of the configured verifiers.]
The PR refers to issue #1772
Type of change
Added document to proposal directory.