Skip to content
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

Improve error message of signature verification #1321

Closed
7 tasks
yizha1 opened this issue Mar 1, 2024 · 2 comments
Closed
7 tasks

Improve error message of signature verification #1321

yizha1 opened this issue Mar 1, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@yizha1
Copy link
Collaborator

yizha1 commented Mar 1, 2024

What would you like to be added?

The error message of signature verification is not concise and actionable, see example,

Example1:
"verification failed: Error: referrers not found, Code: REFERRERS_NOT_FOUND, Component Type: executor"

Example2:
"Original Error: (Original Error: (signature is not produced by a trusted signer), Error: verify signature failure, Code: VERIFY_SIGNATURE_FAILURE, Plugin Name: notation, Component Type: verifier, Documentation: https://github.com/notaryproject/notaryproject/tree/main/specs, Detail: failed to verify signature of digest), Error: verify reference failure, Code: VERIFY_REFERENCE_FAILURE, Plugin Name: notation, Component Type: verifier"

It's hard to understand what happened, what happened and what needs to be done. There is a need for enhancements in the error messages related to signature verification. We can test the common scenarios for both Notary Project signatures and Cosign signatures, and check whether error messages are concise, precise, and actionable

  • Images are not signed
  • Users do not have right roles assigned to access AKV (Notary Project signatures)
  • Users do not have right roles assigned to access AKV (Cosign signatures)
  • Users cannot access registries
  • Images are signed with expected identities/keys that should be able to verify, however, verification configurations are wrong (both Notary Project signatures and Cosign signatures)
  • Images are signed by unknown identities so that verification should fail. (Notary Project signatures)
  • Images are signed by unknown keys so that verification should fail (Cosign key-pair signatures)
  • Images are signed with revoked certificates
  • Images are signed before certificates are expired (Time-stamp support is required and users do not specify TSA root certificate)
  • Images are signed before certificates are expired (Time-stamp support is required and users specify TSA root certificate)

Anything else you would like to add?

Discussed with Yi offline, we have more scenarios need to be covered, check this doc for more details: https://hackmd.io/@H7a8_rG4SuaKwzu4NLT-9Q/HkFHgokv0#User-Scenarios

Work item break down:

  • KMP configuration
  • Store configuration
  • Verifier configuration
  • Policy configuration
  • Access control to ACR/AKV
  • Signature verification

Are you willing to submit PRs to contribute to this feature?

  • Yes, I am willing to implement it.
@yizha1 yizha1 added enhancement New feature or request triage Needs investigation labels Mar 1, 2024
@akashsinghal akashsinghal added this to the v1.3.0 milestone May 16, 2024
@susanshi susanshi removed the triage Needs investigation label May 23, 2024
@binbin-li
Copy link
Collaborator

Scenarios listed in the doc: https://hackmd.io/@H7a8_rG4SuaKwzu4NLT-9Q/HkFHgokv0#User-Scenarios

Work item break down:

  • KMP configuration
  • Store configuration
  • Verifier configuration
  • Policy configuration
  • Access control to ACR/AKV
  • Signature verification

@binbin-li
Copy link
Collaborator

Scenarios listed in the doc: https://hackmd.io/@H7a8_rG4SuaKwzu4NLT-9Q/HkFHgokv0#User-Scenarios

Work item break down:

  • KMP configuration
  • Store configuration
  • Verifier configuration
  • Policy configuration
  • Access control to ACR/AKV
  • Signature verification

After discussion with @yizha1, we have proposed some overall improvements to the current error handling framework.

  1. Users don't need to know everything about an error(the complete stack trace), they would just need the root cause of the error.
  2. We can introduce a new field(reason) to the verifierReports besides message. The message will indicate a general error message, while the reason explaining the root cause.
  3. Probably log the error msg at each level instead of logging a wrapped error on the top.
  4. Probably add an error field to all CR structs. When an error happens while reconciling, we'll add the error. Then during the artifact validation, executor could fetch the error from reconciling.
  5. Consolidate the verifierReports format between configPolicy and regoPolicy.
    a. Add artifactType to each level.
    b. Have message and reason fields
    c. Reduce confusion between type and artifact-type
  6. Refactor the nested error structure to make it well formatted while keeping info to users/devs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants