docs: Create proposal for verifying 'last-n' artifacts only.

[asafalgawi]

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.

[susanshi]

if artifactAgeFilter may contain different values based on verifier, should this be moved to "parameters"

[asafalgawi]

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.

[binbin-li]

Basically, age-based filtering is just one special case of annotation-based filtering. It would be better to have a uniform param for the overall filtering feature.

[susanshi]

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.

[asafalgawi]

Hi @susanshi, thanks for the reviewing the proposal and for the update :)

[yizha1]

Thanks @asafalgawi . I left some comments.

[yizha1]

Suggested change

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.

[yizha1]

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

[yizha1]

@binbin-li If I remembered correctly, verifying multiple signatures can be achieved through Rego policies?

[binbin-li]

Both config policy and rego policy can verify multiple signatures but only rego policy engine supports using multiple Notation verifiers validates the same Notation signature.

[yizha1]

Is this media type applicaiton/vnd.cncf.notary.x509-signature.config.v2 correct?

[asafalgawi]

I copied it from their website, perhaps it is not up to date, i'll check again and update the doc.

[yizha1]

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.

[asafalgawi]

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 ?

[akashsinghal]

^^ I think this is a good point. age based filtering vs generic annotation based filtering operate at different levels of Ratify

[binbin-li]

It makes sense to have the overall filtering code in both executor and verifiers in terms of the current Ratify design. I just feel if we need to do some sorting, e.g. latest artifact, it should reside in the corresponding verifier instead of executor since not all verifiers require sorted artifacts and verifier's configuration should not affect executor's behavior.

[yizha1]

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?

[asafalgawi]

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.

[susanshi]

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 @asafalgawi, reviewing our current executor path below, we want to explore a bit more on the proposed implementation. How will the executor be refactored for filtering? Note we will not be able to change the verifier interfaces to maintain backward compatibility. If the current PR review meeting and community meeting time doesn't work, would you be open to meet and discuss in the once off meeting?

for _, referrerStore := range executor.ReferrerStores {
	for _, reference := range referrersResult.Referrers {  
		for _, verifier := range executor.Verifiers {
                }
        }
}

[akashsinghal]

The verifier api includes a CanVerify which is invoked by Ratify's executor prior to even invoking the Verify method, although external plugins will need to be considered differently in that the pre-validation filtering will need to exist in the actual plugin implementation. If the verifier defines it's own behavior for annotation filtering here, then there would be no extra registry request overhead nor executor-level logic required. Age baed filtering can also be done at the CanVerify level if only comparing a specific's artifact age in relation to an absolute timestamp. (Must be newer/older than this timestamp). If relative comparison among artifacts is needed then yes, executor level age filtering is needed.

[junczhu]

^^ I think this is a good point.

[asafalgawi]

@akashsinghal, I agree, having the annotation based filtering in the CanVerify method would offer a clean solution for that part of the proposal. We could also define how to setup the filtering in a descriptive way in the Verifier configuration to enable external plugin to make use of this. Since the CanVerify method is implemented internally in the VerifierPlugin "verifier" in Ratify.

As for age based, I don't see a way without first pulling all the references and sorting them/placing them in some sort of heap to enable fast access to the latest artifact.

[akashsinghal]

What happens when this annotation does not exist? What would be default behavior here?

What if the user has a custom annotation for time? Will there be a way to specify that?

[asafalgawi]

I think the default behavior should be regarding the artifact as the oldest.
As for custom annotations, that could be added to the proposal.
But as far as going with custom date time formatting, I think it is best to formally require the date value to be formatted in specific manner.

[akashsinghal]

off topic but do we know if there are plans (in distribution spec) for registries to allow for annotation filtering?

[asafalgawi]

@susanshi I've updated the proposal based on our conversation, also added two implementation proposal. a "simple" one which theoretically impacts Ratify's scalability, the second, a bit more complex, which affects Ratify executor design

[susanshi]

@susanshi I've updated the proposal based on our conversation, also added two implementation proposal. a "simple" one which theoretically impacts Ratify's scalability, the second, a bit more complex, which affects Ratify executor design

thanks for the updates and comparison between the two implementation methods. My understanding option1 is more straight forward, and given a 1k referrer list, there are no perf and memory concerns. Option 2 memory benefits is unclear. Major refactoring in the executor is risky, unless the refactoring improves code readability /allows for easier future maintaince/ aligns with the direction of ratify v2 executor change. @binbin-li , do you see for executor refactoring benefits for ratify v2?

[susanshi]

@asafalgawi , just a note, we are not a cncf sandbox project and there is a new requirement on DCO. You might need to squash the current commits and sign a new commit with the -s flag , git commit -s -S -m <commit_message>. see details at https://github.com/ratify-project/ratify/blob/dev/CONTRIBUTING.md#commit :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

see 28 files with indirect coverage changes

[binbin-li]

@susanshi I've updated the proposal based on our conversation, also added two implementation proposal. a "simple" one which theoretically impacts Ratify's scalability, the second, a bit more complex, which affects Ratify executor design

thanks for the updates and comparison between the two implementation methods. My understanding option1 is more straight forward, and given a 1k referrer list, there are no perf and memory concerns. Option 2 memory benefits is unclear. Major refactoring in the executor is risky, unless the refactoring improves code readability /allows for easier future maintaince/ aligns with the direction of ratify v2 executor change. @binbin-li , do you see for executor refactoring benefits for ratify v2?

For this feature, I'd like to keep executor untouched. For v2 design, I plan to make executor just handle the core coordination among different components(policy, verifier, store). The last-n feature and possibly more filtering options sound more like a verifier feature, therefore executor is not a good place to handle it.

[akashsinghal]

@asafalgawi I agree with Susan and I think Option 1 seems like a better approach for now. If larger than 1k referrers becomes a user ask, we can revisit this.

[asafalgawi]

Great I'll start with a POC for option 1 Get Outlook for Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Akash Singhal ***@***.***> Sent: Wednesday, October 16, 2024 8:36:54 PM To: ratify-project/ratify ***@***.***> Cc: Asaf Algawi ***@***.***>; Mention ***@***.***> Subject: Re: [ratify-project/ratify] docs: Create proposal for verifying 'last-n' artifacts only. (PR #1797) @asafalgawi<https://github.com/asafalgawi> I agree with Susan and I think Option 1 seems like a better approach for now. If larger than 1k referrers becomes a user ask, we can revisit this. — Reply to this email directly, view it on GitHub<#1797 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BCRNDOFRKWSDBXYYW4SNGNTZ32P3NAVCNFSM6AAAAABOC5QPAGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJXGQ4DIOBUG4>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[susanshi]

approved in PR meeting