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

Chore: Refactor how we handle verification materials #250

Closed
woodruffw opened this issue Oct 13, 2022 · 9 comments · Fixed by #299
Closed

Chore: Refactor how we handle verification materials #250

woodruffw opened this issue Oct 13, 2022 · 9 comments · Fixed by #299
Labels
component:api Public APIs enhancement New feature or request refactoring Refactoring tasks.

Comments

@woodruffw
Copy link
Member

Conceptually, there are three mandatory pieces of sigstore verification material, and one optional piece:

  • The artifact's SHA256 hash (naive datatype: str)
  • The artifact's signature (naive datatype: bytes, or str if base64-encoded)
  • The artifact's PEM-formatted signing certificate (naive datatype: str)
  • (Optionally) the offline Rekor entry (JSON, deserialized to RekorBundle or RekorEntry in Offline Rekor bundle generation and verification #247)

Right now, the verify API takes these as independent parameters, along with some other options (which will grow over time, as we support different configurations and verification policies):

    def verify(
        self,
        input_: bytes,
        certificate: bytes,
        signature: bytes,
        expected_cert_email: Optional[str] = None,
        expected_cert_oidc_issuer: Optional[str] = None,
        offline_rekor_entry: Optional[RekorEntry] = None,
    ) -> VerificationResult:

This is going to become unwieldly, so we should probably rethink the top-level API here.

Related:

@woodruffw woodruffw added enhancement New feature or request refactoring Refactoring tasks. component:api Public APIs labels Oct 13, 2022
@woodruffw
Copy link
Member Author

One thought: a top-level verify API like this might be nice:

def verify(bundle: VerificationBundle, options: VerificationOptions) -> VerificationResult

...where VerificationBundle could be constructed from a Sigstore bundle, from individual inputs, etc., and VerificationOptions could contain settings for the expected OIDC issuer, etc.

@woodruffw
Copy link
Member Author

More thinking out loud: VerificationBundle should be correct by construction, meaning that it should be impossible to create a VerificationBundle that includes an offline RekorEntry without checking that entry's relevancy to the rest of the verification materials.

@woodruffw
Copy link
Member Author

Temporarily blocked on #289 and #288, since we want this functionality in before a more general refactor.

@woodruffw woodruffw added blocked and removed blocked labels Nov 3, 2022
@mayaCostantini
Copy link
Contributor

Was a verification bundle schema already discussed somewhere? I would be interested in taking up this issue if it has no assignee yet.

@di
Copy link
Member

di commented Nov 7, 2022

@mayaCostantini Yes, here: sigstore/cosign#2131

@woodruffw
Copy link
Member Author

Additionally, the protobuf definition for the bundle is being developed here: https://github.com/sigstore/protobuf-specs

@woodruffw
Copy link
Member Author

I'm currently working on this refactor in ww/verify-api-refactor, although those changes won't include the bundle itself -- it'll only concern cleaning up the API surface and making it easier to consume bundles.

@mayaCostantini
Copy link
Contributor

@di @woodruffw Thank you for the info, I will look into it soon 👍

@woodruffw
Copy link
Member Author

Working on this in #299.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api Public APIs enhancement New feature or request refactoring Refactoring tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants