Content-Length: 557700 | pFad | http://github.com/sigstore/protobuf-specs/pull/520/commits/d55635c2fb168eb742faa3b03f4b81a45809ffca

70 Add Result message to communicate the result of a verification by codysoyland · Pull Request #520 · sigstore/protobuf-specs · GitHub
Skip to content

Add Result message to communicate the result of a verification #520

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add Result message to communicate the result of a verification
  • Loading branch information
codysoyland committed Jan 30, 2025
commit d55635c2fb168eb742faa3b03f4b81a45809ffca
161 changes: 161 additions & 0 deletions protos/sigstore_verification.proto
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,164 @@ message Input {
// provided.
optional Artifact artifact = 4;
}

// CertificateSummary contains verified metadata from the certificate
message CertificateSummary {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should get rid of CertificateIdentity? I prefer this new message as clients shouldn't need to know OID values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They serve different different purposes, the CertificateIdentity is for verifying, as it allows regexps to be set for the SAN.

And it also allows for setting the extensions needed during verification. I agree that clients knowing with OIDs to use os not optimal, but clients can implement helper function here to configure the verification.

// The issuer of the certificate
string certificate_issuer = 1;

// The Subject Alternative Name, or SAN, from the certificate
string subject_alternative_name = 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not be an issue in practice, but flagging: X.509 technically has multiple SANs, and I don't think any of the Sigstore specifications/documentation explicitly say that Sigstore leaf certificates only have a single SAN. In practice this is true, but we might want to flag some documentation improvements as a follow-up to this to emphasize that, as a matter of X.509 profile, Sigstore-compatible CAs only issue EE certs with singular SANs 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should add the type for the SAN too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The certificate spec already mentions this:

https://github.com/sigstore/fulcio/blob/main/docs/certificate-specification.md#issued-certificate

An issued certificate MUST:

Specify exactly one Subject Alternative Name, as a critical extension. It MUST be populated by either:

  • An email
  • A URI


// The OIDC issuer. Should match `iss` claim of ID token or, in the case of
// a federated login like Dex it should match the issuer URL of the
// upstream issuer. The issuer is not set the extensions are invalid and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// upstream issuer. The issuer is not set the extensions are invalid and
// upstream issuer. The issuer is not set if the extensions are invalid and

?

// will fail to render.
// OID 1.3.6.1.4.1.57264.1.8 and 1.3.6.1.4.1.57264.1.1 (Deprecated)
string issuer = 3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we maybe align the field numbers with the last component of the OID?


// Deprecated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecated OIDs, can we ignore them here? They were marked as deprecated two years go (almost to the date) sigstore/fulcio#945.

If a specific client want's to support them, that's fine. But from a standard perspective, I think we should only focus on the current extensions.

// Triggering event of the Github Workflow. Matches the `event_name` claim of ID
// tokens from Github Actions
// OID 1.3.6.1.4.1.57264.1.2
string githubWorkflowTrigger = 4;

// Deprecated
// SHA of git commit being built in Github Actions. Matches the `sha` claim of ID
// tokens from Github Actions
// OID 1.3.6.1.4.1.57264.1.3
string githubWorkflowSHA = 5;

// Deprecated
// Name of Github Actions Workflow. Matches the `workflow` claim of the ID
// tokens from Github Actions
// OID 1.3.6.1.4.1.57264.1.4
string githubWorkflowName = 6;

// Deprecated
// Repository of the Github Actions Workflow. Matches the `repository` claim of the ID
// tokens from Github Actions
// OID 1.3.6.1.4.1.57264.1.5
string githubWorkflowRepository = 7;

// Deprecated
// Git Ref of the Github Actions Workflow. Matches the `ref` claim of the ID tokens
// from Github Actions
// OID 1.3.6.1.4.1.57264.1.6
string githubWorkflowRef = 8;

// Reference to specific build instructions that are responsible for signing.
// OID 1.3.6.1.4.1.57264.1.9
string buildSignerURI = 9;

// Immutable reference to the specific version of the build instructions that is responsible for signing.
// OID 1.3.6.1.4.1.57264.1.10
string buildSignerDigest = 10;

// Specifies whether the build took place in platform-hosted cloud infrastructure or customer/self-hosted infrastructure.
// OID 1.3.6.1.4.1.57264.1.11
string runnerEnvironment = 11;

// Source repository URL that the build was based on.
// OID 1.3.6.1.4.1.57264.1.12
string sourceRepositoryURI = 12;

// Immutable reference to a specific version of the source code that the build was based upon.
// OID 1.3.6.1.4.1.57264.1.13
string sourceRepositoryDigest = 13;

// Source Repository Ref that the build run was based upon.
// OID 1.3.6.1.4.1.57264.1.14
string sourceRepositoryRef = 14;

// Immutable identifier for the source repository the workflow was based upon.
// OID 1.3.6.1.4.1.57264.1.15
string sourceRepositoryIdentifier = 15;

// Source repository owner URL of the owner of the source repository that the build was based on.
// OID 1.3.6.1.4.1.57264.1.16
string sourceRepositoryOwnerURI = 16;

// Immutable identifier for the owner of the source repository that the workflow was based upon.
// OID 1.3.6.1.4.1.57264.1.17
string sourceRepositoryOwnerIdentifier = 17;

// Build Config URL to the top-level/initiating build instructions.
// OID 1.3.6.1.4.1.57264.1.18
string buildConfigURI = 18;

// Immutable reference to the specific version of the top-level/initiating build instructions.
// OID 1.3.6.1.4.1.57264.1.19
string buildConfigDigest = 19;

// Event or action that initiated the build.
// OID 1.3.6.1.4.1.57264.1.20
string buildTrigger = 20;

// Run Invocation URL to uniquely identify the build execution.
// OID 1.3.6.1.4.1.57264.1.21
string runInvocationURI = 21;

// Source repository visibility at the time of signing the certificate.
// OID 1.3.6.1.4.1.57264.1.22
string sourceRepositoryVisibilityAtSigning = 22;
}

message SignatureResult {
// If the bundle contains a public key, the public_key_id field will
// contain the public key ID
string public_key_id = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bundle should contain a public_key_id if a key is to be used to verify it, so is this repetitive? Though maybe this is useful when a keyset is provided and the bundle doesn't contain the ID (since it's an optional hint)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor but this is called hint in the PublicKeyIdentifier message:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be a oneof:

message SignatureResult {
        oneof {
                string public_key_hint = 1;
                CertificateSummary certificate = 2;
        }
}


// If the bundle contains a certificate, the certificate field will
// contain metadata about the certificate
CertificateSummary certificate = 2;
}

message TimestampResult {
// The type of timestamp verification; may be `Tlog`, `TimestampAuthority`, or `CurrentTime`
string type = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should make it a enum


// The URI of the timestamp provider; may contain the base URL of the
// TSA or Tlog that was used to establish a timestamp
string uri = 2;

// The actual timestamp time that was verified
google.protobuf.Timestamp timestamp = 3;
}

message UnverifiedTimestampResult {
// Data about the timestamp that was unverified
TimestampResult result = 1;

// Reason or error text describing why this timestamp was not verified
string reason = 2;
}

message UnverifiedData {
repeated UnverifiedTimestampResult timestamps = 1;
}

// Result captures the result of the verification
message Result {
// MUST be application/vnd.dev.sigstore.verificationresult+json;version=0.1
string media_type = 1;

// MUST be either "success" or "failure"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be an enum?

string status = 2;

// SHOULD contain an error message if status == "failure"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, MUST be empty if status is success?

string error = 3;

// If the bundle contains a DSSE, this result shonuld contain the
// deserialized and verified statement
string statement = 3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bytes instead of string?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comments should be rephrased a bit.

// If the bundle contains DSSE wrapped in-toto statement , this result contains the
// deserialized in-toto statement

As an in-toto statement is a JSON encoded object, i.e. a string.


// Data about the public key or certificate associated with the verified signature
SignatureResult signature = 4;

// Data about verified timestamps
repeated TimestampResult verified_timestamps = 5;

// Any data which was unable to be verified.
UnverifiedData unverified_data = 6;
}
Loading








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/sigstore/protobuf-specs/pull/520/commits/d55635c2fb168eb742faa3b03f4b81a45809ffca

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy